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()) + } +}