From a10800badf625aa0d8a392063136aef24ddcb093 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Mon, 15 Apr 2024 09:20:45 +0200 Subject: [PATCH 01/25] refactor: rewrite DownloadFileOperation in kotlin --- .../e/drive/models/SyncWrapper.java | 20 +- .../tasks/DownloadFileOperation.java | 230 ------------------ .../tasks/DownloadFileOperation.kt | 216 ++++++++++++++++ 3 files changed, 231 insertions(+), 235 deletions(-) delete mode 100644 app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.java create mode 100644 app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt diff --git a/app/src/main/java/foundation/e/drive/models/SyncWrapper.java b/app/src/main/java/foundation/e/drive/models/SyncWrapper.java index 6bbf6221..3deb0c34 100644 --- a/app/src/main/java/foundation/e/drive/models/SyncWrapper.java +++ b/app/src/main/java/foundation/e/drive/models/SyncWrapper.java @@ -1,9 +1,19 @@ /* - * 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 + * Copyright (C) 2024 MURENA SAS + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * */ package foundation.e.drive.models; 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 deleted file mode 100644 index 7399d54d..00000000 --- a/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.java +++ /dev/null @@ -1,230 +0,0 @@ -/* - * Copyright © CLEUS SAS 2018-2019. - * 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.synchronization.tasks; - -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.HOST_NOT_AVAILABLE; -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; - -import static foundation.e.drive.utils.AppConstants.CORRUPTED_TIMESTAMP_IN_MILLISECOND; - -import android.content.Context; -import android.media.MediaScannerConnection; - -import androidx.annotation.NonNull; -import androidx.annotation.Nullable; - -import com.owncloud.android.lib.common.OwnCloudClient; -import com.owncloud.android.lib.common.operations.RemoteOperation; -import com.owncloud.android.lib.common.operations.RemoteOperationResult; -import com.owncloud.android.lib.resources.files.DownloadFileRemoteOperation; -import com.owncloud.android.lib.resources.files.model.RemoteFile; -import java.io.File; -import java.io.IOException; -import java.nio.file.Files; -import java.nio.file.NoSuchFileException; -import java.nio.file.Path; - -import foundation.e.drive.database.DbHelper; -import foundation.e.drive.models.SyncedFileState; -import foundation.e.drive.utils.FileUtils; -import timber.log.Timber; - -/** - * @author Vincent Bourgmayer - * Encapsulate a global download process for a file - * /!\ Doesn't require NextcloudClient yet - */ -public class DownloadFileOperation extends RemoteOperation { - - private final RemoteFile remoteFile; - private final Context context; - private final String targetPath; - private int restartCounter =0; - private final SyncedFileState syncedFileState; - private final String previousEtag; - - /** - * Constructor of download operation where syncedFileState is already known - * @param remoteFile remote file to Download - * @param syncedFileState SyncedFileState corresponding to remote file - */ - public DownloadFileOperation(@NonNull RemoteFile remoteFile, @NonNull SyncedFileState syncedFileState, @NonNull Context context) { - this.remoteFile = remoteFile; - this.syncedFileState = syncedFileState; - this.previousEtag = syncedFileState.getLastEtag(); - this.targetPath = syncedFileState.getLocalPath(); - this.context = context; - } - - @SuppressWarnings("deprecation") - @Override - @NonNull - protected RemoteOperationResult run(@NonNull OwnCloudClient ownCloudClient) { - Timber.v( "run() for %s", remoteFile.getRemotePath()); - if (syncedFileState.getId() == -1) { - this.syncedFileState.setId(DbHelper.manageSyncedFileStateDB(this.syncedFileState, "INSERT", context)); - } - - if (syncedFileState.getLastEtag().equals(remoteFile.getEtag()) - && syncedFileState.getLastModified() > 0L) { - Timber.v( "%s already up-to-date", remoteFile.getRemotePath()); - return new RemoteOperationResult(RemoteOperationResult.ResultCode.ETAG_UNCHANGED); - } - - final String tmpTargetFolderPath = context.getExternalCacheDir().getAbsolutePath(); - final DownloadFileRemoteOperation downloadOperation = new DownloadFileRemoteOperation(remoteFile.getRemotePath(), - tmpTargetFolderPath); - - //noinspection deprecation - final RemoteOperationResult downloadResult = downloadOperation.execute(ownCloudClient); - RemoteOperationResult.ResultCode resultCode; - - boolean mustRestart = true; - - if (downloadResult.isSuccess()) { - resultCode = onDownloadSuccess(tmpTargetFolderPath); - - if (resultCode == OK || resultCode == FORBIDDEN) { - mustRestart = false; - } - - } else if (isNetworkDisconnected(downloadResult)) { - mustRestart = false; - resultCode = downloadResult.getCode(); - } else { - Timber.d("Download failed: %s, %s", downloadResult.getCode(), downloadResult.getLogMessage()); - resultCode = RemoteOperationResult.ResultCode.UNKNOWN_ERROR; - } - - if (mustRestart) { - Timber.v("%s unsuccessfull trial.s of downloading %s", restartCounter, remoteFile.getRemotePath()); - syncedFileState.setLastEtag(this.previousEtag); - if (this.restartCounter < 3) { - this.restartCounter += 1; - return this.run(ownCloudClient); - } else { - resultCode = INVALID_OVERWRITE; - } - } - - //So now, we can update instance of SyncedState and save it to DB - if (DbHelper.manageSyncedFileStateDB(syncedFileState, "UPDATE", context) <= 0) { - //todo : define what to do in this case. Is this test even relevant ? - } - return new RemoteOperationResult(resultCode); - } - - private boolean isNetworkDisconnected(@NonNull final RemoteOperationResult result) { - RemoteOperationResult.ResultCode resultCode = result.getCode(); - return resultCode == NO_NETWORK_CONNECTION - || resultCode == WRONG_CONNECTION - || resultCode == HOST_NOT_AVAILABLE; - } - - private RemoteOperationResult.ResultCode onDownloadSuccess(String tmpTargetFolderPath) { - final String tmpFilePath = tmpTargetFolderPath + remoteFile.getRemotePath(); - final File tmpFile = new File(tmpFilePath); - - if (!tmpFile.exists()) { - Timber.d("Missing downloaded temporary file for %s", remoteFile.getRemotePath()); - return FILE_NOT_FOUND; - } - - final long tmpFileSize = tmpFile.length(); - final long remoteFileSize = remoteFile.getLength(); - - if (tmpFileSize != remoteFileSize) { - Timber.d("Local file size (%s) and remote file size (%s) of %s doesn't match", tmpFileSize, remoteFileSize, remoteFile.getRemotePath()); - tmpFile.delete(); - return INVALID_OVERWRITE; - } - - final File targetFile = new File(targetPath); - if (!moveFileToRealLocation(tmpFile, targetFile)) { - Timber.d("Failed to move %s to %s", tmpFile.getAbsolutePath(), targetPath); - return FORBIDDEN; - } - - if (remoteFile.getModifiedTimestamp() < CORRUPTED_TIMESTAMP_IN_MILLISECOND) { - targetFile.setLastModified(remoteFile.getModifiedTimestamp()); - } - syncedFileState.setLastModified(targetFile.lastModified()); - syncedFileState.setLastEtag(remoteFile.getEtag()); - - doActionMediaScannerConnectionScanFile(context, syncedFileState.getLocalPath()); //required to make Gallery show new image - return OK; - } - - /** - * Move the temporary file downloaded to its final location - * @param tmpFile Temporary file that has just been downloaded - * @param targetFile real location of the file - * @return true if success, false otherwise - */ - private boolean moveFileToRealLocation(File tmpFile, File targetFile) { - final File targetDir = targetFile.getParentFile(); - if (targetDir == null) return false; - try { - if (!targetDir.exists()) targetDir.mkdirs(); - - final Path targetPath = targetFile.toPath(); - final Path tmpPath = tmpFile.toPath(); - - final Path copyResult = Files.copy(tmpPath, targetPath, REPLACE_EXISTING); - 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) { - Timber.e(exception); - } - return false; - } - - /** - * Allow to read remote file path from the operation - * @return path of a remote file - */ - @Nullable - public String getRemoteFilePath() { - return remoteFile.getRemotePath(); - } - - - /** - * Update Gallery for showing the file in parameter - * - * @param context Calling service context - * @param filePath String containing path the file to update by mediaScanner - */ - private void doActionMediaScannerConnectionScanFile(@NonNull Context context, @NonNull final String filePath) { - Timber.v("doActionMediaScannerConnectionScanFile( %s )", filePath); - final String[] filePathArray = new String[] { filePath }; - final String[] mimeType = new String[] {FileUtils.getMimeType(new File(filePath))}; - MediaScannerConnection.scanFile(context, - filePathArray, - mimeType, - (path, uri) -> Timber.tag("MediaScannerConnection") - .v("file %s was scanned with success: %s ", path, uri)); - } -} diff --git a/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt b/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt new file mode 100644 index 00000000..ee21e9e3 --- /dev/null +++ b/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt @@ -0,0 +1,216 @@ +/* + * Copyright (C) 2024 MURENA SAS + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * + */ +package foundation.e.drive.synchronization.tasks + +import android.content.Context +import android.media.MediaScannerConnection + +import com.owncloud.android.lib.common.OwnCloudClient +import com.owncloud.android.lib.common.operations.RemoteOperation +import com.owncloud.android.lib.common.operations.RemoteOperationResult +import com.owncloud.android.lib.common.operations.RemoteOperationResult.ResultCode +import com.owncloud.android.lib.resources.files.DownloadFileRemoteOperation +import com.owncloud.android.lib.resources.files.model.RemoteFile +import foundation.e.drive.database.DbHelper +import foundation.e.drive.models.SyncedFileState +import foundation.e.drive.utils.AppConstants.CORRUPTED_TIMESTAMP_IN_MILLISECOND +import timber.log.Timber +import java.io.File +import java.nio.file.Files +import java.nio.file.StandardCopyOption +import java.util.stream.IntStream.range + +/** + * @author Vincent Bourgmayer + * Encapsulates a global download process for a remote file + * + * /!\ ignores deprecation about download with OwncloudClient + * because NC lib doesn't implemented it with NextcloudClient + */ +class DownloadFileOperation(private val remoteFile: RemoteFile, private val fileState: SyncedFileState, private val context: Context): RemoteOperation() { + + private val previousEtag: String = fileState.lastEtag + private val targetFilePath = fileState.localPath + + override fun run(ocClient: OwnCloudClient): RemoteOperationResult { + Timber.v("DownloadFileOperation.run() for ${remoteFile.remotePath} ") + persistIfNewSyncedFileState() + + if (isFileUpToDate()) { + Timber.d( "${remoteFile.remotePath} already up-to-date") + return RemoteOperationResult(ResultCode.ETAG_UNCHANGED) + } + + val tmpDirectoryPath = context.externalCacheDir!!.absolutePath + val tmpFile = File(tmpDirectoryPath + remoteFile.remotePath) + + val downloadResult = downloadFile(tmpDirectoryPath, tmpFile, ocClient) + + if (downloadResult != ResultCode.OK) { + return RemoteOperationResult(downloadResult) + } + + val targetFile = File(targetFilePath) + setRemoteTimestampOnFile(targetFile) + + makeGalleryDetectTheFile(context, targetFilePath) + persistUpdatedSyncedFileState(targetFile.lastModified()) + return RemoteOperationResult(ResultCode.OK) + } + + /** + * Makes device be aware of this file instantly + * + * @param context Calling service context + * @param filePath String containing path the file to update by mediaScanner + */ + private fun makeGalleryDetectTheFile(context: Context, filePath: String) { + Timber.v("makeGalleryDetectTheFile( $filePath )" ) + + MediaScannerConnection.scanFile(context, + arrayOf(filePath), + arrayOf(remoteFile.mimeType)) { + path, uri -> Timber.tag("MediaScannerConnection") + .v("file %s was scanned with success: %s ", path, uri) + } + } + + private fun setRemoteTimestampOnFile(targetFile: File) { + if (remoteFile.modifiedTimestamp < CORRUPTED_TIMESTAMP_IN_MILLISECOND) { + targetFile.setLastModified(remoteFile.modifiedTimestamp) + } + } + + private fun persistUpdatedSyncedFileState(fileLastModifiedTimestamp: Long) { + fileState.lastModified = fileLastModifiedTimestamp + fileState.lastEtag = remoteFile.etag!! //already checked in "isFileUpToDate()" + + if (DbHelper.manageSyncedFileStateDB(fileState, "UPDATE", context) <= 0) { + //todo : define what to do in this case. Is this test even relevant ? + } + } + + @Suppress("DEPRECATION") + private fun downloadFile(tmpDirPath: String, tmpFile: File, ocClient: OwnCloudClient): ResultCode { + var lastResultCode: ResultCode? = null + + for (trialCount in range(1, 3)) { + fileState.lastEtag = previousEtag + + val downloadFileOperation = DownloadFileRemoteOperation(remoteFile.remotePath, tmpDirPath) + val downloadResult = downloadFileOperation.execute(ocClient) + + lastResultCode = downloadResult.code + + if (downloadResult.isSuccess) { + + lastResultCode = isValidDownloadedFile(tmpFile) + if (lastResultCode == ResultCode.OK) { + break + } + } else { + Timber.d("Download trial: $trialCount failed: ${downloadResult.code}, ${downloadResult.logMessage}") + if (isNetworkFailure(lastResultCode)) { + break + } + } + } + + if (lastResultCode == ResultCode.OK + && !moveTmpFileToRealLocation(tmpFile)) { + return ResultCode.FORBIDDEN + } + + return lastResultCode!! + } + + private fun isValidDownloadedFile(tmpFile: File): ResultCode { + if (!tmpFile.exists()) { + Timber.d("Missing downloaded temporary file for ${remoteFile.remotePath}") + return ResultCode.FILE_NOT_FOUND + } + + if (!isExpectedFileSize(tmpFile.length())) { + tmpFile.delete() + return ResultCode.INVALID_OVERWRITE + } + return ResultCode.OK + } + + /** + * Moves the temporary file downloaded to its final location + * @param tmpFile Temporary file that has just been downloaded + * @return true if success, false otherwise + */ + private fun moveTmpFileToRealLocation(tmpFile: File): Boolean { + val targetFile = File(targetFilePath) + val targetDir = targetFile.parentFile ?: return false + + try { + if (!targetDir.exists()) targetDir.mkdirs() + + val targetPath = targetFile.toPath() + val tmpPath = tmpFile.toPath() + + val copyResult = Files.copy(tmpPath, targetPath, StandardCopyOption.REPLACE_EXISTING) + val copiedFile = copyResult.toFile() + + if (copiedFile.length() == tmpFile.length()) { + tmpFile.delete() + return true + } + copiedFile.delete() + + } catch (noSuchFileException: NoSuchFileException) { + Timber.w(noSuchFileException) + } catch (otherException: Exception) { + Timber.e(otherException) + } + Timber.d("Failed to move ${tmpFile.absolutePath} to $targetFilePath") + return false + } + + private fun isExpectedFileSize(tmpFileSize: Long): Boolean { + val remoteFileSize = remoteFile.length + if (tmpFileSize != remoteFileSize) { + Timber.d("Local file's size $tmpFileSize and remote file's size $remoteFileSize of ${remoteFile.remotePath} doesn't match") + return false + } + return true + } + + /** + * Check if failed download is due to network issue + */ + private fun isNetworkFailure(resultCode: ResultCode): Boolean { + return resultCode in listOf(ResultCode.NO_NETWORK_CONNECTION, + ResultCode.HOST_NOT_AVAILABLE, + ResultCode.WRONG_CONNECTION) + } + + private fun isFileUpToDate(): Boolean = !remoteFile.etag.isNullOrEmpty() + && fileState.lastEtag == remoteFile.etag + && fileState.lastModified > 0L + + private fun persistIfNewSyncedFileState() { + if (fileState.id == -1) { + val id = DbHelper.manageSyncedFileStateDB(fileState, "INSERT", context) + fileState.id = id + } + } +} \ No newline at end of file -- GitLab From 5beaf65437ba34271776fec9986270330f6cf0b5 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Wed, 17 Apr 2024 10:27:17 +0200 Subject: [PATCH 02/25] chore: fix too long line report from linter --- .../e/drive/synchronization/tasks/DownloadFileOperation.kt | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt b/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt index ee21e9e3..af2e3118 100644 --- a/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt +++ b/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt @@ -42,7 +42,9 @@ import java.util.stream.IntStream.range * /!\ ignores deprecation about download with OwncloudClient * because NC lib doesn't implemented it with NextcloudClient */ -class DownloadFileOperation(private val remoteFile: RemoteFile, private val fileState: SyncedFileState, private val context: Context): RemoteOperation() { +class DownloadFileOperation(private val remoteFile: RemoteFile, + private val fileState: SyncedFileState, + private val context: Context): RemoteOperation() { private val previousEtag: String = fileState.lastEtag private val targetFilePath = fileState.localPath @@ -188,7 +190,8 @@ class DownloadFileOperation(private val remoteFile: RemoteFile, private val file private fun isExpectedFileSize(tmpFileSize: Long): Boolean { val remoteFileSize = remoteFile.length if (tmpFileSize != remoteFileSize) { - Timber.d("Local file's size $tmpFileSize and remote file's size $remoteFileSize of ${remoteFile.remotePath} doesn't match") + Timber.d("Local's size $tmpFileSize and remote's size $remoteFileSize " + + "of ${remoteFile.remotePath} don't match") return false } return true -- GitLab From 4a01f7b9d7ae4cbec2ada8662e9fef518a3999db Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Wed, 17 Apr 2024 10:39:35 +0200 Subject: [PATCH 03/25] chore: fix linter report about magic number --- .../drive/synchronization/tasks/DownloadFileOperation.kt | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt b/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt index af2e3118..d9f92076 100644 --- a/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt +++ b/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt @@ -49,6 +49,10 @@ class DownloadFileOperation(private val remoteFile: RemoteFile, private val previousEtag: String = fileState.lastEtag private val targetFilePath = fileState.localPath + companion object { + private const val maxDownloadTrial = 3 + } + override fun run(ocClient: OwnCloudClient): RemoteOperationResult { Timber.v("DownloadFileOperation.run() for ${remoteFile.remotePath} ") persistIfNewSyncedFileState() @@ -111,7 +115,7 @@ class DownloadFileOperation(private val remoteFile: RemoteFile, private fun downloadFile(tmpDirPath: String, tmpFile: File, ocClient: OwnCloudClient): ResultCode { var lastResultCode: ResultCode? = null - for (trialCount in range(1, 3)) { + for (trialCount in range(1, maxDownloadTrial)) { fileState.lastEtag = previousEtag val downloadFileOperation = DownloadFileRemoteOperation(remoteFile.remotePath, tmpDirPath) @@ -216,4 +220,6 @@ class DownloadFileOperation(private val remoteFile: RemoteFile, fileState.id = id } } + + } \ No newline at end of file -- GitLab From 596930f873d6b9a669e4c0b4cc2d9995487ff6d3 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Wed, 17 Apr 2024 10:57:48 +0200 Subject: [PATCH 04/25] chore: add missing extra line at end of file --- .../e/drive/synchronization/tasks/DownloadFileOperation.kt | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt b/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt index d9f92076..3bf84989 100644 --- a/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt +++ b/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt @@ -220,6 +220,4 @@ class DownloadFileOperation(private val remoteFile: RemoteFile, fileState.id = id } } - - -} \ No newline at end of file +} -- GitLab From 940392585090fb350abef1cd80393dbd61fbd072 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Wed, 17 Apr 2024 16:24:02 +0200 Subject: [PATCH 05/25] refactor: move file copy operation from DownloadFileOperation to FileUtils --- .../foundation/e/drive/utils/FileUtils.kt | 65 +++++++++++++++++-- 1 file changed, 59 insertions(+), 6 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/utils/FileUtils.kt b/app/src/main/java/foundation/e/drive/utils/FileUtils.kt index 2a7c87ee..a7b11604 100644 --- a/app/src/main/java/foundation/e/drive/utils/FileUtils.kt +++ b/app/src/main/java/foundation/e/drive/utils/FileUtils.kt @@ -1,9 +1,19 @@ /* - * 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 + * Copyright (C) 2023-2024 MURENA SAS + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * */ package foundation.e.drive.utils @@ -13,6 +23,9 @@ import timber.log.Timber import java.io.File import java.io.IOException import java.net.URLConnection +import java.nio.file.FileSystemException +import java.nio.file.Files +import java.nio.file.StandardCopyOption object FileUtils { /** @@ -72,4 +85,44 @@ object FileUtils { @JvmStatic fun isNotPartFile(file: File) = !isPartFile(file) -} \ No newline at end of file + + @JvmStatic + fun tryToCopyFile(source: File, target: File): Boolean { + var success = false + try { + val copyResult = Files.copy( + source.toPath(), + target.toPath(), + StandardCopyOption.REPLACE_EXISTING + ) + success = validateCopyOperation(copyResult.toFile(), source) + + } catch (fileSystemException: FileSystemException) { + Timber.w(fileSystemException) + } catch (unsupportedOperationException: UnsupportedOperationException ) { + Timber.e(unsupportedOperationException) + } catch (ioException: IOException) { + Timber.e(ioException) + } + return success + } + + @JvmStatic + private fun validateCopyOperation(copiedFile: File, tmpFile: File): Boolean { + return if (copiedFile.length() == tmpFile.length()) { + tmpFile.delete() + true + } else { + copiedFile.delete() + false + } + } + + @JvmStatic + fun ensureDirectoryExists(directory: File): Boolean { + if (!directory.exists()) { + directory.mkdirs() + } + return directory.exists() + } +} -- GitLab From 77601385ec254bdc5a99a1f4b2352d5840806758 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Wed, 17 Apr 2024 16:29:54 +0200 Subject: [PATCH 06/25] refactor: call the copyFile from FileUtils --- .../tasks/DownloadFileOperation.kt | 29 ++++--------------- 1 file changed, 6 insertions(+), 23 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt b/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt index 3bf84989..ae9ded03 100644 --- a/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt +++ b/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt @@ -29,10 +29,9 @@ import com.owncloud.android.lib.resources.files.model.RemoteFile import foundation.e.drive.database.DbHelper import foundation.e.drive.models.SyncedFileState import foundation.e.drive.utils.AppConstants.CORRUPTED_TIMESTAMP_IN_MILLISECOND +import foundation.e.drive.utils.FileUtils import timber.log.Timber import java.io.File -import java.nio.file.Files -import java.nio.file.StandardCopyOption import java.util.stream.IntStream.range /** @@ -167,28 +166,12 @@ class DownloadFileOperation(private val remoteFile: RemoteFile, val targetFile = File(targetFilePath) val targetDir = targetFile.parentFile ?: return false - try { - if (!targetDir.exists()) targetDir.mkdirs() - - val targetPath = targetFile.toPath() - val tmpPath = tmpFile.toPath() - - val copyResult = Files.copy(tmpPath, targetPath, StandardCopyOption.REPLACE_EXISTING) - val copiedFile = copyResult.toFile() - - if (copiedFile.length() == tmpFile.length()) { - tmpFile.delete() - return true - } - copiedFile.delete() - - } catch (noSuchFileException: NoSuchFileException) { - Timber.w(noSuchFileException) - } catch (otherException: Exception) { - Timber.e(otherException) + return if (FileUtils.ensureDirectoryExists(targetDir)) { + FileUtils.tryToCopyFile(tmpFile, targetFile) + } else { + Timber.d("Failed to move ${tmpFile.absolutePath} to $targetFilePath") + false } - Timber.d("Failed to move ${tmpFile.absolutePath} to $targetFilePath") - return false } private fun isExpectedFileSize(tmpFileSize: Long): Boolean { -- GitLab From d4f24a01f211404f980ac339038e5050d0a49f67 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Thu, 18 Apr 2024 10:43:57 +0200 Subject: [PATCH 07/25] chore: fix return number in DownloadFileOperation.run() --- .../tasks/DownloadFileOperation.kt | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt b/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt index ae9ded03..c6e6bb3a 100644 --- a/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt +++ b/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt @@ -66,16 +66,15 @@ class DownloadFileOperation(private val remoteFile: RemoteFile, val downloadResult = downloadFile(tmpDirectoryPath, tmpFile, ocClient) - if (downloadResult != ResultCode.OK) { - return RemoteOperationResult(downloadResult) - } + if (downloadResult == ResultCode.OK) { + val targetFile = File(targetFilePath) + setRemoteTimestampOnFile(targetFile) - val targetFile = File(targetFilePath) - setRemoteTimestampOnFile(targetFile) + makeGalleryDetectTheFile(context, targetFilePath) + persistUpdatedSyncedFileState(targetFile.lastModified()) + } - makeGalleryDetectTheFile(context, targetFilePath) - persistUpdatedSyncedFileState(targetFile.lastModified()) - return RemoteOperationResult(ResultCode.OK) + return RemoteOperationResult(downloadResult) } /** -- GitLab From f6a0eca509a515c2723904b818ad9dc9241c2e38 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Thu, 18 Apr 2024 11:36:22 +0200 Subject: [PATCH 08/25] chore: fix linter report about two many break in loop --- .../tasks/DownloadFileOperation.kt | 26 +++++++++++-------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt b/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt index c6e6bb3a..6284b4ac 100644 --- a/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt +++ b/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt @@ -119,19 +119,18 @@ class DownloadFileOperation(private val remoteFile: RemoteFile, val downloadFileOperation = DownloadFileRemoteOperation(remoteFile.remotePath, tmpDirPath) val downloadResult = downloadFileOperation.execute(ocClient) - lastResultCode = downloadResult.code - - if (downloadResult.isSuccess) { - - lastResultCode = isValidDownloadedFile(tmpFile) - if (lastResultCode == ResultCode.OK) { - break - } + lastResultCode = if (downloadResult.isSuccess) { + isValidDownloadedFile(tmpFile) } else { + downloadResult.code + } + + if (lastResultCode != ResultCode.OK) { Timber.d("Download trial: $trialCount failed: ${downloadResult.code}, ${downloadResult.logMessage}") - if (isNetworkFailure(lastResultCode)) { - break - } + } + + if (!deserveRetry(lastResultCode)) { + break } } @@ -142,6 +141,11 @@ class DownloadFileOperation(private val remoteFile: RemoteFile, return lastResultCode!! } + + private fun deserveRetry(resultCode: ResultCode?): Boolean { + return (resultCode == null + || (resultCode != ResultCode.OK && !isNetworkFailure(resultCode))) + } private fun isValidDownloadedFile(tmpFile: File): ResultCode { if (!tmpFile.exists()) { -- GitLab From 088904c0ed6b40c494360e270ba7c9f801375a23 Mon Sep 17 00:00:00 2001 From: Jonathan Klee Date: Fri, 19 Apr 2024 07:43:42 +0000 Subject: [PATCH 09/25] chore: apply suggestion about code style (space & indentation) --- .../e/drive/synchronization/tasks/DownloadFileOperation.kt | 4 ++-- app/src/main/java/foundation/e/drive/utils/FileUtils.kt | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt b/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt index 6284b4ac..0066da8d 100644 --- a/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt +++ b/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt @@ -89,8 +89,8 @@ class DownloadFileOperation(private val remoteFile: RemoteFile, MediaScannerConnection.scanFile(context, arrayOf(filePath), arrayOf(remoteFile.mimeType)) { - path, uri -> Timber.tag("MediaScannerConnection") - .v("file %s was scanned with success: %s ", path, uri) + path, uri -> Timber.tag("MediaScannerConnection") + .v("file %s was scanned with success: %s ", path, uri) } } diff --git a/app/src/main/java/foundation/e/drive/utils/FileUtils.kt b/app/src/main/java/foundation/e/drive/utils/FileUtils.kt index a7b11604..0eba491b 100644 --- a/app/src/main/java/foundation/e/drive/utils/FileUtils.kt +++ b/app/src/main/java/foundation/e/drive/utils/FileUtils.kt @@ -88,7 +88,7 @@ object FileUtils { @JvmStatic fun tryToCopyFile(source: File, target: File): Boolean { - var success = false + var success = false try { val copyResult = Files.copy( source.toPath(), @@ -97,7 +97,7 @@ object FileUtils { ) success = validateCopyOperation(copyResult.toFile(), source) - } catch (fileSystemException: FileSystemException) { + } catch (fileSystemException: FileSystemException) { Timber.w(fileSystemException) } catch (unsupportedOperationException: UnsupportedOperationException ) { Timber.e(unsupportedOperationException) -- GitLab From 66d8899eb3361c5119a54e08866e91550233b9f5 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Fri, 19 Apr 2024 11:16:00 +0200 Subject: [PATCH 10/25] chore: apply suggestion about coding style: rename method parameter --- .../drive/synchronization/tasks/DownloadFileOperation.kt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt b/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt index 0066da8d..e86895fb 100644 --- a/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt +++ b/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt @@ -52,7 +52,7 @@ class DownloadFileOperation(private val remoteFile: RemoteFile, private const val maxDownloadTrial = 3 } - override fun run(ocClient: OwnCloudClient): RemoteOperationResult { + override fun run(client: OwnCloudClient): RemoteOperationResult { Timber.v("DownloadFileOperation.run() for ${remoteFile.remotePath} ") persistIfNewSyncedFileState() @@ -64,7 +64,7 @@ class DownloadFileOperation(private val remoteFile: RemoteFile, val tmpDirectoryPath = context.externalCacheDir!!.absolutePath val tmpFile = File(tmpDirectoryPath + remoteFile.remotePath) - val downloadResult = downloadFile(tmpDirectoryPath, tmpFile, ocClient) + val downloadResult = downloadFile(tmpDirectoryPath, tmpFile, client) if (downloadResult == ResultCode.OK) { val targetFile = File(targetFilePath) @@ -110,14 +110,14 @@ class DownloadFileOperation(private val remoteFile: RemoteFile, } @Suppress("DEPRECATION") - private fun downloadFile(tmpDirPath: String, tmpFile: File, ocClient: OwnCloudClient): ResultCode { + private fun downloadFile(tmpDirPath: String, tmpFile: File, client: OwnCloudClient): ResultCode { var lastResultCode: ResultCode? = null for (trialCount in range(1, maxDownloadTrial)) { fileState.lastEtag = previousEtag val downloadFileOperation = DownloadFileRemoteOperation(remoteFile.remotePath, tmpDirPath) - val downloadResult = downloadFileOperation.execute(ocClient) + val downloadResult = downloadFileOperation.execute(client) lastResultCode = if (downloadResult.isSuccess) { isValidDownloadedFile(tmpFile) -- GitLab From 965ea0ffcccd6a9db09ff0ed37e55d6f6f0dde3a Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Fri, 19 Apr 2024 11:17:22 +0200 Subject: [PATCH 11/25] chore: apply suggestion: rename method --- .../e/drive/synchronization/tasks/DownloadFileOperation.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt b/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt index e86895fb..d92b0d39 100644 --- a/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt +++ b/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt @@ -120,7 +120,7 @@ class DownloadFileOperation(private val remoteFile: RemoteFile, val downloadResult = downloadFileOperation.execute(client) lastResultCode = if (downloadResult.isSuccess) { - isValidDownloadedFile(tmpFile) + isFileValid(tmpFile) } else { downloadResult.code } @@ -147,7 +147,7 @@ class DownloadFileOperation(private val remoteFile: RemoteFile, || (resultCode != ResultCode.OK && !isNetworkFailure(resultCode))) } - private fun isValidDownloadedFile(tmpFile: File): ResultCode { + private fun isFileValid(tmpFile: File): ResultCode { if (!tmpFile.exists()) { Timber.d("Missing downloaded temporary file for ${remoteFile.remotePath}") return ResultCode.FILE_NOT_FOUND -- GitLab From 992d758eb5fedf9440e6363a50c41a99a4845955 Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Fri, 19 Apr 2024 09:22:12 +0000 Subject: [PATCH 12/25] chore: apply suggestion: remove timber tagging --- .../e/drive/synchronization/tasks/DownloadFileOperation.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt b/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt index d92b0d39..e7bf806e 100644 --- a/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt +++ b/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt @@ -89,7 +89,7 @@ class DownloadFileOperation(private val remoteFile: RemoteFile, MediaScannerConnection.scanFile(context, arrayOf(filePath), arrayOf(remoteFile.mimeType)) { - path, uri -> Timber.tag("MediaScannerConnection") + path, uri -> Timber .v("file %s was scanned with success: %s ", path, uri) } } -- GitLab From d382740e2f0aa24495672fe57e14b453eb26371f Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Fri, 19 Apr 2024 09:24:25 +0000 Subject: [PATCH 13/25] chore: coding style about isFileUpToDate() --- .../e/drive/synchronization/tasks/DownloadFileOperation.kt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt b/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt index e7bf806e..2640920e 100644 --- a/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt +++ b/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt @@ -196,9 +196,11 @@ class DownloadFileOperation(private val remoteFile: RemoteFile, ResultCode.WRONG_CONNECTION) } - private fun isFileUpToDate(): Boolean = !remoteFile.etag.isNullOrEmpty() + private fun isFileUpToDate(): Boolean { + return !remoteFile.etag.isNullOrEmpty() && fileState.lastEtag == remoteFile.etag && fileState.lastModified > 0L + } private fun persistIfNewSyncedFileState() { if (fileState.id == -1) { -- GitLab From 06ebfb085ab0046c30071bff931daca3f1f17b8a Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Fri, 19 Apr 2024 11:34:30 +0200 Subject: [PATCH 14/25] chore: apply suggestion: rename FileUtils method about copying file --- .../e/drive/synchronization/tasks/DownloadFileOperation.kt | 2 +- app/src/main/java/foundation/e/drive/utils/FileUtils.kt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt b/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt index 2640920e..aa94d3a9 100644 --- a/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt +++ b/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt @@ -170,7 +170,7 @@ class DownloadFileOperation(private val remoteFile: RemoteFile, val targetDir = targetFile.parentFile ?: return false return if (FileUtils.ensureDirectoryExists(targetDir)) { - FileUtils.tryToCopyFile(tmpFile, targetFile) + FileUtils.copyFile(tmpFile, targetFile) } else { Timber.d("Failed to move ${tmpFile.absolutePath} to $targetFilePath") false diff --git a/app/src/main/java/foundation/e/drive/utils/FileUtils.kt b/app/src/main/java/foundation/e/drive/utils/FileUtils.kt index 0eba491b..9b755e6a 100644 --- a/app/src/main/java/foundation/e/drive/utils/FileUtils.kt +++ b/app/src/main/java/foundation/e/drive/utils/FileUtils.kt @@ -87,7 +87,7 @@ object FileUtils { fun isNotPartFile(file: File) = !isPartFile(file) @JvmStatic - fun tryToCopyFile(source: File, target: File): Boolean { + fun copyFile(source: File, target: File): Boolean { var success = false try { val copyResult = Files.copy( -- GitLab From 494131a7caad2426fd0508812bc2ebf23add4115 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Fri, 19 Apr 2024 11:52:21 +0200 Subject: [PATCH 15/25] chore: apply suggestion. Rename and change parameter of isExpectedFileSize --- .../e/drive/synchronization/tasks/DownloadFileOperation.kt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt b/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt index aa94d3a9..a50a52c9 100644 --- a/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt +++ b/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt @@ -153,7 +153,7 @@ class DownloadFileOperation(private val remoteFile: RemoteFile, return ResultCode.FILE_NOT_FOUND } - if (!isExpectedFileSize(tmpFile.length())) { + if (!hasExpectedFileSize(tmpFile)) { tmpFile.delete() return ResultCode.INVALID_OVERWRITE } @@ -177,8 +177,9 @@ class DownloadFileOperation(private val remoteFile: RemoteFile, } } - private fun isExpectedFileSize(tmpFileSize: Long): Boolean { + private fun hasExpectedFileSize(tmpFile: File): Boolean { val remoteFileSize = remoteFile.length + val tmpFileSize = tmpFile.length() if (tmpFileSize != remoteFileSize) { Timber.d("Local's size $tmpFileSize and remote's size $remoteFileSize " + "of ${remoteFile.remotePath} don't match") -- GitLab From 686425cd43ff754efb31bbb2678e5573e6674db7 Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Fri, 19 Apr 2024 09:53:42 +0000 Subject: [PATCH 16/25] chore: apply suggestion about a commented code --- .../synchronization/tasks/DownloadFileOperation.kt | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt b/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt index a50a52c9..8b2be28b 100644 --- a/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt +++ b/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt @@ -104,9 +104,14 @@ class DownloadFileOperation(private val remoteFile: RemoteFile, fileState.lastModified = fileLastModifiedTimestamp fileState.lastEtag = remoteFile.etag!! //already checked in "isFileUpToDate()" - if (DbHelper.manageSyncedFileStateDB(fileState, "UPDATE", context) <= 0) { - //todo : define what to do in this case. Is this test even relevant ? - } + DbHelper.manageSyncedFileStateDB(fileState, "UPDATE", context) + // todo : define what to do in this case. Is this test even relevant ? + // replace the above line with the following + // + // val result = DbHelper.manageSyncedFileStateDB(fileState, "UPDATE", context) + // if (result <= 0) { + // .... + // } } @Suppress("DEPRECATION") -- GitLab From 111e66a022e15537349c294d94e0cbf76e6f74c5 Mon Sep 17 00:00:00 2001 From: Fahim Masud Choudhury Date: Thu, 25 Apr 2024 07:22:59 +0000 Subject: [PATCH 17/25] chore: update comments & method's doc --- .../e/drive/synchronization/tasks/DownloadFileOperation.kt | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt b/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt index 8b2be28b..90096628 100644 --- a/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt +++ b/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt @@ -38,8 +38,9 @@ import java.util.stream.IntStream.range * @author Vincent Bourgmayer * Encapsulates a global download process for a remote file * - * /!\ ignores deprecation about download with OwncloudClient - * because NC lib doesn't implemented it with NextcloudClient + * /!\ Ignores deprecation about download with OwncloudClient + * because NC lib doesn't implement it with NextcloudClient + */ */ class DownloadFileOperation(private val remoteFile: RemoteFile, private val fileState: SyncedFileState, @@ -194,7 +195,7 @@ class DownloadFileOperation(private val remoteFile: RemoteFile, } /** - * Check if failed download is due to network issue + * Checks if failed download is due to network issue */ private fun isNetworkFailure(resultCode: ResultCode): Boolean { return resultCode in listOf(ResultCode.NO_NETWORK_CONNECTION, -- GitLab From 68e1ecb6322766a19134f158ababd1fd5f10c35a Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Thu, 25 Apr 2024 09:33:34 +0200 Subject: [PATCH 18/25] chore: apply suggestion: rename const & add supress deprecation annotation --- .../e/drive/synchronization/tasks/DownloadFileOperation.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt b/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt index 90096628..69179a6b 100644 --- a/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt +++ b/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt @@ -41,7 +41,6 @@ import java.util.stream.IntStream.range * /!\ Ignores deprecation about download with OwncloudClient * because NC lib doesn't implement it with NextcloudClient */ - */ class DownloadFileOperation(private val remoteFile: RemoteFile, private val fileState: SyncedFileState, private val context: Context): RemoteOperation() { @@ -50,9 +49,10 @@ class DownloadFileOperation(private val remoteFile: RemoteFile, private val targetFilePath = fileState.localPath companion object { - private const val maxDownloadTrial = 3 + private const val MAX_DOWNLOAD_TRIAL = 3 } + @Suppress("OVERRIDE_DEPRECATION") override fun run(client: OwnCloudClient): RemoteOperationResult { Timber.v("DownloadFileOperation.run() for ${remoteFile.remotePath} ") persistIfNewSyncedFileState() @@ -119,7 +119,7 @@ class DownloadFileOperation(private val remoteFile: RemoteFile, private fun downloadFile(tmpDirPath: String, tmpFile: File, client: OwnCloudClient): ResultCode { var lastResultCode: ResultCode? = null - for (trialCount in range(1, maxDownloadTrial)) { + for (trialCount in range(1, MAX_DOWNLOAD_TRIAL)) { fileState.lastEtag = previousEtag val downloadFileOperation = DownloadFileRemoteOperation(remoteFile.remotePath, tmpDirPath) -- GitLab From 8f20b40ff8d2c3125e070bdd66ed11ccf3082f16 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Thu, 25 Apr 2024 09:40:56 +0200 Subject: [PATCH 19/25] chore: apply suggestion. Add a try catch to setRemoteTimestampOnFile() --- .../drive/synchronization/tasks/DownloadFileOperation.kt | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt b/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt index 69179a6b..f800c31b 100644 --- a/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt +++ b/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt @@ -97,7 +97,13 @@ class DownloadFileOperation(private val remoteFile: RemoteFile, private fun setRemoteTimestampOnFile(targetFile: File) { if (remoteFile.modifiedTimestamp < CORRUPTED_TIMESTAMP_IN_MILLISECOND) { - targetFile.setLastModified(remoteFile.modifiedTimestamp) + try { + targetFile.setLastModified(remoteFile.modifiedTimestamp) + } catch(securityException: SecurityException) { + Timber.w(securityException, "Can't update last modified timestamp") + } catch (illegalArgumentException: IllegalArgumentException) { + Timber.w(illegalArgumentException, "Can't update last modified timestamp to: ${remoteFile.modifiedTimestamp}") + } } } -- GitLab From 4a0295ab79d93e5001ae2eeae304f9a289dc7abf Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Thu, 25 Apr 2024 09:43:59 +0200 Subject: [PATCH 20/25] chore: apply suggestion. change nullable var to not nullable var in downloadFile() --- .../e/drive/synchronization/tasks/DownloadFileOperation.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt b/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt index f800c31b..bb5d54df 100644 --- a/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt +++ b/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt @@ -123,7 +123,7 @@ class DownloadFileOperation(private val remoteFile: RemoteFile, @Suppress("DEPRECATION") private fun downloadFile(tmpDirPath: String, tmpFile: File, client: OwnCloudClient): ResultCode { - var lastResultCode: ResultCode? = null + var lastResultCode: ResultCode = ResultCode.UNKNOWN_ERROR for (trialCount in range(1, MAX_DOWNLOAD_TRIAL)) { fileState.lastEtag = previousEtag @@ -151,7 +151,7 @@ class DownloadFileOperation(private val remoteFile: RemoteFile, return ResultCode.FORBIDDEN } - return lastResultCode!! + return lastResultCode } private fun deserveRetry(resultCode: ResultCode?): Boolean { -- GitLab From 9c194def42306d944cba14e5fe5dcf80ab7e8d98 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Thu, 25 Apr 2024 09:55:34 +0200 Subject: [PATCH 21/25] refactor: apply suggestion. Avoid usage of git add app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt in run() --- .../tasks/DownloadFileOperation.kt | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt b/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt index bb5d54df..f49b3025 100644 --- a/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt +++ b/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt @@ -57,9 +57,9 @@ class DownloadFileOperation(private val remoteFile: RemoteFile, Timber.v("DownloadFileOperation.run() for ${remoteFile.remotePath} ") persistIfNewSyncedFileState() - if (isFileUpToDate()) { - Timber.d( "${remoteFile.remotePath} already up-to-date") - return RemoteOperationResult(ResultCode.ETAG_UNCHANGED) + val canStartResultCode = checkStartCondition() + if (canStartResultCode != ResultCode.OK) { + return RemoteOperationResult(canStartResultCode) } val tmpDirectoryPath = context.externalCacheDir!!.absolutePath @@ -77,6 +77,20 @@ class DownloadFileOperation(private val remoteFile: RemoteFile, return RemoteOperationResult(downloadResult) } + + private fun checkStartCondition(): ResultCode { + var resultCode = ResultCode.OK + + if (isFileUpToDate()) { + Timber.d( "${remoteFile.remotePath} already up-to-date") + resultCode = ResultCode.ETAG_UNCHANGED + } else if (context.externalCacheDir == null) { + Timber.d( "context.externalCacheDir is null") + resultCode = ResultCode.FORBIDDEN + } + + return resultCode + } /** * Makes device be aware of this file instantly -- GitLab From 7168754bbdf61fb8118d7e6d7d6328a652f73252 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Thu, 25 Apr 2024 09:59:32 +0200 Subject: [PATCH 22/25] chore: fix linter report --- .../e/drive/synchronization/tasks/DownloadFileOperation.kt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt b/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt index f49b3025..03d0acf1 100644 --- a/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt +++ b/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt @@ -77,7 +77,7 @@ class DownloadFileOperation(private val remoteFile: RemoteFile, return RemoteOperationResult(downloadResult) } - + private fun checkStartCondition(): ResultCode { var resultCode = ResultCode.OK @@ -116,7 +116,8 @@ class DownloadFileOperation(private val remoteFile: RemoteFile, } catch(securityException: SecurityException) { Timber.w(securityException, "Can't update last modified timestamp") } catch (illegalArgumentException: IllegalArgumentException) { - Timber.w(illegalArgumentException, "Can't update last modified timestamp to: ${remoteFile.modifiedTimestamp}") + Timber.w(illegalArgumentException, + "remoteFile's lastModifiedTimestamp : ${remoteFile.modifiedTimestamp}") } } } -- GitLab From da82b6eff7bb9563efa0068a93a8a87bb317cf17 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Thu, 25 Apr 2024 15:02:47 +0200 Subject: [PATCH 23/25] chore: fix doc style --- .../e/drive/synchronization/tasks/DownloadFileOperation.kt | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt b/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt index 03d0acf1..8b11ec7f 100644 --- a/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt +++ b/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt @@ -36,10 +36,9 @@ import java.util.stream.IntStream.range /** * @author Vincent Bourgmayer - * Encapsulates a global download process for a remote file - * - * /!\ Ignores deprecation about download with OwncloudClient - * because NC lib doesn't implement it with NextcloudClient + * Encapsulates a global download process for a remote file ⚠️ Ignores + * deprecation about download with OwncloudClient because NC lib doesn't + * implement it with NextcloudClient */ class DownloadFileOperation(private val remoteFile: RemoteFile, private val fileState: SyncedFileState, -- GitLab From 9dc0b59888d4c05d5e9b1c38783cc9544f263c23 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Thu, 25 Apr 2024 15:12:47 +0200 Subject: [PATCH 24/25] chore: apply suggestion. Rename method in DownloadFileOperation --- .../e/drive/synchronization/tasks/DownloadFileOperation.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt b/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt index 8b11ec7f..81ba508f 100644 --- a/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt +++ b/app/src/main/java/foundation/e/drive/synchronization/tasks/DownloadFileOperation.kt @@ -71,7 +71,7 @@ class DownloadFileOperation(private val remoteFile: RemoteFile, setRemoteTimestampOnFile(targetFile) makeGalleryDetectTheFile(context, targetFilePath) - persistUpdatedSyncedFileState(targetFile.lastModified()) + updateSyncedFileStateInDb(targetFile.lastModified()) } return RemoteOperationResult(downloadResult) @@ -121,7 +121,7 @@ class DownloadFileOperation(private val remoteFile: RemoteFile, } } - private fun persistUpdatedSyncedFileState(fileLastModifiedTimestamp: Long) { + private fun updateSyncedFileStateInDb(fileLastModifiedTimestamp: Long) { fileState.lastModified = fileLastModifiedTimestamp fileState.lastEtag = remoteFile.etag!! //already checked in "isFileUpToDate()" -- GitLab From fd696a507d696b31b4d897cd1445053460361a89 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Thu, 25 Apr 2024 15:27:20 +0200 Subject: [PATCH 25/25] refactor: apply suggestion. Change method in FileUtils.kt --- .../foundation/e/drive/utils/FileUtils.kt | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/utils/FileUtils.kt b/app/src/main/java/foundation/e/drive/utils/FileUtils.kt index 9b755e6a..14df3210 100644 --- a/app/src/main/java/foundation/e/drive/utils/FileUtils.kt +++ b/app/src/main/java/foundation/e/drive/utils/FileUtils.kt @@ -95,7 +95,14 @@ object FileUtils { target.toPath(), StandardCopyOption.REPLACE_EXISTING ) - success = validateCopyOperation(copyResult.toFile(), source) + val copiedFile = copyResult.toFile() + success = verifyCopyOperation(copiedFile, source) + + if (success) { + source.delete() + } else { + copiedFile.delete() + } } catch (fileSystemException: FileSystemException) { Timber.w(fileSystemException) @@ -108,14 +115,8 @@ object FileUtils { } @JvmStatic - private fun validateCopyOperation(copiedFile: File, tmpFile: File): Boolean { - return if (copiedFile.length() == tmpFile.length()) { - tmpFile.delete() - true - } else { - copiedFile.delete() - false - } + private fun verifyCopyOperation(copiedFile: File, tmpFile: File): Boolean { + return copiedFile.length() == tmpFile.length() } @JvmStatic -- GitLab