From 10a21eceeb7ad9e7f7a48d74c8994bc777732aff Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Wed, 26 Oct 2022 17:28:44 +0200 Subject: [PATCH 01/13] Update RemoteContentScanner to move local file deletion outside and add some test case for this class --- .../contentScanner/RemoteContentScanner.java | 7 +- .../e/drive/models/SyncRequest.java | 2 +- .../RemoteContentScannerTest.java | 104 ++++++++++++++++++ 3 files changed, 111 insertions(+), 2 deletions(-) create mode 100644 app/src/test/java/foundation/e/drive/contentScanner/RemoteContentScannerTest.java diff --git a/app/src/main/java/foundation/e/drive/contentScanner/RemoteContentScanner.java b/app/src/main/java/foundation/e/drive/contentScanner/RemoteContentScanner.java index 5f32c9ac..1bc7bb8a 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/RemoteContentScanner.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/RemoteContentScanner.java @@ -7,6 +7,7 @@ */ package foundation.e.drive.contentScanner; +import static foundation.e.drive.models.SyncRequest.Type.LOCAL_DELETE; import static foundation.e.drive.utils.FileDiffUtils.getActionForFileDiff; import android.accounts.Account; @@ -20,6 +21,7 @@ import java.util.List; import foundation.e.drive.database.DbHelper; import foundation.e.drive.models.DownloadRequest; +import foundation.e.drive.models.SyncRequest; import foundation.e.drive.models.SyncedFileState; import foundation.e.drive.models.SyncedFolder; import foundation.e.drive.utils.CommonUtils; @@ -85,6 +87,9 @@ public class RemoteContentScanner extends AbstractContentScanner { @Override protected void onMissingRemoteFile(SyncedFileState fileState) { + this.syncRequests.put(fileState.getId(), new SyncRequest(fileState, LOCAL_DELETE)); + + /* todo: move commented code into synchronizationService if (!CommonUtils.isThisSyncAllowed(account, fileState.isMediaType())) { Timber.d("Sync of current file: %s isn't allowed", fileState.getName()); return; @@ -110,7 +115,7 @@ public class RemoteContentScanner extends AbstractContentScanner { if (DbHelper.manageSyncedFileStateDB(fileState, "DELETE", context) <= 0) { Timber.e("Failed to remove %s from DB", file.getName()); - } + }*/ } @Override diff --git a/app/src/main/java/foundation/e/drive/models/SyncRequest.java b/app/src/main/java/foundation/e/drive/models/SyncRequest.java index d2b820c8..29bab6b9 100644 --- a/app/src/main/java/foundation/e/drive/models/SyncRequest.java +++ b/app/src/main/java/foundation/e/drive/models/SyncRequest.java @@ -10,7 +10,7 @@ package foundation.e.drive.models; import androidx.annotation.Nullable; public class SyncRequest { - public enum Type { UPLOAD, DOWNLOAD, REMOTE_DELETE}; + public enum Type { UPLOAD, DOWNLOAD, REMOTE_DELETE, LOCAL_DELETE}; private final SyncedFileState syncedFileState; diff --git a/app/src/test/java/foundation/e/drive/contentScanner/RemoteContentScannerTest.java b/app/src/test/java/foundation/e/drive/contentScanner/RemoteContentScannerTest.java new file mode 100644 index 00000000..72799565 --- /dev/null +++ b/app/src/test/java/foundation/e/drive/contentScanner/RemoteContentScannerTest.java @@ -0,0 +1,104 @@ +package foundation.e.drive.contentScanner; + +import static org.junit.Assert.assertEquals; + +import android.accounts.Account; +import android.accounts.AccountManager; +import android.content.Context; +import android.os.Build; + +import androidx.test.core.app.ApplicationProvider; + +import com.owncloud.android.lib.resources.files.model.RemoteFile; + +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.RobolectricTestRunner; +import org.robolectric.annotation.Config; +import org.robolectric.shadows.ShadowLog; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; + +import foundation.e.drive.TestUtils; +import foundation.e.drive.database.DbHelper; +import foundation.e.drive.models.SyncRequest; +import foundation.e.drive.models.SyncedFileState; +import foundation.e.drive.models.SyncedFolder; + +@RunWith(RobolectricTestRunner.class) +@Config(sdk = Build.VERSION_CODES.O, manifest = Config.NONE) +public class RemoteContentScannerTest { + + private final Context context; + private final Account account; + private RemoteContentScanner scanner_under_test; + + public RemoteContentScannerTest(){ + context = ApplicationProvider.getApplicationContext(); + ShadowLog.stream = System.out; + TestUtils.loadServerCredentials(); + TestUtils.prepareValidAccount(AccountManager.get(context)); + account = TestUtils.getValidAccount(); + + } + + @Before + public void setUp() { + prepareDB(); //Create DB + List directoryList = new ArrayList<>(); //Improve to have realistic Content + scanner_under_test = new RemoteContentScanner(context, account, directoryList); + } + + private void prepareDB() { + DbHelper.insertSyncedFolder(createSyncedFolder("small"), context); + DbHelper.insertSyncedFolder(createSyncedFolder("medium"), context); + DbHelper.insertSyncedFolder(createSyncedFolder("large"), context); + + //assertion for debug purpose + assertEquals("There isn't three folder in DB as expected", 3, DbHelper.getSyncedFolderList(context, true).size()); + } + + /** + * Create the syncedFolder instance + * @param name the name of the folder + * @return SyncedFolder instance + */ + private SyncedFolder createSyncedFolder(String name) { + return new SyncedFolder(name, + TestUtils.TEST_LOCAL_ROOT_FOLDER_PATH+name+"/", + TestUtils.TEST_REMOTE_ROOT_FOLDER_PATH+name+"/", + true, true, true, true); + } + + + @Test + public void scanEmptyCloudContent_generateLocalDeletionRequest() { + final List cloudContent = new ArrayList<>(); + Assert.assertTrue("Fake list of cloud Content is not empty", cloudContent.isEmpty()); + + final List dbContent = new ArrayList<>(); + dbContent.add(new SyncedFileState(5, "toto", "local/path/toto", "remote/path/toto", "5555", 22222, 2, true, 3)); + dbContent.add(new SyncedFileState(3, "titi", "local/path/titi", "remote/path/titi", "5556", 22322, 2, true, 3)); + dbContent.add(new SyncedFileState(2, "tata", "local/path/tata", "remote/path/tata", "5557", 22232, 2, true, 3)); + + Assert.assertFalse("List of SyncedFileState is empty", dbContent.isEmpty()); + + final HashMap scanResult = scanner_under_test.scanContent(cloudContent, dbContent); + + //If the list of SyncedFileState is not empty then all the local file must be deleted... + //In fact, the list of SyncedFileStates that is given as parameter to scanContent, should only contains content + //that has been identified has potentially changed, new or removed + //However, the issue is that...the scanner remove the file by itself, which should be out of its scope + //TODO: find how to assert the behaviour in current state ? + + Assert.assertEquals("SyncRequest doesn't match the expected result", 3, scanResult.size()); + + for (SyncRequest request : scanResult.values()) { + Assert.assertEquals("SyncRequest's type should be LOCAL_DELETE but is "+request.getOperationType(), SyncRequest.Type.LOCAL_DELETE, request.getOperationType()); + } + } +} \ No newline at end of file -- GitLab From e3ddd41081c0db086afb08bd86ec6c3174077853 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Thu, 27 Oct 2022 14:03:15 +0200 Subject: [PATCH 02/13] add new basic test case for RemoteContentScanner --- .../e/drive/utils/FileDiffUtils.java | 3 +- .../RemoteContentScannerTest.java | 164 ++++++++++++++++-- 2 files changed, 155 insertions(+), 12 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 3e9410fe..7b5c1814 100644 --- a/app/src/main/java/foundation/e/drive/utils/FileDiffUtils.java +++ b/app/src/main/java/foundation/e/drive/utils/FileDiffUtils.java @@ -90,6 +90,7 @@ public class FileDiffUtils { */ private static boolean isRemoteSizeSameAsLocalSize(RemoteFile remoteFile, File localFile) { // if local file doesn't exist its size will be 0 - return remoteFile.getLength() == localFile.length(); + // remoteFile.getSize() : getSize() is equal to getLength() except that for folder is also sum the content of the folder! + return remoteFile.getSize() == localFile.length(); } } diff --git a/app/src/test/java/foundation/e/drive/contentScanner/RemoteContentScannerTest.java b/app/src/test/java/foundation/e/drive/contentScanner/RemoteContentScannerTest.java index 72799565..8e58f603 100644 --- a/app/src/test/java/foundation/e/drive/contentScanner/RemoteContentScannerTest.java +++ b/app/src/test/java/foundation/e/drive/contentScanner/RemoteContentScannerTest.java @@ -1,3 +1,10 @@ +/* + * 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.contentScanner; import static org.junit.Assert.assertEquals; @@ -15,6 +22,7 @@ import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.Mockito; import org.robolectric.RobolectricTestRunner; import org.robolectric.annotation.Config; import org.robolectric.shadows.ShadowLog; @@ -29,13 +37,17 @@ import foundation.e.drive.models.SyncRequest; import foundation.e.drive.models.SyncedFileState; import foundation.e.drive.models.SyncedFolder; + +/** + * @author vincent Bourgmayergit status + */ @RunWith(RobolectricTestRunner.class) @Config(sdk = Build.VERSION_CODES.O, manifest = Config.NONE) public class RemoteContentScannerTest { private final Context context; private final Account account; - private RemoteContentScanner scanner_under_test; + private RemoteContentScanner scannerUnderTest; public RemoteContentScannerTest(){ context = ApplicationProvider.getApplicationContext(); @@ -48,9 +60,10 @@ public class RemoteContentScannerTest { @Before public void setUp() { - prepareDB(); //Create DB - List directoryList = new ArrayList<>(); //Improve to have realistic Content - scanner_under_test = new RemoteContentScanner(context, account, directoryList); + prepareDB(); + List directoryList = new ArrayList<>(); + directoryList.add(new SyncedFolder("Photos", "local/path/", "remote/path/", true, true, true, true)); + scannerUnderTest = new RemoteContentScanner(context, account, directoryList); } private void prepareDB() { @@ -75,6 +88,11 @@ public class RemoteContentScannerTest { } + /** + * This is a base, that check that it works as expected in a normal situation + * todo: decline this test to cover unexpected cases! + * i.e: what if dbContent is empty + */ @Test public void scanEmptyCloudContent_generateLocalDeletionRequest() { final List cloudContent = new ArrayList<>(); @@ -87,18 +105,142 @@ public class RemoteContentScannerTest { Assert.assertFalse("List of SyncedFileState is empty", dbContent.isEmpty()); - final HashMap scanResult = scanner_under_test.scanContent(cloudContent, dbContent); + final HashMap scanResult = scannerUnderTest.scanContent(cloudContent, dbContent); - //If the list of SyncedFileState is not empty then all the local file must be deleted... - //In fact, the list of SyncedFileStates that is given as parameter to scanContent, should only contains content - //that has been identified has potentially changed, new or removed - //However, the issue is that...the scanner remove the file by itself, which should be out of its scope - //TODO: find how to assert the behaviour in current state ? + /* If the list of SyncedFileState is not empty then all the local file must be deleted... + * In fact, the list of SyncedFileStates that is given as parameter to scanContent, should only contains content + * that has been identified has potentially changed, new or removed + * However, the issue is that...the scanner remove the file by itself, which should be out of its scope + * TODO: find how to assert the behaviour in current state ? */ - Assert.assertEquals("SyncRequest doesn't match the expected result", 3, scanResult.size()); + Assert.assertEquals("scanResult's size doesn't match the expected result", 3, scanResult.size()); for (SyncRequest request : scanResult.values()) { Assert.assertEquals("SyncRequest's type should be LOCAL_DELETE but is "+request.getOperationType(), SyncRequest.Type.LOCAL_DELETE, request.getOperationType()); } } + + private List generateListOfNewRemoteFile() { + final ArrayList result = new ArrayList<>(); + + final RemoteFile newFile1 = Mockito.mock(RemoteFile.class); + Mockito.when(newFile1.getRemotePath()).thenReturn("remote/path/tutu"); + Mockito.when(newFile1.getEtag()).thenReturn("33323"); + + + final RemoteFile newFile2 = Mockito.mock(RemoteFile.class); + Mockito.when(newFile2.getRemotePath()).thenReturn("remote/path/tete"); + Mockito.when(newFile2.getEtag()).thenReturn("33423"); + + + result.add(newFile1); + result.add(newFile2); + + return result; + } + + + /** + * This is a base, that check that it works as expected in a normal situation + * todo: decline this test to cover unexpected cases! + * i.e: what if dbContent is empty + */ + @Test + public void scanUpdatedRemoteContent_generateDownloadRequest() { + final List cloudContent = new ArrayList<>(); + + final RemoteFile updatedFile1 = Mockito.mock(RemoteFile.class); + Mockito.when(updatedFile1.getRemotePath()).thenReturn("remote/path/toto"); + Mockito.when(updatedFile1.getEtag()).thenReturn("33423"); + Mockito.when(updatedFile1.getSize()).thenReturn(12l); + + final RemoteFile updatedFile2 = Mockito.mock(RemoteFile.class); + Mockito.when(updatedFile2.getRemotePath()).thenReturn("remote/path/titi"); + Mockito.when(updatedFile2.getEtag()).thenReturn("34523"); + Mockito.when(updatedFile2.getSize()).thenReturn(14l); + + final RemoteFile uptodateFile1 = Mockito.mock(RemoteFile.class); + Mockito.when(uptodateFile1.getRemotePath()).thenReturn("remote/path/tata"); + Mockito.when(uptodateFile1.getEtag()).thenReturn("5557"); + + cloudContent.add(uptodateFile1); + cloudContent.add(updatedFile1); + cloudContent.add(updatedFile2); + + Assert.assertEquals("Expected cloudContent size: 3 but got : " + cloudContent.size(), 3, cloudContent.size()); + + + final List dbContent = new ArrayList<>(); + dbContent.add(new SyncedFileState(5, "toto", "local/path/toto", "remote/path/toto", "5555", 22222, 2, true, 3)); + dbContent.add(new SyncedFileState(3, "titi", "local/path/titi", "remote/path/titi", "5556", 22322, 2, true, 3)); + dbContent.add(new SyncedFileState(2, "tata", "local/path/tata", "remote/path/tata", "5557", 22232, 2, true, 3)); + + Assert.assertFalse("List of SyncedFileState is empty", dbContent.isEmpty()); + + final HashMap scanResult = scannerUnderTest.scanContent(cloudContent, dbContent); + Assert.assertEquals("scanResult's size doesn't match the expected result", 2, scanResult.size()); + + for (SyncRequest request : scanResult.values()) { + Assert.assertEquals("SyncRequest's type should be DOWNLOAD but is "+request.getOperationType(), SyncRequest.Type.DOWNLOAD, request.getOperationType()); + } + } + + /** + * This is a base, that check that it works as expected in a normal situation + * todo: decline this test to cover unexpected cases! + * i.e: what if dbContent is empty + */ + @Test + public void scanNewRemoteContent_generateDownloadRequest() { + final List cloudContent = generateListOfNewRemoteFile(); + + final RemoteFile updatedFile1 = Mockito.mock(RemoteFile.class); + Mockito.when(updatedFile1.getRemotePath()).thenReturn("remote/path/toto"); + Mockito.when(updatedFile1.getEtag()).thenReturn("5555"); + + final RemoteFile updatedFile2 = Mockito.mock(RemoteFile.class); + Mockito.when(updatedFile2.getRemotePath()).thenReturn("remote/path/titi"); + Mockito.when(updatedFile2.getEtag()).thenReturn("5556"); + + final RemoteFile uptodateFile1 = Mockito.mock(RemoteFile.class); + Mockito.when(uptodateFile1.getRemotePath()).thenReturn("remote/path/tata"); + Mockito.when(uptodateFile1.getEtag()).thenReturn("5557"); + + cloudContent.add(uptodateFile1); + cloudContent.add(updatedFile1); + cloudContent.add(updatedFile2); + + Assert.assertEquals("Expected cloudContent size: 5 but got : " + cloudContent.size(), 5, cloudContent.size()); + + + final List dbContent = new ArrayList<>(); + dbContent.add(new SyncedFileState(5, "toto", "local/path/toto", "remote/path/toto", "5555", 22222, 2, true, 3)); + dbContent.add(new SyncedFileState(3, "titi", "local/path/titi", "remote/path/titi", "5556", 22322, 2, true, 3)); + dbContent.add(new SyncedFileState(2, "tata", "local/path/tata", "remote/path/tata", "5557", 22232, 2, true, 3)); + + Assert.assertFalse("List of SyncedFileState is empty", dbContent.isEmpty()); + + final HashMap scanResult = scannerUnderTest.scanContent(cloudContent, dbContent); + Assert.assertEquals("scanResult's size doesn't match the expected result", 2, scanResult.size()); + + for (SyncRequest request : scanResult.values()) { + Assert.assertEquals("SyncRequest's type should be DOWNLOAD but is "+request.getOperationType(), SyncRequest.Type.DOWNLOAD, request.getOperationType()); + } + } + + + + + @Test + public void whenDoWeGetUnexpectedFileDeletion() { + final List cloudContent = new ArrayList<>(); + final List dbContent = new ArrayList<>(); + + final HashMap scanResult = scannerUnderTest.scanContent(cloudContent, dbContent); + Assert.assertEquals("scanResult's size doesn't match the expected result", 0, scanResult.size()); + + for (SyncRequest request : scanResult.values()) { + System.out.println(request.getOperationType() + "for " +request.getSyncedFileState().getRemotePath()); + } + } } \ No newline at end of file -- GitLab From 954762b23e880f12f06b2cdbdea0167d3d3fd50d Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Fri, 28 Oct 2022 10:34:00 +0200 Subject: [PATCH 03/13] Changes in contentScanner package & listFileRemoteOperation - Fix method name in contentScanner : onMissingRemoteFile() replaced by onMissingFile() - Removed useless comment in contentScanner classes - Extract http request in dedicated method for testing in ListFileRemoteOperation --- .../e/drive/contentScanner/AbstractContentScanner.java | 4 ++-- .../e/drive/contentScanner/LocalContentScanner.java | 8 +++----- .../e/drive/contentScanner/RemoteContentScanner.java | 5 +---- .../e/drive/operations/ListFileRemoteOperation.java | 3 +++ 4 files changed, 9 insertions(+), 11 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/contentScanner/AbstractContentScanner.java b/app/src/main/java/foundation/e/drive/contentScanner/AbstractContentScanner.java index cd841618..45fa47a6 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/AbstractContentScanner.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/AbstractContentScanner.java @@ -67,7 +67,7 @@ public abstract class AbstractContentScanner { } for(SyncedFileState remainingFileState : fileStates) { - onMissingRemoteFile(remainingFileState); + onMissingFile(remainingFileState); } return syncRequests; }; @@ -93,7 +93,7 @@ public abstract class AbstractContentScanner { * When a file doesn't exist anymore we remove it from device/cloud (depending of implementation) & from Database * @param fileState SyncedFileState for which we lack remote file */ - protected abstract void onMissingRemoteFile(SyncedFileState fileState); + protected abstract void onMissingFile(SyncedFileState fileState); /** * A new file has been found diff --git a/app/src/main/java/foundation/e/drive/contentScanner/LocalContentScanner.java b/app/src/main/java/foundation/e/drive/contentScanner/LocalContentScanner.java index bf988fe0..ead7494e 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/LocalContentScanner.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/LocalContentScanner.java @@ -33,7 +33,7 @@ public class LocalContentScanner extends AbstractContentScanner{ } @Override - protected void onMissingRemoteFile(SyncedFileState fileState) { + protected void onMissingFile(SyncedFileState fileState) { if (!fileState.hasBeenSynchronizedOnce()) { return; } @@ -61,12 +61,10 @@ public class LocalContentScanner extends AbstractContentScanner{ if (parentDir.isScanLocal()) scannableValue += 2; } - //create the syncedFile State final SyncedFileState newSyncedFileState = new SyncedFileState(-1, file.getName(), filePath, parentDir.getRemoteFolder() + file.getName(), "", 0, parentDir.getId(), parentDir.isMediaType(),scannableValue); - //Store it in DB - int storedId = DbHelper.manageSyncedFileStateDB(newSyncedFileState, "INSERT", context); - if (storedId > 0){ + final int storedId = DbHelper.manageSyncedFileStateDB(newSyncedFileState, "INSERT", context); + if (storedId > 0) { newSyncedFileState.setId( storedId ); Timber.d("Add upload SyncRequest for new file %s", filePath); syncRequests.put(storedId, new SyncRequest(newSyncedFileState, SyncRequest.Type.UPLOAD)); diff --git a/app/src/main/java/foundation/e/drive/contentScanner/RemoteContentScanner.java b/app/src/main/java/foundation/e/drive/contentScanner/RemoteContentScanner.java index 1bc7bb8a..cb5d64f8 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/RemoteContentScanner.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/RemoteContentScanner.java @@ -12,11 +12,9 @@ import static foundation.e.drive.utils.FileDiffUtils.getActionForFileDiff; import android.accounts.Account; import android.content.Context; -import android.provider.MediaStore; import com.owncloud.android.lib.resources.files.model.RemoteFile; -import java.io.File; import java.util.List; import foundation.e.drive.database.DbHelper; @@ -76,7 +74,6 @@ public class RemoteContentScanner extends AbstractContentScanner { final SyncedFileState newFileState = new SyncedFileState(-1, fileName, parentDir.getLocalFolder() + fileName, remoteFilePath, file.getEtag(), 0, parentDir.getId(), parentDir.isMediaType(), scannableValue); - //Store it in DB final int storedId = DbHelper.manageSyncedFileStateDB(newFileState, "INSERT", context); if (storedId > 0) { newFileState.setId(storedId); @@ -86,7 +83,7 @@ public class RemoteContentScanner extends AbstractContentScanner { } @Override - protected void onMissingRemoteFile(SyncedFileState fileState) { + protected void onMissingFile(SyncedFileState fileState) { this.syncRequests.put(fileState.getId(), new SyncRequest(fileState, LOCAL_DELETE)); /* todo: move commented code into synchronizationService diff --git a/app/src/main/java/foundation/e/drive/operations/ListFileRemoteOperation.java b/app/src/main/java/foundation/e/drive/operations/ListFileRemoteOperation.java index 803e373c..627f31ff 100644 --- a/app/src/main/java/foundation/e/drive/operations/ListFileRemoteOperation.java +++ b/app/src/main/java/foundation/e/drive/operations/ListFileRemoteOperation.java @@ -10,6 +10,9 @@ package foundation.e.drive.operations; import android.content.Context; + +import androidx.annotation.VisibleForTesting; + import com.owncloud.android.lib.common.OwnCloudClient; import com.owncloud.android.lib.common.operations.RemoteOperation; import com.owncloud.android.lib.common.operations.RemoteOperationResult; -- GitLab From 87f51c803e517e63d838b4e5bba1f3716e783ef2 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Fri, 25 Nov 2022 09:26:16 +0100 Subject: [PATCH 04/13] refactor RemoteContentScannerTest: rename methods, and remove comment --- .../RemoteContentScannerTest.java | 34 +++---------------- 1 file changed, 4 insertions(+), 30 deletions(-) diff --git a/app/src/test/java/foundation/e/drive/contentScanner/RemoteContentScannerTest.java b/app/src/test/java/foundation/e/drive/contentScanner/RemoteContentScannerTest.java index 8e58f603..a3f6e718 100644 --- a/app/src/test/java/foundation/e/drive/contentScanner/RemoteContentScannerTest.java +++ b/app/src/test/java/foundation/e/drive/contentScanner/RemoteContentScannerTest.java @@ -87,14 +87,8 @@ public class RemoteContentScannerTest { true, true, true, true); } - - /** - * This is a base, that check that it works as expected in a normal situation - * todo: decline this test to cover unexpected cases! - * i.e: what if dbContent is empty - */ @Test - public void scanEmptyCloudContent_generateLocalDeletionRequest() { + public void scanContent_withoutRemoteContent_returnListWithLocalDeleteRequest() { final List cloudContent = new ArrayList<>(); Assert.assertTrue("Fake list of cloud Content is not empty", cloudContent.isEmpty()); @@ -107,12 +101,6 @@ public class RemoteContentScannerTest { final HashMap scanResult = scannerUnderTest.scanContent(cloudContent, dbContent); - /* If the list of SyncedFileState is not empty then all the local file must be deleted... - * In fact, the list of SyncedFileStates that is given as parameter to scanContent, should only contains content - * that has been identified has potentially changed, new or removed - * However, the issue is that...the scanner remove the file by itself, which should be out of its scope - * TODO: find how to assert the behaviour in current state ? */ - Assert.assertEquals("scanResult's size doesn't match the expected result", 3, scanResult.size()); for (SyncRequest request : scanResult.values()) { @@ -139,14 +127,8 @@ public class RemoteContentScannerTest { return result; } - - /** - * This is a base, that check that it works as expected in a normal situation - * todo: decline this test to cover unexpected cases! - * i.e: what if dbContent is empty - */ @Test - public void scanUpdatedRemoteContent_generateDownloadRequest() { + public void scanContent_withUpdatedRemoteContent_returnListWithDownloadRequest() { final List cloudContent = new ArrayList<>(); final RemoteFile updatedFile1 = Mockito.mock(RemoteFile.class); @@ -185,13 +167,8 @@ public class RemoteContentScannerTest { } } - /** - * This is a base, that check that it works as expected in a normal situation - * todo: decline this test to cover unexpected cases! - * i.e: what if dbContent is empty - */ @Test - public void scanNewRemoteContent_generateDownloadRequest() { + public void scanContent_withNewRemoteContent_returnListWithDownloadRequest() { final List cloudContent = generateListOfNewRemoteFile(); final RemoteFile updatedFile1 = Mockito.mock(RemoteFile.class); @@ -228,11 +205,8 @@ public class RemoteContentScannerTest { } } - - - @Test - public void whenDoWeGetUnexpectedFileDeletion() { + public void scanContent_emptyLists_returnEmptyList() { final List cloudContent = new ArrayList<>(); final List dbContent = new ArrayList<>(); -- GitLab From f0e26d4cc9e480c55b0bb043760e25dfa67ce93f Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Fri, 25 Nov 2022 10:18:55 +0100 Subject: [PATCH 05/13] Remove Account field in AbstractContentScanner, subclasses and test classes for sublcasses (because useless) --- .../AbstractContentScanner.java | 11 +++----- .../contentScanner/LocalContentScanner.java | 5 ++-- .../contentScanner/RemoteContentScanner.java | 6 ++--- .../e/drive/services/ObserverService.java | 4 +-- .../LocalContentScannerTest.java | 27 +++++++++++++++++++ .../RemoteContentScannerTest.java | 14 +++------- 6 files changed, 40 insertions(+), 27 deletions(-) create mode 100644 app/src/test/java/foundation/e/drive/contentScanner/LocalContentScannerTest.java diff --git a/app/src/main/java/foundation/e/drive/contentScanner/AbstractContentScanner.java b/app/src/main/java/foundation/e/drive/contentScanner/AbstractContentScanner.java index 45fa47a6..4917c25e 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/AbstractContentScanner.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/AbstractContentScanner.java @@ -7,7 +7,6 @@ */ package foundation.e.drive.contentScanner; -import android.accounts.Account; import android.content.Context; import com.owncloud.android.lib.resources.files.FileUtils; @@ -27,18 +26,16 @@ import foundation.e.drive.models.SyncedFolder; */ public abstract class AbstractContentScanner { protected final Context context; - protected final Account account; protected final HashMap syncRequests; protected final List syncedFolders; /** * @param context Context used to access Database, etc. - * @param account Account used to checked if user has change some synchronization's settings + * @param syncedFolders List of SyncedFolders */ - protected AbstractContentScanner(Context context, Account account, List syncedFolders) { + protected AbstractContentScanner(Context context, List syncedFolders) { syncRequests = new HashMap<>(); this.context = context; - this.account = account; this.syncedFolders = syncedFolders; } @@ -66,7 +63,7 @@ public abstract class AbstractContentScanner { onNewFileFound(file); } - for(SyncedFileState remainingFileState : fileStates) { + for (SyncedFileState remainingFileState : fileStates) { onMissingFile(remainingFileState); } return syncRequests; @@ -81,7 +78,7 @@ public abstract class AbstractContentScanner { protected SyncedFolder getParentSyncedFolder(String filePath) { final String dirPath = filePath.substring(0, filePath.lastIndexOf(FileUtils.PATH_SEPARATOR) + 1); - for(SyncedFolder syncedFolder : syncedFolders) { + for (SyncedFolder syncedFolder : syncedFolders) { if (isSyncedFolderParentOfFile(syncedFolder, dirPath)) { return syncedFolder; } diff --git a/app/src/main/java/foundation/e/drive/contentScanner/LocalContentScanner.java b/app/src/main/java/foundation/e/drive/contentScanner/LocalContentScanner.java index ead7494e..63a1a62f 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/LocalContentScanner.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/LocalContentScanner.java @@ -7,7 +7,6 @@ */ package foundation.e.drive.contentScanner; -import android.accounts.Account; import android.content.Context; import java.io.File; @@ -27,8 +26,8 @@ import timber.log.Timber; */ public class LocalContentScanner extends AbstractContentScanner{ - public LocalContentScanner(Context context, Account account, List syncedFolders) { - super(context, account, syncedFolders); + public LocalContentScanner(Context context, List syncedFolders) { + super(context, syncedFolders); Timber.tag(LocalContentScanner.class.getSimpleName()); } diff --git a/app/src/main/java/foundation/e/drive/contentScanner/RemoteContentScanner.java b/app/src/main/java/foundation/e/drive/contentScanner/RemoteContentScanner.java index cb5d64f8..c5461f7d 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/RemoteContentScanner.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/RemoteContentScanner.java @@ -10,7 +10,6 @@ package foundation.e.drive.contentScanner; import static foundation.e.drive.models.SyncRequest.Type.LOCAL_DELETE; import static foundation.e.drive.utils.FileDiffUtils.getActionForFileDiff; -import android.accounts.Account; import android.content.Context; import com.owncloud.android.lib.resources.files.model.RemoteFile; @@ -34,10 +33,9 @@ public class RemoteContentScanner extends AbstractContentScanner { /** * @param context Context used to access Database, etc. - * @param account Account used to checked if user has change some synchronization's settings */ - public RemoteContentScanner(Context context, Account account, List syncedFolders) { - super(context, account, syncedFolders); + public RemoteContentScanner(Context context, List syncedFolders) { + super(context, syncedFolders); Timber.tag(RemoteContentScanner.class.getSimpleName()); } 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 0da565c1..3d201f7b 100644 --- a/app/src/main/java/foundation/e/drive/services/ObserverService.java +++ b/app/src/main/java/foundation/e/drive/services/ObserverService.java @@ -315,7 +315,7 @@ public class ObserverService extends Service implements OnRemoteOperationListene syncedFoldersId); if (!remoteFiles.isEmpty() || !syncedFileStates.isEmpty()) { - final RemoteContentScanner scanner = new RemoteContentScanner(getApplicationContext(), mAccount, mSyncedFolders); + final RemoteContentScanner scanner = new RemoteContentScanner(getApplicationContext(), mSyncedFolders); syncRequests.putAll(scanner.scanContent(remoteFiles, syncedFileStates)); } } @@ -404,7 +404,7 @@ public class ObserverService extends Service implements OnRemoteOperationListene folderIdList); if (!syncedFileStates.isEmpty() || !fileList.isEmpty() ) { - final LocalContentScanner scanner= new LocalContentScanner(getApplicationContext(), mAccount, mSyncedFolders); + final LocalContentScanner scanner= new LocalContentScanner(getApplicationContext(), mSyncedFolders); syncRequests.putAll(scanner.scanContent(fileList, syncedFileStates)); } } diff --git a/app/src/test/java/foundation/e/drive/contentScanner/LocalContentScannerTest.java b/app/src/test/java/foundation/e/drive/contentScanner/LocalContentScannerTest.java new file mode 100644 index 00000000..945d8bd2 --- /dev/null +++ b/app/src/test/java/foundation/e/drive/contentScanner/LocalContentScannerTest.java @@ -0,0 +1,27 @@ +/* + * 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.contentScanner; + +import android.os.Build; + +import org.junit.runner.RunWith; +import org.robolectric.RobolectricTestRunner; +import org.robolectric.annotation.Config; + +/** + * @author vincent Bourgmayer + */ +@RunWith(RobolectricTestRunner.class) +@Config(sdk = Build.VERSION_CODES.O, manifest = Config.NONE) +public class LocalContentScannerTest { + + + + +} diff --git a/app/src/test/java/foundation/e/drive/contentScanner/RemoteContentScannerTest.java b/app/src/test/java/foundation/e/drive/contentScanner/RemoteContentScannerTest.java index a3f6e718..e6fa9d87 100644 --- a/app/src/test/java/foundation/e/drive/contentScanner/RemoteContentScannerTest.java +++ b/app/src/test/java/foundation/e/drive/contentScanner/RemoteContentScannerTest.java @@ -9,8 +9,6 @@ package foundation.e.drive.contentScanner; import static org.junit.Assert.assertEquals; -import android.accounts.Account; -import android.accounts.AccountManager; import android.content.Context; import android.os.Build; @@ -37,25 +35,19 @@ import foundation.e.drive.models.SyncRequest; import foundation.e.drive.models.SyncedFileState; import foundation.e.drive.models.SyncedFolder; - /** - * @author vincent Bourgmayergit status + * @author vincent Bourgmayer */ @RunWith(RobolectricTestRunner.class) @Config(sdk = Build.VERSION_CODES.O, manifest = Config.NONE) public class RemoteContentScannerTest { private final Context context; - private final Account account; private RemoteContentScanner scannerUnderTest; - public RemoteContentScannerTest(){ + public RemoteContentScannerTest() { context = ApplicationProvider.getApplicationContext(); ShadowLog.stream = System.out; - TestUtils.loadServerCredentials(); - TestUtils.prepareValidAccount(AccountManager.get(context)); - account = TestUtils.getValidAccount(); - } @Before @@ -63,7 +55,7 @@ public class RemoteContentScannerTest { prepareDB(); List directoryList = new ArrayList<>(); directoryList.add(new SyncedFolder("Photos", "local/path/", "remote/path/", true, true, true, true)); - scannerUnderTest = new RemoteContentScanner(context, account, directoryList); + scannerUnderTest = new RemoteContentScanner(context, directoryList); } private void prepareDB() { -- GitLab From 8d6512ccfe2c2cf8fffa1bb83863642071d9fbdf Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Fri, 25 Nov 2022 11:57:41 +0100 Subject: [PATCH 06/13] Add new test class for testing LocalContentScanner with test cases for 'scanContent(...)' method --- .../LocalContentScannerTest.java | 118 ++++++++++++++++++ .../RemoteContentScannerTest.java | 2 +- 2 files changed, 119 insertions(+), 1 deletion(-) diff --git a/app/src/test/java/foundation/e/drive/contentScanner/LocalContentScannerTest.java b/app/src/test/java/foundation/e/drive/contentScanner/LocalContentScannerTest.java index 945d8bd2..ec4ec503 100644 --- a/app/src/test/java/foundation/e/drive/contentScanner/LocalContentScannerTest.java +++ b/app/src/test/java/foundation/e/drive/contentScanner/LocalContentScannerTest.java @@ -8,11 +8,30 @@ package foundation.e.drive.contentScanner; +import static org.mockito.Mockito.when; + +import android.content.Context; import android.os.Build; +import androidx.test.core.app.ApplicationProvider; + +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.Mockito; import org.robolectric.RobolectricTestRunner; import org.robolectric.annotation.Config; +import org.robolectric.shadows.ShadowLog; + +import java.io.File; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; + +import foundation.e.drive.models.SyncRequest; +import foundation.e.drive.models.SyncedFileState; +import foundation.e.drive.models.SyncedFolder; /** * @author vincent Bourgmayer @@ -21,7 +40,106 @@ import org.robolectric.annotation.Config; @Config(sdk = Build.VERSION_CODES.O, manifest = Config.NONE) public class LocalContentScannerTest { + private final Context context; + private LocalContentScanner scannerUnderTest; + + public LocalContentScannerTest() { + context = ApplicationProvider.getApplicationContext(); + ShadowLog.stream = System.out; + } + + @Before + public void setUp() { + final File folder = new File("local/path/"); + final List directoryList = new ArrayList<>(); + // Without the + "/" for local path of SyncedFolder, some test will fails because folder.getAbsolutePath() doesn't have "/" at the end + //but syncedFolders in database (in production) has the path separator at the end of the local path + directoryList.add(new SyncedFolder("Photos", folder.getAbsolutePath() + "/", "remote/path/", true, true, true, true)); + scannerUnderTest = new LocalContentScanner(context, directoryList); + } + + @Test + public void scanContent_withMissingLocalFiles_returnListWithRemoteDeletionRequest() { + final List localFiles = new ArrayList<>(); + + final List syncedFileStates = new ArrayList<>(); + final SyncedFileState syncedFileState1 = new SyncedFileState(1, "titi.png", "local/path/titi.png", "remote/path/titi.png", "aaaa", 123456, 0, true, 3); + final SyncedFileState syncedFileState2 = new SyncedFileState(2, "tutu.png", "local/path/tutu.png", "remote/path/tutu.png", "bbbb", 234567, 0, true, 3); + + syncedFileStates.add(syncedFileState1); + syncedFileStates.add(syncedFileState2); + + final HashMap result = scannerUnderTest.scanContent( localFiles, syncedFileStates); + Assert.assertEquals("Result is expected to be a list with 2 UploadRequest but has : " + result.size() + "elements", 2, result.size()); + Assert.assertTrue("Each SyncRequests should be upload request but some were not", result.values().stream().allMatch(request -> request.getOperationType().equals(SyncRequest.Type.REMOTE_DELETE))); + } + + @Test + public void scanContent_withLocalFilesUnsynced_returnListWithUploadRequests() { + final List localFiles = new ArrayList<>(); + final File file1 = Mockito.spy(new File("local/path/titi.png")); + when(file1.lastModified()).thenReturn(987654L); + final File file2 = Mockito.spy(new File("local/path/tutu.png")); + when(file2.lastModified()).thenReturn(654321L); + + localFiles.add(file1); + localFiles.add(file2); + + final List syncedFileStates = new ArrayList<>(); + final SyncedFileState syncedFileState1 = new SyncedFileState(1, file1.getName(), file1.getAbsolutePath(), "remote/path/titi.png", "", 0, 0, true, 3); + final SyncedFileState syncedFileState2 = new SyncedFileState(2, file2.getName(), file2.getAbsolutePath(), "remote/path/tutu.png", "", 0, 0, true, 3); + + syncedFileStates.add(syncedFileState1); + syncedFileStates.add(syncedFileState2); + + final HashMap result = scannerUnderTest.scanContent( localFiles, syncedFileStates); + Assert.assertEquals("Result is expected to be a list with 2 UploadRequest but has : " + result.size() + "elements", 2, result.size()); + Assert.assertTrue("Each SyncRequests should be upload request but some were not", result.values().stream().allMatch(request -> request.getOperationType().equals(SyncRequest.Type.UPLOAD))); + } + + + @Test + public void scanContent_withUpdatedLocalFiles_returnListWithUploadRequests() { + final List localFiles = new ArrayList<>(); + final File file1 = Mockito.spy(new File("local/path/titi.png")); + when(file1.lastModified()).thenReturn(987654L); + final File file2 = Mockito.spy(new File("local/path/tutu.png")); + when(file2.lastModified()).thenReturn(654321L); + + localFiles.add(file1); + localFiles.add(file2); + + final List syncedFileStates = new ArrayList<>(); + final SyncedFileState syncedFileState1 = new SyncedFileState(1, file1.getName(), file1.getAbsolutePath(), "remote/path/titi.png", "aaaa", 123456, 0, true, 3); + final SyncedFileState syncedFileState2 = new SyncedFileState(2, file2.getName(), file2.getAbsolutePath(), "remote/path/tutu.png", "bbbb", 234567, 0, true, 3); + + syncedFileStates.add(syncedFileState1); + syncedFileStates.add(syncedFileState2); + + final HashMap result = scannerUnderTest.scanContent( localFiles, syncedFileStates); + Assert.assertEquals("Result is expected to be a list with 2 UploadRequest but has : " + result.size() + "elements", 2, result.size()); + Assert.assertTrue("Each SyncRequests should be upload request but some were not", result.values().stream().allMatch(request -> request.getOperationType().equals(SyncRequest.Type.UPLOAD))); + } + @Test + public void scanContent_withNewLocalFiles_returnListWithUploadRequests() { + final List localFiles = new ArrayList<>(); + final File newFile1 = new File("local/path/titi.png"); + final File newFile2 = new File("local/path/tutu.png"); + localFiles.add(newFile1); + localFiles.add(newFile2); + final List syncedFileStates = new ArrayList<>(); + final HashMap result = scannerUnderTest.scanContent( localFiles, syncedFileStates); + Assert.assertEquals("Result is expected to be a list with 2 UploadRequest but has : " + result.size() + "elements", 2, result.size()); + Assert.assertTrue("Each SyncRequests should be upload request but some were not", result.values().stream().allMatch(request -> request.getOperationType().equals(SyncRequest.Type.UPLOAD))); + } + @Test + public void scanContent_emptyLists_returnEmptyList() { + final List localFiles = new ArrayList<>(); + final List syncedFileStates = new ArrayList<>(); + final HashMap result = scannerUnderTest.scanContent( localFiles, syncedFileStates); + Assert.assertTrue("Result is expected to be an empty list but contained: " + result.size(), result.isEmpty()); + } } diff --git a/app/src/test/java/foundation/e/drive/contentScanner/RemoteContentScannerTest.java b/app/src/test/java/foundation/e/drive/contentScanner/RemoteContentScannerTest.java index e6fa9d87..5f422c12 100644 --- a/app/src/test/java/foundation/e/drive/contentScanner/RemoteContentScannerTest.java +++ b/app/src/test/java/foundation/e/drive/contentScanner/RemoteContentScannerTest.java @@ -53,7 +53,7 @@ public class RemoteContentScannerTest { @Before public void setUp() { prepareDB(); - List directoryList = new ArrayList<>(); + final List directoryList = new ArrayList<>(); directoryList.add(new SyncedFolder("Photos", "local/path/", "remote/path/", true, true, true, true)); scannerUnderTest = new RemoteContentScanner(context, directoryList); } -- GitLab From 2d90752b293cb2b2d3a01f2bb58b28a41bfb0e3f Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Fri, 25 Nov 2022 12:15:44 +0100 Subject: [PATCH 07/13] SynchronizationService handle local file deletion --- .../services/SynchronizationService.java | 26 ++++++++++++------- 1 file changed, 16 insertions(+), 10 deletions(-) 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 678e462e..57993a7c 100644 --- a/app/src/main/java/foundation/e/drive/services/SynchronizationService.java +++ b/app/src/main/java/foundation/e/drive/services/SynchronizationService.java @@ -197,20 +197,26 @@ public class SynchronizationService extends Service implements OnRemoteOperation if (!canStart(threadIndex)) return; final SyncRequest request = this.syncRequestQueue.poll(); //return null if empty - if (request == null) return; + if (request == null || !CommonUtils.isThisSyncAllowed(account, request.getSyncedFileState().isMediaType())) { + return; + } + + if (request.getOperationType().equals(SyncRequest.Type.LOCAL_DELETE)) { + //todo do the local file deletion + return; + } final SyncWrapper syncWrapper = new SyncWrapper(request, account, getApplicationContext()); final RemoteOperation operation = syncWrapper.getRemoteOperation(); - if (operation != null) { - if (CommonUtils.isThisSyncAllowed(account, request.getSyncedFileState().isMediaType())) { - CommonUtils.createNotificationChannel(this); - Timber.v(" starts " + request.getSyncedFileState().getName() - + " " + request.getOperationType().name() + " on thread " + threadIndex); - threadPool[threadIndex] = operation.execute(ocClient, this, handler); - startedSync.put(threadIndex, syncWrapper); - } - } + if (operation == null ) return; + + CommonUtils.createNotificationChannel(this); + Timber.v(" starts " + request.getSyncedFileState().getName() + + " " + request.getOperationType().name() + " on thread " + threadIndex); + + threadPool[threadIndex] = operation.execute(ocClient, this, handler); + startedSync.put(threadIndex, syncWrapper); } /** -- GitLab From 402efb10aa51bc5a8ac70221e4dfc8cbe55db6df Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Fri, 25 Nov 2022 14:14:48 +0100 Subject: [PATCH 08/13] Create Workrequest for localFileDeletion --- .../services/SynchronizationService.java | 19 +++- .../foundation/e/drive/utils/CommonUtils.java | 6 +- .../e/drive/work/LocalFileDeleteWorker.java | 91 +++++++++++++++++++ .../e/drive/work/WorkRequestFactory.java | 37 +++++++- 4 files changed, 146 insertions(+), 7 deletions(-) create mode 100644 app/src/main/java/foundation/e/drive/work/LocalFileDeleteWorker.java 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 57993a7c..c08ef609 100644 --- a/app/src/main/java/foundation/e/drive/services/SynchronizationService.java +++ b/app/src/main/java/foundation/e/drive/services/SynchronizationService.java @@ -21,6 +21,11 @@ import android.os.HandlerThread; import android.os.IBinder; import androidx.annotation.Nullable; +import androidx.work.ExistingWorkPolicy; +import androidx.work.OneTimeWorkRequest; +import androidx.work.OutOfQuotaPolicy; +import androidx.work.WorkManager; +import androidx.work.WorkRequest; import com.nextcloud.common.NextcloudClient; import com.owncloud.android.lib.common.OwnCloudClient; @@ -38,12 +43,15 @@ import foundation.e.drive.database.DbHelper; import foundation.e.drive.database.FailedSyncPrefsManager; import foundation.e.drive.models.SyncRequest; import foundation.e.drive.models.SyncWrapper; +import foundation.e.drive.models.SyncedFileState; import foundation.e.drive.operations.DownloadFileOperation; import foundation.e.drive.operations.RemoveFileOperation; import foundation.e.drive.operations.UploadFileOperation; import foundation.e.drive.utils.AppConstants; import foundation.e.drive.utils.CommonUtils; import foundation.e.drive.utils.DavClientProvider; +import foundation.e.drive.work.LocalFileDeleteWorker; +import foundation.e.drive.work.WorkRequestFactory; import timber.log.Timber; /** @@ -197,12 +205,19 @@ public class SynchronizationService extends Service implements OnRemoteOperation if (!canStart(threadIndex)) return; final SyncRequest request = this.syncRequestQueue.poll(); //return null if empty - if (request == null || !CommonUtils.isThisSyncAllowed(account, request.getSyncedFileState().isMediaType())) { + if (request == null + || request.getSyncedFileState() == null + || !CommonUtils.isThisSyncAllowed(account, request.getSyncedFileState().isMediaType())) { return; } + //@todo worker is not well adapted for that, look to use a runnable or something similar + // to be able to handle next syncRequest after this one if (request.getOperationType().equals(SyncRequest.Type.LOCAL_DELETE)) { - //todo do the local file deletion + final OneTimeWorkRequest workRequest = WorkRequestFactory.getOneTimeWorkRequest(WorkRequestFactory.WorkType.LOCAL_FILE_DELETION, null, request.getSyncedFileState()); + final String workerName = "delete "+request.getSyncedFileState().getLocalPath(); + WorkManager.getInstance(getApplicationContext()).enqueueUniqueWork(workerName, ExistingWorkPolicy.KEEP, workRequest); + return; } diff --git a/app/src/main/java/foundation/e/drive/utils/CommonUtils.java b/app/src/main/java/foundation/e/drive/utils/CommonUtils.java index 3f488756..c490b759 100644 --- a/app/src/main/java/foundation/e/drive/utils/CommonUtils.java +++ b/app/src/main/java/foundation/e/drive/utils/CommonUtils.java @@ -307,15 +307,15 @@ public abstract class CommonUtils { return; } - final OneTimeWorkRequest getUserInfoRequest = WorkRequestFactory.getOneTimeWorkRequest(WorkRequestFactory.WorkType.ONE_TIME_USER_INFO, null); + final OneTimeWorkRequest getUserInfoRequest = WorkRequestFactory.getOneTimeWorkRequest(WorkRequestFactory.WorkType.ONE_TIME_USER_INFO, null, null); final List workRequests = new ArrayList<>(); for (SyncedFolder folder : syncedFolders) { - final OneTimeWorkRequest createRemoteFolderWorkRequest = WorkRequestFactory.getOneTimeWorkRequest(WorkRequestFactory.WorkType.CREATE_REMOTE_DIR, folder); + final OneTimeWorkRequest createRemoteFolderWorkRequest = WorkRequestFactory.getOneTimeWorkRequest(WorkRequestFactory.WorkType.CREATE_REMOTE_DIR, folder, null); workRequests.add(createRemoteFolderWorkRequest); } - final OneTimeWorkRequest firstStartRequest = WorkRequestFactory.getOneTimeWorkRequest(WorkRequestFactory.WorkType.FIRST_START, null); + final OneTimeWorkRequest firstStartRequest = WorkRequestFactory.getOneTimeWorkRequest(WorkRequestFactory.WorkType.FIRST_START, null, null); workManager.beginWith(getUserInfoRequest) .then(workRequests) diff --git a/app/src/main/java/foundation/e/drive/work/LocalFileDeleteWorker.java b/app/src/main/java/foundation/e/drive/work/LocalFileDeleteWorker.java new file mode 100644 index 00000000..d66acb14 --- /dev/null +++ b/app/src/main/java/foundation/e/drive/work/LocalFileDeleteWorker.java @@ -0,0 +1,91 @@ +package foundation.e.drive.work; + +import android.content.Context; +import android.provider.MediaStore; + +import androidx.annotation.NonNull; +import androidx.work.Data; +import androidx.work.Worker; +import androidx.work.WorkerParameters; + +import java.io.File; + +import foundation.e.drive.database.DbHelper; +import foundation.e.drive.models.SyncedFileState; +import foundation.e.drive.utils.CommonUtils; +import timber.log.Timber; + +/** + * This worker is called when a remote file has been detected as removed and that all the + * scanning process lead to choose to delete the local version too. + */ +public class LocalFileDeleteWorker extends Worker { + public static final String DATA_KEY_ID="id"; + public static final String DATA_KEY_NAME = "name"; + public static final String DATA_KEY_LOCAL_PATH="localPath"; + public static final String DATA_KEY_REMOTE_PATH="remotePath"; + public static final String DATA_KEY_LAST_ETAG="etag"; + public static final String DATA_KEY_LAST_MODIFIED="lModified"; + public static final String DATA_KEY_MEDIATYPE="mediaType"; + public static final String DATA_KEY_FOLDER_ID="folderID"; + public static final String DATA_KEY_SCANNABLE="scannable"; + + + public LocalFileDeleteWorker(@NonNull Context context, @NonNull WorkerParameters workerParams) { + super(context, workerParams); + } + + + @NonNull + @Override + public Result doWork() { + final SyncedFileState fileState = loadSyncedFileStateFromData(); + + + //If the file has never been synchronized, then it should not exist as local + if (!fileState.hasBeenSynchronizedOnce()) { + return Result.success(); + } + + final File file = new File(fileState.getLocalPath()); + + if (!file.exists()) { + return Result.success(); + } + + try { + if (!file.delete()) { //May throw SecurityException or IOException + Timber.w("local file ( %s ) removal failed", file.getName()); + return Result.failure(); + } + } catch (SecurityException exception) { + Timber.e(exception); + return Result.failure(); + } + + getApplicationContext().getContentResolver().delete(MediaStore.Files.getContentUri("external"), + MediaStore.Files.FileColumns.DATA + "=?", + new String[]{CommonUtils.getLocalPath(file)}); + + if (DbHelper.manageSyncedFileStateDB(fileState, "DELETE", getApplicationContext()) <= 0) { + Timber.e("Failed to remove %s from DB", file.getName()); + } + return Result.success(); + } + + private SyncedFileState loadSyncedFileStateFromData() { + final Data data = getInputData(); + final SyncedFileState result = new SyncedFileState( + data.getInt(DATA_KEY_ID, -1), + data.getString(DATA_KEY_NAME), + data.getString(DATA_KEY_LOCAL_PATH), + data.getString(DATA_KEY_REMOTE_PATH), + data.getString(DATA_KEY_LAST_ETAG), + data.getLong(DATA_KEY_LAST_MODIFIED, 0L), + data.getLong(DATA_KEY_FOLDER_ID, 0), + data.getBoolean(DATA_KEY_MEDIATYPE, true), + data.getInt(DATA_KEY_SCANNABLE, 3)); + + return result; + } +} diff --git a/app/src/main/java/foundation/e/drive/work/WorkRequestFactory.java b/app/src/main/java/foundation/e/drive/work/WorkRequestFactory.java index e3daf516..c2ccde15 100644 --- a/app/src/main/java/foundation/e/drive/work/WorkRequestFactory.java +++ b/app/src/main/java/foundation/e/drive/work/WorkRequestFactory.java @@ -17,11 +17,13 @@ import androidx.work.Constraints; import androidx.work.Data; import androidx.work.NetworkType; import androidx.work.OneTimeWorkRequest; +import androidx.work.OutOfQuotaPolicy; import androidx.work.PeriodicWorkRequest; import java.security.InvalidParameterException; import java.util.concurrent.TimeUnit; +import foundation.e.drive.models.SyncedFileState; import foundation.e.drive.models.SyncedFolder; import foundation.e.drive.utils.AppConstants; @@ -31,7 +33,8 @@ public class WorkRequestFactory { PERIODIC_USER_INFO, ONE_TIME_USER_INFO, CREATE_REMOTE_DIR, - FIRST_START + FIRST_START, + LOCAL_FILE_DELETION } /** @@ -56,9 +59,10 @@ public class WorkRequestFactory { * @param type Should be ONE_TIME_USER_INFO, or FIRST_START, or CREATE_REMOTE_DIR * or it will throw InvalidParameterException * @param syncedFolder this parameter is required for CREATE_REMOTE_DIR work type. If null it will throw an NPE. + * @param fileState This parameter is required for LOCAL_FILE_DELETION work type. If null it will throw NPE * @return OneTimeWorkRequest's instance. */ - public static OneTimeWorkRequest getOneTimeWorkRequest(WorkType type, @Nullable SyncedFolder syncedFolder) { + public static OneTimeWorkRequest getOneTimeWorkRequest(WorkType type, @Nullable SyncedFolder syncedFolder, @Nullable SyncedFileState fileState) { switch (type) { case ONE_TIME_USER_INFO: return createOneTimeGetUserInfoWorkRequest(); @@ -67,11 +71,25 @@ public class WorkRequestFactory { case CREATE_REMOTE_DIR: if (syncedFolder == null) throw new NullPointerException("Synced folder is null"); return createOneTimeCreateRemoteFolderWorkRequest(syncedFolder); + case LOCAL_FILE_DELETION: + if (fileState == null) throw new NullPointerException("SyncedFileState is null"); + return createOneTimeLocalFileDeleteWorker(fileState); default: throw new InvalidParameterException("Unsupported Work Type: " + type); } } + private static OneTimeWorkRequest createOneTimeLocalFileDeleteWorker(SyncedFileState fileState) { + final OneTimeWorkRequest request = new OneTimeWorkRequest.Builder(LocalFileDeleteWorker.class) + .addTag(AppConstants.WORK_GENERIC_TAG) + .addTag("LOCAL_FILE_DELETION") + .setInputData(createDataFromSyncedFileState(fileState)) + .setExpedited(OutOfQuotaPolicy.RUN_AS_NON_EXPEDITED_WORK_REQUEST) + .build(); + + return request; + } + /** * Create a PeridocWorkRequest instance for @@ -193,4 +211,19 @@ public class WorkRequestFactory { .putBoolean(DATA_KEY_MEDIATYPE, folder.isMediaType()) .build(); } + + + private static Data createDataFromSyncedFileState(SyncedFileState file) { + return new Data.Builder() + .putInt(LocalFileDeleteWorker.DATA_KEY_ID, file.getId()) + .putString(LocalFileDeleteWorker.DATA_KEY_NAME, file.getName()) + .putString(LocalFileDeleteWorker.DATA_KEY_LOCAL_PATH, file.getLocalPath()) + .putString(LocalFileDeleteWorker.DATA_KEY_REMOTE_PATH, file.getRemotePath()) + .putString(LocalFileDeleteWorker.DATA_KEY_LAST_ETAG, file.getLastETAG()) + .putLong(LocalFileDeleteWorker.DATA_KEY_LAST_MODIFIED, file.getLocalLastModified()) + .putLong(LocalFileDeleteWorker.DATA_KEY_FOLDER_ID, file.getSyncedFolderId()) + .putBoolean(LocalFileDeleteWorker.DATA_KEY_MEDIATYPE, file.isMediaType()) + .putInt(LocalFileDeleteWorker.DATA_KEY_SCANNABLE, file.getScannable()) + .build(); + } } -- GitLab From 79f5dd2df603327dd731ce31c3b8d88d0300c926 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Mon, 28 Nov 2022 11:45:51 +0100 Subject: [PATCH 09/13] Transform LocalFileDeleteWorker into LocalFileDeleter which use Runnable - create a new LocalFileDeletionListener's interface which is implemented by SynchronizationService - Update SynchronizationService to be able to handle LocalFileDeletion - Remove code relative to LocalFileDeleteWorker from WorkerRequestFactory - SynchronizationService: add annotation & comment to remove some warnings --- .../services/SynchronizationService.java | 52 ++++++----- .../e/drive/work/LocalFileDeleteWorker.java | 91 ------------------- .../e/drive/work/LocalFileDeleter.java | 72 +++++++++++++++ .../e/drive/work/WorkRequestFactory.java | 33 +------ 4 files changed, 101 insertions(+), 147 deletions(-) delete mode 100644 app/src/main/java/foundation/e/drive/work/LocalFileDeleteWorker.java create mode 100644 app/src/main/java/foundation/e/drive/work/LocalFileDeleter.java 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 c08ef609..8ea93da9 100644 --- a/app/src/main/java/foundation/e/drive/services/SynchronizationService.java +++ b/app/src/main/java/foundation/e/drive/services/SynchronizationService.java @@ -21,12 +21,6 @@ import android.os.HandlerThread; import android.os.IBinder; import androidx.annotation.Nullable; -import androidx.work.ExistingWorkPolicy; -import androidx.work.OneTimeWorkRequest; -import androidx.work.OutOfQuotaPolicy; -import androidx.work.WorkManager; -import androidx.work.WorkRequest; - import com.nextcloud.common.NextcloudClient; import com.owncloud.android.lib.common.OwnCloudClient; import com.owncloud.android.lib.common.operations.OnRemoteOperationListener; @@ -43,21 +37,19 @@ import foundation.e.drive.database.DbHelper; import foundation.e.drive.database.FailedSyncPrefsManager; import foundation.e.drive.models.SyncRequest; import foundation.e.drive.models.SyncWrapper; -import foundation.e.drive.models.SyncedFileState; import foundation.e.drive.operations.DownloadFileOperation; import foundation.e.drive.operations.RemoveFileOperation; import foundation.e.drive.operations.UploadFileOperation; import foundation.e.drive.utils.AppConstants; import foundation.e.drive.utils.CommonUtils; import foundation.e.drive.utils.DavClientProvider; -import foundation.e.drive.work.LocalFileDeleteWorker; -import foundation.e.drive.work.WorkRequestFactory; +import foundation.e.drive.work.LocalFileDeleter; import timber.log.Timber; /** * @author Vincent Bourgmayer */ -public class SynchronizationService extends Service implements OnRemoteOperationListener { +public class SynchronizationService extends Service implements OnRemoteOperationListener, LocalFileDeleter.LocalFileDeletionListener { private final SynchronizationBinder binder = new SynchronizationBinder(); private ConcurrentLinkedDeque syncRequestQueue; private ConcurrentHashMap startedSync; //Integer is thread index (1 to workerAmount) @@ -65,6 +57,7 @@ public class SynchronizationService extends Service implements OnRemoteOperation private Account account; private final int workerAmount = 2; private Thread[] threadPool; + @SuppressWarnings("DeprecatedIsStillUsed") @Deprecated private OwnCloudClient ocClient; private NextcloudClient ncClient; @@ -94,6 +87,7 @@ public class SynchronizationService extends Service implements OnRemoteOperation syncRequestQueue = new ConcurrentLinkedDeque<>(); startedSync = new ConcurrentHashMap<>(); threadPool = new Thread[workerAmount]; + //noinspection deprecation ocClient = DavClientProvider.getInstance().getClientInstance(account, getApplicationContext()); ncClient = DavClientProvider.getInstance().getNcClientInstance(account, getApplicationContext()); @@ -132,6 +126,7 @@ public class SynchronizationService extends Service implements OnRemoteOperation */ public void queueSyncRequest(SyncRequest request) { for (SyncWrapper syncWrapper : startedSync.values()) { + //noinspection EqualsBetweenInconvertibleTypes if (syncWrapper.isRunning() && syncWrapper.equals(request)) { return; } @@ -153,11 +148,12 @@ public class SynchronizationService extends Service implements OnRemoteOperation public void queueSyncRequests(Collection requests) { for (SyncWrapper syncWrapper : startedSync.values()) { if (syncWrapper.isRunning()) { - requests.removeIf(syncRequest -> syncWrapper.equals(syncRequest)); + //noinspection EqualsBetweenInconvertibleTypes + requests.removeIf(syncWrapper::equals); } } - requests.removeIf(syncRequest -> skipBecauseOfPreviousFail(syncRequest)); + requests.removeIf(this::skipBecauseOfPreviousFail); syncRequestQueue.removeAll(requests); syncRequestQueue.addAll(requests); @@ -211,17 +207,17 @@ public class SynchronizationService extends Service implements OnRemoteOperation return; } - //@todo worker is not well adapted for that, look to use a runnable or something similar - // to be able to handle next syncRequest after this one - if (request.getOperationType().equals(SyncRequest.Type.LOCAL_DELETE)) { - final OneTimeWorkRequest workRequest = WorkRequestFactory.getOneTimeWorkRequest(WorkRequestFactory.WorkType.LOCAL_FILE_DELETION, null, request.getSyncedFileState()); - final String workerName = "delete "+request.getSyncedFileState().getLocalPath(); - WorkManager.getInstance(getApplicationContext()).enqueueUniqueWork(workerName, ExistingWorkPolicy.KEEP, workRequest); + final SyncWrapper syncWrapper = new SyncWrapper(request, account, getApplicationContext()); + if (request.getOperationType().equals(SyncRequest.Type.LOCAL_DELETE)) { + final LocalFileDeleter fileDeleter = new LocalFileDeleter(request.getSyncedFileState()); + threadPool[threadIndex] = new Thread(fileDeleter.getRunnable( handler, threadIndex , getApplicationContext(), this)); + threadPool[threadIndex].start(); + startedSync.put(threadIndex, syncWrapper); return; } - final SyncWrapper syncWrapper = new SyncWrapper(request, account, getApplicationContext()); + @SuppressWarnings("rawtypes") final RemoteOperation operation = syncWrapper.getRemoteOperation(); if (operation == null ) return; @@ -230,6 +226,7 @@ public class SynchronizationService extends Service implements OnRemoteOperation Timber.v(" starts " + request.getSyncedFileState().getName() + " " + request.getOperationType().name() + " on thread " + threadIndex); + //noinspection deprecation threadPool[threadIndex] = operation.execute(ocClient, this, handler); startedSync.put(threadIndex, syncWrapper); } @@ -243,11 +240,8 @@ public class SynchronizationService extends Service implements OnRemoteOperation final boolean meteredNetworkAllowed = CommonUtils.isMeteredNetworkAllowed(account); final SyncWrapper syncWrapper = startedSync.get(threadIndex); - if ((syncWrapper != null && syncWrapper.isRunning()) - || !CommonUtils.haveNetworkConnection(getApplicationContext(), meteredNetworkAllowed)) { - return false; - } - return true; + return (syncWrapper == null || !syncWrapper.isRunning()) + && CommonUtils.haveNetworkConnection(getApplicationContext(), meteredNetworkAllowed); } @Override @@ -334,6 +328,16 @@ public class SynchronizationService extends Service implements OnRemoteOperation } } + @Override + public void onDeletionComplete(int threadId, boolean succeed) { + final SyncWrapper wrapper = startedSync.get(threadId); + if (wrapper != null) { + Timber.d(wrapper.getRequest().getSyncedFileState().getLocalPath() + " deletion result: " + succeed); + wrapper.setRunning(false); + } + startWorker(threadId); + } + public class SynchronizationBinder extends Binder{ public SynchronizationService getService(){ return SynchronizationService.this; diff --git a/app/src/main/java/foundation/e/drive/work/LocalFileDeleteWorker.java b/app/src/main/java/foundation/e/drive/work/LocalFileDeleteWorker.java deleted file mode 100644 index d66acb14..00000000 --- a/app/src/main/java/foundation/e/drive/work/LocalFileDeleteWorker.java +++ /dev/null @@ -1,91 +0,0 @@ -package foundation.e.drive.work; - -import android.content.Context; -import android.provider.MediaStore; - -import androidx.annotation.NonNull; -import androidx.work.Data; -import androidx.work.Worker; -import androidx.work.WorkerParameters; - -import java.io.File; - -import foundation.e.drive.database.DbHelper; -import foundation.e.drive.models.SyncedFileState; -import foundation.e.drive.utils.CommonUtils; -import timber.log.Timber; - -/** - * This worker is called when a remote file has been detected as removed and that all the - * scanning process lead to choose to delete the local version too. - */ -public class LocalFileDeleteWorker extends Worker { - public static final String DATA_KEY_ID="id"; - public static final String DATA_KEY_NAME = "name"; - public static final String DATA_KEY_LOCAL_PATH="localPath"; - public static final String DATA_KEY_REMOTE_PATH="remotePath"; - public static final String DATA_KEY_LAST_ETAG="etag"; - public static final String DATA_KEY_LAST_MODIFIED="lModified"; - public static final String DATA_KEY_MEDIATYPE="mediaType"; - public static final String DATA_KEY_FOLDER_ID="folderID"; - public static final String DATA_KEY_SCANNABLE="scannable"; - - - public LocalFileDeleteWorker(@NonNull Context context, @NonNull WorkerParameters workerParams) { - super(context, workerParams); - } - - - @NonNull - @Override - public Result doWork() { - final SyncedFileState fileState = loadSyncedFileStateFromData(); - - - //If the file has never been synchronized, then it should not exist as local - if (!fileState.hasBeenSynchronizedOnce()) { - return Result.success(); - } - - final File file = new File(fileState.getLocalPath()); - - if (!file.exists()) { - return Result.success(); - } - - try { - if (!file.delete()) { //May throw SecurityException or IOException - Timber.w("local file ( %s ) removal failed", file.getName()); - return Result.failure(); - } - } catch (SecurityException exception) { - Timber.e(exception); - return Result.failure(); - } - - getApplicationContext().getContentResolver().delete(MediaStore.Files.getContentUri("external"), - MediaStore.Files.FileColumns.DATA + "=?", - new String[]{CommonUtils.getLocalPath(file)}); - - if (DbHelper.manageSyncedFileStateDB(fileState, "DELETE", getApplicationContext()) <= 0) { - Timber.e("Failed to remove %s from DB", file.getName()); - } - return Result.success(); - } - - private SyncedFileState loadSyncedFileStateFromData() { - final Data data = getInputData(); - final SyncedFileState result = new SyncedFileState( - data.getInt(DATA_KEY_ID, -1), - data.getString(DATA_KEY_NAME), - data.getString(DATA_KEY_LOCAL_PATH), - data.getString(DATA_KEY_REMOTE_PATH), - data.getString(DATA_KEY_LAST_ETAG), - data.getLong(DATA_KEY_LAST_MODIFIED, 0L), - data.getLong(DATA_KEY_FOLDER_ID, 0), - data.getBoolean(DATA_KEY_MEDIATYPE, true), - data.getInt(DATA_KEY_SCANNABLE, 3)); - - return result; - } -} diff --git a/app/src/main/java/foundation/e/drive/work/LocalFileDeleter.java b/app/src/main/java/foundation/e/drive/work/LocalFileDeleter.java new file mode 100644 index 00000000..afb7e91e --- /dev/null +++ b/app/src/main/java/foundation/e/drive/work/LocalFileDeleter.java @@ -0,0 +1,72 @@ +package foundation.e.drive.work; + +import android.content.Context; +import android.os.Handler; +import android.provider.MediaStore; + +import java.io.File; + +import foundation.e.drive.database.DbHelper; +import foundation.e.drive.models.SyncedFileState; +import foundation.e.drive.utils.CommonUtils; +import timber.log.Timber; + +/** + * This worker is called when a remote file has been detected as removed and that all the + * scanning process lead to choose to delete the local version too. + */ +public class LocalFileDeleter { + + public interface LocalFileDeletionListener { + void onDeletionComplete(int threadId, boolean succeed); + } + + private final SyncedFileState fileState; + + public LocalFileDeleter(SyncedFileState syncedFileState) { + fileState = syncedFileState; + } + + public Runnable getRunnable(final Handler handler, final int threadId, final Context context, final LocalFileDeletionListener listener) { + return () -> { + final boolean succeed = deleteFile(fileState, context); + notifyCompletion(threadId, listener, succeed, handler); + }; + } + + private boolean deleteFile(SyncedFileState fileState, Context context) { + //If the file has never been synchronized, then it should not exist as local + if (!fileState.hasBeenSynchronizedOnce()) { + return false; + } + + final File file = new File(fileState.getLocalPath()); + + if (!file.exists()) { + return true; + } + + try { + if (!file.delete()) { //May throw SecurityException or IOException + Timber.w("local file ( %s ) removal failed", file.getName()); + return false; + } + } catch (SecurityException exception) { + Timber.e(exception); + return false; + } + + context.getContentResolver().delete(MediaStore.Files.getContentUri("external"), + MediaStore.Files.FileColumns.DATA + "=?", + new String[]{CommonUtils.getLocalPath(file)}); + + if (DbHelper.manageSyncedFileStateDB(fileState, "DELETE", context) <= 0) { + Timber.e("Failed to remove %s from DB", file.getName()); + } + return true; + } + + private void notifyCompletion(final int threadId, final LocalFileDeletionListener listener, final boolean success, final Handler handler) { + handler.post(() -> listener.onDeletionComplete(threadId, success)); + } +} diff --git a/app/src/main/java/foundation/e/drive/work/WorkRequestFactory.java b/app/src/main/java/foundation/e/drive/work/WorkRequestFactory.java index c2ccde15..433d1c62 100644 --- a/app/src/main/java/foundation/e/drive/work/WorkRequestFactory.java +++ b/app/src/main/java/foundation/e/drive/work/WorkRequestFactory.java @@ -33,8 +33,7 @@ public class WorkRequestFactory { PERIODIC_USER_INFO, ONE_TIME_USER_INFO, CREATE_REMOTE_DIR, - FIRST_START, - LOCAL_FILE_DELETION + FIRST_START } /** @@ -71,26 +70,11 @@ public class WorkRequestFactory { case CREATE_REMOTE_DIR: if (syncedFolder == null) throw new NullPointerException("Synced folder is null"); return createOneTimeCreateRemoteFolderWorkRequest(syncedFolder); - case LOCAL_FILE_DELETION: - if (fileState == null) throw new NullPointerException("SyncedFileState is null"); - return createOneTimeLocalFileDeleteWorker(fileState); default: throw new InvalidParameterException("Unsupported Work Type: " + type); } } - private static OneTimeWorkRequest createOneTimeLocalFileDeleteWorker(SyncedFileState fileState) { - final OneTimeWorkRequest request = new OneTimeWorkRequest.Builder(LocalFileDeleteWorker.class) - .addTag(AppConstants.WORK_GENERIC_TAG) - .addTag("LOCAL_FILE_DELETION") - .setInputData(createDataFromSyncedFileState(fileState)) - .setExpedited(OutOfQuotaPolicy.RUN_AS_NON_EXPEDITED_WORK_REQUEST) - .build(); - - return request; - } - - /** * Create a PeridocWorkRequest instance for * a Full scan with constraints on network (should @@ -211,19 +195,4 @@ public class WorkRequestFactory { .putBoolean(DATA_KEY_MEDIATYPE, folder.isMediaType()) .build(); } - - - private static Data createDataFromSyncedFileState(SyncedFileState file) { - return new Data.Builder() - .putInt(LocalFileDeleteWorker.DATA_KEY_ID, file.getId()) - .putString(LocalFileDeleteWorker.DATA_KEY_NAME, file.getName()) - .putString(LocalFileDeleteWorker.DATA_KEY_LOCAL_PATH, file.getLocalPath()) - .putString(LocalFileDeleteWorker.DATA_KEY_REMOTE_PATH, file.getRemotePath()) - .putString(LocalFileDeleteWorker.DATA_KEY_LAST_ETAG, file.getLastETAG()) - .putLong(LocalFileDeleteWorker.DATA_KEY_LAST_MODIFIED, file.getLocalLastModified()) - .putLong(LocalFileDeleteWorker.DATA_KEY_FOLDER_ID, file.getSyncedFolderId()) - .putBoolean(LocalFileDeleteWorker.DATA_KEY_MEDIATYPE, file.isMediaType()) - .putInt(LocalFileDeleteWorker.DATA_KEY_SCANNABLE, file.getScannable()) - .build(); - } } -- GitLab From 5a27d819468d385d735a0a4ba551d5024d940473 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Mon, 28 Nov 2022 16:29:19 +0100 Subject: [PATCH 10/13] fix error in SyncedFileStateDAO.getBySyncedFolderIds-) --- .../e/drive/database/SyncedFileStateDAO.java | 24 ++++++++++++++----- .../database/SyncedFileStateDAOTest.java | 5 +--- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/database/SyncedFileStateDAO.java b/app/src/main/java/foundation/e/drive/database/SyncedFileStateDAO.java index 18cff0df..dec40fd0 100644 --- a/app/src/main/java/foundation/e/drive/database/SyncedFileStateDAO.java +++ b/app/src/main/java/foundation/e/drive/database/SyncedFileStateDAO.java @@ -19,6 +19,7 @@ import android.database.sqlite.SQLiteDoneException; import android.database.sqlite.SQLiteStatement; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import foundation.e.drive.models.SyncedFileState; @@ -195,15 +196,26 @@ import static foundation.e.drive.database.SyncedFileStateContract.SYNCEDFOLDER_I /* package */ List getBySyncedFolderIds(List syncedFolderIds) { final List result = new ArrayList<>(); if (syncedFolderIds == null || syncedFolderIds.isEmpty()) { + Timber.d("getBySyncedFolderIds(): SyncedFOlderIds is empty or null"); return result; } - final String whereClause = SYNCEDFOLDER_ID + " IN (?)"; - final String[] whereValue = new String[] { - syncedFolderIds.toString() - .replace("[", "(") - .replace("]", ")") }; - final Cursor cursor = mDB.query(TABLE_NAME, allColumns, whereClause, whereValue, null, null, null); + final List matcherList = new ArrayList<>(); //list of "?" to be replaced by value by SQliteDatabase.query() call + + final String[] whereValue = new String[syncedFolderIds.size()]; + for (int i = -1; ++i < syncedFolderIds.size();) { + matcherList.add("?"); + whereValue[i] = syncedFolderIds.get(i).toString(); + } + + final StringBuilder whereClause = new StringBuilder(); + whereClause.append(SYNCEDFOLDER_ID) + .append(" IN ") + .append(matcherList.toString() + .replace("[", "(") + .replace("]", ")")); + + final Cursor cursor = mDB.query(TABLE_NAME, allColumns, whereClause.toString(), whereValue, null, null, null); cursor.moveToFirst(); while(!cursor.isAfterLast() ) { result.add( cursorToSyncedFileState(cursor) ); diff --git a/app/src/test/java/foundation/e/drive/database/SyncedFileStateDAOTest.java b/app/src/test/java/foundation/e/drive/database/SyncedFileStateDAOTest.java index 945b8694..a954e22b 100644 --- a/app/src/test/java/foundation/e/drive/database/SyncedFileStateDAOTest.java +++ b/app/src/test/java/foundation/e/drive/database/SyncedFileStateDAOTest.java @@ -67,11 +67,8 @@ public class SyncedFileStateDAOTest { Assert.assertTrue(result.isEmpty()); } - /* - * @todo find why this test fails - */ + @Test - @Ignore public void getBySyncedFolderIds_notEmpty_returnOneFileState() { final List folderIds = new ArrayList<>(); folderIds.add(new Long(12)); -- GitLab From 982a4885f57ae5e1fc517e8d2c081645e904143b Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Mon, 28 Nov 2022 17:22:10 +0100 Subject: [PATCH 11/13] fix LocalFileDeleter --- .../e/drive/work/LocalFileDeleter.java | 31 +++++++------------ 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/work/LocalFileDeleter.java b/app/src/main/java/foundation/e/drive/work/LocalFileDeleter.java index afb7e91e..38d6a073 100644 --- a/app/src/main/java/foundation/e/drive/work/LocalFileDeleter.java +++ b/app/src/main/java/foundation/e/drive/work/LocalFileDeleter.java @@ -35,30 +35,23 @@ public class LocalFileDeleter { } private boolean deleteFile(SyncedFileState fileState, Context context) { - //If the file has never been synchronized, then it should not exist as local - if (!fileState.hasBeenSynchronizedOnce()) { - return false; - } - final File file = new File(fileState.getLocalPath()); - - if (!file.exists()) { - return true; - } - - try { - if (!file.delete()) { //May throw SecurityException or IOException - Timber.w("local file ( %s ) removal failed", file.getName()); + if (file.exists()) { + try { + if (!file.delete()) { + Timber.w("local file ( %s ) removal failed", file.getName()); + return false; + } + } catch (SecurityException exception) { + Timber.e(exception); return false; } - } catch (SecurityException exception) { - Timber.e(exception); - return false; + + context.getContentResolver().delete(MediaStore.Files.getContentUri("external"), + MediaStore.Files.FileColumns.DATA + "=?", + new String[]{CommonUtils.getLocalPath(file)}); } - context.getContentResolver().delete(MediaStore.Files.getContentUri("external"), - MediaStore.Files.FileColumns.DATA + "=?", - new String[]{CommonUtils.getLocalPath(file)}); if (DbHelper.manageSyncedFileStateDB(fileState, "DELETE", context) <= 0) { Timber.e("Failed to remove %s from DB", file.getName()); -- GitLab From 61c1553a50387d5f3d8872eb32df3ee1e9af13ff Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Tue, 29 Nov 2022 10:40:17 +0100 Subject: [PATCH 12/13] fix reported by reviewers - Add missing licence headers to LocalFileDeleter - update licence headers to RemoteContentScanner and remove commented code - update licence headers to LocalContentScanner and add a @author to the class - rewrite for loop in SycnedFileStateDAO for less performance but better readibility - remove a space before a comma in SynchronizationService - update licence headers for RemoteContentScannerTest & LocalContentScannerTest --- .../contentScanner/LocalContentScanner.java | 3 +- .../contentScanner/RemoteContentScanner.java | 30 +------------------ .../e/drive/database/SyncedFileStateDAO.java | 2 +- .../services/SynchronizationService.java | 2 +- .../e/drive/work/LocalFileDeleter.java | 8 +++++ .../LocalContentScannerTest.java | 2 +- .../RemoteContentScannerTest.java | 2 +- 7 files changed, 15 insertions(+), 34 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/contentScanner/LocalContentScanner.java b/app/src/main/java/foundation/e/drive/contentScanner/LocalContentScanner.java index 63a1a62f..0187ac57 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/LocalContentScanner.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/LocalContentScanner.java @@ -1,5 +1,5 @@ /* - * Copyright © ECORP SAS 2022. + * Copyright © MURENA 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 @@ -23,6 +23,7 @@ import timber.log.Timber; /** * Class to encapsulate function about scanning local file and * create syncRequest when needed + * @author vincent Bourgmayer */ public class LocalContentScanner extends AbstractContentScanner{ diff --git a/app/src/main/java/foundation/e/drive/contentScanner/RemoteContentScanner.java b/app/src/main/java/foundation/e/drive/contentScanner/RemoteContentScanner.java index c5461f7d..342d9505 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/RemoteContentScanner.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/RemoteContentScanner.java @@ -1,5 +1,5 @@ /* - * Copyright © ECORP SAS 2022. + * Copyright © MURENA 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 @@ -83,34 +83,6 @@ public class RemoteContentScanner extends AbstractContentScanner { @Override protected void onMissingFile(SyncedFileState fileState) { this.syncRequests.put(fileState.getId(), new SyncRequest(fileState, LOCAL_DELETE)); - - /* todo: move commented code into synchronizationService - if (!CommonUtils.isThisSyncAllowed(account, fileState.isMediaType())) { - Timber.d("Sync of current file: %s isn't allowed", fileState.getName()); - return; - } - - if (!fileState.hasBeenSynchronizedOnce()) { - return; - } - - final File file = new File(fileState.getLocalPath()); - if (!file.exists()) { - return; - } - - context.getContentResolver().delete(MediaStore.Files.getContentUri("external"), - MediaStore.Files.FileColumns.DATA + "=?", - new String[]{CommonUtils.getLocalPath(file)}); - - if (!file.delete()) { //May throw SecurityException or IOException - Timber.d("local file ( %s ) removal failed",file.getName()); - return; - } - - if (DbHelper.manageSyncedFileStateDB(fileState, "DELETE", context) <= 0) { - Timber.e("Failed to remove %s from DB", file.getName()); - }*/ } @Override diff --git a/app/src/main/java/foundation/e/drive/database/SyncedFileStateDAO.java b/app/src/main/java/foundation/e/drive/database/SyncedFileStateDAO.java index dec40fd0..7f14da65 100644 --- a/app/src/main/java/foundation/e/drive/database/SyncedFileStateDAO.java +++ b/app/src/main/java/foundation/e/drive/database/SyncedFileStateDAO.java @@ -203,7 +203,7 @@ import static foundation.e.drive.database.SyncedFileStateContract.SYNCEDFOLDER_I final List matcherList = new ArrayList<>(); //list of "?" to be replaced by value by SQliteDatabase.query() call final String[] whereValue = new String[syncedFolderIds.size()]; - for (int i = -1; ++i < syncedFolderIds.size();) { + for (int i = 0; i < syncedFolderIds.size(); i++) { matcherList.add("?"); whereValue[i] = syncedFolderIds.get(i).toString(); } 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 8ea93da9..ed0f2ab9 100644 --- a/app/src/main/java/foundation/e/drive/services/SynchronizationService.java +++ b/app/src/main/java/foundation/e/drive/services/SynchronizationService.java @@ -211,7 +211,7 @@ public class SynchronizationService extends Service implements OnRemoteOperation if (request.getOperationType().equals(SyncRequest.Type.LOCAL_DELETE)) { final LocalFileDeleter fileDeleter = new LocalFileDeleter(request.getSyncedFileState()); - threadPool[threadIndex] = new Thread(fileDeleter.getRunnable( handler, threadIndex , getApplicationContext(), this)); + threadPool[threadIndex] = new Thread(fileDeleter.getRunnable( handler, threadIndex, getApplicationContext(), this)); threadPool[threadIndex].start(); startedSync.put(threadIndex, syncWrapper); return; diff --git a/app/src/main/java/foundation/e/drive/work/LocalFileDeleter.java b/app/src/main/java/foundation/e/drive/work/LocalFileDeleter.java index 38d6a073..f6178551 100644 --- a/app/src/main/java/foundation/e/drive/work/LocalFileDeleter.java +++ b/app/src/main/java/foundation/e/drive/work/LocalFileDeleter.java @@ -1,3 +1,10 @@ +/* + * Copyright © MURENA 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.work; import android.content.Context; @@ -14,6 +21,7 @@ import timber.log.Timber; /** * This worker is called when a remote file has been detected as removed and that all the * scanning process lead to choose to delete the local version too. + * @author vincent Bourgmayer */ public class LocalFileDeleter { diff --git a/app/src/test/java/foundation/e/drive/contentScanner/LocalContentScannerTest.java b/app/src/test/java/foundation/e/drive/contentScanner/LocalContentScannerTest.java index ec4ec503..118dd6a2 100644 --- a/app/src/test/java/foundation/e/drive/contentScanner/LocalContentScannerTest.java +++ b/app/src/test/java/foundation/e/drive/contentScanner/LocalContentScannerTest.java @@ -1,5 +1,5 @@ /* - * Copyright © ECORP SAS 2022. + * Copyright © MURENA 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 diff --git a/app/src/test/java/foundation/e/drive/contentScanner/RemoteContentScannerTest.java b/app/src/test/java/foundation/e/drive/contentScanner/RemoteContentScannerTest.java index 5f422c12..ae291d14 100644 --- a/app/src/test/java/foundation/e/drive/contentScanner/RemoteContentScannerTest.java +++ b/app/src/test/java/foundation/e/drive/contentScanner/RemoteContentScannerTest.java @@ -1,5 +1,5 @@ /* - * Copyright © ECORP SAS 2022. + * Copyright © MURENA 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 -- GitLab From 0641e47654519f1f10becb7e0ffed191b7ee845a Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Tue, 29 Nov 2022 15:07:50 +0100 Subject: [PATCH 13/13] Remove remaining codes from gave up implementation --- app/src/main/java/foundation/e/drive/utils/CommonUtils.java | 6 +++--- .../java/foundation/e/drive/work/WorkRequestFactory.java | 6 ++---- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/utils/CommonUtils.java b/app/src/main/java/foundation/e/drive/utils/CommonUtils.java index c490b759..3f488756 100644 --- a/app/src/main/java/foundation/e/drive/utils/CommonUtils.java +++ b/app/src/main/java/foundation/e/drive/utils/CommonUtils.java @@ -307,15 +307,15 @@ public abstract class CommonUtils { return; } - final OneTimeWorkRequest getUserInfoRequest = WorkRequestFactory.getOneTimeWorkRequest(WorkRequestFactory.WorkType.ONE_TIME_USER_INFO, null, null); + final OneTimeWorkRequest getUserInfoRequest = WorkRequestFactory.getOneTimeWorkRequest(WorkRequestFactory.WorkType.ONE_TIME_USER_INFO, null); final List workRequests = new ArrayList<>(); for (SyncedFolder folder : syncedFolders) { - final OneTimeWorkRequest createRemoteFolderWorkRequest = WorkRequestFactory.getOneTimeWorkRequest(WorkRequestFactory.WorkType.CREATE_REMOTE_DIR, folder, null); + final OneTimeWorkRequest createRemoteFolderWorkRequest = WorkRequestFactory.getOneTimeWorkRequest(WorkRequestFactory.WorkType.CREATE_REMOTE_DIR, folder); workRequests.add(createRemoteFolderWorkRequest); } - final OneTimeWorkRequest firstStartRequest = WorkRequestFactory.getOneTimeWorkRequest(WorkRequestFactory.WorkType.FIRST_START, null, null); + final OneTimeWorkRequest firstStartRequest = WorkRequestFactory.getOneTimeWorkRequest(WorkRequestFactory.WorkType.FIRST_START, null); workManager.beginWith(getUserInfoRequest) .then(workRequests) diff --git a/app/src/main/java/foundation/e/drive/work/WorkRequestFactory.java b/app/src/main/java/foundation/e/drive/work/WorkRequestFactory.java index 433d1c62..e3daf516 100644 --- a/app/src/main/java/foundation/e/drive/work/WorkRequestFactory.java +++ b/app/src/main/java/foundation/e/drive/work/WorkRequestFactory.java @@ -17,13 +17,11 @@ import androidx.work.Constraints; import androidx.work.Data; import androidx.work.NetworkType; import androidx.work.OneTimeWorkRequest; -import androidx.work.OutOfQuotaPolicy; import androidx.work.PeriodicWorkRequest; import java.security.InvalidParameterException; import java.util.concurrent.TimeUnit; -import foundation.e.drive.models.SyncedFileState; import foundation.e.drive.models.SyncedFolder; import foundation.e.drive.utils.AppConstants; @@ -58,10 +56,9 @@ public class WorkRequestFactory { * @param type Should be ONE_TIME_USER_INFO, or FIRST_START, or CREATE_REMOTE_DIR * or it will throw InvalidParameterException * @param syncedFolder this parameter is required for CREATE_REMOTE_DIR work type. If null it will throw an NPE. - * @param fileState This parameter is required for LOCAL_FILE_DELETION work type. If null it will throw NPE * @return OneTimeWorkRequest's instance. */ - public static OneTimeWorkRequest getOneTimeWorkRequest(WorkType type, @Nullable SyncedFolder syncedFolder, @Nullable SyncedFileState fileState) { + public static OneTimeWorkRequest getOneTimeWorkRequest(WorkType type, @Nullable SyncedFolder syncedFolder) { switch (type) { case ONE_TIME_USER_INFO: return createOneTimeGetUserInfoWorkRequest(); @@ -75,6 +72,7 @@ public class WorkRequestFactory { } } + /** * Create a PeridocWorkRequest instance for * a Full scan with constraints on network (should -- GitLab