Uploader upload shared files multiple times

[DavidWiesner]

I found a bug when uploading files. When select e.g.: multiple images in the gallery and share this with owncloud app. The files should be uploaded only once.

Expected behaviour

• sharing 3 files (1.img, 2.img, 3.img ) to a folder A
• folder A contains 3 files (1.img, 2.img, 3.img )

Actual behaviour

• folder A contains 5 files (1.img, 1 (2).img, 2.img, 2. (2)img, 3.img)

Steps to reproduce

1. open Gallery App
2. select at least three images
3. share to owncloud app
4. select any folder to upload
5. wait until upload finished
6. take a look to the folder

Bug:

the Bug is in uploadFiles in Uploader.java#L468

The Intent is created and filled inside the loop with local and remote paths array. So for three images (1.jpg, 2.jpg, 3.jpg) the service will be started three times with the multiple paths.

    public void uploadFiles() {
        try {
            ArrayList<String> local = new ArrayList<String>();
            ArrayList<String> remote = new ArrayList<String>();
            for (Parcelable mStream : mStreamsToUpload) {
                //...
                remote.add(filePath);
                local.add(data);

                Intent intent = new Intent(getApplicationContext(), FileUploadService.class);
                intent.putExtra(FileUploadService.KEY_UPLOAD_TYPE,
                        FileUploadService.UploadSingleMulti.UPLOAD_MULTIPLE_FILES);
                intent.putExtra(FileUploadService.KEY_LOCAL_FILE,
                        local.toArray(new String[local.size()]));
                intent.putExtra(FileUploadService.KEY_REMOTE_FILE,
                        remote.toArray(new String[remote.size()]));
                intent.putExtra(FileUploadService.KEY_ACCOUNT, getAccount());
                startService(intent);
                //...
                finish();
            }
            //...
        } catch (SecurityException e) {
            //...
        }
    }

Fix:

Move Intent creation, startService() until finish() outside the loop.

--- src/com/owncloud/android/ui/activity/Uploader.java  (revision )
+++ src/com/owncloud/android/ui/activity/Uploader.java  (revision )
@@ -565,25 +565,25 @@
                 else {
                     throw new SecurityException();
                 }
+            }

-                Intent intent = new Intent(getApplicationContext(), FileUploadService.class);
-                intent.putExtra(FileUploadService.KEY_UPLOAD_TYPE,
-                        FileUploadService.UploadSingleMulti.UPLOAD_MULTIPLE_FILES);
-                intent.putExtra(FileUploadService.KEY_LOCAL_FILE,
-                        local.toArray(new String[local.size()]));
-                intent.putExtra(FileUploadService.KEY_REMOTE_FILE,
-                        remote.toArray(new String[remote.size()]));
-                intent.putExtra(FileUploadService.KEY_ACCOUNT, getAccount());
-                startService(intent);
+            Intent intent = new Intent(getApplicationContext(), FileUploadService.class);
+            intent.putExtra(FileUploadService.KEY_UPLOAD_TYPE,
+                    FileUploadService.UploadSingleMulti.UPLOAD_MULTIPLE_FILES);
+            intent.putExtra(FileUploadService.KEY_LOCAL_FILE,
+                    local.toArray(new String[local.size()]));
+            intent.putExtra(FileUploadService.KEY_REMOTE_FILE,
+                    remote.toArray(new String[remote.size()]));
+            intent.putExtra(FileUploadService.KEY_ACCOUNT, getAccount());
+            startService(intent);

-                //Save the path to shared preferences
-                SharedPreferences.Editor appPrefs = PreferenceManager
-                        .getDefaultSharedPreferences(getApplicationContext()).edit();
-                appPrefs.putString("last_upload_path", mUploadPath);
-                appPrefs.apply();
+            //Save the path to shared preferences
+            SharedPreferences.Editor appPrefs = PreferenceManager
+                    .getDefaultSharedPreferences(getApplicationContext()).edit();
+            appPrefs.putString("last_upload_path", mUploadPath);
+            appPrefs.apply();

-                finish();
+            finish();
-            }

         } catch (SecurityException e) {
             String message = String.format(getString(R.string.uploader_error_forbidden_content),

Environment data

Android version:
Stock Android 4.4
Device model:
SamsungNexus Galaxy
Stock or customized system:

ownCloud app version:
https://github.com/owncloud/android/tree/ef7cf6edb0c4f9914b37ce30cc7468d24ca368bf

ownCloud server version:
8.1

Logs

adb android log

08-31 12:14:38.279    9331-9402/com.owncloud.android D/FileUploadService﹕ Intent { cmp=com.owncloud.android/.files.services.FileUploadService (has extras) }
08-31 12:14:38.279    9331-9402/com.owncloud.android D/FileUploadService﹕ mPendingUploads size:0 - before adding new uploads.
08-31 12:14:38.286    9331-9402/com.owncloud.android D/FileUploadService﹕ Receive upload intent.
08-31 12:14:38.302    9331-9402/com.owncloud.android D/FileUploadService﹕ mPendingUploads added: /sdcard/1.jpg status:UPLOAD_LATER result:null
08-31 12:14:39.427    9331-9402/com.owncloud.android W/FileUploadService﹕ There was an old DB entry /sdcard/1.jpg which had to be removed in order to add new one.
08-31 12:14:39.740    9331-9817/com.owncloud.android D/FileUploadService﹕ mPendingUploads size:1 - before uploading.
08-31 12:14:39.747    9331-9817/com.owncloud.android D/FileUploadService﹕ Calling uploadFile for /Bilder/1.jpg
08-31 12:14:39.747    9331-9402/com.owncloud.android D/FileUploadService﹕ onHandleIntent end
08-31 12:14:39.763    9331-9402/com.owncloud.android D/FileUploadService﹕ Intent { cmp=com.owncloud.android/.files.services.FileUploadService (has extras) }
08-31 12:14:39.763    9331-9817/com.owncloud.android D/FileUploadService﹕ mCurrentUpload = new UploadFileOperation
08-31 12:14:39.779    9331-9402/com.owncloud.android D/FileUploadService﹕ mPendingUploads size:1 - before adding new uploads.
08-31 12:14:39.794    9331-9402/com.owncloud.android D/FileUploadService﹕ Receive upload intent.
08-31 12:14:40.005    9331-9402/com.owncloud.android W/FileUploadService﹕ FileUploadService got upload intent for file which is already queued: /Bilder/1.jpg
08-31 12:14:40.005    9331-9402/com.owncloud.android D/FileUploadService﹕ mPendingUploads added: /sdcard/2.jpg status:UPLOAD_LATER result:null
08-31 12:14:41.310    9331-9402/com.owncloud.android W/FileUploadService﹕ There was an old DB entry /sdcard/2.jpg which had to be removed in order to add new one.
08-31 12:14:41.466    9331-9402/com.owncloud.android D/FileUploadService﹕ onHandleIntent end
08-31 12:14:41.474    9331-9402/com.owncloud.android D/FileUploadService﹕ Intent { cmp=com.owncloud.android/.files.services.FileUploadService (has extras) }
08-31 12:14:41.474    9331-9402/com.owncloud.android D/FileUploadService﹕ mPendingUploads size:2 - before adding new uploads.
08-31 12:14:41.482    9331-9402/com.owncloud.android D/FileUploadService﹕ Receive upload intent.
08-31 12:14:41.490    9331-9402/com.owncloud.android W/FileUploadService﹕ FileUploadService got upload intent for file which is already queued: /Bilder/1.jpg
08-31 12:14:41.490    9331-9402/com.owncloud.android W/FileUploadService﹕ FileUploadService got upload intent for file which is already queued: /Bilder/2.jpg
08-31 12:14:41.497    9331-9402/com.owncloud.android D/FileUploadService﹕ mPendingUploads added: /sdcard/3.jpg status:UPLOAD_LATER result:null
08-31 12:14:41.654    9331-9402/com.owncloud.android W/FileUploadService﹕ There was an old DB entry /sdcard/3.jpg which had to be removed in order to add new one.
08-31 12:14:41.786    9331-9402/com.owncloud.android D/FileUploadService﹕ onHandleIntent end
08-31 12:14:50.060    9331-9817/com.owncloud.android D/FileUploadService﹕ Remove CurrentUploadItem from pending upload Item Map.
08-31 12:14:50.060    9331-9817/com.owncloud.android D/FileUploadService﹕ updateDataseUploadResult uploadResult: com.owncloud.android.lib.common.operations.RemoteOperationResult@425a9740 upload: com.owncloud.android.operations.UploadFileOperation@42446730
08-31 12:14:50.497    9331-9817/com.owncloud.android D/FileUploadService﹕ NotifyUploadResult with resultCode: OK
08-31 12:14:50.521    9331-9817/com.owncloud.android D/FileUploadService﹕ Upload with result OK: Operation finished with HTTP status code 201 (success) will be abandoned.
08-31 12:14:50.521    9331-9817/com.owncloud.android D/FileUploadService﹕ mCurrentUpload = null
08-31 12:14:50.521    9331-9817/com.owncloud.android D/FileUploadService﹕ mPendingUploads size:2 - after uploading.
08-31 12:14:50.568    9331-9817/com.owncloud.android D/FileUploadService﹕ mPendingUploads size:2 - before uploading.
08-31 12:14:50.575    9331-9817/com.owncloud.android D/FileUploadService﹕ Calling uploadFile for /Bilder/1.jpg
08-31 12:14:50.575    9331-9817/com.owncloud.android D/FileUploadService﹕ mCurrentUpload = new UploadFileOperation
08-31 12:14:59.349    9331-9817/com.owncloud.android D/FileUploadService﹕ Remove CurrentUploadItem from pending upload Item Map.
08-31 12:14:59.349    9331-9817/com.owncloud.android D/FileUploadService﹕ updateDataseUploadResult uploadResult: com.owncloud.android.lib.common.operations.RemoteOperationResult@424b6fd8 upload: com.owncloud.android.operations.UploadFileOperation@4241c508
08-31 12:14:59.357    9331-9817/com.owncloud.android D/FileUploadService﹕ NotifyUploadResult with resultCode: OK
08-31 12:14:59.372    9331-9817/com.owncloud.android D/FileUploadService﹕ Upload with result OK: Operation finished with HTTP status code 201 (success) will be abandoned.
08-31 12:14:59.372    9331-9817/com.owncloud.android D/FileUploadService﹕ mCurrentUpload = null
08-31 12:14:59.380    9331-9817/com.owncloud.android D/FileUploadService﹕ mPendingUploads size:2 - after uploading.
08-31 12:14:59.380    9331-9817/com.owncloud.android D/FileUploadService﹕ mPendingUploads size:2 - before uploading.
08-31 12:14:59.380    9331-9817/com.owncloud.android D/FileUploadService﹕ Calling uploadFile for /Bilder/2.jpg
08-31 12:14:59.388    9331-9817/com.owncloud.android D/FileUploadService﹕ mCurrentUpload = new UploadFileOperation
08-31 12:15:07.755    9331-9817/com.owncloud.android D/FileUploadService﹕ Remove CurrentUploadItem from pending upload Item Map.
08-31 12:15:07.763    9331-9817/com.owncloud.android D/FileUploadService﹕ updateDataseUploadResult uploadResult: com.owncloud.android.lib.common.operations.RemoteOperationResult@425d24d0 upload: com.owncloud.android.operations.UploadFileOperation@42784f20
08-31 12:15:07.974    9331-9817/com.owncloud.android D/FileUploadService﹕ NotifyUploadResult with resultCode: OK
08-31 12:15:08.091    9331-9817/com.owncloud.android D/FileUploadService﹕ Upload with result OK: Operation finished with HTTP status code 201 (success) will be abandoned.
08-31 12:15:08.115    9331-9817/com.owncloud.android D/FileUploadService﹕ mCurrentUpload = null
08-31 12:15:08.169    9331-9817/com.owncloud.android D/FileUploadService﹕ mPendingUploads size:1 - after uploading.
08-31 12:15:08.247    9331-9817/com.owncloud.android D/FileUploadService﹕ mPendingUploads size:1 - before uploading.
08-31 12:15:08.255    9331-9817/com.owncloud.android D/FileUploadService﹕ mPendingUploads size:1 - before uploading.
08-31 12:15:08.263    9331-9817/com.owncloud.android D/FileUploadService﹕ Calling uploadFile for /Bilder/3.jpg
08-31 12:15:08.263    9331-9817/com.owncloud.android D/FileUploadService﹕ mCurrentUpload = new UploadFileOperation
08-31 12:15:17.365    9331-9817/com.owncloud.android D/FileUploadService﹕ Remove CurrentUploadItem from pending upload Item Map.
08-31 12:15:17.443    9331-9817/com.owncloud.android D/FileUploadService﹕ updateDataseUploadResult uploadResult: com.owncloud.android.lib.common.operations.RemoteOperationResult@4250ccd0 upload: com.owncloud.android.operations.UploadFileOperation@4282b948
08-31 12:15:17.786    9331-9817/com.owncloud.android D/FileUploadService﹕ NotifyUploadResult with resultCode: OK
08-31 12:15:17.802    9331-9817/com.owncloud.android D/FileUploadService﹕ Upload with result OK: Operation finished with HTTP status code 201 (success) will be abandoned.
08-31 12:15:17.810    9331-9817/com.owncloud.android D/FileUploadService﹕ mCurrentUpload = null
08-31 12:15:17.810    9331-9817/com.owncloud.android D/FileUploadService﹕ mPendingUploads size:0 - after uploading.
08-31 12:15:17.818    9331-9817/com.owncloud.android D/FileUploadService﹕ mPendingUploads size:0 - before uploading.
08-31 12:15:17.825    9331-9817/com.owncloud.android D/FileUploadService﹕ Calling uploadFile for /Bilder/2.jpg
08-31 12:15:17.841    9331-9817/com.owncloud.android D/FileUploadService﹕ mCurrentUpload = new UploadFileOperation
08-31 12:15:27.786    9331-9817/com.owncloud.android D/FileUploadService﹕ Remove CurrentUploadItem from pending upload Item Map.
08-31 12:15:27.786    9331-9817/com.owncloud.android D/FileUploadService﹕ updateDataseUploadResult uploadResult: com.owncloud.android.lib.common.operations.RemoteOperationResult@42623940 upload: com.owncloud.android.operations.UploadFileOperation@42655c88
08-31 12:15:27.802    9331-9817/com.owncloud.android D/FileUploadService﹕ NotifyUploadResult with resultCode: OK
08-31 12:15:27.872    9331-9817/com.owncloud.android D/FileUploadService﹕ Upload with result OK: Operation finished with HTTP status code 201 (success) will be abandoned.
08-31 12:15:27.896    9331-9817/com.owncloud.android D/FileUploadService﹕ mCurrentUpload = null
08-31 12:15:27.896    9331-9817/com.owncloud.android D/FileUploadService﹕ mPendingUploads size:0 - after uploadin

[LukeOwlclaw]

Yes, this seems to be odd. I am sure @davivel and @masensio will sort it out as they are refactoring the PR. There is another multiple uploads issue, too: #762 (comment) (At least I think it is another issue...)

[malkomich]

Since we have refactored all the uploading process, you can update to new version and check if it's already solved.