Enhance failed uploads #6161
Enhance failed uploads #6161
Conversation
…traints only - enhanced remove uploads with no accounts associated - use List<OCUpload> instead of OCUpload[] Signed-off-by: tobiasKaminsky <[email protected]>
|
master-Unit test failed, but no output was generated. Maybe a preliminary stage failed. |
|
stable-Unit test failed, but no output was generated. Maybe a preliminary stage failed. |
|
stable-Screenshot test failed: https://www.kaminsky.me/nc-dev/android-integrationTests/14200-stable-Screenshot |
|
master-Screenshot test failed, but no output was generated. Maybe a preliminary stage failed. |
| } | ||
|
|
||
| public void setWhileChargingOnly(boolean whileChargingOnly) { | ||
| public OCUpload setWhileChargingOnly(boolean whileChargingOnly) { |
| return; | ||
| } | ||
|
|
||
| // always clean up |
There was a problem hiding this comment.
...except when there is no internet or power-saving
--> the following removal call could just be executed no matter the above (doesn't hurt the way it is now either)
There was a problem hiding this comment.
try { ... } finally { ... } can be used to deal with such situation.
|
looking good, just some minor comments |
Signed-off-by: tobiasKaminsky <[email protected]>
This reverts commit ddf7176.
retry failed uploads only once Signed-off-by: tobiasKaminsky <[email protected]>
|
APK file: https://www.kaminsky.me/nc-dev/android-artifacts/14258.apk |
Codacy392Lint
SpotBugs (new)
SpotBugs (master)
SpotBugs increased! |
| @@ -149,6 +142,192 @@ public void corruptedUpload() { | |||
| uploadsStorageManager.getAllStoredUploads(); | |||
| } | |||
|
|
|||
| @Test | |||
| public void testConstraintsWithFailedUploads() { | |||
There was a problem hiding this comment.
This test is pretty intimidating by it's size. I'd consider splitting that into smaller units.
| @@ -988,61 +997,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} | |||
There was a problem hiding this comment.
Superficial param description - safe to remove maybe?
| * Get all uploads which where successfully completed. | ||
| */ | ||
| public OCUpload[] getFinishedUploads() { | ||
| public void removeUploadsWithExpiredUsers(List<Account> existingAccounts) { |
There was a problem hiding this comment.
Seems to be a new method. Would it be possible to use List<User> instead of Account?
| return; | ||
| } | ||
|
|
||
| // always clean up |
There was a problem hiding this comment.
try { ... } finally { ... } can be used to deal with such situation.
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 [email protected]
Testing
Writing tests is very important. Please try to write some tests for your PR.
If you need help, please do not hesitate to ask in this PR for help.
unit tests
instrumented tests
UI tests