From e8e44c094fc1b76724dc619e46888129fe5ffbfe Mon Sep 17 00:00:00 2001 From: tobiasKaminsky Date: Wed, 27 May 2020 07:34:58 +0200 Subject: [PATCH 1/4] - first check constraints, then get failed uploads based on this constraints only - enhanced remove uploads with no accounts associated - use List instead of OCUpload[] Signed-off-by: tobiasKaminsky --- .../datamodel/UploadStorageManagerTest.java | 209 ++++++++++++++++-- .../datamodel/UploadsStorageManager.java | 110 +++++---- .../com/owncloud/android/db/OCUpload.java | 16 +- .../android/files/services/FileUploader.java | 75 +++---- .../ui/activity/UploadListActivity.java | 4 +- .../android/ui/adapter/UploadListAdapter.java | 30 +-- .../android/utils/FilesSyncHelper.java | 26 +-- 7 files changed, 321 insertions(+), 149 deletions(-) diff --git a/src/androidTest/java/com/owncloud/android/datamodel/UploadStorageManagerTest.java b/src/androidTest/java/com/owncloud/android/datamodel/UploadStorageManagerTest.java index 64fa12a2da81..3fdbbcc5210b 100644 --- a/src/androidTest/java/com/owncloud/android/datamodel/UploadStorageManagerTest.java +++ b/src/androidTest/java/com/owncloud/android/datamodel/UploadStorageManagerTest.java @@ -26,12 +26,14 @@ import java.io.File; import java.util.ArrayList; +import java.util.List; import java.util.Random; import androidx.test.InstrumentationRegistry; import androidx.test.filters.SmallTest; import androidx.test.runner.AndroidJUnit4; +import static com.owncloud.android.datamodel.UploadsStorageManager.UploadStatus.UPLOAD_FAILED; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -94,7 +96,7 @@ public void largeTest() { int size = 3000; ArrayList uploads = new ArrayList<>(); - assertEquals(0, uploadsStorageManager.getAllStoredUploads().length); + assertEquals(0, uploadsStorageManager.getAllStoredUploads().size()); for (int i = 0; i < size; i++) { OCUpload upload = createUpload(account); @@ -103,11 +105,11 @@ public void largeTest() { uploadsStorageManager.storeUpload(upload); } - OCUpload[] storedUploads = uploadsStorageManager.getAllStoredUploads(); - assertEquals(size, uploadsStorageManager.getAllStoredUploads().length); + List storedUploads = uploadsStorageManager.getAllStoredUploads(); + assertEquals(size, uploadsStorageManager.getAllStoredUploads().size()); for (int i = 0; i < size; i++) { - assertTrue(contains(uploads, storedUploads[i])); + assertTrue(contains(uploads, storedUploads.get(i))); } } @@ -127,15 +129,6 @@ public void testIsSame() { assertFalse(upload1.isSame(new OCFile("/test"))); } - private boolean contains(ArrayList uploads, OCUpload storedUpload) { - for (int i = 0; i < uploads.size(); i++) { - if (storedUpload.isSame(uploads.get(i))) { - return true; - } - } - return false; - } - @Test(expected = IllegalArgumentException.class) public void corruptedUpload() { OCUpload corruptUpload = new OCUpload(File.separator + "LocalPath", @@ -149,6 +142,192 @@ public void corruptedUpload() { uploadsStorageManager.getAllStoredUploads(); } + @Test + public void testConstraintsWithFailedUploads() { + // no constraints + uploadsStorageManager.storeUpload(new OCUpload("/test", "/test", account.name) + .setUploadStatus(UPLOAD_FAILED) + .setUseWifiOnly(false) + .setWhileChargingOnly(false)); + + uploadsStorageManager.storeUpload(new OCUpload("/test", "/test", account.name) + .setUploadStatus(UPLOAD_FAILED) + .setLastResult(UploadResult.LOCK_FAILED) + .setUseWifiOnly(false) + .setWhileChargingOnly(false)); + + // wifi only + uploadsStorageManager.storeUpload(new OCUpload("/test", "/test", account.name) + .setUploadStatus(UPLOAD_FAILED) + .setUseWifiOnly(true) + .setWhileChargingOnly(false)); + + uploadsStorageManager.storeUpload(new OCUpload("/test", "/test", account.name) + .setUploadStatus(UPLOAD_FAILED) + .setUseWifiOnly(true) + .setWhileChargingOnly(false)); + + // charging only + uploadsStorageManager.storeUpload(new OCUpload("/test", "/test", account.name) + .setUploadStatus(UPLOAD_FAILED) + .setUseWifiOnly(false) + .setWhileChargingOnly(true)); + + uploadsStorageManager.storeUpload(new OCUpload("/test", "/test", account.name) + .setUploadStatus(UPLOAD_FAILED) + .setUseWifiOnly(false) + .setWhileChargingOnly(true)); + + uploadsStorageManager.storeUpload(new OCUpload("/test", "/test", account.name) + .setUploadStatus(UPLOAD_FAILED) + .setUseWifiOnly(false) + .setWhileChargingOnly(true)); + + // wifi only, charging only + uploadsStorageManager.storeUpload(new OCUpload("/test", "/test", account.name) + .setUploadStatus(UPLOAD_FAILED) + .setUseWifiOnly(true) + .setWhileChargingOnly(true)); + + uploadsStorageManager.storeUpload(new OCUpload("/test", "/test", account.name) + .setUploadStatus(UPLOAD_FAILED) + .setUseWifiOnly(true) + .setWhileChargingOnly(true)); + + uploadsStorageManager.storeUpload(new OCUpload("/test", "/test", account.name) + .setUploadStatus(UPLOAD_FAILED) + .setUseWifiOnly(true) + .setWhileChargingOnly(true)); + + uploadsStorageManager.storeUpload(new OCUpload("/test", "/test", account.name) + .setUploadStatus(UPLOAD_FAILED) + .setUseWifiOnly(true) + .setWhileChargingOnly(true)); + + // should not automatically retried + uploadsStorageManager.storeUpload(new OCUpload("/test", "/test", account.name) + .setUploadStatus(UPLOAD_FAILED) + .setLastResult(UploadResult.SYNC_CONFLICT) + .setUseWifiOnly(false) + .setWhileChargingOnly(false)); + + uploadsStorageManager.storeUpload(new OCUpload("/test", "/test", account.name) + .setUploadStatus(UPLOAD_FAILED) + .setLastResult(UploadResult.FILE_NOT_FOUND) + .setUseWifiOnly(false) + .setWhileChargingOnly(false)); + + assertEquals(11, uploadsStorageManager.getFailedUploads().size()); + assertEquals(2, uploadsStorageManager.getFailedUploads(false, false).size()); + assertEquals(2, uploadsStorageManager.getFailedUploads(true, false).size()); + assertEquals(3, uploadsStorageManager.getFailedUploads(false, true).size()); + assertEquals(4, uploadsStorageManager.getFailedUploads(true, true).size()); + + // scenario "no wifi, no charging" + List failedUploads; + failedUploads = uploadsStorageManager.getFailedUploads(false, false); + assertEquals(2, failedUploads.size()); + failedUploads.clear(); + + // scenario "wifi, no charging" + failedUploads = uploadsStorageManager.getFailedUploads(false, false); + failedUploads.addAll(uploadsStorageManager.getFailedUploads(true, false)); + assertEquals(4, failedUploads.size()); + failedUploads.clear(); + + // scenario "no wifi, charging" + failedUploads = uploadsStorageManager.getFailedUploads(false, false); + failedUploads.addAll(uploadsStorageManager.getFailedUploads(false, true)); + assertEquals(5, failedUploads.size()); + failedUploads.clear(); + + // scenario "wifi, charging" + failedUploads = uploadsStorageManager.getFailedUploads(false, false); + failedUploads.addAll(uploadsStorageManager.getFailedUploads(true, false)); + failedUploads.addAll(uploadsStorageManager.getFailedUploads(false, true)); + failedUploads.addAll(uploadsStorageManager.getFailedUploads(true, true)); + assertEquals(11, failedUploads.size()); + failedUploads.clear(); + } + + @Test + public void testDeleteUploadsWithOneValidAccount() { + uploadsStorageManager.storeUpload(new OCUpload("/test", "/test", account.name) + .setUploadStatus(UPLOAD_FAILED)); + uploadsStorageManager.storeUpload(new OCUpload("/test", "/test", "oldAccount1") + .setUploadStatus(UPLOAD_FAILED)); + uploadsStorageManager.storeUpload(new OCUpload("/test", "/test", "oldAccount2") + .setUploadStatus(UPLOAD_FAILED)); + uploadsStorageManager.storeUpload(new OCUpload("/test", "/test", "oldAccount3") + .setUploadStatus(UPLOAD_FAILED)); + + assertEquals(4, uploadsStorageManager.getFailedUploads().size()); + + ArrayList accountList = new ArrayList<>(); + accountList.add(account); + uploadsStorageManager.removeUploadsWithExpiredUsers(accountList); + + assertEquals(1, uploadsStorageManager.getFailedUploads().size()); + } + + @Test + public void testDeleteUploadsWithTwoValidAccount() { + uploadsStorageManager.storeUpload(new OCUpload("/test", "/test", account.name) + .setUploadStatus(UPLOAD_FAILED)); + uploadsStorageManager.storeUpload(new OCUpload("/test", "/test", account2.name) + .setUploadStatus(UPLOAD_FAILED)); + uploadsStorageManager.storeUpload(new OCUpload("/test", "/test", "oldAccount1") + .setUploadStatus(UPLOAD_FAILED)); + uploadsStorageManager.storeUpload(new OCUpload("/test", "/test", "oldAccount2") + .setUploadStatus(UPLOAD_FAILED)); + + assertEquals(4, uploadsStorageManager.getFailedUploads().size()); + + ArrayList accountList = new ArrayList<>(); + accountList.add(account); + accountList.add(account2); + uploadsStorageManager.removeUploadsWithExpiredUsers(accountList); + + assertEquals(2, uploadsStorageManager.getFailedUploads().size()); + } + + @Test + public void testDeleteUploadsWithManyValidAccount() { + Account account3 = new Account("test3@server.com", "Test-Type"); + + uploadsStorageManager.storeUpload(new OCUpload("/test", "/test", account.name) + .setUploadStatus(UPLOAD_FAILED)); + uploadsStorageManager.storeUpload(new OCUpload("/test", "/test", account2.name) + .setUploadStatus(UPLOAD_FAILED)); + uploadsStorageManager.storeUpload(new OCUpload("/test", "/test", account3.name) + .setUploadStatus(UPLOAD_FAILED)); + uploadsStorageManager.storeUpload(new OCUpload("/test", "/test", "oldAccount1") + .setUploadStatus(UPLOAD_FAILED)); + uploadsStorageManager.storeUpload(new OCUpload("/test", "/test", "oldAccount2") + .setUploadStatus(UPLOAD_FAILED)); + uploadsStorageManager.storeUpload(new OCUpload("/test", "/test", "oldAccount3") + .setUploadStatus(UPLOAD_FAILED)); + + assertEquals(6, uploadsStorageManager.getFailedUploads().size()); + + ArrayList accountList = new ArrayList<>(); + accountList.add(account); + accountList.add(account2); + accountList.add(account3); + uploadsStorageManager.removeUploadsWithExpiredUsers(accountList); + + assertEquals(3, uploadsStorageManager.getFailedUploads().size()); + } + + private boolean contains(ArrayList uploads, OCUpload storedUpload) { + for (int i = 0; i < uploads.size(); i++) { + if (storedUpload.isSame(uploads.get(i))) { + return true; + } + } + return false; + } + private void insertUploads(Account account, int rowsToInsert) { for (int i = 0; i < rowsToInsert; i++) { uploadsStorageManager.storeUpload(createUpload(account)); @@ -177,9 +356,7 @@ private OCUpload createUpload(Account account) { @After public void tearDown() { - for (Account account : getAllAccounts()) { - uploadsStorageManager.removeAccountUploads(account); - } + uploadsStorageManager.removeAllUploads(); AccountManager platformAccountManager = AccountManager.get(targetContext); platformAccountManager.removeAccountExplicitly(account2); diff --git a/src/main/java/com/owncloud/android/datamodel/UploadsStorageManager.java b/src/main/java/com/owncloud/android/datamodel/UploadsStorageManager.java index 9ba7f18682a4..982da68bcf32 100644 --- a/src/main/java/com/owncloud/android/datamodel/UploadsStorageManager.java +++ b/src/main/java/com/owncloud/android/datamodel/UploadsStorageManager.java @@ -42,6 +42,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Calendar; +import java.util.List; import java.util.Locale; import java.util.Observable; @@ -282,11 +283,15 @@ public int removeUploads(String accountName) { return result; } - public OCUpload[] getAllStoredUploads() { + void removeAllUploads() { + getDB().delete(ProviderTableMeta.CONTENT_URI_UPLOADS, "1 = 1", null); + } + + public List getAllStoredUploads() { return getUploads(null, (String[]) null); } - private OCUpload[] getUploads(@Nullable String selection, @Nullable String... selectionArgs) { + private List getUploads(@Nullable String selection, @Nullable String... selectionArgs) { ArrayList uploads = new ArrayList<>(); final long pageSize = 100; long page = 0; @@ -358,7 +363,7 @@ private OCUpload[] getUploads(@Nullable String selection, @Nullable String... se page )); - return uploads.toArray(new OCUpload[0]); + return uploads; } private OCUpload createOCUploadFromCursor(Cursor c) { @@ -391,7 +396,7 @@ private OCUpload createOCUploadFromCursor(Cursor c) { return upload; } - public OCUpload[] getCurrentAndPendingUploadsForCurrentAccount() { + public List getCurrentAndPendingUploadsForCurrentAccount() { User user = currentAccountProvider.getUser(); return getUploads(ProviderTableMeta.UPLOADS_STATUS + "==" + UploadStatus.UPLOAD_IN_PROGRESS.value + @@ -407,40 +412,66 @@ public OCUpload[] getCurrentAndPendingUploadsForCurrentAccount() { user.getAccountName()); } - /** - * Get all failed uploads. - */ - public OCUpload[] getFailedUploads() { - return getUploads("(" + ProviderTableMeta.UPLOADS_STATUS + "== ?" + - " OR " + ProviderTableMeta.UPLOADS_LAST_RESULT + - "==" + UploadResult.DELAYED_FOR_WIFI.getValue() + - " OR " + ProviderTableMeta.UPLOADS_LAST_RESULT + - "==" + UploadResult.LOCK_FAILED.getValue() + - " OR " + ProviderTableMeta.UPLOADS_LAST_RESULT + - "==" + UploadResult.DELAYED_FOR_CHARGING.getValue() + - " OR " + ProviderTableMeta.UPLOADS_LAST_RESULT + - "==" + UploadResult.DELAYED_IN_POWER_SAVE_MODE.getValue() + - " ) AND " + ProviderTableMeta.UPLOADS_LAST_RESULT + - "!= " + UploadResult.VIRUS_DETECTED.getValue() - , String.valueOf(UploadStatus.UPLOAD_FAILED.value)); + + List getFailedUploads() { + return getFailedUploads(null, null); } - public OCUpload[] getFinishedUploadsForCurrentAccount() { - User user = currentAccountProvider.getUser(); + public List getFailedUploads(Boolean wifiOnly, Boolean chargingOnly) { + String query = "(" + ProviderTableMeta.UPLOADS_STATUS + "== " + + UploadStatus.UPLOAD_FAILED.getValue() + + " OR " + ProviderTableMeta.UPLOADS_LAST_RESULT + + "==" + UploadResult.DELAYED_FOR_WIFI.getValue() + + " OR " + ProviderTableMeta.UPLOADS_LAST_RESULT + + "==" + UploadResult.LOCK_FAILED.getValue() + + " OR " + ProviderTableMeta.UPLOADS_LAST_RESULT + + "==" + UploadResult.DELAYED_FOR_CHARGING.getValue() + + " OR " + ProviderTableMeta.UPLOADS_LAST_RESULT + + "==" + UploadResult.DELAYED_IN_POWER_SAVE_MODE.getValue() + + " ) AND " + ProviderTableMeta.UPLOADS_LAST_RESULT + + "!= " + UploadResult.VIRUS_DETECTED.getValue() + + " AND " + ProviderTableMeta.UPLOADS_LAST_RESULT + + " != " + UploadResult.SYNC_CONFLICT.getValue() + + " AND " + ProviderTableMeta.UPLOADS_LAST_RESULT + + " != " + UploadResult.FILE_NOT_FOUND.getValue(); + + if (wifiOnly != null) { + query += " AND " + ProviderTableMeta.UPLOADS_IS_WIFI_ONLY + " == " + (wifiOnly ? 1 : 0); + } - return getUploads(ProviderTableMeta.UPLOADS_STATUS + "==" + UploadStatus.UPLOAD_SUCCEEDED.value + AND + - ProviderTableMeta.UPLOADS_ACCOUNT_NAME + "== ?", user.getAccountName()); + if (chargingOnly != null) { + query += " AND " + ProviderTableMeta.UPLOADS_IS_WHILE_CHARGING_ONLY + " == " + (chargingOnly ? 1 : 0); + } + + return getUploads(query); } - /** - * Get all uploads which where successfully completed. - */ - public OCUpload[] getFinishedUploads() { + public void removeUploadsWithExpiredUsers(List existingAccounts) { + String[] existingAccountNames = new String[existingAccounts.size()]; + for (int i = 0; i < existingAccounts.size(); i++) { + existingAccountNames[i] = existingAccounts.get(i).name; + } + + String where = ""; + for (int i = 0; i < existingAccounts.size(); i++) { + where += ProviderTableMeta.UPLOADS_ACCOUNT_NAME + " != ? "; - return getUploads(ProviderTableMeta.UPLOADS_STATUS + "==" + UploadStatus.UPLOAD_SUCCEEDED.value, (String[]) null); + if (i < existingAccounts.size() - 1) { + where += " AND "; + } + } + + getDB().delete(ProviderTableMeta.CONTENT_URI_UPLOADS, where, existingAccountNames); } - public OCUpload[] getFailedButNotDelayedUploadsForCurrentAccount() { + public List getFinishedUploadsForCurrentAccount() { + User user = currentAccountProvider.getUser(); + + return getUploads(ProviderTableMeta.UPLOADS_STATUS + "==" + UploadStatus.UPLOAD_SUCCEEDED.value + AND + + ProviderTableMeta.UPLOADS_ACCOUNT_NAME + "== ?", user.getAccountName()); + } + + public List getFailedButNotDelayedUploadsForCurrentAccount() { User user = currentAccountProvider.getUser(); return getUploads(ProviderTableMeta.UPLOADS_STATUS + "==" + UploadStatus.UPLOAD_FAILED.value + @@ -456,25 +487,6 @@ public OCUpload[] getFailedButNotDelayedUploadsForCurrentAccount() { user.getAccountName()); } - /** - * Get all failed uploads, except for those that were not performed due to lack of Wifi connection. - * - * @return Array of failed uploads, except for those that were not performed due to lack of Wifi connection. - */ - public OCUpload[] getFailedButNotDelayedUploads() { - - return getUploads(ProviderTableMeta.UPLOADS_STATUS + "==" + UploadStatus.UPLOAD_FAILED.value + AND + - ProviderTableMeta.UPLOADS_LAST_RESULT + "<>" + UploadResult.LOCK_FAILED.getValue() + - AND + ProviderTableMeta.UPLOADS_LAST_RESULT + - "<>" + UploadResult.DELAYED_FOR_WIFI.getValue() + - AND + ProviderTableMeta.UPLOADS_LAST_RESULT + - "<>" + UploadResult.DELAYED_FOR_CHARGING.getValue() + - AND + ProviderTableMeta.UPLOADS_LAST_RESULT + - "<>" + UploadResult.DELAYED_IN_POWER_SAVE_MODE.getValue(), - (String[]) null - ); - } - private ContentResolver getDB() { return mContentResolver; } diff --git a/src/main/java/com/owncloud/android/db/OCUpload.java b/src/main/java/com/owncloud/android/db/OCUpload.java index 8b230e5d57a3..11d3f5ee7731 100644 --- a/src/main/java/com/owncloud/android/db/OCUpload.java +++ b/src/main/java/com/owncloud/android/db/OCUpload.java @@ -194,16 +194,20 @@ public void setDataFixed(FileUploader.FileUploaderBinder binder) { * Sets uploadStatus AND SETS lastResult = null; * @param uploadStatus the uploadStatus to set */ - public void setUploadStatus(UploadStatus uploadStatus) { + public OCUpload setUploadStatus(UploadStatus uploadStatus) { this.uploadStatus = uploadStatus; setLastResult(UploadResult.UNKNOWN); + + return this; } /** * @param lastResult the lastResult to set */ - public void setLastResult(UploadResult lastResult) { + public OCUpload setLastResult(UploadResult lastResult) { this.lastResult = lastResult != null ? lastResult : UploadResult.UNKNOWN; + + return this; } public UploadStatus getFixedUploadStatus() { @@ -440,12 +444,16 @@ public void setUploadEndTimestamp(long uploadEndTimestamp) { this.uploadEndTimestamp = uploadEndTimestamp; } - public void setUseWifiOnly(boolean useWifiOnly) { + public OCUpload setUseWifiOnly(boolean useWifiOnly) { this.useWifiOnly = useWifiOnly; + + return this; } - public void setWhileChargingOnly(boolean whileChargingOnly) { + public OCUpload setWhileChargingOnly(boolean whileChargingOnly) { this.whileChargingOnly = whileChargingOnly; + + return this; } public void setFolderUnlockToken(String folderUnlockToken) { diff --git a/src/main/java/com/owncloud/android/files/services/FileUploader.java b/src/main/java/com/owncloud/android/files/services/FileUploader.java index 5940e28767e9..00072a644b5d 100644 --- a/src/main/java/com/owncloud/android/files/services/FileUploader.java +++ b/src/main/java/com/owncloud/android/files/services/FileUploader.java @@ -79,6 +79,7 @@ import java.io.File; import java.util.ArrayList; +import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -988,61 +989,61 @@ public static void retryUpload(@NonNull Context context, @NonNull Account accoun * Retry a subset of all the stored failed uploads. * * @param context Caller {@link Context} - * @param account If not null, only failed uploads to this OC account will be retried; otherwise, uploads of - * all accounts will be retried. - * @param uploadResult If not null, only failed uploads with the result specified will be retried; otherwise, failed - * uploads due to any result will be retried. */ public static void retryFailedUploads( @NonNull final Context context, - @Nullable final Account account, @NonNull final UploadsStorageManager uploadsStorageManager, @NonNull final ConnectivityService connectivityService, @NonNull final UserAccountManager accountManager, - @NonNull final PowerManagementService powerManagementService, - @Nullable final UploadResult uploadResult + @NonNull final PowerManagementService powerManagementService ) { - OCUpload[] failedUploads = uploadsStorageManager.getFailedUploads(); - Account currentAccount = null; - boolean resultMatch; - boolean accountMatch; - final Connectivity connectivity = connectivityService.getConnectivity(); final boolean gotNetwork = connectivity.isConnected() && !connectivityService.isInternetWalled(); + final boolean isPowerSaving = powerManagementService.isPowerSavingEnabled(); + + if (!gotNetwork || isPowerSaving) { + // do nothing if no internet or power saving + return; + } + + // always clean up + uploadsStorageManager.removeUploadsWithExpiredUsers(Arrays.asList(accountManager.getAccounts())); + final boolean gotWifi = connectivity.isWifi(); final BatteryStatus batteryStatus = powerManagementService.getBattery(); final boolean charging = batteryStatus.isCharging() || batteryStatus.isFull(); - final boolean isPowerSaving = powerManagementService.isPowerSavingEnabled(); - for (OCUpload failedUpload : failedUploads) { - accountMatch = account == null || account.name.equals(failedUpload.getAccountName()); - resultMatch = uploadResult == null || uploadResult == failedUpload.getLastResult(); - if (accountMatch && resultMatch) { - if (currentAccount == null || !currentAccount.name.equals(failedUpload.getAccountName())) { - currentAccount = failedUpload.getAccount(accountManager); - } + // pre-filter on failed uploads + List failedUploads; - if (!new File(failedUpload.getLocalPath()).exists()) { - if (failedUpload.getLastResult() != UploadResult.FILE_NOT_FOUND) { - failedUpload.setLastResult(UploadResult.FILE_NOT_FOUND); - uploadsStorageManager.updateUpload(failedUpload); - } - } else { + // find all without constraints + failedUploads = uploadsStorageManager.getFailedUploads(false, false); - if (!isPowerSaving && gotNetwork && canUploadBeRetried(failedUpload, gotWifi, charging)) { - retryUpload(context, currentAccount, failedUpload); - } - } - } + // wifi only + if (gotWifi) { + failedUploads.addAll(uploadsStorageManager.getFailedUploads(true, false)); } - } - private static boolean canUploadBeRetried(OCUpload upload, boolean gotWifi, boolean isCharging) { - File file = new File(upload.getLocalPath()); - boolean needsWifi = upload.isUseWifiOnly(); - boolean needsCharging = upload.isWhileChargingOnly(); + // battery only + if (charging) { + failedUploads.addAll(uploadsStorageManager.getFailedUploads(false, true)); + } + + // battery and wifi only + if (gotWifi && charging) { + failedUploads.addAll(uploadsStorageManager.getFailedUploads(true, true)); + } - return file.exists() && (!needsWifi || gotWifi) && (!needsCharging || isCharging); + for (OCUpload failedUpload : failedUploads) { + if (!new File(failedUpload.getLocalPath()).exists()) { + if (failedUpload.getLastResult() != UploadResult.FILE_NOT_FOUND) { + failedUpload.setLastResult(UploadResult.FILE_NOT_FOUND); + uploadsStorageManager.updateUpload(failedUpload); + } + } else { + retryUpload(context, failedUpload.getAccount(accountManager), failedUpload); + } + } } public static String getUploadsAddedMessage() { diff --git a/src/main/java/com/owncloud/android/ui/activity/UploadListActivity.java b/src/main/java/com/owncloud/android/ui/activity/UploadListActivity.java index 83d353f24f25..1cbed7e04ed0 100755 --- a/src/main/java/com/owncloud/android/ui/activity/UploadListActivity.java +++ b/src/main/java/com/owncloud/android/ui/activity/UploadListActivity.java @@ -182,12 +182,10 @@ private void refresh() { // retry failed uploads new Thread(() -> FileUploader.retryFailedUploads( this, - null, uploadsStorageManager, connectivityService, userAccountManager, - powerManagementService, - null + powerManagementService )).start(); // update UI diff --git a/src/main/java/com/owncloud/android/ui/adapter/UploadListAdapter.java b/src/main/java/com/owncloud/android/ui/adapter/UploadListAdapter.java index eef8dbed9fca..6868fd90c9f3 100755 --- a/src/main/java/com/owncloud/android/ui/adapter/UploadListAdapter.java +++ b/src/main/java/com/owncloud/android/ui/adapter/UploadListAdapter.java @@ -71,7 +71,9 @@ import com.owncloud.android.utils.ThemeUtils; import java.io.File; -import java.util.Arrays; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; import androidx.annotation.NonNull; import butterknife.BindView; @@ -101,7 +103,7 @@ public int getSectionCount() { @Override public int getItemCount(int section) { - return uploadGroups[section].getItems().length; + return uploadGroups[section].getItems().size(); } @Override @@ -145,12 +147,10 @@ public void onBindHeaderViewHolder(SectionedViewHolder holder, int section, bool case FAILED: new Thread(() -> FileUploader.retryFailedUploads( parentActivity, - null, uploadsStorageManager, connectivityService, accountManager, - powerManagementService, - null + powerManagementService )).start(); break; @@ -810,44 +810,44 @@ enum Type { abstract class UploadGroup implements Refresh { private Type type; - private OCUpload[] items; + private List items; private String name; UploadGroup(Type type, String groupName) { this.type = type; this.name = groupName; - items = new OCUpload[0]; + items = new ArrayList<>(); } private String getGroupName() { return name; } - public OCUpload[] getItems() { + public List getItems() { return items; } public OCUpload getItem(int position) { - return items[position]; + return items.get(position); } - public void setItems(OCUpload... items) { + public void setItems(List items) { this.items = items; } - void fixAndSortItems(OCUpload... array) { + void fixAndSortItems(List list) { FileUploader.FileUploaderBinder binder = parentActivity.getFileUploaderBinder(); - for (OCUpload upload : array) { + for (OCUpload upload : list) { upload.setDataFixed(binder); } - Arrays.sort(array, new OCUploadComparator()); + Collections.sort(list, new OCUploadComparator()); - setItems(array); + setItems(list); } private int getGroupItemCount() { - return items == null ? 0 : items.length; + return items == null ? 0 : items.size(); } } } diff --git a/src/main/java/com/owncloud/android/utils/FilesSyncHelper.java b/src/main/java/com/owncloud/android/utils/FilesSyncHelper.java index 3ccfa80ae448..b12a777550c2 100644 --- a/src/main/java/com/owncloud/android/utils/FilesSyncHelper.java +++ b/src/main/java/com/owncloud/android/utils/FilesSyncHelper.java @@ -23,7 +23,6 @@ */ package com.owncloud.android.utils; -import android.accounts.Account; import android.content.ContentResolver; import android.content.Context; import android.database.Cursor; @@ -43,7 +42,6 @@ import com.owncloud.android.datamodel.SyncedFolder; import com.owncloud.android.datamodel.SyncedFolderProvider; import com.owncloud.android.datamodel.UploadsStorageManager; -import com.owncloud.android.db.OCUpload; import com.owncloud.android.files.services.FileUploader; import com.owncloud.android.lib.common.utils.Log_OC; @@ -183,36 +181,14 @@ public static void restartJobsIfNeeded(final UploadsStorageManager uploadsStorag final PowerManagementService powerManagementService) { final Context context = MainApp.getAppContext(); - boolean accountExists; - - OCUpload[] failedUploads = uploadsStorageManager.getFailedUploads(); - - for (OCUpload failedUpload : failedUploads) { - accountExists = 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); - } - } - new Thread(() -> { if (connectivityService.getConnectivity().isConnected() && !connectivityService.isInternetWalled()) { FileUploader.retryFailedUploads( context, - null, uploadsStorageManager, connectivityService, accountManager, - powerManagementService, - null + powerManagementService ); } }).start(); From ddf71762372c0c648319bae3686f0b2a348176c7 Mon Sep 17 00:00:00 2001 From: tobiasKaminsky Date: Fri, 29 May 2020 10:30:10 +0200 Subject: [PATCH 2/4] sort failed uploads according to UI Signed-off-by: tobiasKaminsky --- .../android/files/services/FileUploader.java | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/main/java/com/owncloud/android/files/services/FileUploader.java b/src/main/java/com/owncloud/android/files/services/FileUploader.java index 00072a644b5d..d605a5276b5a 100644 --- a/src/main/java/com/owncloud/android/files/services/FileUploader.java +++ b/src/main/java/com/owncloud/android/files/services/FileUploader.java @@ -61,6 +61,7 @@ import com.owncloud.android.datamodel.UploadsStorageManager; import com.owncloud.android.datamodel.UploadsStorageManager.UploadStatus; import com.owncloud.android.db.OCUpload; +import com.owncloud.android.db.OCUploadComparator; import com.owncloud.android.db.UploadResult; import com.owncloud.android.lib.common.OwnCloudAccount; import com.owncloud.android.lib.common.OwnCloudClient; @@ -80,6 +81,7 @@ import java.io.File; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -303,7 +305,13 @@ public int onStartCommand(Intent intent, int flags, int startId) { boolean onWifiOnly = intent.getBooleanExtra(KEY_WHILE_ON_WIFI_ONLY, false); boolean whileChargingOnly = intent.getBooleanExtra(KEY_WHILE_CHARGING_ONLY, false); - if (!retry) { // Start new uploads + if (retry) { // Retry uploads + if (!intent.hasExtra(KEY_ACCOUNT) || !intent.hasExtra(KEY_RETRY_UPLOAD)) { + Log_OC.e(TAG, "Not enough information provided in intent: no KEY_RETRY_UPLOAD_KEY"); + return START_NOT_STICKY; + } + retryUploads(intent, account, requestedUploads); + } else { // Start new uploads if (!(intent.hasExtra(KEY_LOCAL_FILE) || intent.hasExtra(KEY_FILE))) { Log_OC.e(TAG, "Not enough information provided in intent"); return Service.START_NOT_STICKY; @@ -313,12 +321,6 @@ public int onStartCommand(Intent intent, int flags, int startId) { if (error != null) { return error; } - } else { // Retry uploads - if (!intent.hasExtra(KEY_ACCOUNT) || !intent.hasExtra(KEY_RETRY_UPLOAD)) { - Log_OC.e(TAG, "Not enough information provided in intent: no KEY_RETRY_UPLOAD_KEY"); - return START_NOT_STICKY; - } - retryUploads(intent, account, requestedUploads); } if (requestedUploads.size() > 0) { @@ -1034,6 +1036,9 @@ public static void retryFailedUploads( failedUploads.addAll(uploadsStorageManager.getFailedUploads(true, true)); } + // sort them, so they are retried in same way as UI + Collections.sort(failedUploads, new OCUploadComparator()); + for (OCUpload failedUpload : failedUploads) { if (!new File(failedUpload.getLocalPath()).exists()) { if (failedUpload.getLastResult() != UploadResult.FILE_NOT_FOUND) { From 975be14ddf6367b06ef0a6de82ed68c7d7fad47c Mon Sep 17 00:00:00 2001 From: tobiasKaminsky Date: Fri, 29 May 2020 13:16:58 +0200 Subject: [PATCH 3/4] Revert "sort failed uploads according to UI" This reverts commit ddf71762372c0c648319bae3686f0b2a348176c7. --- .../android/files/services/FileUploader.java | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/src/main/java/com/owncloud/android/files/services/FileUploader.java b/src/main/java/com/owncloud/android/files/services/FileUploader.java index d605a5276b5a..00072a644b5d 100644 --- a/src/main/java/com/owncloud/android/files/services/FileUploader.java +++ b/src/main/java/com/owncloud/android/files/services/FileUploader.java @@ -61,7 +61,6 @@ import com.owncloud.android.datamodel.UploadsStorageManager; import com.owncloud.android.datamodel.UploadsStorageManager.UploadStatus; import com.owncloud.android.db.OCUpload; -import com.owncloud.android.db.OCUploadComparator; import com.owncloud.android.db.UploadResult; import com.owncloud.android.lib.common.OwnCloudAccount; import com.owncloud.android.lib.common.OwnCloudClient; @@ -81,7 +80,6 @@ import java.io.File; import java.util.ArrayList; import java.util.Arrays; -import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -305,13 +303,7 @@ public int onStartCommand(Intent intent, int flags, int startId) { boolean onWifiOnly = intent.getBooleanExtra(KEY_WHILE_ON_WIFI_ONLY, false); boolean whileChargingOnly = intent.getBooleanExtra(KEY_WHILE_CHARGING_ONLY, false); - if (retry) { // Retry uploads - if (!intent.hasExtra(KEY_ACCOUNT) || !intent.hasExtra(KEY_RETRY_UPLOAD)) { - Log_OC.e(TAG, "Not enough information provided in intent: no KEY_RETRY_UPLOAD_KEY"); - return START_NOT_STICKY; - } - retryUploads(intent, account, requestedUploads); - } else { // Start new uploads + if (!retry) { // Start new uploads if (!(intent.hasExtra(KEY_LOCAL_FILE) || intent.hasExtra(KEY_FILE))) { Log_OC.e(TAG, "Not enough information provided in intent"); return Service.START_NOT_STICKY; @@ -321,6 +313,12 @@ public int onStartCommand(Intent intent, int flags, int startId) { if (error != null) { return error; } + } else { // Retry uploads + if (!intent.hasExtra(KEY_ACCOUNT) || !intent.hasExtra(KEY_RETRY_UPLOAD)) { + Log_OC.e(TAG, "Not enough information provided in intent: no KEY_RETRY_UPLOAD_KEY"); + return START_NOT_STICKY; + } + retryUploads(intent, account, requestedUploads); } if (requestedUploads.size() > 0) { @@ -1036,9 +1034,6 @@ public static void retryFailedUploads( failedUploads.addAll(uploadsStorageManager.getFailedUploads(true, true)); } - // sort them, so they are retried in same way as UI - Collections.sort(failedUploads, new OCUploadComparator()); - for (OCUpload failedUpload : failedUploads) { if (!new File(failedUpload.getLocalPath()).exists()) { if (failedUpload.getLastResult() != UploadResult.FILE_NOT_FOUND) { From bf5eb646451e30fb0ecbaf9e761935d2433fc9f7 Mon Sep 17 00:00:00 2001 From: tobiasKaminsky Date: Tue, 2 Jun 2020 09:44:19 +0200 Subject: [PATCH 4/4] Create thumbnail only if it does not yet exist retry failed uploads only once Signed-off-by: tobiasKaminsky --- .../datamodel/ThumbnailsCacheManager.java | 4 ++-- .../android/files/services/FileUploader.java | 20 +++++++++++++------ .../ui/activity/UploadListActivity.java | 9 --------- 3 files changed, 16 insertions(+), 17 deletions(-) diff --git a/src/main/java/com/owncloud/android/datamodel/ThumbnailsCacheManager.java b/src/main/java/com/owncloud/android/datamodel/ThumbnailsCacheManager.java index 304fe6a15dd0..a889b005f9f8 100644 --- a/src/main/java/com/owncloud/android/datamodel/ThumbnailsCacheManager.java +++ b/src/main/java/com/owncloud/android/datamodel/ThumbnailsCacheManager.java @@ -681,7 +681,7 @@ private int getThumbnailDimension() { private Bitmap doFileInBackground() { File file = (File)mFile; - final String imageKey; + String imageKey; if (mImageKey != null) { imageKey = mImageKey; } else { @@ -689,7 +689,7 @@ private Bitmap doFileInBackground() { } // local file should always generate a thumbnail - mImageKey = PREFIX_THUMBNAIL + mImageKey; + imageKey = PREFIX_THUMBNAIL + imageKey; // Check disk cache in background thread Bitmap thumbnail = getBitmapFromDiskCache(imageKey); diff --git a/src/main/java/com/owncloud/android/files/services/FileUploader.java b/src/main/java/com/owncloud/android/files/services/FileUploader.java index 00072a644b5d..9c4317907883 100644 --- a/src/main/java/com/owncloud/android/files/services/FileUploader.java +++ b/src/main/java/com/owncloud/android/files/services/FileUploader.java @@ -636,14 +636,22 @@ public void uploadFile(String uploadKey) { sendBroadcastUploadFinished(mCurrentUpload, uploadResult, removeResult.second); } - // generate new Thumbnail - final ThumbnailsCacheManager.ThumbnailGenerationTask task = - new ThumbnailsCacheManager.ThumbnailGenerationTask(mStorageManager, mCurrentAccount); - + // generate new Thumbnail, only if it does not already exist File file = new File(mCurrentUpload.getOriginalStoragePath()); - String remoteId = mCurrentUpload.getFile().getRemoteId(); + String imageKey = mCurrentUpload.getFile().getRemoteId(); + + if (imageKey == null) { + imageKey = String.valueOf(file.hashCode()); + } - task.execute(new ThumbnailsCacheManager.ThumbnailGenerationTaskObject(file, remoteId)); + boolean exists = ThumbnailsCacheManager.containsBitmap(ThumbnailsCacheManager.PREFIX_THUMBNAIL + + imageKey); + if (!exists) { + final ThumbnailsCacheManager.ThumbnailGenerationTask task = + new ThumbnailsCacheManager.ThumbnailGenerationTask(mStorageManager, mCurrentAccount); + + task.execute(new ThumbnailsCacheManager.ThumbnailGenerationTaskObject(file, imageKey)); + } } } diff --git a/src/main/java/com/owncloud/android/ui/activity/UploadListActivity.java b/src/main/java/com/owncloud/android/ui/activity/UploadListActivity.java index 1cbed7e04ed0..06ba5773e86d 100755 --- a/src/main/java/com/owncloud/android/ui/activity/UploadListActivity.java +++ b/src/main/java/com/owncloud/android/ui/activity/UploadListActivity.java @@ -179,15 +179,6 @@ private void loadItems() { private void refresh() { backgroundJobManager.startImmediateFilesSyncJob(false, true); - // retry failed uploads - new Thread(() -> FileUploader.retryFailedUploads( - this, - uploadsStorageManager, - connectivityService, - userAccountManager, - powerManagementService - )).start(); - // update UI uploadListAdapter.loadUploadItemsFromDb(); swipeListRefreshLayout.setRefreshing(false);