From 64fd41e1cbfdeadc5f25f65c8742e9cddcdcaaef Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Fri, 22 Dec 2023 14:02:59 +0600 Subject: [PATCH 1/2] 942-Fix_wrong_corruptTimestamp_issue issue: https://gitlab.e.foundation/e/os/backlog/-/issues/942 --- .../drive/periodicScan/contentScanner/FileDiffUtils.kt | 8 ++++---- .../synchronization/tasks/DownloadFileOperation.java | 10 +++++++--- .../main/java/foundation/e/drive/utils/AppConstants.kt | 2 +- .../periodicScan/contentScanner/FileDiffUtilsTest.kt | 10 +++++----- 4 files changed, 17 insertions(+), 13 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/periodicScan/contentScanner/FileDiffUtils.kt b/app/src/main/java/foundation/e/drive/periodicScan/contentScanner/FileDiffUtils.kt index 41e34693..5019cfbf 100644 --- a/app/src/main/java/foundation/e/drive/periodicScan/contentScanner/FileDiffUtils.kt +++ b/app/src/main/java/foundation/e/drive/periodicScan/contentScanner/FileDiffUtils.kt @@ -24,7 +24,7 @@ object FileDiffUtils { @JvmStatic fun getActionForFileDiff(remoteFile: RemoteFile, fileState: SyncedFileState): Action { if (!hasEtagChanged(remoteFile, fileState)) { - if (isCorruptedTimestamp(remoteFile.modifiedTimestamp / 1000)) return Action.Upload + if (isCorruptedTimestamp(remoteFile.modifiedTimestamp)) return Action.Upload if (hasAlreadyBeenDownloaded(fileState)) return Action.Skip } @@ -93,13 +93,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 timestampInSecond remote file timestamp (Long) + * @param timestampInMillisecond remote file timestamp (Long) * @return true if the timestamp is equal to max of unsigned int 32 */ @VisibleForTesting @JvmStatic - fun isCorruptedTimestamp(timestampInSecond: Long): Boolean { - return timestampInSecond >= AppConstants.CORRUPTED_TIMESTAMP_IN_SECOND + fun isCorruptedTimestamp(timestampInMillisecond: Long): Boolean { + return timestampInMillisecond >= AppConstants.CORRUPTED_TIMESTAMP_IN_MILLISECOND } /** diff --git a/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.java b/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.java index 4874e460..7399d54d 100644 --- a/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.java +++ b/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.java @@ -19,7 +19,7 @@ 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 static foundation.e.drive.utils.AppConstants.CORRUPTED_TIMESTAMP_IN_MILLISECOND; import android.content.Context; import android.media.MediaScannerConnection; @@ -159,7 +159,7 @@ public class DownloadFileOperation extends RemoteOperation { return FORBIDDEN; } - if (remoteFile.getModifiedTimestamp() < CORRUPTED_TIMESTAMP_IN_SECOND) { + if (remoteFile.getModifiedTimestamp() < CORRUPTED_TIMESTAMP_IN_MILLISECOND) { targetFile.setLastModified(remoteFile.getModifiedTimestamp()); } syncedFileState.setLastModified(targetFile.lastModified()); @@ -185,10 +185,14 @@ public class DownloadFileOperation extends RemoteOperation { final Path tmpPath = tmpFile.toPath(); final Path copyResult = Files.copy(tmpPath, targetPath, REPLACE_EXISTING); - if (copyResult.toFile().length() == tmpFile.length()) { + final File copyResultFile = copyResult.toFile(); + + if (copyResultFile.length() == tmpFile.length()) { tmpFile.delete(); return true; } + + copyResultFile.delete(); } catch (NoSuchFileException exception) { Timber.w(exception); } catch (IOException | SecurityException | NullPointerException exception) { 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 f3721e60..99423f99 100644 --- a/app/src/main/java/foundation/e/drive/utils/AppConstants.kt +++ b/app/src/main/java/foundation/e/drive/utils/AppConstants.kt @@ -38,7 +38,7 @@ object AppConstants { const val notificationChannelID = "foundation.e.drive" const val WORK_GENERIC_TAG = "eDrive" const val WORK_SETUP_TAG = "eDrive-init" - const val CORRUPTED_TIMESTAMP_IN_SECOND = 4294967295L + const val CORRUPTED_TIMESTAMP_IN_MILLISECOND = 4294967295000L @JvmField val USER_AGENT = "eos(" + buildTime + ")-eDrive(" + BuildConfig.VERSION_NAME + ")" diff --git a/app/src/test/java/foundation/e/drive/periodicScan/contentScanner/FileDiffUtilsTest.kt b/app/src/test/java/foundation/e/drive/periodicScan/contentScanner/FileDiffUtilsTest.kt index f078df1d..75d22856 100644 --- a/app/src/test/java/foundation/e/drive/periodicScan/contentScanner/FileDiffUtilsTest.kt +++ b/app/src/test/java/foundation/e/drive/periodicScan/contentScanner/FileDiffUtilsTest.kt @@ -294,21 +294,21 @@ internal class FileDiffUtilsTest { /* isCorruptedTimestamp for localFile */ @Test fun `isCorruptedTimestamp() return true with timestamp equal to max of Int32 `() { - val corruptedTimestamp = 4294967295L + val corruptedTimestamp = 4294967295000L val resultUnderTest = FileDiffUtils.isCorruptedTimestamp(corruptedTimestamp) - Assert.assertTrue("isCorruptedTimestamp(4294967295L) returned $resultUnderTest instead of true", resultUnderTest) + Assert.assertTrue("isCorruptedTimestamp(4294967295000L) returned $resultUnderTest instead of true", resultUnderTest) } @Test fun `isCorruptedTimestamp() return true with timestamp bigger than max of Int32 `() { - val corruptedTimestamp = 4294967295L + 1 + val corruptedTimestamp = 4294967295000L + 1 val resultUnderTest = FileDiffUtils.isCorruptedTimestamp(corruptedTimestamp) - Assert.assertTrue("isCorruptedTimestamp(4294967295L) returned $resultUnderTest instead of true", resultUnderTest) + Assert.assertTrue("isCorruptedTimestamp(4294967295000L) returned $resultUnderTest instead of true", resultUnderTest) } @Test fun `isCorruptedTimestamp() return false with timestamp smaller than max of Int32 `() { - val corruptedTimestamp = 4294967295L - 1 + val corruptedTimestamp = 4294967295000L - 1 val resultUnderTest = FileDiffUtils.isCorruptedTimestamp(corruptedTimestamp) Assert.assertFalse("isCorruptedTimestamp(4294967295000L) returned $resultUnderTest instead of false", resultUnderTest) } -- GitLab From 01187f5e8401c1607a355c7095065ee5b75f9c61 Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Fri, 22 Dec 2023 16:37:15 +0600 Subject: [PATCH 2/2] update regarding review --- .../drive/periodicScan/contentScanner/FileDiffUtilsTest.kt | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/app/src/test/java/foundation/e/drive/periodicScan/contentScanner/FileDiffUtilsTest.kt b/app/src/test/java/foundation/e/drive/periodicScan/contentScanner/FileDiffUtilsTest.kt index 75d22856..f94fdebf 100644 --- a/app/src/test/java/foundation/e/drive/periodicScan/contentScanner/FileDiffUtilsTest.kt +++ b/app/src/test/java/foundation/e/drive/periodicScan/contentScanner/FileDiffUtilsTest.kt @@ -10,6 +10,7 @@ package foundation.e.drive.periodicScan.contentScanner import com.owncloud.android.lib.resources.files.model.RemoteFile import foundation.e.drive.models.SyncedFileState import foundation.e.drive.periodicScan.contentScanner.FileDiffUtils +import foundation.e.drive.utils.AppConstants import org.junit.Assert import org.junit.Test import org.mockito.Mockito @@ -294,21 +295,21 @@ internal class FileDiffUtilsTest { /* isCorruptedTimestamp for localFile */ @Test fun `isCorruptedTimestamp() return true with timestamp equal to max of Int32 `() { - val corruptedTimestamp = 4294967295000L + val corruptedTimestamp = AppConstants.CORRUPTED_TIMESTAMP_IN_MILLISECOND val resultUnderTest = FileDiffUtils.isCorruptedTimestamp(corruptedTimestamp) Assert.assertTrue("isCorruptedTimestamp(4294967295000L) returned $resultUnderTest instead of true", resultUnderTest) } @Test fun `isCorruptedTimestamp() return true with timestamp bigger than max of Int32 `() { - val corruptedTimestamp = 4294967295000L + 1 + val corruptedTimestamp = AppConstants.CORRUPTED_TIMESTAMP_IN_MILLISECOND + 1 val resultUnderTest = FileDiffUtils.isCorruptedTimestamp(corruptedTimestamp) Assert.assertTrue("isCorruptedTimestamp(4294967295000L) returned $resultUnderTest instead of true", resultUnderTest) } @Test fun `isCorruptedTimestamp() return false with timestamp smaller than max of Int32 `() { - val corruptedTimestamp = 4294967295000L - 1 + val corruptedTimestamp = AppConstants.CORRUPTED_TIMESTAMP_IN_MILLISECOND - 1 val resultUnderTest = FileDiffUtils.isCorruptedTimestamp(corruptedTimestamp) Assert.assertFalse("isCorruptedTimestamp(4294967295000L) returned $resultUnderTest instead of false", resultUnderTest) } -- GitLab