From: jabarros Date: Fri, 9 Jan 2015 14:16:52 +0000 (+0100) Subject: Fix. Subfolders are not cancelled correctly X-Git-Tag: oc-android-1.7.0_signed~23^2~14^2~1 X-Git-Url: http://git.linex4red.de/pub/Android/ownCloud.git/commitdiff_plain/6fee98e6a4dc85b836f185cecf2b41637469e453?ds=inline;hp=--cc Fix. Subfolders are not cancelled correctly --- 6fee98e6a4dc85b836f185cecf2b41637469e453 diff --git a/src/com/owncloud/android/operations/SynchronizeFolderOperation.java b/src/com/owncloud/android/operations/SynchronizeFolderOperation.java index 3e12f95e..420adcab 100644 --- a/src/com/owncloud/android/operations/SynchronizeFolderOperation.java +++ b/src/com/owncloud/android/operations/SynchronizeFolderOperation.java @@ -34,6 +34,7 @@ import com.owncloud.android.lib.resources.files.ReadRemoteFileOperation; import com.owncloud.android.lib.resources.files.ReadRemoteFolderOperation; import com.owncloud.android.lib.resources.files.RemoteFile; import com.owncloud.android.operations.common.SyncOperation; +import com.owncloud.android.services.OperationsService; import com.owncloud.android.utils.FileStorageUtils; import java.io.File; @@ -98,8 +99,6 @@ public class SynchronizeFolderOperation extends SyncOperation { private List mFavouriteFilesToSyncContents; // this will be used for every file when 'folder synchronization' replaces 'folder download' - private List mFoldersToWalkDown; - private final AtomicBoolean mCancellationRequested; /** @@ -119,7 +118,6 @@ public class SynchronizeFolderOperation extends SyncOperation { mFilesForDirectDownload = new Vector(); mFilesToSyncContentsWithoutUpload = new Vector(); mFavouriteFilesToSyncContents = new Vector(); - mFoldersToWalkDown = new Vector(); mCancellationRequested = new AtomicBoolean(false); } @@ -302,11 +300,8 @@ public class SynchronizeFolderOperation extends SyncOperation { mFilesToSyncContentsWithoutUpload.clear(); mFavouriteFilesToSyncContents.clear(); - synchronized(mFoldersToWalkDown) { - if (mCancellationRequested.get()) { - throw new OperationCancelledException(); - } - mFoldersToWalkDown.clear(); + if (mCancellationRequested.get()) { + throw new OperationCancelledException(); } // get current data about local contents of the folder to synchronize @@ -363,18 +358,10 @@ public class SynchronizeFolderOperation extends SyncOperation { /// classify file to sync/download contents later if (remoteFile.isFolder()) { /// to download children files recursively - SynchronizeFolderOperation synchFolderOp = new SynchronizeFolderOperation( - mContext, - remoteFile.getRemotePath(), - mAccount, - mCurrentSyncTime - ); + startSyncFolderOperation(remoteFile.getRemotePath()); - synchronized(mFoldersToWalkDown) { - if (mCancellationRequested.get()) { - throw new OperationCancelledException(); - } - mFoldersToWalkDown.add(synchFolderOp); + if (mCancellationRequested.get()) { + throw new OperationCancelledException(); } } else if (remoteFile.keepInSync()) { @@ -416,18 +403,9 @@ public class SynchronizeFolderOperation extends SyncOperation { /// classify file to sync/download contents later if (child.isFolder()) { /// to download children files recursively - SynchronizeFolderOperation synchFolderOp = new SynchronizeFolderOperation( - mContext, - child.getRemotePath(), - mAccount, - mCurrentSyncTime - ); - - synchronized(mFoldersToWalkDown) { - if (mCancellationRequested.get()) { - throw new OperationCancelledException(); - } - mFoldersToWalkDown.add(synchFolderOp); + startSyncFolderOperation(child.getRemotePath()); + if (mCancellationRequested.get()) { + throw new OperationCancelledException(); } } else { @@ -444,7 +422,6 @@ public class SynchronizeFolderOperation extends SyncOperation { startDirectDownloads(); startContentSynchronizations(mFilesToSyncContentsWithoutUpload, client); startContentSynchronizations(mFavouriteFilesToSyncContents, client); - walkSubfolders(client); // this must be the last! } @@ -497,26 +474,6 @@ public class SynchronizeFolderOperation extends SyncOperation { } } - - private void walkSubfolders(OwnCloudClient client) throws OperationCancelledException { - RemoteOperationResult contentsResult = null; - for (SyncOperation op: mFoldersToWalkDown) { - if (mCancellationRequested.get()) { - throw new OperationCancelledException(); - } - contentsResult = op.execute(client, getStorageManager()); // to watch out: possibly deep recursion - if (!contentsResult.isSuccess()) { - // TODO - some kind of error count, and use it with notifications - if (contentsResult.getException() != null) { - Log_OC.e(TAG, "Non blocking exception : " - + contentsResult.getLogMessage(), contentsResult.getException()); - } else { - Log_OC.e(TAG, "Non blocking error : " + contentsResult.getLogMessage()); - } - } // won't let these fails break the synchronization process - } - } - /** * Creates and populates a new {@link com.owncloud.android.datamodel.OCFile} object with the data read from the server. @@ -570,13 +527,6 @@ public class SynchronizeFolderOperation extends SyncOperation { */ public void cancel() { mCancellationRequested.set(true); - - synchronized(mFoldersToWalkDown) { - // cancel 'child' synchronizations - for (SyncOperation synchOp : mFoldersToWalkDown) { - ((SynchronizeFolderOperation) synchOp).cancel(); - } - } } public String getFolderPath() { @@ -586,4 +536,12 @@ public class SynchronizeFolderOperation extends SyncOperation { } return FileStorageUtils.getDefaultSavePathFor(mAccount.name, mLocalFolder); } + + private void startSyncFolderOperation(String path){ + Intent intent = new Intent(mContext, OperationsService.class); + intent.setAction(OperationsService.ACTION_SYNC_FOLDER); + intent.putExtra(OperationsService.EXTRA_ACCOUNT, mAccount); + intent.putExtra(OperationsService.EXTRA_REMOTE_PATH, path); + mContext.startService(intent); + } } diff --git a/src/com/owncloud/android/services/OperationsService.java b/src/com/owncloud/android/services/OperationsService.java index 2b51660d..c6651f55 100644 --- a/src/com/owncloud/android/services/OperationsService.java +++ b/src/com/owncloud/android/services/OperationsService.java @@ -18,6 +18,7 @@ package com.owncloud.android.services; import java.io.IOException; +import java.util.ArrayList; import java.util.Iterator; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentLinkedQueue; @@ -482,24 +483,44 @@ public class OperationsService extends Service { mPendingOperations.putIfAbsent(syncKey,syncFolderOperation); } - /** - * Cancels a pending or current sync operation. - * + * Cancels sync operations. * @param account Owncloud account where the remote file is stored. - * @param file File + * @param file File OCFile */ - public void cancel(Account account, OCFile file) { + public void cancel(Account account, OCFile file){ SynchronizeFolderOperation syncOperation = null; + String targetKey = buildRemoteName(account, file.getRemotePath()); + ArrayList keyItems = new ArrayList(); synchronized (mPendingOperations) { - syncOperation = mPendingOperations.remove(buildRemoteName(account, file.getRemotePath())); + if (file.isFolder()) { + Log_OC.d(TAG, "Canceling pending sync operations"); + Iterator it = mPendingOperations.keySet().iterator(); + boolean found = false; + while (it.hasNext()) { + String keySyncOperation = it.next(); + found = keySyncOperation.contains(targetKey); + if (found) { + keyItems.add(keySyncOperation); + } + } + } else { + // this is not really expected... + Log_OC.d(TAG, "Canceling sync operation"); + keyItems.add(buildRemoteName(account, file.getRemotePath())); + } } - if (syncOperation != null) { - syncOperation.cancel(); + for (String item: keyItems) { + syncOperation = mPendingOperations.remove(item); + Log_OC.d(TAG, "Key sync operations removed: " + item); + + if (syncOperation != null) { + syncOperation.cancel(); + } } /// cancellation of download needs to be done separately in any case; a SynchronizeFolderOperation - // may finish much sooner than the real download of the files in the folder + // may finish much sooner than the real download of the files in the folder Intent intent = new Intent(mService, FileDownloader.class); intent.setAction(FileDownloader.ACTION_CANCEL_FILE_DOWNLOAD); intent.putExtra(FileDownloader.EXTRA_ACCOUNT, account);