feat: enable auto-sync for file access via Storage Access Framework (SAF). #89 v2

opencloudApp/src/main/java/eu/opencloud/android/presentation/documentsprovider/DocumentsStorageProvider.kt

@@ -62,11 +62,13 @@ import eu.opencloud.android.presentation.documentsprovider.cursors.RootCursor
 import eu.opencloud.android.presentation.documentsprovider.cursors.SpaceCursor
 import eu.opencloud.android.presentation.settings.security.SettingsSecurityFragment.Companion.PREFERENCE_LOCK_ACCESS_FROM_DOCUMENT_PROVIDER
 import eu.opencloud.android.usecases.synchronization.SynchronizeFileUseCase
-import eu.opencloud.android.usecases.synchronization.SynchronizeFolderUseCase
 import eu.opencloud.android.usecases.transfers.downloads.DownloadFileUseCase
+import eu.opencloud.android.usecases.synchronization.SynchronizeFolderUseCase
 import eu.opencloud.android.usecases.transfers.uploads.UploadFilesFromSystemUseCase
 import eu.opencloud.android.utils.FileStorageUtils
 import eu.opencloud.android.utils.NotificationUtils
+import androidx.work.WorkInfo
+import androidx.work.WorkManager
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.launch
@@ -75,7 +77,10 @@ import timber.log.Timber
 import java.io.File
 import java.io.FileNotFoundException
 import java.io.IOException
+import java.util.UUID
 import java.util.Vector
+import java.util.concurrent.ConcurrentHashMap
+import java.util.concurrent.CompletableFuture
 
 class DocumentsStorageProvider : DocumentsProvider() {
     /**
@@ -90,6 +95,16 @@ class DocumentsStorageProvider : DocumentsProvider() {
 
     private lateinit var fileToUpload: OCFile
 
+    // Cache to avoid redundant PROPFINDs when apps (e.g. Google Photos) call
+    // openDocument many times for the same file. Two layers:
+    // 1. In-flight dedup: concurrent calls for the same file share one PROPFIND via
+    //    a CompletableFuture. The first caller does the actual work, others wait.
+    // 2. TTL cache: after a sync completes, skip re-checking the same file for a
+    //    few seconds to handle rapid sequential calls.
+    private val inFlightSyncs = ConcurrentHashMap<Long, CompletableFuture<SynchronizeFileUseCase.SyncType?>>()
+    private var propfindCacheFileId: Long? = null
+    private var propfindCacheTimestamp: Long = 0
+
     override fun openDocument(
         documentId: String,
         mode: String,
@@ -108,17 +123,79 @@ class DocumentsStorageProvider : DocumentsProvider() {
             ocFile = getFileByIdOrException(documentId.toInt())
 
             if (!ocFile.isAvailableLocally) {
+                // File has never been downloaded. Enqueue the download directly —
+                // no need for a PROPFIND since we already know we need the file.
                 val downloadFileUseCase: DownloadFileUseCase by inject()
+                val workerId = downloadFileUseCase(
+                    DownloadFileUseCase.Params(accountName = ocFile.owner, file = ocFile)
+                )
+                if (!waitForDownload(workerId, documentId.toInt(), signal)) {
+                    return null
+                }
+                ocFile = getFileByIdOrException(documentId.toInt())
+                if (!ocFile.isAvailableLocally) {
+                    return null
+                }
+                // Seed the TTL cache — the file was just downloaded, so there's no need
+                // for a PROPFIND if Google Photos immediately calls openDocument again.
+                propfindCacheFileId = ocFile.id
+                propfindCacheTimestamp = System.currentTimeMillis()
+            } else if (!isWrite) {
+                // File is available locally and opened for reading. Check with the server
+                // (PROPFIND) whether a newer version exists, and download it if so.
+                //
+                // Apps like Google Photos call openDocument many times concurrently for
+                // the same file. Without dedup, each call does its own PROPFIND, and due
+                // to the synchronized lock in OpenCloudClient.executeHttpMethod they
+                // serialize — causing 10+ second waits per extra call. We handle this
+                // with two layers:
+                // 1. TTL cache: skip if we just confirmed this file is up-to-date.
+                // 2. In-flight dedup: concurrent calls share one PROPFIND result.
+                val fileId = ocFile.id!!
+                val now = System.currentTimeMillis()
+                if (fileId == propfindCacheFileId && now - propfindCacheTimestamp <= PROPFIND_CACHE_TTL_MS) {
+                    Timber.d("Skipping PROPFIND for file $fileId, recently synced ${now - propfindCacheTimestamp}ms ago")
+                } else {
+                    val syncResult = syncFileWithServerCoalesced(ocFile)
+
+                    when (syncResult) {
+                        is SynchronizeFileUseCase.SyncType.AlreadySynchronized -> {
+                            // File is up to date, nothing to wait for.
+                        }
+                        is SynchronizeFileUseCase.SyncType.DownloadEnqueued -> {
+                            // A newer version exists. SynchronizeFileUseCase only enqueues
+                            // a WorkManager download, it does not wait for it to finish.
+                            if (!waitForDownload(syncResult.workerId, documentId.toInt(), signal)) {
+                                return null
+                            }
+                        }
+                        is SynchronizeFileUseCase.SyncType.ConflictDetected -> {
+                            // File changed both locally and remotely. Notify the user and
+                            // serve the local version (same behavior as before).
+                            context?.let {
+                                NotificationUtils.notifyConflict(fileInConflict = ocFile, context = it)
+                            }
+                        }
+                        is SynchronizeFileUseCase.SyncType.FileNotFound -> {
+                            return null
+                        }
+                        is SynchronizeFileUseCase.SyncType.UploadEnqueued -> {
+                            // Local file is newer, upload was enqueued. Serve the local version.
+                        }
+                        null -> {
+                            // Sync failed, serve the local version anyway.
+                        }
+                    }
 
-                downloadFileUseCase(DownloadFileUseCase.Params(accountName = ocFile.owner, file = ocFile))
+                    propfindCacheFileId = fileId
+                    propfindCacheTimestamp = System.currentTimeMillis()
 
-                do {
-                    if (!waitOrGetCancelled(signal)) {
+                    // Re-read the file from DB to get the updated state after download.
+                    ocFile = getFileByIdOrException(documentId.toInt())
+                    if (!ocFile.isAvailableLocally) {
                         return null
                     }
-                    ocFile = getFileByIdOrException(documentId.toInt())
-
-                } while (!ocFile.isAvailableLocally)
+                }
             }
         } else {
             ocFile = fileToUpload
@@ -150,23 +227,7 @@ class DocumentsStorageProvider : DocumentsProvider() {
                             uploadFilesUseCase(uploadFilesUseCaseParams)
                         }
                     } else {
-                        Thread {
-                            val synchronizeFileUseCase: SynchronizeFileUseCase by inject()
-                            val result = synchronizeFileUseCase(
-                                SynchronizeFileUseCase.Params(
-                                    fileToSynchronize = ocFile,
-                                )
-                            )
-                            Timber.d("Synced ${ocFile.remotePath} from ${ocFile.owner} with result: $result")
-                            if (result.getDataOrNull() is SynchronizeFileUseCase.SyncType.ConflictDetected) {
-                                context?.let {
-                                    NotificationUtils.notifyConflict(
-                                        fileInConflict = ocFile,
-                                        context = it
-                                    )
-                                }
-                            }
-                        }.start()
+                        syncFileWithServerAsync(ocFile)
                     }
                 }
             } catch (e: IOException) {
@@ -488,6 +549,134 @@ class DocumentsStorageProvider : DocumentsProvider() {
         return NONEXISTENT_DOCUMENT_ID
     }
 
+    /**
+     * Synchronize a file with the server, coalescing concurrent requests.
+     *
+     * If another thread is already syncing this file, we wait for its result instead of
+     * starting a second PROPFIND. This avoids the serialized lock contention in
+     * OpenCloudClient.executeHttpMethod when multiple binder threads call openDocument
+     * for the same file simultaneously.
+     *
+     * The future is always removed from [inFlightSyncs] when done (via finally),
+     * so errors or timeouts cannot leave stale entries that would block future syncs.
+     */
+    private fun syncFileWithServerCoalesced(fileToSync: OCFile): SynchronizeFileUseCase.SyncType? {
+        val fileId = fileToSync.id!!
+        val newFuture = CompletableFuture<SynchronizeFileUseCase.SyncType?>()
+        val existingFuture = inFlightSyncs.putIfAbsent(fileId, newFuture)
+
+        if (existingFuture != null) {
+            // Another thread is already syncing this file. Wait for its result.
+            Timber.d("Sync for file $fileId already in flight, waiting for result")
+            return try {
+                existingFuture.get()
+            } catch (e: Exception) {
+                Timber.w(e, "In-flight sync for file $fileId failed, serving local version")
+                null
+            }
+        }
+
+        // We are the first thread — do the actual PROPFIND.
+        return try {
+            val result = syncFileWithServerBlocking(fileToSync)
+            newFuture.complete(result)
+            result
+        } catch (e: Exception) {
+            newFuture.completeExceptionally(e)
+            throw e
+        } finally {
+            inFlightSyncs.remove(fileId)
+        }
+    }
+
+    /**
+     * Synchronize a file with the server and return the result.
+     * Runs synchronously on the calling thread (blocks until the PROPFIND completes).
+     * Note: if a download is needed, this only *enqueues* it — use [waitForDownload] to
+     * wait for the actual download to finish.
+     */
+    private fun syncFileWithServerBlocking(fileToSync: OCFile): SynchronizeFileUseCase.SyncType? {
+        Timber.d("Trying to sync file ${fileToSync.id} with server (blocking)")
+
+        val synchronizeFileUseCase: SynchronizeFileUseCase by inject()
+        val useCaseResult = synchronizeFileUseCase(
+            SynchronizeFileUseCase.Params(fileToSynchronize = fileToSync)
+        )
+        Timber.d("${fileToSync.remotePath} from ${fileToSync.owner} synced with result: $useCaseResult")
+
+        return useCaseResult.getDataOrNull()
+    }
+
+    /**
+     * Fire-and-forget sync: used in the close handler after writes,
+     * where we don't need to wait for the result.
+     */
+    private fun syncFileWithServerAsync(fileToSync: OCFile) {
+        Timber.d("Trying to sync file ${fileToSync.id} with server (async)")
+
+        val synchronizeFileUseCase: SynchronizeFileUseCase by inject()
+        CoroutineScope(Dispatchers.IO).launch {
+            val useCaseResult = synchronizeFileUseCase(
+                SynchronizeFileUseCase.Params(fileToSynchronize = fileToSync)
+            )
+            Timber.d("${fileToSync.remotePath} from ${fileToSync.owner} synced with result: $useCaseResult")
+
+            if (useCaseResult.getDataOrNull() is SynchronizeFileUseCase.SyncType.ConflictDetected) {
+                context?.let {
+                    NotificationUtils.notifyConflict(fileInConflict = fileToSync, context = it)
+                }
+            }
+        }
+    }
+
+    /**
+     * Wait for a download to finish.
+     *
+     * If [workerId] is non-null, we use WorkManager to wait directly for that specific job.
+     * If [workerId] is null, it means a download for this file was already in progress
+     * (enqueued by a previous call), so we fall back to polling the DB until the file
+     * becomes available locally.
+     *
+     * Note: openDocument can be called concurrently on multiple binder threads for the
+     * same file (e.g. the calling app retries or requests the file multiple times).
+     * The first call enqueues the download and gets a workerId; subsequent concurrent
+     * calls get null (DownloadFileUseCase deduplicates) and use the polling fallback.
+     *
+     * @return true if the file is ready, false if cancelled.
+     */
+    private fun waitForDownload(workerId: UUID?, fileId: Int, signal: CancellationSignal?): Boolean {
+        if (workerId != null) {
+            // Poll WorkManager until this specific job reaches a terminal state.
+            // Note: getWorkInfoById().get() returns the *current* state immediately,
+            // it does NOT block until the work finishes.
+            Timber.d("Waiting for download worker $workerId to finish")
+            val workManager = WorkManager.getInstance(context!!)
+            do {
+                if (!waitOrGetCancelled(signal)) {
+                    return false
+                }
+                val workInfo = workManager.getWorkInfoById(workerId).get()
+                Timber.d("Download worker $workerId state: ${workInfo.state}")
+                when (workInfo.state) {
+                    WorkInfo.State.SUCCEEDED -> return true
+                    WorkInfo.State.FAILED, WorkInfo.State.CANCELLED -> return false
+                    else -> { /* ENQUEUED, RUNNING, BLOCKED — keep waiting */ }
+                }
+            } while (true)
+        }
+
+        // workerId is null — a download was already in progress from a previous request.
+        // Poll until the file appears locally, checking for cancellation each second.
+        Timber.d("Download already in progress for file $fileId, polling until available")
+        do {
+            if (!waitOrGetCancelled(signal)) {
+                return false
+            }
+            val file = getFileByIdOrException(fileId)
+            if (file.isAvailableLocally) return true
+        } while (true)
+    }
+
     private fun syncDirectoryWithServer(parentDocumentId: String) {
         Timber.d("Trying to sync $parentDocumentId with server")
         val folderToSync = getFileByIdOrException(parentDocumentId.toInt())
@@ -583,5 +772,6 @@ class DocumentsStorageProvider : DocumentsProvider() {
 
     companion object {
         const val NONEXISTENT_DOCUMENT_ID = "-1"
+        const val PROPFIND_CACHE_TTL_MS = 3000L
     }
 }

opencloudApp/src/main/java/eu/opencloud/android/workers/DownloadFileWorker.kt

@@ -205,14 +205,24 @@ class DownloadFileWorker(
      * We will update info about local storage (where it was stored and its size)
      */
     private fun updateDatabaseWithLatestInfoForThisFile() {
+        val finalFile = File(finalLocationForFile)
         val currentTime = System.currentTimeMillis()
         ocFile.apply {
             needsToUpdateThumbnail = true
             modificationTimestamp = downloadRemoteFileOperation.modificationTimestamp
             etag = downloadRemoteFileOperation.etag
             storagePath = finalLocationForFile
-            length = (File(finalLocationForFile).length())
-            lastSyncDateForData = currentTime
+            length = finalFile.length()
+            // Use the file's actual mtime, not the current time. SynchronizeFileUseCase
+            // compares lastSyncDateForData against the file's filesystem mtime to detect
+            // local modifications. Using currentTime here can be slightly earlier than the
+            // file's mtime (due to write time and second-precision rounding), which causes
+            // a false "changed locally" detection and an unnecessary upload.
+            // This mainly affects SAF (DocumentsStorageProvider) where apps like Google
+            // Photos call openDocument repeatedly right after download, triggering the
+            // false positive immediately. In normal app usage, the next sync is typically
+            // much later and the small timestamp difference doesn't cause issues.
+            lastSyncDateForData = finalFile.lastModified()
             modifiedAtLastSyncForData = downloadRemoteFileOperation.modificationTimestamp
             lastUsage = currentTime
         }