From e04c24494015bd528683a2ecc718ae20906e5744 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Mon, 31 Oct 2022 15:53:39 +0100 Subject: [PATCH 01/12] Implement new classes to handle listing of existing content The goal is to have dedicated classes which will allow dedicated tests - AbstractFileLister: which is an abstract class that bear the general workflow - RemoteFileLister: implementation of AbstractFileLister for RemoteContent - LocalFileLister: implementation of AbstractFileLister for LocalContent - FolderWrapper: a utility class used by the implementations of AbstractFileLister --- .../contentScanner/AbstractFileLister.java | 192 ++++++++++++++++++ .../e/drive/contentScanner/FolderWrapper.java | 55 +++++ .../drive/contentScanner/LocalFileLister.java | 93 +++++++++ .../contentScanner/RemoteFileLister.java | 117 +++++++++++ 4 files changed, 457 insertions(+) create mode 100644 app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java create mode 100644 app/src/main/java/foundation/e/drive/contentScanner/FolderWrapper.java create mode 100644 app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java create mode 100644 app/src/main/java/foundation/e/drive/contentScanner/RemoteFileLister.java diff --git a/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java b/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java new file mode 100644 index 00000000..59516904 --- /dev/null +++ b/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java @@ -0,0 +1,192 @@ +/* + * 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.content.Context; + +import java.util.ArrayList; +import java.util.List; +import java.util.ListIterator; + +import foundation.e.drive.database.DbHelper; +import foundation.e.drive.models.SyncedFolder; +import timber.log.Timber; + +/** + * Factorisation of codes used to list file on cloud and on device + * Implementation will vary depending of it is RemoteFile or File instance + * that we want to list + * @author vincent Bourgmayer + */ +public abstract class AbstractFileLister { + + /** + * This allows to use RemoteOperationResult for RemoteFileLister + * todo: some optimization may be done with FolderWrapper + * @param RemoteFile or File + */ + /* package */ interface DirectoryLoader { + /* package */ FolderWrapper getFolder(); + /* package */ boolean load(SyncedFolder folder); + } + + protected final List folders; + private final List contentToScan; + + /** + * constructor to be call with super + * @param folders List of SyncedFolder to read + */ + protected AbstractFileLister(List folders) { + this.folders = folders; + this.contentToScan = new ArrayList<>(); + } + + /** + * Perform the listing of files + * @param context + * @return + */ + public boolean listContentToScan(Context context) { + //For each SyncedFolder + final ListIterator iterator = folders.listIterator() ; + boolean isSomeContentToSync = false; + while (iterator.hasNext()) { + final SyncedFolder syncedFolder = iterator.next(); + Timber.v("SyncedFolder : %s, %s, %s, %s, %s", syncedFolder.getLibelle(), syncedFolder.getLocalFolder(), syncedFolder.getLastModified(), syncedFolder.isScanLocal(), syncedFolder.getId()); + isSomeContentToSync = isSomeContentToSync || hasDirectoryContentToScan(syncedFolder, iterator, context); + } + + if (isSomeContentToSync) { + DbHelper.updateSyncedFolders(folders, context); //@ToDo: maybe do this when all contents will be synced. + } + return isSomeContentToSync; + } + + + /** + * Detailed process for reading a single folder + * @param syncedFolder the SyncedFolder to check + * @param iterator iterator over list of SyncedFolder + * @param context context used to call DB + * @return true the directory has content to synchronized false otherwise + */ + private boolean hasDirectoryContentToScan( SyncedFolder syncedFolder, ListIterator iterator, Context context) { + // I check if I can skip it; If then then go to next + final DirectoryLoader dirLoader = getDirectoryLoader(); + if (skipSyncedFolder(syncedFolder) + || !isSyncedFolderInDb(syncedFolder, context) + || !dirLoader.load(syncedFolder)) { // I try to get the real directory (File or RemoteFile). + iterator.remove(); + return false; + } + + final FolderWrapper currentDirectory = dirLoader.getFolder(); + + // If directory is missing (not exist or http 404) + if (currentDirectory.isMissing()) { + return true; + } + + //--> I no content has changed, skip + if (skipDirectory(currentDirectory.getFolder(), syncedFolder, context)) { + iterator.remove(); + return false; + } + + //else I have to check if some content has changed inside + final List dirContent = currentDirectory.getContent(); + + //todo: look where to put in subclasses : syncedFolder.setLastEtag(directory.getEtag()).setToSync(true); + + if (!dirContent.isEmpty()) { + contentToScan.addAll(sortContent(iterator, syncedFolder, dirContent)); + } + return true; + } + + + /** + * Tell if the given syncedFolder has been persisted in database. + * If it's not the case initally, it tries to persist it. + * @param syncedFolder SyncedFolder to check for persistance + * @param context context to contact database + * @return false if not persisted in db + */ + private boolean isSyncedFolderInDb(SyncedFolder syncedFolder, Context context) { + if (syncedFolder.getId() > 0) return true; + + final int syncedFolder_id = (int) DbHelper.insertSyncedFolder(syncedFolder, context); //It will return -1 if there is an error, like an already existing folder with same value + if (syncedFolder_id <= 0) { + Timber.v("insertion of syncedFolder for %s failed: %s ", syncedFolder.getRemoteFolder(), syncedFolder_id); + return false; + } + + syncedFolder.setId(syncedFolder_id); + return true; + } + + + /** + * Split content of a folder: subfolder on one side are added into the iterator loop + * while subfiles are added to result list + * @param iterator iterator intance over list of SyncedFolder + * @param syncedFolder the SyncFolder which own the content + * @param dirContent Content to sort + * @return List of subfiles to scan or empty list if nothing + */ + private List sortContent(ListIterator iterator, SyncedFolder syncedFolder, List dirContent) { + final List result = new ArrayList<>(); + if (dirContent == null) return result; + + for (T subFile : dirContent) { + final String fileName = getFileName(subFile); + if (isDirectory(subFile)) { + final SyncedFolder subSyncedFolder = new SyncedFolder(syncedFolder, getFileName(subFile), 0L, "");//Need to set lastModified to 0 to handle it on next iteration + iterator.add(subSyncedFolder); + iterator.previous(); + } else if (!skipFile(subFile)) { + Timber.v("added %s into list of file to scan", fileName); + result.add(subFile); + } + } + + return result; + } + + /** + * List of file to scan + * @return List or List is expected based on implementations + */ + public List getContentToScan() { + return contentToScan; + } + + /** + * Share the list of syncedFolder's ID for syncedFolder which has content to scan + * @return List of syncedFolder ids + */ + public List getSyncedFoldersId() { + final List result = new ArrayList<>(); + for (SyncedFolder syncedFolder : folders) { + //if (syncedFolder.isToSync() ){ //todo: is it usefull ? maybe this propertie can be removed from SyncedFolder + result.add( (long) syncedFolder.getId() ); + //} + } + return result; + } + + + + protected abstract boolean skipSyncedFolder(SyncedFolder syncedFolder); + protected abstract boolean skipFile(T file); + protected abstract boolean skipDirectory(T currentDirectory, SyncedFolder syncedFolder, Context context); + protected abstract boolean isDirectory(T file); + protected abstract String getFileName(T file); + protected abstract DirectoryLoader getDirectoryLoader(); +} diff --git a/app/src/main/java/foundation/e/drive/contentScanner/FolderWrapper.java b/app/src/main/java/foundation/e/drive/contentScanner/FolderWrapper.java new file mode 100644 index 00000000..ca273584 --- /dev/null +++ b/app/src/main/java/foundation/e/drive/contentScanner/FolderWrapper.java @@ -0,0 +1,55 @@ +/* + * 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 java.util.ArrayList; +import java.util.List; + +/** + * This wrapper is used to contains either a File/RemoteFile instance with its content + * or a "missing file" information. The explanation of this, is because scanning remote file + * may result in different type of error while loading content (network issues, etc.) + * but we want to handle 404 error (= missing resources) has to be considered like a + * potential file deletions. There is probably something better to do for that + * @author vincent Bourgmayer + * @param File or RemoteFile expected + */ +public class FolderWrapper { + + private final boolean missing; + private final T folder; + private final List content; + + protected FolderWrapper(T folder) { + content = new ArrayList<>(); + this.folder = folder; + missing = false; + } + + protected FolderWrapper() { + missing = true; + folder = null; + content = null; + } + + public T getFolder() { + return folder; + } + + public List getContent() { + return content; + } + + public void addContent(List newContent) { + content.addAll(newContent); + } + + public boolean isMissing() { + return missing; + } +} diff --git a/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java b/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java new file mode 100644 index 00000000..93fed784 --- /dev/null +++ b/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java @@ -0,0 +1,93 @@ +/* + * 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.content.Context; + +import java.io.File; +import java.io.FileFilter; +import java.util.Arrays; +import java.util.List; + +import foundation.e.drive.database.DbHelper; +import foundation.e.drive.fileFilters.FileFilterFactory; +import foundation.e.drive.models.SyncedFolder; +import foundation.e.drive.utils.CommonUtils; +import timber.log.Timber; + +/** + * Implementation of AbstractFileLister with method adapted to local content + * @author vincent Bourgmayer + */ +public class LocalFileLister extends AbstractFileLister { + public LocalFileLister(List directories) { + super(directories); + } + + @Override + protected boolean skipDirectory(File currentDirectory, SyncedFolder syncedFolder, Context context) { + return currentDirectory.lastModified() == syncedFolder.getLastModified() + || !DbHelper.syncedFolderHasContentToUpload(syncedFolder.getId(), context); + } + + @Override + protected boolean skipSyncedFolder(SyncedFolder syncedFolder) { + final String fileName = CommonUtils.getFileNameFromPath(syncedFolder.getLocalFolder()); + if (fileName == null || fileName.startsWith(".") || !syncedFolder.isScanLocal()) { + return true; + } + + return false; + } + + @Override + protected boolean skipFile(File file) { + return file.isHidden(); + } + + @Override + protected boolean isDirectory(File file) { + return file.isDirectory(); + } + + @Override + protected String getFileName(File file) { + return file.getName(); + } + + @Override + protected DirectoryLoader getDirectoryLoader() { + return new LocalDirLoader(); + } + + private final class LocalDirLoader implements DirectoryLoader { + private FolderWrapper folder; + + @Override + public FolderWrapper getFolder() { + return folder; + } + + @Override + public boolean load(SyncedFolder syncedFolder) { + final File dir = new File(syncedFolder.getLocalFolder()); + Timber.v("Local Folder (last modified / exists): %s, %s", dir.lastModified(),dir.exists()); + + if (!dir.exists()) { + folder = new FolderWrapper(); //missing Folder Wrapper + } + else { + folder = new FolderWrapper(dir); + final String category = syncedFolder.isMediaType() ? "media" : syncedFolder.getLibelle(); + final FileFilter filter = FileFilterFactory.getFileFilter(category); + folder.addContent(Arrays.asList(dir.listFiles(filter))); + } + return true; + } + } +} diff --git a/app/src/main/java/foundation/e/drive/contentScanner/RemoteFileLister.java b/app/src/main/java/foundation/e/drive/contentScanner/RemoteFileLister.java new file mode 100644 index 00000000..8a3a0313 --- /dev/null +++ b/app/src/main/java/foundation/e/drive/contentScanner/RemoteFileLister.java @@ -0,0 +1,117 @@ +/* + * 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.content.Context; + +import androidx.annotation.VisibleForTesting; + +import com.owncloud.android.lib.common.OwnCloudClient; +import com.owncloud.android.lib.common.operations.RemoteOperationResult; +import com.owncloud.android.lib.resources.files.ReadFolderRemoteOperation; +import com.owncloud.android.lib.resources.files.model.RemoteFile; + +import java.util.ArrayList; +import java.util.List; + +import foundation.e.drive.database.DbHelper; +import foundation.e.drive.models.SyncedFolder; +import foundation.e.drive.utils.CommonUtils; + + +/** + * Implementation of AbstractFileLister with method adapted to remote content + * @author vincent Bourgmayer + */ +public class RemoteFileLister extends AbstractFileLister { + private static final int HTTP_404 = 404; + private OwnCloudClient client; + + public RemoteFileLister(List directories, OwnCloudClient client) { + super(directories); + this.client = client; + } + + @Override + protected boolean skipSyncedFolder(SyncedFolder syncedFolder) { + return (syncedFolder.isMediaType() + && CommonUtils.getFileNameFromPath(syncedFolder.getRemoteFolder()).startsWith(".")) + || !syncedFolder.isScanRemote(); + } + + @Override + protected boolean skipFile(RemoteFile file) { + final String fileName = CommonUtils.getFileNameFromPath(file.getRemotePath()); + return fileName.startsWith("."); + } + + @Override + protected boolean skipDirectory(RemoteFile currentDirectory, SyncedFolder syncedFolder, Context context) { + return currentDirectory.getEtag().equals(syncedFolder.getLastEtag()) + || !DbHelper.syncedFolderHasContentToDownload(syncedFolder.getId(), context); + } + + @Override + protected boolean isDirectory(RemoteFile file) { + return file.getMimeType().equals("DIR"); + } + + @Override + protected String getFileName(RemoteFile file) { + return null; + } + + @Override + protected DirectoryLoader getDirectoryLoader() { + return new RemoteDirectoryLoader(); + } + + private final class RemoteDirectoryLoader implements DirectoryLoader { + + private FolderWrapper directory; + + @Override + public boolean load(SyncedFolder syncedFolder) { + final RemoteOperationResult remoteOperationResult = readRemoteFolder(syncedFolder.getRemoteFolder(), client); + + if (!remoteOperationResult.isSuccess()) { + if (remoteOperationResult.getHttpCode() == HTTP_404) { + directory = new FolderWrapper<>(); + return true; + } + return false; + } + + final int dataSize = remoteOperationResult.getData().size(); + directory = new FolderWrapper<>( (RemoteFile) remoteOperationResult.getData().get(0)); + List subFiles = new ArrayList<>(); + + for (Object o: remoteOperationResult.getData().subList(1, dataSize)) { + subFiles.add((RemoteFile) o); + } + return true; + } + + /** + * Execute the Propfind query to list files under a directory + * @param remotePath RemotePath of the directory to observe + * @param client Client used to execute query + * @return RemoteOperationResult + */ + @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) + private RemoteOperationResult readRemoteFolder(String remotePath, OwnCloudClient client) { + final ReadFolderRemoteOperation operation = new ReadFolderRemoteOperation(remotePath); + return operation.execute(client); + } + + @Override + public FolderWrapper getFolder() { + return directory; + } + } +} -- GitLab From b8dadb1e00c876a160b13fab19b907e87bc5aa4a Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Wed, 2 Nov 2022 15:18:55 +0100 Subject: [PATCH 02/12] Add new Test file: RemoteFileListerTest.java with unit tests - Fix error in LocalFileLister.skipDirectory(...) and RemoteFileLister.skipDirectory() - Add test cases for skipSyncedFolder(...) in RemoteFileListerTest - Add test cases for SkipFile(...) in RemoteFileListerTest - Add test cases for SkipDirectory(...) in RemoteFileListerTest - Add test cases for LocalFileLister.RemoteDirectoryLoader.load() - rename some method to replace "directory" by "folder" to keep consistency across classes - Prevent NPE & indexOutOfBoundsException in RemoteFileLister.RemoteFolderLoader.load --- .../contentScanner/AbstractFileLister.java | 12 +- .../drive/contentScanner/LocalFileLister.java | 8 +- .../contentScanner/RemoteFileLister.java | 39 +- .../contentScanner/LocalFileListerTest.java | 2 + .../contentScanner/RemoteFileListerTest.java | 365 ++++++++++++++++++ 5 files changed, 399 insertions(+), 27 deletions(-) create mode 100644 app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java create mode 100644 app/src/test/java/foundation/e/drive/contentScanner/RemoteFileListerTest.java diff --git a/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java b/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java index 59516904..34dfe74b 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java @@ -30,8 +30,8 @@ public abstract class AbstractFileLister { * todo: some optimization may be done with FolderWrapper * @param RemoteFile or File */ - /* package */ interface DirectoryLoader { - /* package */ FolderWrapper getFolder(); + /* package */ interface FolderLoader { + /* package */ FolderWrapper getFolderWrapper(); /* package */ boolean load(SyncedFolder folder); } @@ -78,7 +78,7 @@ public abstract class AbstractFileLister { */ private boolean hasDirectoryContentToScan( SyncedFolder syncedFolder, ListIterator iterator, Context context) { // I check if I can skip it; If then then go to next - final DirectoryLoader dirLoader = getDirectoryLoader(); + final FolderLoader dirLoader = getFolderLoader(); if (skipSyncedFolder(syncedFolder) || !isSyncedFolderInDb(syncedFolder, context) || !dirLoader.load(syncedFolder)) { // I try to get the real directory (File or RemoteFile). @@ -86,7 +86,7 @@ public abstract class AbstractFileLister { return false; } - final FolderWrapper currentDirectory = dirLoader.getFolder(); + final FolderWrapper currentDirectory = dirLoader.getFolderWrapper(); // If directory is missing (not exist or http 404) if (currentDirectory.isMissing()) { @@ -147,7 +147,7 @@ public abstract class AbstractFileLister { for (T subFile : dirContent) { final String fileName = getFileName(subFile); if (isDirectory(subFile)) { - final SyncedFolder subSyncedFolder = new SyncedFolder(syncedFolder, getFileName(subFile), 0L, "");//Need to set lastModified to 0 to handle it on next iteration + final SyncedFolder subSyncedFolder = new SyncedFolder(syncedFolder, fileName, 0L, "");//Need to set lastModified to 0 to handle it on next iteration iterator.add(subSyncedFolder); iterator.previous(); } else if (!skipFile(subFile)) { @@ -188,5 +188,5 @@ public abstract class AbstractFileLister { protected abstract boolean skipDirectory(T currentDirectory, SyncedFolder syncedFolder, Context context); protected abstract boolean isDirectory(T file); protected abstract String getFileName(T file); - protected abstract DirectoryLoader getDirectoryLoader(); + protected abstract FolderLoader getFolderLoader(); } diff --git a/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java b/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java index 93fed784..a37fcf00 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java @@ -32,7 +32,7 @@ public class LocalFileLister extends AbstractFileLister { @Override protected boolean skipDirectory(File currentDirectory, SyncedFolder syncedFolder, Context context) { return currentDirectory.lastModified() == syncedFolder.getLastModified() - || !DbHelper.syncedFolderHasContentToUpload(syncedFolder.getId(), context); + && !DbHelper.syncedFolderHasContentToUpload(syncedFolder.getId(), context); } @Override @@ -61,15 +61,15 @@ public class LocalFileLister extends AbstractFileLister { } @Override - protected DirectoryLoader getDirectoryLoader() { + protected FolderLoader getFolderLoader() { return new LocalDirLoader(); } - private final class LocalDirLoader implements DirectoryLoader { + private final class LocalDirLoader implements FolderLoader { private FolderWrapper folder; @Override - public FolderWrapper getFolder() { + public FolderWrapper getFolderWrapper() { return folder; } diff --git a/app/src/main/java/foundation/e/drive/contentScanner/RemoteFileLister.java b/app/src/main/java/foundation/e/drive/contentScanner/RemoteFileLister.java index 8a3a0313..eec94bf9 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/RemoteFileLister.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/RemoteFileLister.java @@ -29,7 +29,6 @@ import foundation.e.drive.utils.CommonUtils; * @author vincent Bourgmayer */ public class RemoteFileLister extends AbstractFileLister { - private static final int HTTP_404 = 404; private OwnCloudClient client; public RemoteFileLister(List directories, OwnCloudClient client) { @@ -47,13 +46,13 @@ public class RemoteFileLister extends AbstractFileLister { @Override protected boolean skipFile(RemoteFile file) { final String fileName = CommonUtils.getFileNameFromPath(file.getRemotePath()); - return fileName.startsWith("."); + return fileName == null || fileName.isEmpty() || fileName.startsWith("."); } @Override - protected boolean skipDirectory(RemoteFile currentDirectory, SyncedFolder syncedFolder, Context context) { - return currentDirectory.getEtag().equals(syncedFolder.getLastEtag()) - || !DbHelper.syncedFolderHasContentToDownload(syncedFolder.getId(), context); + protected boolean skipDirectory(RemoteFile directory, SyncedFolder syncedFolder, Context context) { + return directory.getEtag().equals(syncedFolder.getLastEtag()) + && !DbHelper.syncedFolderHasContentToDownload(syncedFolder.getId(), context); } @Override @@ -63,23 +62,25 @@ public class RemoteFileLister extends AbstractFileLister { @Override protected String getFileName(RemoteFile file) { - return null; + return CommonUtils.getFileNameFromPath(file.getRemotePath()); } @Override - protected DirectoryLoader getDirectoryLoader() { - return new RemoteDirectoryLoader(); + protected FolderLoader getFolderLoader() { + return new RemoteFolderLoader(); } - private final class RemoteDirectoryLoader implements DirectoryLoader { - + @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) + public class RemoteFolderLoader implements FolderLoader { + private static final int HTTP_404 = 404; private FolderWrapper directory; @Override public boolean load(SyncedFolder syncedFolder) { final RemoteOperationResult remoteOperationResult = readRemoteFolder(syncedFolder.getRemoteFolder(), client); - + System.out.println("succeed: " + remoteOperationResult.isSuccess()); if (!remoteOperationResult.isSuccess()) { + if (remoteOperationResult.getHttpCode() == HTTP_404) { directory = new FolderWrapper<>(); return true; @@ -87,13 +88,17 @@ public class RemoteFileLister extends AbstractFileLister { return false; } - final int dataSize = remoteOperationResult.getData().size(); - directory = new FolderWrapper<>( (RemoteFile) remoteOperationResult.getData().get(0)); - List subFiles = new ArrayList<>(); + final List datas = remoteOperationResult.getData(); + if (datas == null || datas.size() < 1) return false; + + final int dataSize = datas.size(); + directory = new FolderWrapper<>( (RemoteFile) datas.get(0)); - for (Object o: remoteOperationResult.getData().subList(1, dataSize)) { + final List subFiles = new ArrayList<>(); + for (Object o: datas.subList(1, dataSize)) { subFiles.add((RemoteFile) o); } + directory.addContent(subFiles); return true; } @@ -104,13 +109,13 @@ public class RemoteFileLister extends AbstractFileLister { * @return RemoteOperationResult */ @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) - private RemoteOperationResult readRemoteFolder(String remotePath, OwnCloudClient client) { + public RemoteOperationResult readRemoteFolder(String remotePath, OwnCloudClient client) { final ReadFolderRemoteOperation operation = new ReadFolderRemoteOperation(remotePath); return operation.execute(client); } @Override - public FolderWrapper getFolder() { + public FolderWrapper getFolderWrapper() { return directory; } } diff --git a/app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java b/app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java new file mode 100644 index 00000000..fe37fd75 --- /dev/null +++ b/app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java @@ -0,0 +1,2 @@ +package foundation.e.drive.contentScanner;public class LocalFileListerTest { +} diff --git a/app/src/test/java/foundation/e/drive/contentScanner/RemoteFileListerTest.java b/app/src/test/java/foundation/e/drive/contentScanner/RemoteFileListerTest.java new file mode 100644 index 00000000..200e3456 --- /dev/null +++ b/app/src/test/java/foundation/e/drive/contentScanner/RemoteFileListerTest.java @@ -0,0 +1,365 @@ +/* + * 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.accounts.Account; +import android.accounts.AccountManager; +import android.content.Context; +import android.database.sqlite.SQLiteDatabase; +import android.os.Build; + +import androidx.test.core.app.ApplicationProvider; + +import com.owncloud.android.lib.common.OwnCloudClient; +import com.owncloud.android.lib.common.operations.RemoteOperationResult; +import com.owncloud.android.lib.resources.files.model.RemoteFile; + +import org.junit.Assert; +import org.junit.Before; +import org.junit.Ignore; +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.util.ArrayList; + +import foundation.e.drive.TestUtils; +import foundation.e.drive.database.DbHelper; +import foundation.e.drive.models.SyncedFileState; +import foundation.e.drive.models.SyncedFolder; +import foundation.e.drive.utils.DavClientProvider; + +/** + * @author vincent Bourgmayer + */ +@RunWith(RobolectricTestRunner.class) +@Config(sdk = Build.VERSION_CODES.O, manifest = Config.NONE) +public class RemoteFileListerTest { + private static final String DIR_NO_SKIP_EXPECTED = "directory shouldn't have been skipped"; + private static final String DIR_SKIP_EXPECTED = "directory should have been skipped"; + + private RemoteFileLister fileListerUnderTest; + private final Account account; + private final OwnCloudClient ocClient; + private final Context context; + + public RemoteFileListerTest() { + context = ApplicationProvider.getApplicationContext(); + ShadowLog.stream = System.out; + TestUtils.loadServerCredentials(); + TestUtils.prepareValidAccount(AccountManager.get(context)); + account = TestUtils.getValidAccount(); + ocClient = DavClientProvider.getInstance().getClientInstance(account, context); + } + + @Before + public void setUp() { + fileListerUnderTest = new RemoteFileLister(null, ocClient); + SQLiteDatabase.deleteDatabase(context.getDatabasePath(DbHelper.DATABASE_NAME)); + } + + @Test + public void skipSyncedFolder_notMedia_notHidden_notScannable_shouldReturnTrue() { + final SyncedFolder folder = buildSyncedFolderForSkipSyncedFolderTest(false, false, false); + final boolean skipFolder = fileListerUnderTest.skipSyncedFolder(folder); + Assert.assertTrue("Not media, not hidden, not remotly scannable " + DIR_SKIP_EXPECTED, skipFolder); + } + + @Test + public void skipSyncedFolder_notMedia_notHidden_shouldReturnFalse() { + final SyncedFolder folder = buildSyncedFolderForSkipSyncedFolderTest(false, false, true); + final boolean skipFolder = fileListerUnderTest.skipSyncedFolder(folder); + Assert.assertFalse("Not media, not hidden, remotly scannable " + DIR_NO_SKIP_EXPECTED, skipFolder); + } + + @Test + public void skipSyncedFolder_notMedia_hidden_notScannable_shouldReturnTrue() { + final SyncedFolder folder = buildSyncedFolderForSkipSyncedFolderTest(false, true, false); + final boolean skipFolder = fileListerUnderTest.skipSyncedFolder(folder); + Assert.assertTrue("Not media, hidden, not remotly scannable " + DIR_SKIP_EXPECTED, skipFolder); + } + + @Test + public void skipSyncedFolder_notMedia_hidden_scannable_shouldReturnFalse() { + final SyncedFolder folder = buildSyncedFolderForSkipSyncedFolderTest(false, true, true); + final boolean skipFolder = fileListerUnderTest.skipSyncedFolder(folder); + Assert.assertFalse("Not media, hidden, remotly scannable " + DIR_NO_SKIP_EXPECTED, skipFolder); + } + + @Test + public void skipSyncedFolder_media_notHidden_notScannable_shouldReturnTrue() { + final SyncedFolder folder = buildSyncedFolderForSkipSyncedFolderTest(true, false, false); + final boolean skipFolder = fileListerUnderTest.skipSyncedFolder(folder); + Assert.assertTrue("media, not hidden, not remotly scannable " + DIR_SKIP_EXPECTED, skipFolder); + } + + @Test + public void skipSyncedFolder_media_notHidden_scannable_shouldReturnFalse() { + final SyncedFolder folder = buildSyncedFolderForSkipSyncedFolderTest(true, false, true); + final boolean skipFolder = fileListerUnderTest.skipSyncedFolder(folder); + Assert.assertFalse("media, not hidden, remotly scannable " + DIR_NO_SKIP_EXPECTED, skipFolder); + } + + @Test + public void skipSyncedFolder_media_hidden_notScannable_shouldReturnTrue() { + final SyncedFolder folder = buildSyncedFolderForSkipSyncedFolderTest(true, true, false); + final boolean skipFolder = fileListerUnderTest.skipSyncedFolder(folder); + Assert.assertTrue("media, hidden, not remotly scannable " + DIR_SKIP_EXPECTED, skipFolder); + } + + @Test + public void skipSyncedFolder_media_hidden_scannable_shouldReturnTrue() { + final SyncedFolder folder = buildSyncedFolderForSkipSyncedFolderTest(true, true, true); + final boolean skipFolder = fileListerUnderTest.skipSyncedFolder(folder); + Assert.assertTrue("media, hidden, remotly scannable " + DIR_SKIP_EXPECTED, skipFolder); + } + + private SyncedFolder buildSyncedFolderForSkipSyncedFolderTest(boolean isMedia, boolean isHidden, boolean isRemoteScannable) { + final String dirName = isHidden ? ".myFolder/" : "myFolder/"; + return new SyncedFolder("any", "/local/path/" + dirName, "/remote/path/" + dirName, true, isRemoteScannable, true, isMedia); + } + + + @Test + public void skipFile_hidden_shouldReturnTrue() { + final String hiddenRemoteFilePath = "remote/path/.tutu.png"; + final RemoteFile file = Mockito.mock(RemoteFile.class); + Mockito.when(file.getRemotePath()).thenReturn(hiddenRemoteFilePath); + + final boolean skipFile = fileListerUnderTest.skipFile(file); + Assert.assertTrue("Hidden remoteFile (" + hiddenRemoteFilePath + ") should have been skipped", skipFile); + } + + @Test + public void skipFile_notHidden_shouldReturnFalse() { + final String notHiddenRemoteFilePath = "remote/path/tutu.png"; + final RemoteFile file = Mockito.mock(RemoteFile.class); + Mockito.when(file.getRemotePath()).thenReturn(notHiddenRemoteFilePath); + + final boolean skipFile = fileListerUnderTest.skipFile(file); + Assert.assertFalse("Not hidden remoteFile (" + notHiddenRemoteFilePath + ") should not have been skipped", skipFile); + } + + @Test + public void skipFile_emptyPath_shouldReturnTrue() { + final String emptyPath = ""; + final RemoteFile file = Mockito.mock(RemoteFile.class); + Mockito.when(file.getRemotePath()).thenReturn(emptyPath); + + final boolean skipFile = fileListerUnderTest.skipFile(file); + Assert.assertTrue("RemoteFile with empty path (" + emptyPath + ") should have been skipped", skipFile); + } + + @Test + @Ignore + public void skipFile_invalidPath_shouldReturnTrue() { + final String invalidPath = "/invalid/path/"; + /*in this case: No file, just a directory + the current implementation of CommonUtils.getFileName(String path) + will return "path" while empty string should be expected. + todo: fix CommonUtils.getFileName(String path)*/ + final RemoteFile file = Mockito.mock(RemoteFile.class); + Mockito.when(file.getRemotePath()).thenReturn(invalidPath); + + final boolean skipFile = fileListerUnderTest.skipFile(file); + Assert.assertTrue("RemoteFile with empty path (" + invalidPath + ") should have been skipped", skipFile); + } + + @Test + public void skipDirectory_etagChanged_noUnsyncedContent_shouldReturnFalse() { + final RemoteFile remoteFile = Mockito.mock(RemoteFile.class); + Mockito.when(remoteFile.getEtag()).thenReturn("6666bbbb"); + + final SyncedFolder syncedFolder = new SyncedFolder("", "local", "remote", true); + syncedFolder.setId(120); + syncedFolder.setLastEtag("5555aaaa"); + + final boolean skip = fileListerUnderTest.skipDirectory(remoteFile, syncedFolder, context); + + Assert.assertFalse("Remote directory with new etag should not have been skipped but it had", skip); + } + + @Test + public void skipDirectory_etagSame_noUnsyncedContent_shouldReturnTrue() { + final RemoteFile remoteFile = Mockito.mock(RemoteFile.class); + Mockito.when(remoteFile.getEtag()).thenReturn("5555aaaa"); + + final SyncedFolder syncedFolder = new SyncedFolder("", "local", "remote", true); + syncedFolder.setId(12); + syncedFolder.setLastEtag("5555aaaa"); + + final boolean skip = fileListerUnderTest.skipDirectory(remoteFile, syncedFolder, context); + + Assert.assertTrue("Remote directory with no new etag should have been skipped but had not", skip); + } + + @Test + public void skipDirectory_etagSame_contentToSync_shouldReturnFalse() { + final RemoteFile remoteFile = Mockito.mock(RemoteFile.class); + Mockito.when(remoteFile.getEtag()).thenReturn("5555aaaa"); + + final SyncedFolder syncedFolder = new SyncedFolder("", "local", "remote", true); + syncedFolder.setId(12); + syncedFolder.setLastEtag("5555aaaa"); + + final SyncedFileState dummy = new SyncedFileState(6, "tutu.jpg", "local/tutu.jpg", "remote/tutu.jpg", "7777", 0L, 12, true, 3); + DbHelper.manageSyncedFileStateDB(dummy, "INSERT", context); + + final boolean skip = fileListerUnderTest.skipDirectory(remoteFile, syncedFolder, context); + + Assert.assertFalse("Remote directory with no new etag but there is content to sync from DB: should not have been skipped but it had", skip); + } + + @Test + public void skipDirectory_etagChanged_contentToSync_shouldReturnFalse() { + final RemoteFile remoteFile = Mockito.mock(RemoteFile.class); + Mockito.when(remoteFile.getEtag()).thenReturn("6666bbbb"); + + final SyncedFolder syncedFolder = new SyncedFolder("", "local", "remote", true); + syncedFolder.setId(12); + syncedFolder.setLastEtag("5555aaaa"); + + final SyncedFileState dummy = new SyncedFileState(6, "tutu.jpg", "local/tutu.jpg", "remote/tutu.jpg", "7777", 0L, 12, true, 3); + DbHelper.manageSyncedFileStateDB(dummy, "INSERT", context); + + final boolean skip = fileListerUnderTest.skipDirectory(remoteFile, syncedFolder, context); + + Assert.assertFalse("Remote directory with new etag and there is content to sync from DB: should not have been skipped but it had", skip); + } + //Test about inner class: DirectoryLoader + + @Test + public void loadDirectory_serverResponse404_shouldReturnTrue() { + final String remotePath = "remote/path/myFolder/"; + final OwnCloudClient client = DavClientProvider.getInstance().getClientInstance(account, context); + final SyncedFolder folder = new SyncedFolder("any", "local/path/myFolder", remotePath, true); + + final RemoteOperationResult fakeResult = Mockito.mock(RemoteOperationResult.class); + Mockito.when(fakeResult.getHttpCode()).thenReturn(404); + Mockito.when(fakeResult.isSuccess()).thenReturn(false); + + final RemoteFileLister.RemoteFolderLoader mockLoader = (RemoteFileLister.RemoteFolderLoader) fileListerUnderTest.getFolderLoader(); + + RemoteFileLister.RemoteFolderLoader mockLoaderSpied = Mockito.spy(mockLoader); + + Mockito.when(mockLoaderSpied.readRemoteFolder(remotePath, client)).thenReturn(fakeResult); + + final boolean loadSuccess = mockLoaderSpied.load(folder); + Assert.assertTrue("Loading remote folder resulting in 404 should return true", loadSuccess); + } + + @Test + public void loadDirectory_serverResponse405_shouldReturnFalse() { + final String remotePath = "remote/path/myFolder/"; + final OwnCloudClient client = DavClientProvider.getInstance().getClientInstance(account, context); + final SyncedFolder folder = new SyncedFolder("any", "local/path/myFolder", remotePath, true); + + final RemoteOperationResult fakeResult = Mockito.mock(RemoteOperationResult.class); + Mockito.when(fakeResult.getHttpCode()).thenReturn(405); + Mockito.when(fakeResult.isSuccess()).thenReturn(false); + + final RemoteFileLister.RemoteFolderLoader mockLoader = (RemoteFileLister.RemoteFolderLoader) fileListerUnderTest.getFolderLoader(); + + RemoteFileLister.RemoteFolderLoader mockLoaderSpied = Mockito.spy(mockLoader); + + Mockito.when(mockLoaderSpied.readRemoteFolder(remotePath, client)).thenReturn(fakeResult); + + final boolean loadSuccess = mockLoaderSpied.load(folder); + Assert.assertFalse("Loading remote folder resulting in 405 should return false", loadSuccess); + } + + @Test + public void loadDirectory_successAndErrorCode_shouldReturnTrue() { + final String remotePath = "remote/path/myFolder/"; + final OwnCloudClient client = DavClientProvider.getInstance().getClientInstance(account, context); + final SyncedFolder folder = new SyncedFolder("any", "local/path/myFolder", remotePath, true); + + final RemoteOperationResult fakeResult = Mockito.mock(RemoteOperationResult.class); + Mockito.when(fakeResult.getHttpCode()).thenReturn(405); + Mockito.when(fakeResult.isSuccess()).thenReturn(true); + + final ArrayList fakeData = new ArrayList<>(); + final RemoteFile mockRemoteFile = Mockito.mock(RemoteFile.class); + fakeData.add(mockRemoteFile); + Mockito.when(fakeResult.getData()).thenReturn(fakeData); + + final RemoteFileLister.RemoteFolderLoader loaderUnderTest = (RemoteFileLister.RemoteFolderLoader) fileListerUnderTest.getFolderLoader(); + + RemoteFileLister.RemoteFolderLoader spiedLoaderUnderTest = Mockito.spy(loaderUnderTest); + + Mockito.when(spiedLoaderUnderTest.readRemoteFolder(remotePath, client)).thenReturn(fakeResult); + + final boolean loadSuccess = spiedLoaderUnderTest.load(folder); + Assert.assertTrue("Loading remote folder with success despite error code (405) should return true", loadSuccess); + Assert.assertEquals("Loaded Folder is not the one expected despite loading success", mockRemoteFile, spiedLoaderUnderTest.getFolderWrapper().getFolder()); + } + + @Test + public void loadDirectory_successAndRemoteContent_shouldReturnTrue() { + final String remotePath = "remote/path/myFolder/"; + final OwnCloudClient client = DavClientProvider.getInstance().getClientInstance(account, context); + final SyncedFolder folder = new SyncedFolder("any", "local/path/myFolder", remotePath, true); + + final RemoteOperationResult fakeResult = Mockito.mock(RemoteOperationResult.class); + Mockito.when(fakeResult.getHttpCode()).thenReturn(405); + Mockito.when(fakeResult.isSuccess()).thenReturn(true); + + final ArrayList fakeData = new ArrayList<>(); + final RemoteFile mockDirRemoteFile = Mockito.mock(RemoteFile.class); + fakeData.add(mockDirRemoteFile); + final RemoteFile mockContentRemoteFile1 = Mockito.mock(RemoteFile.class); + final RemoteFile mockContentRemoteFile2 = Mockito.mock(RemoteFile.class); + fakeData.add(mockContentRemoteFile1); + fakeData.add(mockContentRemoteFile2); + Mockito.when(fakeResult.getData()).thenReturn(fakeData); + + final RemoteFileLister.RemoteFolderLoader loaderUnderTest = (RemoteFileLister.RemoteFolderLoader) fileListerUnderTest.getFolderLoader(); + + RemoteFileLister.RemoteFolderLoader spiedLoaderUnderTest = Mockito.spy(loaderUnderTest); + + Mockito.when(spiedLoaderUnderTest.readRemoteFolder(remotePath, client)).thenReturn(fakeResult); + + final boolean loadSuccess = spiedLoaderUnderTest.load(folder); + Assert.assertTrue("Loading remote folder with success despite error code (405) should return true", loadSuccess); + Assert.assertEquals("Loaded Folder is not the one expected despite loading success", mockDirRemoteFile, spiedLoaderUnderTest.getFolderWrapper().getFolder()); + Assert.assertEquals("mockDirRemoteFile should have been the directory loader but is not", 2, spiedLoaderUnderTest.getFolderWrapper().getContent().size()); + Assert.assertTrue("mockContentRemoteFile1 should have been in the result of loading but is not", spiedLoaderUnderTest.getFolderWrapper().getContent().contains(mockContentRemoteFile1)); + Assert.assertTrue("mockContentRemoteFile2 should have been in the result of loading but is not", spiedLoaderUnderTest.getFolderWrapper().getContent().contains(mockContentRemoteFile2)); + } + + + + + @Test + public void loadDirectory_successButNoData_shouldReturnFalse() { + final String remotePath = "remote/path/myFolder/"; + final OwnCloudClient client = DavClientProvider.getInstance().getClientInstance(account, context); + final SyncedFolder folder = new SyncedFolder("any", "local/path/myFolder", remotePath, true); + + final RemoteOperationResult fakeResult = Mockito.mock(RemoteOperationResult.class); + Mockito.when(fakeResult.getHttpCode()).thenReturn(204); + Mockito.when(fakeResult.isSuccess()).thenReturn(true); + + final ArrayList fakeData = new ArrayList<>(); + Mockito.when(fakeResult.getData()).thenReturn(fakeData); + + final RemoteFileLister.RemoteFolderLoader mockLoader = (RemoteFileLister.RemoteFolderLoader) fileListerUnderTest.getFolderLoader(); + + RemoteFileLister.RemoteFolderLoader mockLoaderSpied = Mockito.spy(mockLoader); + + Mockito.when(mockLoaderSpied.readRemoteFolder(remotePath, client)).thenReturn(fakeResult); + + final boolean loadSuccess = mockLoaderSpied.load(folder); + Assert.assertFalse("Loading remote folder resulting in 204 but without any RemoteFile should return false", loadSuccess); + } + +} -- GitLab From f9763705710c46e289d5e4d179fc538f5f5a2988 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Fri, 4 Nov 2022 12:13:57 +0100 Subject: [PATCH 03/12] Add unit test for LocalFileLister.java and small refactoring - Rewrite skipSyncedFolder(SynceFolder folder) method of LocalFileLister - Add test cases for skipSyncedFolder(SyncedFolder) method - Check syncedFolder not null in AbstractFileFilter - Add NonNull annotation in FileLister classes - move Abstract method to top of AbstractFileLister - Add missing if null or not null - Add test cases for localFileLister.skipDirectory() - Add test cases fpr LocalFolderLoader.loader() - Update LocalFileLister to make LocalFolderLoader available for test --- .../contentScanner/AbstractFileLister.java | 55 ++-- .../e/drive/contentScanner/FolderWrapper.java | 19 +- .../drive/contentScanner/LocalFileLister.java | 52 ++-- .../contentScanner/RemoteFileLister.java | 31 ++- .../contentScanner/LocalFileListerTest.java | 241 +++++++++++++++++- 5 files changed, 338 insertions(+), 60 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java b/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java index 34dfe74b..71745489 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java @@ -9,6 +9,8 @@ package foundation.e.drive.contentScanner; import android.content.Context; +import androidx.annotation.NonNull; + import java.util.ArrayList; import java.util.List; import java.util.ListIterator; @@ -25,14 +27,29 @@ import timber.log.Timber; */ public abstract class AbstractFileLister { + protected abstract boolean skipSyncedFolder(@NonNull SyncedFolder syncedFolder); + protected abstract boolean skipFile(@NonNull T file); + + /** + * If the given folder has not changed, compared to database's data, skip it + * @param currentDirectory + * @param syncedFolder + * @param context + * @return + */ + protected abstract boolean skipDirectory(@NonNull T currentDirectory, @NonNull SyncedFolder syncedFolder, @NonNull Context context); + protected abstract boolean isDirectory(@NonNull T file); + protected abstract String getFileName(@NonNull T file); + protected abstract FolderLoader getFolderLoader(); + /** * This allows to use RemoteOperationResult for RemoteFileLister - * todo: some optimization may be done with FolderWrapper + * todo: some optimization could be done with FolderWrapper * @param RemoteFile or File */ /* package */ interface FolderLoader { /* package */ FolderWrapper getFolderWrapper(); - /* package */ boolean load(SyncedFolder folder); + /* package */ boolean load(@NonNull SyncedFolder folder); } protected final List folders; @@ -42,7 +59,7 @@ public abstract class AbstractFileLister { * constructor to be call with super * @param folders List of SyncedFolder to read */ - protected AbstractFileLister(List folders) { + protected AbstractFileLister(@NonNull List folders) { this.folders = folders; this.contentToScan = new ArrayList<>(); } @@ -52,12 +69,12 @@ public abstract class AbstractFileLister { * @param context * @return */ - public boolean listContentToScan(Context context) { - //For each SyncedFolder + public boolean listContentToScan(@NonNull Context context) { final ListIterator iterator = folders.listIterator() ; boolean isSomeContentToSync = false; while (iterator.hasNext()) { final SyncedFolder syncedFolder = iterator.next(); + if (syncedFolder == null) continue; Timber.v("SyncedFolder : %s, %s, %s, %s, %s", syncedFolder.getLibelle(), syncedFolder.getLocalFolder(), syncedFolder.getLastModified(), syncedFolder.isScanLocal(), syncedFolder.getId()); isSomeContentToSync = isSomeContentToSync || hasDirectoryContentToScan(syncedFolder, iterator, context); } @@ -68,7 +85,6 @@ public abstract class AbstractFileLister { return isSomeContentToSync; } - /** * Detailed process for reading a single folder * @param syncedFolder the SyncedFolder to check @@ -76,8 +92,8 @@ public abstract class AbstractFileLister { * @param context context used to call DB * @return true the directory has content to synchronized false otherwise */ - private boolean hasDirectoryContentToScan( SyncedFolder syncedFolder, ListIterator iterator, Context context) { - // I check if I can skip it; If then then go to next + private boolean hasDirectoryContentToScan(@NonNull SyncedFolder syncedFolder, @NonNull ListIterator iterator, @NonNull Context context) { + final FolderLoader dirLoader = getFolderLoader(); if (skipSyncedFolder(syncedFolder) || !isSyncedFolderInDb(syncedFolder, context) @@ -88,23 +104,19 @@ public abstract class AbstractFileLister { final FolderWrapper currentDirectory = dirLoader.getFolderWrapper(); - // If directory is missing (not exist or http 404) if (currentDirectory.isMissing()) { return true; } - //--> I no content has changed, skip if (skipDirectory(currentDirectory.getFolder(), syncedFolder, context)) { iterator.remove(); return false; } - //else I have to check if some content has changed inside - final List dirContent = currentDirectory.getContent(); - //todo: look where to put in subclasses : syncedFolder.setLastEtag(directory.getEtag()).setToSync(true); - if (!dirContent.isEmpty()) { + final List dirContent = currentDirectory.getContent(); + if (dirContent != null && !dirContent.isEmpty()) { contentToScan.addAll(sortContent(iterator, syncedFolder, dirContent)); } return true; @@ -118,7 +130,7 @@ public abstract class AbstractFileLister { * @param context context to contact database * @return false if not persisted in db */ - private boolean isSyncedFolderInDb(SyncedFolder syncedFolder, Context context) { + private boolean isSyncedFolderInDb(@NonNull SyncedFolder syncedFolder, @NonNull Context context) { if (syncedFolder.getId() > 0) return true; final int syncedFolder_id = (int) DbHelper.insertSyncedFolder(syncedFolder, context); //It will return -1 if there is an error, like an already existing folder with same value @@ -140,11 +152,13 @@ public abstract class AbstractFileLister { * @param dirContent Content to sort * @return List of subfiles to scan or empty list if nothing */ - private List sortContent(ListIterator iterator, SyncedFolder syncedFolder, List dirContent) { + private List sortContent(@NonNull ListIterator iterator, @NonNull SyncedFolder syncedFolder, List dirContent) { final List result = new ArrayList<>(); if (dirContent == null) return result; for (T subFile : dirContent) { + if (subFile == null) continue; + final String fileName = getFileName(subFile); if (isDirectory(subFile)) { final SyncedFolder subSyncedFolder = new SyncedFolder(syncedFolder, fileName, 0L, "");//Need to set lastModified to 0 to handle it on next iteration @@ -180,13 +194,4 @@ public abstract class AbstractFileLister { } return result; } - - - - protected abstract boolean skipSyncedFolder(SyncedFolder syncedFolder); - protected abstract boolean skipFile(T file); - protected abstract boolean skipDirectory(T currentDirectory, SyncedFolder syncedFolder, Context context); - protected abstract boolean isDirectory(T file); - protected abstract String getFileName(T file); - protected abstract FolderLoader getFolderLoader(); } diff --git a/app/src/main/java/foundation/e/drive/contentScanner/FolderWrapper.java b/app/src/main/java/foundation/e/drive/contentScanner/FolderWrapper.java index ca273584..6fb4d307 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/FolderWrapper.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/FolderWrapper.java @@ -7,6 +7,8 @@ */ package foundation.e.drive.contentScanner; +import androidx.annotation.NonNull; + import java.util.ArrayList; import java.util.List; @@ -25,7 +27,7 @@ public class FolderWrapper { private final T folder; private final List content; - protected FolderWrapper(T folder) { + protected FolderWrapper(@NonNull T folder) { content = new ArrayList<>(); this.folder = folder; missing = false; @@ -37,18 +39,31 @@ public class FolderWrapper { content = null; } + /** + * Get the folder (should be RemoteFile or File instance ) represented by this abstraction + * @return null if it is missing + */ public T getFolder() { return folder; } + /** + * Get subfiles (including sub folder) of the current folder + * @return Null if the directory is missing + */ public List getContent() { return content; } - public void addContent(List newContent) { + public void addContent(@NonNull List newContent) { content.addAll(newContent); } + /** + * Should return true if local File for directory doesn't exists, or if ReadFolderRemoteOperation return 404 + * for a remote file + * @return + */ public boolean isMissing() { return missing; } diff --git a/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java b/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java index a37fcf00..7e3caa0a 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java @@ -9,6 +9,9 @@ package foundation.e.drive.contentScanner; import android.content.Context; +import androidx.annotation.NonNull; +import androidx.annotation.VisibleForTesting; + import java.io.File; import java.io.FileFilter; import java.util.Arrays; @@ -17,7 +20,6 @@ import java.util.List; import foundation.e.drive.database.DbHelper; import foundation.e.drive.fileFilters.FileFilterFactory; import foundation.e.drive.models.SyncedFolder; -import foundation.e.drive.utils.CommonUtils; import timber.log.Timber; /** @@ -25,47 +27,45 @@ import timber.log.Timber; * @author vincent Bourgmayer */ public class LocalFileLister extends AbstractFileLister { - public LocalFileLister(List directories) { + + public LocalFileLister(@NonNull List directories) { super(directories); } @Override - protected boolean skipDirectory(File currentDirectory, SyncedFolder syncedFolder, Context context) { + protected boolean skipDirectory(@NonNull File currentDirectory, @NonNull SyncedFolder syncedFolder, @NonNull Context context) { return currentDirectory.lastModified() == syncedFolder.getLastModified() && !DbHelper.syncedFolderHasContentToUpload(syncedFolder.getId(), context); } @Override - protected boolean skipSyncedFolder(SyncedFolder syncedFolder) { - final String fileName = CommonUtils.getFileNameFromPath(syncedFolder.getLocalFolder()); - if (fileName == null || fileName.startsWith(".") || !syncedFolder.isScanLocal()) { - return true; - } - - return false; + protected boolean skipSyncedFolder(@NonNull SyncedFolder syncedFolder) { + final File folder = new File(syncedFolder.getLocalFolder()); + return (syncedFolder.isMediaType() && folder.isHidden()) || !syncedFolder.isScanLocal(); } @Override - protected boolean skipFile(File file) { + protected boolean skipFile(@NonNull File file) { return file.isHidden(); } @Override - protected boolean isDirectory(File file) { + protected boolean isDirectory(@NonNull File file) { return file.isDirectory(); } @Override - protected String getFileName(File file) { + protected String getFileName(@NonNull File file) { return file.getName(); } @Override protected FolderLoader getFolderLoader() { - return new LocalDirLoader(); + return new LocalFolderLoader(); } - private final class LocalDirLoader implements FolderLoader { + @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) + public class LocalFolderLoader implements FolderLoader { private FolderWrapper folder; @Override @@ -73,19 +73,31 @@ public class LocalFileLister extends AbstractFileLister { return folder; } + + /** + * "Load" a directory to check if it exist, and list its content + * It is expected to always return true. Its remote equivalent (in RemoteFileLister) + * Might return false because of network, & RemoteFile classes. But that's not the case + * here. + * @param syncedFolder + * @return always true because it's about "File" instance. + */ @Override - public boolean load(SyncedFolder syncedFolder) { + public boolean load(@NonNull SyncedFolder syncedFolder) { final File dir = new File(syncedFolder.getLocalFolder()); - Timber.v("Local Folder (last modified / exists): %s, %s", dir.lastModified(),dir.exists()); + Timber.v("Local Folder (last modified / exists): %s, %s", dir.lastModified(), dir.exists()); if (!dir.exists()) { folder = new FolderWrapper(); //missing Folder Wrapper - } - else { + } else { folder = new FolderWrapper(dir); final String category = syncedFolder.isMediaType() ? "media" : syncedFolder.getLibelle(); + final FileFilter filter = FileFilterFactory.getFileFilter(category); - folder.addContent(Arrays.asList(dir.listFiles(filter))); + final File[] subFiles = dir.listFiles(filter); + if (subFiles != null) { + folder.addContent(Arrays.asList(subFiles)); + } } return true; } diff --git a/app/src/main/java/foundation/e/drive/contentScanner/RemoteFileLister.java b/app/src/main/java/foundation/e/drive/contentScanner/RemoteFileLister.java index eec94bf9..1e0b6e09 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/RemoteFileLister.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/RemoteFileLister.java @@ -9,6 +9,7 @@ package foundation.e.drive.contentScanner; import android.content.Context; +import androidx.annotation.NonNull; import androidx.annotation.VisibleForTesting; import com.owncloud.android.lib.common.OwnCloudClient; @@ -31,37 +32,37 @@ import foundation.e.drive.utils.CommonUtils; public class RemoteFileLister extends AbstractFileLister { private OwnCloudClient client; - public RemoteFileLister(List directories, OwnCloudClient client) { + public RemoteFileLister(@NonNull List directories, @NonNull OwnCloudClient client) { super(directories); this.client = client; } @Override - protected boolean skipSyncedFolder(SyncedFolder syncedFolder) { + protected boolean skipSyncedFolder(@NonNull SyncedFolder syncedFolder) { return (syncedFolder.isMediaType() && CommonUtils.getFileNameFromPath(syncedFolder.getRemoteFolder()).startsWith(".")) || !syncedFolder.isScanRemote(); } @Override - protected boolean skipFile(RemoteFile file) { + protected boolean skipFile(@NonNull RemoteFile file) { final String fileName = CommonUtils.getFileNameFromPath(file.getRemotePath()); return fileName == null || fileName.isEmpty() || fileName.startsWith("."); } @Override - protected boolean skipDirectory(RemoteFile directory, SyncedFolder syncedFolder, Context context) { + protected boolean skipDirectory(@NonNull RemoteFile directory, @NonNull SyncedFolder syncedFolder, @NonNull Context context) { return directory.getEtag().equals(syncedFolder.getLastEtag()) && !DbHelper.syncedFolderHasContentToDownload(syncedFolder.getId(), context); } @Override - protected boolean isDirectory(RemoteFile file) { + protected boolean isDirectory(@NonNull RemoteFile file) { return file.getMimeType().equals("DIR"); } @Override - protected String getFileName(RemoteFile file) { + protected String getFileName(@NonNull RemoteFile file) { return CommonUtils.getFileNameFromPath(file.getRemotePath()); } @@ -76,11 +77,11 @@ public class RemoteFileLister extends AbstractFileLister { private FolderWrapper directory; @Override - public boolean load(SyncedFolder syncedFolder) { + public boolean load(@NonNull SyncedFolder syncedFolder) { + if (syncedFolder.getRemoteFolder() == null) return false; final RemoteOperationResult remoteOperationResult = readRemoteFolder(syncedFolder.getRemoteFolder(), client); - System.out.println("succeed: " + remoteOperationResult.isSuccess()); - if (!remoteOperationResult.isSuccess()) { + if (!remoteOperationResult.isSuccess()) { if (remoteOperationResult.getHttpCode() == HTTP_404) { directory = new FolderWrapper<>(); return true; @@ -92,11 +93,17 @@ public class RemoteFileLister extends AbstractFileLister { if (datas == null || datas.size() < 1) return false; final int dataSize = datas.size(); - directory = new FolderWrapper<>( (RemoteFile) datas.get(0)); + final RemoteFile remoteDir = (RemoteFile) datas.get(0); + if (remoteDir == null) return false; + + directory = new FolderWrapper<>(remoteDir); final List subFiles = new ArrayList<>(); for (Object o: datas.subList(1, dataSize)) { - subFiles.add((RemoteFile) o); + final RemoteFile subFile = (RemoteFile) o; + if (subFile != null) { + subFiles.add(subFile); + } } directory.addContent(subFiles); return true; @@ -109,7 +116,7 @@ public class RemoteFileLister extends AbstractFileLister { * @return RemoteOperationResult */ @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) - public RemoteOperationResult readRemoteFolder(String remotePath, OwnCloudClient client) { + public RemoteOperationResult readRemoteFolder(@NonNull String remotePath, @NonNull OwnCloudClient client) { final ReadFolderRemoteOperation operation = new ReadFolderRemoteOperation(remotePath); return operation.execute(client); } diff --git a/app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java b/app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java index fe37fd75..ed3cefc0 100644 --- a/app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java +++ b/app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java @@ -1,2 +1,241 @@ -package foundation.e.drive.contentScanner;public class LocalFileListerTest { +/* + * 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.accounts.Account; +import android.accounts.AccountManager; +import android.content.Context; +import android.database.sqlite.SQLiteDatabase; +import android.os.Build; + +import androidx.test.core.app.ApplicationProvider; + +import com.owncloud.android.lib.common.OwnCloudClient; +import com.owncloud.android.lib.common.operations.RemoteOperationResult; + +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.io.IOException; + +import foundation.e.drive.TestUtils; +import foundation.e.drive.database.DbHelper; +import foundation.e.drive.models.SyncedFileState; +import foundation.e.drive.models.SyncedFolder; +import foundation.e.drive.utils.DavClientProvider; + +/** + * @author vincent Bourgmayer + */ +@RunWith(RobolectricTestRunner.class) +@Config(sdk = Build.VERSION_CODES.O, manifest = Config.NONE) +public class LocalFileListerTest { + + private LocalFileLister fileListerUnderTest; + private final Account account; + private final OwnCloudClient ocClient; + private final Context context; + + public LocalFileListerTest() { + context = ApplicationProvider.getApplicationContext(); + ShadowLog.stream = System.out; + TestUtils.loadServerCredentials(); + TestUtils.prepareValidAccount(AccountManager.get(context)); + account = TestUtils.getValidAccount(); + ocClient = DavClientProvider.getInstance().getClientInstance(account, context); + } + + @Before + public void setUp() { + fileListerUnderTest = new LocalFileLister(null); + SQLiteDatabase.deleteDatabase(context.getDatabasePath(DbHelper.DATABASE_NAME)); + } + + @Test + public void skipSyncedFolder_notMedia_notHidden_notScannable_shouldReturnTrue() { + final SyncedFolder folder = buildSyncedFolderForSkipSyncedFolderTest(false, false, false); + Assert.assertTrue(fileListerUnderTest.skipSyncedFolder(folder)); + } + + @Test + public void skipSyncedFolder_notMedia_notHidden_scannable_shouldReturnFalse() { + final SyncedFolder folder = buildSyncedFolderForSkipSyncedFolderTest(false, false, true); + Assert.assertFalse(fileListerUnderTest.skipSyncedFolder(folder)); + } + + @Test + public void skipSyncedFolder_notMedia_hidden_notScannable_shouldReturnTrue() { + final SyncedFolder folder = buildSyncedFolderForSkipSyncedFolderTest(false, true, false); + Assert.assertTrue(fileListerUnderTest.skipSyncedFolder(folder)); + } + + @Test + public void skipSyncedFolder_notMedia_hidden_scannable_shouldReturnFalse() { + final SyncedFolder folder = buildSyncedFolderForSkipSyncedFolderTest(false, true, true); + Assert.assertFalse(fileListerUnderTest.skipSyncedFolder(folder)); + } + + @Test + public void skipSyncedFolder_media_notHidden_notScannable_shouldReturnTrue() { + final SyncedFolder folder = buildSyncedFolderForSkipSyncedFolderTest(true, false, false); + Assert.assertTrue(fileListerUnderTest.skipSyncedFolder(folder)); + } + + @Test + public void skipSyncedFolder_media_notHidden_scannable_shouldReturnFalse() { + final SyncedFolder folder = buildSyncedFolderForSkipSyncedFolderTest(true, false, true); + Assert.assertFalse("folder shouldn't be skip but it had", fileListerUnderTest.skipSyncedFolder(folder)); + } + + @Test + public void skipSyncedFolder_media_Hidden_notScannable_shouldReturnTrue() { + final SyncedFolder folder = buildSyncedFolderForSkipSyncedFolderTest(true, true, false); + Assert.assertTrue(fileListerUnderTest.skipSyncedFolder(folder)); + } + + @Test + public void skipSyncedFolder_media_Hidden_scannable_shouldReturnTrue() { + final SyncedFolder folder = buildSyncedFolderForSkipSyncedFolderTest(true, true, true); + Assert.assertTrue(fileListerUnderTest.skipSyncedFolder(folder)); + } + + private SyncedFolder buildSyncedFolderForSkipSyncedFolderTest(boolean isMedia, boolean isHidden, boolean isLocalScannable) { + final String dirName = isHidden ? ".myFolder/" : "myFolder/"; + return new SyncedFolder("any", "/local/path/" + dirName, "/remote/path/" + dirName, isLocalScannable, true, true, isMedia); + } + + @Test + public void skipDirectory_lastModifiedSame_noUnsyncedContent_shouldReturnTrue() { + final long lastModified = 123456L; + final File directory = Mockito.spy(new File("/local/path/test/")); + Mockito.when(directory.lastModified()).thenReturn(lastModified); + + final SyncedFolder syncedFolder = new SyncedFolder("any", "/local/path/test/", "/remote/path/test/", true, true, true, true); + syncedFolder.setId(12); + syncedFolder.setLastModified(lastModified); + + final boolean skip = fileListerUnderTest.skipDirectory(directory, syncedFolder, context); + Assert.assertTrue("directory without new last modified and no content waiting for sync has not been skip", skip); + } + + @Test + public void skipDirectory_lastModifiedSame_unsyncedContent_shouldReturnFalse() { + final long lastModified = 123456L; + final File directory = Mockito.spy(new File("/local/path/test/")); + Mockito.when(directory.lastModified()).thenReturn(lastModified); + + final SyncedFileState dummy = new SyncedFileState(6, "tutu.jpg", "/local/path/test/tutu.jpg", "/remote/path/test/tutu.jpg", "", 0L, 12, true, 3); + DbHelper.manageSyncedFileStateDB(dummy, "INSERT", context); + + final SyncedFolder syncedFolder = new SyncedFolder("any", "/local/path/test/" , "/remote/path/test/", true, true, true, true); + syncedFolder.setId(12); + syncedFolder.setLastModified(lastModified); + + final boolean skip = fileListerUnderTest.skipDirectory(directory, syncedFolder, context); + Assert.assertFalse("directory without new last modified but content waiting for sync has been skip", skip); + } + + @Test + public void skipDirectory_lastModifiedChanged_noUnsyncedContent_shouldReturnFalse() { + final File directory = Mockito.spy(new File("/local/path/test/")); + Mockito.when(directory.lastModified()).thenReturn(654321L); + + final SyncedFolder syncedFolder = new SyncedFolder("any", "/local/path/test/", "/remote/path/test/", true, true, true, true); + syncedFolder.setId(12); + syncedFolder.setLastModified(123456L); + + final boolean skip = fileListerUnderTest.skipDirectory(directory, syncedFolder, context); + Assert.assertFalse("directory with new last modified and no content waiting for sync has been skip", skip); + } + + @Test + public void skipDirectory_lastModifiedChanged_unsyncedContent_shouldReturnFalse() { + final File directory = Mockito.spy(new File("/local/path/test/")); + Mockito.when(directory.lastModified()).thenReturn(654321L); + + final SyncedFolder syncedFolder = new SyncedFolder("any", "/local/path/test/", "/remote/path/test/", true, true, true, true); + syncedFolder.setId(12); + syncedFolder.setLastModified(123456L); + + final SyncedFileState dummy = new SyncedFileState(6, "tutu.jpg", "/local/path/test/tutu.jpg", "/remote/path/test/tutu.jpg", "", 0L, 12, true, 3); + DbHelper.manageSyncedFileStateDB(dummy, "INSERT", context); + + final boolean skip = fileListerUnderTest.skipDirectory(directory, syncedFolder, context); + Assert.assertFalse("directory with new last modified and content waiting for sync has been skip", skip); + } + + //Test about inner class: DirectoryLoader + + + @Test + public void loadDirectory_dirNotExist_shouldReturnTrue() { + final SyncedFolder folder = new SyncedFolder("any", "/local/path/myFolder", "/remote/path/myFolder", true); + + final LocalFileLister.LocalFolderLoader folderLoader = (LocalFileLister.LocalFolderLoader) fileListerUnderTest.getFolderLoader(); + + final boolean loadSuccess = folderLoader.load(folder); + Assert.assertTrue("Loading remote folder resulting in 404 should return true", loadSuccess); + Assert.assertTrue(folderLoader.getFolderWrapper().isMissing()); + } + + @Test + public void loadDirectory_dirExist_noContent_shouldReturnTrue() { + final SyncedFolder folder = new SyncedFolder("any", "../local/path/myFolder/", "/remote/path/myFolder/", true); + final File file = new File(folder.getLocalFolder()); + final boolean dirCreationSucceed = file.mkdirs(); + + Assert.assertTrue("cannot create test folder: " + file.getAbsolutePath(), dirCreationSucceed); + + final LocalFileLister.LocalFolderLoader folderLoader = (LocalFileLister.LocalFolderLoader) fileListerUnderTest.getFolderLoader(); + + final boolean loadSuccess = folderLoader.load(folder); + final FolderWrapper wrapper = folderLoader.getFolderWrapper(); + Assert.assertTrue("Loading remote folder resulting in 404 should return true", loadSuccess); + Assert.assertFalse("dir shouldExist!", wrapper.isMissing()); + Assert.assertTrue("List of folderWrapper's content expected to be empty but contains: "+wrapper.getContent().size(), wrapper.getContent().isEmpty()); + //@todo beware, in case of previous assertion failure, the below code won't be executed + Assert.assertTrue("Test file has not been successfully removed: " + file.getAbsolutePath(), file.delete()); + } + + @Test + public void loadDirectory_dirExist_withSubContent_shouldReturnTrue() { + final SyncedFolder folder = new SyncedFolder("any", "../local/path/myFolder/", "/remote/path/myFolder/", true); + final File folderFile = new File(folder.getLocalFolder()); + final boolean dirCreationSucceed = folderFile.mkdirs(); + + final File subContent = new File(folderFile, "toto.txt"); + try { + subContent.createNewFile(); + } catch (IOException exception) { + exception.printStackTrace(); + Assert.fail("IO Exception: Can't create new file as content of test folder"); + } + + Assert.assertTrue("cannot create test folder: " + folderFile.getAbsolutePath(), dirCreationSucceed); + + final LocalFileLister.LocalFolderLoader folderLoader = (LocalFileLister.LocalFolderLoader) fileListerUnderTest.getFolderLoader(); + + final boolean loadSuccess = folderLoader.load(folder); + final FolderWrapper wrapper = folderLoader.getFolderWrapper(); + Assert.assertTrue("Loading remote folder resulting in 404 should return true", loadSuccess); + Assert.assertFalse("dir shouldExist!", wrapper.isMissing()); + Assert.assertEquals("List of folderWrapper's content size expected to one but was: "+wrapper.getContent().size(), 1, wrapper.getContent().size()); + //@todo beware, in case of previous assertion failure, the below code won't be executed + Assert.assertTrue("SubFile has not been removed", subContent.delete()); + Assert.assertTrue("Test folder has not been removed: " + folderFile.getAbsolutePath(), folderFile.delete()); + } } -- GitLab From 4e6b9f9636667b25641ab3a97c46f1641845a8db Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Thu, 10 Nov 2022 12:10:34 +0100 Subject: [PATCH 04/12] * LocalFileListerTest: - add a test for the listContentToScan() - add a @After section to remove fake file generated for test * AbstractFileLister: - fix a boolean assignement that generate errors - Add a new boolean to define if we can dive into a skipped directory to list content of subDirectory * LocalFileLister: - overriden boolean to dive into subContent of a skipped Directory --- .../contentScanner/AbstractFileLister.java | 16 +- .../drive/contentScanner/LocalFileLister.java | 3 +- .../contentScanner/LocalFileListerTest.java | 156 ++++++++++++++++++ 3 files changed, 171 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java b/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java index 71745489..7168b1fd 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java @@ -54,6 +54,7 @@ public abstract class AbstractFileLister { protected final List folders; private final List contentToScan; + protected boolean diveIntoSkippedDir = false; //to be overriden only /** * constructor to be call with super @@ -76,7 +77,7 @@ public abstract class AbstractFileLister { final SyncedFolder syncedFolder = iterator.next(); if (syncedFolder == null) continue; Timber.v("SyncedFolder : %s, %s, %s, %s, %s", syncedFolder.getLibelle(), syncedFolder.getLocalFolder(), syncedFolder.getLastModified(), syncedFolder.isScanLocal(), syncedFolder.getId()); - isSomeContentToSync = isSomeContentToSync || hasDirectoryContentToScan(syncedFolder, iterator, context); + isSomeContentToSync = hasDirectoryContentToScan(syncedFolder, iterator, context) || isSomeContentToSync; } if (isSomeContentToSync) { @@ -110,9 +111,18 @@ public abstract class AbstractFileLister { if (skipDirectory(currentDirectory.getFolder(), syncedFolder, context)) { iterator.remove(); - return false; + syncedFolder.setToSync(false); + /** + * LocalFileLister need to be able to access subFiles even for skipped directory + * because A folder's lastModified value won't change if a subfolder's content is changed, removed or created + * RemoteFileLister is based on eTag, which change even if a subfolder's content is changed + */ + if (!diveIntoSkippedDir) return false; + } else { + syncedFolder.setToSync(true); } + //todo: look where to put in subclasses : syncedFolder.setLastEtag(directory.getEtag()).setToSync(true); final List dirContent = currentDirectory.getContent(); @@ -164,7 +174,7 @@ public abstract class AbstractFileLister { final SyncedFolder subSyncedFolder = new SyncedFolder(syncedFolder, fileName, 0L, "");//Need to set lastModified to 0 to handle it on next iteration iterator.add(subSyncedFolder); iterator.previous(); - } else if (!skipFile(subFile)) { + } else if (syncedFolder.isToSync() && !skipFile(subFile)) { Timber.v("added %s into list of file to scan", fileName); result.add(subFile); } diff --git a/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java b/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java index 7e3caa0a..785fafa7 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java @@ -30,6 +30,7 @@ public class LocalFileLister extends AbstractFileLister { public LocalFileLister(@NonNull List directories) { super(directories); + super.diveIntoSkippedDir = true; } @Override @@ -56,7 +57,7 @@ public class LocalFileLister extends AbstractFileLister { @Override protected String getFileName(@NonNull File file) { - return file.getName(); + return "/" + file.getName(); } @Override diff --git a/app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java b/app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java index ed3cefc0..951478a6 100644 --- a/app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java +++ b/app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java @@ -19,6 +19,7 @@ import androidx.test.core.app.ApplicationProvider; import com.owncloud.android.lib.common.OwnCloudClient; import com.owncloud.android.lib.common.operations.RemoteOperationResult; +import org.junit.After; import org.junit.Assert; import org.junit.Before; import org.junit.Test; @@ -31,6 +32,8 @@ import org.robolectric.shadows.ShadowLog; import java.io.File; import java.io.IOException; +import java.util.ArrayList; +import java.util.List; import foundation.e.drive.TestUtils; import foundation.e.drive.database.DbHelper; @@ -45,6 +48,8 @@ import foundation.e.drive.utils.DavClientProvider; @Config(sdk = Build.VERSION_CODES.O, manifest = Config.NONE) public class LocalFileListerTest { + private static final String TEST_FILE_TREE_ROOT_PATH = "../tmp/"; + private LocalFileLister fileListerUnderTest; private final Account account; private final OwnCloudClient ocClient; @@ -62,7 +67,17 @@ public class LocalFileListerTest { @Before public void setUp() { fileListerUnderTest = new LocalFileLister(null); + } + + + @After + public void tearDown() { SQLiteDatabase.deleteDatabase(context.getDatabasePath(DbHelper.DATABASE_NAME)); + + final File rootDirectory = new File(TEST_FILE_TREE_ROOT_PATH); + if (rootDirectory.exists()) { + deleteDirectory(rootDirectory); + } } @Test @@ -238,4 +253,145 @@ public class LocalFileListerTest { Assert.assertTrue("SubFile has not been removed", subContent.delete()); Assert.assertTrue("Test folder has not been removed: " + folderFile.getAbsolutePath(), folderFile.delete()); } + + + /** + * Run on a more realistic files tree to check the whole behaviour with more than + * one directory: + * + * It should contains : + * - one directory with a subdirectory with some files in both + * - one hidden directory with some files + * - one empty directory + * - one "new" directory with subfiles + * + * with correspond syncedFolder in database: + * - except for one, so it will considered as new + * Something like: + * + * ## File tree + * >Folder 1 (unChanged) + * --> .File 1 (should be ignored) + * --> File 2 (should be ignored) + * --> Folder 2 (unsynced dir) + * ----> File 3 (new file should be added) + * ----> .File 4 (should be ignored) + * >Folder 3 (unChanged) + * -->Folder4 (new dir (so unsynced) + * ----> File 5 (new file should be added) + * -->.Folder5 (skip: hidden dir) + * ----> File 6 (skip because directory is hidden) + * + * + * ## SyncedFolder + * SyncedFolder 1 + * SyncedFolder 2 with empty lastModified + * SyncedFolder 3 + * SyncedFolderRemoved which correspond to a missing directory + */ + @Test + public void bigWholeTest() { + Assert.assertTrue("Preparation, of file Tree and syncedFolder has failed", prepareFakeContent()); + Assert.assertTrue("File listing failed", fileListerUnderTest.listContentToScan(context)); + + final List foldersId = fileListerUnderTest.getSyncedFoldersId(); + Assert.assertEquals("List of folders to scan's id should contains 3 but contained: " + foldersId.size(), 3, foldersId.size()); + Assert.assertTrue(foldersId.contains(2L)); //id of changed folder detected + Assert.assertTrue(foldersId.contains(5L)); //id of new folder detected + Assert.assertTrue(foldersId.contains(4L)); //id of Removed Folder detected + final List contentListed = fileListerUnderTest.getContentToScan(); + Assert.assertEquals("ContentListed should have 2 files but contained: " + contentListed.size(), 2, contentListed.size()); + } + + private boolean prepareFakeContent() { + /** + * Generate folders + */ + final File rootDirectory = new File(TEST_FILE_TREE_ROOT_PATH); + if (!rootDirectory.mkdirs()) return false; + + final File folder1 = new File(rootDirectory, "folder1/"); + if (!folder1.mkdirs()) return false; + final File folder2 = new File(folder1, "folder2/"); + if (!folder2.mkdirs()) return false; + + final File folder3 = new File(rootDirectory, "folder3/"); + if (!folder3.mkdirs()) return false; + final File folder4 = new File(folder3, "folder4/"); + if (!folder4.mkdirs()) return false; + final File folder5 = new File(folder3, ".folder5/"); + if (!folder5.mkdirs()) return false; + + + /** + * Generate files + * /!\ must be done before syncedFolders generation + * otherwise lastModified value of directory will change due to file creation + */ + try { + final File file1 = new File(folder1, ".file1.txt"); + if (!file1.createNewFile()) return false; + + final File file2 = new File(folder1, "file2.txt"); + if (!file2.createNewFile()) return false; + + final File file3 = new File(folder2, "file3.txt"); + if (!file3.createNewFile()) return false; + + final File file4 = new File(folder2, ".file4.txt"); + if (!file4.createNewFile()) return false; + + final File file5 = new File(folder4, "file5.txt"); + if (!file5.createNewFile()) return false; + + final File file6 = new File(folder5, "file6.txt"); + if (!file6.createNewFile()) return false; + } catch (IOException exception) { + exception.printStackTrace(); + return false; + } + + + /** + * Generate SyncedFolders + */ + final SyncedFolder syncedFolder1 = new SyncedFolder("any", folder1.getAbsolutePath(), "/remote/path/myFolder1/", true); + syncedFolder1.setLastModified(folder1.lastModified()); + syncedFolder1.setId((int) DbHelper.insertSyncedFolder(syncedFolder1, context)); + final SyncedFolder syncedFolder2 = new SyncedFolder("any", folder2.getAbsolutePath(), "/remote/path/myFolder2/", true); + syncedFolder2.setLastModified(0L); + syncedFolder2.setId((int) DbHelper.insertSyncedFolder(syncedFolder2, context)); + final SyncedFolder syncedFolder3 = new SyncedFolder("any", folder3.getAbsolutePath(), "/remote/path/myFolder3/", true); + syncedFolder3.setLastModified(folder3.lastModified()); + syncedFolder3.setId((int) DbHelper.insertSyncedFolder(syncedFolder3, context)); + final SyncedFolder syncedFolderRemoved = new SyncedFolder("any", rootDirectory.getAbsolutePath()+"lambda/", "/remote/lambda", true); + syncedFolderRemoved.setLastModified(987654321L); + syncedFolderRemoved.setLastEtag("123456789"); + syncedFolderRemoved.setId((int) DbHelper.insertSyncedFolder(syncedFolderRemoved, context)); + + final ArrayList syncedFolders = new ArrayList<>(); + syncedFolders.add(syncedFolder1); + syncedFolders.add(syncedFolder2); + syncedFolders.add(syncedFolder3); + syncedFolders.add(syncedFolderRemoved); + fileListerUnderTest = new LocalFileLister(syncedFolders); + + return true; + } + + + + + private void deleteDirectory(File file) + { + for (File subfile : file.listFiles()) { + + if (subfile.isDirectory()) { + deleteDirectory(subfile); + } + subfile.delete(); + } + file.delete(); + } + } -- GitLab From 88e7f79d8a46cdc79f22dc867807587dfc91eeff Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Tue, 15 Nov 2022 12:23:59 +0100 Subject: [PATCH 05/12] move method to delete files for test into TestUtils --- .../java/foundation/e/drive/TestUtils.java | 18 ++++++++++++++++++ .../contentScanner/LocalFileListerTest.java | 16 ++-------------- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/app/src/test/java/foundation/e/drive/TestUtils.java b/app/src/test/java/foundation/e/drive/TestUtils.java index 1da09d5d..19861db8 100644 --- a/app/src/test/java/foundation/e/drive/TestUtils.java +++ b/app/src/test/java/foundation/e/drive/TestUtils.java @@ -155,6 +155,24 @@ public abstract class TestUtils { } + /** + * Delete a local directory with all its content. + * Adapted from https://www.geeksforgeeks.org/java-program-to-delete-a-directory/ + * @param file + */ + public static void deleteDirectory(File file) + { + for (File subfile : file.listFiles()) { + + if (subfile.isDirectory()) { + deleteDirectory(subfile); + } + subfile.delete(); + } + file.delete(); + } + + public static void initializeWorkmanager(Context context) { final Configuration config = new Configuration.Builder() .setMinimumLoggingLevel(Log.DEBUG) diff --git a/app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java b/app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java index 951478a6..d3211065 100644 --- a/app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java +++ b/app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java @@ -17,7 +17,6 @@ import android.os.Build; import androidx.test.core.app.ApplicationProvider; import com.owncloud.android.lib.common.OwnCloudClient; -import com.owncloud.android.lib.common.operations.RemoteOperationResult; import org.junit.After; import org.junit.Assert; @@ -51,7 +50,6 @@ public class LocalFileListerTest { private static final String TEST_FILE_TREE_ROOT_PATH = "../tmp/"; private LocalFileLister fileListerUnderTest; - private final Account account; private final OwnCloudClient ocClient; private final Context context; @@ -60,7 +58,7 @@ public class LocalFileListerTest { ShadowLog.stream = System.out; TestUtils.loadServerCredentials(); TestUtils.prepareValidAccount(AccountManager.get(context)); - account = TestUtils.getValidAccount(); + final Account account = TestUtils.getValidAccount(); ocClient = DavClientProvider.getInstance().getClientInstance(account, context); } @@ -76,7 +74,7 @@ public class LocalFileListerTest { final File rootDirectory = new File(TEST_FILE_TREE_ROOT_PATH); if (rootDirectory.exists()) { - deleteDirectory(rootDirectory); + TestUtils.deleteDirectory(rootDirectory); } } @@ -382,16 +380,6 @@ public class LocalFileListerTest { - private void deleteDirectory(File file) - { - for (File subfile : file.listFiles()) { - if (subfile.isDirectory()) { - deleteDirectory(subfile); - } - subfile.delete(); - } - file.delete(); - } } -- GitLab From 91ab7d42a2ae5923f415660befdd02b1114cb6c5 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Tue, 15 Nov 2022 15:15:46 +0100 Subject: [PATCH 06/12] Refactor RemoteFileListerTest & LocalFileListerTest Also do small refactoring in FileLister classes (i.e coding style) --- .../contentScanner/AbstractFileLister.java | 8 +- .../drive/contentScanner/LocalFileLister.java | 2 +- .../contentScanner/RemoteFileLister.java | 2 +- .../contentScanner/LocalFileListerTest.java | 118 +++---- .../contentScanner/RemoteFileListerTest.java | 311 +++++++++++++----- 5 files changed, 292 insertions(+), 149 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java b/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java index 7168b1fd..0fb400e6 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java @@ -81,7 +81,7 @@ public abstract class AbstractFileLister { } if (isSomeContentToSync) { - DbHelper.updateSyncedFolders(folders, context); //@ToDo: maybe do this when all contents will be synced. + DbHelper.updateSyncedFolders(folders, context); //@todo: maybe do this when all contents will be synced. } return isSomeContentToSync; } @@ -170,6 +170,8 @@ public abstract class AbstractFileLister { if (subFile == null) continue; final String fileName = getFileName(subFile); + if (fileName == null) continue; + if (isDirectory(subFile)) { final SyncedFolder subSyncedFolder = new SyncedFolder(syncedFolder, fileName, 0L, "");//Need to set lastModified to 0 to handle it on next iteration iterator.add(subSyncedFolder); @@ -198,9 +200,7 @@ public abstract class AbstractFileLister { public List getSyncedFoldersId() { final List result = new ArrayList<>(); for (SyncedFolder syncedFolder : folders) { - //if (syncedFolder.isToSync() ){ //todo: is it usefull ? maybe this propertie can be removed from SyncedFolder - result.add( (long) syncedFolder.getId() ); - //} + result.add((long) syncedFolder.getId()); } return result; } diff --git a/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java b/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java index 785fafa7..95abc72b 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java @@ -61,7 +61,7 @@ public class LocalFileLister extends AbstractFileLister { } @Override - protected FolderLoader getFolderLoader() { + protected LocalFolderLoader getFolderLoader() { return new LocalFolderLoader(); } diff --git a/app/src/main/java/foundation/e/drive/contentScanner/RemoteFileLister.java b/app/src/main/java/foundation/e/drive/contentScanner/RemoteFileLister.java index 1e0b6e09..1e32511c 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/RemoteFileLister.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/RemoteFileLister.java @@ -67,7 +67,7 @@ public class RemoteFileLister extends AbstractFileLister { } @Override - protected FolderLoader getFolderLoader() { + protected RemoteFolderLoader getFolderLoader() { return new RemoteFolderLoader(); } diff --git a/app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java b/app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java index d3211065..b0b9c720 100644 --- a/app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java +++ b/app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java @@ -8,16 +8,12 @@ package foundation.e.drive.contentScanner; -import android.accounts.Account; -import android.accounts.AccountManager; import android.content.Context; import android.database.sqlite.SQLiteDatabase; import android.os.Build; import androidx.test.core.app.ApplicationProvider; -import com.owncloud.android.lib.common.OwnCloudClient; - import org.junit.After; import org.junit.Assert; import org.junit.Before; @@ -28,7 +24,6 @@ import org.robolectric.RobolectricTestRunner; import org.robolectric.annotation.Config; import org.robolectric.shadows.ShadowLog; - import java.io.File; import java.io.IOException; import java.util.ArrayList; @@ -38,7 +33,6 @@ import foundation.e.drive.TestUtils; import foundation.e.drive.database.DbHelper; import foundation.e.drive.models.SyncedFileState; import foundation.e.drive.models.SyncedFolder; -import foundation.e.drive.utils.DavClientProvider; /** * @author vincent Bourgmayer @@ -50,16 +44,13 @@ public class LocalFileListerTest { private static final String TEST_FILE_TREE_ROOT_PATH = "../tmp/"; private LocalFileLister fileListerUnderTest; - private final OwnCloudClient ocClient; private final Context context; + private final File testRootDirectory; public LocalFileListerTest() { context = ApplicationProvider.getApplicationContext(); ShadowLog.stream = System.out; - TestUtils.loadServerCredentials(); - TestUtils.prepareValidAccount(AccountManager.get(context)); - final Account account = TestUtils.getValidAccount(); - ocClient = DavClientProvider.getInstance().getClientInstance(account, context); + testRootDirectory = new File(TEST_FILE_TREE_ROOT_PATH); } @Before @@ -72,9 +63,8 @@ public class LocalFileListerTest { public void tearDown() { SQLiteDatabase.deleteDatabase(context.getDatabasePath(DbHelper.DATABASE_NAME)); - final File rootDirectory = new File(TEST_FILE_TREE_ROOT_PATH); - if (rootDirectory.exists()) { - TestUtils.deleteDirectory(rootDirectory); + if (testRootDirectory.exists()) { + TestUtils.deleteDirectory(testRootDirectory); } } @@ -128,16 +118,17 @@ public class LocalFileListerTest { private SyncedFolder buildSyncedFolderForSkipSyncedFolderTest(boolean isMedia, boolean isHidden, boolean isLocalScannable) { final String dirName = isHidden ? ".myFolder/" : "myFolder/"; - return new SyncedFolder("any", "/local/path/" + dirName, "/remote/path/" + dirName, isLocalScannable, true, true, isMedia); + return new SyncedFolder("any", TEST_FILE_TREE_ROOT_PATH + dirName, "/remote/path/" + dirName, isLocalScannable, true, true, isMedia); } @Test public void skipDirectory_lastModifiedSame_noUnsyncedContent_shouldReturnTrue() { final long lastModified = 123456L; - final File directory = Mockito.spy(new File("/local/path/test/")); + + final File directory = Mockito.spy(testRootDirectory); Mockito.when(directory.lastModified()).thenReturn(lastModified); - final SyncedFolder syncedFolder = new SyncedFolder("any", "/local/path/test/", "/remote/path/test/", true, true, true, true); + final SyncedFolder syncedFolder = new SyncedFolder("any", directory.getAbsolutePath(), "/remote/path/test/", true, true, true, true); syncedFolder.setId(12); syncedFolder.setLastModified(lastModified); @@ -148,13 +139,16 @@ public class LocalFileListerTest { @Test public void skipDirectory_lastModifiedSame_unsyncedContent_shouldReturnFalse() { final long lastModified = 123456L; - final File directory = Mockito.spy(new File("/local/path/test/")); + + final String dirPath = testRootDirectory.getAbsolutePath(); + + final File directory = Mockito.spy(testRootDirectory); Mockito.when(directory.lastModified()).thenReturn(lastModified); - final SyncedFileState dummy = new SyncedFileState(6, "tutu.jpg", "/local/path/test/tutu.jpg", "/remote/path/test/tutu.jpg", "", 0L, 12, true, 3); + final SyncedFileState dummy = new SyncedFileState(6, "foo.jpg", dirPath + "foo.jpg", "/remote/path/test/foo.jpg", "", 0L, 12, true, 3); DbHelper.manageSyncedFileStateDB(dummy, "INSERT", context); - final SyncedFolder syncedFolder = new SyncedFolder("any", "/local/path/test/" , "/remote/path/test/", true, true, true, true); + final SyncedFolder syncedFolder = new SyncedFolder("any", dirPath , "/remote/path/test/", true, true, true, true); syncedFolder.setId(12); syncedFolder.setLastModified(lastModified); @@ -164,10 +158,12 @@ public class LocalFileListerTest { @Test public void skipDirectory_lastModifiedChanged_noUnsyncedContent_shouldReturnFalse() { - final File directory = Mockito.spy(new File("/local/path/test/")); + final String dirPath = testRootDirectory.getAbsolutePath(); + + final File directory = Mockito.spy(testRootDirectory); Mockito.when(directory.lastModified()).thenReturn(654321L); - final SyncedFolder syncedFolder = new SyncedFolder("any", "/local/path/test/", "/remote/path/test/", true, true, true, true); + final SyncedFolder syncedFolder = new SyncedFolder("any", dirPath, "/remote/path/test/", true, true, true, true); syncedFolder.setId(12); syncedFolder.setLastModified(123456L); @@ -177,14 +173,16 @@ public class LocalFileListerTest { @Test public void skipDirectory_lastModifiedChanged_unsyncedContent_shouldReturnFalse() { - final File directory = Mockito.spy(new File("/local/path/test/")); + final String dirPath = testRootDirectory.getAbsolutePath(); + + final File directory = Mockito.spy(testRootDirectory); Mockito.when(directory.lastModified()).thenReturn(654321L); - final SyncedFolder syncedFolder = new SyncedFolder("any", "/local/path/test/", "/remote/path/test/", true, true, true, true); + final SyncedFolder syncedFolder = new SyncedFolder("any", dirPath, "/remote/path/test/", true, true, true, true); syncedFolder.setId(12); syncedFolder.setLastModified(123456L); - final SyncedFileState dummy = new SyncedFileState(6, "tutu.jpg", "/local/path/test/tutu.jpg", "/remote/path/test/tutu.jpg", "", 0L, 12, true, 3); + final SyncedFileState dummy = new SyncedFileState(6, "foo.jpg", dirPath + "foo.jpg", "/remote/path/test/foo.jpg", "", 0L, 12, true, 3); DbHelper.manageSyncedFileStateDB(dummy, "INSERT", context); final boolean skip = fileListerUnderTest.skipDirectory(directory, syncedFolder, context); @@ -192,13 +190,13 @@ public class LocalFileListerTest { } //Test about inner class: DirectoryLoader - - @Test public void loadDirectory_dirNotExist_shouldReturnTrue() { - final SyncedFolder folder = new SyncedFolder("any", "/local/path/myFolder", "/remote/path/myFolder", true); + final String dirPath = testRootDirectory.getAbsolutePath(); - final LocalFileLister.LocalFolderLoader folderLoader = (LocalFileLister.LocalFolderLoader) fileListerUnderTest.getFolderLoader(); + final SyncedFolder folder = new SyncedFolder("any", dirPath, "/remote/path/test", true); + + final LocalFileLister.LocalFolderLoader folderLoader = fileListerUnderTest.getFolderLoader(); final boolean loadSuccess = folderLoader.load(folder); Assert.assertTrue("Loading remote folder resulting in 404 should return true", loadSuccess); @@ -207,49 +205,47 @@ public class LocalFileListerTest { @Test public void loadDirectory_dirExist_noContent_shouldReturnTrue() { - final SyncedFolder folder = new SyncedFolder("any", "../local/path/myFolder/", "/remote/path/myFolder/", true); - final File file = new File(folder.getLocalFolder()); - final boolean dirCreationSucceed = file.mkdirs(); + final String dirPath = testRootDirectory.getAbsolutePath(); + + final SyncedFolder folder = new SyncedFolder("any", dirPath, "/remote/path/test/", true); - Assert.assertTrue("cannot create test folder: " + file.getAbsolutePath(), dirCreationSucceed); + final boolean dirCreationSucceed = testRootDirectory.mkdirs(); - final LocalFileLister.LocalFolderLoader folderLoader = (LocalFileLister.LocalFolderLoader) fileListerUnderTest.getFolderLoader(); + Assert.assertTrue("Cannot create test folder: " + dirPath, dirCreationSucceed); + + final LocalFileLister.LocalFolderLoader folderLoader = fileListerUnderTest.getFolderLoader(); final boolean loadSuccess = folderLoader.load(folder); final FolderWrapper wrapper = folderLoader.getFolderWrapper(); Assert.assertTrue("Loading remote folder resulting in 404 should return true", loadSuccess); Assert.assertFalse("dir shouldExist!", wrapper.isMissing()); - Assert.assertTrue("List of folderWrapper's content expected to be empty but contains: "+wrapper.getContent().size(), wrapper.getContent().isEmpty()); - //@todo beware, in case of previous assertion failure, the below code won't be executed - Assert.assertTrue("Test file has not been successfully removed: " + file.getAbsolutePath(), file.delete()); + Assert.assertTrue("List of folderWrapper's content expected to be empty but contains: " + wrapper.getContent().size(), wrapper.getContent().isEmpty()); } @Test public void loadDirectory_dirExist_withSubContent_shouldReturnTrue() { - final SyncedFolder folder = new SyncedFolder("any", "../local/path/myFolder/", "/remote/path/myFolder/", true); - final File folderFile = new File(folder.getLocalFolder()); - final boolean dirCreationSucceed = folderFile.mkdirs(); + final String dirPath = testRootDirectory.getAbsolutePath(); + + final SyncedFolder folder = new SyncedFolder("any", dirPath, "/remote/path/test/", true); + final boolean dirCreationSucceed = testRootDirectory.mkdirs(); + Assert.assertTrue("cannot create test folder: " + dirPath, dirCreationSucceed); + - final File subContent = new File(folderFile, "toto.txt"); + final File subContent = new File(testRootDirectory, "foo.txt"); try { - subContent.createNewFile(); + Assert.assertTrue("Can't create subFile", subContent.createNewFile()); } catch (IOException exception) { exception.printStackTrace(); Assert.fail("IO Exception: Can't create new file as content of test folder"); } - Assert.assertTrue("cannot create test folder: " + folderFile.getAbsolutePath(), dirCreationSucceed); - - final LocalFileLister.LocalFolderLoader folderLoader = (LocalFileLister.LocalFolderLoader) fileListerUnderTest.getFolderLoader(); + final LocalFileLister.LocalFolderLoader folderLoader = fileListerUnderTest.getFolderLoader(); final boolean loadSuccess = folderLoader.load(folder); final FolderWrapper wrapper = folderLoader.getFolderWrapper(); Assert.assertTrue("Loading remote folder resulting in 404 should return true", loadSuccess); Assert.assertFalse("dir shouldExist!", wrapper.isMissing()); - Assert.assertEquals("List of folderWrapper's content size expected to one but was: "+wrapper.getContent().size(), 1, wrapper.getContent().size()); - //@todo beware, in case of previous assertion failure, the below code won't be executed - Assert.assertTrue("SubFile has not been removed", subContent.delete()); - Assert.assertTrue("Test folder has not been removed: " + folderFile.getAbsolutePath(), folderFile.delete()); + Assert.assertEquals("List of folderWrapper's content size expected to one but was: " + wrapper.getContent().size(), 1, wrapper.getContent().size()); } @@ -288,7 +284,7 @@ public class LocalFileListerTest { * SyncedFolderRemoved which correspond to a missing directory */ @Test - public void bigWholeTest() { + public void test_listContentToScan() { Assert.assertTrue("Preparation, of file Tree and syncedFolder has failed", prepareFakeContent()); Assert.assertTrue("File listing failed", fileListerUnderTest.listContentToScan(context)); @@ -302,18 +298,17 @@ public class LocalFileListerTest { } private boolean prepareFakeContent() { - /** + /* * Generate folders */ - final File rootDirectory = new File(TEST_FILE_TREE_ROOT_PATH); - if (!rootDirectory.mkdirs()) return false; + if (!testRootDirectory.mkdirs()) return false; - final File folder1 = new File(rootDirectory, "folder1/"); + final File folder1 = new File(testRootDirectory, "folder1/"); if (!folder1.mkdirs()) return false; final File folder2 = new File(folder1, "folder2/"); if (!folder2.mkdirs()) return false; - final File folder3 = new File(rootDirectory, "folder3/"); + final File folder3 = new File(testRootDirectory, "folder3/"); if (!folder3.mkdirs()) return false; final File folder4 = new File(folder3, "folder4/"); if (!folder4.mkdirs()) return false; @@ -321,7 +316,7 @@ public class LocalFileListerTest { if (!folder5.mkdirs()) return false; - /** + /* * Generate files * /!\ must be done before syncedFolders generation * otherwise lastModified value of directory will change due to file creation @@ -349,8 +344,7 @@ public class LocalFileListerTest { return false; } - - /** + /* * Generate SyncedFolders */ final SyncedFolder syncedFolder1 = new SyncedFolder("any", folder1.getAbsolutePath(), "/remote/path/myFolder1/", true); @@ -362,7 +356,7 @@ public class LocalFileListerTest { final SyncedFolder syncedFolder3 = new SyncedFolder("any", folder3.getAbsolutePath(), "/remote/path/myFolder3/", true); syncedFolder3.setLastModified(folder3.lastModified()); syncedFolder3.setId((int) DbHelper.insertSyncedFolder(syncedFolder3, context)); - final SyncedFolder syncedFolderRemoved = new SyncedFolder("any", rootDirectory.getAbsolutePath()+"lambda/", "/remote/lambda", true); + final SyncedFolder syncedFolderRemoved = new SyncedFolder("any", testRootDirectory.getAbsolutePath() + "lambda/", "/remote/lambda", true); syncedFolderRemoved.setLastModified(987654321L); syncedFolderRemoved.setLastEtag("123456789"); syncedFolderRemoved.setId((int) DbHelper.insertSyncedFolder(syncedFolderRemoved, context)); @@ -376,10 +370,4 @@ public class LocalFileListerTest { return true; } - - - - - - } diff --git a/app/src/test/java/foundation/e/drive/contentScanner/RemoteFileListerTest.java b/app/src/test/java/foundation/e/drive/contentScanner/RemoteFileListerTest.java index 200e3456..e7c86ee3 100644 --- a/app/src/test/java/foundation/e/drive/contentScanner/RemoteFileListerTest.java +++ b/app/src/test/java/foundation/e/drive/contentScanner/RemoteFileListerTest.java @@ -8,6 +8,8 @@ package foundation.e.drive.contentScanner; +import static org.mockito.Mockito.when; + import android.accounts.Account; import android.accounts.AccountManager; import android.content.Context; @@ -26,11 +28,14 @@ import org.junit.Ignore; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mockito; +import org.mockito.internal.configuration.injection.MockInjection; 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.List; import foundation.e.drive.TestUtils; import foundation.e.drive.database.DbHelper; @@ -46,6 +51,7 @@ import foundation.e.drive.utils.DavClientProvider; public class RemoteFileListerTest { private static final String DIR_NO_SKIP_EXPECTED = "directory shouldn't have been skipped"; private static final String DIR_SKIP_EXPECTED = "directory should have been skipped"; + private static final String TEST_FILE_TREE_ROOT_PATH = "/remote/"; private RemoteFileLister fileListerUnderTest; private final Account account; @@ -125,15 +131,15 @@ public class RemoteFileListerTest { private SyncedFolder buildSyncedFolderForSkipSyncedFolderTest(boolean isMedia, boolean isHidden, boolean isRemoteScannable) { final String dirName = isHidden ? ".myFolder/" : "myFolder/"; - return new SyncedFolder("any", "/local/path/" + dirName, "/remote/path/" + dirName, true, isRemoteScannable, true, isMedia); + return new SyncedFolder("any", "/local/path/" + dirName, TEST_FILE_TREE_ROOT_PATH + dirName, true, isRemoteScannable, true, isMedia); } @Test public void skipFile_hidden_shouldReturnTrue() { - final String hiddenRemoteFilePath = "remote/path/.tutu.png"; + final String hiddenRemoteFilePath = TEST_FILE_TREE_ROOT_PATH + ".foo.png"; final RemoteFile file = Mockito.mock(RemoteFile.class); - Mockito.when(file.getRemotePath()).thenReturn(hiddenRemoteFilePath); + when(file.getRemotePath()).thenReturn(hiddenRemoteFilePath); final boolean skipFile = fileListerUnderTest.skipFile(file); Assert.assertTrue("Hidden remoteFile (" + hiddenRemoteFilePath + ") should have been skipped", skipFile); @@ -141,9 +147,9 @@ public class RemoteFileListerTest { @Test public void skipFile_notHidden_shouldReturnFalse() { - final String notHiddenRemoteFilePath = "remote/path/tutu.png"; + final String notHiddenRemoteFilePath = TEST_FILE_TREE_ROOT_PATH + "foo.png"; final RemoteFile file = Mockito.mock(RemoteFile.class); - Mockito.when(file.getRemotePath()).thenReturn(notHiddenRemoteFilePath); + when(file.getRemotePath()).thenReturn(notHiddenRemoteFilePath); final boolean skipFile = fileListerUnderTest.skipFile(file); Assert.assertFalse("Not hidden remoteFile (" + notHiddenRemoteFilePath + ") should not have been skipped", skipFile); @@ -153,7 +159,7 @@ public class RemoteFileListerTest { public void skipFile_emptyPath_shouldReturnTrue() { final String emptyPath = ""; final RemoteFile file = Mockito.mock(RemoteFile.class); - Mockito.when(file.getRemotePath()).thenReturn(emptyPath); + when(file.getRemotePath()).thenReturn(emptyPath); final boolean skipFile = fileListerUnderTest.skipFile(file); Assert.assertTrue("RemoteFile with empty path (" + emptyPath + ") should have been skipped", skipFile); @@ -166,9 +172,9 @@ public class RemoteFileListerTest { /*in this case: No file, just a directory the current implementation of CommonUtils.getFileName(String path) will return "path" while empty string should be expected. - todo: fix CommonUtils.getFileName(String path)*/ + todo: fix CommonUtils.getFileName(String path) */ final RemoteFile file = Mockito.mock(RemoteFile.class); - Mockito.when(file.getRemotePath()).thenReturn(invalidPath); + when(file.getRemotePath()).thenReturn(invalidPath); final boolean skipFile = fileListerUnderTest.skipFile(file); Assert.assertTrue("RemoteFile with empty path (" + invalidPath + ") should have been skipped", skipFile); @@ -177,10 +183,10 @@ public class RemoteFileListerTest { @Test public void skipDirectory_etagChanged_noUnsyncedContent_shouldReturnFalse() { final RemoteFile remoteFile = Mockito.mock(RemoteFile.class); - Mockito.when(remoteFile.getEtag()).thenReturn("6666bbbb"); + when(remoteFile.getEtag()).thenReturn("6666bbbb"); - final SyncedFolder syncedFolder = new SyncedFolder("", "local", "remote", true); - syncedFolder.setId(120); + final SyncedFolder syncedFolder = new SyncedFolder("", "local", TEST_FILE_TREE_ROOT_PATH, true); + syncedFolder.setId(12); syncedFolder.setLastEtag("5555aaaa"); final boolean skip = fileListerUnderTest.skipDirectory(remoteFile, syncedFolder, context); @@ -191,9 +197,9 @@ public class RemoteFileListerTest { @Test public void skipDirectory_etagSame_noUnsyncedContent_shouldReturnTrue() { final RemoteFile remoteFile = Mockito.mock(RemoteFile.class); - Mockito.when(remoteFile.getEtag()).thenReturn("5555aaaa"); + when(remoteFile.getEtag()).thenReturn("5555aaaa"); - final SyncedFolder syncedFolder = new SyncedFolder("", "local", "remote", true); + final SyncedFolder syncedFolder = new SyncedFolder("", "local", TEST_FILE_TREE_ROOT_PATH, true); syncedFolder.setId(12); syncedFolder.setLastEtag("5555aaaa"); @@ -205,53 +211,47 @@ public class RemoteFileListerTest { @Test public void skipDirectory_etagSame_contentToSync_shouldReturnFalse() { final RemoteFile remoteFile = Mockito.mock(RemoteFile.class); - Mockito.when(remoteFile.getEtag()).thenReturn("5555aaaa"); + when(remoteFile.getEtag()).thenReturn("5555aaaa"); - final SyncedFolder syncedFolder = new SyncedFolder("", "local", "remote", true); + final SyncedFolder syncedFolder = new SyncedFolder("", "local", TEST_FILE_TREE_ROOT_PATH, true); syncedFolder.setId(12); syncedFolder.setLastEtag("5555aaaa"); - final SyncedFileState dummy = new SyncedFileState(6, "tutu.jpg", "local/tutu.jpg", "remote/tutu.jpg", "7777", 0L, 12, true, 3); + final SyncedFileState dummy = new SyncedFileState(6, "foo.jpg", "local/foo.jpg", TEST_FILE_TREE_ROOT_PATH + "foo.jpg", "7777", 0L, 12, true, 3); DbHelper.manageSyncedFileStateDB(dummy, "INSERT", context); final boolean skip = fileListerUnderTest.skipDirectory(remoteFile, syncedFolder, context); - Assert.assertFalse("Remote directory with no new etag but there is content to sync from DB: should not have been skipped but it had", skip); } @Test public void skipDirectory_etagChanged_contentToSync_shouldReturnFalse() { final RemoteFile remoteFile = Mockito.mock(RemoteFile.class); - Mockito.when(remoteFile.getEtag()).thenReturn("6666bbbb"); + when(remoteFile.getEtag()).thenReturn("6666bbbb"); - final SyncedFolder syncedFolder = new SyncedFolder("", "local", "remote", true); + final SyncedFolder syncedFolder = new SyncedFolder("", "local", TEST_FILE_TREE_ROOT_PATH, true); syncedFolder.setId(12); syncedFolder.setLastEtag("5555aaaa"); - final SyncedFileState dummy = new SyncedFileState(6, "tutu.jpg", "local/tutu.jpg", "remote/tutu.jpg", "7777", 0L, 12, true, 3); + final SyncedFileState dummy = new SyncedFileState(6, "foo.jpg", "local/foo.jpg", TEST_FILE_TREE_ROOT_PATH + "foo.jpg", "7777", 0L, 12, true, 3); DbHelper.manageSyncedFileStateDB(dummy, "INSERT", context); final boolean skip = fileListerUnderTest.skipDirectory(remoteFile, syncedFolder, context); - Assert.assertFalse("Remote directory with new etag and there is content to sync from DB: should not have been skipped but it had", skip); } - //Test about inner class: DirectoryLoader + //Test about inner class: DirectoryLoader @Test public void loadDirectory_serverResponse404_shouldReturnTrue() { - final String remotePath = "remote/path/myFolder/"; - final OwnCloudClient client = DavClientProvider.getInstance().getClientInstance(account, context); - final SyncedFolder folder = new SyncedFolder("any", "local/path/myFolder", remotePath, true); + final String remotePath = TEST_FILE_TREE_ROOT_PATH + "myFolder/"; + final SyncedFolder folder = new SyncedFolder("any", "local/myFolder", remotePath, true); final RemoteOperationResult fakeResult = Mockito.mock(RemoteOperationResult.class); - Mockito.when(fakeResult.getHttpCode()).thenReturn(404); - Mockito.when(fakeResult.isSuccess()).thenReturn(false); - - final RemoteFileLister.RemoteFolderLoader mockLoader = (RemoteFileLister.RemoteFolderLoader) fileListerUnderTest.getFolderLoader(); + when(fakeResult.getHttpCode()).thenReturn(404); + when(fakeResult.isSuccess()).thenReturn(false); - RemoteFileLister.RemoteFolderLoader mockLoaderSpied = Mockito.spy(mockLoader); - - Mockito.when(mockLoaderSpied.readRemoteFolder(remotePath, client)).thenReturn(fakeResult); + final RemoteFileLister.RemoteFolderLoader mockLoaderSpied = Mockito.spy(fileListerUnderTest.getFolderLoader()); + when(mockLoaderSpied.readRemoteFolder(remotePath, ocClient)).thenReturn(fakeResult); final boolean loadSuccess = mockLoaderSpied.load(folder); Assert.assertTrue("Loading remote folder resulting in 404 should return true", loadSuccess); @@ -259,19 +259,15 @@ public class RemoteFileListerTest { @Test public void loadDirectory_serverResponse405_shouldReturnFalse() { - final String remotePath = "remote/path/myFolder/"; - final OwnCloudClient client = DavClientProvider.getInstance().getClientInstance(account, context); - final SyncedFolder folder = new SyncedFolder("any", "local/path/myFolder", remotePath, true); + final String remotePath = TEST_FILE_TREE_ROOT_PATH + "myFolder/"; + final SyncedFolder folder = new SyncedFolder("any", "local/myFolder", remotePath, true); final RemoteOperationResult fakeResult = Mockito.mock(RemoteOperationResult.class); - Mockito.when(fakeResult.getHttpCode()).thenReturn(405); - Mockito.when(fakeResult.isSuccess()).thenReturn(false); - - final RemoteFileLister.RemoteFolderLoader mockLoader = (RemoteFileLister.RemoteFolderLoader) fileListerUnderTest.getFolderLoader(); - - RemoteFileLister.RemoteFolderLoader mockLoaderSpied = Mockito.spy(mockLoader); + when(fakeResult.getHttpCode()).thenReturn(405); + when(fakeResult.isSuccess()).thenReturn(false); - Mockito.when(mockLoaderSpied.readRemoteFolder(remotePath, client)).thenReturn(fakeResult); + final RemoteFileLister.RemoteFolderLoader mockLoaderSpied = Mockito.spy(fileListerUnderTest.getFolderLoader()); + when(mockLoaderSpied.readRemoteFolder(remotePath, ocClient)).thenReturn(fakeResult); final boolean loadSuccess = mockLoaderSpied.load(folder); Assert.assertFalse("Loading remote folder resulting in 405 should return false", loadSuccess); @@ -279,24 +275,20 @@ public class RemoteFileListerTest { @Test public void loadDirectory_successAndErrorCode_shouldReturnTrue() { - final String remotePath = "remote/path/myFolder/"; - final OwnCloudClient client = DavClientProvider.getInstance().getClientInstance(account, context); - final SyncedFolder folder = new SyncedFolder("any", "local/path/myFolder", remotePath, true); + final String remotePath = TEST_FILE_TREE_ROOT_PATH + "myFolder/"; + final SyncedFolder folder = new SyncedFolder("any", "local/myFolder", remotePath, true); final RemoteOperationResult fakeResult = Mockito.mock(RemoteOperationResult.class); - Mockito.when(fakeResult.getHttpCode()).thenReturn(405); - Mockito.when(fakeResult.isSuccess()).thenReturn(true); + when(fakeResult.getHttpCode()).thenReturn(405); + when(fakeResult.isSuccess()).thenReturn(true); final ArrayList fakeData = new ArrayList<>(); final RemoteFile mockRemoteFile = Mockito.mock(RemoteFile.class); fakeData.add(mockRemoteFile); - Mockito.when(fakeResult.getData()).thenReturn(fakeData); + when(fakeResult.getData()).thenReturn(fakeData); - final RemoteFileLister.RemoteFolderLoader loaderUnderTest = (RemoteFileLister.RemoteFolderLoader) fileListerUnderTest.getFolderLoader(); - - RemoteFileLister.RemoteFolderLoader spiedLoaderUnderTest = Mockito.spy(loaderUnderTest); - - Mockito.when(spiedLoaderUnderTest.readRemoteFolder(remotePath, client)).thenReturn(fakeResult); + final RemoteFileLister.RemoteFolderLoader spiedLoaderUnderTest = Mockito.spy(fileListerUnderTest.getFolderLoader()); + when(spiedLoaderUnderTest.readRemoteFolder(remotePath, ocClient)).thenReturn(fakeResult); final boolean loadSuccess = spiedLoaderUnderTest.load(folder); Assert.assertTrue("Loading remote folder with success despite error code (405) should return true", loadSuccess); @@ -305,13 +297,12 @@ public class RemoteFileListerTest { @Test public void loadDirectory_successAndRemoteContent_shouldReturnTrue() { - final String remotePath = "remote/path/myFolder/"; - final OwnCloudClient client = DavClientProvider.getInstance().getClientInstance(account, context); - final SyncedFolder folder = new SyncedFolder("any", "local/path/myFolder", remotePath, true); + final String remotePath = TEST_FILE_TREE_ROOT_PATH + "myFolder/"; + final SyncedFolder folder = new SyncedFolder("any", "local/myFolder", remotePath, true); final RemoteOperationResult fakeResult = Mockito.mock(RemoteOperationResult.class); - Mockito.when(fakeResult.getHttpCode()).thenReturn(405); - Mockito.when(fakeResult.isSuccess()).thenReturn(true); + when(fakeResult.getHttpCode()).thenReturn(405); + when(fakeResult.isSuccess()).thenReturn(true); final ArrayList fakeData = new ArrayList<>(); final RemoteFile mockDirRemoteFile = Mockito.mock(RemoteFile.class); @@ -320,13 +311,10 @@ public class RemoteFileListerTest { final RemoteFile mockContentRemoteFile2 = Mockito.mock(RemoteFile.class); fakeData.add(mockContentRemoteFile1); fakeData.add(mockContentRemoteFile2); - Mockito.when(fakeResult.getData()).thenReturn(fakeData); - - final RemoteFileLister.RemoteFolderLoader loaderUnderTest = (RemoteFileLister.RemoteFolderLoader) fileListerUnderTest.getFolderLoader(); + when(fakeResult.getData()).thenReturn(fakeData); - RemoteFileLister.RemoteFolderLoader spiedLoaderUnderTest = Mockito.spy(loaderUnderTest); - - Mockito.when(spiedLoaderUnderTest.readRemoteFolder(remotePath, client)).thenReturn(fakeResult); + final RemoteFileLister.RemoteFolderLoader spiedLoaderUnderTest = Mockito.spy(fileListerUnderTest.getFolderLoader()); + when(spiedLoaderUnderTest.readRemoteFolder(remotePath, ocClient)).thenReturn(fakeResult); final boolean loadSuccess = spiedLoaderUnderTest.load(folder); Assert.assertTrue("Loading remote folder with success despite error code (405) should return true", loadSuccess); @@ -336,30 +324,197 @@ public class RemoteFileListerTest { Assert.assertTrue("mockContentRemoteFile2 should have been in the result of loading but is not", spiedLoaderUnderTest.getFolderWrapper().getContent().contains(mockContentRemoteFile2)); } - - - @Test public void loadDirectory_successButNoData_shouldReturnFalse() { - final String remotePath = "remote/path/myFolder/"; - final OwnCloudClient client = DavClientProvider.getInstance().getClientInstance(account, context); - final SyncedFolder folder = new SyncedFolder("any", "local/path/myFolder", remotePath, true); + final String remotePath = TEST_FILE_TREE_ROOT_PATH + "myFolder/"; + final SyncedFolder folder = new SyncedFolder("any", "local/myFolder", remotePath, true); final RemoteOperationResult fakeResult = Mockito.mock(RemoteOperationResult.class); - Mockito.when(fakeResult.getHttpCode()).thenReturn(204); - Mockito.when(fakeResult.isSuccess()).thenReturn(true); + when(fakeResult.getHttpCode()).thenReturn(204); + when(fakeResult.isSuccess()).thenReturn(true); final ArrayList fakeData = new ArrayList<>(); - Mockito.when(fakeResult.getData()).thenReturn(fakeData); + when(fakeResult.getData()).thenReturn(fakeData); - final RemoteFileLister.RemoteFolderLoader mockLoader = (RemoteFileLister.RemoteFolderLoader) fileListerUnderTest.getFolderLoader(); - - RemoteFileLister.RemoteFolderLoader mockLoaderSpied = Mockito.spy(mockLoader); - - Mockito.when(mockLoaderSpied.readRemoteFolder(remotePath, client)).thenReturn(fakeResult); + final RemoteFileLister.RemoteFolderLoader mockLoaderSpied = Mockito.spy(fileListerUnderTest.getFolderLoader()); + when(mockLoaderSpied.readRemoteFolder(remotePath, ocClient)).thenReturn(fakeResult); final boolean loadSuccess = mockLoaderSpied.load(folder); Assert.assertFalse("Loading remote folder resulting in 204 but without any RemoteFile should return false", loadSuccess); } + + /** + * Test the whole behaviour of localFileLister with a fake file's tree + * + * ## File tree + * >Folder 1 (unChanged) + * --> File 1 (should be ignored) + * --> Folder 2 (unsynced dir) + * ----> File 2 (new file should be added) + * ----> .File 3 (should be ignored) + * >Folder 3 (unChanged) + * -->Folder4 (skip because folder 3 unchanged) + * ----> File 4 (skip because folder 3 unchanged) + * -->.Folder5 (skip: hidden dir) + * ----> File 5 (skip because directory is hidden) + * + * There is also the missing remote folder, which is expected to be in the result's folder's id + */ + @Test + public void test_listContentToScan() { + prepareFakeContent(); + Assert.assertTrue("File listing failed", fileListerUnderTest.listContentToScan(context)); + + final List foldersId = fileListerUnderTest.getSyncedFoldersId(); + Assert.assertEquals("List of folders to scan's id should contains 2 but contained: " + foldersId.size(), 2, foldersId.size()); + + final List contentListed = fileListerUnderTest.getContentToScan(); + Assert.assertEquals("ContentListed should have 1 files but contained: " + contentListed.size(), 1, contentListed.size()); + } + + /** + * Generate fake file tree & mock object for a full test of RemoteFileLister + */ + private void prepareFakeContent() { + /* + * Generate folders + */ + final String folder1Path = TEST_FILE_TREE_ROOT_PATH + "folder1/"; + final RemoteFile folder1 = Mockito.mock(RemoteFile.class); //unchanged + when(folder1.getRemotePath()).thenReturn(folder1Path); + when(folder1.getEtag()).thenReturn("123456789"); + when(folder1.getMimeType()).thenReturn("DIR"); + + final String folder2Path = folder1Path + "folder2/"; + final RemoteFile folder2 = Mockito.mock(RemoteFile.class); //updated one + when(folder2.getRemotePath()).thenReturn(folder2Path); + when(folder2.getEtag()).thenReturn("234567891"); + when(folder2.getMimeType()).thenReturn("DIR"); + + final String folder3Path = TEST_FILE_TREE_ROOT_PATH + "folder3/"; + final RemoteFile folder3 = Mockito.mock(RemoteFile.class); //unchanged + when(folder3.getRemotePath()).thenReturn(folder3Path); + when(folder3.getEtag()).thenReturn("345678912"); + when(folder3.getMimeType()).thenReturn("DIR"); + + final String folder4Path = folder3Path + "folder4/"; + final RemoteFile folder4 = Mockito.mock(RemoteFile.class); //new one + when(folder4.getRemotePath()).thenReturn(folder4Path); + when(folder4.getEtag()).thenReturn("456789123"); + when(folder4.getMimeType()).thenReturn("DIR"); + + final String folder5Path = folder3.getRemotePath() + ".folder5/"; + final RemoteFile folder5 = Mockito.mock(RemoteFile.class); //hidden one + when(folder5.getRemotePath()).thenReturn(folder5Path); + when(folder5.getEtag()).thenReturn("567891234"); + when(folder5.getMimeType()).thenReturn("DIR"); + + /* + * Generate files + */ + final RemoteFile file1 = Mockito.mock(RemoteFile.class); //ignored because dir unchanged + when(file1.getRemotePath()).thenReturn(folder1Path + "file1.png"); + when(file1.getMimeType()).thenReturn("any"); + final RemoteFile file2 = Mockito.mock(RemoteFile.class); //new file one, should be added + when(file2.getRemotePath()).thenReturn(folder2Path + "file2.png"); + when(file2.getMimeType()).thenReturn("any"); + final RemoteFile file3 = Mockito.mock(RemoteFile.class); //hidden one: ignored + when(file3.getRemotePath()).thenReturn(folder2Path + ".file3.png"); + when(file3.getMimeType()).thenReturn("any"); + final RemoteFile file4 = Mockito.mock(RemoteFile.class); //new one should be added + when(file4.getRemotePath()).thenReturn(folder4Path + "file4.png"); + when(file4.getMimeType()).thenReturn("any"); + final RemoteFile file5 = Mockito.mock(RemoteFile.class); //ignored because dir is hidden + when(file5.getRemotePath()).thenReturn(folder5Path + "file5.png"); + when(file5.getMimeType()).thenReturn("any"); + + /* + * Generate SyncedFolders + */ + final SyncedFolder syncedFolder1 = new SyncedFolder("any", "local/folder1/", folder1Path, true); + syncedFolder1.setLastEtag(folder1.getEtag()); + syncedFolder1.setId((int) DbHelper.insertSyncedFolder(syncedFolder1, context)); + + final SyncedFolder syncedFolder2 = new SyncedFolder("any", "local/folder1/folder2/", folder2Path, true); + syncedFolder2.setLastEtag(""); + syncedFolder2.setId((int) DbHelper.insertSyncedFolder(syncedFolder2, context)); + + final SyncedFolder syncedFolder3 = new SyncedFolder("any","local/folder3/", folder3Path, true); + syncedFolder3.setLastEtag(folder3.getEtag()); + syncedFolder3.setId((int) DbHelper.insertSyncedFolder(syncedFolder3, context)); + + final SyncedFolder syncedFolderRemoved = new SyncedFolder("any", "local/missingFolder/", TEST_FILE_TREE_ROOT_PATH + "lambda/", true); + syncedFolderRemoved.setLastModified(987654321L); + syncedFolderRemoved.setLastEtag("123456789"); + syncedFolderRemoved.setId((int) DbHelper.insertSyncedFolder(syncedFolderRemoved, context)); + + final ArrayList syncedFolders = new ArrayList<>(); + syncedFolders.add(syncedFolder1); + syncedFolders.add(syncedFolder2); + syncedFolders.add(syncedFolder3); + syncedFolders.add(syncedFolderRemoved); + + fileListerUnderTest = Mockito.spy(new RemoteFileLister(syncedFolders, ocClient)); + + + /* Mock the method depending to network and cloud */ + final RemoteFileLister.RemoteFolderLoader spiedDirLoader = Mockito.spy(fileListerUnderTest.getFolderLoader()); + + final RemoteOperationResult folder1Result = Mockito.mock(RemoteOperationResult.class); + when(folder1Result.getHttpCode()).thenReturn(207); + when(folder1Result.isSuccess()).thenReturn(true); + final ArrayList folder1Data = new ArrayList<>(); + folder1Data.add(folder1); + folder1Data.add(file1); + folder1Data.add(folder2); + when(folder1Result.getData()).thenReturn(folder1Data); + when(spiedDirLoader.readRemoteFolder(syncedFolder1.getRemoteFolder(), ocClient)).thenReturn(folder1Result); + + final RemoteOperationResult folder2Result = Mockito.mock(RemoteOperationResult.class); + when(folder2Result.getHttpCode()).thenReturn(207); + when(folder2Result.isSuccess()).thenReturn(true); + final ArrayList folder2Data = new ArrayList<>(); + folder2Data.add(folder2); + folder2Data.add(file2); + folder2Data.add(file3); + when(folder2Result.getData()).thenReturn(folder2Data); + when(spiedDirLoader.readRemoteFolder(syncedFolder2.getRemoteFolder(), ocClient)).thenReturn(folder2Result); + + final RemoteOperationResult folder3Result = Mockito.mock(RemoteOperationResult.class); + when(folder3Result.getHttpCode()).thenReturn(207); + when(folder3Result.isSuccess()).thenReturn(true); + final ArrayList folder3Data = new ArrayList<>(); + folder3Data.add(folder3); + folder3Data.add(folder4); + folder3Data.add(folder5); + when(folder3Result.getData()).thenReturn(folder3Data); + when(spiedDirLoader.readRemoteFolder(syncedFolder3.getRemoteFolder(), ocClient)).thenReturn(folder3Result); + + + final RemoteOperationResult folder4Result = Mockito.mock(RemoteOperationResult.class); + when(folder4Result.getHttpCode()).thenReturn(207); + when(folder4Result.isSuccess()).thenReturn(true); + final ArrayList folder4Data = new ArrayList<>(); + folder4Data.add(folder4); + folder4Data.add(file4); + when(folder4Result.getData()).thenReturn(folder4Data); + when(spiedDirLoader.readRemoteFolder(folder4.getRemotePath(), ocClient)).thenReturn(folder4Result); + + final RemoteOperationResult folder5Result = Mockito.mock(RemoteOperationResult.class); + when(folder5Result.getHttpCode()).thenReturn(207); + when(folder5Result.isSuccess()).thenReturn(true); + final ArrayList folder5Data = new ArrayList<>(); + folder5Data.add(folder4); + folder5Data.add(file4); + when(folder5Result.getData()).thenReturn(folder5Data); + when(spiedDirLoader.readRemoteFolder(folder5.getRemotePath(), ocClient)).thenReturn(folder5Result); + + final RemoteOperationResult missingFolderResult = Mockito.mock(RemoteOperationResult.class); + when(missingFolderResult.getHttpCode()).thenReturn(404); + when(missingFolderResult.isSuccess()).thenReturn(false); + when(spiedDirLoader.readRemoteFolder(syncedFolderRemoved.getRemoteFolder(), ocClient)).thenReturn(missingFolderResult); + + when(fileListerUnderTest.getFolderLoader()).thenReturn(spiedDirLoader); + } } -- GitLab From 5992007d698ac66342eeb2d569028be283f7961b Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Wed, 16 Nov 2022 15:29:17 +0100 Subject: [PATCH 07/12] add new method to update SyncedFolder etag & last modified after listing files --- .../e/drive/contentScanner/AbstractFileLister.java | 3 ++- .../foundation/e/drive/contentScanner/LocalFileLister.java | 5 +++++ .../foundation/e/drive/contentScanner/RemoteFileLister.java | 5 +++++ 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java b/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java index 0fb400e6..4515cb31 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java @@ -41,6 +41,7 @@ public abstract class AbstractFileLister { protected abstract boolean isDirectory(@NonNull T file); protected abstract String getFileName(@NonNull T file); protected abstract FolderLoader getFolderLoader(); + protected abstract void updateSyncedFolder(@NonNull SyncedFolder syncedFolder,@NonNull T folder); /** * This allows to use RemoteOperationResult for RemoteFileLister @@ -121,7 +122,7 @@ public abstract class AbstractFileLister { } else { syncedFolder.setToSync(true); } - + updateSyncedFolder(syncedFolder, currentDirectory.getFolder()); //todo: look where to put in subclasses : syncedFolder.setLastEtag(directory.getEtag()).setToSync(true); diff --git a/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java b/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java index 95abc72b..378c52ff 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java @@ -65,6 +65,11 @@ public class LocalFileLister extends AbstractFileLister { return new LocalFolderLoader(); } + @Override + protected void updateSyncedFolder(@NonNull SyncedFolder syncedFolder, @NonNull File folder) { + syncedFolder.setLastModified(folder.lastModified()); + } + @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) public class LocalFolderLoader implements FolderLoader { private FolderWrapper folder; diff --git a/app/src/main/java/foundation/e/drive/contentScanner/RemoteFileLister.java b/app/src/main/java/foundation/e/drive/contentScanner/RemoteFileLister.java index 1e32511c..99f9a257 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/RemoteFileLister.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/RemoteFileLister.java @@ -71,6 +71,11 @@ public class RemoteFileLister extends AbstractFileLister { return new RemoteFolderLoader(); } + @Override + protected void updateSyncedFolder(@NonNull SyncedFolder syncedFolder, @NonNull RemoteFile folder) { + syncedFolder.setLastEtag(folder.getEtag()); + } + @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) public class RemoteFolderLoader implements FolderLoader { private static final int HTTP_404 = 404; -- GitLab From daf169443a8b66a2288967da792609f22d3371c3 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Mon, 31 Oct 2022 15:59:58 +0100 Subject: [PATCH 08/12] Call FileLister implementation from ObserverService & ListFileRemoteOperation Remove code for listing local and remote content from ObserverService & ListFileRemoteOperation It makes the class lighter, easier to understand and to test --- .../operations/ListFileRemoteOperation.java | 147 ++---------------- .../e/drive/services/ObserverService.java | 109 +++---------- 2 files changed, 35 insertions(+), 221 deletions(-) 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 9463e43b..803e373c 100644 --- a/app/src/main/java/foundation/e/drive/operations/ListFileRemoteOperation.java +++ b/app/src/main/java/foundation/e/drive/operations/ListFileRemoteOperation.java @@ -19,6 +19,8 @@ import java.io.File; import java.util.ArrayList; import java.util.List; import java.util.ListIterator; + +import foundation.e.drive.contentScanner.RemoteFileLister; import foundation.e.drive.database.DbHelper; import foundation.e.drive.models.SyncedFolder; import foundation.e.drive.utils.CommonUtils; @@ -30,19 +32,17 @@ import timber.log.Timber; * @author Vincent Bourgmayer */ public class ListFileRemoteOperation extends RemoteOperation> { - private static final int HTTP_404 = 404; private final List syncedFolders; private final Context context; - private final int initialFolderNumber; - private final ArrayList remoteFiles; - public ListFileRemoteOperation(List syncedFolders, Context context, int initialFolderNumber) { + private final ArrayList updatedSyncedFoldersId; + + public ListFileRemoteOperation(List syncedFolders, Context context) { Timber.tag(ListFileRemoteOperation.class.getSimpleName()); this.syncedFolders = syncedFolders; this.context = context; - this.initialFolderNumber = initialFolderNumber; - remoteFiles = new ArrayList<>(); + updatedSyncedFoldersId = new ArrayList<>(); } /** @@ -52,140 +52,23 @@ public class ListFileRemoteOperation extends RemoteOperation> run(OwnCloudClient ownCloudClient) { - RemoteOperationResult finalResult; - - boolean atLeastOneDirAsChanged = false; - final ListIterator mSyncedFolderIterator = syncedFolders.listIterator(); - - while (mSyncedFolderIterator.hasNext()) { - try { - Thread.sleep(150); - } catch (InterruptedException exception) { - Timber.v( "listFileRemoteOperation's sleep had been interrupted"); - } - - final SyncedFolder syncedFolder = mSyncedFolderIterator.next(); - if (shouldSkipSyncedFolder(syncedFolder)) { - mSyncedFolderIterator.remove(); - continue; - } + final RemoteFileLister fileLister = new RemoteFileLister(syncedFolders, ownCloudClient); + final boolean isContentToScan = fileLister.listContentToScan(context); - if (syncedFolder.getId() == -1) { - final int syncedFolderId = (int) DbHelper.insertSyncedFolder(syncedFolder, context); - if (syncedFolderId <= 0) { - mSyncedFolderIterator.remove(); - Timber.v("insertion of syncedFolder for %s failed: %s ", syncedFolder.getRemoteFolder(), syncedFolderId); - continue; - } - syncedFolder.setId(syncedFolderId); - } - - final ReadFolderRemoteOperation operation = new ReadFolderRemoteOperation(syncedFolder.getRemoteFolder()); - final RemoteOperationResult result = operation.execute(ownCloudClient); - if (result.isSuccess()) { - final boolean dirHasChanged = onListFileSuccess(mSyncedFolderIterator, syncedFolder, result); - atLeastOneDirAsChanged = dirHasChanged || atLeastOneDirAsChanged; //stay true if set previously - } else { - if (result.getHttpCode() == HTTP_404) { //dir has not been found - atLeastOneDirAsChanged = true; - onHttp404Received(mSyncedFolderIterator, syncedFolder); - } - Timber.d("ReadFolderRemoteOperation failed : http %s, %s => ignored", result.getHttpCode(), result.getLogMessage()); - } - } - finalResult = new RemoteOperationResult<>(RemoteOperationResult.ResultCode.OK); - if (atLeastOneDirAsChanged) { + final RemoteOperationResult result = new RemoteOperationResult<>(RemoteOperationResult.ResultCode.OK); + if (isContentToScan) { DbHelper.updateSyncedFolders(this.syncedFolders, this.context); - finalResult.setResultData(remoteFiles); - } - return finalResult; - } - - - /** - * Analyze list of remote files for a given Directory - * @param iterator ListIterator contains list of syncedFolder to check - * @param syncedFolder the SyncedFolder for the given Directory - * @param result Result of ListRemoteFile operation - * @return true if some directory has changed or its content - */ - private boolean onListFileSuccess(final ListIterator iterator, SyncedFolder syncedFolder, RemoteOperationResult result) { - final int dataSize = result.getData().size(); - final RemoteFile directory = (RemoteFile) result.getData().get(0); - - if (directory.getEtag().equals(syncedFolder.getLastEtag()) - && !DbHelper.syncedFolderHasContentToDownload(syncedFolder.getId(), context)) { - return false; - } - syncedFolder.setLastEtag(directory.getEtag()).setToSync(true); - - final List subFiles = result.getData().subList(1, dataSize); - - for (int i = -1, subFilesSize = subFiles.size(); ++i < subFilesSize;) { - final RemoteFile remoteFile = (RemoteFile) subFiles.get(i); - - if (remoteFile.getMimeType().equals("DIR")) { - final String suffixPath = remoteFile.getRemotePath().substring(syncedFolder.getRemoteFolder().length()); - final SyncedFolder subSyncedFolder = new SyncedFolder(syncedFolder, suffixPath, 0L, ""); //need to set empty etag to allow it to be scan - iterator.add(subSyncedFolder); - iterator.previous(); - } else if (!isHiddenFile(remoteFile)) { - Timber.v("Add remote file %s", remoteFile.getRemotePath()); - this.remoteFiles.add(remoteFile); - } + result.setResultData(fileLister.getContentToScan()); } - return true; - } - - /** - * Handle a case where the checked directory is missing on cloud - * @param iterator ListIterator list of syncedFolder to scan - * @param syncedFolder Missing syncedFolder on cloud - */ - private void onHttp404Received(final ListIterator iterator, SyncedFolder syncedFolder) { - syncedFolder.setToSync(true); + updatedSyncedFoldersId.addAll(fileLister.getSyncedFoldersId()); - final File localFolder = new File(syncedFolder.getLocalFolder()); - final File[] files = localFolder.listFiles(); - if (files != null && files.length == 0) { - localFolder.delete(); - } - - if (!localFolder.exists()) { - if (syncedFolder.getId() > this.initialFolderNumber) { //Do not remove initial syncedFolder - DbHelper.deleteSyncedFolder(syncedFolder.getId(), context); - } - iterator.remove(); - } - } - - - /** - * Indicate if remote file is an hidden file - * @param file Remote file - * @return true if it's an hidden file - */ - private boolean isHiddenFile(RemoteFile file) { - final String fileName = CommonUtils.getFileNameFromPath(file.getRemotePath()); - return fileName != null && fileName.startsWith("."); - } - - /** - * Indicate if we should skip SyncedFolder - * @param syncedFolder - * @return - */ - private boolean shouldSkipSyncedFolder(SyncedFolder syncedFolder) { - final String fileName = CommonUtils.getFileNameFromPath(syncedFolder.getRemoteFolder()); - return (syncedFolder.isMediaType() - && fileName != null && fileName.startsWith(".")) - || !syncedFolder.isScanRemote(); + return result; } /** * @return list of syncedFolder */ - public List getSyncedFolderList(){ - return this.syncedFolders; + public List getSyncedFoldersId(){ + return this.updatedSyncedFoldersId; } } 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 7d6a2239..0da565c1 100644 --- a/app/src/main/java/foundation/e/drive/services/ObserverService.java +++ b/app/src/main/java/foundation/e/drive/services/ObserverService.java @@ -41,6 +41,7 @@ import java.util.List; import java.util.ListIterator; import foundation.e.drive.contentScanner.LocalContentScanner; +import foundation.e.drive.contentScanner.LocalFileLister; import foundation.e.drive.contentScanner.RemoteContentScanner; import foundation.e.drive.database.DbHelper; import foundation.e.drive.fileFilters.CrashlogsFileFilter; @@ -69,7 +70,6 @@ public class ObserverService extends Service implements OnRemoteOperationListene private List mSyncedFolders; //List of synced folder private boolean isWorking = false; - private int initialFolderCounter; private Account mAccount; private HashMap syncRequests; //integer is SyncedFileState id; Parcelable is the operation private SynchronizationServiceConnection synchronizationServiceConnection = new SynchronizationServiceConnection(); @@ -113,7 +113,6 @@ public class ObserverService extends Service implements OnRemoteOperationListene } this.syncRequests = new HashMap<>(); - initialFolderCounter = prefs.getInt(AppConstants.INITIALFOLDERS_NUMBER, 0); handlerThread = new HandlerThread("syncService_onResponse"); handlerThread.start(); @@ -263,7 +262,7 @@ public class ObserverService extends Service implements OnRemoteOperationListene } try { - final ListFileRemoteOperation loadOperation = new ListFileRemoteOperation(this.mSyncedFolders, this, this.initialFolderCounter); + final ListFileRemoteOperation loadOperation = new ListFileRemoteOperation(this.mSyncedFolders, this); loadOperation.execute(client, this, handler); } catch (IllegalArgumentException exception) { Timber.e(exception); @@ -288,7 +287,7 @@ public class ObserverService extends Service implements OnRemoteOperationListene } else if (settingsSyncedEnabled) { return DbHelper.getSyncedFolderList(this, false); } else { - return new ArrayList(); + return new ArrayList<>(); } } @@ -309,16 +308,15 @@ public class ObserverService extends Service implements OnRemoteOperationListene final List remoteFiles = ((RemoteOperationResult>)result).getResultData(); if (remoteFiles != null) { - final ListFileRemoteOperation listFileOperation = (ListFileRemoteOperation) operation; - mSyncedFolders = listFileOperation.getSyncedFolderList(); //The list may have been reduced if some directory hasn't changed + final List syncedFoldersId = listFileOperation.getSyncedFoldersId(); - final List syncedFileStateList = DbHelper.getSyncedFileStatesByFolders(this, - getIdsFromFolderToScan()); + final List syncedFileStates = DbHelper.getSyncedFileStatesByFolders(this, + syncedFoldersId); - if (!remoteFiles.isEmpty() || !syncedFileStateList.isEmpty()) { + if (!remoteFiles.isEmpty() || !syncedFileStates.isEmpty()) { final RemoteContentScanner scanner = new RemoteContentScanner(getApplicationContext(), mAccount, mSyncedFolders); - syncRequests.putAll(scanner.scanContent(remoteFiles, syncedFileStateList)); + syncRequests.putAll(scanner.scanContent(remoteFiles, syncedFileStates)); } } @@ -350,22 +348,6 @@ public class ObserverService extends Service implements OnRemoteOperationListene } } - /** - * Method to get Id of SyncedFolder to scan - * @return List id of SyncedFolder to scan - */ - private List getIdsFromFolderToScan() { - final List result = new ArrayList<>(); - for (int i = -1, size = this.mSyncedFolders.size(); ++i < size;) { - final SyncedFolder syncedFolder = this.mSyncedFolders.get(i); - if (syncedFolder.isToSync() ){ - result.add( (long) syncedFolder.getId() ); - } - } - return result; - } - - /* Methods related to device Scanning */ /** @@ -405,76 +387,25 @@ public class ObserverService extends Service implements OnRemoteOperationListene */ private void scanLocalFiles(){ Timber.i("scanLocalFiles()"); - final List fileList = new ArrayList<>(); - final List folderIdList= new ArrayList<>(); - boolean contentToSyncFound = false; - if (CommonUtils.isSettingsSyncEnabled(mAccount)) generateAppListFile(); - final ListIterator iterator = mSyncedFolders.listIterator() ; - - while(iterator.hasNext()) { - final SyncedFolder syncedFolder = iterator.next(); - Timber.v("SyncedFolder : %s, %s, %s, %s, %s", syncedFolder.getLibelle(), syncedFolder.getLocalFolder(), syncedFolder.getLastModified(), syncedFolder.isScanLocal(), syncedFolder.getId()); - - //Check it's not a hidden file - final String fileName = CommonUtils.getFileNameFromPath(syncedFolder.getLocalFolder()); - if (fileName == null || fileName.startsWith(".") || !syncedFolder.isScanLocal()) { - iterator.remove(); - continue; - } - if (syncedFolder.getId() == -1) { - final int syncedFolder_id = (int) DbHelper.insertSyncedFolder(syncedFolder, this); //It will return -1 if there is an error, like an already existing folder with same value - if (syncedFolder_id <= 0) { - iterator.remove(); - continue; - } - syncedFolder.setId(syncedFolder_id); - } - - - final File localDirectory = new File(syncedFolder.getLocalFolder()); //Obtention du fichier local - Timber.v("Local Folder (last modified / exists): %s, %s", localDirectory.lastModified(),localDirectory.exists()); - - if (!localDirectory.exists()) { - contentToSyncFound = true; - folderIdList.add( (long) syncedFolder.getId() ); - continue; - } - - if (localDirectory.lastModified() > syncedFolder.getLastModified() - || DbHelper.syncedFolderHasContentToUpload(syncedFolder.getId(), getApplicationContext())) { - syncedFolder.setLastModified(localDirectory.lastModified()); //@Todo: it would be better to set it after all it's content has been synced - contentToSyncFound = true; - folderIdList.add((long) syncedFolder.getId()); - } + final LocalFileLister fileLister = new LocalFileLister(mSyncedFolders); + final boolean isContentToScan = fileLister.listContentToScan(getApplicationContext()); - final FileFilter filter = FileFilterFactory.getFileFilter( (syncedFolder.isMediaType()) ? "media" : syncedFolder.getLibelle() ); - final File[] subFiles = localDirectory.listFiles(filter); //skip hidden media files - - if (subFiles == null) continue; - for (File subFile : subFiles) { - if (subFile.isDirectory()) { - final SyncedFolder subSyncedFolder = new SyncedFolder(syncedFolder, subFile.getName() + FileUtils.PATH_SEPARATOR, 0L, "");//Need to set lastModified to 0 to handle it on next iteration - iterator.add(subSyncedFolder); - iterator.previous(); - } else if (contentToSyncFound) { - Timber.v("added %s into list of file to scan", subFile.getAbsolutePath()); - fileList.add(subFile); - } - } + if (!isContentToScan) { + return; } - if (contentToSyncFound) { - DbHelper.updateSyncedFolders(mSyncedFolders, this); //@ToDo: maybe do this when all contents will be synced. - final List syncedFileStates = DbHelper.getSyncedFileStatesByFolders(this, - folderIdList); + final List fileList = fileLister.getContentToScan(); + final List folderIdList = fileLister.getSyncedFoldersId(); - if (!syncedFileStates.isEmpty() || !fileList.isEmpty() ) { - final LocalContentScanner scanner= new LocalContentScanner(getApplicationContext(), mAccount, mSyncedFolders); - syncRequests.putAll(scanner.scanContent(fileList, syncedFileStates)); - } + final List syncedFileStates = DbHelper.getSyncedFileStatesByFolders(this, + folderIdList); + + if (!syncedFileStates.isEmpty() || !fileList.isEmpty() ) { + final LocalContentScanner scanner= new LocalContentScanner(getApplicationContext(), mAccount, mSyncedFolders); + syncRequests.putAll(scanner.scanContent(fileList, syncedFileStates)); } } /* end of methods related to device Scanning */ -- GitLab From 7ff8356ffb1c36152e66955abee705e5b6f392d0 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Thu, 17 Nov 2022 13:00:25 +0100 Subject: [PATCH 09/12] fix path issue with syncedFolder --- .../contentScanner/AbstractFileLister.java | 4 +++- .../drive/contentScanner/LocalFileLister.java | 2 +- .../contentScanner/LocalFileListerTest.java | 18 +++++++++--------- 3 files changed, 13 insertions(+), 11 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java b/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java index 4515cb31..5d9df206 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java @@ -11,6 +11,8 @@ import android.content.Context; import androidx.annotation.NonNull; +import com.owncloud.android.lib.resources.files.FileUtils; + import java.util.ArrayList; import java.util.List; import java.util.ListIterator; @@ -174,7 +176,7 @@ public abstract class AbstractFileLister { if (fileName == null) continue; if (isDirectory(subFile)) { - final SyncedFolder subSyncedFolder = new SyncedFolder(syncedFolder, fileName, 0L, "");//Need to set lastModified to 0 to handle it on next iteration + final SyncedFolder subSyncedFolder = new SyncedFolder(syncedFolder, fileName + FileUtils.PATH_SEPARATOR, 0L, "");//Need to set lastModified to 0 to handle it on next iteration iterator.add(subSyncedFolder); iterator.previous(); } else if (syncedFolder.isToSync() && !skipFile(subFile)) { diff --git a/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java b/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java index 378c52ff..e2bca5fd 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java @@ -57,7 +57,7 @@ public class LocalFileLister extends AbstractFileLister { @Override protected String getFileName(@NonNull File file) { - return "/" + file.getName(); + return file.getName(); } @Override diff --git a/app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java b/app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java index b0b9c720..edcc9a0f 100644 --- a/app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java +++ b/app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java @@ -303,16 +303,16 @@ public class LocalFileListerTest { */ if (!testRootDirectory.mkdirs()) return false; - final File folder1 = new File(testRootDirectory, "folder1/"); + final File folder1 = new File(testRootDirectory, "folder1"); if (!folder1.mkdirs()) return false; - final File folder2 = new File(folder1, "folder2/"); + final File folder2 = new File(folder1, "folder2"); if (!folder2.mkdirs()) return false; - final File folder3 = new File(testRootDirectory, "folder3/"); + final File folder3 = new File(testRootDirectory, "folder3"); if (!folder3.mkdirs()) return false; - final File folder4 = new File(folder3, "folder4/"); + final File folder4 = new File(folder3, "folder4"); if (!folder4.mkdirs()) return false; - final File folder5 = new File(folder3, ".folder5/"); + final File folder5 = new File(folder3, ".folder5"); if (!folder5.mkdirs()) return false; @@ -347,16 +347,16 @@ public class LocalFileListerTest { /* * Generate SyncedFolders */ - final SyncedFolder syncedFolder1 = new SyncedFolder("any", folder1.getAbsolutePath(), "/remote/path/myFolder1/", true); + final SyncedFolder syncedFolder1 = new SyncedFolder("any", folder1.getAbsolutePath() + "/", "/remote/path/myFolder1/", true); syncedFolder1.setLastModified(folder1.lastModified()); syncedFolder1.setId((int) DbHelper.insertSyncedFolder(syncedFolder1, context)); - final SyncedFolder syncedFolder2 = new SyncedFolder("any", folder2.getAbsolutePath(), "/remote/path/myFolder2/", true); + final SyncedFolder syncedFolder2 = new SyncedFolder("any", folder2.getAbsolutePath() + "/", "/remote/path/myFolder2/", true); syncedFolder2.setLastModified(0L); syncedFolder2.setId((int) DbHelper.insertSyncedFolder(syncedFolder2, context)); - final SyncedFolder syncedFolder3 = new SyncedFolder("any", folder3.getAbsolutePath(), "/remote/path/myFolder3/", true); + final SyncedFolder syncedFolder3 = new SyncedFolder("any", folder3.getAbsolutePath() + "/", "/remote/path/myFolder3/", true); syncedFolder3.setLastModified(folder3.lastModified()); syncedFolder3.setId((int) DbHelper.insertSyncedFolder(syncedFolder3, context)); - final SyncedFolder syncedFolderRemoved = new SyncedFolder("any", testRootDirectory.getAbsolutePath() + "lambda/", "/remote/lambda", true); + final SyncedFolder syncedFolderRemoved = new SyncedFolder("any", testRootDirectory.getAbsolutePath() + "/lambda/", "/remote/lambda", true); syncedFolderRemoved.setLastModified(987654321L); syncedFolderRemoved.setLastEtag("123456789"); syncedFolderRemoved.setId((int) DbHelper.insertSyncedFolder(syncedFolderRemoved, context)); -- GitLab From b9c03854d29e029b9240ac9ff490bf6f2c01ffd6 Mon Sep 17 00:00:00 2001 From: Jonathan Klee Date: Fri, 18 Nov 2022 08:35:32 +0000 Subject: [PATCH 10/12] Apply 8 suggestion(s) to 5 file(s) --- .../contentScanner/AbstractFileLister.java | 75 +++++++++++-------- .../drive/contentScanner/LocalFileLister.java | 22 +++--- .../contentScanner/RemoteFileLister.java | 8 +- 3 files changed, 59 insertions(+), 46 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java b/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java index 5d9df206..a8dd9d4c 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java @@ -29,15 +29,26 @@ import timber.log.Timber; */ public abstract class AbstractFileLister { + /** + * Check if the given syncedFolder can be skip + * @param syncedFolder The syncedFolder instance + * @return true if we can skip + */ protected abstract boolean skipSyncedFolder(@NonNull SyncedFolder syncedFolder); + + /** + * Check if the file can be skip or if it should be added to the list to scan + * @param file file Object to check + * @return true if file must be ignored + */ protected abstract boolean skipFile(@NonNull T file); /** * If the given folder has not changed, compared to database's data, skip it - * @param currentDirectory - * @param syncedFolder - * @param context - * @return + * @param currentDirectory The real latest state of the directory + * @param syncedFolder The last known state of the directory + * @param context Context used to access database, to look for unsynced file for the given directory + * @return true if we can skip this directory */ protected abstract boolean skipDirectory(@NonNull T currentDirectory, @NonNull SyncedFolder syncedFolder, @NonNull Context context); protected abstract boolean isDirectory(@NonNull T file); @@ -91,17 +102,17 @@ public abstract class AbstractFileLister { /** * Detailed process for reading a single folder - * @param syncedFolder the SyncedFolder to check + * @param folder the SyncedFolder to check * @param iterator iterator over list of SyncedFolder * @param context context used to call DB * @return true the directory has content to synchronized false otherwise */ - private boolean hasDirectoryContentToScan(@NonNull SyncedFolder syncedFolder, @NonNull ListIterator iterator, @NonNull Context context) { + private boolean hasDirectoryContentToScan(@NonNull SyncedFolder folder, @NonNull ListIterator iterator, @NonNull Context context) { final FolderLoader dirLoader = getFolderLoader(); - if (skipSyncedFolder(syncedFolder) - || !isSyncedFolderInDb(syncedFolder, context) - || !dirLoader.load(syncedFolder)) { // I try to get the real directory (File or RemoteFile). + if (skipSyncedFolder(folder) + || !isSyncedFolderInDb(folder, context) + || !dirLoader.load(folder)) { // I try to get the real directory (File or RemoteFile). iterator.remove(); return false; } @@ -112,9 +123,9 @@ public abstract class AbstractFileLister { return true; } - if (skipDirectory(currentDirectory.getFolder(), syncedFolder, context)) { + if (skipDirectory(currentDirectory.getFolder(), folder, context)) { iterator.remove(); - syncedFolder.setToSync(false); + folder.setToSync(false); /** * LocalFileLister need to be able to access subFiles even for skipped directory * because A folder's lastModified value won't change if a subfolder's content is changed, removed or created @@ -122,15 +133,15 @@ public abstract class AbstractFileLister { */ if (!diveIntoSkippedDir) return false; } else { - syncedFolder.setToSync(true); + folder.setToSync(true); } - updateSyncedFolder(syncedFolder, currentDirectory.getFolder()); + updateSyncedFolder(folder, currentDirectory.getFolder()); //todo: look where to put in subclasses : syncedFolder.setLastEtag(directory.getEtag()).setToSync(true); final List dirContent = currentDirectory.getContent(); if (dirContent != null && !dirContent.isEmpty()) { - contentToScan.addAll(sortContent(iterator, syncedFolder, dirContent)); + contentToScan.addAll(sortContent(iterator, folder, dirContent)); } return true; } @@ -139,20 +150,20 @@ public abstract class AbstractFileLister { /** * Tell if the given syncedFolder has been persisted in database. * If it's not the case initally, it tries to persist it. - * @param syncedFolder SyncedFolder to check for persistance + * @param folder SyncedFolder to check for persistance * @param context context to contact database * @return false if not persisted in db */ - private boolean isSyncedFolderInDb(@NonNull SyncedFolder syncedFolder, @NonNull Context context) { - if (syncedFolder.getId() > 0) return true; + private boolean isSyncedFolderInDb(@NonNull SyncedFolder folder, @NonNull Context context) { + if (folder.getId() > 0) return true; - final int syncedFolder_id = (int) DbHelper.insertSyncedFolder(syncedFolder, context); //It will return -1 if there is an error, like an already existing folder with same value - if (syncedFolder_id <= 0) { - Timber.v("insertion of syncedFolder for %s failed: %s ", syncedFolder.getRemoteFolder(), syncedFolder_id); + final int syncedFolderId = (int) DbHelper.insertSyncedFolder(folder, context); //It will return -1 if there is an error, like an already existing folder with same value + if (syncedFolderId <= 0) { + Timber.v("insertion of syncedFolder for %s failed: %s ", folder.getRemoteFolder(), syncedFolderId); return false; } - syncedFolder.setId(syncedFolder_id); + folder.setId(syncedFolderId); return true; } @@ -161,27 +172,27 @@ public abstract class AbstractFileLister { * Split content of a folder: subfolder on one side are added into the iterator loop * while subfiles are added to result list * @param iterator iterator intance over list of SyncedFolder - * @param syncedFolder the SyncFolder which own the content + * @param folder the SyncFolder which own the content * @param dirContent Content to sort * @return List of subfiles to scan or empty list if nothing */ - private List sortContent(@NonNull ListIterator iterator, @NonNull SyncedFolder syncedFolder, List dirContent) { + private List sortContent(@NonNull ListIterator iterator, @NonNull SyncedFolder folder, List dirContent) { final List result = new ArrayList<>(); if (dirContent == null) return result; - for (T subFile : dirContent) { - if (subFile == null) continue; + for (T file : dirContent) { + if (file == null) continue; - final String fileName = getFileName(subFile); + final String fileName = getFileName(file); if (fileName == null) continue; - if (isDirectory(subFile)) { - final SyncedFolder subSyncedFolder = new SyncedFolder(syncedFolder, fileName + FileUtils.PATH_SEPARATOR, 0L, "");//Need to set lastModified to 0 to handle it on next iteration + if (isDirectory(file)) { + final SyncedFolder subSyncedFolder = new SyncedFolder(folder, fileName + FileUtils.PATH_SEPARATOR, 0L, "");//Need to set lastModified to 0 to handle it on next iteration iterator.add(subSyncedFolder); iterator.previous(); - } else if (syncedFolder.isToSync() && !skipFile(subFile)) { + } else if (folder.isToSync() && !skipFile(file)) { Timber.v("added %s into list of file to scan", fileName); - result.add(subFile); + result.add(file); } } @@ -202,8 +213,8 @@ public abstract class AbstractFileLister { */ public List getSyncedFoldersId() { final List result = new ArrayList<>(); - for (SyncedFolder syncedFolder : folders) { - result.add((long) syncedFolder.getId()); + for (SyncedFolder folder : folders) { + result.add((long) folder.getId()); } return result; } diff --git a/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java b/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java index e2bca5fd..8ac205ef 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java @@ -85,7 +85,7 @@ public class LocalFileLister extends AbstractFileLister { * It is expected to always return true. Its remote equivalent (in RemoteFileLister) * Might return false because of network, & RemoteFile classes. But that's not the case * here. - * @param syncedFolder + * @param syncedFolder SyncedFolder instance with data about the folder to load * @return always true because it's about "File" instance. */ @Override @@ -95,16 +95,18 @@ public class LocalFileLister extends AbstractFileLister { if (!dir.exists()) { folder = new FolderWrapper(); //missing Folder Wrapper - } else { - folder = new FolderWrapper(dir); - final String category = syncedFolder.isMediaType() ? "media" : syncedFolder.getLibelle(); - - final FileFilter filter = FileFilterFactory.getFileFilter(category); - final File[] subFiles = dir.listFiles(filter); - if (subFiles != null) { - folder.addContent(Arrays.asList(subFiles)); - } + return true; + } + + folder = new FolderWrapper(dir); + final String category = syncedFolder.isMediaType() ? "media" : syncedFolder.getLibelle(); + + final FileFilter filter = FileFilterFactory.getFileFilter(category); + final File[] files = dir.listFiles(filter); + if (files != null) { + folder.addContent(Arrays.asList(files)); } + return true; } } diff --git a/app/src/main/java/foundation/e/drive/contentScanner/RemoteFileLister.java b/app/src/main/java/foundation/e/drive/contentScanner/RemoteFileLister.java index 99f9a257..260cb991 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/RemoteFileLister.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/RemoteFileLister.java @@ -38,10 +38,10 @@ public class RemoteFileLister extends AbstractFileLister { } @Override - protected boolean skipSyncedFolder(@NonNull SyncedFolder syncedFolder) { - return (syncedFolder.isMediaType() - && CommonUtils.getFileNameFromPath(syncedFolder.getRemoteFolder()).startsWith(".")) - || !syncedFolder.isScanRemote(); + protected boolean skipSyncedFolder(@NonNull SyncedFolder folder) { + return (folder.isMediaType() + && CommonUtils.getFileNameFromPath(folder.getRemoteFolder()).startsWith(".")) + || !folder.isScanRemote(); } @Override -- GitLab From ee0596d96c65ad2d2423264a6ebaeabbc1f3226f Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Fri, 18 Nov 2022 11:59:39 +0100 Subject: [PATCH 11/12] - AbstractFileLister & subclasses: rename getFolderLoader() into createFolderLoader() (Apply Jonathan's suggestion) - Improve FolderWrapper: remove useless variable (apply Sayantan's suggestion) --- .../drive/contentScanner/AbstractFileLister.java | 4 ++-- .../e/drive/contentScanner/FolderWrapper.java | 5 +---- .../e/drive/contentScanner/LocalFileLister.java | 2 +- .../e/drive/contentScanner/RemoteFileLister.java | 2 +- .../contentScanner/LocalFileListerTest.java | 6 +++--- .../contentScanner/RemoteFileListerTest.java | 16 +++++++--------- 6 files changed, 15 insertions(+), 20 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java b/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java index a8dd9d4c..ceb77e1c 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java @@ -53,7 +53,7 @@ public abstract class AbstractFileLister { protected abstract boolean skipDirectory(@NonNull T currentDirectory, @NonNull SyncedFolder syncedFolder, @NonNull Context context); protected abstract boolean isDirectory(@NonNull T file); protected abstract String getFileName(@NonNull T file); - protected abstract FolderLoader getFolderLoader(); + protected abstract FolderLoader createFolderLoader(); protected abstract void updateSyncedFolder(@NonNull SyncedFolder syncedFolder,@NonNull T folder); /** @@ -109,7 +109,7 @@ public abstract class AbstractFileLister { */ private boolean hasDirectoryContentToScan(@NonNull SyncedFolder folder, @NonNull ListIterator iterator, @NonNull Context context) { - final FolderLoader dirLoader = getFolderLoader(); + final FolderLoader dirLoader = createFolderLoader(); if (skipSyncedFolder(folder) || !isSyncedFolderInDb(folder, context) || !dirLoader.load(folder)) { // I try to get the real directory (File or RemoteFile). diff --git a/app/src/main/java/foundation/e/drive/contentScanner/FolderWrapper.java b/app/src/main/java/foundation/e/drive/contentScanner/FolderWrapper.java index 6fb4d307..a1a03caf 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/FolderWrapper.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/FolderWrapper.java @@ -23,18 +23,15 @@ import java.util.List; */ public class FolderWrapper { - private final boolean missing; private final T folder; private final List content; protected FolderWrapper(@NonNull T folder) { content = new ArrayList<>(); this.folder = folder; - missing = false; } protected FolderWrapper() { - missing = true; folder = null; content = null; } @@ -65,6 +62,6 @@ public class FolderWrapper { * @return */ public boolean isMissing() { - return missing; + return folder == null; } } diff --git a/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java b/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java index 8ac205ef..934718ac 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java @@ -61,7 +61,7 @@ public class LocalFileLister extends AbstractFileLister { } @Override - protected LocalFolderLoader getFolderLoader() { + protected LocalFolderLoader createFolderLoader() { return new LocalFolderLoader(); } diff --git a/app/src/main/java/foundation/e/drive/contentScanner/RemoteFileLister.java b/app/src/main/java/foundation/e/drive/contentScanner/RemoteFileLister.java index 260cb991..440548e9 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/RemoteFileLister.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/RemoteFileLister.java @@ -67,7 +67,7 @@ public class RemoteFileLister extends AbstractFileLister { } @Override - protected RemoteFolderLoader getFolderLoader() { + protected RemoteFolderLoader createFolderLoader() { return new RemoteFolderLoader(); } diff --git a/app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java b/app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java index edcc9a0f..ee0d7b75 100644 --- a/app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java +++ b/app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java @@ -196,7 +196,7 @@ public class LocalFileListerTest { final SyncedFolder folder = new SyncedFolder("any", dirPath, "/remote/path/test", true); - final LocalFileLister.LocalFolderLoader folderLoader = fileListerUnderTest.getFolderLoader(); + final LocalFileLister.LocalFolderLoader folderLoader = fileListerUnderTest.createFolderLoader(); final boolean loadSuccess = folderLoader.load(folder); Assert.assertTrue("Loading remote folder resulting in 404 should return true", loadSuccess); @@ -213,7 +213,7 @@ public class LocalFileListerTest { Assert.assertTrue("Cannot create test folder: " + dirPath, dirCreationSucceed); - final LocalFileLister.LocalFolderLoader folderLoader = fileListerUnderTest.getFolderLoader(); + final LocalFileLister.LocalFolderLoader folderLoader = fileListerUnderTest.createFolderLoader(); final boolean loadSuccess = folderLoader.load(folder); final FolderWrapper wrapper = folderLoader.getFolderWrapper(); @@ -239,7 +239,7 @@ public class LocalFileListerTest { Assert.fail("IO Exception: Can't create new file as content of test folder"); } - final LocalFileLister.LocalFolderLoader folderLoader = fileListerUnderTest.getFolderLoader(); + final LocalFileLister.LocalFolderLoader folderLoader = fileListerUnderTest.createFolderLoader(); final boolean loadSuccess = folderLoader.load(folder); final FolderWrapper wrapper = folderLoader.getFolderWrapper(); diff --git a/app/src/test/java/foundation/e/drive/contentScanner/RemoteFileListerTest.java b/app/src/test/java/foundation/e/drive/contentScanner/RemoteFileListerTest.java index e7c86ee3..5e3e2431 100644 --- a/app/src/test/java/foundation/e/drive/contentScanner/RemoteFileListerTest.java +++ b/app/src/test/java/foundation/e/drive/contentScanner/RemoteFileListerTest.java @@ -28,12 +28,10 @@ import org.junit.Ignore; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mockito; -import org.mockito.internal.configuration.injection.MockInjection; 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.List; @@ -250,7 +248,7 @@ public class RemoteFileListerTest { when(fakeResult.getHttpCode()).thenReturn(404); when(fakeResult.isSuccess()).thenReturn(false); - final RemoteFileLister.RemoteFolderLoader mockLoaderSpied = Mockito.spy(fileListerUnderTest.getFolderLoader()); + final RemoteFileLister.RemoteFolderLoader mockLoaderSpied = Mockito.spy(fileListerUnderTest.createFolderLoader()); when(mockLoaderSpied.readRemoteFolder(remotePath, ocClient)).thenReturn(fakeResult); final boolean loadSuccess = mockLoaderSpied.load(folder); @@ -266,7 +264,7 @@ public class RemoteFileListerTest { when(fakeResult.getHttpCode()).thenReturn(405); when(fakeResult.isSuccess()).thenReturn(false); - final RemoteFileLister.RemoteFolderLoader mockLoaderSpied = Mockito.spy(fileListerUnderTest.getFolderLoader()); + final RemoteFileLister.RemoteFolderLoader mockLoaderSpied = Mockito.spy(fileListerUnderTest.createFolderLoader()); when(mockLoaderSpied.readRemoteFolder(remotePath, ocClient)).thenReturn(fakeResult); final boolean loadSuccess = mockLoaderSpied.load(folder); @@ -287,7 +285,7 @@ public class RemoteFileListerTest { fakeData.add(mockRemoteFile); when(fakeResult.getData()).thenReturn(fakeData); - final RemoteFileLister.RemoteFolderLoader spiedLoaderUnderTest = Mockito.spy(fileListerUnderTest.getFolderLoader()); + final RemoteFileLister.RemoteFolderLoader spiedLoaderUnderTest = Mockito.spy(fileListerUnderTest.createFolderLoader()); when(spiedLoaderUnderTest.readRemoteFolder(remotePath, ocClient)).thenReturn(fakeResult); final boolean loadSuccess = spiedLoaderUnderTest.load(folder); @@ -313,7 +311,7 @@ public class RemoteFileListerTest { fakeData.add(mockContentRemoteFile2); when(fakeResult.getData()).thenReturn(fakeData); - final RemoteFileLister.RemoteFolderLoader spiedLoaderUnderTest = Mockito.spy(fileListerUnderTest.getFolderLoader()); + final RemoteFileLister.RemoteFolderLoader spiedLoaderUnderTest = Mockito.spy(fileListerUnderTest.createFolderLoader()); when(spiedLoaderUnderTest.readRemoteFolder(remotePath, ocClient)).thenReturn(fakeResult); final boolean loadSuccess = spiedLoaderUnderTest.load(folder); @@ -336,7 +334,7 @@ public class RemoteFileListerTest { final ArrayList fakeData = new ArrayList<>(); when(fakeResult.getData()).thenReturn(fakeData); - final RemoteFileLister.RemoteFolderLoader mockLoaderSpied = Mockito.spy(fileListerUnderTest.getFolderLoader()); + final RemoteFileLister.RemoteFolderLoader mockLoaderSpied = Mockito.spy(fileListerUnderTest.createFolderLoader()); when(mockLoaderSpied.readRemoteFolder(remotePath, ocClient)).thenReturn(fakeResult); final boolean loadSuccess = mockLoaderSpied.load(folder); @@ -459,7 +457,7 @@ public class RemoteFileListerTest { /* Mock the method depending to network and cloud */ - final RemoteFileLister.RemoteFolderLoader spiedDirLoader = Mockito.spy(fileListerUnderTest.getFolderLoader()); + final RemoteFileLister.RemoteFolderLoader spiedDirLoader = Mockito.spy(fileListerUnderTest.createFolderLoader()); final RemoteOperationResult folder1Result = Mockito.mock(RemoteOperationResult.class); when(folder1Result.getHttpCode()).thenReturn(207); @@ -515,6 +513,6 @@ public class RemoteFileListerTest { when(missingFolderResult.isSuccess()).thenReturn(false); when(spiedDirLoader.readRemoteFolder(syncedFolderRemoved.getRemoteFolder(), ocClient)).thenReturn(missingFolderResult); - when(fileListerUnderTest.getFolderLoader()).thenReturn(spiedDirLoader); + when(fileListerUnderTest.createFolderLoader()).thenReturn(spiedDirLoader); } } -- GitLab From 55edf442a8a0e430f9c69f4ebcb9af2c79708617 Mon Sep 17 00:00:00 2001 From: Jonathan Klee Date: Sat, 19 Nov 2022 23:33:14 +0000 Subject: [PATCH 12/12] Apply 1 suggestion(s) to 1 file(s) --- .../foundation/e/drive/contentScanner/AbstractFileLister.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java b/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java index ceb77e1c..1f4284cd 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java @@ -62,8 +62,8 @@ public abstract class AbstractFileLister { * @param RemoteFile or File */ /* package */ interface FolderLoader { - /* package */ FolderWrapper getFolderWrapper(); - /* package */ boolean load(@NonNull SyncedFolder folder); + FolderWrapper getFolderWrapper(); + boolean load(@NonNull SyncedFolder folder); } protected final List folders; -- GitLab