From 5bbf83a3d1ae95c31da427102dbfd5722cfdfae4 Mon Sep 17 00:00:00 2001 From: Oskars Pucka <89988568+oskarsss@users.noreply.github.com> Date: Thu, 8 Oct 2026 20:17:22 +0300 Subject: [PATCH] fix(sync): queue recursive ongoing folder synchronization Visit cached descendants even when their parent ETag is unchanged and persist newly discovered folders before visiting their children. Process missing files sequentially within the foreground two-way sync worker. Keep manual service dispatch, propagate transfer errors to the root, check ongoing constraints between files, and save completion status using fresh folder metadata. Cover traversal, cancellation, error handling, operation reuse, and worker completion with unit tests. Assisted-by: Codex:gpt-6.1-sol Signed-off-by: Oskars Pucka <89988568+oskarsss@users.noreply.github.com> --- .../client/jobs/InternalTwoWaySyncWork.kt | 117 ++++-- .../operations/SynchronizeFileOperation.kt | 37 +- .../SynchronizeFolderOperation.java | 385 +++++++++++------- .../client/jobs/InternalTwoWaySyncWorkTest.kt | 126 ++++++ .../operations/SynchronizeFileDownloadTest.kt | 117 ++++++ .../SynchronizeFolderOperationTest.kt | 180 ++++++++ .../SynchronizeFolderOperationTestFixture.kt | 197 +++++++++ .../SynchronizeFolderTwoWaySyncTest.kt | 81 ++++ 8 files changed, 1031 insertions(+), 209 deletions(-) create mode 100644 app/src/test/java/com/nextcloud/client/jobs/InternalTwoWaySyncWorkTest.kt create mode 100644 app/src/test/java/com/owncloud/android/operations/SynchronizeFileDownloadTest.kt create mode 100644 app/src/test/java/com/owncloud/android/operations/SynchronizeFolderOperationTest.kt create mode 100644 app/src/test/java/com/owncloud/android/operations/SynchronizeFolderOperationTestFixture.kt create mode 100644 app/src/test/java/com/owncloud/android/operations/SynchronizeFolderTwoWaySyncTest.kt diff --git a/app/src/main/java/com/nextcloud/client/jobs/InternalTwoWaySyncWork.kt b/app/src/main/java/com/nextcloud/client/jobs/InternalTwoWaySyncWork.kt index 4016d55010f0..8d2673cc1948 100644 --- a/app/src/main/java/com/nextcloud/client/jobs/InternalTwoWaySyncWork.kt +++ b/app/src/main/java/com/nextcloud/client/jobs/InternalTwoWaySyncWork.kt @@ -10,17 +10,22 @@ package com.nextcloud.client.jobs import android.content.Context import androidx.work.Worker import androidx.work.WorkerParameters +import com.nextcloud.client.account.User import com.nextcloud.client.account.UserAccountManager import com.nextcloud.client.device.PowerManagementService +import com.nextcloud.client.jobs.notification.WorkerNotificationManager import com.nextcloud.client.network.ConnectivityService import com.nextcloud.client.preferences.AppPreferences import com.owncloud.android.MainApp +import com.owncloud.android.R import com.owncloud.android.datamodel.FileDataStorageManager import com.owncloud.android.datamodel.OCFile import com.owncloud.android.lib.common.utils.Log_OC import com.owncloud.android.operations.SynchronizeFolderOperation +import com.owncloud.android.ui.notifications.NotificationUtils import com.owncloud.android.utils.FileStorageUtils import java.io.File +import java.util.concurrent.TimeUnit @Suppress("Detekt.NestedBlockDepth", "ReturnCount", "LongParameterList") class InternalTwoWaySyncWork( @@ -31,7 +36,10 @@ class InternalTwoWaySyncWork( private val connectivityService: ConnectivityService, private val appPreferences: AppPreferences ) : Worker(context, params) { + @Volatile private var shouldRun = true + + @Volatile private var operation: SynchronizeFolderOperation? = null override fun doWork(): Result { @@ -39,14 +47,16 @@ class InternalTwoWaySyncWork( var result = true - @Suppress("ComplexCondition") - if (!appPreferences.isTwoWaySyncEnabled || - powerManagementService.isPowerSavingEnabled || - !connectivityService.isConnected || - connectivityService.isInternetWalled() || - !connectivityService.connectivity.isWifi - ) { - Log_OC.d(TAG, "Not starting due to constraints!") + val constraint = when { + !appPreferences.isTwoWaySyncEnabled -> "disabled" + powerManagementService.isPowerSavingEnabled -> "power-saving" + !connectivityService.isConnected -> "disconnected" + !connectivityService.connectivity.isWifi -> "not-wifi" + connectivityService.isInternetWalled() -> "server-unavailable" + else -> null + } + if (constraint != null) { + Log_OC.i(TAG, "SyncTrace skipped reason=$constraint") return Result.success() } @@ -66,34 +76,8 @@ class InternalTwoWaySyncWork( return checkFreeSpaceResult } - Log_OC.d(TAG, "Folder ${folder.remotePath}: started!") - operation = - SynchronizeFolderOperation( - context, - folder.remotePath, - user, - fileDataStorageManager, - false, - true - ) - val operationResult = operation?.execute(context) - - if (operationResult?.isSuccess == true) { - Log_OC.d(TAG, "Folder ${folder.remotePath}: finished!") - } else { - Log_OC.d(TAG, "Folder ${folder.remotePath} failed!") - result = false - } - - folder.apply { - operationResult?.let { - internalFolderSyncResult = it.code.toString() - } - - internalFolderSyncTimestamp = System.currentTimeMillis() - } - - fileDataStorageManager.saveFile(folder) + if (!startForeground(folder)) return Result.failure() + if (!synchronizeFolder(folder, user, fileDataStorageManager)) result = false } } @@ -106,6 +90,48 @@ class InternalTwoWaySyncWork( } } + private fun synchronizeFolder(folder: OCFile, user: User, fileDataStorageManager: FileDataStorageManager): Boolean { + val startedAt = System.nanoTime() + Log_OC.i(TAG, "SyncTrace root start folder=${folder.remotePath}") + operation = + SynchronizeFolderOperation.forOngoingSync( + context, + folder.remotePath, + user, + fileDataStorageManager + ) { canContinueSynchronization() } + if (!shouldRun) { + operation?.cancel() + return false + } + val operationResult = operation?.execute(context) + val elapsedMs = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - startedAt) + Log_OC.i( + TAG, + "SyncTrace root end folder=${folder.remotePath} elapsedMs=$elapsedMs " + + "result=${operationResult?.code}" + ) + + if (operationResult?.isSuccess == true) { + Log_OC.d(TAG, "Folder ${folder.remotePath}: finished!") + } else { + Log_OC.d(TAG, "Folder ${folder.remotePath} failed!") + } + + fileDataStorageManager.getFileByEncryptedRemotePath(folder.remotePath)?.let { currentFolder -> + operationResult?.let { + currentFolder.internalFolderSyncResult = it.code.toString() + } + currentFolder.internalFolderSyncTimestamp = System.currentTimeMillis() + fileDataStorageManager.saveFile(currentFolder) + } + return operationResult?.isSuccess == true + } + + private fun canContinueSynchronization(): Boolean = shouldRun && appPreferences.isTwoWaySyncEnabled && + !powerManagementService.isPowerSavingEnabled && connectivityService.isConnected && + connectivityService.connectivity.isWifi + override fun onStopped() { Log_OC.d(TAG, "OnStopped of worker called!") operation?.cancel() @@ -113,6 +139,24 @@ class InternalTwoWaySyncWork( super.onStopped() } + @Suppress("TooGenericExceptionCaught") + private fun startForeground(folder: OCFile): Boolean = try { + val notifications = WorkerNotificationManager( + SYNC_NOTIFICATION_ID, + context, + null, + R.string.folder_download_worker_ticker_id, + NotificationUtils.NOTIFICATION_CHANNEL_DOWNLOAD + ) + val notification = notifications.createSilentNotification(folder.fileName, R.drawable.ic_sync) + setForegroundAsync(notifications.getForegroundInfo(notification)).get() + true + } catch (e: Exception) { + if (e is InterruptedException) Thread.currentThread().interrupt() + Log_OC.e(TAG, "Could not start foreground folder synchronization", e) + false + } + @Suppress("TooGenericExceptionCaught") private fun checkFreeSpace(folder: OCFile): Result? { val storagePath = folder.storagePath ?: MainApp.getStoragePath() @@ -139,6 +183,7 @@ class InternalTwoWaySyncWork( } companion object { + private const val SYNC_NOTIFICATION_ID = 392 const val TAG = "InternalTwoWaySyncWork" } } diff --git a/app/src/main/java/com/owncloud/android/operations/SynchronizeFileOperation.kt b/app/src/main/java/com/owncloud/android/operations/SynchronizeFileOperation.kt index 345c56dd882b..81331a2f06aa 100644 --- a/app/src/main/java/com/owncloud/android/operations/SynchronizeFileOperation.kt +++ b/app/src/main/java/com/owncloud/android/operations/SynchronizeFileOperation.kt @@ -104,7 +104,6 @@ class SynchronizeFileOperation : SyncOperation { val result = if (localFile?.isDown == false) { requestForDownload(localFile) - RemoteOperationResult(RemoteOperationResult.ResultCode.OK) } else { syncWithServer(client) } @@ -192,11 +191,12 @@ class SynchronizeFileOperation : SyncOperation { return RemoteOperationResult(RemoteOperationResult.ResultCode.OK) } + @Suppress("ReturnCount") private fun handleServerChange(serverFile: OCFile): RemoteOperationResult<*> { localFile?.remoteId = serverFile.remoteId if (syncFileContents) { - requestForDownload(localFile) + return requestForDownload(localFile) } else { val local = localFile ?: return RemoteOperationResult(RemoteOperationResult.ResultCode.LOCAL_FILE_NOT_FOUND) @@ -231,29 +231,28 @@ class SynchronizeFileOperation : SyncOperation { transferWasRequested = true } - private fun requestForDownload(file: OCFile?) { - val file = file ?: return + @Suppress("TooGenericExceptionCaught", "ReturnCount") + private fun requestForDownload(file: OCFile?): RemoteOperationResult<*> { + val file = file ?: return RemoteOperationResult(RemoteOperationResult.ResultCode.LOCAL_FILE_NOT_FOUND) val fileDownloadHelper = FileDownloadHelper.instance() if (useWorkerWithNotification) { - Log_OC.d(TAG, "downloading file with worker: ${file.fileName}") fileDownloadHelper.downloadFile(user, file) transferWasRequested = true - } else { - Log_OC.d(TAG, "downloading file without worker: ${file.fileName}") - runCatching { - val operation = DownloadFileOperation(user, file, context) - val result = operation.execute(client) - transferWasRequested = true - if (result.isSuccess) { - fileDownloadHelper.saveFile(file, operation, storageManager) - Log_OC.d(TAG, "requestForDownload completed for: ${file.fileName}") - } else { - Log_OC.d(TAG, "requestForDownload failed for: ${file.fileName}") - } - }.onFailure { e -> - Log_OC.d(TAG, "Exception caught at requestForDownload: $e") + return RemoteOperationResult(RemoteOperationResult.ResultCode.OK) + } + + return try { + val operation = DownloadFileOperation(user, file, context) + val result = operation.execute(client) + transferWasRequested = true + if (result.isSuccess) { + fileDownloadHelper.saveFile(file, operation, storageManager) } + result + } catch (e: Exception) { + Log_OC.e(TAG, "Inline download failed for ${file.remotePath}", e) + RemoteOperationResult(e) } } diff --git a/app/src/main/java/com/owncloud/android/operations/SynchronizeFolderOperation.java b/app/src/main/java/com/owncloud/android/operations/SynchronizeFolderOperation.java index 62299ebb96a5..3e386493183a 100644 --- a/app/src/main/java/com/owncloud/android/operations/SynchronizeFolderOperation.java +++ b/app/src/main/java/com/owncloud/android/operations/SynchronizeFolderOperation.java @@ -39,14 +39,21 @@ import com.owncloud.android.utils.MimeTypeUtil; import java.io.File; +import java.util.ArrayDeque; import java.util.ArrayList; +import java.util.Deque; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Set; import java.util.Vector; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.BooleanSupplier; +import java.util.function.Consumer; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import kotlin.Unit; /** @@ -54,8 +61,7 @@ * in a folder identified with its remote path. * Fetches the list and properties of the files contained in the given folder, including their * properties, and updates the local database with them. - * Does NOT enter in the child folders to synchronize their contents also, BUT requests for a new operation instance - * doing so. + * Ongoing synchronization processes descendants on the calling background thread. */ public class SynchronizeFolderOperation extends SyncOperation { @@ -74,17 +80,12 @@ public class SynchronizeFolderOperation extends SyncOperation { private OCFile mLocalFolder; private volatile boolean mIsPartOfInternalTwoWaySync; - - /** Counter of conflicts found between local and remote files */ - private final AtomicInteger mConflictsFound = new AtomicInteger(); - - /** Counter of failed operations in synchronization of kept-in-sync files */ - private final AtomicInteger mFailsInFileSyncsFound = new AtomicInteger(); + private final TwoWaySyncOperation twoWaySyncOperation; /** * 'True' means that the remote folder changed and should be fetched */ - private volatile boolean mRemoteFolderChanged; + private boolean mRemoteFolderChanged; private List mFilesForDirectDownload; // to avoid extra PROPFINDs when there was no change in the folder @@ -92,17 +93,17 @@ public class SynchronizeFolderOperation extends SyncOperation { private List mFilesToSyncContents; // this will be used for every file when 'folder synchronization' replaces 'folder download' - private final List mSubfoldersToSync; - private final AtomicBoolean mCancellationRequested; private final boolean useWorkerWithNotification; private final boolean syncAll; - final FolderDownloadWorkerNotificationManager notificationManager; + private final Consumer subfolderScheduler; + private final BooleanSupplier continueSynchronization; + private final Deque pendingSubfolders = new ArrayDeque<>(); - private final TwoWaySyncOperation twoWaySyncOperation; + final FolderDownloadWorkerNotificationManager notificationManager; /** * Creates a new instance of {@link SynchronizeFolderOperation}. @@ -117,6 +118,29 @@ public SynchronizeFolderOperation(Context context, FileDataStorageManager storageManager, boolean useWorkerWithNotification, boolean syncAll) { + this(context, remotePath, user, storageManager, useWorkerWithNotification, syncAll, + new AtomicBoolean(false), + path -> startSyncFolderOperation(context, path, user, syncAll), () -> true); + } + + public static SynchronizeFolderOperation forOngoingSync(Context context, + String remotePath, + User user, + FileDataStorageManager storageManager, + BooleanSupplier continueSynchronization) { + return new SynchronizeFolderOperation(context, remotePath, user, storageManager, false, true, + new AtomicBoolean(false), null, continueSynchronization); + } + + private SynchronizeFolderOperation(Context context, + String remotePath, + User user, + FileDataStorageManager storageManager, + boolean useWorkerWithNotification, + boolean syncAll, + AtomicBoolean cancellationRequested, + Consumer subfolderScheduler, + BooleanSupplier continueSynchronization) { super(storageManager); mRemotePath = remotePath; @@ -125,10 +149,11 @@ public SynchronizeFolderOperation(Context context, mRemoteFolderChanged = false; mFilesForDirectDownload = new Vector<>(); mFilesToSyncContents = new Vector<>(); - mSubfoldersToSync = new ArrayList<>(); - mCancellationRequested = new AtomicBoolean(false); + mCancellationRequested = cancellationRequested; this.useWorkerWithNotification = useWorkerWithNotification; this.syncAll = syncAll; + this.subfolderScheduler = subfolderScheduler; + this.continueSynchronization = continueSynchronization; notificationManager = new FolderDownloadWorkerNotificationManager(context, false,null); twoWaySyncOperation = new TwoWaySyncOperation(context, user, storageManager, mCancellationRequested); } @@ -141,16 +166,21 @@ public SynchronizeFolderOperation(Context context, */ @Override protected RemoteOperationResult run(OwnCloudClient client) { - RemoteOperationResult result; - mFailsInFileSyncsFound.set(0); - mConflictsFound.set(0); + long startedAt = System.nanoTime(); + RemoteOperationResult result = null; + mFilesForDirectDownload.clear(); + mFilesToSyncContents.clear(); + pendingSubfolders.clear(); + Log_OC.i(TAG, "SyncTrace start folder=" + mRemotePath + " notificationWorker=" + + useWorkerWithNotification + " syncAll=" + syncAll); try { // get locally cached information about folder mLocalFolder = getStorageManager().getFileByPath(mRemotePath); if (mLocalFolder == null) { Log_OC.e(TAG, "Local folder is null, cannot run synchronize folder operation, remote path: " + mRemotePath); - return new RemoteOperationResult<>(ResultCode.FILE_NOT_FOUND); + result = new RemoteOperationResult<>(ResultCode.FILE_NOT_FOUND); + return result; } mIsPartOfInternalTwoWaySync = getStorageManager().isPartOfInternalTwoWaySync(mLocalFolder); @@ -168,27 +198,69 @@ protected RemoteOperationResult run(OwnCloudClient client) { } if (result.isSuccess()) { - syncContents(client); + result = syncContents(client); } } - if (mCancellationRequested.get()) { + if (subfolderScheduler == null && !pendingSubfolders.isEmpty()) { + RemoteOperationResult descendantResult = synchronizeSubfolders(client); + if (result.isSuccess()) { + result = descendantResult; + } + } + + if (isCancellationRequested()) { throw new OperationCancelledException(); } } catch (OperationCancelledException e) { result = new RemoteOperationResult(e); + } finally { + Log_OC.i(TAG, "SyncTrace end folder=" + mRemotePath + " elapsedMs=" + + TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - startedAt) + " directFiles=" + + mFilesForDirectDownload.size() + " contentOperations=" + mFilesToSyncContents.size() + + " result=" + (result == null ? "exception" : result.getCode())); } return result; } + private RemoteOperationResult synchronizeSubfolders(OwnCloudClient client) throws OperationCancelledException { + RemoteOperationResult result = new RemoteOperationResult<>(ResultCode.OK); + Set visited = new HashSet<>(pendingSubfolders.size() + 1, 1.0f); + visited.add(mRemotePath); + + while (!pendingSubfolders.isEmpty()) { + if (isCancellationRequested()) { + throw new OperationCancelledException(); + } + String path = pendingSubfolders.removeLast(); + if (!visited.add(path)) { + continue; + } + + SynchronizeFolderOperation child = new SynchronizeFolderOperation( + mContext, path, user, getStorageManager(), false, syncAll, + mCancellationRequested, pendingSubfolders::addLast, continueSynchronization + ); + RemoteOperationResult childResult = child.execute(client); + if (result.isSuccess() && !childResult.isSuccess()) { + result = childResult; + } + } + return result; + } + + @SuppressFBWarnings( + value = "AT_STALE_THREAD_WRITE_OF_PRIMITIVE", + justification = "Folder metadata is confined to the executing thread; cancellation uses an atomic flag." + ) private RemoteOperationResult checkForChanges(OwnCloudClient client) throws OperationCancelledException { Log_OC.d(TAG, "Checking changes in " + user.getAccountName() + mRemotePath); mRemoteFolderChanged = true; - if (mCancellationRequested.get()) { + if (isCancellationRequested()) { throw new OperationCancelledException(); } @@ -225,7 +297,7 @@ private RemoteOperationResult checkForChanges(OwnCloudClient client) throws Oper private RemoteOperationResult fetchAndSyncRemoteFolder(OwnCloudClient client) throws OperationCancelledException { - if (mCancellationRequested.get()) { + if (isCancellationRequested()) { throw new OperationCancelledException(); } @@ -236,10 +308,6 @@ private RemoteOperationResult fetchAndSyncRemoteFolder(OwnCloudClient client) th if (result.isSuccess()) { synchronizeData(result.getData()); - if (mConflictsFound.get() > 0 || mFailsInFileSyncsFound.get() > 0) { - result = new RemoteOperationResult<>(ResultCode.SYNC_CONFLICT); - // should be a different result code, but will do the job - } } else { if (result.getCode() == ResultCode.FILE_NOT_FOUND) { removeLocalFolder(); @@ -280,11 +348,7 @@ private void synchronizeData(List folderAndFiles) throws OperationCancel Log_OC.d(TAG, "Remote folder " + mLocalFolder.getRemotePath() + " changed - starting update of local data "); - mFilesForDirectDownload.clear(); - mFilesToSyncContents.clear(); - mSubfoldersToSync.clear(); - - if (mCancellationRequested.get()) { + if (isCancellationRequested()) { throw new OperationCancelledException(); } @@ -353,9 +417,16 @@ private void synchronizeData(List folderAndFiles) throws OperationCancel boolean encrypted = updatedFile.isEncrypted() || mLocalFolder.isEncrypted(); updatedFile.setEncrypted(encrypted); - if (syncFileOrFolder(remoteFile, localFile)) { - updatedFiles.add(updatedFile); + if (mIsPartOfInternalTwoWaySync && twoWaySyncOperation.isDeletedLocally(remoteFile, localFile) && + twoWaySyncOperation.deleteRemoteFile(localFile, getClient())) { + continue; } + + if (!updatedFile.isFolder() && updatedFile.isDown()) { + syncFileOrFolder(remoteFile, localFile == null ? updatedFile : localFile); + } + + updatedFiles.add(updatedFile); } // update file name for encrypted files @@ -372,15 +443,13 @@ private void synchronizeData(List folderAndFiles) throws OperationCancel mLocalFolder.setLastSyncDateForData(System.currentTimeMillis()); storageManager.saveFile(mLocalFolder); - startSubfolderSynchronizations(); - } - - private void startSubfolderSynchronizations() throws OperationCancelledException { - for (String remotePath : mSubfoldersToSync) { - if (mCancellationRequested.get()) { - throw new OperationCancelledException(); + // Child operations need their folder entries to exist in the database before they start. + for (OCFile child : updatedFiles) { + if (child.isFolder()) { + syncFileOrFolder(child, null); + } else if (!child.isDown()) { + mFilesForDirectDownload.add(child); } - startSyncFolderOperation(remotePath); } } @@ -418,70 +487,74 @@ private void updateLocalStateData(OCFile remoteFile, OCFile localFile, OCFile up *

* If the remote file is a regular file, a {@link SynchronizeFileOperation} is created * and added to the list of pending file synchronizations. - * If the remote file is a folder, it is queued for a folder synchronization operation, + * If the remote file is a folder, the method triggers a folder synchronization operation, * which recursively synchronizes all nested files and subfolders. - * Exception: in an internal two-way sync folder, a file or folder the user deleted locally - * is removed from the server instead of being re-downloaded. *

* * @param remoteFile the remote file or folder to synchronize * @param localFile the corresponding local file or folder - * @return {@code false} if {@code remoteFile} was deleted from the server and should no - * longer be tracked locally, {@code true} otherwise + * @throws OperationCancelledException if the synchronization was cancelled */ - private boolean syncFileOrFolder(OCFile remoteFile, OCFile localFile) { - if (mIsPartOfInternalTwoWaySync && twoWaySyncOperation.isDeletedLocally(remoteFile, localFile)) { - return !twoWaySyncOperation.deleteRemoteFile(localFile, getClient()); - } - + private void syncFileOrFolder(OCFile remoteFile, OCFile localFile) throws OperationCancelledException { if (remoteFile.isFolder()) { - mSubfoldersToSync.add(remoteFile.getRemotePath()); - return true; + if (isCancellationRequested()) { + throw new OperationCancelledException(); + } + scheduleSubfolderSynchronization(remoteFile.getRemotePath()); + } else { + SynchronizeFileOperation operation = new SynchronizeFileOperation( + localFile, + remoteFile, + user, + true, + mContext, + getStorageManager(), + useWorkerWithNotification + ); + mFilesToSyncContents.add(operation); } - - SynchronizeFileOperation operation = new SynchronizeFileOperation( - localFile, - remoteFile, - user, - true, - mContext, - getStorageManager(), - useWorkerWithNotification - ); - mFilesToSyncContents.add(operation); - return true; } private void prepareOpsFromLocalKnowledge() throws OperationCancelledException { List children = getStorageManager().getFolderContent(mLocalFolder, false); for (OCFile child : children) { - if (!child.isFolder()) { - if (!child.isDown()) { - mFilesForDirectDownload.add(child); - } else { - /// this should result in direct upload of files that were locally modified - SynchronizeFileOperation operation = new SynchronizeFileOperation( - child, - child.getEtagInConflict() != null ? child : null, - user, - true, - mContext, - getStorageManager(), - useWorkerWithNotification - ); - mFilesToSyncContents.add(operation); - } + if (child.isFolder()) { + syncFileOrFolder(child, child); + continue; + } + + if (!child.isDown()) { + mFilesForDirectDownload.add(child); + } else { + /// this should result in direct upload of files that were locally modified + SynchronizeFileOperation operation = new SynchronizeFileOperation( + child, + child.getEtagInConflict() != null ? child : null, + user, + true, + mContext, + getStorageManager(), + useWorkerWithNotification + ); + mFilesToSyncContents.add(operation); } } } - private void syncContents(OwnCloudClient client) throws OperationCancelledException { - startDirectDownloads(); - startContentSynchronizations(mFilesToSyncContents); + private RemoteOperationResult syncContents(OwnCloudClient client) throws OperationCancelledException { + RemoteOperationResult downloadResult = startDirectDownloads(); + if (isCancellationRequested()) { + throw new OperationCancelledException(); + } + RemoteOperationResult contentResult = startContentSynchronizations(mFilesToSyncContents); + if (isCancellationRequested()) { + throw new OperationCancelledException(); + } if (mIsPartOfInternalTwoWaySync) { twoWaySyncOperation.uploadNewLocalEntries(mLocalFolder, client); } updateETag(client); + return downloadResult.isSuccess() ? contentResult : downloadResult; } /** @@ -491,10 +564,10 @@ private void syncContents(OwnCloudClient client) throws OperationCancelledExcept * @param client the OwnCloudClient instance used to execute remote operations. */ private void updateETag(OwnCloudClient client) { - ReadFolderRemoteOperation operation = new ReadFolderRemoteOperation(mRemotePath); + ReadFileRemoteOperation operation = new ReadFileRemoteOperation(mRemotePath); final var result = operation.execute(client); if (!result.isSuccess()) { - Log_OC.w(TAG, "Cannot update eTag, read folder operation is failed"); + Log_OC.w(TAG, "Cannot update eTag, read file operation failed"); return; } @@ -507,40 +580,42 @@ private void updateETag(OwnCloudClient client) { } } - private void startDirectDownloads() { + private RemoteOperationResult startDirectDownloads() throws OperationCancelledException { final var fileDownloadHelper = FileDownloadHelper.Companion.instance(); - + RemoteOperationResult result = new RemoteOperationResult<>(ResultCode.OK); if (useWorkerWithNotification) { - fileDownloadHelper.downloadFolder(mLocalFolder, user.getAccountName()); - } else { - try { - for (OCFile file: mFilesForDirectDownload) { - if (mCancellationRequested.get()) { - break; - } - - if (file == null) { - continue; - } - - final var operation = new DownloadFileOperation(user, file, mContext); - var result = operation.execute(getClient()); + if (!mFilesForDirectDownload.isEmpty()) { + fileDownloadHelper.downloadFolder(mLocalFolder, user.getAccountName()); + } + return result; + } - String filename = file.getFileName(); - if (filename == null) { - continue; - } + for (OCFile file : mFilesForDirectDownload) { + if (isCancellationRequested()) { + throw new OperationCancelledException(); + } + RemoteOperationResult downloadResult = downloadFile(file, fileDownloadHelper); + if (result.isSuccess() && !downloadResult.isSuccess()) { + result = downloadResult; + } + } + return result; + } - if (result.isSuccess()) { - fileDownloadHelper.saveFile(file, operation, getStorageManager()); - Log_OC.d(TAG, "startDirectDownloads completed for: " + file.getFileName()); - } else { - Log_OC.d(TAG, "startDirectDownloads failed for: " + file.getFileName()); - } - } - } catch (Exception e) { - Log_OC.d(TAG, "Exception caught at startDirectDownloads" + e); + private RemoteOperationResult downloadFile(OCFile file, FileDownloadHelper fileDownloadHelper) { + try { + final var operation = new DownloadFileOperation(user, file, mContext); + var result = operation.execute(getClient()); + if (result.isSuccess()) { + fileDownloadHelper.saveFile(file, operation, getStorageManager()); + } else { + Log_OC.w(TAG, "SyncTrace download failed path=" + file.getRemotePath() + + " result=" + result.getCode()); } + return result; + } catch (Exception e) { + Log_OC.e(TAG, "SyncTrace direct download failed path=" + file.getRemotePath(), e); + return new RemoteOperationResult(e); } } @@ -553,47 +628,37 @@ private void startDirectDownloads() { * * @param filesToSyncContents Synchronization operations to execute. */ - private void startContentSynchronizations(List filesToSyncContents) throws OperationCancelledException { - Log_OC.v(TAG, "Starting content synchronization... "); - + private RemoteOperationResult startContentSynchronizations(List filesToSyncContents) + throws OperationCancelledException { + RemoteOperationResult result = new RemoteOperationResult<>(ResultCode.OK); int total = filesToSyncContents.size(); String folderName = mLocalFolder.getFileName(); - - for (int current = 0; current < filesToSyncContents.size(); current++) { - if (mCancellationRequested.get()) { - throw new OperationCancelledException(); - } - - final var synchronizeFileOperation = filesToSyncContents.get(current); - final var result = synchronizeFileOperation.execute(mContext); - final var file = synchronizeFileOperation.getLocalFile(); - - if (result.isSuccess() && file != null) { - notificationManager.showProgressNotification(folderName, file.getFileName(), current, total); - } else { - if (result.getCode() == ResultCode.SYNC_CONFLICT) { - mConflictsFound.incrementAndGet(); + try { + for (int current = 0; current < total; current++) { + if (isCancellationRequested()) { + throw new OperationCancelledException(); + } + final var operation = filesToSyncContents.get(current); + final var fileResult = operation.execute(mContext); + final var file = operation.getLocalFile(); + if (fileResult.isSuccess()) { + if (file != null) { + notificationManager.showProgressNotification(folderName, file.getFileName(), current, total); + } } else { - mFailsInFileSyncsFound.incrementAndGet(); - - String message = "Error while synchronizing file : "; - - if (result.getException() != null) { - message = message + result.getLogMessage() + " Exception: " - + result.getException().getMessage(); - } else { - message = message + result.getLogMessage(); + if (result.isSuccess()) { + result = fileResult; } - - Log_OC.e(TAG, message); + Log_OC.e(TAG, "Error while synchronizing file: " + fileResult.getLogMessage()); } } + return result; + } finally { + ExtensionsKt.mainThread(0L, () -> { + notificationManager.dismiss(); + return Unit.INSTANCE; + }); } - - ExtensionsKt.mainThread(0L, () -> { - notificationManager.dismiss(); - return Unit.INSTANCE; - }); } @@ -620,13 +685,25 @@ public Optional getFolderNameFromPath() { return Optional.of(folder.getName()); } - private void startSyncFolderOperation(String path) { - Intent intent = new Intent(mContext, OperationsService.class); + private boolean isCancellationRequested() { + return mCancellationRequested.get() || !continueSynchronization.getAsBoolean(); + } + + private static void startSyncFolderOperation(Context context, String path, User user, boolean syncAll) { + Intent intent = new Intent(context, OperationsService.class); intent.setAction(OperationsService.ACTION_SYNC_FOLDER); intent.putExtra(OperationsService.EXTRA_ACCOUNT, user.toPlatformAccount()); intent.putExtra(OperationsService.EXTRA_REMOTE_PATH, path); intent.putExtra(OperationsService.EXTRA_SYNC_ALL, syncAll); - mContext.startService(intent); + context.startService(intent); + } + + private void scheduleSubfolderSynchronization(String path) { + if (subfolderScheduler != null) { + subfolderScheduler.accept(path); + } else { + pendingSubfolders.addLast(path); + } } public String getRemotePath() { diff --git a/app/src/test/java/com/nextcloud/client/jobs/InternalTwoWaySyncWorkTest.kt b/app/src/test/java/com/nextcloud/client/jobs/InternalTwoWaySyncWorkTest.kt new file mode 100644 index 000000000000..80127debb321 --- /dev/null +++ b/app/src/test/java/com/nextcloud/client/jobs/InternalTwoWaySyncWorkTest.kt @@ -0,0 +1,126 @@ +/* + * Nextcloud - Android Client + * + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ +package com.nextcloud.client.jobs + +import android.app.Notification +import android.content.Context +import androidx.work.ForegroundInfo +import androidx.work.ListenableWorker +import androidx.work.WorkerParameters +import com.google.common.util.concurrent.ListenableFuture +import com.nextcloud.client.account.User +import com.nextcloud.client.account.UserAccountManager +import com.nextcloud.client.device.PowerManagementService +import com.nextcloud.client.jobs.notification.WorkerNotificationManager +import com.nextcloud.client.network.Connectivity +import com.nextcloud.client.network.ConnectivityService +import com.nextcloud.client.preferences.AppPreferences +import com.owncloud.android.MainApp +import com.owncloud.android.datamodel.FileDataStorageManager +import com.owncloud.android.datamodel.OCFile +import com.owncloud.android.lib.common.operations.RemoteOperationResult +import com.owncloud.android.lib.common.operations.RemoteOperationResult.ResultCode +import com.owncloud.android.operations.SynchronizeFolderOperation +import io.mockk.every +import io.mockk.mockk +import io.mockk.mockkStatic +import io.mockk.unmockkAll +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Before +import org.junit.Test +import org.mockito.MockedConstruction +import org.mockito.Mockito +import org.mockito.kotlin.same +import org.mockito.kotlin.never +import org.mockito.kotlin.verify + +class InternalTwoWaySyncWorkTest { + private val context = mockk(relaxed = true) + private val params = mockk(relaxed = true) + private val accounts = mockk(relaxed = true) + private val power = mockk(relaxed = true) + private val connectivity = mockk(relaxed = true) + private val preferences = mockk(relaxed = true) + private val user = mockk(relaxed = true) + private val selectedFolder = OCFile("/Root/") + private var currentFolder: OCFile? = OCFile("/Root/") + private var operationResult = RemoteOperationResult(ResultCode.OK) + private lateinit var storage: MockedConstruction + private lateinit var operations: MockedConstruction + private lateinit var notifications: MockedConstruction + + @Before + fun setUp() { + every { accounts.allUsers } returns listOf(user) + every { preferences.isTwoWaySyncEnabled } returns true + every { power.isPowerSavingEnabled } returns false + every { connectivity.isConnected } returns true + every { connectivity.connectivity } returns Connectivity(isWifi = true) + every { connectivity.isInternetWalled() } returns false + val foregroundFuture = mockk>(relaxed = true) + every { params.foregroundUpdater.setForegroundAsync(any(), any(), any()) } returns foregroundFuture + mockkStatic(MainApp::class) + every { MainApp.getStoragePath() } returns "/nonexistent-nextcloud-worker-test" + storage = Mockito.mockConstruction(FileDataStorageManager::class.java) { manager, _ -> + Mockito.`when`(manager.getInternalTwoWaySyncFolders(user)).thenReturn(listOf(selectedFolder)) + Mockito.`when`(manager.getFileByEncryptedRemotePath("/Root/")).thenAnswer { currentFolder } + } + operations = Mockito.mockConstruction(SynchronizeFolderOperation::class.java) { operation, _ -> + Mockito.`when`(operation.execute(context)).thenAnswer { operationResult } + } + notifications = Mockito.mockConstruction(WorkerNotificationManager::class.java) { manager, _ -> + val notification = Mockito.mock(Notification::class.java) + Mockito.`when`(manager.createSilentNotification(Mockito.anyString(), Mockito.anyInt())) + .thenReturn(notification) + Mockito.`when`(manager.getForegroundInfo(notification)).thenReturn(ForegroundInfo(392, notification)) + } + } + + @After + fun tearDown() { + notifications.close() + operations.close() + storage.close() + unmockkAll() + } + + @Test + fun `completion updates freshly loaded folder metadata`() { + val refreshed = currentFolder!! + refreshed.modificationTimestamp = 1234L + assertEquals(ListenableWorker.Result.success(), worker().doWork()) + verify(storage.constructed().single()).saveFile(same(refreshed)) + verify(storage.constructed().single(), never()).saveFile(same(selectedFolder)) + assertEquals(1234L, refreshed.modificationTimestamp) + assertEquals("OK", refreshed.internalFolderSyncResult) + } + + @Test + fun `a folder removed during synchronization is not recreated`() { + currentFolder = null + assertEquals(ListenableWorker.Result.success(), worker().doWork()) + verify(storage.constructed().single(), never()).saveFile(org.mockito.kotlin.any()) + } + + @Test + fun `download failure is recorded and the worker reports failure`() { + operationResult = RemoteOperationResult(ResultCode.UNAUTHORIZED) + assertEquals(ListenableWorker.Result.failure(), worker().doWork()) + assertEquals("UNAUTHORIZED", currentFolder!!.internalFolderSyncResult) + } + + @Test + fun `disabled ongoing sync does not inspect selected folders`() { + every { preferences.isTwoWaySyncEnabled } returns false + assertEquals(ListenableWorker.Result.success(), worker().doWork()) + assertEquals(0, storage.constructed().size) + assertEquals(0, operations.constructed().size) + } + + private fun worker() = InternalTwoWaySyncWork(context, params, accounts, power, connectivity, preferences) +} diff --git a/app/src/test/java/com/owncloud/android/operations/SynchronizeFileDownloadTest.kt b/app/src/test/java/com/owncloud/android/operations/SynchronizeFileDownloadTest.kt new file mode 100644 index 000000000000..d001d7423ca2 --- /dev/null +++ b/app/src/test/java/com/owncloud/android/operations/SynchronizeFileDownloadTest.kt @@ -0,0 +1,117 @@ +/* + * Nextcloud - Android Client + * + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ +package com.owncloud.android.operations + +import android.content.Context +import android.text.TextUtils +import com.nextcloud.client.account.User +import com.nextcloud.client.jobs.download.FileDownloadHelper +import com.owncloud.android.datamodel.FileDataStorageManager +import com.owncloud.android.datamodel.OCFile +import com.owncloud.android.lib.common.OwnCloudClient +import com.owncloud.android.lib.common.operations.RemoteOperationResult +import com.owncloud.android.lib.common.operations.RemoteOperationResult.ResultCode +import io.mockk.every +import io.mockk.mockk +import io.mockk.mockkObject +import io.mockk.mockkStatic +import io.mockk.unmockkAll +import io.mockk.verify +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.rules.TemporaryFolder +import org.mockito.MockedConstruction +import org.mockito.Mockito + +class SynchronizeFileDownloadTest { + @get:Rule + val temporaryFolder = TemporaryFolder() + + private val context = mockk(relaxed = true) + private val user = mockk(relaxed = true) + private val storage = mockk(relaxed = true) + private val helper = mockk(relaxed = true) + private val client = Mockito.mock(OwnCloudClient::class.java) + private var downloadResult = RemoteOperationResult(ResultCode.UNAUTHORIZED) + private lateinit var downloads: MockedConstruction + private val localFile = OCFile("/Root/file.txt").apply { + mimeType = "text/plain" + etag = "old" + lastSyncDateForData = Long.MAX_VALUE + } + private val serverFile = OCFile("/Root/file.txt").apply { etag = "new" } + + @Before + fun setUp() { + mockkStatic(TextUtils::class) + every { TextUtils.isEmpty(any()) } answers { firstArg().isNullOrEmpty() } + mockkObject(FileDownloadHelper.Companion) + every { FileDownloadHelper.instance() } returns helper + downloads = Mockito.mockConstruction(DownloadFileOperation::class.java) { operation, _ -> + Mockito.`when`(operation.execute(client)).thenAnswer { downloadResult } + } + } + + @After + fun tearDown() { + downloads.close() + unmockkAll() + } + + @Test + fun `missing file download failure is returned to the caller`() { + val operation = operation() + assertEquals(ResultCode.UNAUTHORIZED, operation.execute(client).code) + verify(exactly = 0) { helper.saveFile(any(), any(), any()) } + assertTrue(operation.transferWasRequested) + } + + @Test + fun `existing file refresh failure is returned to the caller`() { + localFile.storagePath = temporaryFolder.newFile().absolutePath + assertEquals(ResultCode.UNAUTHORIZED, operation().execute(client).code) + verify(exactly = 0) { helper.saveFile(any(), any(), any()) } + } + + @Test + fun `successful inline download is saved`() { + downloadResult = RemoteOperationResult(ResultCode.OK) + assertTrue(operation().execute(client).isSuccess) + verify(exactly = 1) { helper.saveFile(localFile, any(), storage) } + } + + @Test + fun `persistence failure is reported rather than swallowed`() { + downloadResult = RemoteOperationResult(ResultCode.OK) + every { helper.saveFile(any(), any(), any()) } throws IllegalStateException("Cannot save downloaded file") + assertFalse(operation().execute(client).isSuccess) + } + + @Test + fun `notification downloads retain asynchronous dispatch`() { + val operation = operation(notificationWorker = true) + assertTrue(operation.execute(client).isSuccess) + assertTrue(operation.transferWasRequested) + verify(exactly = 1) { helper.downloadFile(user, localFile) } + assertTrue(downloads.constructed().isEmpty()) + } + + private fun operation(notificationWorker: Boolean = false) = SynchronizeFileOperation( + localFile, + serverFile, + user, + true, + context, + storage, + notificationWorker + ) +} diff --git a/app/src/test/java/com/owncloud/android/operations/SynchronizeFolderOperationTest.kt b/app/src/test/java/com/owncloud/android/operations/SynchronizeFolderOperationTest.kt new file mode 100644 index 000000000000..3abb79ba1c16 --- /dev/null +++ b/app/src/test/java/com/owncloud/android/operations/SynchronizeFolderOperationTest.kt @@ -0,0 +1,180 @@ +/* + * Nextcloud - Android Client + * + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ +package com.owncloud.android.operations + +import android.content.Intent +import com.owncloud.android.lib.common.operations.RemoteOperationResult +import com.owncloud.android.lib.common.operations.RemoteOperationResult.ResultCode +import com.owncloud.android.lib.resources.files.ReadFileRemoteOperation +import io.mockk.EqMatcher +import io.mockk.every +import io.mockk.verify +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertTrue +import org.junit.Test +import org.mockito.Mockito + +class SynchronizeFolderOperationTest : SynchronizeFolderOperationTestFixture() { + @Test + fun `unchanged parent discovers previously unlisted descendants`() { + tree() + cacheFolder("/Root/", listOf("/Root/A/")) + + synchronizeTree() + + assertEquals(remoteChildren.keys.toList(), visitedFolders) + assertNotNull(cachedFiles[DEEP_FILE]) + assertEquals(listOf(DEEP_FILE), filesToSynchronize) + } + + @Test + fun `new child folders are persisted before their synchronization starts`() { + tree() + cacheFolder("/Root/", emptyList()) + cachedFiles.getValue("/Root/").etag = "" + + synchronizeTree() + + assertEquals(remoteChildren.keys.toList(), visitedFolders) + assertEquals(listOf(DEEP_FILE), filesToSynchronize) + } + + @Test + fun `browsing intermediate folders does not change synchronized files`() { + tree() + remoteChildren.forEach { (path, children) -> cacheFolder(path, children) } + + synchronizeTree() + + assertEquals(remoteChildren.keys.toList(), visitedFolders) + assertEquals(listOf(DEEP_FILE), filesToSynchronize) + } + + @Test + fun `mixed files and directories are all synchronized`() { + remoteFolder("/Root/", listOf("/Root/root.txt", "/Root/A/", "/Root/C/")) + remoteFolder("/Root/A/", listOf("/Root/A/a.txt", "/Root/A/B/")) + remoteFolder("/Root/A/B/", listOf("/Root/A/B/b.txt")) + remoteFolder("/Root/C/", listOf("/Root/C/c.txt")) + cacheFolder("/Root/", remoteChildren.getValue("/Root/")) + + synchronizeTree() + + assertEquals( + setOf("/Root/root.txt", "/Root/A/a.txt", "/Root/A/B/b.txt", "/Root/C/c.txt"), + filesToSynchronize.toSet() + ) + } + + @Test + fun `existing local files use content synchronization rather than direct download`() { + val path = "/Root/existing.txt" + remoteFolder("/Root/", listOf(path)) + cacheFolder("/Root/", listOf(path)) + cachedFiles.getValue(path).storagePath = localFolder.newFile().absolutePath + + synchronizeTree() + + assertEquals(listOf(path), filesToSynchronize) + assertTrue(downloads.constructed().isEmpty()) + } + + @Test + fun `ongoing descendant downloads finish before the root returns`() { + tree() + cacheFolder("/Root/", listOf("/Root/A/")) + assertTrue(executeTree().isSuccess) + assertEquals(listOf(DEEP_FILE), filesToSynchronize) + verify(exactly = 0) { downloadHelper.downloadFolder(any(), any()) } + verify(exactly = 0) { context.startService(any()) } + } + + @Test + fun `a failed descendant makes the root fail while other branches continue`() { + remoteFolder("/Root/", listOf("/Root/A/", "/Root/B/")) + remoteFolder("/Root/A/", listOf("/Root/A/file.txt")) + remoteFolder("/Root/B/", emptyList()) + cacheFolder("/Root/", remoteChildren.getValue("/Root/")) + every { constructedWith(EqMatcher("/Root/B/")).execute(client) } returns + RemoteOperationResult(ResultCode.FILE_NOT_FOUND) + assertFalse(executeTree().isSuccess) + assertEquals(listOf("/Root/A/file.txt"), filesToSynchronize) + } + + @Test + fun `cancellation after a child download stops remaining branches`() { + remoteFolder("/Root/", listOf("/Root/A/", "/Root/B/")) + remoteFolder("/Root/A/", listOf("/Root/A/file.txt")) + remoteFolder("/Root/B/", listOf("/Root/B/file.txt")) + cacheFolder("/Root/", remoteChildren.getValue("/Root/")) + onFileSaved = { rootOperation.cancel() } + assertFalse(executeTree().isSuccess) + assertEquals(listOf("/Root/B/file.txt"), filesToSynchronize) + assertEquals(listOf("/Root/", "/Root/B/"), visitedFolders) + } + + @Test + fun `a failed direct download is reported by the root`() { + tree() + cacheFolder("/Root/", listOf("/Root/A/")) + downloadResult = RemoteOperationResult(ResultCode.UNKNOWN_ERROR) + assertEquals(ResultCode.UNKNOWN_ERROR, executeTree().code) + assertTrue(filesToSynchronize.isEmpty()) + } + + @Test + fun `a failed local file synchronization preserves its error code`() { + val path = "/Root/existing.txt" + remoteFolder("/Root/", listOf(path)) + cacheFolder("/Root/", listOf(path)) + cachedFiles.getValue(path).storagePath = localFolder.newFile().absolutePath + contentResult = RemoteOperationResult(ResultCode.UNAUTHORIZED) + assertEquals(ResultCode.UNAUTHORIZED, executeTree().code) + } + + @Test + fun `a save failure does not abandon other downloads in the folder`() { + remoteFolder("/Root/", listOf("/Root/a.txt", "/Root/b.txt")) + cacheFolder("/Root/", remoteChildren.getValue("/Root/")) + onFileSaved = { throw IllegalStateException("Cannot persist downloaded file") } + assertFalse(executeTree().isSuccess) + assertEquals(listOf("/Root/a.txt", "/Root/b.txt"), filesToSynchronize) + } + + @Test + fun `changed constraints stop the queue between files`() { + remoteFolder("/Root/", listOf("/Root/a.txt", "/Root/b.txt")) + cacheFolder("/Root/", remoteChildren.getValue("/Root/")) + onFileSaved = { continueSync = false } + assertFalse(executeTree().isSuccess) + assertEquals(listOf("/Root/a.txt"), filesToSynchronize) + } + + @Test + fun `reusing a folder operation does not duplicate pending downloads`() { + remoteFolder("/Root/", listOf("/Root/a.txt")) + cacheFolder("/Root/", remoteChildren.getValue("/Root/")) + assertTrue(executeTree().isSuccess) + assertTrue(rootOperation.execute(client).isSuccess) + assertEquals(listOf("/Root/a.txt", "/Root/a.txt"), filesToSynchronize) + } + + @Test + fun `manual folder synchronization retains service dispatch for descendants`() { + tree() + cacheFolder("/Root/", listOf("/Root/A/")) + Mockito.mockConstruction(Intent::class.java).use { + val operation = SynchronizeFolderOperation(context, "/Root/", user, storage, true, false) + assertTrue(operation.execute(client).isSuccess) + verify(exactly = 1) { context.startService(any()) } + assertEquals(listOf("/Root/"), visitedFolders) + assertTrue(filesToSynchronize.isEmpty()) + } + } +} diff --git a/app/src/test/java/com/owncloud/android/operations/SynchronizeFolderOperationTestFixture.kt b/app/src/test/java/com/owncloud/android/operations/SynchronizeFolderOperationTestFixture.kt new file mode 100644 index 000000000000..fe9e2fabdd02 --- /dev/null +++ b/app/src/test/java/com/owncloud/android/operations/SynchronizeFolderOperationTestFixture.kt @@ -0,0 +1,197 @@ +/* + * Nextcloud - Android Client + * + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ +package com.owncloud.android.operations + +import android.content.Context +import android.text.TextUtils +import com.nextcloud.client.account.User +import com.nextcloud.client.jobs.download.FileDownloadHelper +import com.nextcloud.client.jobs.folderDownload.FolderDownloadWorkerNotificationManager +import com.nextcloud.utils.share.UnifiedShareSharees +import com.owncloud.android.datamodel.FileDataStorageManager +import com.owncloud.android.datamodel.OCFile +import com.owncloud.android.lib.common.OwnCloudClient +import com.owncloud.android.lib.common.operations.RemoteOperationResult +import com.owncloud.android.lib.common.operations.RemoteOperationResult.ResultCode +import com.owncloud.android.lib.resources.files.ReadFileRemoteOperation +import com.owncloud.android.lib.resources.files.ReadFolderRemoteOperation +import com.owncloud.android.lib.resources.files.model.RemoteFile +import com.owncloud.android.operations.synchronize.TwoWaySyncOperation +import com.owncloud.android.utils.FileStorageUtils +import com.owncloud.android.utils.MimeType +import io.mockk.EqMatcher +import io.mockk.every +import io.mockk.just +import io.mockk.mockk +import io.mockk.mockkConstructor +import io.mockk.mockkObject +import io.mockk.mockkStatic +import io.mockk.runs +import io.mockk.verify +import io.mockk.unmockkAll +import org.junit.After +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Rule +import org.junit.rules.TemporaryFolder +import org.mockito.MockedConstruction +import org.mockito.Mockito + +@Suppress("TooManyFunctions") +abstract class SynchronizeFolderOperationTestFixture { + @get:Rule + val localFolder = TemporaryFolder() + + protected lateinit var notifications: MockedConstruction + protected lateinit var twoWaySynchronizations: MockedConstruction + protected lateinit var downloads: MockedConstruction + protected lateinit var fileSynchronizations: MockedConstruction + protected val context: Context = mockk(relaxed = true) + protected val user: User = mockk(relaxed = true) + protected val client: OwnCloudClient = Mockito.mock(OwnCloudClient::class.java) + protected val storage: FileDataStorageManager = mockk(relaxed = true) + protected val remoteChildren = linkedMapOf>() + protected val cachedFiles = mutableMapOf() + protected lateinit var rootOperation: SynchronizeFolderOperation + protected val downloadHelper = mockk(relaxed = true) + protected var onTwoWaySyncCreated: (TwoWaySyncOperation) -> Unit = {} + protected var onFileSaved: () -> Unit = {} + protected var continueSync = true + protected var contentResult = RemoteOperationResult(ResultCode.OK) + protected var downloadResult = RemoteOperationResult(ResultCode.OK) + protected val visitedFolders = mutableListOf() + protected val filesToSynchronize = mutableListOf() + + @Before + fun setUp() { + twoWaySynchronizations = Mockito.mockConstruction(TwoWaySyncOperation::class.java) { operation, _ -> + onTwoWaySyncCreated(operation) + } + notifications = Mockito.mockConstruction(FolderDownloadWorkerNotificationManager::class.java) + mockkConstructor(ReadFileRemoteOperation::class) + mockkConstructor(ReadFolderRemoteOperation::class) + downloads = Mockito.mockConstruction(DownloadFileOperation::class.java) { operation, _ -> + Mockito.`when`(operation.execute(client)).thenAnswer { downloadResult } + } + mockkStatic("com.nextcloud.utils.extensions.ExtensionsKt") + every { com.nextcloud.utils.extensions.mainThread(any(), any()) } just runs + fileSynchronizations = Mockito.mockConstruction( + SynchronizeFileOperation::class.java + ) { operation, construction -> + val file = (construction.arguments()[1] ?: construction.arguments()[0]) as OCFile + Mockito.`when`(operation.execute(context)).thenAnswer { + filesToSynchronize.add(file.remotePath) + contentResult + } + Mockito.`when`(operation.localFile).thenReturn(file) + } + mockkStatic(TextUtils::class) + every { TextUtils.isEmpty(any()) } answers { firstArg().isNullOrEmpty() } + mockkStatic(FileStorageUtils::class) + mockkStatic(RefreshFolderOperation::class) + mockkStatic(UnifiedShareSharees::class) + mockkObject(FileDownloadHelper.Companion) + every { FileDownloadHelper.instance() } returns downloadHelper + every { downloadHelper.saveFile(any(), any(), any()) } answers { + filesToSynchronize.add(firstArg().remotePath) + onFileSaved() + } + every { UnifiedShareSharees.fillBlocking(any(), any()) } just runs + every { FileStorageUtils.checkEncryptionStatus(any(), any()) } returns false + every { FileStorageUtils.searchForLocalFileInDefaultPath(any(), any()) } just runs + every { RefreshFolderOperation.getDecryptedFolderMetadata(any(), any(), any(), any(), any()) } returns null + every { RefreshFolderOperation.prefillLocalFilesMap(any(), any()) } answers { + secondArg>().associateBy { it.remotePath }.toMutableMap() + } + every { storage.getFileByPath(any()) } answers { cachedFiles[firstArg()] } + every { storage.getFolderContent(any(), false) } answers { + val parent = firstArg() + cachedFiles.values.filter { it.parentId == parent.fileId } + } + every { storage.saveFolder(any(), any(), any()) } answers { + secondArg>().forEach { file -> + if (file.fileId == -1L) file.fileId = cachedFiles.size.toLong() + 1 + cachedFiles[file.remotePath] = file + } + } + } + + @After + fun tearDown() { + fileSynchronizations.close() + downloads.close() + notifications.close() + twoWaySynchronizations.close() + unmockkAll() + } + + protected fun tree() { + remoteFolder("/Root/", listOf("/Root/A/")) + remoteFolder("/Root/A/", listOf("/Root/A/B/")) + remoteFolder("/Root/A/B/", listOf(DEEP_FILE)) + } + + protected fun remoteFolder(path: String, children: List) { + remoteChildren[path] = children + val remoteFolder = remoteFile(path) + every { constructedWith(EqMatcher(path)).execute(client) } answers { + assertNotNull("Folder must be persisted before synchronization: $path", cachedFiles[path]) + if (path !in visitedFolders) visitedFolders.add(path) + result(listOf(remoteFolder)) + } + every { constructedWith(EqMatcher(path)).execute(client) } returns + result(listOf(remoteFolder) + children.map(::remoteFile)) + } + + protected fun remoteFile(path: String): RemoteFile { + val remote = mockk { + every { etag } returns "server-etag" + } + every { FileStorageUtils.fillOCFile(remote) } answers { file(path) } + return remote + } + + protected fun result(files: List): RemoteOperationResult = mockk(relaxed = true) { + every { isSuccess } returns true + every { data } returns ArrayList(files) + every { code } returns ResultCode.OK + } + + protected fun file(path: String): OCFile = OCFile(path).apply { + mimeType = if (path.endsWith('/')) MimeType.DIRECTORY else "text/plain" + etag = "server-etag" + } + + protected fun cacheFolder(path: String, children: List) { + val parent = cachedFiles.getOrPut(path) { file(path).apply { fileId = cachedFiles.size.toLong() + 1 } } + children.forEach { childPath -> + cachedFiles.getOrPut(childPath) { + file(childPath).apply { + fileId = cachedFiles.size.toLong() + 1 + parentId = parent.fileId + etag = "" + } + } + } + parent.etag = "server-etag" + } + + protected fun synchronizeTree() { + assertTrue(executeTree().isSuccess) + verify(exactly = 0) { context.startService(any()) } + } + + protected fun executeTree(): RemoteOperationResult<*> { + rootOperation = SynchronizeFolderOperation.forOngoingSync(context, "/Root/", user, storage) { continueSync } + return rootOperation.execute(client) + } + + companion object { + const val DEEP_FILE = "/Root/A/B/file.txt" + } +} diff --git a/app/src/test/java/com/owncloud/android/operations/SynchronizeFolderTwoWaySyncTest.kt b/app/src/test/java/com/owncloud/android/operations/SynchronizeFolderTwoWaySyncTest.kt new file mode 100644 index 000000000000..41cef7397934 --- /dev/null +++ b/app/src/test/java/com/owncloud/android/operations/SynchronizeFolderTwoWaySyncTest.kt @@ -0,0 +1,81 @@ +/* + * Nextcloud - Android Client + * + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ +package com.owncloud.android.operations + +import com.owncloud.android.datamodel.OCFile +import com.owncloud.android.lib.resources.files.ReadFolderRemoteOperation +import io.mockk.EqMatcher +import io.mockk.every +import io.mockk.verify +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test +import org.mockito.kotlin.any +import org.mockito.kotlin.anyOrNull +import org.mockito.kotlin.never +import org.mockito.kotlin.verify as verifyMockito +import org.mockito.kotlin.whenever + +class SynchronizeFolderTwoWaySyncTest : SynchronizeFolderOperationTestFixture() { + @Test + fun `ongoing sync refreshes unchanged folder listings`() { + remoteFolder("/Root/", listOf("/Root/new.txt")) + cacheFolder("/Root/", emptyList()) + + synchronizeTree() + + verify(exactly = 1) { + constructedWith(EqMatcher("/Root/")).execute(client) + } + assertTrue("/Root/new.txt" in filesToSynchronize) + } + + @Test + fun `ongoing descendants retain local directory preparation and uploads`() { + tree() + cacheFolder("/Root/", listOf("/Root/A/")) + every { storage.isPartOfInternalTwoWaySync(any()) } returns true + + synchronizeTree() + + twoWaySynchronizations.constructed().forEach { + verifyMockito(it).ensureLocalFolderExists(any()) + verifyMockito(it).uploadNewLocalEntries(any(), any()) + } + } + + @Test + fun `locally deleted folders are not saved or scheduled after remote deletion`() { + remoteFolder("/Root/", listOf("/Root/deleted/")) + cacheFolder("/Root/", listOf("/Root/deleted/")) + every { storage.isPartOfInternalTwoWaySync(any()) } returns true + onTwoWaySyncCreated = { + whenever(it.isDeletedLocally(any(), anyOrNull())).thenReturn(true) + whenever(it.deleteRemoteFile(any(), any())).thenReturn(true) + } + + synchronizeTree() + + verify(exactly = 1) { + storage.saveFolder(any(), match> { it.isEmpty() }, any()) + } + assertFalse("/Root/deleted/" in visitedFolders) + verifyMockito(twoWaySynchronizations.constructed().single()).deleteRemoteFile(any(), any()) + } + + @Test + fun `cancellation between downloads prevents local uploads`() { + remoteFolder("/Root/", listOf("/Root/a.txt")) + cacheFolder("/Root/", listOf("/Root/a.txt")) + every { storage.isPartOfInternalTwoWaySync(any()) } returns true + onFileSaved = { rootOperation.cancel() } + + assertFalse(executeTree().isSuccess) + + verifyMockito(twoWaySynchronizations.constructed().single(), never()).uploadNewLocalEntries(any(), any()) + } +}