Merge pull request #13160 from nextcloud/make-upload-restart-more-efficient

Remove unnecessary checks and calls from restartFailedUploads

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

@@ -182,7 +182,7 @@ class FilesSyncWork(
         } else {
             // Check every file in synced folder for changes and update
             // filesystemDataProvider database (potentially needs a long time)
-            FilesSyncHelper.insertAllDBEntries(syncedFolder, powerManagementService)
+            FilesSyncHelper.insertAllDBEntriesForSyncedFolder(syncedFolder)
         }
     }
 

app/src/main/java/com/nextcloud/client/jobs/upload/FileUploadHelper.kt

@@ -12,9 +12,11 @@ import android.content.Context
 import android.content.Intent
 import com.nextcloud.client.account.User
 import com.nextcloud.client.account.UserAccountManager
+import com.nextcloud.client.device.BatteryStatus
 import com.nextcloud.client.device.PowerManagementService
 import com.nextcloud.client.jobs.BackgroundJobManager
 import com.nextcloud.client.jobs.upload.FileUploadWorker.Companion.currentUploadFileOperation
+import com.nextcloud.client.network.Connectivity
 import com.nextcloud.client.network.ConnectivityService
 import com.owncloud.android.MainApp
 import com.owncloud.android.datamodel.OCFile
@@ -28,9 +30,9 @@ import com.owncloud.android.lib.common.operations.RemoteOperationResult
 import com.owncloud.android.lib.common.utils.Log_OC
 import com.owncloud.android.lib.resources.files.ReadFileRemoteOperation
 import com.owncloud.android.lib.resources.files.model.RemoteFile
+import com.owncloud.android.operations.UploadFileOperation
 import com.owncloud.android.utils.FileUtil
 import java.io.File
-import java.util.Optional
 import javax.inject.Inject
 
 @Suppress("TooManyFunctions")
@@ -117,34 +119,44 @@ class FileUploadHelper {
         failedUploads: Array<OCUpload>
     ): Boolean {
         var showNotExistMessage = false
-        val (gotNetwork, _, gotWifi) = connectivityService.connectivity
+        val isOnline = checkConnectivity(connectivityService)
+        val connectivity = connectivityService.connectivity
         val batteryStatus = powerManagementService.battery
-        val charging = batteryStatus.isCharging || batteryStatus.isFull
-        val isPowerSaving = powerManagementService.isPowerSavingEnabled
-        var uploadUser = Optional.empty<User>()
+        val accountNames = accountManager.accounts.filter { account ->
+            accountManager.getUser(account.name).isPresent
+        }.map { account ->
+            account.name
+        }.toHashSet()
 
         for (failedUpload in failedUploads) {
-            val isDeleted = !File(failedUpload.localPath).exists()
-            if (isDeleted) {
-                showNotExistMessage = true
+            if (!accountNames.contains(failedUpload.accountName)) {
+                uploadsStorageManager.removeUpload(failedUpload)
+                continue
+            }
+
+            val uploadResult =
+                checkUploadConditions(failedUpload, connectivity, batteryStatus, powerManagementService, isOnline)
 
-                // 2A. for deleted files, mark as permanently failed
-                if (failedUpload.lastResult != UploadResult.FILE_NOT_FOUND) {
-                    failedUpload.lastResult = UploadResult.FILE_NOT_FOUND
+            if (uploadResult != UploadResult.UPLOADED) {
+                if (failedUpload.lastResult != uploadResult) {
+                    failedUpload.lastResult = uploadResult
                     uploadsStorageManager.updateUpload(failedUpload)
                 }
-            } else if (!isPowerSaving && gotNetwork &&
-                canUploadBeRetried(failedUpload, gotWifi, charging) && !connectivityService.isInternetWalled
-            ) {
-                // 2B. for existing local files, try restarting it if possible
-                failedUpload.uploadStatus = UploadStatus.UPLOAD_IN_PROGRESS
-                uploadsStorageManager.updateUpload(failedUpload)
+                if (uploadResult == UploadResult.FILE_NOT_FOUND) {
+                    showNotExistMessage = true
+                }
+                continue
             }
+
+            failedUpload.uploadStatus = UploadStatus.UPLOAD_IN_PROGRESS
+            uploadsStorageManager.updateUpload(failedUpload)
         }
 
-        accountManager.accounts.forEach {
-            val user = accountManager.getUser(it.name)
-            if (user.isPresent) backgroundJobManager.startFilesUploadJob(user.get())
+        accountNames.forEach { accountName ->
+            val user = accountManager.getUser(accountName)
+            if (user.isPresent) {
+                backgroundJobManager.startFilesUploadJob(user.get())
+            }
         }
 
         return showNotExistMessage
@@ -216,11 +228,50 @@ class FileUploadHelper {
         return upload.uploadStatus == UploadStatus.UPLOAD_IN_PROGRESS
     }
 
-    private fun canUploadBeRetried(upload: OCUpload, gotWifi: Boolean, isCharging: Boolean): Boolean {
-        val file = File(upload.localPath)
-        val needsWifi = upload.isUseWifiOnly
-        val needsCharging = upload.isWhileChargingOnly
-        return file.exists() && (!needsWifi || gotWifi) && (!needsCharging || isCharging)
+    private fun checkConnectivity(connectivityService: ConnectivityService): Boolean {
+        // check that connection isn't walled off and that the server is reachable
+        return connectivityService.getConnectivity().isConnected && !connectivityService.isInternetWalled()
+    }
+
+    /**
+     * Dupe of [UploadFileOperation.checkConditions], needed to check if the upload should even be scheduled
+     * @return [UploadResult.UPLOADED] if the upload should be scheduled, otherwise the reason why it shouldn't
+     */
+    private fun checkUploadConditions(
+        upload: OCUpload,
+        connectivity: Connectivity,
+        battery: BatteryStatus,
+        powerManagementService: PowerManagementService,
+        hasGeneralConnection: Boolean
+    ): UploadResult {
+        var conditions = UploadResult.UPLOADED
+
+        // check that internet is available
+        if (!hasGeneralConnection) {
+            conditions = UploadResult.NETWORK_CONNECTION
+        }
+
+        // check that local file exists; skip the upload otherwise
+        if (!File(upload.localPath).exists()) {
+            conditions = UploadResult.FILE_NOT_FOUND
+        }
+
+        // check that connectivity conditions are met; delay upload otherwise
+        if (upload.isUseWifiOnly && (!connectivity.isWifi || connectivity.isMetered)) {
+            conditions = UploadResult.DELAYED_FOR_WIFI
+        }
+
+        // check if charging conditions are met; delay upload otherwise
+        if (upload.isWhileChargingOnly && !battery.isCharging && !battery.isFull) {
+            conditions = UploadResult.DELAYED_FOR_CHARGING
+        }
+
+        // check that device is not in power save mode; delay upload otherwise
+        if (powerManagementService.isPowerSavingEnabled) {
+            conditions = UploadResult.DELAYED_IN_POWER_SAVE_MODE
+        }
+
+        return conditions
     }
 
     @Suppress("ReturnCount")

app/src/main/java/com/nextcloud/client/jobs/upload/FileUploadWorker.kt

@@ -130,7 +130,7 @@ class FileUploadWorker(
     @Suppress("ReturnCount")
     private fun retrievePagesBySortingUploadsByID(): Result {
         val accountName = inputData.getString(ACCOUNT) ?: return Result.failure()
-        var uploadsPerPage = uploadsStorageManager.getCurrentAndPendingUploadsForAccountPageAscById(-1, accountName)
+        var uploadsPerPage = uploadsStorageManager.getCurrentUploadsForAccountPageAscById(-1, accountName)
         val totalUploadSize = uploadsStorageManager.getTotalUploadSize(accountName)
 
         Log_OC.d(TAG, "Total upload size: $totalUploadSize")
@@ -148,7 +148,7 @@ class FileUploadWorker(
             val lastId = uploadsPerPage.last().uploadId
             uploadFiles(totalUploadSize, uploadsPerPage, accountName)
             uploadsPerPage =
-                uploadsStorageManager.getCurrentAndPendingUploadsForAccountPageAscById(lastId, accountName)
+                uploadsStorageManager.getCurrentUploadsForAccountPageAscById(lastId, accountName)
         }
 
         if (isStopped) {

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

@@ -471,7 +471,7 @@ public class UploadsStorageManager extends Observable {
         return getUploadPage(QUERY_PAGE_SIZE, afterId, true, selection, selectionArgs);
     }
 
-    private String getInProgressUploadsSelection() {
+    private String getInProgressAndDelayedUploadsSelection() {
         return "( " + ProviderTableMeta.UPLOADS_STATUS + EQUAL + UploadStatus.UPLOAD_IN_PROGRESS.value +
             OR + ProviderTableMeta.UPLOADS_LAST_RESULT +
             EQUAL + UploadResult.DELAYED_FOR_WIFI.getValue() +
@@ -485,7 +485,8 @@ public class UploadsStorageManager extends Observable {
     }
 
     public int getTotalUploadSize(@Nullable String... selectionArgs) {
-        final String selection = getInProgressUploadsSelection();
+        final String selection = ProviderTableMeta.UPLOADS_STATUS + EQUAL + UploadStatus.UPLOAD_IN_PROGRESS.value +
+            AND + ProviderTableMeta.UPLOADS_ACCOUNT_NAME + IS_EQUAL;
         int totalSize = 0;
 
         Cursor cursor = getDB().query(
@@ -605,17 +606,29 @@ public class UploadsStorageManager extends Observable {
     }
 
     public OCUpload[] getCurrentAndPendingUploadsForAccount(final @NonNull String accountName) {
-        String inProgressUploadsSelection = getInProgressUploadsSelection();
+        String inProgressUploadsSelection = getInProgressAndDelayedUploadsSelection();
         return getUploads(inProgressUploadsSelection, accountName);
     }
 
+    public OCUpload[] getCurrentUploadsForAccount(final @NonNull String accountName) {
+        return getUploads(ProviderTableMeta.UPLOADS_STATUS + EQUAL + UploadStatus.UPLOAD_IN_PROGRESS.value + AND +
+                              ProviderTableMeta.UPLOADS_ACCOUNT_NAME + IS_EQUAL, accountName);
+    }
+
+    public List<OCUpload> getCurrentUploadsForAccountPageAscById(final long afterId, final @NonNull String accountName) {
+        final String selection = ProviderTableMeta.UPLOADS_STATUS + EQUAL + UploadStatus.UPLOAD_IN_PROGRESS.value +
+            AND + ProviderTableMeta.UPLOADS_ACCOUNT_NAME + IS_EQUAL;
+        return getUploadPage(QUERY_PAGE_SIZE, afterId, false, selection, accountName);
+    }
+
+
     /**
      * Gets a page of uploads after <code>afterId</code>, where uploads are sorted by ascending upload id.
      * <p>
      * If <code>afterId</code> is -1, returns the first page
      */
     public List<OCUpload> getCurrentAndPendingUploadsForAccountPageAscById(final long afterId, final @NonNull String accountName) {
-        final String selection = getInProgressUploadsSelection();
+        final String selection = getInProgressAndDelayedUploadsSelection();
         return getUploadPage(QUERY_PAGE_SIZE, afterId, false, selection, accountName);
     }
 

app/src/main/java/com/owncloud/android/utils/FilesSyncHelper.java

@@ -108,7 +108,7 @@ public final class FilesSyncHelper {
         }
     }
 
-    private static void insertAllDBEntriesForSyncedFolder(SyncedFolder syncedFolder) {
+    public static void insertAllDBEntriesForSyncedFolder(SyncedFolder syncedFolder) {
         final Context context = MainApp.getAppContext();
         final ContentResolver contentResolver = context.getContentResolver();
 
@@ -146,18 +146,6 @@ public final class FilesSyncHelper {
         }
     }
 
-    public static void insertAllDBEntries(SyncedFolder syncedFolder,
-                                          PowerManagementService powerManagementService) {
-        if (syncedFolder.isEnabled() &&
-            !(syncedFolder.isChargingOnly() &&
-                !powerManagementService.getBattery().isCharging() &&
-                !powerManagementService.getBattery().isFull()
-            )
-        ) {
-            insertAllDBEntriesForSyncedFolder(syncedFolder);
-        }
-    }
-
     public static void insertChangedEntries(SyncedFolder syncedFolder,
                                             String[] changedFiles) {
         final ContentResolver contentResolver = MainApp.getAppContext().getContentResolver();
@@ -248,66 +236,13 @@ public final class FilesSyncHelper {
                                               final UserAccountManager accountManager,
                                               final ConnectivityService connectivityService,
                                               final PowerManagementService powerManagementService) {
-        boolean accountExists;
-
-        boolean whileChargingOnly = true;
-        boolean useWifiOnly = true;
-
-        // Make all in progress downloads failed to restart upload worker
-        uploadsStorageManager.failInProgressUploads(UploadResult.SERVICE_INTERRUPTED);
-
-        OCUpload[] failedUploads = uploadsStorageManager.getFailedUploads();
-
-        for (OCUpload failedUpload : failedUploads) {
-            accountExists = false;
-            if (!failedUpload.isWhileChargingOnly()) {
-                whileChargingOnly = false;
-            }
-            if (!failedUpload.isUseWifiOnly()) {
-                useWifiOnly = false;
-            }
-
-            // check if accounts still exists
-            for (Account account : accountManager.getAccounts()) {
-                if (account.name.equals(failedUpload.getAccountName())) {
-                    accountExists = true;
-                    break;
-                }
-            }
-
-            if (!accountExists) {
-                uploadsStorageManager.removeUpload(failedUpload);
-            }
-        }
-
-        failedUploads = uploadsStorageManager.getFailedUploads();
-        if (failedUploads.length == 0) {
-            //nothing to do
-            return;
-        }
-
-        if (whileChargingOnly) {
-            final BatteryStatus batteryStatus = powerManagementService.getBattery();
-            final boolean charging = batteryStatus.isCharging() || batteryStatus.isFull();
-            if (!charging) {
-                //all uploads requires charging
-                return;
-            }
-        }
-
-        if (useWifiOnly && !connectivityService.getConnectivity().isWifi()) {
-            //all uploads requires wifi
-            return;
-        }
-
+        
         new Thread(() -> {
-            if (connectivityService.getConnectivity().isConnected()) {
-                FileUploadHelper.Companion.instance().retryFailedUploads(
-                    uploadsStorageManager,
-                    connectivityService,
-                    accountManager,
-                    powerManagementService);
-            }
+            FileUploadHelper.Companion.instance().retryFailedUploads(
+                uploadsStorageManager,
+                connectivityService,
+                accountManager,
+                powerManagementService);
         }).start();
     }