From a37f9cfa181bf8937135722244a5031c8d9cfc16 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Mon, 22 May 2023 17:06:28 +0200 Subject: [PATCH 1/3] handle invalid remote timestamp & refactor fileDiffUtils.java into fileDiffChecker.kt --- .../FileDiffChecker.kt} | 78 ++++++++++--------- .../contentScanner/LocalContentScanner.java | 3 +- .../contentScanner/RemoteContentScanner.java | 32 ++++---- 3 files changed, 60 insertions(+), 53 deletions(-) rename app/src/main/java/foundation/e/drive/{utils/FileDiffUtils.java => contentScanner/FileDiffChecker.kt} (53%) diff --git a/app/src/main/java/foundation/e/drive/utils/FileDiffUtils.java b/app/src/main/java/foundation/e/drive/contentScanner/FileDiffChecker.kt similarity index 53% rename from app/src/main/java/foundation/e/drive/utils/FileDiffUtils.java rename to app/src/main/java/foundation/e/drive/contentScanner/FileDiffChecker.kt index 59bc0d9e..352767a9 100644 --- a/app/src/main/java/foundation/e/drive/utils/FileDiffUtils.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/FileDiffChecker.kt @@ -1,28 +1,22 @@ /* - * Copyright © ECORP SAS 2022. + * Copyright © MURENA SAS 2023. * All rights reserved. This program and the accompanying materials * are made available under the terms of the GNU Public License v3.0 * which accompanies this distribution, and is available at * http://www.gnu.org/licenses/gpl.html */ +package foundation.e.drive.contentScanner -package foundation.e.drive.utils; -import androidx.annotation.NonNull; - -import com.owncloud.android.lib.resources.files.model.RemoteFile; - -import java.io.File; - -import foundation.e.drive.models.SyncedFileState; +import com.owncloud.android.lib.resources.files.model.RemoteFile +import foundation.e.drive.models.SyncedFileState +import java.io.File /** - * This class encapsulate code to compare syncedFile & Remote file - * but also RemoteFolder and SyncedFolder - * @author vincent Bourgmayer + * Singleton design to compare files with data stored in DB + * @author Vincent Bourgmayer */ -public class FileDiffUtils { - - public enum Action { +object FileDiffChecker { + enum class Action { Upload, Download, skip, @@ -35,34 +29,33 @@ public class FileDiffUtils { * @param fileState SyncedFileState instance * @return Action from Enum */ - @NonNull - public static Action getActionForFileDiff(@NonNull RemoteFile remoteFile, @NonNull SyncedFileState fileState) { + fun getActionForFileDiff(remoteFile: RemoteFile, fileState: SyncedFileState): Action { + if (hasInvalidRemoteTimestamp(remoteFile.modifiedTimestamp)) { + return Action.Upload + } + if (hasAlreadyBeenDownloaded(fileState) && !hasEtagChanged(remoteFile, fileState)) { - return Action.skip; + return Action.skip } - final File localFile = new File(fileState.getLocalPath()); + val localFile = File(fileState.localPath) - if (isRemoteSizeSameAsLocalSize(remoteFile, localFile)) { - return Action.updateDB; - } - return Action.Download; + return if (isRemoteSizeSameAsLocalSize(remoteFile, localFile)) { + Action.updateDB + } else Action.Download } - /** * Define what to do of local file for which we know the Database equivalent * @param localFile File instance representing a file on the device * @param fileState SyncedFileState instance. Containing data from Database * @return Action from Enum */ - @NonNull - public static Action getActionForFileDiff(@NonNull File localFile, @NonNull SyncedFileState fileState) { + fun getActionForFileDiff(localFile: File, fileState: SyncedFileState): Action { //If no etag is stored in sfs, the file hasn't been sync up to server. then do upload - if (fileState.getLocalLastModified() < localFile.lastModified() || !fileState.isLastEtagStored()) { - return Action.Upload; - } - return Action.skip; + return if (fileState.localLastModified < localFile.lastModified() || !fileState.isLastEtagStored) { + Action.Upload + } else Action.skip } /** @@ -71,9 +64,9 @@ public class FileDiffUtils { * @param fileState last store file's state * @return true if ETag */ - private static boolean hasEtagChanged(@NonNull RemoteFile file, @NonNull SyncedFileState fileState) { + private fun hasEtagChanged(file: RemoteFile, fileState: SyncedFileState): Boolean { //if SyncedFileState has no Etag then it hasn't been uploaded and so must not exist on server - return fileState.isLastEtagStored() && !file.getEtag().equals(fileState.getLastETAG()); + return fileState.isLastEtagStored && file.etag != fileState.lastETAG } /** @@ -82,8 +75,8 @@ public class FileDiffUtils { * @param fileState SyncedFileState containing data from Database * @return true if localLastModified store in Database == 0 */ - private static boolean hasAlreadyBeenDownloaded(@NonNull SyncedFileState fileState) { - return fileState.getLocalLastModified() > 0L; + private fun hasAlreadyBeenDownloaded(fileState: SyncedFileState): Boolean { + return fileState.localLastModified > 0L } /** @@ -92,9 +85,20 @@ public class FileDiffUtils { * @param localFile File instance * @return true if remote file size is same as local file size */ - private static boolean isRemoteSizeSameAsLocalSize(@NonNull RemoteFile remoteFile, @NonNull File localFile) { + private fun isRemoteSizeSameAsLocalSize(remoteFile: RemoteFile, localFile: File): Boolean { // if local file doesn't exist its size will be 0 // remoteFile.getSize() : getSize() is equal to getLength() except that for folder is also sum the content of the folder! - return remoteFile.getSize() == localFile.length(); + return remoteFile.size == localFile.length() + } + + /** + * Check if timestamp is equal to max of unsigned int 32 + * + * For yet unknown reason, some remote files have this value in remote DB + * the only way to fix cloud DB is to force re upload of the file with correct value + */ + private fun hasInvalidRemoteTimestamp(timestamp: Long): Boolean { + val maxInt32 = 4294967295L + return timestamp == maxInt32 } -} +} \ No newline at end of file diff --git a/app/src/main/java/foundation/e/drive/contentScanner/LocalContentScanner.java b/app/src/main/java/foundation/e/drive/contentScanner/LocalContentScanner.java index dbc5864c..253c3f4e 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/LocalContentScanner.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/LocalContentScanner.java @@ -19,7 +19,6 @@ import foundation.e.drive.models.SyncRequest; import foundation.e.drive.models.SyncedFileState; import foundation.e.drive.models.SyncedFolder; import foundation.e.drive.utils.CommonUtils; -import foundation.e.drive.utils.FileDiffUtils; import timber.log.Timber; /** @@ -73,7 +72,7 @@ public class LocalContentScanner extends AbstractContentScanner{ @Override protected void onKnownFileFound(@NonNull File file, @NonNull SyncedFileState fileState) { - if (FileDiffUtils.getActionForFileDiff(file, fileState) == FileDiffUtils.Action.Upload) { + if (FileDiffChecker.INSTANCE.getActionForFileDiff(file, fileState) == FileDiffChecker.Action.Upload) { Timber.d("Add upload SyncRequest for %s", file.getAbsolutePath()); syncRequests.put(fileState.getId(), new SyncRequest(fileState, SyncRequest.Type.UPLOAD)); } diff --git a/app/src/main/java/foundation/e/drive/contentScanner/RemoteContentScanner.java b/app/src/main/java/foundation/e/drive/contentScanner/RemoteContentScanner.java index 37b1ffc5..be48ed23 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/RemoteContentScanner.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/RemoteContentScanner.java @@ -8,7 +8,6 @@ package foundation.e.drive.contentScanner; import static foundation.e.drive.models.SyncRequest.Type.LOCAL_DELETE; -import static foundation.e.drive.utils.FileDiffUtils.getActionForFileDiff; import android.content.Context; @@ -24,7 +23,6 @@ import foundation.e.drive.models.SyncRequest; import foundation.e.drive.models.SyncedFileState; import foundation.e.drive.models.SyncedFolder; import foundation.e.drive.utils.CommonUtils; -import foundation.e.drive.utils.FileDiffUtils; import timber.log.Timber; /** @@ -43,18 +41,24 @@ public class RemoteContentScanner extends AbstractContentScanner { @Override protected void onKnownFileFound(@NonNull RemoteFile file, @NonNull SyncedFileState fileState) { - final FileDiffUtils.Action action = getActionForFileDiff(file, fileState); - if (action == FileDiffUtils.Action.Download) { - - Timber.d("Add download SyncRequest for %s", file.getRemotePath()); - syncRequests.put(fileState.getId(), new DownloadRequest(file, fileState)); - - } else if (action == FileDiffUtils.Action.updateDB) { - - fileState.setLastETAG(file.getEtag()); - final int affectedRows = DbHelper.manageSyncedFileStateDB(fileState, "UPDATE", context); - if (affectedRows == 0) Timber.d("Error while updating eTag in DB for: %s", file.getRemotePath()); - + final FileDiffChecker.Action action = FileDiffChecker.INSTANCE.getActionForFileDiff(file, fileState); + + switch(action) { + case Download: + Timber.d("Add download SyncRequest for %s", file.getRemotePath()); + syncRequests.put(fileState.getId(), new DownloadRequest(file, fileState)); + break; + case updateDB: + fileState.setLastETAG(file.getEtag()); + final int affectedRows = DbHelper.manageSyncedFileStateDB(fileState, "UPDATE", context); + if (affectedRows == 0) Timber.d("Error while updating eTag in DB for: %s", file.getRemotePath()); + break; + case Upload: + fileState.setLastETAG(""); //Force fake lastEtag value to bypass condition check on UploadFileOperation + syncRequests.put(fileState.getId(), new SyncRequest(fileState, SyncRequest.Type.UPLOAD)); + break; + default: + break; } } -- GitLab From 91809e5ab5d12bee86630b02993477d4f4be1514 Mon Sep 17 00:00:00 2001 From: Jonathan Klee Date: Tue, 23 May 2023 07:36:47 +0000 Subject: [PATCH 2/3] Apply 2 suggestion(s) to 1 file(s) --- .../foundation/e/drive/contentScanner/FileDiffChecker.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/contentScanner/FileDiffChecker.kt b/app/src/main/java/foundation/e/drive/contentScanner/FileDiffChecker.kt index 352767a9..ba5f5f98 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/FileDiffChecker.kt +++ b/app/src/main/java/foundation/e/drive/contentScanner/FileDiffChecker.kt @@ -19,8 +19,8 @@ object FileDiffChecker { enum class Action { Upload, Download, - skip, - updateDB + Skip, + UpdateDB } /** @@ -85,7 +85,7 @@ object FileDiffChecker { * @param localFile File instance * @return true if remote file size is same as local file size */ - private fun isRemoteSizeSameAsLocalSize(remoteFile: RemoteFile, localFile: File): Boolean { + private fun haveSameSize(remoteFile: RemoteFile, localFile: File): Boolean { // if local file doesn't exist its size will be 0 // remoteFile.getSize() : getSize() is equal to getLength() except that for folder is also sum the content of the folder! return remoteFile.size == localFile.length() -- GitLab From 9bdf47c9b4dd4f43c7e1e146c04e79654e89800c Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Tue, 23 May 2023 09:43:44 +0200 Subject: [PATCH 3/3] fix call of method 'haveSameSize', update javadoc & clean usage of Action enum --- .../e/drive/contentScanner/FileDiffChecker.kt | 12 +++++++----- .../e/drive/contentScanner/RemoteContentScanner.java | 2 +- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/contentScanner/FileDiffChecker.kt b/app/src/main/java/foundation/e/drive/contentScanner/FileDiffChecker.kt index ba5f5f98..dc4bcb74 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/FileDiffChecker.kt +++ b/app/src/main/java/foundation/e/drive/contentScanner/FileDiffChecker.kt @@ -35,13 +35,13 @@ object FileDiffChecker { } if (hasAlreadyBeenDownloaded(fileState) && !hasEtagChanged(remoteFile, fileState)) { - return Action.skip + return Action.Skip } val localFile = File(fileState.localPath) - return if (isRemoteSizeSameAsLocalSize(remoteFile, localFile)) { - Action.updateDB + return if (haveSameSize(remoteFile, localFile)) { + Action.UpdateDB } else Action.Download } @@ -55,7 +55,7 @@ object FileDiffChecker { //If no etag is stored in sfs, the file hasn't been sync up to server. then do upload return if (fileState.localLastModified < localFile.lastModified() || !fileState.isLastEtagStored) { Action.Upload - } else Action.skip + } else Action.Skip } /** @@ -80,7 +80,7 @@ object FileDiffChecker { } /** - * + * Compare file size to last known data (from DB) * @param remoteFile RemoteFile instance * @param localFile File instance * @return true if remote file size is same as local file size @@ -96,6 +96,8 @@ object FileDiffChecker { * * For yet unknown reason, some remote files have this value in remote DB * the only way to fix cloud DB is to force re upload of the file with correct value + * @param timestamp remote file timestamp (Long) + * @return true if the timestamp is equal to max of unsigned int 32 */ private fun hasInvalidRemoteTimestamp(timestamp: Long): Boolean { val maxInt32 = 4294967295L diff --git a/app/src/main/java/foundation/e/drive/contentScanner/RemoteContentScanner.java b/app/src/main/java/foundation/e/drive/contentScanner/RemoteContentScanner.java index be48ed23..a3dc9fd4 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/RemoteContentScanner.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/RemoteContentScanner.java @@ -48,7 +48,7 @@ public class RemoteContentScanner extends AbstractContentScanner { Timber.d("Add download SyncRequest for %s", file.getRemotePath()); syncRequests.put(fileState.getId(), new DownloadRequest(file, fileState)); break; - case updateDB: + case UpdateDB: fileState.setLastETAG(file.getEtag()); final int affectedRows = DbHelper.manageSyncedFileStateDB(fileState, "UPDATE", context); if (affectedRows == 0) Timber.d("Error while updating eTag in DB for: %s", file.getRemotePath()); -- GitLab