From a182ca6a49f07c055a56220c3973c4cacc22d38c Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Tue, 5 Sep 2023 10:25:55 +0200 Subject: [PATCH 01/13] convert FileDiffUtils in kotlin --- .../foundation/e/drive/utils/FileDiffUtils.kt | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 app/src/main/java/foundation/e/drive/utils/FileDiffUtils.kt diff --git a/app/src/main/java/foundation/e/drive/utils/FileDiffUtils.kt b/app/src/main/java/foundation/e/drive/utils/FileDiffUtils.kt new file mode 100644 index 00000000..f79b5acc --- /dev/null +++ b/app/src/main/java/foundation/e/drive/utils/FileDiffUtils.kt @@ -0,0 +1,48 @@ +package foundation.e.drive.utils + +import com.owncloud.android.lib.resources.files.model.RemoteFile +import foundation.e.drive.models.SyncedFileState +import java.io.File + +enum class Action {Upload, Download, Skip, UpdateDB } +object FileDiffUtils { + + /** + * Define what to do of RemoteFile for which we know the Database equivalent + * @param remoteFile RemoteFile + * @param fileState SyncedFileState instance + * @return Action from Enum + */ + @JvmStatic + fun getActionForFileDiff(remoteFile: RemoteFile, fileState: SyncedFileState): Action { + if () { + return Action.Skip + } + val localFile = File(fileState.localPath) + if () { + return Action.UpdateDB + } + return 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 + */ + @JvmStatic + fun getActionForFileDiff(localFile: File, fileState: SyncedFileState): Action { + + val isFileMoreRecentThanDB = fileState.lastModified < localFile.lastModified() + + return if (isFileMoreRecentThanDB || !fileState.isLastEtagStored()) { + Action.Upload + } else Action.Skip + } + + @JvmStatic + private fun hasEtagChanged(remoteFile: RemoteFile, fileState: SyncedFileState): Boolean { + return fileState.isLastEtagStored() && !remoteFile.etag == fileState.lastEtag + } +} \ No newline at end of file -- GitLab From da39b574eba489eb9526610ad649a0c2c6904abc Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Tue, 5 Sep 2023 10:29:40 +0200 Subject: [PATCH 02/13] fix error in SyncedFileState class --- .../main/java/foundation/e/drive/models/SyncedFileState.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/models/SyncedFileState.kt b/app/src/main/java/foundation/e/drive/models/SyncedFileState.kt index ccd10465..73b55595 100644 --- a/app/src/main/java/foundation/e/drive/models/SyncedFileState.kt +++ b/app/src/main/java/foundation/e/drive/models/SyncedFileState.kt @@ -56,7 +56,7 @@ data class SyncedFileState (var id: Int, parcel.readLong(), parcel.readByte() != 0.toByte(), parcel.readInt() - ) { } + ) override fun writeToParcel(parcel: Parcel, flags: Int) { parcel.writeInt(id) @@ -93,7 +93,7 @@ data class SyncedFileState (var id: Int, } fun disableScanning() { - this.scanScope == DO_NOT_SCAN + this.scanScope = DO_NOT_SCAN } override fun toString(): String { -- GitLab From 7cf1ad4b230474edf8f9f91e24aec26651585ace Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Tue, 5 Sep 2023 10:50:18 +0200 Subject: [PATCH 03/13] Finish FileDiffUtils conversion to kotlin and move it to ContentScannerPackage --- .../e/drive/contentScanner/FileDiffUtils.kt | 84 +++++++++++++++ .../contentScanner/LocalContentScanner.java | 1 - .../contentScanner/RemoteContentScanner.java | 5 +- .../e/drive/utils/FileDiffUtils.java | 100 ------------------ .../foundation/e/drive/utils/FileDiffUtils.kt | 48 --------- 5 files changed, 86 insertions(+), 152 deletions(-) create mode 100644 app/src/main/java/foundation/e/drive/contentScanner/FileDiffUtils.kt delete mode 100644 app/src/main/java/foundation/e/drive/utils/FileDiffUtils.java delete mode 100644 app/src/main/java/foundation/e/drive/utils/FileDiffUtils.kt diff --git a/app/src/main/java/foundation/e/drive/contentScanner/FileDiffUtils.kt b/app/src/main/java/foundation/e/drive/contentScanner/FileDiffUtils.kt new file mode 100644 index 00000000..bc4a34f7 --- /dev/null +++ b/app/src/main/java/foundation/e/drive/contentScanner/FileDiffUtils.kt @@ -0,0 +1,84 @@ +/* + * Copyright © MURENA SAS 3. + * 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 + +import com.owncloud.android.lib.resources.files.model.RemoteFile +import foundation.e.drive.models.SyncedFileState +import java.io.File + + +object FileDiffUtils { + enum class Action { Upload, Download, Skip, UpdateDB } + /** + * Define what to do of RemoteFile for which we know the Database equivalent + * @param remoteFile RemoteFile + * @param fileState SyncedFileState instance + * @return Action from Enum + */ + @JvmStatic + fun getActionForFileDiff(remoteFile: RemoteFile, fileState: SyncedFileState): Action { + if (hasAlreadyBeenDownloaded(fileState) && !hasEtagChanged(remoteFile, fileState)) { + return Action.Skip + } + val localFile = File(fileState.localPath) + if (isRemoteAndLocalSizeEqual(remoteFile, localFile)) { + return Action.UpdateDB + } + return 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 + */ + @JvmStatic + fun getActionForFileDiff(localFile: File, fileState: SyncedFileState): Action { + + val isFileMoreRecentThanDB = fileState.lastModified < localFile.lastModified() + + return if (isFileMoreRecentThanDB || !fileState.isLastEtagStored()) { + Action.Upload + } else Action.Skip + } + + /** + * Compare RemoteFile's eTag with the one stored in Database + * @param remoteFile RemoteFile + * @param fileState last store file's state + * @return true if ETag + */ + @JvmStatic + private fun hasEtagChanged(remoteFile: RemoteFile, fileState: SyncedFileState): Boolean { + return fileState.isLastEtagStored() && remoteFile.etag != fileState.lastEtag + } + + /** + * Indicate if the file has already been downloaded + * or detected on the device + * @param fileState SyncedFileState containing data from Database + * @return true if localLastModified store in Database == 0 + */ + @JvmStatic + private fun hasAlreadyBeenDownloaded(fileState: SyncedFileState): Boolean { + return fileState.lastModified > 0 + } + + /** + * Compare file size for remote file & local file + * @param remoteFile RemoteFile instance + * @param localFile File instance + * @return true if remote file size is same as local file size + */ + @JvmStatic + private fun isRemoteAndLocalSizeEqual(remoteFile: RemoteFile, localFile: File): Boolean { + // remoteFile.getSize() : getSize() is equal to getLength() except for folder where it is also sum the content of the folder! + return remoteFile.size == localFile.length() + } +} \ 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 0d94ae0a..6e62d02d 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/LocalContentScanner.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/LocalContentScanner.java @@ -22,7 +22,6 @@ import foundation.e.drive.database.DbHelper; import foundation.e.drive.models.SyncRequest; import foundation.e.drive.models.SyncedFileState; import foundation.e.drive.models.SyncedFolder; -import foundation.e.drive.utils.FileDiffUtils; import foundation.e.drive.utils.FileUtils; import timber.log.Timber; 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 0780d6a0..d43975a6 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/RemoteContentScanner.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/RemoteContentScanner.java @@ -11,7 +11,7 @@ import static foundation.e.drive.models.SyncRequest.Type.DISABLE_SYNCING; import static foundation.e.drive.models.SyncedFileStateKt.DO_NOT_SCAN; import static foundation.e.drive.models.SyncedFileStateKt.SCAN_ON_CLOUD; import static foundation.e.drive.models.SyncedFileStateKt.SCAN_ON_DEVICE; -import static foundation.e.drive.utils.FileDiffUtils.getActionForFileDiff; +import static foundation.e.drive.contentScanner.FileDiffUtils.getActionForFileDiff; import android.content.Context; @@ -26,7 +26,6 @@ import foundation.e.drive.models.DownloadRequest; import foundation.e.drive.models.SyncRequest; import foundation.e.drive.models.SyncedFileState; import foundation.e.drive.models.SyncedFolder; -import foundation.e.drive.utils.FileDiffUtils; import foundation.e.drive.utils.FileUtils; import timber.log.Timber; @@ -54,7 +53,7 @@ public class RemoteContentScanner extends AbstractContentScanner { Timber.d("Add download SyncRequest for %s", file.getRemotePath()); syncRequests.put(fileState.getId(), new DownloadRequest(file, fileState)); - } else if (action == FileDiffUtils.Action.updateDB) { + } else if (action == FileDiffUtils.Action.UpdateDB) { fileState.setLastEtag(file.getEtag()); final int affectedRows = DbHelper.manageSyncedFileStateDB(fileState, "UPDATE", context); diff --git a/app/src/main/java/foundation/e/drive/utils/FileDiffUtils.java b/app/src/main/java/foundation/e/drive/utils/FileDiffUtils.java deleted file mode 100644 index 7f5fe817..00000000 --- a/app/src/main/java/foundation/e/drive/utils/FileDiffUtils.java +++ /dev/null @@ -1,100 +0,0 @@ -/* - * Copyright © MURENA SAS 2022-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.utils; -import androidx.annotation.NonNull; - -import com.owncloud.android.lib.resources.files.model.RemoteFile; - -import java.io.File; - -import foundation.e.drive.models.SyncedFileState; - -/** - * This class encapsulate code to compare syncedFile & Remote file - * but also RemoteFolder and SyncedFolder - * @author vincent Bourgmayer - */ -public class FileDiffUtils { - - public enum Action { - Upload, - Download, - skip, - updateDB - } - - /** - * Define what to do of RemoteFile for which we know the Database equivalent - * @param remoteFile RemoteFile - * @param fileState SyncedFileState instance - * @return Action from Enum - */ - @NonNull - public static Action getActionForFileDiff(@NonNull RemoteFile remoteFile, @NonNull SyncedFileState fileState) { - if (hasAlreadyBeenDownloaded(fileState) && !hasEtagChanged(remoteFile, fileState)) { - return Action.skip; - } - - final File localFile = new File(fileState.getLocalPath()); - - if (isRemoteSizeSameAsLocalSize(remoteFile, localFile)) { - return Action.updateDB; - } - return 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) { - //If no etag is stored in sfs, the file hasn't been sync up to server. then do upload - if (fileState.getLastModified() < localFile.lastModified() || !fileState.isLastEtagStored()) { - return Action.Upload; - } - return Action.skip; - } - - /** - * Compare RemoteFile's eTag with the one stored in Database - * @param file RemoteFile - * @param fileState last store file's state - * @return true if ETag - */ - private static boolean hasEtagChanged(@NonNull RemoteFile file, @NonNull SyncedFileState fileState) { - //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()); - } - - /** - * Indicate if the file has already been downloaded - * or detected on the device - * @param fileState SyncedFileState containing data from Database - * @return true if localLastModified store in Database == 0 - */ - private static boolean hasAlreadyBeenDownloaded(@NonNull SyncedFileState fileState) { - return fileState.getLastModified() > 0L; - } - - /** - * - * @param remoteFile RemoteFile instance - * @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) { - // 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(); - } -} \ No newline at end of file diff --git a/app/src/main/java/foundation/e/drive/utils/FileDiffUtils.kt b/app/src/main/java/foundation/e/drive/utils/FileDiffUtils.kt deleted file mode 100644 index f79b5acc..00000000 --- a/app/src/main/java/foundation/e/drive/utils/FileDiffUtils.kt +++ /dev/null @@ -1,48 +0,0 @@ -package foundation.e.drive.utils - -import com.owncloud.android.lib.resources.files.model.RemoteFile -import foundation.e.drive.models.SyncedFileState -import java.io.File - -enum class Action {Upload, Download, Skip, UpdateDB } -object FileDiffUtils { - - /** - * Define what to do of RemoteFile for which we know the Database equivalent - * @param remoteFile RemoteFile - * @param fileState SyncedFileState instance - * @return Action from Enum - */ - @JvmStatic - fun getActionForFileDiff(remoteFile: RemoteFile, fileState: SyncedFileState): Action { - if () { - return Action.Skip - } - val localFile = File(fileState.localPath) - if () { - return Action.UpdateDB - } - return 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 - */ - @JvmStatic - fun getActionForFileDiff(localFile: File, fileState: SyncedFileState): Action { - - val isFileMoreRecentThanDB = fileState.lastModified < localFile.lastModified() - - return if (isFileMoreRecentThanDB || !fileState.isLastEtagStored()) { - Action.Upload - } else Action.Skip - } - - @JvmStatic - private fun hasEtagChanged(remoteFile: RemoteFile, fileState: SyncedFileState): Boolean { - return fileState.isLastEtagStored() && !remoteFile.etag == fileState.lastEtag - } -} \ No newline at end of file -- GitLab From 00f5109c26e3e8a9298fe520870803e0a20a5c14 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Tue, 5 Sep 2023 17:26:19 +0200 Subject: [PATCH 04/13] add Unit test class for FileDiffUtilsTest missed unit test for getActionForFileDiff(RemoteFile, SyncedFileState) --- app/build.gradle | 4 + .../e/drive/contentScanner/FileDiffUtils.kt | 11 +- .../e/drive/models/SyncedFileState.kt | 2 +- .../drive/contentScanner/FileDiffUtilsTest.kt | 280 ++++++++++++++++++ 4 files changed, 292 insertions(+), 5 deletions(-) create mode 100644 app/src/test/java/foundation/e/drive/contentScanner/FileDiffUtilsTest.kt diff --git a/app/build.gradle b/app/build.gradle index fdeb8dd6..930cc22c 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -69,6 +69,10 @@ android { } } + kotlinOptions { + jvmTarget = "11" + } + testOptions { unitTests { returnDefaultValues = true diff --git a/app/src/main/java/foundation/e/drive/contentScanner/FileDiffUtils.kt b/app/src/main/java/foundation/e/drive/contentScanner/FileDiffUtils.kt index bc4a34f7..412929a9 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/FileDiffUtils.kt +++ b/app/src/main/java/foundation/e/drive/contentScanner/FileDiffUtils.kt @@ -7,6 +7,7 @@ */ package foundation.e.drive.contentScanner +import androidx.annotation.VisibleForTesting import com.owncloud.android.lib.resources.files.model.RemoteFile import foundation.e.drive.models.SyncedFileState import java.io.File @@ -40,7 +41,6 @@ object FileDiffUtils { */ @JvmStatic fun getActionForFileDiff(localFile: File, fileState: SyncedFileState): Action { - val isFileMoreRecentThanDB = fileState.lastModified < localFile.lastModified() return if (isFileMoreRecentThanDB || !fileState.isLastEtagStored()) { @@ -54,8 +54,9 @@ object FileDiffUtils { * @param fileState last store file's state * @return true if ETag */ + @VisibleForTesting @JvmStatic - private fun hasEtagChanged(remoteFile: RemoteFile, fileState: SyncedFileState): Boolean { + fun hasEtagChanged(remoteFile: RemoteFile, fileState: SyncedFileState): Boolean { return fileState.isLastEtagStored() && remoteFile.etag != fileState.lastEtag } @@ -65,8 +66,9 @@ object FileDiffUtils { * @param fileState SyncedFileState containing data from Database * @return true if localLastModified store in Database == 0 */ + @VisibleForTesting @JvmStatic - private fun hasAlreadyBeenDownloaded(fileState: SyncedFileState): Boolean { + fun hasAlreadyBeenDownloaded(fileState: SyncedFileState): Boolean { return fileState.lastModified > 0 } @@ -76,8 +78,9 @@ object FileDiffUtils { * @param localFile File instance * @return true if remote file size is same as local file size */ + @VisibleForTesting @JvmStatic - private fun isRemoteAndLocalSizeEqual(remoteFile: RemoteFile, localFile: File): Boolean { + fun isRemoteAndLocalSizeEqual(remoteFile: RemoteFile, localFile: File): Boolean { // remoteFile.getSize() : getSize() is equal to getLength() except for folder where it is also sum the content of the folder! return remoteFile.size == localFile.length() } diff --git a/app/src/main/java/foundation/e/drive/models/SyncedFileState.kt b/app/src/main/java/foundation/e/drive/models/SyncedFileState.kt index 73b55595..e43e7cc5 100644 --- a/app/src/main/java/foundation/e/drive/models/SyncedFileState.kt +++ b/app/src/main/java/foundation/e/drive/models/SyncedFileState.kt @@ -85,7 +85,7 @@ data class SyncedFileState (var id: Int, } fun isLastEtagStored() : Boolean { - return this.lastEtag.isNotEmpty() + return !this.lastEtag.isNullOrEmpty() } fun hasBeenSynchronizedOnce(): Boolean { diff --git a/app/src/test/java/foundation/e/drive/contentScanner/FileDiffUtilsTest.kt b/app/src/test/java/foundation/e/drive/contentScanner/FileDiffUtilsTest.kt new file mode 100644 index 00000000..bdd3b6be --- /dev/null +++ b/app/src/test/java/foundation/e/drive/contentScanner/FileDiffUtilsTest.kt @@ -0,0 +1,280 @@ +/* + * 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 + +import com.owncloud.android.lib.resources.files.model.RemoteFile +import foundation.e.drive.models.SyncedFileState +import org.junit.Assert +import org.junit.Test +import org.mockito.Mockito +import java.io.File + +/** + * Unit for test FileDiffUtils's method + * + * note: when test method name mention 'DB' it refers to SyncedFileState, because those object + * represent data fetched from DB in the scope of the class under test. + * + * 'mTime' means modification time, it is a shortcut for 'lastModified' + * + * @author Vincent Bourgmayer + */ +internal class FileDiffUtilsTest { + + /* isRemoteAndLocalSizeEqual() */ + @Test + fun `isRemoteAndLocalSizeEqual() return false when local file's is size smaller`() { + val remoteSize: Long = 12346789 + val remoteFile = Mockito.mock(RemoteFile::class.java) + Mockito.`when`(remoteFile.size).thenReturn(remoteSize) + + val localSize: Long = 1234 + val localFile = Mockito.mock(File::class.java) + Mockito.`when`(localFile.length()).thenReturn(localSize) + + Assert.assertFalse("isRemoteAndLocalSizeEqual() returned true instead of false", + FileDiffUtils.isRemoteAndLocalSizeEqual(remoteFile, localFile)) + } + + @Test + fun `isRemoteAndLocalSizeEqual() return false when local file's size is bigger`() { + val remoteSize: Long = 1234 + val remoteFile = Mockito.mock(RemoteFile::class.java) + Mockito.`when`(remoteFile.size).thenReturn(remoteSize) + + val localSize: Long = 12346789 + val localFile = Mockito.mock(File::class.java) + Mockito.`when`(localFile.length()).thenReturn(localSize) + + Assert.assertFalse("isRemoteAndLocalSizeEqual() returned true instead of false", + FileDiffUtils.isRemoteAndLocalSizeEqual(remoteFile, localFile)) + } + + @Test + fun `isRemoteAndLocalSizeEqual() return true when remote & local file size are equal`() { + val remoteSize: Long = 12346789 + val remoteFile = Mockito.mock(RemoteFile::class.java) + Mockito.`when`(remoteFile.size).thenReturn(remoteSize) + + val localSize: Long = 12346789 + val localFile = Mockito.mock(File::class.java) + Mockito.`when`(localFile.length()).thenReturn(localSize) + + Assert.assertTrue("isRemoteAndLocalSizeEqual() returned false instead of true", + FileDiffUtils.isRemoteAndLocalSizeEqual(remoteFile, localFile)) + } + + /* hasAlreadyBeenDownloaded() */ + @Test + fun `hasAlreadyBeenDownloaded() return true when last modified is bigger than 0`() { + val fileState = Mockito.mock(SyncedFileState::class.java) + Mockito.`when`(fileState.lastModified).thenReturn(1) + + Assert.assertTrue("hasAlreadyBeenDownloaded() returned false instead of true", + FileDiffUtils.hasAlreadyBeenDownloaded(fileState)) + } + + @Test + fun `hasAlreadyBeenDownloaded() return false when last modified equal to 0`() { + val fileState = Mockito.mock(SyncedFileState::class.java) + Mockito.`when`(fileState.lastModified).thenReturn(0) + + Assert.assertFalse("hasAlreadyBeenDownloaded() returned true instead of false", + FileDiffUtils.hasAlreadyBeenDownloaded(fileState)) + } + + /* hasEtagChanged() */ + @Test + fun `hasEtagChanged() return false when DB doesn't have eTag return false`() { + val remoteFile = Mockito.mock(RemoteFile::class.java) + + val fileState = Mockito.mock(SyncedFileState::class.java) + Mockito.`when`(fileState.isLastEtagStored()).thenReturn(false) + + Assert.assertFalse("hasEtagChanged() returned true instead of false", + FileDiffUtils.hasEtagChanged(remoteFile, fileState)) + } + + @Test + fun `hasEtagChanged() return false when eTag from remoteFile & DB are the same`() { + val remoteFile = Mockito.mock(RemoteFile::class.java) + Mockito.`when`(remoteFile.etag).thenReturn("123456789") + + val fileState = Mockito.mock(SyncedFileState::class.java) + Mockito.`when`(fileState.lastEtag).thenReturn("123456789") + Mockito.`when`(fileState.isLastEtagStored()).thenReturn(true) + + Assert.assertFalse("hasEtagChanged() returned true instead of false", + FileDiffUtils.hasEtagChanged(remoteFile, fileState)) + } + + @Test + fun `hasEtagChanged() return true when Remote eTag is empty while DB is not`() { + val remoteFile = Mockito.mock(RemoteFile::class.java) + Mockito.`when`(remoteFile.etag).thenReturn("") + + val fileState = Mockito.mock(SyncedFileState::class.java) + Mockito.`when`(fileState.lastEtag).thenReturn("123456789") + Mockito.`when`(fileState.isLastEtagStored()).thenReturn(true) + + Assert.assertTrue("hasEtagChanged() returned false instead of true", + FileDiffUtils.hasEtagChanged(remoteFile, fileState)) + } + + @Test + fun `hasEtagChanged() return true when eTag from DB & Remote are different`() { + val remoteFile = Mockito.mock(RemoteFile::class.java) + //Mockito.`when`(remoteFile.etag).thenReturn("123456789") + remoteFile.etag = "123456789" + val fileState = Mockito.mock(SyncedFileState::class.java) + Mockito.`when`(fileState.lastEtag).thenReturn("987654321") + Mockito.`when`(fileState.isLastEtagStored()).thenReturn(true) + + Assert.assertTrue("hasEtagChanged() returned false instead of true", + FileDiffUtils.hasEtagChanged(remoteFile, fileState)) + } + + + /* getActionForFileDiff for localFile */ + + @Test + fun `getActionForFileDiff(File) return Upload when local file mTime is smaller & no eTag in DB`() { + val mockedLastModified: Long = 1 + val mockedFile = Mockito.spy(File("/local/path.txt")) + Mockito.`when`(mockedFile.lastModified()).thenReturn(mockedLastModified) + + val mockedLastModifiedFromDB: Long = 10 + val fileState = Mockito.mock(SyncedFileState::class.java) + Mockito.`when`(fileState.isLastEtagStored()).thenReturn(false) + Mockito.`when`(fileState.lastModified).thenReturn(mockedLastModifiedFromDB) + + val resultUnderTest = FileDiffUtils.getActionForFileDiff(mockedFile, fileState) + Assert.assertEquals("getActionForFileDiff() returned $resultUnderTest instead of Action.Upload", FileDiffUtils.Action.Upload, resultUnderTest) + } + + @Test + fun `getActionForFileDiff(File) return Skip when local file mTime is smaller & eTag is in DB`() { + val mockedLastModified: Long = 1 + val mockedFile = Mockito.spy(File("/local/path.txt")) + Mockito.`when`(mockedFile.lastModified()).thenReturn(mockedLastModified) + + val mockedLastModifiedFromDB: Long = 10 + val fileState = Mockito.mock(SyncedFileState::class.java) + Mockito.`when`(fileState.isLastEtagStored()).thenReturn(true) + Mockito.`when`(fileState.lastModified).thenReturn(mockedLastModifiedFromDB) + + val resultUnderTest = FileDiffUtils.getActionForFileDiff(mockedFile, fileState) + Assert.assertEquals("getActionForFileDiff() returned $resultUnderTest instead of Action.Skip", FileDiffUtils.Action.Skip, resultUnderTest) + } + + @Test + fun `getActionForFileDiff(File) return Upload when local file mTime is equal to DB & eTag not in DB`() { + val mockedLastModified: Long = 10 + val mockedFile = Mockito.spy(File("/local/path.txt")) + Mockito.`when`(mockedFile.lastModified()).thenReturn(mockedLastModified) + + val mockedLastModifiedFromDB: Long = 10 + val fileState = Mockito.mock(SyncedFileState::class.java) + Mockito.`when`(fileState.isLastEtagStored()).thenReturn(false) + Mockito.`when`(fileState.lastModified).thenReturn(mockedLastModifiedFromDB) + + val resultUnderTest = FileDiffUtils.getActionForFileDiff(mockedFile, fileState) + Assert.assertEquals("getActionForFileDiff() returned $resultUnderTest instead of Action.Upload", FileDiffUtils.Action.Upload, resultUnderTest) + } + + @Test + fun `getActionForFileDiff(File) return Skip when local file mTime is equal to DB & eTag is in DB`() { + val mockedLastModified: Long = 10 + val mockedFile = Mockito.spy(File("/local/path.txt")) + Mockito.`when`(mockedFile.lastModified()).thenReturn(mockedLastModified) + + val mockedLastModifiedFromDB: Long = 10 + val fileState = Mockito.mock(SyncedFileState::class.java) + Mockito.`when`(fileState.isLastEtagStored()).thenReturn(true) + Mockito.`when`(fileState.lastModified).thenReturn(mockedLastModifiedFromDB) + + val resultUnderTest = FileDiffUtils.getActionForFileDiff(mockedFile, fileState) + Assert.assertEquals("getActionForFileDiff() returned $resultUnderTest instead of Action.Skip", FileDiffUtils.Action.Skip, resultUnderTest) + } + + @Test + fun `getActionForFileDiff(File) return Upload when local file mTime is bigger & eTag not in DB`() { + val mockedLastModified: Long = 10 + val mockedFile = Mockito.spy(File("/local/path.txt")) + Mockito.`when`(mockedFile.lastModified()).thenReturn(mockedLastModified) + + val mockedLastModifiedFromDB: Long = 1 + val fileState = Mockito.mock(SyncedFileState::class.java) + Mockito.`when`(fileState.isLastEtagStored()).thenReturn(false) + Mockito.`when`(fileState.lastModified).thenReturn(mockedLastModifiedFromDB) + + val resultUnderTest = FileDiffUtils.getActionForFileDiff(mockedFile, fileState) + Assert.assertEquals("getActionForFileDiff() returned $resultUnderTest instead of Action.Upload", FileDiffUtils.Action.Upload, resultUnderTest) + } + + @Test + fun `getActionForFileDiff(File) return Upload when local file mTime is bigger & eTag is in DB`() { + val mockedLastModified: Long = 10 + val mockedFile = Mockito.spy(File("/local/path.txt")) + Mockito.`when`(mockedFile.lastModified()).thenReturn(mockedLastModified) + + val mockedLastModifiedFromDB: Long = 1 + val fileState = Mockito.mock(SyncedFileState::class.java) + Mockito.`when`(fileState.isLastEtagStored()).thenReturn(true) + Mockito.`when`(fileState.lastModified).thenReturn(mockedLastModifiedFromDB) + + val resultUnderTest = FileDiffUtils.getActionForFileDiff(mockedFile, fileState) + Assert.assertEquals("getActionForFileDiff() returned $resultUnderTest instead of Action.Upload", FileDiffUtils.Action.Upload, resultUnderTest) + } + + /* getActionForFileDiff for remoteFile */ + + @Test + fun `getActionForFileDiff(RemoteFile) return Skip when file is up to date`() { + val mockedRemoteFile = Mockito.mock(RemoteFile::class.java) + Mockito.`when`(mockedRemoteFile.etag).thenReturn("123456") + val mockedFileState = Mockito.mock(SyncedFileState::class.java) + Mockito.`when`(mockedFileState.lastModified).thenReturn(1L) + Mockito.`when`(mockedFileState.lastEtag).thenReturn("123456") + Mockito.`when`(mockedFileState.isLastEtagStored()).thenReturn(true) + + val resultUnderTest = FileDiffUtils.getActionForFileDiff(mockedRemoteFile, mockedFileState) + Assert.assertEquals("getActionForFileDiff(RemoteFile) returned $resultUnderTest instead of Action.Skip", FileDiffUtils.Action.Skip, resultUnderTest) + } + + @Test + fun `getActionForFileDiff(RemoteFile) return UpdateDB when file is up to date but not DB`() { + val mockedRemoteFile = Mockito.mock(RemoteFile::class.java) + Mockito.`when`(mockedRemoteFile.etag).thenReturn("123456") + Mockito.`when`(mockedRemoteFile.size).thenReturn(0) //local file doesn't exist for test, so its size will be 0 + val mockedFileState = Mockito.mock(SyncedFileState::class.java) + Mockito.`when`(mockedFileState.lastModified).thenReturn(1L) + Mockito.`when`(mockedFileState.localPath).thenReturn("/local/file.txt") + Mockito.`when`(mockedFileState.lastEtag).thenReturn("12") + Mockito.`when`(mockedFileState.isLastEtagStored()).thenReturn(true) + + val resultUnderTest = FileDiffUtils.getActionForFileDiff(mockedRemoteFile, mockedFileState) + Assert.assertEquals("getActionForFileDiff(RemoteFile) returned $resultUnderTest instead of Action.UpdateDB", FileDiffUtils.Action.UpdateDB, resultUnderTest) + } + + + @Test + fun `getActionForFileDiff(RemoteFile) return Download when file is not up to date`() { + val mockedRemoteFile = Mockito.mock(RemoteFile::class.java) + Mockito.`when`(mockedRemoteFile.etag).thenReturn("123456") + Mockito.`when`(mockedRemoteFile.size).thenReturn(1) //local file doesn't exist for test, so its size will be 0 + val mockedFileState = Mockito.mock(SyncedFileState::class.java) + Mockito.`when`(mockedFileState.lastModified).thenReturn(1L) + Mockito.`when`(mockedFileState.localPath).thenReturn("/local/file.txt") + Mockito.`when`(mockedFileState.lastEtag).thenReturn("12") + Mockito.`when`(mockedFileState.isLastEtagStored()).thenReturn(true) + + val resultUnderTest = FileDiffUtils.getActionForFileDiff(mockedRemoteFile, mockedFileState) + Assert.assertEquals("getActionForFileDiff(RemoteFile) returned $resultUnderTest instead of Action.Download", FileDiffUtils.Action.Download, resultUnderTest) + } +} \ No newline at end of file -- GitLab From 6c1db390462a4c9b0a9d4ae431c57a7e5c87b1a4 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Wed, 6 Sep 2023 16:02:10 +0200 Subject: [PATCH 05/13] add method to check for corrupted timestamp from remote file with unit test --- .../e/drive/contentScanner/FileDiffUtils.kt | 18 +++++++++ .../drive/contentScanner/FileDiffUtilsTest.kt | 39 ++++++++++++++++++- 2 files changed, 55 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/contentScanner/FileDiffUtils.kt b/app/src/main/java/foundation/e/drive/contentScanner/FileDiffUtils.kt index 412929a9..221e2a51 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/FileDiffUtils.kt +++ b/app/src/main/java/foundation/e/drive/contentScanner/FileDiffUtils.kt @@ -23,6 +23,8 @@ object FileDiffUtils { */ @JvmStatic fun getActionForFileDiff(remoteFile: RemoteFile, fileState: SyncedFileState): Action { + if (isCorruptedTimestamp(remoteFile.modifiedTimestamp)) return Action.Upload + if (hasAlreadyBeenDownloaded(fileState) && !hasEtagChanged(remoteFile, fileState)) { return Action.Skip } @@ -84,4 +86,20 @@ object FileDiffUtils { // remoteFile.getSize() : getSize() is equal to getLength() except for folder where it is also sum the content of the folder! 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 on cloud (DB & file system) + * the only way to fix them 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 + */ + @VisibleForTesting + @JvmStatic + fun isCorruptedTimestamp(timestamp: Long): Boolean { + val maxInt32 = 4294967295L + return timestamp >= maxInt32 + } } \ No newline at end of file diff --git a/app/src/test/java/foundation/e/drive/contentScanner/FileDiffUtilsTest.kt b/app/src/test/java/foundation/e/drive/contentScanner/FileDiffUtilsTest.kt index bdd3b6be..02b84eb2 100644 --- a/app/src/test/java/foundation/e/drive/contentScanner/FileDiffUtilsTest.kt +++ b/app/src/test/java/foundation/e/drive/contentScanner/FileDiffUtilsTest.kt @@ -25,7 +25,7 @@ import java.io.File * @author Vincent Bourgmayer */ internal class FileDiffUtilsTest { - + val currentTimestampInSecond = System.currentTimeMillis()/1000 /* isRemoteAndLocalSizeEqual() */ @Test fun `isRemoteAndLocalSizeEqual() return false when local file's is size smaller`() { @@ -233,10 +233,10 @@ internal class FileDiffUtilsTest { } /* getActionForFileDiff for remoteFile */ - @Test fun `getActionForFileDiff(RemoteFile) return Skip when file is up to date`() { val mockedRemoteFile = Mockito.mock(RemoteFile::class.java) + Mockito.`when`(mockedRemoteFile.modifiedTimestamp).thenReturn(currentTimestampInSecond) Mockito.`when`(mockedRemoteFile.etag).thenReturn("123456") val mockedFileState = Mockito.mock(SyncedFileState::class.java) Mockito.`when`(mockedFileState.lastModified).thenReturn(1L) @@ -250,6 +250,7 @@ internal class FileDiffUtilsTest { @Test fun `getActionForFileDiff(RemoteFile) return UpdateDB when file is up to date but not DB`() { val mockedRemoteFile = Mockito.mock(RemoteFile::class.java) + Mockito.`when`(mockedRemoteFile.modifiedTimestamp).thenReturn(currentTimestampInSecond) Mockito.`when`(mockedRemoteFile.etag).thenReturn("123456") Mockito.`when`(mockedRemoteFile.size).thenReturn(0) //local file doesn't exist for test, so its size will be 0 val mockedFileState = Mockito.mock(SyncedFileState::class.java) @@ -266,6 +267,7 @@ internal class FileDiffUtilsTest { @Test fun `getActionForFileDiff(RemoteFile) return Download when file is not up to date`() { val mockedRemoteFile = Mockito.mock(RemoteFile::class.java) + Mockito.`when`(mockedRemoteFile.modifiedTimestamp).thenReturn(currentTimestampInSecond) Mockito.`when`(mockedRemoteFile.etag).thenReturn("123456") Mockito.`when`(mockedRemoteFile.size).thenReturn(1) //local file doesn't exist for test, so its size will be 0 val mockedFileState = Mockito.mock(SyncedFileState::class.java) @@ -277,4 +279,37 @@ internal class FileDiffUtilsTest { val resultUnderTest = FileDiffUtils.getActionForFileDiff(mockedRemoteFile, mockedFileState) Assert.assertEquals("getActionForFileDiff(RemoteFile) returned $resultUnderTest instead of Action.Download", FileDiffUtils.Action.Download, resultUnderTest) } + + @Test + fun `getActionForFileDiff(RemoteFile) return Upload when remote modifiedTimestamp is corrupted`() { + val mockedRemoteFile = Mockito.mock(RemoteFile::class.java) + Mockito.`when`(mockedRemoteFile.modifiedTimestamp).thenReturn(4294967295L) + + val mockedFileState = Mockito.mock(SyncedFileState::class.java) + + val resultUnderTest = FileDiffUtils.getActionForFileDiff(mockedRemoteFile, mockedFileState) + Assert.assertEquals("getActionForFileDiff(RemoteFile) returned $resultUnderTest instead of Action.Upload", FileDiffUtils.Action.Upload, resultUnderTest) + } + + /* isCorruptedTimestamp for localFile */ + @Test + fun `isCorruptedTimestamp() return true with timestamp equal to max of Int32 `() { + val corruptedTimestamp = 4294967295L + val resultUnderTest = FileDiffUtils.isCorruptedTimestamp(corruptedTimestamp) + Assert.assertTrue("isCorruptedTimestamp(4294967295L) returned $resultUnderTest instead of true", resultUnderTest) + } + + @Test + fun `isCorruptedTimestamp() return true with timestamp bigger than max of Int32 `() { + val corruptedTimestamp = 4294967295L + 1 + val resultUnderTest = FileDiffUtils.isCorruptedTimestamp(corruptedTimestamp) + Assert.assertTrue("isCorruptedTimestamp(4294967295L) returned $resultUnderTest instead of true", resultUnderTest) + } + + @Test + fun `isCorruptedTimestamp() return false with timestamp smaller than max of Int32 `() { + val corruptedTimestamp = 4294967295L - 1 + val resultUnderTest = FileDiffUtils.isCorruptedTimestamp(corruptedTimestamp) + Assert.assertFalse("isCorruptedTimestamp(4294967295L) returned $resultUnderTest instead of false", resultUnderTest) + } } \ No newline at end of file -- GitLab From a65820e5a233e8f359689c5b91300940becc9795 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Wed, 6 Sep 2023 16:56:06 +0200 Subject: [PATCH 06/13] generate upload request from remotefile --- .../contentScanner/RemoteContentScanner.java | 27 ++++++++++--------- 1 file changed, 15 insertions(+), 12 deletions(-) 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 d43975a6..a92e2119 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/RemoteContentScanner.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/RemoteContentScanner.java @@ -46,19 +46,22 @@ public class RemoteContentScanner extends AbstractContentScanner { @Override protected void onKnownFileFound(@NonNull RemoteFile file, @NonNull SyncedFileState fileState) { if (fileState.getScanScope() == DO_NOT_SCAN) return; - 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()); + switch (action) { + 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; + 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; } } @@ -107,4 +110,4 @@ public class RemoteContentScanner extends AbstractContentScanner { protected boolean isSyncedFolderParentOfFile(@NonNull SyncedFolder syncedFolder, @NonNull String dirPath) { return syncedFolder.getRemoteFolder().equals(dirPath); } -} +} \ No newline at end of file -- GitLab From 088621470f233afeb1c5c055e7762d779226d406 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Thu, 7 Sep 2023 10:19:35 +0200 Subject: [PATCH 07/13] refactor FileDiffUtils.isRemoteAndLocalSizeEqual() --- .../e/drive/contentScanner/FileDiffUtils.kt | 20 ++++++++++--- .../drive/contentScanner/FileDiffUtilsTest.kt | 30 +++++-------------- 2 files changed, 24 insertions(+), 26 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/contentScanner/FileDiffUtils.kt b/app/src/main/java/foundation/e/drive/contentScanner/FileDiffUtils.kt index 221e2a51..8d8f5e72 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/FileDiffUtils.kt +++ b/app/src/main/java/foundation/e/drive/contentScanner/FileDiffUtils.kt @@ -28,8 +28,8 @@ object FileDiffUtils { if (hasAlreadyBeenDownloaded(fileState) && !hasEtagChanged(remoteFile, fileState)) { return Action.Skip } - val localFile = File(fileState.localPath) - if (isRemoteAndLocalSizeEqual(remoteFile, localFile)) { + val localFileSize = getLocalFileSize(fileState.localPath) + if (isRemoteAndLocalSizeEqual(remoteFile.size, localFileSize)) { return Action.UpdateDB } return Action.Download @@ -82,9 +82,9 @@ object FileDiffUtils { */ @VisibleForTesting @JvmStatic - fun isRemoteAndLocalSizeEqual(remoteFile: RemoteFile, localFile: File): Boolean { + fun isRemoteAndLocalSizeEqual(remoteFileSize: Long, localFileSize: Long): Boolean { // remoteFile.getSize() : getSize() is equal to getLength() except for folder where it is also sum the content of the folder! - return remoteFile.size == localFile.length() + return remoteFileSize == localFileSize } @@ -102,4 +102,16 @@ object FileDiffUtils { val maxInt32 = 4294967295L return timestamp >= maxInt32 } + + /** + * Fetch file size on device + * @param path Path of the file + * @return 0 if path is empty or file size + */ + @VisibleForTesting + @JvmStatic + fun getLocalFileSize(path: String): Long { + if (path.isEmpty()) return 0 + return File(path).length() + } } \ No newline at end of file diff --git a/app/src/test/java/foundation/e/drive/contentScanner/FileDiffUtilsTest.kt b/app/src/test/java/foundation/e/drive/contentScanner/FileDiffUtilsTest.kt index 02b84eb2..5262e15b 100644 --- a/app/src/test/java/foundation/e/drive/contentScanner/FileDiffUtilsTest.kt +++ b/app/src/test/java/foundation/e/drive/contentScanner/FileDiffUtilsTest.kt @@ -25,48 +25,34 @@ import java.io.File * @author Vincent Bourgmayer */ internal class FileDiffUtilsTest { - val currentTimestampInSecond = System.currentTimeMillis()/1000 + private val currentTimestampInSecond = System.currentTimeMillis()/1000 + /* isRemoteAndLocalSizeEqual() */ @Test fun `isRemoteAndLocalSizeEqual() return false when local file's is size smaller`() { val remoteSize: Long = 12346789 - val remoteFile = Mockito.mock(RemoteFile::class.java) - Mockito.`when`(remoteFile.size).thenReturn(remoteSize) - val localSize: Long = 1234 - val localFile = Mockito.mock(File::class.java) - Mockito.`when`(localFile.length()).thenReturn(localSize) - Assert.assertFalse("isRemoteAndLocalSizeEqual() returned true instead of false", - FileDiffUtils.isRemoteAndLocalSizeEqual(remoteFile, localFile)) + Assert.assertFalse("isRemoteAndLocalSizeEqual($remoteSize, $localSize) returned true instead of false", + FileDiffUtils.isRemoteAndLocalSizeEqual(remoteSize, localSize)) } @Test fun `isRemoteAndLocalSizeEqual() return false when local file's size is bigger`() { val remoteSize: Long = 1234 - val remoteFile = Mockito.mock(RemoteFile::class.java) - Mockito.`when`(remoteFile.size).thenReturn(remoteSize) - val localSize: Long = 12346789 - val localFile = Mockito.mock(File::class.java) - Mockito.`when`(localFile.length()).thenReturn(localSize) - Assert.assertFalse("isRemoteAndLocalSizeEqual() returned true instead of false", - FileDiffUtils.isRemoteAndLocalSizeEqual(remoteFile, localFile)) + Assert.assertFalse("isRemoteAndLocalSizeEqual($remoteSize, $localSize) returned true instead of false", + FileDiffUtils.isRemoteAndLocalSizeEqual(remoteSize, localSize)) } @Test fun `isRemoteAndLocalSizeEqual() return true when remote & local file size are equal`() { val remoteSize: Long = 12346789 - val remoteFile = Mockito.mock(RemoteFile::class.java) - Mockito.`when`(remoteFile.size).thenReturn(remoteSize) - val localSize: Long = 12346789 - val localFile = Mockito.mock(File::class.java) - Mockito.`when`(localFile.length()).thenReturn(localSize) - Assert.assertTrue("isRemoteAndLocalSizeEqual() returned false instead of true", - FileDiffUtils.isRemoteAndLocalSizeEqual(remoteFile, localFile)) + Assert.assertTrue("isRemoteAndLocalSizeEqual($remoteSize, $localSize) returned false instead of true", + FileDiffUtils.isRemoteAndLocalSizeEqual(remoteSize, localSize)) } /* hasAlreadyBeenDownloaded() */ -- GitLab From cb02d07bb99087e6524ddb8ca8048f01631ffd08 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Thu, 7 Sep 2023 10:32:05 +0200 Subject: [PATCH 08/13] prevent DownloadedFile to use corrupted timestamp --- .../foundation/e/drive/contentScanner/FileDiffUtils.kt | 8 ++++---- .../e/drive/operations/DownloadFileOperation.java | 6 +++++- .../main/java/foundation/e/drive/utils/AppConstants.kt | 3 +++ 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/contentScanner/FileDiffUtils.kt b/app/src/main/java/foundation/e/drive/contentScanner/FileDiffUtils.kt index 8d8f5e72..1908961d 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/FileDiffUtils.kt +++ b/app/src/main/java/foundation/e/drive/contentScanner/FileDiffUtils.kt @@ -10,6 +10,7 @@ package foundation.e.drive.contentScanner import androidx.annotation.VisibleForTesting import com.owncloud.android.lib.resources.files.model.RemoteFile import foundation.e.drive.models.SyncedFileState +import foundation.e.drive.utils.AppConstants import java.io.File @@ -93,14 +94,13 @@ object FileDiffUtils { * * For yet unknown reason, some remote files have this value on cloud (DB & file system) * the only way to fix them is to force re upload of the file with correct value - * @param timestamp remote file timestamp (Long) + * @param timestampInSecond remote file timestamp (Long) * @return true if the timestamp is equal to max of unsigned int 32 */ @VisibleForTesting @JvmStatic - fun isCorruptedTimestamp(timestamp: Long): Boolean { - val maxInt32 = 4294967295L - return timestamp >= maxInt32 + fun isCorruptedTimestamp(timestampInSecond: Long): Boolean { + return timestampInSecond >= AppConstants.CORRUPTED_TIMESTAMP_IN_SECOND } /** 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 256487fc..63fbc406 100644 --- a/app/src/main/java/foundation/e/drive/operations/DownloadFileOperation.java +++ b/app/src/main/java/foundation/e/drive/operations/DownloadFileOperation.java @@ -18,6 +18,8 @@ import static com.owncloud.android.lib.common.operations.RemoteOperationResult.R import static java.nio.file.StandardCopyOption.REPLACE_EXISTING; +import static foundation.e.drive.utils.AppConstants.CORRUPTED_TIMESTAMP_IN_SECOND; + import android.content.Context; import android.media.MediaScannerConnection; @@ -154,7 +156,9 @@ public class DownloadFileOperation extends RemoteOperation { return FORBIDDEN; } - targetFile.setLastModified(remoteFile.getModifiedTimestamp()); + if (remoteFile.getModifiedTimestamp() < CORRUPTED_TIMESTAMP_IN_SECOND) { + targetFile.setLastModified(remoteFile.getModifiedTimestamp()*1000); //android uses timestamp in MS + } syncedFileState.setLastModified(targetFile.lastModified()); syncedFileState.setLastEtag(remoteFile.getEtag()); diff --git a/app/src/main/java/foundation/e/drive/utils/AppConstants.kt b/app/src/main/java/foundation/e/drive/utils/AppConstants.kt index 9822b5b8..33fd937e 100644 --- a/app/src/main/java/foundation/e/drive/utils/AppConstants.kt +++ b/app/src/main/java/foundation/e/drive/utils/AppConstants.kt @@ -38,6 +38,9 @@ object AppConstants { const val notificationChannelID = "foundation.e.drive" const val WORK_GENERIC_TAG = "eDrive" const val WORK_INITIALIZATION_TAG = "eDrive-init" + const val CORRUPTED_TIMESTAMP_IN_SECOND = 4294967295L + + @JvmField val USER_AGENT = "eos(" + buildTime + ")-eDrive(" + BuildConfig.VERSION_NAME + ")" -- GitLab From 0cc8709b731e1dd95eb4b7daf18712a07c2f3865 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Thu, 7 Sep 2023 10:51:15 +0200 Subject: [PATCH 09/13] fix situation where it could override remote file with too old local file --- .../e/drive/contentScanner/FileDiffUtils.kt | 12 ++++++------ .../drive/contentScanner/FileDiffUtilsTest.kt | 19 ++++++++++++++++--- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/contentScanner/FileDiffUtils.kt b/app/src/main/java/foundation/e/drive/contentScanner/FileDiffUtils.kt index 1908961d..f54f1523 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/FileDiffUtils.kt +++ b/app/src/main/java/foundation/e/drive/contentScanner/FileDiffUtils.kt @@ -24,10 +24,10 @@ object FileDiffUtils { */ @JvmStatic fun getActionForFileDiff(remoteFile: RemoteFile, fileState: SyncedFileState): Action { - if (isCorruptedTimestamp(remoteFile.modifiedTimestamp)) return Action.Upload + if (!hasEtagChanged(remoteFile, fileState)) { + if (isCorruptedTimestamp(remoteFile.modifiedTimestamp)) return Action.Upload - if (hasAlreadyBeenDownloaded(fileState) && !hasEtagChanged(remoteFile, fileState)) { - return Action.Skip + if (hasAlreadyBeenDownloaded(fileState)) return Action.Skip } val localFileSize = getLocalFileSize(fileState.localPath) if (isRemoteAndLocalSizeEqual(remoteFile.size, localFileSize)) { @@ -77,14 +77,14 @@ object FileDiffUtils { /** * Compare file size for remote file & local file - * @param remoteFile RemoteFile instance - * @param localFile File instance + * @param remoteFileSize RemoteFile.size. Note: it's equal to getLength() except for + * folder where it also sum size of its content + * @param localFileSize File.length() * @return true if remote file size is same as local file size */ @VisibleForTesting @JvmStatic fun isRemoteAndLocalSizeEqual(remoteFileSize: Long, localFileSize: Long): Boolean { - // remoteFile.getSize() : getSize() is equal to getLength() except for folder where it is also sum the content of the folder! return remoteFileSize == localFileSize } diff --git a/app/src/test/java/foundation/e/drive/contentScanner/FileDiffUtilsTest.kt b/app/src/test/java/foundation/e/drive/contentScanner/FileDiffUtilsTest.kt index 5262e15b..2478e14a 100644 --- a/app/src/test/java/foundation/e/drive/contentScanner/FileDiffUtilsTest.kt +++ b/app/src/test/java/foundation/e/drive/contentScanner/FileDiffUtilsTest.kt @@ -267,16 +267,29 @@ internal class FileDiffUtilsTest { } @Test - fun `getActionForFileDiff(RemoteFile) return Upload when remote modifiedTimestamp is corrupted`() { + fun `getActionForFileDiff(RemoteFile) return Upload when remote modifiedTimestamp is corrupted and eTag unchanged`() { val mockedRemoteFile = Mockito.mock(RemoteFile::class.java) Mockito.`when`(mockedRemoteFile.modifiedTimestamp).thenReturn(4294967295L) - + Mockito.`when`(mockedRemoteFile.etag).thenReturn("123456") val mockedFileState = Mockito.mock(SyncedFileState::class.java) - + Mockito.`when`(mockedFileState.lastEtag).thenReturn("123456") val resultUnderTest = FileDiffUtils.getActionForFileDiff(mockedRemoteFile, mockedFileState) Assert.assertEquals("getActionForFileDiff(RemoteFile) returned $resultUnderTest instead of Action.Upload", FileDiffUtils.Action.Upload, resultUnderTest) } + @Test + fun `getActionForFileDiff(RemoteFile) don't return Upload when remote modifiedTimestamp is corrupted but eTag changed`() { + val mockedRemoteFile = Mockito.mock(RemoteFile::class.java) + Mockito.`when`(mockedRemoteFile.modifiedTimestamp).thenReturn(4294967295L) + Mockito.`when`(mockedRemoteFile.etag).thenReturn("654321") + val mockedFileState = Mockito.mock(SyncedFileState::class.java) + Mockito.`when`(mockedFileState.lastEtag).thenReturn("123456789") + Mockito.`when`(mockedFileState.isLastEtagStored()).thenReturn(true) + Mockito.`when`(mockedFileState.localPath).thenReturn("/local/file.txt") + val resultUnderTest = FileDiffUtils.getActionForFileDiff(mockedRemoteFile, mockedFileState) + Assert.assertNotEquals("getActionForFileDiff(RemoteFile) returned $resultUnderTest while it should not", FileDiffUtils.Action.Upload, resultUnderTest) + } + /* isCorruptedTimestamp for localFile */ @Test fun `isCorruptedTimestamp() return true with timestamp equal to max of Int32 `() { -- GitLab From 44d9c62ca53611302d39d20b7da09499c947be6c Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Thu, 7 Sep 2023 14:47:47 +0200 Subject: [PATCH 10/13] fix typo in licence header --- .../java/foundation/e/drive/contentScanner/FileDiffUtils.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/foundation/e/drive/contentScanner/FileDiffUtils.kt b/app/src/main/java/foundation/e/drive/contentScanner/FileDiffUtils.kt index f54f1523..9953a60b 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/FileDiffUtils.kt +++ b/app/src/main/java/foundation/e/drive/contentScanner/FileDiffUtils.kt @@ -1,5 +1,5 @@ /* - * Copyright © MURENA SAS 3. + * 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 -- GitLab From 40a3264431f0f64a08026ec21a4f43f20a6e1faf Mon Sep 17 00:00:00 2001 From: Hasib Prince Date: Fri, 8 Sep 2023 08:43:43 +0000 Subject: [PATCH 11/13] Apply 1 suggestion(s) to 1 file(s) --- .../main/java/foundation/e/drive/contentScanner/FileDiffUtils.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/app/src/main/java/foundation/e/drive/contentScanner/FileDiffUtils.kt b/app/src/main/java/foundation/e/drive/contentScanner/FileDiffUtils.kt index 9953a60b..e5a3f498 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/FileDiffUtils.kt +++ b/app/src/main/java/foundation/e/drive/contentScanner/FileDiffUtils.kt @@ -13,7 +13,6 @@ import foundation.e.drive.models.SyncedFileState import foundation.e.drive.utils.AppConstants import java.io.File - object FileDiffUtils { enum class Action { Upload, Download, Skip, UpdateDB } /** -- GitLab From 0d214732ca665a117d0b23fe2868f470210d3630 Mon Sep 17 00:00:00 2001 From: Hasib Prince Date: Fri, 8 Sep 2023 08:54:49 +0000 Subject: [PATCH 12/13] Apply 2 suggestion(s) to 2 file(s) --- .../foundation/e/drive/operations/DownloadFileOperation.java | 2 +- app/src/main/java/foundation/e/drive/utils/AppConstants.kt | 1 - 2 files changed, 1 insertion(+), 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 63fbc406..370e9ede 100644 --- a/app/src/main/java/foundation/e/drive/operations/DownloadFileOperation.java +++ b/app/src/main/java/foundation/e/drive/operations/DownloadFileOperation.java @@ -157,7 +157,7 @@ public class DownloadFileOperation extends RemoteOperation { } if (remoteFile.getModifiedTimestamp() < CORRUPTED_TIMESTAMP_IN_SECOND) { - targetFile.setLastModified(remoteFile.getModifiedTimestamp()*1000); //android uses timestamp in MS + targetFile.setLastModified(remoteFile.getModifiedTimestamp() * 1000); //android uses timestamp in MS } syncedFileState.setLastModified(targetFile.lastModified()); syncedFileState.setLastEtag(remoteFile.getEtag()); diff --git a/app/src/main/java/foundation/e/drive/utils/AppConstants.kt b/app/src/main/java/foundation/e/drive/utils/AppConstants.kt index 33fd937e..fbf2dc62 100644 --- a/app/src/main/java/foundation/e/drive/utils/AppConstants.kt +++ b/app/src/main/java/foundation/e/drive/utils/AppConstants.kt @@ -40,7 +40,6 @@ object AppConstants { const val WORK_INITIALIZATION_TAG = "eDrive-init" const val CORRUPTED_TIMESTAMP_IN_SECOND = 4294967295L - @JvmField val USER_AGENT = "eos(" + buildTime + ")-eDrive(" + BuildConfig.VERSION_NAME + ")" -- GitLab From c67051ab2e614693e72006b0c6dd1333c1cddcd4 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Fri, 8 Sep 2023 11:10:19 +0200 Subject: [PATCH 13/13] Improve readability thanks to Hasib's suggestion --- .../contentScanner/RemoteContentScanner.java | 26 ++++++++++++++----- 1 file changed, 19 insertions(+), 7 deletions(-) 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 a92e2119..3848eb89 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/RemoteContentScanner.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/RemoteContentScanner.java @@ -50,17 +50,13 @@ public class RemoteContentScanner extends AbstractContentScanner { switch (action) { case Upload: - fileState.setLastEtag(""); //Force fake lastEtag value to bypass condition check on UploadFileOperation - syncRequests.put(fileState.getId(), new SyncRequest(fileState, SyncRequest.Type.UPLOAD)); + addUploadRequest(fileState); break; case Download: - Timber.d("Add download SyncRequest for %s", file.getRemotePath()); - syncRequests.put(fileState.getId(), new DownloadRequest(file, fileState)); + addDownloadRequest(fileState, file); 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()); + addUpdateDBRequest(fileState, file); break; } } @@ -110,4 +106,20 @@ public class RemoteContentScanner extends AbstractContentScanner { protected boolean isSyncedFolderParentOfFile(@NonNull SyncedFolder syncedFolder, @NonNull String dirPath) { return syncedFolder.getRemoteFolder().equals(dirPath); } + + private void addUploadRequest(SyncedFileState fileState) { + fileState.setLastEtag(""); //Force fake lastEtag value to bypass condition check on UploadFileOperation + syncRequests.put(fileState.getId(), new SyncRequest(fileState, SyncRequest.Type.UPLOAD)); + } + + private void addUpdateDBRequest(SyncedFileState fileState, RemoteFile file) { + 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()); + } + + private void addDownloadRequest(SyncedFileState fileState, RemoteFile file) { + Timber.d("Add download SyncRequest for %s", file.getRemotePath()); + syncRequests.put(fileState.getId(), new DownloadRequest(file, fileState)); + } } \ No newline at end of file -- GitLab