Merge pull request #10946 from nextcloud/optimize-uploads

Optimizations for uploads

app/src/main/java/com/nextcloud/client/jobs/FilesSyncWork.kt

@@ -25,7 +25,6 @@ package com.nextcloud.client.jobs
 import android.content.ContentResolver
 import android.content.Context
 import android.content.res.Resources
-import android.os.PowerManager.WakeLock
 import android.text.TextUtils
 import androidx.exifinterface.media.ExifInterface
 import androidx.work.Worker
@@ -74,16 +73,12 @@ class FilesSyncWork(
         const val TAG = "FilesSyncJob"
         const val SKIP_CUSTOM = "skipCustom"
         const val OVERRIDE_POWER_SAVING = "overridePowerSaving"
-        private const val WAKELOCK_TAG_SEPARATION = ":"
-        private const val WAKELOCK_ACQUIRE_TIMEOUT_MS = 10L * 60L * 1000L
     }
 
     override fun doWork(): Result {
-        val wakeLock: WakeLock? = null
         val overridePowerSaving = inputData.getBoolean(OVERRIDE_POWER_SAVING, false)
         // If we are in power save mode, better to postpone upload
         if (powerManagementService.isPowerSavingEnabled && !overridePowerSaving) {
-            wakeLock?.release()
             return Result.success()
         }
         val resources = context.resources
@@ -96,7 +91,7 @@ class FilesSyncWork(
             powerManagementService
         )
         FilesSyncHelper.insertAllDBEntries(preferences, clock, skipCustom)
-        // Create all the providers we'll needq
+        // Create all the providers we'll need
         val filesystemDataProvider = FilesystemDataProvider(contentResolver)
         val syncedFolderProvider = SyncedFolderProvider(contentResolver, preferences, clock)
         val currentLocale = resources.configuration.locale
@@ -115,7 +110,6 @@ class FilesSyncWork(
                 )
             }
         }
-        wakeLock?.release()
         return Result.success()
     }
 
@@ -129,11 +123,9 @@ class FilesSyncWork(
         sFormatter: SimpleDateFormat,
         syncedFolder: SyncedFolder
     ) {
-        var remotePath: String?
-        var subfolderByDate: Boolean
-        var uploadAction: Int?
-        var needsCharging: Boolean
-        var needsWifi: Boolean
+        val uploadAction: Int?
+        val needsCharging: Boolean
+        val needsWifi: Boolean
         var file: File
         val accountName = syncedFolder.account
         val optionalUser = userAccountManager.getUser(accountName)
@@ -148,56 +140,90 @@ class FilesSyncWork(
         }
         val paths = filesystemDataProvider.getFilesForUpload(
             syncedFolder.localPath,
-            java.lang.Long.toString(syncedFolder.id)
+            syncedFolder.id.toString()
         )
-        for (path in paths) {
+
+        if (paths.size == 0) {
+            return
+        }
+
+        val pathsAndMimes = paths.map { path ->
             file = File(path)
-            val lastModificationTime = calculateLastModificationTime(file, syncedFolder, sFormatter)
-            val mimeType = MimeTypeUtil.getBestMimeTypeByFilename(file.absolutePath)
-            if (lightVersion) {
-                needsCharging = resources.getBoolean(R.bool.syncedFolder_light_on_charging)
-                needsWifi = arbitraryDataProvider!!.getBooleanValue(
-                    accountName,
-                    SettingsActivity.SYNCED_FOLDER_LIGHT_UPLOAD_ON_WIFI
-                )
-                val uploadActionString = resources.getString(R.string.syncedFolder_light_upload_behaviour)
-                uploadAction = getUploadAction(uploadActionString)
-                subfolderByDate = resources.getBoolean(R.bool.syncedFolder_light_use_subfolders)
-                remotePath = resources.getString(R.string.syncedFolder_remote_folder)
-            } else {
-                needsCharging = syncedFolder.isChargingOnly
-                needsWifi = syncedFolder.isWifiOnly
-                uploadAction = syncedFolder.uploadAction
-                subfolderByDate = syncedFolder.isSubfolderByDate
-                remotePath = syncedFolder.remotePath
-            }
-            FileUploader.uploadNewFile(
-                context,
-                user,
-                file.absolutePath,
-                FileStorageUtils.getInstantUploadFilePath(
-                    file,
-                    currentLocale,
-                    remotePath,
-                    syncedFolder.localPath,
-                    lastModificationTime,
-                    subfolderByDate
-                ),
-                uploadAction!!,
-                mimeType,
-                true, // create parent folder if not existent
-                UploadFileOperation.CREATED_AS_INSTANT_PICTURE,
-                needsWifi,
-                needsCharging,
-                syncedFolder.nameCollisionPolicy
+            val localPath = file.absolutePath
+            Triple(
+                localPath,
+                getRemotePath(file, syncedFolder, sFormatter, lightVersion, resources, currentLocale),
+                MimeTypeUtil.getBestMimeTypeByFilename(localPath)
+            )
+        }
+        val localPaths = pathsAndMimes.map { it.first }.toTypedArray()
+        val remotePaths = pathsAndMimes.map { it.second }.toTypedArray()
+        val mimetypes = pathsAndMimes.map { it.third }.toTypedArray()
+
+        if (lightVersion) {
+            needsCharging = resources.getBoolean(R.bool.syncedFolder_light_on_charging)
+            needsWifi = arbitraryDataProvider!!.getBooleanValue(
+                accountName,
+                SettingsActivity.SYNCED_FOLDER_LIGHT_UPLOAD_ON_WIFI
             )
+            val uploadActionString = resources.getString(R.string.syncedFolder_light_upload_behaviour)
+            uploadAction = getUploadAction(uploadActionString)
+        } else {
+            needsCharging = syncedFolder.isChargingOnly
+            needsWifi = syncedFolder.isWifiOnly
+            uploadAction = syncedFolder.uploadAction
+        }
+        FileUploader.uploadNewFile(
+            context,
+            user,
+            localPaths,
+            remotePaths,
+            mimetypes,
+            uploadAction!!,
+            true, // create parent folder if not existent
+            UploadFileOperation.CREATED_AS_INSTANT_PICTURE,
+            needsWifi,
+            needsCharging,
+            syncedFolder.nameCollisionPolicy
+        )
+
+        for (path in paths) {
+            // TODO batch update
             filesystemDataProvider.updateFilesystemFileAsSentForUpload(
                 path,
-                java.lang.Long.toString(syncedFolder.id)
+                syncedFolder.id.toString()
             )
         }
     }
 
+    private fun getRemotePath(
+        file: File,
+        syncedFolder: SyncedFolder,
+        sFormatter: SimpleDateFormat,
+        lightVersion: Boolean,
+        resources: Resources,
+        currentLocale: Locale
+    ): String {
+        val lastModificationTime = calculateLastModificationTime(file, syncedFolder, sFormatter)
+        val remoteFolder: String
+        val useSubfolders: Boolean
+        if (lightVersion) {
+            useSubfolders = resources.getBoolean(R.bool.syncedFolder_light_use_subfolders)
+            remoteFolder = resources.getString(R.string.syncedFolder_remote_folder)
+        } else {
+            useSubfolders = syncedFolder.isSubfolderByDate
+            remoteFolder = syncedFolder.remotePath
+        }
+        return FileStorageUtils.getInstantUploadFilePath(
+            file,
+            currentLocale,
+            remoteFolder,
+            syncedFolder.localPath,
+            lastModificationTime,
+            useSubfolders
+        )
+    }
+
     private fun hasExif(file: File): Boolean {
         val mimeType = FileStorageUtils.getMimeTypeFromName(file.absolutePath)
         return MimeType.JPEG.equals(mimeType, ignoreCase = true) || MimeType.TIFF.equals(mimeType, ignoreCase = true)

app/src/main/java/com/nextcloud/client/jobs/FilesUploadWorker.kt

@@ -80,12 +80,12 @@ class FilesUploadWorker(
 
         // get all pending uploads
         var currentAndPendingUploadsForAccount =
-            uploadsStorageManager.getCurrentAndPendingUploadsForAccount(accountName)
+            uploadsStorageManager.getCurrentAndPendingUploadsForAccount(MAX_UPLOADS_QUERY, accountName)
         while (currentAndPendingUploadsForAccount.isNotEmpty()) {
             Log_OC.d(TAG, "Handling ${currentAndPendingUploadsForAccount.size} uploads for account $accountName")
             handlePendingUploads(currentAndPendingUploadsForAccount, accountName)
             currentAndPendingUploadsForAccount =
-                uploadsStorageManager.getCurrentAndPendingUploadsForAccount(accountName)
+                uploadsStorageManager.getCurrentAndPendingUploadsForAccount(MAX_UPLOADS_QUERY, accountName)
         }
 
         Log_OC.d(TAG, "No more pending uploads for account $accountName, stopping work")
@@ -240,8 +240,9 @@ class FilesUploadWorker(
 
     companion object {
         val TAG: String = FilesUploadWorker::class.java.simpleName
-        const val FOREGROUND_SERVICE_ID: Int = 412
-        const val MAX_PROGRESS: Int = 100
+        private const val MAX_UPLOADS_QUERY = 100
+        private const val FOREGROUND_SERVICE_ID: Int = 412
+        private const val MAX_PROGRESS: Int = 100
         const val ACCOUNT = "data_account"
     }
 }

app/src/main/java/com/owncloud/android/datamodel/UploadsStorageManager.java

@@ -23,13 +23,18 @@
  */
 package com.owncloud.android.datamodel;
 
+import android.content.ContentProviderOperation;
+import android.content.ContentProviderResult;
 import android.content.ContentResolver;
 import android.content.ContentValues;
+import android.content.OperationApplicationException;
 import android.database.Cursor;
 import android.net.Uri;
+import android.os.RemoteException;
 
 import com.nextcloud.client.account.CurrentAccountProvider;
 import com.nextcloud.client.account.User;
+import com.owncloud.android.MainApp;
 import com.owncloud.android.db.OCUpload;
 import com.owncloud.android.db.ProviderMeta.ProviderTableMeta;
 import com.owncloud.android.db.UploadResult;
@@ -42,6 +47,7 @@ import com.owncloud.android.operations.UploadFileOperation;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Calendar;
+import java.util.List;
 import java.util.Locale;
 import java.util.Observable;
 
@@ -82,6 +88,52 @@ public class UploadsStorageManager extends Observable {
     public long storeUpload(OCUpload ocUpload) {
         Log_OC.v(TAG, "Inserting " + ocUpload.getLocalPath() + " with status=" + ocUpload.getUploadStatus());
 
+        ContentValues cv = getContentValues(ocUpload);
+        Uri result = getDB().insert(ProviderTableMeta.CONTENT_URI_UPLOADS, cv);
+
+        Log_OC.d(TAG, "storeUpload returns with: " + result + " for file: " + ocUpload.getLocalPath());
+        if (result == null) {
+            Log_OC.e(TAG, "Failed to insert item " + ocUpload.getLocalPath() + " into upload db.");
+            return -1;
+        } else {
+            long new_id = Long.parseLong(result.getPathSegments().get(1));
+            ocUpload.setUploadId(new_id);
+            notifyObserversNow();
+            return new_id;
+        }
+    }
+
+    public long[] storeUploads(final List<OCUpload> ocUploads) {
+        Log_OC.v(TAG, "Inserting " + ocUploads.size() + " uploads");
+        ArrayList<ContentProviderOperation> operations = new ArrayList<>(ocUploads.size());
+        for (OCUpload ocUpload : ocUploads) {
+            final ContentProviderOperation operation = ContentProviderOperation
+                .newInsert(ProviderTableMeta.CONTENT_URI_UPLOADS)
+                .withValues(getContentValues(ocUpload))
+                .build();
+            operations.add(operation);
+        }
+
+        try {
+            final ContentProviderResult[] contentProviderResults = getDB().applyBatch(MainApp.getAuthority(), operations);
+            final long[] newIds = new long[ocUploads.size()];
+            for (int i = 0; i < contentProviderResults.length; i++) {
+                final ContentProviderResult result = contentProviderResults[i];
+                final long new_id = Long.parseLong(result.uri.getPathSegments().get(1));
+                ocUploads.get(i).setUploadId(new_id);
+                newIds[i] = new_id;
+            }
+            notifyObserversNow();
+            return newIds;
+        } catch (OperationApplicationException | RemoteException e) {
+            Log_OC.e(TAG, "Error inserting uploads", e);
+        }
+
+        return null;
+    }
+
+    @NonNull
+    private ContentValues getContentValues(OCUpload ocUpload) {
         ContentValues cv = new ContentValues();
         cv.put(ProviderTableMeta.UPLOADS_LOCAL_PATH, ocUpload.getLocalPath());
         cv.put(ProviderTableMeta.UPLOADS_REMOTE_PATH, ocUpload.getRemotePath());
@@ -96,20 +148,10 @@ public class UploadsStorageManager extends Observable {
         cv.put(ProviderTableMeta.UPLOADS_IS_WHILE_CHARGING_ONLY, ocUpload.isWhileChargingOnly() ? 1 : 0);
         cv.put(ProviderTableMeta.UPLOADS_IS_WIFI_ONLY, ocUpload.isUseWifiOnly() ? 1 : 0);
         cv.put(ProviderTableMeta.UPLOADS_FOLDER_UNLOCK_TOKEN, ocUpload.getFolderUnlockToken());
-        Uri result = getDB().insert(ProviderTableMeta.CONTENT_URI_UPLOADS, cv);
-
-        Log_OC.d(TAG, "storeUpload returns with: " + result + " for file: " + ocUpload.getLocalPath());
-        if (result == null) {
-            Log_OC.e(TAG, "Failed to insert item " + ocUpload.getLocalPath() + " into upload db.");
-            return -1;
-        } else {
-            long new_id = Long.parseLong(result.getPathSegments().get(1));
-            ocUpload.setUploadId(new_id);
-            notifyObserversNow();
-            return new_id;
-        }
+        return cv;
     }
 
+
     /**
      * Update an upload object in DB.
      *
@@ -322,6 +364,10 @@ public class UploadsStorageManager extends Observable {
     }
 
     private OCUpload[] getUploads(@Nullable String selection, @Nullable String... selectionArgs) {
+        return getUploads(0, selection, selectionArgs);
+    }
+
+    private OCUpload[] getUploads(final int limit, @Nullable String selection, @Nullable String... selectionArgs) {
         ArrayList<OCUpload> uploads = new ArrayList<>();
         final long pageSize = 100;
         long page = 0;
@@ -384,7 +430,11 @@ public class UploadsStorageManager extends Observable {
             } else {
                 break;
             }
-        } while (rowsRead > 0);
+        } while (rowsRead > 0 && (limit <= 0 || rowsRead < limit));
+
+        if (limit > 0 && uploads.size() > limit) {
+            uploads = new ArrayList<>(uploads.subList(0, limit));
+        }
 
         Log_OC.v(TAG, String.format(Locale.ENGLISH,
                                     "getUploads() returning %d (%d) rows after reading %d pages",
@@ -393,6 +443,7 @@ public class UploadsStorageManager extends Observable {
                                     page
                                    ));
 
+
         return uploads.toArray(new OCUpload[0]);
     }
 
@@ -433,7 +484,11 @@ public class UploadsStorageManager extends Observable {
     }
 
     public OCUpload[] getCurrentAndPendingUploadsForAccount(final @NonNull String accountName) {
-        return getUploads(ProviderTableMeta.UPLOADS_STATUS + "==" + UploadStatus.UPLOAD_IN_PROGRESS.value +
+        return getCurrentAndPendingUploadsForAccount(0, accountName);
+    }
+
+    public OCUpload[] getCurrentAndPendingUploadsForAccount(final int limit, final @NonNull String accountName) {
+        return getUploads(limit, ProviderTableMeta.UPLOADS_STATUS + "==" + UploadStatus.UPLOAD_IN_PROGRESS.value +
                               " OR " + ProviderTableMeta.UPLOADS_LAST_RESULT +
                               "==" + UploadResult.DELAYED_FOR_WIFI.getValue() +
                               " OR " + ProviderTableMeta.UPLOADS_LAST_RESULT +

app/src/main/java/com/owncloud/android/operations/UploadFileOperation.java

@@ -388,13 +388,7 @@ public class UploadFileOperation extends SyncOperation {
         mCancellationRequested.set(false);
         mUploadStarted.set(true);
 
-        for (OCUpload ocUpload : uploadsStorageManager.getAllStoredUploads()) {
-            if (ocUpload.getUploadId() == getOCUploadId()) {
-                ocUpload.setFileSize(0);
-                uploadsStorageManager.updateUpload(ocUpload);
-                break;
-            }
-        }
+        updateSize(0);
 
         String remoteParentPath = new File(getRemotePath()).getParent();
         remoteParentPath = remoteParentPath.endsWith(OCFile.PATH_SEPARATOR) ?
@@ -567,13 +561,8 @@ public class UploadFileOperation extends SyncOperation {
                 size = new File(mFile.getStoragePath()).length();
             }
 
-            for (OCUpload ocUpload : uploadsStorageManager.getAllStoredUploads()) {
-                if (ocUpload.getUploadId() == getOCUploadId()) {
-                    ocUpload.setFileSize(size);
-                    uploadsStorageManager.updateUpload(ocUpload);
-                    break;
-                }
-            }
+
+            updateSize(size);
 
             /// perform the upload
             if (size > ChunkedFileUploadRemoteOperation.CHUNK_SIZE_MOBILE) {
@@ -749,6 +738,8 @@ public class UploadFileOperation extends SyncOperation {
         File originalFile = new File(mOriginalStoragePath);
         File expectedFile = null;
         FileLock fileLock = null;
+        FileChannel channel = null;
+
         long size;
 
         try {
@@ -779,7 +770,6 @@ public class UploadFileOperation extends SyncOperation {
 
             final Long creationTimestamp = FileUtil.getCreationTimestamp(originalFile);
 
-            FileChannel channel = null;
             try {
                 channel = new RandomAccessFile(mFile.getStoragePath(), "rw").getChannel();
                 fileLock = channel.tryLock();
@@ -810,13 +800,7 @@ public class UploadFileOperation extends SyncOperation {
                 size = new File(mFile.getStoragePath()).length();
             }
 
-            for (OCUpload ocUpload : uploadsStorageManager.getAllStoredUploads()) {
-                if (ocUpload.getUploadId() == getOCUploadId()) {
-                    ocUpload.setFileSize(size);
-                    uploadsStorageManager.updateUpload(ocUpload);
-                    break;
-                }
-            }
+            updateSize(size);
 
             // perform the upload
             if (size > ChunkedFileUploadRemoteOperation.CHUNK_SIZE_MOBILE) {
@@ -876,6 +860,14 @@ public class UploadFileOperation extends SyncOperation {
                 }
             }
 
+            if (channel != null) {
+                try {
+                    channel.close();
+                } catch (IOException e) {
+                    Log_OC.w(TAG, "Failed to close file channel");
+                }
+            }
+
             if (temporalFile != null && !originalFile.equals(temporalFile)) {
                 temporalFile.delete();
             }
@@ -901,6 +893,14 @@ public class UploadFileOperation extends SyncOperation {
         return result;
     }
 
+    private void updateSize(long size) {
+        OCUpload ocUpload = uploadsStorageManager.getUploadById(getOCUploadId());
+        if(ocUpload != null){
+            ocUpload.setFileSize(size);
+            uploadsStorageManager.updateUpload(ocUpload);
+        }
+    }
+
     private void logResult(RemoteOperationResult result, String sourcePath, String targetPath) {
         if (result.isSuccess()) {
             Log_OC.i(TAG, "Upload of " + sourcePath + " to " + targetPath + ": " + result.getLogMessage());

app/src/main/java/com/owncloud/android/ui/activity/UploadListActivity.java

@@ -43,6 +43,7 @@ import com.nextcloud.client.core.Clock;
 import com.nextcloud.client.device.PowerManagementService;
 import com.nextcloud.client.jobs.BackgroundJobManager;
 import com.nextcloud.client.network.ConnectivityService;
+import com.nextcloud.client.utils.Throttler;
 import com.owncloud.android.R;
 import com.owncloud.android.databinding.UploadListLayoutBinding;
 import com.owncloud.android.datamodel.OCFile;
@@ -103,6 +104,8 @@ public class UploadListActivity extends FileActivity {
     @Inject
     ViewThemeUtils viewThemeUtils;
 
+    @Inject Throttler throttler;
+
     private UploadListLayoutBinding binding;
 
     public static Intent createIntent(OCFile file, User user, Integer flag, Context context) {
@@ -120,6 +123,8 @@ public class UploadListActivity extends FileActivity {
     protected void onCreate(Bundle savedInstanceState) {
         super.onCreate(savedInstanceState);
 
+        throttler.setIntervalMillis(1000);
+
         binding = UploadListLayoutBinding.inflate(getLayoutInflater());
         setContentView(binding.getRoot());
 
@@ -351,7 +356,10 @@ public class UploadListActivity extends FileActivity {
          */
         @Override
         public void onReceive(Context context, Intent intent) {
-            uploadListAdapter.loadUploadItemsFromDb();
+
+            throttler.run("update_upload_list", () -> {
+                uploadListAdapter.loadUploadItemsFromDb();
+            });
         }
     }
 }

app/src/main/java/com/owncloud/android/utils/FilesUploadHelper.kt

@@ -55,8 +55,8 @@ class FilesUploadHelper {
         nameCollisionPolicy: NameCollisionPolicy,
         localBehavior: Int
     ) {
-        for (i in localPaths.indices) {
-            OCUpload(localPaths[i], remotePaths[i], user.accountName).apply {
+        val uploads = localPaths.mapIndexed { index, localPath ->
+            OCUpload(localPath, remotePaths[index], user.accountName).apply {
                 this.nameCollisionPolicy = nameCollisionPolicy
                 isUseWifiOnly = requiresWifi
                 isWhileChargingOnly = requiresCharging
@@ -64,10 +64,9 @@ class FilesUploadHelper {
                 this.createdBy = createdBy
                 isCreateRemoteFolder = createRemoteFolder
                 localAction = localBehavior
-            }.also {
-                uploadsStorageManager.storeUpload(it)
             }
         }
+        uploadsStorageManager.storeUploads(uploads)
         backgroundJobManager.startFilesUploadJob(user)
     }
 
@@ -79,7 +78,7 @@ class FilesUploadHelper {
     ) {
         Log_OC.d(this, "upload updated file")
 
-        for (file in existingFiles) {
+        val uploads = existingFiles.map { file ->
             OCUpload(file, user).apply {
                 fileSize = file.fileLength
                 this.nameCollisionPolicy = nameCollisionPolicy
@@ -88,10 +87,9 @@ class FilesUploadHelper {
                 isUseWifiOnly = false
                 isWhileChargingOnly = false
                 uploadStatus = UploadsStorageManager.UploadStatus.UPLOAD_IN_PROGRESS
-            }.also {
-                uploadsStorageManager.storeUpload(it)
             }
         }
+        uploadsStorageManager.storeUploads(uploads)
         backgroundJobManager.startFilesUploadJob(user)
     }