From fd55d73fa2424d001a8f7a8a553ab1b0a864f030 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Thu, 1 Sep 2022 15:54:29 +0200 Subject: [PATCH 01/21] Extract code about fileDiff beetween RemoteFile & Database or local file into a dedicated class. Clean ObserverService.handleRemoteFile part about fileDiff checking --- .../e/drive/services/ObserverService.java | 35 +++----- .../e/drive/utils/FileDiffUtils.java | 81 +++++++++++++++++++ 2 files changed, 94 insertions(+), 22 deletions(-) create mode 100644 app/src/main/java/foundation/e/drive/utils/FileDiffUtils.java diff --git a/app/src/main/java/foundation/e/drive/services/ObserverService.java b/app/src/main/java/foundation/e/drive/services/ObserverService.java index e57f135e..f2bc993e 100644 --- a/app/src/main/java/foundation/e/drive/services/ObserverService.java +++ b/app/src/main/java/foundation/e/drive/services/ObserverService.java @@ -47,11 +47,13 @@ import foundation.e.drive.receivers.DebugCmdReceiver; import foundation.e.drive.utils.AppConstants; import foundation.e.drive.utils.CommonUtils; import foundation.e.drive.utils.DavClientProvider; +import foundation.e.drive.utils.FileDiffUtils; import foundation.e.drive.utils.ServiceExceptionHandler; import foundation.e.drive.utils.SynchronizationServiceConnection; import static com.owncloud.android.lib.resources.files.FileUtils.PATH_SEPARATOR; import static foundation.e.drive.utils.AppConstants.INITIALIZATION_HAS_BEEN_DONE; +import static foundation.e.drive.utils.FileDiffUtils.getActionForFileDiff; import androidx.annotation.Nullable; @@ -380,41 +382,30 @@ public class ObserverService extends Service implements OnRemoteOperationListene continue; } - Log.v(TAG, "syncedFileState.getRemotePath() :"+syncedFileState.getRemotePath() ); - - //If syncedFileState match the remote file if (remoteFilePath.equals(syncedFileState.getRemotePath())) { Log.d(TAG, "correspondant found for "+remoteFilePath ); correspondant_found = true; - if (syncedFileState.isLastEtagStored() //there is an etag stored - && (!remoteFile.getEtag().equals(syncedFileState.getLastETAG()) //If etag has changed - || syncedFileState.getLocalLastModified() == 0L)) { //File hasn't been downloaded - - Log.v(TAG, "etag and localLastModified are valids for "+remoteFilePath ); + final FileDiffUtils.Action action = getActionForFileDiff(remoteFile, syncedFileState); + if (action == FileDiffUtils.Action.Download) { - //compare size with local file - if (remoteFile.getLength() == new File(syncedFileState.getLocalPath()).length()) { //length is 0 is file doesn't exist - Log.v(TAG, "file size are the same for local and remote, just update syncedFileState etag"); + Log.i(TAG, "Add download operation for file "+syncedFileState.getId()); + this.syncRequests.put(syncedFileState.getId(), new DownloadRequest(remoteFile, syncedFileState)); - syncedFileState.setLastETAG(remoteFile.getEtag()); - int affectedRows = DbHelper.manageSyncedFileStateDB(syncedFileState, "UPDATE", this); - Log.v(TAG, affectedRows + " syncedFileState.s row in DB has been updated."); - } else { - Log.i(TAG, "Add download operation for file "+syncedFileState.getId()); + } else if (action == FileDiffUtils.Action.updateDB) { - this.syncRequests.put(syncedFileState.getId(), new DownloadRequest(remoteFile, syncedFileState)); - } + syncedFileState.setLastETAG(remoteFile.getEtag()); + final int affectedRows = DbHelper.manageSyncedFileStateDB(syncedFileState, "UPDATE", this); + if (affectedRows == 0) Log.e(TAG, "Error while updating eTag in DB for: "+remoteFilePath); } - syncedFileListIterator.remove(); //we can delete syncedFile from list because its correspondant has already been found and handled + syncedFileListIterator.remove(); break; } } - if ( correspondant_found )continue; + if (correspondant_found) continue; - //If we get here, RemoteFile is a new file to download - Log.v(TAG, "SyncedFileState corresponding to remoteFile not found."); + Log.v(TAG, remoteFilePath + "is a new file"); //Extract parent folder's path of remote file final String parentOfKnownPath = remoteFilePath.substring(0, remoteFilePath.lastIndexOf(FileUtils.PATH_SEPARATOR) + 1); diff --git a/app/src/main/java/foundation/e/drive/utils/FileDiffUtils.java b/app/src/main/java/foundation/e/drive/utils/FileDiffUtils.java new file mode 100644 index 00000000..55510464 --- /dev/null +++ b/app/src/main/java/foundation/e/drive/utils/FileDiffUtils.java @@ -0,0 +1,81 @@ +/* + * Copyright © ECORP SAS 2022. + * 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 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, + Remove, + 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 + */ + public static Action getActionForFileDiff(RemoteFile remoteFile, 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; + } else { + return Action.Download; + } + } + + /** + * 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(RemoteFile file, 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(SyncedFileState fileState) { + return fileState.getLocalLastModified() == 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(RemoteFile remoteFile, File localFile) { + // if local file doesn't exist its size will be 0 + return remoteFile.getLength() == localFile.length(); + } +} -- GitLab From 10da3b6eda6dafc1cfe101b1935afd872de196ef Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Fri, 2 Sep 2022 10:03:44 +0200 Subject: [PATCH 02/21] clean codes --- .../java/foundation/e/drive/services/ObserverService.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/services/ObserverService.java b/app/src/main/java/foundation/e/drive/services/ObserverService.java index f2bc993e..2fa93e89 100644 --- a/app/src/main/java/foundation/e/drive/services/ObserverService.java +++ b/app/src/main/java/foundation/e/drive/services/ObserverService.java @@ -47,7 +47,7 @@ import foundation.e.drive.receivers.DebugCmdReceiver; import foundation.e.drive.utils.AppConstants; import foundation.e.drive.utils.CommonUtils; import foundation.e.drive.utils.DavClientProvider; -import foundation.e.drive.utils.FileDiffUtils; +import foundation.e.drive.utils.FileDiffUtils.Action; import foundation.e.drive.utils.ServiceExceptionHandler; import foundation.e.drive.utils.SynchronizationServiceConnection; @@ -386,13 +386,13 @@ public class ObserverService extends Service implements OnRemoteOperationListene Log.d(TAG, "correspondant found for "+remoteFilePath ); correspondant_found = true; - final FileDiffUtils.Action action = getActionForFileDiff(remoteFile, syncedFileState); - if (action == FileDiffUtils.Action.Download) { + final Action action = getActionForFileDiff(remoteFile, syncedFileState); + if (action == Action.Download) { Log.i(TAG, "Add download operation for file "+syncedFileState.getId()); this.syncRequests.put(syncedFileState.getId(), new DownloadRequest(remoteFile, syncedFileState)); - } else if (action == FileDiffUtils.Action.updateDB) { + } else if (action == Action.updateDB) { syncedFileState.setLastETAG(remoteFile.getEtag()); final int affectedRows = DbHelper.manageSyncedFileStateDB(syncedFileState, "UPDATE", this); -- GitLab From f01b25e44995e7fee2f254c3a23f025622a1c231 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Fri, 2 Sep 2022 10:23:54 +0200 Subject: [PATCH 03/21] extract code to compare local file & Database into a dedicated method of FileDiffUtils --- .../e/drive/services/ObserverService.java | 16 +++++----------- .../e/drive/utils/FileDiffUtils.java | 18 ++++++++++++++++++ 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/services/ObserverService.java b/app/src/main/java/foundation/e/drive/services/ObserverService.java index 2fa93e89..10aa50f6 100644 --- a/app/src/main/java/foundation/e/drive/services/ObserverService.java +++ b/app/src/main/java/foundation/e/drive/services/ObserverService.java @@ -47,6 +47,7 @@ import foundation.e.drive.receivers.DebugCmdReceiver; import foundation.e.drive.utils.AppConstants; import foundation.e.drive.utils.CommonUtils; import foundation.e.drive.utils.DavClientProvider; +import foundation.e.drive.utils.FileDiffUtils; import foundation.e.drive.utils.FileDiffUtils.Action; import foundation.e.drive.utils.ServiceExceptionHandler; import foundation.e.drive.utils.SynchronizationServiceConnection; @@ -651,23 +652,20 @@ public class ObserverService extends Service implements OnRemoteOperationListene private void handleLocalFiles(List localFileList, List syncedFileStates ){ Log.i(TAG, "handleLocalFiles()"); Log.d(TAG, "Loop through local file list"); - Log.v(TAG, "format: filePath, exist, lastModified) :"); //Loop through local files - for(int i =-1, localFilesSize = localFileList.size(); ++i < localFilesSize;){ + for (int i =-1, localFilesSize = localFileList.size(); ++i < localFilesSize;){ File localFile = localFileList.get(i); String filePath = CommonUtils.getLocalPath( localFile ); boolean correspondant_found = false; - Log.v(TAG, "Current file is "+filePath+", "+localFile.exists()+", "+localFile.lastModified() ); + Log.v(TAG, "Current file is "+filePath+", exist: "+localFile.exists()+", last modified: "+localFile.lastModified() ); ListIterator syncedFileListIterator = syncedFileStates.listIterator(); Log.d(TAG, "Loop through syncedFileStates "); - Log.v(TAG, "format: (Path, Id, last Modified)"); - while( syncedFileListIterator.hasNext() ) { - SyncedFileState syncedFileState = syncedFileListIterator.next(); + final SyncedFileState syncedFileState = syncedFileListIterator.next(); //Ignore hidden media file store in DB if (syncedFileState.isMediaType() && syncedFileState.getName().startsWith(".")){ @@ -681,19 +679,15 @@ public class ObserverService extends Service implements OnRemoteOperationListene if ( syncedFileState.getLocalPath().equals( filePath ) ){ correspondant_found = true; - //If no etag is stored in sfs, the file hasn't been sync up to server. then do upload - if ( syncedFileState.getLocalLastModified() < localFile.lastModified() || !syncedFileState.isLastEtagStored()){ - Log.i(TAG, "Add upload request for file "+syncedFileState.getId()); + if (FileDiffUtils.getActionForFileDiff(localFile, syncedFileState) == Action.Upload) { this.syncRequests.put(syncedFileState.getId(), new SyncRequest(syncedFileState, SyncRequest.Type.UPLOAD)); } - // No need to reloop on it. syncedFileListIterator.remove(); break; } } if ( correspondant_found ) continue; - //if no correspondance, then it is a new file Log.v(TAG, "this is a new file to sync"); //Extract parent path from knownPath diff --git a/app/src/main/java/foundation/e/drive/utils/FileDiffUtils.java b/app/src/main/java/foundation/e/drive/utils/FileDiffUtils.java index 55510464..d392a653 100644 --- a/app/src/main/java/foundation/e/drive/utils/FileDiffUtils.java +++ b/app/src/main/java/foundation/e/drive/utils/FileDiffUtils.java @@ -7,10 +7,13 @@ */ package foundation.e.drive.utils; +import android.util.Log; + import com.owncloud.android.lib.resources.files.model.RemoteFile; import java.io.File; +import foundation.e.drive.models.SyncRequest; import foundation.e.drive.models.SyncedFileState; /** @@ -47,6 +50,21 @@ public class FileDiffUtils { } } + + /** + * 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 + */ + public static Action getActionForFileDiff(File localFile, SyncedFileState fileState) { + //If no etag is stored in sfs, the file hasn't been sync up to server. then do upload + if ( fileState.getLocalLastModified() < localFile.lastModified() || !fileState.isLastEtagStored()) { + return Action.Upload; + } + return Action.skip; + } + /** * Compare RemoteFile's eTag with the one stored in Database * @param file RemoteFile -- GitLab From b021e807e995f6f94f71496f515f8102ba0587ae Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Fri, 2 Sep 2022 09:33:47 +0000 Subject: [PATCH 04/21] Apply 1 suggestion(s) to 1 file(s) --- .../main/java/foundation/e/drive/services/ObserverService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/foundation/e/drive/services/ObserverService.java b/app/src/main/java/foundation/e/drive/services/ObserverService.java index 2fa93e89..d5e41fd8 100644 --- a/app/src/main/java/foundation/e/drive/services/ObserverService.java +++ b/app/src/main/java/foundation/e/drive/services/ObserverService.java @@ -389,7 +389,7 @@ public class ObserverService extends Service implements OnRemoteOperationListene final Action action = getActionForFileDiff(remoteFile, syncedFileState); if (action == Action.Download) { - Log.i(TAG, "Add download operation for file "+syncedFileState.getId()); + Log.i(TAG, "Add download operation for file " + syncedFileState.getId()); this.syncRequests.put(syncedFileState.getId(), new DownloadRequest(remoteFile, syncedFileState)); } else if (action == Action.updateDB) { -- GitLab From 68cee7f736fc80851067185312679dacc8dbd020 Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Fri, 2 Sep 2022 09:33:56 +0000 Subject: [PATCH 05/21] Apply 1 suggestion(s) to 1 file(s) --- .../main/java/foundation/e/drive/services/ObserverService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/foundation/e/drive/services/ObserverService.java b/app/src/main/java/foundation/e/drive/services/ObserverService.java index d5e41fd8..b80f4066 100644 --- a/app/src/main/java/foundation/e/drive/services/ObserverService.java +++ b/app/src/main/java/foundation/e/drive/services/ObserverService.java @@ -396,7 +396,7 @@ public class ObserverService extends Service implements OnRemoteOperationListene syncedFileState.setLastETAG(remoteFile.getEtag()); final int affectedRows = DbHelper.manageSyncedFileStateDB(syncedFileState, "UPDATE", this); - if (affectedRows == 0) Log.e(TAG, "Error while updating eTag in DB for: "+remoteFilePath); + if (affectedRows == 0) Log.e(TAG, "Error while updating eTag in DB for: " + remoteFilePath); } syncedFileListIterator.remove(); break; -- GitLab From 81629ef3df2ef6963087d5acc6c85feb98a0c716 Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Fri, 2 Sep 2022 09:34:05 +0000 Subject: [PATCH 06/21] Apply 1 suggestion(s) to 1 file(s) --- app/src/main/java/foundation/e/drive/utils/FileDiffUtils.java | 1 + 1 file changed, 1 insertion(+) diff --git a/app/src/main/java/foundation/e/drive/utils/FileDiffUtils.java b/app/src/main/java/foundation/e/drive/utils/FileDiffUtils.java index 55510464..3b3ae9a6 100644 --- a/app/src/main/java/foundation/e/drive/utils/FileDiffUtils.java +++ b/app/src/main/java/foundation/e/drive/utils/FileDiffUtils.java @@ -5,6 +5,7 @@ * which accompanies this distribution, and is available at * http://www.gnu.org/licenses/gpl.html */ + package foundation.e.drive.utils; import com.owncloud.android.lib.resources.files.model.RemoteFile; -- GitLab From 5fbab34d462f882d733e27db1f52005274dddfea Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Fri, 2 Sep 2022 09:34:15 +0000 Subject: [PATCH 07/21] Apply 1 suggestion(s) to 1 file(s) --- app/src/main/java/foundation/e/drive/utils/FileDiffUtils.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/utils/FileDiffUtils.java b/app/src/main/java/foundation/e/drive/utils/FileDiffUtils.java index 3b3ae9a6..87b62cea 100644 --- a/app/src/main/java/foundation/e/drive/utils/FileDiffUtils.java +++ b/app/src/main/java/foundation/e/drive/utils/FileDiffUtils.java @@ -43,9 +43,9 @@ public class FileDiffUtils { final File localFile = new File(fileState.getLocalPath()); if (isRemoteSizeSameAsLocalSize(remoteFile, localFile)) { return Action.updateDB; - } else { - return Action.Download; } + + return Action.Download; } /** -- GitLab From e15865c6121a62279ae1fe75a31faac95985ffb5 Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Fri, 2 Sep 2022 09:34:28 +0000 Subject: [PATCH 08/21] Apply 1 suggestion(s) to 1 file(s) --- app/src/main/java/foundation/e/drive/utils/FileDiffUtils.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/src/main/java/foundation/e/drive/utils/FileDiffUtils.java b/app/src/main/java/foundation/e/drive/utils/FileDiffUtils.java index 87b62cea..1edaa713 100644 --- a/app/src/main/java/foundation/e/drive/utils/FileDiffUtils.java +++ b/app/src/main/java/foundation/e/drive/utils/FileDiffUtils.java @@ -40,7 +40,9 @@ public class FileDiffUtils { if (hasAlreadyBeenDownloaded(fileState) && !hasEtagChanged(remoteFile, fileState)) { return Action.skip; } + final File localFile = new File(fileState.getLocalPath()); + if (isRemoteSizeSameAsLocalSize(remoteFile, localFile)) { return Action.updateDB; } -- GitLab From a5654cd61e5bf234f97e9bab4eab05818c0d910f Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Mon, 5 Sep 2022 10:03:22 +0200 Subject: [PATCH 09/21] small refactoring of ObserverService & remove 'Remove' from enum Action --- .../e/drive/models/SyncedFileState.java | 9 +- .../e/drive/services/ObserverService.java | 103 +++++++++--------- .../e/drive/utils/FileDiffUtils.java | 7 -- 3 files changed, 62 insertions(+), 57 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/models/SyncedFileState.java b/app/src/main/java/foundation/e/drive/models/SyncedFileState.java index 68b1a71d..4b7e34f4 100644 --- a/app/src/main/java/foundation/e/drive/models/SyncedFileState.java +++ b/app/src/main/java/foundation/e/drive/models/SyncedFileState.java @@ -23,7 +23,6 @@ public class SyncedFileState implements Parcelable { public static final int DEVICE_SCANNABLE=2; public static final int ALL_SCANNABLE=3; - protected SyncedFileState(){}; //@ToRemove. Test Only. It's to allow to make a mock SyncedFileState Class in test. private int id; private String name; //name of the file private String localPath; //Path on the device file system @@ -144,6 +143,14 @@ public class SyncedFileState implements Parcelable { return (this.lastETAG != null && !this.lastETAG.isEmpty() ); } + /** + * Determine in it has already been synchronized once. + * @return true if contains data for both local (local last modified) & remote file (eTag) + */ + public boolean hasBeenSynchronizedOnce() { + return (this.isLastEtagStored() && this.getLocalLastModified() > 0L); + } + /** * Get the syncedFolder _id * @return long diff --git a/app/src/main/java/foundation/e/drive/services/ObserverService.java b/app/src/main/java/foundation/e/drive/services/ObserverService.java index 262e3920..ae7cc4cc 100644 --- a/app/src/main/java/foundation/e/drive/services/ObserverService.java +++ b/app/src/main/java/foundation/e/drive/services/ObserverService.java @@ -416,7 +416,7 @@ public class ObserverService extends Service implements OnRemoteOperationListene if ( mSyncedFolders.get(j).getRemoteFolder().equals( parentOfKnownPath ) ) { //We have found the parent folder final SyncedFolder parentFolder = mSyncedFolders.get(j); - String fileName = CommonUtils.getFileNameFromPath(remoteFilePath); //get remote file's name + final String fileName = CommonUtils.getFileNameFromPath(remoteFilePath); if (fileName != null) { int scannableValue = 0; @@ -462,38 +462,42 @@ public class ObserverService extends Service implements OnRemoteOperationListene //Loop through remaining file state for(int i = -1, size = syncedFileStates.size(); ++i < size; ){ - SyncedFileState syncedFileState = syncedFileStates.get(i); - if ( !CommonUtils.isThisSyncAllowed( mAccount, syncedFileState.isMediaType() ) ){ - Log.d(TAG, "Sync of current file: "+syncedFileState.getName()+" isn't allowed"); + + final SyncedFileState syncedFileState = syncedFileStates.get(i); + + if ( !CommonUtils.isThisSyncAllowed(mAccount, syncedFileState.isMediaType() ) ){ + Log.d(TAG, "Sync of current file: " + syncedFileState.getName() + " isn't allowed"); continue; } //Check that file has already been synced fully - if ( syncedFileState.isLastEtagStored() && syncedFileState.getLocalLastModified() > 0L) { - - //Get local file - File file = new File( syncedFileStates.get(i).getLocalPath() ); - - //Try to remove local file - boolean fileExists = file.exists(); - if ( fileExists) { - Log.d(TAG, file.getName()+" exists *1"); - //delete file - int rowAffected = getContentResolver().delete(MediaStore.Files.getContentUri("external"), - MediaStore.Files.FileColumns.DATA + "=?", - new String[]{CommonUtils.getLocalPath(file)}); - Log.d(TAG, "deleted rows by mediastore : "+rowAffected); - //sometimes (it seems to be relative to file's type) mediastore don't remove local file from storage - fileExists = !file.delete(); - } + if (!syncedFileState.hasBeenSynchronizedOnce()) { + continue; + } - //if it succeed, remove syncedFileState in DB - if (! fileExists ) { - //It means that file has been correctly deleted from device. So update DB. - if (DbHelper.manageSyncedFileStateDB(syncedFileState, "DELETE", this) <= 0) - Log.e(TAG, "SyncedFileState row hasn't been deleted from DB"); - } else - Log.w(TAG, "local file:"+file.getName()+" still exist and can't be remove"); + final File file = new File( syncedFileStates.get(i).getLocalPath() ); + + //Try to remove local file + boolean fileExists = file.exists(); + if (fileExists) { + Log.d(TAG, file.getName()+" exists *1"); + //delete file + int rowAffected = getContentResolver().delete(MediaStore.Files.getContentUri("external"), + MediaStore.Files.FileColumns.DATA + "=?", + new String[]{CommonUtils.getLocalPath(file)}); + Log.d(TAG, "deleted rows by mediastore : "+rowAffected); + //sometimes (it seems to be relative to file's type) mediastore don't remove local file from storage + fileExists = !file.delete(); + } + + //if it succeed, remove syncedFileState in DB + if (!fileExists) { + //It means that file has been correctly deleted from device. So update DB. + if (DbHelper.manageSyncedFileStateDB(syncedFileState, "DELETE", this) <= 0) { + Log.e(TAG, "SyncedFileState row hasn't been deleted from DB"); + } + } else { + Log.w(TAG, "local file:" + file.getName() + " still exist and can't be remove"); } } } @@ -546,10 +550,10 @@ public class ObserverService extends Service implements OnRemoteOperationListene if (CommonUtils.isSettingsSyncEnabled(mAccount)) generateAppListFile(); - ListIterator iterator = mSyncedFolders.listIterator() ; + final ListIterator iterator = mSyncedFolders.listIterator() ; //Loop through folders - while(iterator.hasNext() ){ - SyncedFolder syncedFolder = iterator.next(); + while(iterator.hasNext()) { + final SyncedFolder syncedFolder = iterator.next(); Log.d(TAG, "SyncedFolder :"+syncedFolder.getLibelle()+", "+syncedFolder.getLocalFolder()+", "+syncedFolder.getLastModified()+", "+syncedFolder.isScanLocal()+", "+syncedFolder.getId() ); //Check it's not a hidden file @@ -656,19 +660,20 @@ public class ObserverService extends Service implements OnRemoteOperationListene //Loop through local files for (int i =-1, localFilesSize = localFileList.size(); ++i < localFilesSize;){ - File localFile = localFileList.get(i); - String filePath = CommonUtils.getLocalPath( localFile ); + final File localFile = localFileList.get(i); + final String filePath = CommonUtils.getLocalPath( localFile ); boolean correspondant_found = false; Log.v(TAG, "Current file is "+filePath+", exist: "+localFile.exists()+", last modified: "+localFile.lastModified() ); - ListIterator syncedFileListIterator = syncedFileStates.listIterator(); + final ListIterator syncedFileListIterator = syncedFileStates.listIterator(); Log.d(TAG, "Loop through syncedFileStates "); + while( syncedFileListIterator.hasNext() ) { final SyncedFileState syncedFileState = syncedFileListIterator.next(); //Ignore hidden media file store in DB - if (syncedFileState.isMediaType() && syncedFileState.getName().startsWith(".")){ + if (syncedFileState.isMediaType() && syncedFileState.getName().startsWith(".")) { syncedFileListIterator.remove(); continue; } @@ -676,7 +681,7 @@ public class ObserverService extends Service implements OnRemoteOperationListene Log.v(TAG, syncedFileState.getLocalPath()+", "+syncedFileState.getId()+", "+syncedFileState.getLocalLastModified()); //if syncedFileState correspond to local file - if ( syncedFileState.getLocalPath().equals( filePath ) ){ + if (syncedFileState.getLocalPath().equals(filePath)) { correspondant_found = true; if (FileDiffUtils.getActionForFileDiff(localFile, syncedFileState) == Action.Upload) { @@ -686,7 +691,7 @@ public class ObserverService extends Service implements OnRemoteOperationListene break; } } - if ( correspondant_found ) continue; + if (correspondant_found) continue; Log.v(TAG, "this is a new file to sync"); @@ -703,7 +708,7 @@ public class ObserverService extends Service implements OnRemoteOperationListene } //create the syncedFile State - SyncedFileState newSyncedFileState = new SyncedFileState(-1, localFile.getName(), filePath, syncedFolder.getRemoteFolder() + localFile.getName(), "", 0, syncedFolder.getId(), syncedFolder.isMediaType(),scannableValue); + final SyncedFileState newSyncedFileState = new SyncedFileState(-1, localFile.getName(), filePath, syncedFolder.getRemoteFolder() + localFile.getName(), "", 0, syncedFolder.getId(), syncedFolder.isMediaType(),scannableValue); //Store it in DB int storedId = DbHelper.manageSyncedFileStateDB(newSyncedFileState, "INSERT", this); @@ -728,18 +733,18 @@ public class ObserverService extends Service implements OnRemoteOperationListene */ private void handleLocalRemainingSyncedFileState(List syncedFileStates){ Log.i(TAG, "handleLocalRemainingSyncedFileState(...)"); - //Loop through remaining SyncedFileState + for(SyncedFileState fileState : syncedFileStates){ - if (fileState.isLastEtagStored() && fileState.getLocalLastModified() > 0L){ - //try to get File - File file = new File(fileState.getLocalPath()); - Log.v(TAG, "File : "+file.getAbsolutePath()+","+file.exists()); - if (file.exists()){ - Log.w(TAG, "The file still exist. There is a problem!"); - } else { - Log.i(TAG, "Add remote remove request for file "+fileState.getId()); - this.syncRequests.put(fileState.getId(), new SyncRequest(fileState, SyncRequest.Type.REMOTE_DELETE)); - } + if (!fileState.hasBeenSynchronizedOnce()) { + continue; + } + final File file = new File(fileState.getLocalPath()); + + if (file.exists()){ + Log.w(TAG, file.getAbsolutePath() + "The file still exist. There is a problem!"); + } else { + Log.i(TAG, "Add remove SyncRequest for file " + file.getAbsolutePath()); + this.syncRequests.put(fileState.getId(), new SyncRequest(fileState, SyncRequest.Type.REMOTE_DELETE)); } } } diff --git a/app/src/main/java/foundation/e/drive/utils/FileDiffUtils.java b/app/src/main/java/foundation/e/drive/utils/FileDiffUtils.java index b60fc177..e416e82d 100644 --- a/app/src/main/java/foundation/e/drive/utils/FileDiffUtils.java +++ b/app/src/main/java/foundation/e/drive/utils/FileDiffUtils.java @@ -7,14 +7,9 @@ */ package foundation.e.drive.utils; - -import android.util.Log; - import com.owncloud.android.lib.resources.files.model.RemoteFile; import java.io.File; - -import foundation.e.drive.models.SyncRequest; import foundation.e.drive.models.SyncedFileState; /** @@ -27,12 +22,10 @@ public class FileDiffUtils { public enum Action { Upload, Download, - Remove, skip, updateDB } - /** * Define what to do of RemoteFile for which we know the Database equivalent * @param remoteFile RemoteFile -- GitLab From b3c15f643ed2b85078be04afdb0494c530f0ee34 Mon Sep 17 00:00:00 2001 From: Vincent Bourgmayer Date: Tue, 6 Sep 2022 14:04:05 +0000 Subject: [PATCH 10/21] Extract code about fileDiff beetween RemoteFile & Database or local file --- app/src/main/java/foundation/e/drive/utils/FileDiffUtils.java | 1 + 1 file changed, 1 insertion(+) diff --git a/app/src/main/java/foundation/e/drive/utils/FileDiffUtils.java b/app/src/main/java/foundation/e/drive/utils/FileDiffUtils.java index e416e82d..117d6b55 100644 --- a/app/src/main/java/foundation/e/drive/utils/FileDiffUtils.java +++ b/app/src/main/java/foundation/e/drive/utils/FileDiffUtils.java @@ -10,6 +10,7 @@ package foundation.e.drive.utils; import com.owncloud.android.lib.resources.files.model.RemoteFile; import java.io.File; + import foundation.e.drive.models.SyncedFileState; /** -- GitLab From 81ebc43fe9823429f1a67c55f854e06efe6381b1 Mon Sep 17 00:00:00 2001 From: Vincent Bourgmayer Date: Wed, 7 Sep 2022 07:45:11 +0000 Subject: [PATCH 11/21] Extract ObserverService code into dedicated method: Start's condition check --- .../e/drive/services/ObserverService.java | 78 ++++++++++++------- 1 file changed, 48 insertions(+), 30 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/services/ObserverService.java b/app/src/main/java/foundation/e/drive/services/ObserverService.java index ae7cc4cc..d24661c8 100644 --- a/app/src/main/java/foundation/e/drive/services/ObserverService.java +++ b/app/src/main/java/foundation/e/drive/services/ObserverService.java @@ -95,61 +95,79 @@ public class ObserverService extends Service implements OnRemoteOperationListene CommonUtils.setServiceUnCaughtExceptionHandler(this); final SharedPreferences prefs = this.getSharedPreferences(AppConstants.SHARED_PREFERENCE_NAME, Context.MODE_PRIVATE); - String accountName = prefs.getString(AccountManager.KEY_ACCOUNT_NAME, ""); - String accountType = prefs.getString(AccountManager.KEY_ACCOUNT_TYPE, ""); + final String accountName = prefs.getString(AccountManager.KEY_ACCOUNT_NAME, ""); + final String accountType = prefs.getString(AccountManager.KEY_ACCOUNT_TYPE, ""); this.mAccount = CommonUtils.getAccount(accountName, accountType, AccountManager.get(this)); - initialFolderCounter = prefs.getInt(AppConstants.INITIALFOLDERS_NUMBER, 0); - // Check if account is invalid - if (this.mAccount == null){ - Log.w(TAG, "No account registered"); + final boolean forcedSync = intent != null && DebugCmdReceiver.ACTION_FORCE_SYNC.equals(intent.getAction()); + + if (!checkStartCondition(prefs, forcedSync)) { return super.onStartCommand(intent, flags, startId); } - //check if user have disable eDrive's sync in account's settings + this.syncRequests = new HashMap<>(); + initialFolderCounter = prefs.getInt(AppConstants.INITIALFOLDERS_NUMBER, 0); + begin(); + return START_NOT_STICKY; + } + + /** + * This method check that all condition are met + * to start ObserverService: + * - a valid account as been registered + * - Synchronization of media and/or settings is enabled + * - Initialization task has been done properly + * - Service isn't already running + * - Check minimum delay since last call if not forced sync + * - Check that network is available depending of metered network allowed or not + * + * It also display log depending of the failure and send intent for initialization if this has + * not been done + * @return false if at least one condition is false + */ + private boolean checkStartCondition(final SharedPreferences prefs, final boolean forcedSync) { + // Check Account not null + if (mAccount == null) { + Log.e(TAG, "No account registered"); + return false; + } + + // Check that Media & Settings sync is enable if (!CommonUtils.isMediaSyncEnabled(mAccount) && !CommonUtils.isSettingsSyncEnabled(mAccount) ){ Log.w(TAG, "eDrive syncing has been disabled in /e/ account's settings"); - return super.onStartCommand(intent, flags, startId); + return false; } - //check if init has been done. I should check if it's really required... + // Check that Initialization has been done if (!prefs.getBoolean(INITIALIZATION_HAS_BEEN_DONE, false)) { Log.w(TAG, "Initialization hasn't been done"); Intent initializerIntent = new Intent(this, InitializerService.class); startService(initializerIntent); - return super.onStartCommand( intent, flags, startId ); + return false; } - //Check this service isn't already working - if (isWorking){ + // Check this service isn't already working + if (isWorking){ //TODO check if really used... Log.w(TAG, "ObserverService is already working"); - return super.onStartCommand(intent,flags,startId); - } - - //Check a minimum delay has been respected between two start. - long lastSyncTime = prefs.getLong(AppConstants.KEY_LAST_SYNC_TIME, 0L); - long currentTime = System.currentTimeMillis(); - boolean forceSync = false; - if (intent != null) { - forceSync = DebugCmdReceiver.ACTION_FORCE_SYNC.equals(intent.getAction()); + return false; } - //if time diff between current sync and last sync is higher or equal to delay minimum between two sync - if (!forceSync && (currentTime - lastSyncTime ) < INTERSYNC_MINIMUM_DELAY ){ + // Check minimum delay since last call & not forced sync + final long lastSyncTime = prefs.getLong(AppConstants.KEY_LAST_SYNC_TIME, 0L); + final long currentTime = System.currentTimeMillis(); + if (!forcedSync && (currentTime - lastSyncTime ) < INTERSYNC_MINIMUM_DELAY ){ Log.w(TAG, "Delay between now and last call is too short"); - return super.onStartCommand( intent, flags, startId ); + return false; } + // Check that network is available depending of metered network allowed or not final boolean meteredNetworkAllowed = CommonUtils.isMeteredNetworkAllowed(mAccount); //check for the case where intent has been launched by initializerService if (!CommonUtils.haveNetworkConnection(this, meteredNetworkAllowed)) { - Log.w(TAG, "There is no Internet connexion."); - return super.onStartCommand( intent, flags, startId ); + Log.w(TAG, "There is no allowed internet connexion."); + return false; } - this.syncRequests = new HashMap<>(); - - begin(); - return START_NOT_STICKY; + return true; } /* Common methods */ -- GitLab From d8ed4b0df947a85b7abdd9241fc8c7c1a1dd09d1 Mon Sep 17 00:00:00 2001 From: Abhishek Aggarwal Date: Mon, 5 Sep 2022 15:35:58 +0000 Subject: [PATCH 12/21] eDrive: Fix userId being null --- app/build.gradle | 14 ++++++++++++++ .../e/drive/work/AccountUserInfoWorker.java | 6 +++--- keystore/platform.keystore | Bin 0 -> 2899 bytes 3 files changed, 17 insertions(+), 3 deletions(-) create mode 100644 keystore/platform.keystore diff --git a/app/build.gradle b/app/build.gradle index bbcd73a2..1199d219 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -33,11 +33,25 @@ android { setProperty("archivesBaseName", "eDrive-$versionName") testInstrumentationRunner "androidx.test.runner.AndroidJUnitRunner" } + + signingConfigs { + debugConfig { + storeFile file("../keystore/platform.keystore") + storePassword 'android' + keyAlias 'platform' + keyPassword 'android' + } + } + buildTypes { release { minifyEnabled false proguardFiles getDefaultProguardFile('proguard-android.txt'), 'proguard-rules.pro' } + + debug { + signingConfig signingConfigs.debugConfig + } } diff --git a/app/src/main/java/foundation/e/drive/work/AccountUserInfoWorker.java b/app/src/main/java/foundation/e/drive/work/AccountUserInfoWorker.java index d027aaac..957d5eeb 100644 --- a/app/src/main/java/foundation/e/drive/work/AccountUserInfoWorker.java +++ b/app/src/main/java/foundation/e/drive/work/AccountUserInfoWorker.java @@ -61,7 +61,6 @@ public class AccountUserInfoWorker extends Worker { private final Context mContext; private Account account; - private String userId; public AccountUserInfoWorker(@NonNull Context context, @NonNull WorkerParameters workerParams) { super(context, workerParams); @@ -98,10 +97,10 @@ public class AccountUserInfoWorker extends Worker { final UserInfo userInfo = ocsResult.getResultData(); if (accountManager.getUserData(account, ACCOUNT_USER_ID_KEY) == null) { - userId = userInfo.getId(); + final String userId = userInfo.getId(); client.setUserId(userId); AccountManager.get(mContext).setUserData(account, ACCOUNT_USER_ID_KEY, userId); - Log.v(TAG, "UserId "+userId+" saved for account"); + Log.v(TAG, "UserId "+ userId +" saved for account"); } final Quota userQuota = userInfo.getQuota(); final double relativeQuota = userQuota.getRelative(); @@ -185,6 +184,7 @@ public class AccountUserInfoWorker extends Worker { private boolean fetchAliases() { final OwnCloudClient ocClient = DavClientProvider.getInstance().getClientInstance(account, mContext); + final String userId = accountManager.getUserData(account, ACCOUNT_USER_ID_KEY); final GetAliasOperation getAliasOperation = new GetAliasOperation(userId); final RemoteOperationResult> ocsResult = getAliasOperation.execute(ocClient); String aliases = ""; diff --git a/keystore/platform.keystore b/keystore/platform.keystore new file mode 100644 index 0000000000000000000000000000000000000000..574b2203d6dc78b3853cf899c3c7a1f60c773507 GIT binary patch literal 2899 zcmY+EX*d*$8pmhG7z|^Vbue-)Gp2@Y(MX|?b*5yjA^SR(>}1LkjeRFZ#mL$;k~M^g zO6XX!9SJG23?oaq?tSh(=eZx==Y5{v|Np)pe;5*{IS9ytA#t)pIFJd33A->L8<0YR z?tw|r9SjM&i6OE2{$IrE04A~89Am5F!9h6xd&LC>f+!^L0fq$L#VA6c|HH@6#leET zh)uE#j+#uRX_$XL@a-t2CB6i)0J<%}ByhjU`3a{ShJ!$NUlN>dXc?j|RB{R;kHY9? zsqlSVE7W3`GS=*yB7BfUsUwu7E$V%x{Dp`%wG4N*^~@%T%QTAoW#TqOx9z*Vi+#x5 zqw~h^!@{Sf<-_pgrmRG}4)@-hHYTre3Qi0C^-|^j2WiTlWVX0PXDy$d-FtPJ>U>jz zD?6?o*sANw=?!Y5(Ysl5-sfT)>Yt_!aozeB+A*b4b+hc6U6@MHox*!ny;F00s-_`-HQX&hYpC9yRXA160dX34Osgvf`SQZ(U}aBdKiW zn*QUlqoZM@^t`!GOGknotFbJdAi_^)idWxY0)lpy_OpNI#l_srYe}Nw&_rl-!>)(?HOsAWyD~qvC|%CPhI1^8EIv*NPN0a zUHpX$rrXYj>gh)srXBJL={Sxbk!(Ft9Z?5o0(fHg?ci50Ww&N%4j6&2OXx{ucBlUA z%`MF0pT2mZ<;g@;d?zdxrv2N%;cKXME9|HDAD2iM8&yOJDnE%OhX%lQw~*?XkTG^Z ze!5PyYG#&(C~ZS;kiPL)BJJ+iLVNv4iqCZ6r}orj60ImbCO_ly)oFaLeRlK+41>>- zJz-v|Gx?*bOyK#l!o9hWT0QdM3lE(C%i4rT4&_2M0jP7|rUHzT2mQS=eO&OXH58m; z%bUNP5V;HI;}G>XbrOpe)xoOHZ-Ll@9rH3dgiAtj=L0$#bcHWOx(}Vxy_NK1OU60w z{ei4G?QZQFXJEbQ*~(z-Xyg4g-=!Y!g4WyGphs>v%>|};QAqC(VP=eFcJcZZ5iP;5 z)Wt)F-x8r`4`y24l{37&UXzTjxTQM{cmeW={xlxjGSec{$nzR(a8t=5PG8mK=$uTE zSYyf7>r~-x34QX{n4_z5r(JNqdZWcwLnYJLfQMt#ot7L|?$3g@&b2SD^$F!J+=)N1 zV}y!!M*&;*6|+NE?{=M{mve`C1ggFZ2uRiXbMnFR`(_sN@ZyE0kLoQ9!4D5<7O+Vf z+aD*z_3|oGIQKnE`RC@b+?G3fq}vO+M+Ni2=Zh;G6HF8ju62;UoUO_fkSncLsz7Hn zHb;kTyp10*i>giZiM4&Y&8d((7TEf*Q3m&uFFep`NC_=QbFEA6{G#yf*Du~Y9~D-4 zTfO?(aWYpqT+QNTXd8#wiu&ugoLXNP$8K9uzrm*~k^S5IAF!>P8C16Ex3sE>Up|LS z>bJJ4P9-RhrBRYo29x~47_PAXjV&4>W@CZ%I&5maChhJA4npe)O`bdz`&OY#s9A}0 z>MgG;r7s6HK@>XBTuF|Ii_|`Jn2(>uK@L$AW!r9o1Jz>?{!l5q;ZABt8pP65R=lEq zL?Hb%SD)sA%VkBK8$7KBPA>T)5NTCz)V9sTo7RZ=6xq9Iq1S%3uD|;KVjo=TOUZlm z_$TFiS=hk+&oaeo7PFnwXBNA}B-yG`+QQ4<-I-zgysIZ*=)1S$^t36~MT=|TV`0Gd z*RD?Nd(Wlm3?gGpb@uxick7i$+3)zT-QpOkm%C+mW22>pJ293F`5>m4oQ^j=#fMY8 zmiWZ~U4}%{%WZOHq3xZlJ4kOc$SS zff=)>Q+!(#{y`GK=wj;5Pap$frfuh2LwFx%)f2~{R32gd6?gsd)biJ6g>OV==7i;+ z+w)SrpEnix=GEzT<9DJj-Lv{tfRy{xILA=MN*}rEXR(Ho7&4ZOxb<4J*-@q4+NPijP_wncaoc@P3G-ka`wbpg!|tiuQJYHSN`tzWAv zo9>c5ye=KD`>VzO(5)(k_}4?BmFiRkr)SA$0n+tr z98Ce)GnXep>vpuVwVL|@;Tal&Z^v(~Qt?z38{l@I&dE)y-9|r=^&k$H1C)5_$zV|* zTRfAc;_Ni=Ai{;oJcA4u+1@|WoQ9-c_fHIkY&dywgdPg8S|tl*2`JW~c;qnGizOCi zEC@l8f53!_YJ-`VR;T)7VoEWHU2|UQVJN(b923=7RjuteD1uxP=hVox z-hBCh!2Slsn%3V6^DZcrDJOXE#}}#IJ|~nCRE@nIE2g%zz6$fZ=K1223J2y!82_nK z{m@MGmYPlnsw8!At3@r}QBp{CpKi_}BAZ_GJYz5RooB9 zYgvBR9p3c9|Lb2fpl;S6SvxNzGfSFmhS9+wFdPt86gLYSDJfykpXWpJ);TuJ7 nR@Gk0o9*VNg9Vw^0in;n$EYa^uVQ Date: Mon, 5 Sep 2022 15:48:12 +0000 Subject: [PATCH 13/21] Bump to v1.1.1 --- app/build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/build.gradle b/app/build.gradle index 1199d219..1c3dfc29 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -5,7 +5,7 @@ plugins { def versionMajor = 1 def versionMinor = 1 -def versionPatch = 0 +def versionPatch = 1 -- GitLab From f7f0fe766d6cd40caa110771b66ce20ac9b3ec78 Mon Sep 17 00:00:00 2001 From: Vincent Bourgmayer Date: Tue, 6 Sep 2022 15:06:16 +0000 Subject: [PATCH 14/21] 284 o chunk upload v2 --- app/build.gradle | 4 +- .../e/drive/models/SyncWrapper.java | 10 + .../drive/operations/UploadFileOperation.java | 174 +++++++++++++----- .../services/SynchronizationService.java | 2 +- 4 files changed, 138 insertions(+), 52 deletions(-) diff --git a/app/build.gradle b/app/build.gradle index 1c3dfc29..9d45c7ba 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -4,8 +4,8 @@ plugins { def versionMajor = 1 -def versionMinor = 1 -def versionPatch = 1 +def versionMinor = 2 +def versionPatch = 0 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 f6bc9313..a1984972 100644 --- a/app/src/main/java/foundation/e/drive/models/SyncWrapper.java +++ b/app/src/main/java/foundation/e/drive/models/SyncWrapper.java @@ -9,9 +9,12 @@ package foundation.e.drive.models; import android.accounts.Account; import android.content.Context; +import android.util.Log; import com.owncloud.android.lib.common.operations.RemoteOperation; +import java.io.File; + import foundation.e.drive.operations.DownloadFileOperation; import foundation.e.drive.operations.UploadFileOperation; @@ -21,6 +24,7 @@ import foundation.e.drive.operations.UploadFileOperation; * @author Vincent Bourgmayer */ public class SyncWrapper { + private final static String TAG = SyncWrapper.class.getSimpleName(); private final SyncRequest request; private final RemoteOperation remoteOperation; private boolean isRunning; @@ -59,6 +63,12 @@ public class SyncWrapper { switch (request.getOperationType()) { case UPLOAD: final SyncedFileState sfs = request.getSyncedFileState(); + final File file = new File(sfs.getLocalPath()); + if (!file.exists()) { + operation = null; + Log.w(TAG, "createRemoteOperation: local file doesn't exist for upload"); + break; + } operation = new UploadFileOperation(sfs, account, context); break; case DOWNLOAD: diff --git a/app/src/main/java/foundation/e/drive/operations/UploadFileOperation.java b/app/src/main/java/foundation/e/drive/operations/UploadFileOperation.java index 4cfc10b4..2c6bdb09 100644 --- a/app/src/main/java/foundation/e/drive/operations/UploadFileOperation.java +++ b/app/src/main/java/foundation/e/drive/operations/UploadFileOperation.java @@ -20,12 +20,17 @@ import com.owncloud.android.lib.common.OwnCloudClient; import com.owncloud.android.lib.common.UserInfo; import com.owncloud.android.lib.common.operations.RemoteOperation; import com.owncloud.android.lib.common.operations.RemoteOperationResult; +import com.owncloud.android.lib.resources.files.ChunkedFileUploadRemoteOperation; import com.owncloud.android.lib.resources.files.CreateFolderRemoteOperation; import com.owncloud.android.lib.resources.files.FileUtils; +import com.owncloud.android.lib.resources.files.ReadFileRemoteOperation; import com.owncloud.android.lib.resources.files.UploadFileRemoteOperation; +import com.owncloud.android.lib.resources.files.model.RemoteFile; import com.owncloud.android.lib.resources.users.GetUserInfoRemoteOperation; import com.owncloud.android.lib.common.operations.RemoteOperationResult.ResultCode; import java.io.File; +import java.util.ArrayList; + import foundation.e.drive.database.DbHelper; import foundation.e.drive.models.SyncedFileState; import foundation.e.drive.utils.CommonUtils; @@ -37,12 +42,10 @@ import foundation.e.drive.utils.DavClientProvider; */ public class UploadFileOperation extends RemoteOperation { private final static String TAG = UploadFileOperation.class.getSimpleName(); - - private int restartCounter =0; - private long previousLastModified; //get to restore real value if all trials fails + private final static int FILE_SIZE_FLOOR_FOR_CHUNKED = 3072000; //3MB private final Context context; private final SyncedFileState syncedState; - private final Account account; // /!\ this is temporary because NC library doesn't use NextcloudClient for every operation yet + private final Account account; // TODO Remove as soon as nextcloud library move all Operation to NextcloudClient instead of OwncloudClient /** * Construct an upload operation with an already known syncedFileState @@ -50,7 +53,6 @@ public class UploadFileOperation extends RemoteOperation { */ public UploadFileOperation (final SyncedFileState syncedFileState, final Account account, final Context context) { this.syncedState = syncedFileState; - this.previousLastModified = syncedState.getLocalLastModified(); this.context = context; this.account = account; } @@ -69,68 +71,111 @@ public class UploadFileOperation extends RemoteOperation { @Override protected RemoteOperationResult run(OwnCloudClient client ) { final File file = new File(syncedState.getLocalPath()); + + final ResultCode conditionCheckResult = checkCondition(file, client); + if (conditionCheckResult != ResultCode.OK) { + return new RemoteOperationResult(conditionCheckResult); + } + + final RemoteOperationResult uploadResult; + if (file.length() >= FILE_SIZE_FLOOR_FOR_CHUNKED) { + Log.d(TAG, "upload " + file.getName() + " as chunked file"); + uploadResult = uploadChunkedFile(file, client); + } else { + uploadResult = uploadFile(file, client); + } + + final ResultCode resultCode; + if (uploadResult.isSuccess()) { + updateSyncedFileState(uploadResult, file.lastModified(), client); + resultCode = uploadResult.getCode(); + } else { + resultCode = onUploadFailure(uploadResult.getCode(), file.getName()); + } + + DbHelper.manageSyncedFileStateDB(syncedState, "UPDATE", context); + return new RemoteOperationResult(resultCode); + } + + + /** + * Handle upload's failure + * @param uploadResult + * @param fileName + * @return + */ + private ResultCode onUploadFailure(final ResultCode uploadResult, final String fileName) { + if (uploadResult != ResultCode.CONFLICT + && uploadResult != ResultCode.QUOTA_EXCEEDED) { + Log.e(TAG, "UploadFileRemoteOperation for : " + fileName + " failed => code: " + uploadResult); + return ResultCode.UNKNOWN_ERROR; + } + return uploadResult; + } + + /** + * Check condition required to upload the file: + * - the local file exist + * - the local file is not already up to date with the remote one + * - there is enough free storage + * - the remote directory exists + * @param file file to upload + * @param client client used to execute some request + * @return ResultCode.OK if everything is alright + */ + private ResultCode checkCondition(final File file, final OwnCloudClient client) { if (file == null || !file.exists()) { Log.w(TAG, "Can't get the file. It might have been deleted"); - return new RemoteOperationResult(ResultCode.FORBIDDEN); + return ResultCode.FORBIDDEN; } - final String targetPath = syncedState.getRemotePath(); - //If file already up-to-date & synced if (syncedState.isLastEtagStored() && syncedState.getLocalLastModified() == file.lastModified()) { Log.d(TAG, "syncedState last modified: "+ syncedState.getLocalLastModified()+" <=> file last modified: "+file.lastModified() +": So return sync_conflict"); - return new RemoteOperationResult(ResultCode.SYNC_CONFLICT); + return ResultCode.SYNC_CONFLICT; } + final NextcloudClient ncClient = DavClientProvider.getInstance().getNcClientInstance(account, context); final RemoteOperationResult checkQuotaResult = checkAvailableSpace(ncClient, file.length()); if (checkQuotaResult.getCode() != ResultCode.OK) { Log.e(TAG, "Impossible to check quota. Upload of " + syncedState.getLocalPath() + "cancelled"); - return new RemoteOperationResult(checkQuotaResult.getCode()); + return checkQuotaResult.getCode(); } + final String targetPath = syncedState.getRemotePath(); if (!createRemoteFolder(targetPath, client)) { - return new RemoteOperationResult(ResultCode.UNKNOWN_ERROR); + return ResultCode.UNKNOWN_ERROR; } + return ResultCode.OK; + } - final ResultCode resultCode; - boolean mustRestart = true; - final RemoteOperationResult uploadResult = uploadFile(file, client); - if (uploadResult.isSuccess()) { - final String etag = uploadResult.getResultData(); - if (etag != null) { - syncedState.setLastETAG(etag); - } - syncedState.setLocalLastModified(file.lastModified()); - resultCode = uploadResult.getCode(); - mustRestart = false; - } else { - if (uploadResult.getCode() == ResultCode.CONFLICT ) { - resultCode = ResultCode.CONFLICT; - Log.d(TAG, "Catched a conflict result for : "+ file.getName()); - mustRestart = false; - } else if (uploadResult.getCode() == ResultCode.QUOTA_EXCEEDED) { - resultCode = ResultCode.QUOTA_EXCEEDED; - mustRestart = false; - } else { - Log.e(TAG, "UploadFileRemoteOperation for : " + file.getName() + " failed => code: " + uploadResult.getCode()); - resultCode = ResultCode.UNKNOWN_ERROR; - mustRestart = false; + /** + * Update syncedFileState (etag & last modified) in case of successful upload + * @param uploadResult The Upload's result instance + * @param fileLastModified value of local file's last modified + * @param client The client used to check etag if missing + */ + private void updateSyncedFileState(final RemoteOperationResult uploadResult, final long fileLastModified, final OwnCloudClient client) { + //The below if statement should only be called for chunked upload. But + //for some unknown reason, the simple file upload doesn't give the etag in the result + // so, I moved the code here as a security + if (uploadResult.getResultData() == null) { + final RemoteOperationResult result = readRemoteFile(syncedState.getRemotePath(), client); + final ArrayList resultData = result.getData(); + if (result.isSuccess() && resultData != null && !resultData.isEmpty()) { + final String latestETag = ((RemoteFile) resultData.get(0)).getEtag(); + uploadResult.setResultData(latestETag); } } + final String etag = uploadResult.getResultData(); - if (mustRestart) { - if (this.restartCounter < 1) { - this.restartCounter += 1; - return this.run(client); - } else { - syncedState.setLocalLastModified(this.previousLastModified); //Revert syncFileState to its previous state - } + if (etag != null) { + syncedState.setLastETAG(etag); } - DbHelper.manageSyncedFileStateDB(syncedState, "UPDATE", context); - return new RemoteOperationResult(resultCode); + syncedState.setLocalLastModified(fileLastModified); } /** @@ -164,11 +209,41 @@ public class UploadFileOperation extends RemoteOperation { } /** - * Effectively upload the file + * Read remote file after upload to retrieve eTag + * @param remotePath file's remote path + * @param client Owncloudclient instance. @TODO will be replaced by NextcloudClient in future. + * @return RemoteOperationResult instance containing failure details or RemoteFile instance + */ + @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) + public RemoteOperationResult readRemoteFile(final String remotePath, final OwnCloudClient client) { + final ReadFileRemoteOperation readRemoteFile = new ReadFileRemoteOperation(remotePath); + return readRemoteFile.execute(client); + } + + /** + * Upload a chunked file + * Used for file bigger than 3MB + * @param file File to upload + * @param client OwncloudClient to perform the upload. @TODO will be replaced by NextcloudClient in future. + * @return RemoteOperationResult instance containing success or failure status with details + */ + @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) + public RemoteOperationResult uploadChunkedFile(final File file, final OwnCloudClient client) { + final String mimeType = CommonUtils.getMimeType(file); + final ChunkedFileUploadRemoteOperation uploadOperation = new ChunkedFileUploadRemoteOperation(syncedState.getLocalPath(), + syncedState.getRemotePath(), + mimeType, syncedState.getLastETAG(), + syncedState.getLocalLastModified()+"", false); + return uploadOperation.execute(client); + } + + + /** + * Upload a file * note: this has been extracted from run(...) for * testing purpose - * @param client client to run the method - * @return RemoteOperationResult + * @param client client to run the method. @TODO will be replaced by NextcloudClient in future. + * @return RemoteOperationResult the instance must contains etag in resultData if successful. */ @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) public RemoteOperationResult uploadFile(final File file, final OwnCloudClient client) { @@ -183,10 +258,11 @@ public class UploadFileOperation extends RemoteOperation { /** * Create remote parent folder of the file if missing - * @param targetPath - * @param client still OwnCloudClient at the moment, but will be Nextcloud client in the futur - * @return + * @param targetPath Path of remote directory to create or check for existence + * @param client Client to perform the request. @TODO will be replaced by NextcloudClient in future. + * @return true if the parent directory has been created, false either */ + @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) public boolean createRemoteFolder(String targetPath, OwnCloudClient client) { final String remoteFolderPath = targetPath.substring(0, targetPath.lastIndexOf(FileUtils.PATH_SEPARATOR) + 1); final CreateFolderRemoteOperation createFolderOperation = new CreateFolderRemoteOperation(remoteFolderPath, true); diff --git a/app/src/main/java/foundation/e/drive/services/SynchronizationService.java b/app/src/main/java/foundation/e/drive/services/SynchronizationService.java index 020429da..363e8167 100644 --- a/app/src/main/java/foundation/e/drive/services/SynchronizationService.java +++ b/app/src/main/java/foundation/e/drive/services/SynchronizationService.java @@ -54,7 +54,7 @@ public class SynchronizationService extends Service implements OnRemoteOperation private ConcurrentHashMap startedSync; //Integer is thread index (1 to workerAmount) private Account account; - private final int workerAmount = 4; + private final int workerAmount = 2; private Thread[] threadPool; @Deprecated private OwnCloudClient ocClient; -- GitLab From dab7414f55e3d65704d6faa3f15e54924a4c1cc6 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Thu, 1 Sep 2022 15:54:29 +0200 Subject: [PATCH 15/21] Extract code about fileDiff beetween RemoteFile & Database or local file into a dedicated class. Clean ObserverService.handleRemoteFile part about fileDiff checking --- .../e/drive/services/ObserverService.java | 16 +++++++++------- .../foundation/e/drive/utils/FileDiffUtils.java | 2 +- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/services/ObserverService.java b/app/src/main/java/foundation/e/drive/services/ObserverService.java index d24661c8..ff11e265 100644 --- a/app/src/main/java/foundation/e/drive/services/ObserverService.java +++ b/app/src/main/java/foundation/e/drive/services/ObserverService.java @@ -9,6 +9,10 @@ package foundation.e.drive.services; +import static com.owncloud.android.lib.resources.files.FileUtils.PATH_SEPARATOR; +import static foundation.e.drive.utils.AppConstants.INITIALIZATION_HAS_BEEN_DONE; +import static foundation.e.drive.utils.FileDiffUtils.getActionForFileDiff; + import android.accounts.Account; import android.accounts.AccountManager; import android.app.Service; @@ -20,12 +24,16 @@ import android.os.Handler; import android.os.IBinder; import android.provider.MediaStore; import android.util.Log; + +import androidx.annotation.Nullable; + import com.owncloud.android.lib.common.OwnCloudClient; import com.owncloud.android.lib.common.operations.OnRemoteOperationListener; import com.owncloud.android.lib.common.operations.RemoteOperation; import com.owncloud.android.lib.common.operations.RemoteOperationResult; import com.owncloud.android.lib.resources.files.FileUtils; import com.owncloud.android.lib.resources.files.model.RemoteFile; + import java.io.File; import java.io.FileFilter; import java.io.FileOutputStream; @@ -40,8 +48,8 @@ import foundation.e.drive.fileFilters.FileFilterFactory; import foundation.e.drive.fileFilters.OnlyFileFilter; import foundation.e.drive.models.DownloadRequest; import foundation.e.drive.models.SyncRequest; -import foundation.e.drive.models.SyncedFolder; import foundation.e.drive.models.SyncedFileState; +import foundation.e.drive.models.SyncedFolder; import foundation.e.drive.operations.ListFileRemoteOperation; import foundation.e.drive.receivers.DebugCmdReceiver; import foundation.e.drive.utils.AppConstants; @@ -52,12 +60,6 @@ import foundation.e.drive.utils.FileDiffUtils.Action; import foundation.e.drive.utils.ServiceExceptionHandler; import foundation.e.drive.utils.SynchronizationServiceConnection; -import static com.owncloud.android.lib.resources.files.FileUtils.PATH_SEPARATOR; -import static foundation.e.drive.utils.AppConstants.INITIALIZATION_HAS_BEEN_DONE; -import static foundation.e.drive.utils.FileDiffUtils.getActionForFileDiff; - -import androidx.annotation.Nullable; - /** * @author Vincent Bourgmayer * @author Nicolas Gelot diff --git a/app/src/main/java/foundation/e/drive/utils/FileDiffUtils.java b/app/src/main/java/foundation/e/drive/utils/FileDiffUtils.java index 117d6b55..c5fcd55b 100644 --- a/app/src/main/java/foundation/e/drive/utils/FileDiffUtils.java +++ b/app/src/main/java/foundation/e/drive/utils/FileDiffUtils.java @@ -80,7 +80,7 @@ public class FileDiffUtils { * @return true if localLastModified store in Database == 0 */ private static boolean hasAlreadyBeenDownloaded(SyncedFileState fileState) { - return fileState.getLocalLastModified() == 0l; + return fileState.getLocalLastModified() == 0L; } /** -- GitLab From cf0ac960e22e2fc73c45a237657a5f1efba716f3 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Wed, 7 Sep 2022 11:31:06 +0200 Subject: [PATCH 16/21] fix coding style --- .../foundation/e/drive/services/ObserverService.java | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/services/ObserverService.java b/app/src/main/java/foundation/e/drive/services/ObserverService.java index ff11e265..e6edb9be 100644 --- a/app/src/main/java/foundation/e/drive/services/ObserverService.java +++ b/app/src/main/java/foundation/e/drive/services/ObserverService.java @@ -754,18 +754,20 @@ public class ObserverService extends Service implements OnRemoteOperationListene private void handleLocalRemainingSyncedFileState(List syncedFileStates){ Log.i(TAG, "handleLocalRemainingSyncedFileState(...)"); - for(SyncedFileState fileState : syncedFileStates){ + for(SyncedFileState fileState : syncedFileStates) { if (!fileState.hasBeenSynchronizedOnce()) { continue; } + final File file = new File(fileState.getLocalPath()); - if (file.exists()){ + if (file.exists()) { Log.w(TAG, file.getAbsolutePath() + "The file still exist. There is a problem!"); - } else { - Log.i(TAG, "Add remove SyncRequest for file " + file.getAbsolutePath()); - this.syncRequests.put(fileState.getId(), new SyncRequest(fileState, SyncRequest.Type.REMOTE_DELETE)); + continue; } + + Log.i(TAG, "Add remove SyncRequest for file " + file.getAbsolutePath()); + this.syncRequests.put(fileState.getId(), new SyncRequest(fileState, SyncRequest.Type.REMOTE_DELETE)); } } /* end of methods related to device Scanning */ -- GitLab From 3818e114ec7a2d1aaba90a676406df4a86cf8180 Mon Sep 17 00:00:00 2001 From: Jonathan Klee Date: Wed, 7 Sep 2022 14:35:42 +0000 Subject: [PATCH 17/21] Apply 1 suggestion(s) to 1 file(s) --- .../main/java/foundation/e/drive/models/SyncedFileState.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/foundation/e/drive/models/SyncedFileState.java b/app/src/main/java/foundation/e/drive/models/SyncedFileState.java index 4b7e34f4..7104feb8 100644 --- a/app/src/main/java/foundation/e/drive/models/SyncedFileState.java +++ b/app/src/main/java/foundation/e/drive/models/SyncedFileState.java @@ -148,7 +148,7 @@ public class SyncedFileState implements Parcelable { * @return true if contains data for both local (local last modified) & remote file (eTag) */ public boolean hasBeenSynchronizedOnce() { - return (this.isLastEtagStored() && this.getLocalLastModified() > 0L); + return this.isLastEtagStored() && this.getLocalLastModified() > 0L; } /** -- GitLab From 63fc73557f0c5ddeafdbb7ed2812770cbce3d17b Mon Sep 17 00:00:00 2001 From: Jonathan Klee Date: Wed, 7 Sep 2022 14:35:53 +0000 Subject: [PATCH 18/21] Apply 1 suggestion(s) to 1 file(s) --- .../main/java/foundation/e/drive/services/ObserverService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/foundation/e/drive/services/ObserverService.java b/app/src/main/java/foundation/e/drive/services/ObserverService.java index e6edb9be..8621d4af 100644 --- a/app/src/main/java/foundation/e/drive/services/ObserverService.java +++ b/app/src/main/java/foundation/e/drive/services/ObserverService.java @@ -485,7 +485,7 @@ public class ObserverService extends Service implements OnRemoteOperationListene final SyncedFileState syncedFileState = syncedFileStates.get(i); - if ( !CommonUtils.isThisSyncAllowed(mAccount, syncedFileState.isMediaType() ) ){ + if (!CommonUtils.isThisSyncAllowed(mAccount, syncedFileState.isMediaType())) { Log.d(TAG, "Sync of current file: " + syncedFileState.getName() + " isn't allowed"); continue; } -- GitLab From 01e57fd7e52e8ae1fdbe1ad6b3a83ee43321b013 Mon Sep 17 00:00:00 2001 From: Jonathan Klee Date: Wed, 7 Sep 2022 14:36:08 +0000 Subject: [PATCH 19/21] Apply 1 suggestion(s) to 1 file(s) --- .../main/java/foundation/e/drive/services/ObserverService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/foundation/e/drive/services/ObserverService.java b/app/src/main/java/foundation/e/drive/services/ObserverService.java index 8621d4af..909348f8 100644 --- a/app/src/main/java/foundation/e/drive/services/ObserverService.java +++ b/app/src/main/java/foundation/e/drive/services/ObserverService.java @@ -495,7 +495,7 @@ public class ObserverService extends Service implements OnRemoteOperationListene continue; } - final File file = new File( syncedFileStates.get(i).getLocalPath() ); + final File file = new File(syncedFileStates.get(i).getLocalPath()); //Try to remove local file boolean fileExists = file.exists(); -- GitLab From 03bab19ea2807d4ba2c5a55f3d1c908f3e28c70c Mon Sep 17 00:00:00 2001 From: Jonathan Klee Date: Wed, 7 Sep 2022 14:38:21 +0000 Subject: [PATCH 20/21] Apply 1 suggestion(s) to 1 file(s) --- app/src/main/java/foundation/e/drive/utils/FileDiffUtils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/foundation/e/drive/utils/FileDiffUtils.java b/app/src/main/java/foundation/e/drive/utils/FileDiffUtils.java index c5fcd55b..2880ec2a 100644 --- a/app/src/main/java/foundation/e/drive/utils/FileDiffUtils.java +++ b/app/src/main/java/foundation/e/drive/utils/FileDiffUtils.java @@ -56,7 +56,7 @@ public class FileDiffUtils { */ public static Action getActionForFileDiff(File localFile, SyncedFileState fileState) { //If no etag is stored in sfs, the file hasn't been sync up to server. then do upload - if ( fileState.getLocalLastModified() < localFile.lastModified() || !fileState.isLastEtagStored()) { + if (fileState.getLocalLastModified() < localFile.lastModified() || !fileState.isLastEtagStored()) { return Action.Upload; } return Action.skip; -- GitLab From fdb2c84b6934ea067117ff111f025dbc41edf7fc Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Thu, 8 Sep 2022 10:01:51 +0200 Subject: [PATCH 21/21] refactor end of 'ObserverService.handleRemoteRemainingSyncedFileState()' --- .../e/drive/services/ObserverService.java | 24 +++++++------------ 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/services/ObserverService.java b/app/src/main/java/foundation/e/drive/services/ObserverService.java index e6edb9be..093a63d3 100644 --- a/app/src/main/java/foundation/e/drive/services/ObserverService.java +++ b/app/src/main/java/foundation/e/drive/services/ObserverService.java @@ -498,26 +498,20 @@ public class ObserverService extends Service implements OnRemoteOperationListene final File file = new File( syncedFileStates.get(i).getLocalPath() ); //Try to remove local file - boolean fileExists = file.exists(); - if (fileExists) { - Log.d(TAG, file.getName()+" exists *1"); - //delete file + if (file.exists()) { int rowAffected = getContentResolver().delete(MediaStore.Files.getContentUri("external"), MediaStore.Files.FileColumns.DATA + "=?", new String[]{CommonUtils.getLocalPath(file)}); - Log.d(TAG, "deleted rows by mediastore : "+rowAffected); - //sometimes (it seems to be relative to file's type) mediastore don't remove local file from storage - fileExists = !file.delete(); - } + Log.d(TAG, "deleted rows by mediastore : " + rowAffected); - //if it succeed, remove syncedFileState in DB - if (!fileExists) { - //It means that file has been correctly deleted from device. So update DB. - if (DbHelper.manageSyncedFileStateDB(syncedFileState, "DELETE", this) <= 0) { - Log.e(TAG, "SyncedFileState row hasn't been deleted from DB"); + if (!file.delete()) { //May throw SecurityException or IOException + Log.w(TAG, "local file (" + file.getName() + ") removal failed."); + continue; } - } else { - Log.w(TAG, "local file:" + file.getName() + " still exist and can't be remove"); + } + + if (DbHelper.manageSyncedFileStateDB(syncedFileState, "DELETE", this) <= 0) { + Log.e(TAG, "Failed to remove " + file.getName() + " from DB"); } } } -- GitLab