From b3e7c0b37ded5aaa6a18740ef92e4891e62f8e8a Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Fri, 14 Apr 2023 17:47:58 +0600 Subject: [PATCH 1/5] 307-Fix_on_network_loss_sync_stuck_issue issue: https://gitlab.e.foundation/e/os/backlog/-/issues/307 Fix on network loss in the middle of the sync, sync stuck until device reboot --- .../e/drive/operations/DownloadFileOperation.java | 9 +++++++++ .../e/drive/operations/UploadFileOperation.java | 15 +++++++++++++-- .../e/drive/services/SynchronizationService.java | 12 ++++++++++++ 3 files changed, 34 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/operations/DownloadFileOperation.java b/app/src/main/java/foundation/e/drive/operations/DownloadFileOperation.java index 9bc6a966..c820d73b 100644 --- a/app/src/main/java/foundation/e/drive/operations/DownloadFileOperation.java +++ b/app/src/main/java/foundation/e/drive/operations/DownloadFileOperation.java @@ -12,7 +12,9 @@ package foundation.e.drive.operations; import static com.owncloud.android.lib.common.operations.RemoteOperationResult.ResultCode.FILE_NOT_FOUND; import static com.owncloud.android.lib.common.operations.RemoteOperationResult.ResultCode.FORBIDDEN; import static com.owncloud.android.lib.common.operations.RemoteOperationResult.ResultCode.INVALID_OVERWRITE; +import static com.owncloud.android.lib.common.operations.RemoteOperationResult.ResultCode.NO_NETWORK_CONNECTION; import static com.owncloud.android.lib.common.operations.RemoteOperationResult.ResultCode.OK; +import static com.owncloud.android.lib.common.operations.RemoteOperationResult.ResultCode.WRONG_CONNECTION; import static java.nio.file.StandardCopyOption.REPLACE_EXISTING; @@ -93,6 +95,9 @@ public class DownloadFileOperation extends RemoteOperation { mustRestart = false; } + } else if (isNetworkDisconnectedResultCode(downloadResult)) { + mustRestart = false; + resultCode = downloadResult.getCode(); } else { Timber.d("Download failed: %s, %s", downloadResult.getCode(), downloadResult.getLogMessage()); resultCode = RemoteOperationResult.ResultCode.UNKNOWN_ERROR; @@ -116,6 +121,10 @@ public class DownloadFileOperation extends RemoteOperation { return new RemoteOperationResult(resultCode); } + private boolean isNetworkDisconnectedResultCode(final RemoteOperationResult result) { + RemoteOperationResult.ResultCode resultCode = result.getCode(); + return resultCode == NO_NETWORK_CONNECTION || resultCode == WRONG_CONNECTION; + } private RemoteOperationResult.ResultCode onDownloadSuccess(String tmpTargetFolderPath) { final String tmpFilePath = tmpTargetFolderPath + remoteFile.getRemotePath(); diff --git a/app/src/main/java/foundation/e/drive/operations/UploadFileOperation.java b/app/src/main/java/foundation/e/drive/operations/UploadFileOperation.java index 321df21a..ed3ebb23 100644 --- a/app/src/main/java/foundation/e/drive/operations/UploadFileOperation.java +++ b/app/src/main/java/foundation/e/drive/operations/UploadFileOperation.java @@ -31,6 +31,8 @@ import com.owncloud.android.lib.resources.users.GetUserInfoRemoteOperation; import com.owncloud.android.lib.common.operations.RemoteOperationResult.ResultCode; import java.io.File; import java.util.ArrayList; +import java.util.HashSet; +import java.util.Set; import foundation.e.drive.database.DbHelper; import foundation.e.drive.models.SyncedFileState; @@ -109,14 +111,23 @@ public class UploadFileOperation extends RemoteOperation { * @return */ private ResultCode onUploadFailure(final ResultCode uploadResult, final String fileName) { - if (uploadResult != ResultCode.CONFLICT - && uploadResult != ResultCode.QUOTA_EXCEEDED) { + if (!getHandledFailureCodes().contains(uploadResult)) { Timber.d("Upload for %s failed : %s", fileName, uploadResult); return ResultCode.UNKNOWN_ERROR; } return uploadResult; } + private Set getHandledFailureCodes() { + final Set handledResultCodes = new HashSet<>(); + handledResultCodes.add(ResultCode.CONFLICT); + handledResultCodes.add(ResultCode.QUOTA_EXCEEDED); + handledResultCodes.add(ResultCode.WRONG_CONNECTION); + handledResultCodes.add(ResultCode.NO_NETWORK_CONNECTION); + + return handledResultCodes; + } + /** * Check condition required to upload the file: * - the local file exist diff --git a/app/src/main/java/foundation/e/drive/services/SynchronizationService.java b/app/src/main/java/foundation/e/drive/services/SynchronizationService.java index 8218f5b2..195c570a 100644 --- a/app/src/main/java/foundation/e/drive/services/SynchronizationService.java +++ b/app/src/main/java/foundation/e/drive/services/SynchronizationService.java @@ -277,6 +277,9 @@ public class SynchronizationService extends Service implements OnRemoteOperation @Override public void onRemoteOperationFinish(@NonNull RemoteOperation callerOperation, @NonNull RemoteOperationResult result) { Timber.i("onRemoteOperationFinish()"); + + boolean isNetworkDisconnected = false; + if (callerOperation instanceof RemoveFileOperation && result.isSuccess()) { DbHelper .manageSyncedFileStateDB( @@ -326,6 +329,11 @@ public class SynchronizationService extends Service implements OnRemoteOperation //Case specific to DownloadFileOperation Timber.d("%s : Sync_conflict: File is already up to date", operationClassName); break; + case NO_NETWORK_CONNECTION: + case WRONG_CONNECTION: + isNetworkDisconnected = true; + Timber.d("%s : network issue: %s", operationClassName, result.getCode()); + break; } } @@ -339,6 +347,10 @@ public class SynchronizationService extends Service implements OnRemoteOperation break; } } + + if (isNetworkDisconnected) { + syncRequestQueue.clear(); + } } private void updateFailureCounter(SyncRequest request, boolean success) { -- GitLab From fdc6726c1a24bb1619f8e2b7cb49ed8b08743e4e Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Mon, 17 Apr 2023 13:04:07 +0600 Subject: [PATCH 2/5] On start sync test, if network is missing clean the queue --- .../e/drive/services/SynchronizationService.java | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/services/SynchronizationService.java b/app/src/main/java/foundation/e/drive/services/SynchronizationService.java index 195c570a..5918ddf4 100644 --- a/app/src/main/java/foundation/e/drive/services/SynchronizationService.java +++ b/app/src/main/java/foundation/e/drive/services/SynchronizationService.java @@ -224,7 +224,12 @@ public class SynchronizationService extends Service implements OnRemoteOperation } } - private void startWorker(Integer threadIndex){ + private void startWorker(Integer threadIndex) { + if (!isNetworkAvailable()) { + syncRequestQueue.clear(); + return; + } + if (!canStart(threadIndex) || isPaused()) return; final SyncRequest request = this.syncRequestQueue.poll(); //return null if empty @@ -267,11 +272,13 @@ public class SynchronizationService extends Service implements OnRemoteOperation * @return false if nogo */ private boolean canStart(int threadIndex) { - final boolean meteredNetworkAllowed = CommonUtils.isMeteredNetworkAllowed(account); final SyncWrapper syncWrapper = startedSync.get(threadIndex); + return (syncWrapper == null || !syncWrapper.isRunning()); + } - return (syncWrapper == null || !syncWrapper.isRunning()) - && CommonUtils.haveNetworkConnection(getApplicationContext(), meteredNetworkAllowed); + private boolean isNetworkAvailable() { + final boolean meteredNetworkAllowed = CommonUtils.isMeteredNetworkAllowed(account); + return CommonUtils.haveNetworkConnection(getApplicationContext(), meteredNetworkAllowed); } @Override -- GitLab From 03734361462423e350d8dbc47f420b12ed3c1628 Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Mon, 17 Apr 2023 16:47:32 +0600 Subject: [PATCH 3/5] refactor according to review --- .../e/drive/operations/DownloadFileOperation.java | 4 ++-- .../e/drive/operations/UploadFileOperation.java | 9 ++++++--- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/operations/DownloadFileOperation.java b/app/src/main/java/foundation/e/drive/operations/DownloadFileOperation.java index c820d73b..322d86b4 100644 --- a/app/src/main/java/foundation/e/drive/operations/DownloadFileOperation.java +++ b/app/src/main/java/foundation/e/drive/operations/DownloadFileOperation.java @@ -95,7 +95,7 @@ public class DownloadFileOperation extends RemoteOperation { mustRestart = false; } - } else if (isNetworkDisconnectedResultCode(downloadResult)) { + } else if (isNetworkDisconnected(downloadResult)) { mustRestart = false; resultCode = downloadResult.getCode(); } else { @@ -121,7 +121,7 @@ public class DownloadFileOperation extends RemoteOperation { return new RemoteOperationResult(resultCode); } - private boolean isNetworkDisconnectedResultCode(final RemoteOperationResult result) { + private boolean isNetworkDisconnected(final RemoteOperationResult result) { RemoteOperationResult.ResultCode resultCode = result.getCode(); return resultCode == NO_NETWORK_CONNECTION || resultCode == WRONG_CONNECTION; } diff --git a/app/src/main/java/foundation/e/drive/operations/UploadFileOperation.java b/app/src/main/java/foundation/e/drive/operations/UploadFileOperation.java index ed3ebb23..35484f2f 100644 --- a/app/src/main/java/foundation/e/drive/operations/UploadFileOperation.java +++ b/app/src/main/java/foundation/e/drive/operations/UploadFileOperation.java @@ -45,8 +45,10 @@ import timber.log.Timber; * High level Operation which upload a local file to a remote cloud storage */ public class UploadFileOperation extends RemoteOperation { - private final static String TAG = UploadFileOperation.class.getSimpleName(); public final static int FILE_SIZE_FLOOR_FOR_CHUNKED = 3072000; //3MB + + private static final Set handledFailureCodes = getHandledFailureCodes(); + private final Context context; private final SyncedFileState syncedState; private final Account account; // TODO Remove as soon as nextcloud library move all Operation to NextcloudClient instead of OwncloudClient @@ -111,14 +113,15 @@ public class UploadFileOperation extends RemoteOperation { * @return */ private ResultCode onUploadFailure(final ResultCode uploadResult, final String fileName) { - if (!getHandledFailureCodes().contains(uploadResult)) { + if (!handledFailureCodes.contains(uploadResult)) { Timber.d("Upload for %s failed : %s", fileName, uploadResult); return ResultCode.UNKNOWN_ERROR; } return uploadResult; } - private Set getHandledFailureCodes() { + @NonNull + private static Set getHandledFailureCodes() { final Set handledResultCodes = new HashSet<>(); handledResultCodes.add(ResultCode.CONFLICT); handledResultCodes.add(ResultCode.QUOTA_EXCEEDED); -- GitLab From e5a69c1af0890bbd199b169790b004a7e7bb439b Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Tue, 18 Apr 2023 12:16:19 +0600 Subject: [PATCH 4/5] refactor according to review --- .../foundation/e/drive/services/SynchronizationService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/foundation/e/drive/services/SynchronizationService.java b/app/src/main/java/foundation/e/drive/services/SynchronizationService.java index 5918ddf4..1c2bc934 100644 --- a/app/src/main/java/foundation/e/drive/services/SynchronizationService.java +++ b/app/src/main/java/foundation/e/drive/services/SynchronizationService.java @@ -224,7 +224,7 @@ public class SynchronizationService extends Service implements OnRemoteOperation } } - private void startWorker(Integer threadIndex) { + private void startWorker(int threadIndex) { if (!isNetworkAvailable()) { syncRequestQueue.clear(); return; -- GitLab From ed0c9ee829670175c2addc9d2431154680367595 Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Tue, 18 Apr 2023 13:37:24 +0600 Subject: [PATCH 5/5] refactor according to review --- .../foundation/e/drive/operations/DownloadFileOperation.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/foundation/e/drive/operations/DownloadFileOperation.java b/app/src/main/java/foundation/e/drive/operations/DownloadFileOperation.java index 322d86b4..f2549c6d 100644 --- a/app/src/main/java/foundation/e/drive/operations/DownloadFileOperation.java +++ b/app/src/main/java/foundation/e/drive/operations/DownloadFileOperation.java @@ -121,7 +121,7 @@ public class DownloadFileOperation extends RemoteOperation { return new RemoteOperationResult(resultCode); } - private boolean isNetworkDisconnected(final RemoteOperationResult result) { + private boolean isNetworkDisconnected(@NonNull final RemoteOperationResult result) { RemoteOperationResult.ResultCode resultCode = result.getCode(); return resultCode == NO_NETWORK_CONNECTION || resultCode == WRONG_CONNECTION; } -- GitLab