Fix. Subfolders are not cancelled correctly

diff --git a/src/com/owncloud/android/operations/SynchronizeFolderOperation.java b/src/com/owncloud/android/operations/SynchronizeFolderOperation.java
index 3e12f95..420adca 100644 (file)
--- 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<SyncOperation> mFavouriteFilesToSyncContents;
         // this will be used for every file when 'folder synchronization' replaces 'folder download' 
 
-    private List<SyncOperation> mFoldersToWalkDown;
-
     private final AtomicBoolean mCancellationRequested;
 
     /**
@@ -119,7 +118,6 @@ public class SynchronizeFolderOperation extends SyncOperation {
         mFilesForDirectDownload = new Vector<OCFile>();
         mFilesToSyncContentsWithoutUpload = new Vector<SyncOperation>();
         mFavouriteFilesToSyncContents = new Vector<SyncOperation>();
-        mFoldersToWalkDown = new Vector<SyncOperation>();
         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 2b51660..c6651f5 100644 (file)
--- 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<String> keyItems = new ArrayList<String>();
             synchronized (mPendingOperations) {
-                syncOperation = mPendingOperations.remove(buildRemoteName(account, file.getRemotePath()));
+                if (file.isFolder()) {
+                    Log_OC.d(TAG, "Canceling pending sync operations");
+                    Iterator<String> 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);