From 9c865baf2a6240a3763aad0a7753c42ae9a75085 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Wed, 18 Oct 2023 11:10:31 +0200 Subject: [PATCH 01/11] implementation of SyncManager class --- .../e/drive/synchronization/SyncManager.kt | 155 ++++++++++++++++++ 1 file changed, 155 insertions(+) create mode 100644 app/src/main/java/foundation/e/drive/synchronization/SyncManager.kt diff --git a/app/src/main/java/foundation/e/drive/synchronization/SyncManager.kt b/app/src/main/java/foundation/e/drive/synchronization/SyncManager.kt new file mode 100644 index 00000000..7eeba415 --- /dev/null +++ b/app/src/main/java/foundation/e/drive/synchronization/SyncManager.kt @@ -0,0 +1,155 @@ +/* + * Copyright © MURENA SAS 2023. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the GNU Public License v3.0 + * which accompanies this distribution, and is available at + * http://www.gnu.org/licenses/gpl.html + */ +package foundation.e.drive.synchronization + +import android.content.Context +import foundation.e.drive.database.FailedSyncPrefsManager +import foundation.e.drive.models.SyncRequest +import foundation.e.drive.models.SyncWrapper +import java.util.concurrent.ConcurrentHashMap +import java.util.concurrent.ConcurrentLinkedQueue + +/** + * This interface is used to restrict access to a limited set of method + * @author Vincent Bourgmayer + */ +interface ISyncRequestCollector { + fun queueSyncRequest(request: SyncRequest, context: Context) + fun queueSyncRequests(requests: MutableCollection, context: Context) +} + +/** + * This interface is used to restrict access to a limited set of method + * @author Vincent Bourgmayer + */ +interface ISyncManager { + fun pollSyncRequest(): SyncRequest? + + fun declareStartedSync(threadId: Int, syncWrapper: SyncWrapper) + + fun isQueueEmpty(): Boolean +} + +/** + * This class goals is to act as a proxy between file's change detection & performing synchronization + * todo 1. should I rename to SyncProxy ? I feel that it is less clear what it is + * todo 2. Shouldn't I use an Object instead of a class ? + * todo 3. one improvement could be to handle file that has just been synced in order to prevent instant detection to trigger a syncRequest in reaction (i.e in case of a download) + * + * This object must allow concurrent access between (periodic | instant) file's change detection and synchronization + * it holds the SyncRequest Queue and a list of running sync + * @author Vincent Bourgmayer + */ +class SyncManager private constructor(): ISyncRequestCollector, ISyncManager { + private val syncRequestQueue: ConcurrentLinkedQueue = ConcurrentLinkedQueue() //could we use channel instead ? + private val startedSync: ConcurrentHashMap = ConcurrentHashMap() + + companion object { + @Volatile + private var instance: SyncManager? = null + + fun getInstance(): SyncManager { + if (instance == null) { + synchronized(this) { + if (instance == null) { + instance = SyncManager() + } + } + } + return instance!! + } + } + + /** + * Add a SyncRequest into waiting queue if it matches some conditions: + * - an equivalent sync Request (same file & same operation type) isn't already running + * - failure limit isn't reach for this file + * + * It also remove already existing request for the same file from the waiting queue + * @param request request to add to waiting queue + * @param context used to check previous failure of file sync + */ + override fun queueSyncRequest(request: SyncRequest, context: Context) { + for (syncWrapper in startedSync.values) { + if (syncWrapper.isRunning && syncWrapper == request) { + return + } + } + + if (skipBecauseOfPreviousFail(request, context)) return + + syncRequestQueue.remove(request) + syncRequestQueue.add(request) + } + + /** + * Add a collection of SyncRequest into waiting queue if they met some conditions: + * - an equivalent sync Request (same file & same operation type) isn't already running + * - failure limit isn't reach for this file + * + * It also remove already existing request for the same file from the waiting queue + * @param requests collections of requests to add + * @param context used to check previous failure of file sync + */ + override fun queueSyncRequests(requests: MutableCollection, context: Context) { + for (syncWrapper in startedSync.values) { + if (syncWrapper.isRunning) { + requests.removeIf { obj: SyncRequest? -> + syncWrapper == obj + } + } + } + requests.removeIf { request: SyncRequest? -> + skipBecauseOfPreviousFail(request!!, context) + } + + syncRequestQueue.removeAll(requests.toSet()) + syncRequestQueue.addAll(requests) + } + + /** + * Progressively delay synchronization of a file in case of failure + * @param request SyncRequest for the given file + * @return false if file can be sync or true if it must wait + */ + private fun skipBecauseOfPreviousFail(request: SyncRequest, context: Context): Boolean { + val failedSyncPref = FailedSyncPrefsManager.getInstance(context) + val fileStateId = request.syncedFileState.id + val delay: Long = when (failedSyncPref.getFailureCounterForFile(fileStateId)) { + 0 -> return false + 1 -> 3600 //1h + 2 -> 7200 //2h + 3 -> 18000 // 5h + else -> return true + } + val timeStamp = System.currentTimeMillis() / 1000 + return timeStamp < failedSyncPref.getLastFailureTimeForFile(fileStateId) + delay + } + + override fun pollSyncRequest(): SyncRequest? { + return syncRequestQueue.poll() + } + + override fun declareStartedSync(threadId: Int, syncWrapper: SyncWrapper) { + startedSync[threadId] = syncWrapper + } + + override fun isQueueEmpty(): Boolean { + return syncRequestQueue.isEmpty() + } + + //todo Remove if no usage + fun getStartedSync(): ConcurrentHashMap { + return startedSync + } + + //todo find in which where it should be used + fun clearRequestQueue() { + syncRequestQueue.clear() + } +} \ No newline at end of file -- GitLab From 1631b3e61314576ecee9d9ae750f5c02790345d0 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Wed, 18 Oct 2023 12:53:40 +0200 Subject: [PATCH 02/11] make ObserverService to use syncManager --- .../e/drive/services/ObserverService.java | 53 +++++-------------- .../e/drive/synchronization/SyncManager.kt | 34 +++++++++++- 2 files changed, 47 insertions(+), 40 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/services/ObserverService.java b/app/src/main/java/foundation/e/drive/services/ObserverService.java index 813b02b9..ab4a20e8 100644 --- a/app/src/main/java/foundation/e/drive/services/ObserverService.java +++ b/app/src/main/java/foundation/e/drive/services/ObserverService.java @@ -14,7 +14,6 @@ import static foundation.e.drive.utils.AppConstants.SETUP_COMPLETED; import android.accounts.Account; import android.accounts.AccountManager; import android.app.Service; -import android.content.ComponentName; import android.content.Context; import android.content.Intent; import android.content.SharedPreferences; @@ -48,11 +47,12 @@ import foundation.e.drive.models.SyncedFileState; import foundation.e.drive.models.SyncedFolder; import foundation.e.drive.operations.ListFileRemoteOperation; import foundation.e.drive.receivers.DebugCmdReceiver; +import foundation.e.drive.synchronization.ISyncRequestCollector; +import foundation.e.drive.synchronization.SyncManager; import foundation.e.drive.utils.AppConstants; import foundation.e.drive.utils.CommonUtils; import foundation.e.drive.utils.DavClientProvider; import foundation.e.drive.utils.ServiceExceptionHandler; -import foundation.e.drive.utils.SynchronizationServiceConnection; import timber.log.Timber; /** @@ -62,26 +62,15 @@ import timber.log.Timber; * This service look for remote or looale file to synchronize */ public class ObserverService extends Service implements OnRemoteOperationListener{ - private final static int INTERSYNC_MINIMUM_DELAY = 900000; // min delay between two sync in ms. + private final static int INTERSYNC_MINIMUM_DELAY = 900000; // min delay execution two sync in ms. private List mSyncedFolders; //List of synced folder private boolean isWorking = false; private Account mAccount; private HashMap syncRequests; //integer is SyncedFileState id; Parcelable is the operation - private SynchronizationServiceConnection synchronizationServiceConnection = new SynchronizationServiceConnection() { - @Override - public void onServiceConnected(@Nullable ComponentName componentName, @NonNull IBinder iBinder) { - super.onServiceConnected(componentName, iBinder); - final SharedPreferences prefs = getApplicationContext().getSharedPreferences(AppConstants.SHARED_PREFERENCE_NAME, Context.MODE_PRIVATE); - - if (!checkStartCondition(prefs, forcedSync)) { - stopSelf(); - return; - } - begin(); - } - }; + private final ISyncRequestCollector syncManager = SyncManager.Companion.getInstance(); + private Handler handler; private HandlerThread handlerThread; // protected to avoid SyntheticAccessor @@ -91,7 +80,6 @@ public class ObserverService extends Service implements OnRemoteOperationListene @Override public void onDestroy(){ Timber.v("onDestroy()"); - synchronizationServiceConnection.unBind(getApplicationContext()); if (handlerThread != null) handlerThread.quitSafely(); mSyncedFolders = null; super.onDestroy(); @@ -101,7 +89,6 @@ public class ObserverService extends Service implements OnRemoteOperationListene public void onCreate() { super.onCreate(); Timber.tag(ObserverService.class.getSimpleName()); - synchronizationServiceConnection.bindService(getApplicationContext()); } @Override @@ -120,6 +107,12 @@ public class ObserverService extends Service implements OnRemoteOperationListene this.syncRequests = new HashMap<>(); + if (!checkStartCondition(prefs, forcedSync)) { + stopSelf(); + return START_NOT_STICKY; + } + + begin(); return START_STICKY; } @@ -178,10 +171,8 @@ public class ObserverService extends Service implements OnRemoteOperationListene return false; } - final SynchronizationService service = - synchronizationServiceConnection.getSynchronizationService(); - final boolean isSyncServicePaused = service != null && service.pauseSync(); + final boolean isSyncServicePaused = syncManager.pauseSync(); Timber.d("isSyncServicePaused ? %s", isSyncServicePaused); return isSyncServicePaused; } @@ -330,14 +321,11 @@ public class ObserverService extends Service implements OnRemoteOperationListene startScan(false); - SynchronizationService service = synchronizationServiceConnection.getSynchronizationService(); - if (service != null) { - service.unPauseSync(); - } + syncManager.unpauseSync(); if (!syncRequests.isEmpty()) { Timber.d("syncRequests contains %s", syncRequests.size()); - passSyncRequestsToSynchronizationService(); + syncManager.queueSyncRequests(syncRequests.values(), getApplicationContext()); } else { Timber.i("There is no file to sync."); getSharedPreferences(AppConstants.SHARED_PREFERENCE_NAME, Context.MODE_PRIVATE) @@ -350,19 +338,6 @@ public class ObserverService extends Service implements OnRemoteOperationListene this.stopSelf(); } - /** - * Send all gathered SyncRequest to SynchronizationService - */ - private void passSyncRequestsToSynchronizationService() { - SynchronizationService service = synchronizationServiceConnection.getSynchronizationService(); - if (service != null) { - service.queueSyncRequests(syncRequests.values()); - service.startSynchronization(); - } else { - Timber.e("ERROR: binding to SynchronizationService lost"); - } - } - /** * Prepare the list of files and SyncedFileState for synchronisation */ diff --git a/app/src/main/java/foundation/e/drive/synchronization/SyncManager.kt b/app/src/main/java/foundation/e/drive/synchronization/SyncManager.kt index 7eeba415..3399802c 100644 --- a/app/src/main/java/foundation/e/drive/synchronization/SyncManager.kt +++ b/app/src/main/java/foundation/e/drive/synchronization/SyncManager.kt @@ -11,6 +11,7 @@ import android.content.Context import foundation.e.drive.database.FailedSyncPrefsManager import foundation.e.drive.models.SyncRequest import foundation.e.drive.models.SyncWrapper +import timber.log.Timber import java.util.concurrent.ConcurrentHashMap import java.util.concurrent.ConcurrentLinkedQueue @@ -21,6 +22,9 @@ import java.util.concurrent.ConcurrentLinkedQueue interface ISyncRequestCollector { fun queueSyncRequest(request: SyncRequest, context: Context) fun queueSyncRequests(requests: MutableCollection, context: Context) + + fun pauseSync(): Boolean + fun unpauseSync() } /** @@ -33,6 +37,10 @@ interface ISyncManager { fun declareStartedSync(threadId: Int, syncWrapper: SyncWrapper) fun isQueueEmpty(): Boolean + + fun isPaused(): Boolean + + fun clearRequestQueue() } /** @@ -48,6 +56,9 @@ interface ISyncManager { class SyncManager private constructor(): ISyncRequestCollector, ISyncManager { private val syncRequestQueue: ConcurrentLinkedQueue = ConcurrentLinkedQueue() //could we use channel instead ? private val startedSync: ConcurrentHashMap = ConcurrentHashMap() + private var isSyncPaused: Boolean = false + private var pauseStartTime = 0L + private val maxPauseTimeInMs: Long = 300000 //5 minutes, might need to be adapted companion object { @Volatile @@ -112,6 +123,22 @@ class SyncManager private constructor(): ISyncRequestCollector, ISyncManager { syncRequestQueue.addAll(requests) } + override fun pauseSync(): Boolean { + pauseStartTime = System.currentTimeMillis() + val isQueueEmpty = syncRequestQueue.isEmpty() + val isNoStartedSync = + startedSync.values.stream().noneMatch { obj: SyncWrapper -> obj.isRunning } + + Timber.v("is queue empty ? %s ; is no started sync ? %s", isQueueEmpty, isNoStartedSync) + val canBePaused = isQueueEmpty && isNoStartedSync + if (!canBePaused) pauseStartTime = 0L + return canBePaused + } + + override fun unpauseSync() { + pauseStartTime = 0L + } + /** * Progressively delay synchronization of a file in case of failure * @param request SyncRequest for the given file @@ -143,13 +170,18 @@ class SyncManager private constructor(): ISyncRequestCollector, ISyncManager { return syncRequestQueue.isEmpty() } + override fun isPaused(): Boolean { + return if (pauseStartTime === 0L) false + else System.currentTimeMillis() - pauseStartTime < maxPauseTimeInMs + } + //todo Remove if no usage fun getStartedSync(): ConcurrentHashMap { return startedSync } //todo find in which where it should be used - fun clearRequestQueue() { + override fun clearRequestQueue() { syncRequestQueue.clear() } } \ No newline at end of file -- GitLab From df5a8fd75fb7a5a84037ac450dbee0f45bf75857 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Wed, 18 Oct 2023 12:54:10 +0200 Subject: [PATCH 03/11] make FileObserver to use syncManager --- .../FileObservers/FileEventListener.java | 23 ++++--------------- 1 file changed, 5 insertions(+), 18 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/FileObservers/FileEventListener.java b/app/src/main/java/foundation/e/drive/FileObservers/FileEventListener.java index 2d724ef8..f9b74b5f 100644 --- a/app/src/main/java/foundation/e/drive/FileObservers/FileEventListener.java +++ b/app/src/main/java/foundation/e/drive/FileObservers/FileEventListener.java @@ -28,8 +28,8 @@ import foundation.e.drive.database.DbHelper; import foundation.e.drive.models.SyncRequest; import foundation.e.drive.models.SyncedFileState; import foundation.e.drive.models.SyncedFolder; -import foundation.e.drive.services.SynchronizationService; -import foundation.e.drive.utils.SynchronizationServiceConnection; +import foundation.e.drive.synchronization.ISyncRequestCollector; +import foundation.e.drive.synchronization.SyncManager; import timber.log.Timber; /** @@ -39,7 +39,6 @@ import timber.log.Timber; public class FileEventListener { private final Context appContext; - private final SynchronizationServiceConnection serviceConnection = new SynchronizationServiceConnection(); public FileEventListener(@NonNull Context applicationContext) { Timber.tag(FileEventListener.class.getSimpleName()); @@ -108,14 +107,10 @@ public class FileEventListener { private void sendSyncRequestToSynchronizationService(@NonNull SyncRequest request) { Timber.d("Sending a SyncRequest for %s", request.getSyncedFileState().getName()); - SynchronizationService service = serviceConnection.getSynchronizationService(); + final ISyncRequestCollector syncManager = SyncManager.Companion.getInstance(); + syncManager.queueSyncRequest(request, appContext.getApplicationContext()); + //todo service.startSynchronization(); - if (service != null) { - service.queueSyncRequest(request); - service.startSynchronization(); - } else { - Timber.d("Impossible to send SyncRequest. FileEventListener is not bound to SynchronizationService"); - } } /** @@ -245,12 +240,4 @@ public class FileEventListener { this.sendSyncRequestToSynchronizationService(disableSyncingRequest); } } - - public void unbindFromSynchronizationService(){ - serviceConnection.unBind(appContext); - } - - public void bindToSynchronizationService(){ - serviceConnection.bindService(appContext); - } } -- GitLab From ea0ed46937d1c97984265ad0d64af77f25668be9 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Wed, 18 Oct 2023 13:01:34 +0200 Subject: [PATCH 04/11] use syncManager in SynchronizationService --- .../services/SynchronizationService.java | 149 ++---------------- .../e/drive/synchronization/SyncManager.kt | 16 +- .../SynchronizationServiceConnection.java | 79 ---------- 3 files changed, 27 insertions(+), 217 deletions(-) delete mode 100644 app/src/main/java/foundation/e/drive/utils/SynchronizationServiceConnection.java diff --git a/app/src/main/java/foundation/e/drive/services/SynchronizationService.java b/app/src/main/java/foundation/e/drive/services/SynchronizationService.java index 08b7f76c..791374ac 100644 --- a/app/src/main/java/foundation/e/drive/services/SynchronizationService.java +++ b/app/src/main/java/foundation/e/drive/services/SynchronizationService.java @@ -15,7 +15,6 @@ import android.app.Service; import android.content.Context; import android.content.Intent; import android.content.SharedPreferences; -import android.os.Binder; import android.os.Handler; import android.os.HandlerThread; import android.os.IBinder; @@ -28,10 +27,7 @@ import com.owncloud.android.lib.common.operations.RemoteOperation; import com.owncloud.android.lib.common.operations.RemoteOperationResult; import java.io.File; -import java.util.Collection; import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ConcurrentLinkedDeque; import foundation.e.drive.R; import foundation.e.drive.database.DbHelper; @@ -41,6 +37,8 @@ import foundation.e.drive.models.SyncWrapper; import foundation.e.drive.models.SyncedFileState; import foundation.e.drive.operations.DownloadFileOperation; import foundation.e.drive.operations.UploadFileOperation; +import foundation.e.drive.synchronization.ISyncManager; +import foundation.e.drive.synchronization.SyncManager; import foundation.e.drive.utils.AppConstants; import foundation.e.drive.utils.CommonUtils; import foundation.e.drive.utils.DavClientProvider; @@ -51,10 +49,7 @@ import timber.log.Timber; * @author Vincent Bourgmayer */ public class SynchronizationService extends Service implements OnRemoteOperationListener, FileSyncDisabler.FileSyncDisablingListener { - private final SynchronizationBinder binder = new SynchronizationBinder(); - private final static long maxPauseTimeInMs = 300000; //5 minutes, might need to be adapted - private ConcurrentLinkedDeque syncRequestQueue; - private ConcurrentHashMap startedSync; //Integer is thread index (1 to workerAmount) + private final ISyncManager syncManager = SyncManager.Companion.getInstance(); private Account account; private final int workerAmount = 2; @@ -64,7 +59,6 @@ public class SynchronizationService extends Service implements OnRemoteOperation private OwnCloudClient ocClient; private Handler handler; private HandlerThread handlerThread; - private long pauseStartTime = 0L; @Override public void onCreate() { @@ -88,8 +82,6 @@ public class SynchronizationService extends Service implements OnRemoteOperation return START_NOT_STICKY; } - syncRequestQueue = new ConcurrentLinkedDeque<>(); - startedSync = new ConcurrentHashMap<>(); threadPool = new Thread[workerAmount]; ocClient = DavClientProvider.getInstance().getClientInstance(account, getApplicationContext()); @@ -113,114 +105,6 @@ public class SynchronizationService extends Service implements OnRemoteOperation super.onDestroy(); } - @Nullable - @Override - public IBinder onBind(@Nullable Intent intent) { - return binder; - } - - /** - * Try to pause synchronization. - * Can only be paused if no sync is already running and no request queue is started - * @return true if service successfully paused - */ - public boolean pauseSync() { - pauseStartTime = System.currentTimeMillis(); - - final boolean isQueueEmpty = syncRequestQueue.isEmpty(); - final boolean isNoStartedSync = startedSync.values().stream().noneMatch(SyncWrapper::isRunning); - - Timber.v("is queue empty ? %s ; is no started sync ? %s", isQueueEmpty, isNoStartedSync); - final boolean isPausable = isQueueEmpty && isNoStartedSync; - if (!isPausable) pauseStartTime = 0L; - return isPausable; - } - - /** - * unpause synchronization - */ - public void unPauseSync() { - pauseStartTime = 0L; - } - - private boolean isPaused() { - if (pauseStartTime == 0L) return false; - return System.currentTimeMillis() - pauseStartTime < maxPauseTimeInMs; - } - - /** - * Add a SyncRequest into waiting queue if it matches some conditions: - * - an equivalent sync Request (same file & same operation type) isn't already running - * - failure limit isn't reach for this file - * - * It also remove already existing request for the same file from the waiting queue - * @param request request to add to waiting queue - */ - public void queueSyncRequest(@NonNull SyncRequest request) { - for (SyncWrapper syncWrapper : startedSync.values()) { - //noinspection EqualsBetweenInconvertibleTypes - if (syncWrapper.isRunning() && syncWrapper.equals(request)) { - return; - } - } - if (skipBecauseOfPreviousFail(request)) return; - - syncRequestQueue.remove(request); - syncRequestQueue.add(request); - } - - /** - * Add a collection of SyncRequest into waiting queue if they met some conditions: - * - an equivalent sync Request (same file & same operation type) isn't already running - * - failure limit isn't reach for this file - * - * It also remove already existing request for the same file from the waiting queue - * @param requests collections of requests to add - */ - public void queueSyncRequests(@NonNull Collection requests) { - for (SyncWrapper syncWrapper : startedSync.values()) { - if (syncWrapper.isRunning()) { - //noinspection EqualsBetweenInconvertibleTypes - requests.removeIf(syncWrapper::equals); - } - } - requests.removeIf(this::skipBecauseOfPreviousFail); - - syncRequestQueue.removeAll(requests); - syncRequestQueue.addAll(requests); - } - - /** - * Progressively delay synchronization of a file in case of failure - * @param request SyncRequest for the given file - * @return false if file can be sync or true if it must wait - */ - private boolean skipBecauseOfPreviousFail(SyncRequest request) { - final FailedSyncPrefsManager failedSyncPref = FailedSyncPrefsManager.getInstance(getApplicationContext()); - final int fileStateId = request.getSyncedFileState().getId(); - final int failureCounter = failedSyncPref.getFailureCounterForFile(fileStateId); - - long delay; - switch (failureCounter) { - case 0: - return false; - case 1: - delay = 3600; //1h - break; - case 2: - delay = 7200; //2h - break; - case 3: - delay = 18000; // 5h - break; - default: - return true; - } - - final long timeStamp = System.currentTimeMillis() / 1000; - return timeStamp < failedSyncPref.getLastFailureTimeForFile(fileStateId) + delay; - } - public void startSynchronization(){ Timber.d("startAllThreads"); for(int i =-1; ++i < workerAmount;){ @@ -236,16 +120,16 @@ public class SynchronizationService extends Service implements OnRemoteOperation if (!isNetworkAvailable()) { Timber.d("No network available: Clear syncRequestQueue"); - syncRequestQueue.clear(); + syncManager.clearRequestQueue(); return; } - if (!canStart(threadIndex) || isPaused()) { + if (!canStart(threadIndex) || syncManager.isPaused()) { Timber.d("Can't start thread #%s, Sync is paused or thread is already running", threadIndex); return; } - final SyncRequest request = this.syncRequestQueue.poll(); //return null if empty + final SyncRequest request = this.syncManager.pollSyncRequest(); //return null if empty if (request == null) { Timber.d("Thread #%s: No more sync request to start.", threadIndex); return; @@ -263,7 +147,7 @@ public class SynchronizationService extends Service implements OnRemoteOperation final FileSyncDisabler fileSyncDisabler = new FileSyncDisabler(request.getSyncedFileState()); threadPool[threadIndex] = new Thread(fileSyncDisabler.getRunnable(handler, threadIndex, getApplicationContext(), this)); threadPool[threadIndex].start(); - startedSync.put(threadIndex, syncWrapper); + syncManager.addStartedSync(threadIndex,syncWrapper); return; } @@ -278,7 +162,7 @@ public class SynchronizationService extends Service implements OnRemoteOperation //noinspection deprecation threadPool[threadIndex] = operation.execute(ocClient, this, handler); - startedSync.put(threadIndex, syncWrapper); + syncManager.addStartedSync(threadIndex, syncWrapper); } /** @@ -287,7 +171,7 @@ public class SynchronizationService extends Service implements OnRemoteOperation * @return false if nogo */ private boolean canStart(int threadIndex) { - final SyncWrapper syncWrapper = startedSync.get(threadIndex); + final SyncWrapper syncWrapper = syncManager.getStartedSync(threadIndex); return (syncWrapper == null || !syncWrapper.isRunning()); } @@ -318,7 +202,7 @@ public class SynchronizationService extends Service implements OnRemoteOperation break; } - for (Map.Entry keyValue : startedSync.entrySet()) { + for (Map.Entry keyValue : syncManager.getStartedSync().entrySet()) { final SyncWrapper wrapper = keyValue.getValue(); final RemoteOperation wrapperOperation = wrapper.getRemoteOperation(); if (wrapperOperation != null && wrapperOperation.equals(callerOperation)) { @@ -330,7 +214,7 @@ public class SynchronizationService extends Service implements OnRemoteOperation } if (isNetworkDisconnected) { - syncRequestQueue.clear(); + syncManager.clearRequestQueue(); } } @@ -371,7 +255,7 @@ public class SynchronizationService extends Service implements OnRemoteOperation @Override public void onSyncDisabled(int threadId, boolean succeed) { - final SyncWrapper wrapper = startedSync.get(threadId); + final SyncWrapper wrapper = syncManager.getStartedSync(threadId); if (wrapper != null) { final SyncRequest request = wrapper.getRequest(); Timber.d("%s sync disabled ? %s", request.getSyncedFileState().getLocalPath(), succeed); @@ -381,10 +265,9 @@ public class SynchronizationService extends Service implements OnRemoteOperation startWorker(threadId); } - public class SynchronizationBinder extends Binder { - @NonNull - public SynchronizationService getService(){ - return SynchronizationService.this; - } + @Nullable + @Override + public IBinder onBind(@Nullable Intent intent) { + throw new UnsupportedOperationException(); } } \ No newline at end of file diff --git a/app/src/main/java/foundation/e/drive/synchronization/SyncManager.kt b/app/src/main/java/foundation/e/drive/synchronization/SyncManager.kt index 3399802c..87800af5 100644 --- a/app/src/main/java/foundation/e/drive/synchronization/SyncManager.kt +++ b/app/src/main/java/foundation/e/drive/synchronization/SyncManager.kt @@ -34,13 +34,17 @@ interface ISyncRequestCollector { interface ISyncManager { fun pollSyncRequest(): SyncRequest? - fun declareStartedSync(threadId: Int, syncWrapper: SyncWrapper) + fun addStartedSync(threadId: Int, syncWrapper: SyncWrapper) fun isQueueEmpty(): Boolean fun isPaused(): Boolean fun clearRequestQueue() + + fun getStartedSync(threadIndex: Int): SyncWrapper? + fun getStartedSync(): ConcurrentHashMap + } /** @@ -56,7 +60,6 @@ interface ISyncManager { class SyncManager private constructor(): ISyncRequestCollector, ISyncManager { private val syncRequestQueue: ConcurrentLinkedQueue = ConcurrentLinkedQueue() //could we use channel instead ? private val startedSync: ConcurrentHashMap = ConcurrentHashMap() - private var isSyncPaused: Boolean = false private var pauseStartTime = 0L private val maxPauseTimeInMs: Long = 300000 //5 minutes, might need to be adapted @@ -162,7 +165,7 @@ class SyncManager private constructor(): ISyncRequestCollector, ISyncManager { return syncRequestQueue.poll() } - override fun declareStartedSync(threadId: Int, syncWrapper: SyncWrapper) { + override fun addStartedSync(threadId: Int, syncWrapper: SyncWrapper) { startedSync[threadId] = syncWrapper } @@ -176,12 +179,15 @@ class SyncManager private constructor(): ISyncRequestCollector, ISyncManager { } //todo Remove if no usage - fun getStartedSync(): ConcurrentHashMap { + override fun getStartedSync(): ConcurrentHashMap { return startedSync } - //todo find in which where it should be used override fun clearRequestQueue() { syncRequestQueue.clear() } + + override fun getStartedSync(threadIndex: Int): SyncWrapper? { + return startedSync.get(threadIndex) + } } \ No newline at end of file diff --git a/app/src/main/java/foundation/e/drive/utils/SynchronizationServiceConnection.java b/app/src/main/java/foundation/e/drive/utils/SynchronizationServiceConnection.java deleted file mode 100644 index 1227e96c..00000000 --- a/app/src/main/java/foundation/e/drive/utils/SynchronizationServiceConnection.java +++ /dev/null @@ -1,79 +0,0 @@ -/* - * Copyright © ECORP SAS 2022. - * All rights reserved. This program and the accompanying materials - * are made available under the terms of the GNU Public License v3.0 - * which accompanies this distribution, and is available at - * http://www.gnu.org/licenses/gpl.html - */ - -package foundation.e.drive.utils; - -import android.content.ComponentName; -import android.content.Context; -import android.content.Intent; -import android.content.ServiceConnection; -import android.os.IBinder; - -import androidx.annotation.NonNull; -import androidx.annotation.Nullable; - -import foundation.e.drive.services.SynchronizationService; -import timber.log.Timber; - -/** - * @author Vincent Bourgmayer - */ -public class SynchronizationServiceConnection implements ServiceConnection { - - @Nullable - private SynchronizationService synchronizationService = null; - private boolean attemptingToBind = false; - - @Override - public void onServiceConnected(@Nullable ComponentName componentName, @NonNull IBinder iBinder) { - Timber.tag(SynchronizationServiceConnection.class.getSimpleName()) - .v("binding to SynchronizationService"); - attemptingToBind = false; - final SynchronizationService.SynchronizationBinder binder = (SynchronizationService.SynchronizationBinder) iBinder; - synchronizationService = binder.getService(); - } - - @Override - public void onServiceDisconnected(@Nullable ComponentName componentName) { - Timber.tag(SynchronizationServiceConnection.class.getSimpleName()) - .v("onServiceDisconnected()"); - synchronizationService = null; - } - - /** - * Get SynchronizationService. Might be null! - * @return - */ - @Nullable - public SynchronizationService getSynchronizationService() { - return synchronizationService; - } - - /** - * Sources: https://stackoverflow.com/questions/8614335/android-how-to-safely-unbind-a-service - * @param context - */ - public void unBind(@NonNull Context context) { - attemptingToBind = false; - if (synchronizationService != null) { - context.unbindService(this); - synchronizationService = null; - } - } - - /** - * Sources: https://stackoverflow.com/questions/8614335/android-how-to-safely-unbind-a-service - * @param context - */ - public void bindService(@NonNull Context context) { - if (!attemptingToBind) { - attemptingToBind = true; - context.bindService(new Intent(context, SynchronizationService.class), this, Context.BIND_AUTO_CREATE); - } - } -} -- GitLab From d405cbb3b5b4553c4dc2ed3d0a66d26a83f0843d Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Wed, 18 Oct 2023 15:55:58 +0200 Subject: [PATCH 05/11] apply Jonathan's suggestion & fix some warning raised by AndroidStudio --- .../FileObservers/FileEventListener.java | 6 ++--- .../e/drive/services/ObserverService.java | 6 ++--- .../services/SynchronizationService.java | 6 ++--- .../{SyncManager.kt => SyncProxy.kt} | 24 +++++++------------ 4 files changed, 17 insertions(+), 25 deletions(-) rename app/src/main/java/foundation/e/drive/synchronization/{SyncManager.kt => SyncProxy.kt} (94%) diff --git a/app/src/main/java/foundation/e/drive/FileObservers/FileEventListener.java b/app/src/main/java/foundation/e/drive/FileObservers/FileEventListener.java index f9b74b5f..039482f4 100644 --- a/app/src/main/java/foundation/e/drive/FileObservers/FileEventListener.java +++ b/app/src/main/java/foundation/e/drive/FileObservers/FileEventListener.java @@ -28,8 +28,8 @@ import foundation.e.drive.database.DbHelper; import foundation.e.drive.models.SyncRequest; import foundation.e.drive.models.SyncedFileState; import foundation.e.drive.models.SyncedFolder; -import foundation.e.drive.synchronization.ISyncRequestCollector; -import foundation.e.drive.synchronization.SyncManager; +import foundation.e.drive.synchronization.SyncRequestCollector; +import foundation.e.drive.synchronization.SyncProxy; import timber.log.Timber; /** @@ -107,7 +107,7 @@ public class FileEventListener { private void sendSyncRequestToSynchronizationService(@NonNull SyncRequest request) { Timber.d("Sending a SyncRequest for %s", request.getSyncedFileState().getName()); - final ISyncRequestCollector syncManager = SyncManager.Companion.getInstance(); + final SyncRequestCollector syncManager = SyncProxy.Companion.getInstance(); syncManager.queueSyncRequest(request, appContext.getApplicationContext()); //todo service.startSynchronization(); 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 ab4a20e8..45d8dc3c 100644 --- a/app/src/main/java/foundation/e/drive/services/ObserverService.java +++ b/app/src/main/java/foundation/e/drive/services/ObserverService.java @@ -47,8 +47,8 @@ import foundation.e.drive.models.SyncedFileState; import foundation.e.drive.models.SyncedFolder; import foundation.e.drive.operations.ListFileRemoteOperation; import foundation.e.drive.receivers.DebugCmdReceiver; -import foundation.e.drive.synchronization.ISyncRequestCollector; -import foundation.e.drive.synchronization.SyncManager; +import foundation.e.drive.synchronization.SyncRequestCollector; +import foundation.e.drive.synchronization.SyncProxy; import foundation.e.drive.utils.AppConstants; import foundation.e.drive.utils.CommonUtils; import foundation.e.drive.utils.DavClientProvider; @@ -69,7 +69,7 @@ public class ObserverService extends Service implements OnRemoteOperationListene private Account mAccount; private HashMap syncRequests; //integer is SyncedFileState id; Parcelable is the operation - private final ISyncRequestCollector syncManager = SyncManager.Companion.getInstance(); + private final SyncRequestCollector syncManager = SyncProxy.Companion.getInstance(); private Handler handler; private HandlerThread handlerThread; diff --git a/app/src/main/java/foundation/e/drive/services/SynchronizationService.java b/app/src/main/java/foundation/e/drive/services/SynchronizationService.java index 791374ac..37836acc 100644 --- a/app/src/main/java/foundation/e/drive/services/SynchronizationService.java +++ b/app/src/main/java/foundation/e/drive/services/SynchronizationService.java @@ -37,8 +37,8 @@ import foundation.e.drive.models.SyncWrapper; import foundation.e.drive.models.SyncedFileState; import foundation.e.drive.operations.DownloadFileOperation; import foundation.e.drive.operations.UploadFileOperation; -import foundation.e.drive.synchronization.ISyncManager; import foundation.e.drive.synchronization.SyncManager; +import foundation.e.drive.synchronization.SyncProxy; import foundation.e.drive.utils.AppConstants; import foundation.e.drive.utils.CommonUtils; import foundation.e.drive.utils.DavClientProvider; @@ -49,7 +49,7 @@ import timber.log.Timber; * @author Vincent Bourgmayer */ public class SynchronizationService extends Service implements OnRemoteOperationListener, FileSyncDisabler.FileSyncDisablingListener { - private final ISyncManager syncManager = SyncManager.Companion.getInstance(); + private final SyncManager syncManager = SyncProxy.Companion.getInstance(); private Account account; private final int workerAmount = 2; @@ -268,6 +268,6 @@ public class SynchronizationService extends Service implements OnRemoteOperation @Nullable @Override public IBinder onBind(@Nullable Intent intent) { - throw new UnsupportedOperationException(); + return null; } } \ No newline at end of file diff --git a/app/src/main/java/foundation/e/drive/synchronization/SyncManager.kt b/app/src/main/java/foundation/e/drive/synchronization/SyncProxy.kt similarity index 94% rename from app/src/main/java/foundation/e/drive/synchronization/SyncManager.kt rename to app/src/main/java/foundation/e/drive/synchronization/SyncProxy.kt index 87800af5..edb7755f 100644 --- a/app/src/main/java/foundation/e/drive/synchronization/SyncManager.kt +++ b/app/src/main/java/foundation/e/drive/synchronization/SyncProxy.kt @@ -19,10 +19,9 @@ import java.util.concurrent.ConcurrentLinkedQueue * This interface is used to restrict access to a limited set of method * @author Vincent Bourgmayer */ -interface ISyncRequestCollector { +interface SyncRequestCollector { fun queueSyncRequest(request: SyncRequest, context: Context) fun queueSyncRequests(requests: MutableCollection, context: Context) - fun pauseSync(): Boolean fun unpauseSync() } @@ -31,20 +30,14 @@ interface ISyncRequestCollector { * This interface is used to restrict access to a limited set of method * @author Vincent Bourgmayer */ -interface ISyncManager { +interface SyncManager { fun pollSyncRequest(): SyncRequest? - fun addStartedSync(threadId: Int, syncWrapper: SyncWrapper) - fun isQueueEmpty(): Boolean - fun isPaused(): Boolean - fun clearRequestQueue() - fun getStartedSync(threadIndex: Int): SyncWrapper? fun getStartedSync(): ConcurrentHashMap - } /** @@ -57,7 +50,7 @@ interface ISyncManager { * it holds the SyncRequest Queue and a list of running sync * @author Vincent Bourgmayer */ -class SyncManager private constructor(): ISyncRequestCollector, ISyncManager { +class SyncProxy private constructor(): SyncRequestCollector, SyncManager { private val syncRequestQueue: ConcurrentLinkedQueue = ConcurrentLinkedQueue() //could we use channel instead ? private val startedSync: ConcurrentHashMap = ConcurrentHashMap() private var pauseStartTime = 0L @@ -65,13 +58,13 @@ class SyncManager private constructor(): ISyncRequestCollector, ISyncManager { companion object { @Volatile - private var instance: SyncManager? = null + private var instance: SyncProxy? = null - fun getInstance(): SyncManager { + fun getInstance(): SyncProxy { if (instance == null) { synchronized(this) { if (instance == null) { - instance = SyncManager() + instance = SyncProxy() } } } @@ -174,11 +167,10 @@ class SyncManager private constructor(): ISyncRequestCollector, ISyncManager { } override fun isPaused(): Boolean { - return if (pauseStartTime === 0L) false + return if (pauseStartTime == 0L) false else System.currentTimeMillis() - pauseStartTime < maxPauseTimeInMs } - //todo Remove if no usage override fun getStartedSync(): ConcurrentHashMap { return startedSync } @@ -188,6 +180,6 @@ class SyncManager private constructor(): ISyncRequestCollector, ISyncManager { } override fun getStartedSync(threadIndex: Int): SyncWrapper? { - return startedSync.get(threadIndex) + return startedSync[threadIndex] } } \ No newline at end of file -- GitLab From dec8baee5262f42786946c8c7f51f751bf3062b7 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Wed, 18 Oct 2023 16:35:52 +0200 Subject: [PATCH 06/11] replace SyncProxy as a class by an Object --- .../drive/FileObservers/FileEventListener.java | 2 +- .../e/drive/services/ObserverService.java | 2 +- .../drive/services/SynchronizationService.java | 2 +- .../e/drive/synchronization/SyncProxy.kt | 18 +----------------- 4 files changed, 4 insertions(+), 20 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/FileObservers/FileEventListener.java b/app/src/main/java/foundation/e/drive/FileObservers/FileEventListener.java index 039482f4..e4e4dcbf 100644 --- a/app/src/main/java/foundation/e/drive/FileObservers/FileEventListener.java +++ b/app/src/main/java/foundation/e/drive/FileObservers/FileEventListener.java @@ -107,7 +107,7 @@ public class FileEventListener { private void sendSyncRequestToSynchronizationService(@NonNull SyncRequest request) { Timber.d("Sending a SyncRequest for %s", request.getSyncedFileState().getName()); - final SyncRequestCollector syncManager = SyncProxy.Companion.getInstance(); + final SyncRequestCollector syncManager = SyncProxy.INSTANCE; syncManager.queueSyncRequest(request, appContext.getApplicationContext()); //todo service.startSynchronization(); 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 45d8dc3c..147d9bee 100644 --- a/app/src/main/java/foundation/e/drive/services/ObserverService.java +++ b/app/src/main/java/foundation/e/drive/services/ObserverService.java @@ -69,7 +69,7 @@ public class ObserverService extends Service implements OnRemoteOperationListene private Account mAccount; private HashMap syncRequests; //integer is SyncedFileState id; Parcelable is the operation - private final SyncRequestCollector syncManager = SyncProxy.Companion.getInstance(); + private final SyncRequestCollector syncManager = SyncProxy.INSTANCE; private Handler handler; private HandlerThread handlerThread; diff --git a/app/src/main/java/foundation/e/drive/services/SynchronizationService.java b/app/src/main/java/foundation/e/drive/services/SynchronizationService.java index 37836acc..cc7b8512 100644 --- a/app/src/main/java/foundation/e/drive/services/SynchronizationService.java +++ b/app/src/main/java/foundation/e/drive/services/SynchronizationService.java @@ -49,7 +49,7 @@ import timber.log.Timber; * @author Vincent Bourgmayer */ public class SynchronizationService extends Service implements OnRemoteOperationListener, FileSyncDisabler.FileSyncDisablingListener { - private final SyncManager syncManager = SyncProxy.Companion.getInstance(); + private final SyncManager syncManager = SyncProxy.INSTANCE; private Account account; private final int workerAmount = 2; diff --git a/app/src/main/java/foundation/e/drive/synchronization/SyncProxy.kt b/app/src/main/java/foundation/e/drive/synchronization/SyncProxy.kt index edb7755f..14c4c49b 100644 --- a/app/src/main/java/foundation/e/drive/synchronization/SyncProxy.kt +++ b/app/src/main/java/foundation/e/drive/synchronization/SyncProxy.kt @@ -50,28 +50,12 @@ interface SyncManager { * it holds the SyncRequest Queue and a list of running sync * @author Vincent Bourgmayer */ -class SyncProxy private constructor(): SyncRequestCollector, SyncManager { +object SyncProxy: SyncRequestCollector, SyncManager { private val syncRequestQueue: ConcurrentLinkedQueue = ConcurrentLinkedQueue() //could we use channel instead ? private val startedSync: ConcurrentHashMap = ConcurrentHashMap() private var pauseStartTime = 0L private val maxPauseTimeInMs: Long = 300000 //5 minutes, might need to be adapted - companion object { - @Volatile - private var instance: SyncProxy? = null - - fun getInstance(): SyncProxy { - if (instance == null) { - synchronized(this) { - if (instance == null) { - instance = SyncProxy() - } - } - } - return instance!! - } - } - /** * Add a SyncRequest into waiting queue if it matches some conditions: * - an equivalent sync Request (same file & same operation type) isn't already running -- GitLab From 50d06f76ce94892286bc8dc83b81e013d0a60a83 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Thu, 19 Oct 2023 09:14:15 +0200 Subject: [PATCH 07/11] fix build by removing call of removed method --- app/src/main/java/foundation/e/drive/EdriveApplication.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/EdriveApplication.java b/app/src/main/java/foundation/e/drive/EdriveApplication.java index 586b5073..1e9b0d32 100644 --- a/app/src/main/java/foundation/e/drive/EdriveApplication.java +++ b/app/src/main/java/foundation/e/drive/EdriveApplication.java @@ -70,7 +70,6 @@ public class EdriveApplication extends Application { */ public void startRecursiveFileObserver() { if (!mFileObserver.isWatching()) { - fileEventListener.bindToSynchronizationService(); mFileObserver.startWatching(); Timber.d("Started RecursiveFileObserver on root folder"); } @@ -78,7 +77,6 @@ public class EdriveApplication extends Application { public void stopRecursiveFileObserver() { mFileObserver.stopWatching(); - fileEventListener.unbindFromSynchronizationService(); Timber.d("Stopped RecursiveFileObserver on root folder"); } -- GitLab From ef9f3dc81302c08d7bbd1c298567751fadbeb36a Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Thu, 19 Oct 2023 09:17:35 +0200 Subject: [PATCH 08/11] remove ObserverService test class because all test are ignored --- .../e/drive/services/ObserverServiceTest.java | 274 ------------------ 1 file changed, 274 deletions(-) delete mode 100644 app/src/test/java/foundation/e/drive/services/ObserverServiceTest.java diff --git a/app/src/test/java/foundation/e/drive/services/ObserverServiceTest.java b/app/src/test/java/foundation/e/drive/services/ObserverServiceTest.java deleted file mode 100644 index e9a9b707..00000000 --- a/app/src/test/java/foundation/e/drive/services/ObserverServiceTest.java +++ /dev/null @@ -1,274 +0,0 @@ -package foundation.e.drive.services; - -import android.accounts.AccountManager; -import android.app.job.JobScheduler; -import android.content.Context; -import android.net.ConnectivityManager; -import android.net.NetworkInfo; - -import com.owncloud.android.lib.common.operations.RemoteOperationResult; -import com.owncloud.android.lib.resources.files.CreateFolderRemoteOperation; - -import org.junit.Ignore; -import org.junit.Test; -import org.robolectric.Robolectric; -import org.robolectric.RuntimeEnvironment; -import org.robolectric.android.controller.ServiceController; -import org.robolectric.shadows.ShadowLog; -import org.robolectric.shadows.ShadowNetworkInfo; - -import java.io.File; -import java.util.List; - -import foundation.e.drive.TestUtils; -import foundation.e.drive.database.DbHelper; -import foundation.e.drive.models.SyncedFolder; -import foundation.e.drive.utils.AppConstants; -import foundation.e.drive.utils.CommonUtils; - - -import static foundation.e.drive.TestUtils.TEST_LOCAL_ROOT_FOLDER_PATH; -import static foundation.e.drive.TestUtils.TEST_REMOTE_ROOT_FOLDER_PATH; -import static foundation.e.drive.TestUtils.getValidAccount; -import static foundation.e.drive.utils.AppConstants.SETUP_COMPLETED; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; -import static org.robolectric.Shadows.shadowOf; - -import androidx.test.core.app.ApplicationProvider; - -/** - * The RunWith & Config annotation are done in the AbstractServiceIT - */ -public class ObserverServiceTest extends AbstractServiceIT { - private final ServiceController syncServiceController; - private final SynchronizationService syncService; - - public ObserverServiceTest(){ - syncServiceController = Robolectric.buildService(SynchronizationService.class); - syncService = syncServiceController.get(); - mServiceController = Robolectric.buildService(ObserverService.class); - mService = mServiceController.get(); - context = ApplicationProvider.getApplicationContext(); - accountManager = AccountManager.get(context); - jobScheduler = (JobScheduler) context.getSystemService(Context.JOB_SCHEDULER_SERVICE); - contentResolver = context.getContentResolver(); - sharedPreferences = context.getSharedPreferences( AppConstants.SHARED_PREFERENCE_NAME, Context.MODE_PRIVATE); - connectivityManager = (ConnectivityManager) context.getSystemService(Context.CONNECTIVITY_SERVICE); - dbHelper = new DbHelper(context); - client = TestUtils.getNcClient(context); - } - - /** - * Set the network status to an available wifi - */ - private void setWifiNetworkStatus() { - NetworkInfo netInfo = ShadowNetworkInfo.newInstance(null, - ConnectivityManager.TYPE_WIFI, 0, true, NetworkInfo.State.CONNECTED); - assertEquals("NetworkInfo type is invalid",ConnectivityManager.TYPE_WIFI,netInfo.getType()); - shadowOf(connectivityManager).setActiveNetworkInfo(netInfo); - } - - /** - * Create a network status where no connection is available - */ - private void setUnavailableWifiNetworkStatus() { - NetworkInfo netInfo = ShadowNetworkInfo.newInstance(null, - ConnectivityManager.TYPE_WIFI, 0, true, NetworkInfo.State.DISCONNECTED); - assertEquals("NetworkInfo type is invalid",ConnectivityManager.TYPE_WIFI,netInfo.getType()); - - shadowOf(connectivityManager).setActiveNetworkInfo(netInfo); - } - - /** - * Create a single 'SyncedFolder' instance for 'eDrive-test' folder - * @return SyncedFolder instance - */ - private SyncedFolder createSingleTestSyncedFolder() { - final File folder = new File(TEST_LOCAL_ROOT_FOLDER_PATH); - try{ - folder.mkdirs(); - }catch(SecurityException e){ - fail(e.getMessage()); - } - final SyncedFolder sFolder = new SyncedFolder("Images", TEST_LOCAL_ROOT_FOLDER_PATH, TEST_REMOTE_ROOT_FOLDER_PATH, true); - - return sFolder; - } - - - /** - * Send request to server to create the remote folder of the given syncedFolder - * @param folder the local folder metadata to create - */ - private void createRemoteFolder(SyncedFolder folder) { - try { - TestUtils.testConnection(client, context); - }catch(Exception e){ - System.out.println("Test connection failed :"+e.getMessage()); - } - - final CreateFolderRemoteOperation op = new CreateFolderRemoteOperation(folder.getRemoteFolder(), true); - final RemoteOperationResult result = op.execute(client); //Give SSL issue - - assertTrue("Creation of remote test folder failed",result.isSuccess()); - - final int dbFolderListSize =DbHelper.getAllSyncedFolders(context).size(); - assertEquals("Expected DB's SyncedFolder table content was 1, but got:"+dbFolderListSize, 1, dbFolderListSize); - - - initial_folder_number =1; - } - - - /** - * Run a test which correspond to a non blocking context - */ - @Ignore("Ignore until a correct assertion has been found") - @Test - public void shouldWork() { - setWifiNetworkStatus(); - prepareValidAccount(); - enableMediaAndSettingsSync(getValidAccount()); - createRemoteFolder(createSingleTestSyncedFolder()); - registerSharedPref(); - - mServiceController.create().startCommand(0, 0); - - List logs = ShadowLog.getLogs(); - ShadowLog.LogItem lastLog = logs.get(logs.size()-3); - - assertEquals("expected: 'Going to scan remote files' but found: '"+lastLog.msg, "Going to scan remote files", lastLog.msg); - } - - - - - /** - * This assert that ObserverService doesn't start scanning remote or local if there is no network - */ - @Ignore("Ignore until a way to prepare unavailable Network has been founded") - @Test - public void noNetwork_shouldStop() { - prepareValidAccount(); - enableMediaAndSettingsSync(getValidAccount()); - - registerSharedPref(); - - boolean haveNetworkConnexion = CommonUtils.haveNetworkConnection(RuntimeEnvironment.application, true); - String msg = "CommonUtils.haveNetworkConnexion should return false but return "+haveNetworkConnexion; - assertFalse(msg, haveNetworkConnexion); - - mServiceController.create().startCommand(0, 0); - - List logs = ShadowLog.getLogs(); - ShadowLog.LogItem lastLog = logs.get(logs.size()-1); - assertEquals("Last log isn't the one expected", "There is no Internet connexion.", lastLog.msg); - - } - - - /** - * This assert that ObserverService won't start if the minimum delay between two sync isn't over - */ - @Ignore("Binding to synchronizationService make test fails") - @Test - public void lastSyncWasLessThan15minAgo_shouldStop() { - last_sync_time = System.currentTimeMillis() - 899900; - setWifiNetworkStatus(); - prepareValidAccount(); - enableMediaAndSettingsSync(getValidAccount()); - registerSharedPref(); - - //Start the service - mServiceController.create().startCommand(0, 0); - - List logs = ShadowLog.getLogs(); - ShadowLog.LogItem lastLog = logs.get(logs.size()-1); - - //Note: Checking log is the only way I've found to check that the service stopped as expected - assertEquals("Last log isn't the one expected", "Delay between now and last call is too short", lastLog.msg); - - } - - /** - * This assert that ObserverService will do its job if the minimum delay between two sync is over - */ - @Ignore("Binding to synchronizationService make test fails") - @Test - public void lastSync15minAnd30secAgo_shouldStart() { - //decrease 15min and 30sec - last_sync_time = System.currentTimeMillis() - 930000; - setWifiNetworkStatus(); - prepareValidAccount(); - enableMediaAndSettingsSync(getValidAccount()); - registerSharedPref(); - syncServiceController.create().startCommand(0, 0); - - //Start the service - mServiceController.create().startCommand(0, 0); - - List logs = ShadowLog.getLogs(); - for(ShadowLog.LogItem log : logs){ - assertFalse("Log shouldn't contains 'delay between now and last call is too short' but it does", log.msg.equals("Delay between now and last call is too short")); //There isn't assertNotEquals - } - } - - /** - * This assert that ObserverService won't start if it's already running - */ - @Ignore("Not yet implemented") - @Test - public void syncAlreadyStarted_shouldStop() { - //@TODO need to find how to access the "isRunning" private field inside the ObserverService for this test - fail("Not yet implemented "); - } - - - /** - * Check that service stop if no account provided - */ - @Ignore("Binding to synchronizationService make test fails") - @Test - public void noAccount_shouldStop() { - - mServiceController.create().startCommand(0, 0); - - List logs = ShadowLog.getLogs(); - ShadowLog.LogItem lastLog = logs.get(logs.size()-1); - - assertEquals("Last expected log wasn't: 'No account registered' but "+lastLog.msg, "No account registered",lastLog.msg ); - assertTrue("jobScheduler expected to be have no pending job",jobScheduler.getAllPendingJobs().isEmpty()); - } - - /** - * This test will assert that the ObserverService won't do its job - * if Initialization hasn't been done - */ - @Ignore("Binding to synchronizationService make test fails") - @Test - public void setupNotDone_shouldStop() { - init_done = false; //This is the key settings for this test - setWifiNetworkStatus(); - prepareValidAccount(); - enableMediaAndSettingsSync(getValidAccount()); - registerSharedPref(); - - assertFalse("SharedPref doesn't contains the expected value for Initialization_has_been_done key", sharedPreferences.getBoolean(SETUP_COMPLETED, true)); - - mServiceController.create().startCommand(0, 0); - //How to assert this... ? - - List logs = ShadowLog.getLogs(); - ShadowLog.LogItem lastLog = logs.get(logs.size()-1); - - //Note: Checking log is the only way I've found to check that the service stopped as expected - assertEquals("Last log isn't the one expected", "Initialization hasn't been done", lastLog.msg); - //assertTrue(logs.contains()); //@TODO how to assert that logs doesn't contain the expecteed message ? - - //tearDown - Remove DB if it had been created - mService.deleteDatabase(DbHelper.DATABASE_NAME); - } -} \ No newline at end of file -- GitLab From f4f7b851707b84ede0cb169dc103da05fa31d14d Mon Sep 17 00:00:00 2001 From: Vincent Bourgmayer Date: Tue, 24 Oct 2023 12:34:48 +0000 Subject: [PATCH 09/11] [eDrive][Remove persistent flag][Step 3.5] Make eDrive to work without service binding --- .../foundation/e/drive/EdriveApplication.java | 10 +- .../FileObservers/FileEventListener.java | 2 +- .../receivers/BootCompletedReceiver.java | 4 + .../e/drive/services/ObserverService.java | 19 +- .../services/SynchronizationService.java | 29 +-- .../e/drive/synchronization/StateMachine.kt | 78 ++++++++ .../e/drive/synchronization/SyncProxy.kt | 133 +++++++++---- .../e/drive/work/FirstStartWorker.java | 3 - .../drive/synchronization/StateMachineTest.kt | 183 ++++++++++++++++++ 9 files changed, 392 insertions(+), 69 deletions(-) create mode 100644 app/src/main/java/foundation/e/drive/synchronization/StateMachine.kt create mode 100644 app/src/test/java/foundation/e/drive/synchronization/StateMachineTest.kt diff --git a/app/src/main/java/foundation/e/drive/EdriveApplication.java b/app/src/main/java/foundation/e/drive/EdriveApplication.java index 1e9b0d32..fe9b303f 100644 --- a/app/src/main/java/foundation/e/drive/EdriveApplication.java +++ b/app/src/main/java/foundation/e/drive/EdriveApplication.java @@ -12,14 +12,12 @@ import android.accounts.Account; import android.accounts.AccountManager; import android.app.Application; import android.content.Context; -import android.content.Intent; import android.content.SharedPreferences; import android.os.Environment; import foundation.e.drive.FileObservers.FileEventListener; import foundation.e.drive.FileObservers.RecursiveFileObserver; import foundation.e.drive.database.FailedSyncPrefsManager; -import foundation.e.drive.services.SynchronizationService; import foundation.e.drive.utils.AppConstants; import foundation.e.drive.utils.CommonUtils; import foundation.e.drive.utils.ReleaseTree; @@ -58,24 +56,20 @@ public class EdriveApplication extends Application { .apply(); } - startRecursiveFileObserver(); FailedSyncPrefsManager.getInstance(getApplicationContext()).clearPreferences(); - - final Intent SynchronizationServiceIntent = new Intent(getApplicationContext(), SynchronizationService.class); - startService(SynchronizationServiceIntent); } /** * Start Recursive FileObserver if not already watching */ - public void startRecursiveFileObserver() { + synchronized public void startRecursiveFileObserver() { if (!mFileObserver.isWatching()) { mFileObserver.startWatching(); Timber.d("Started RecursiveFileObserver on root folder"); } } - public void stopRecursiveFileObserver() { + synchronized public void stopRecursiveFileObserver() { mFileObserver.stopWatching(); Timber.d("Stopped RecursiveFileObserver on root folder"); } diff --git a/app/src/main/java/foundation/e/drive/FileObservers/FileEventListener.java b/app/src/main/java/foundation/e/drive/FileObservers/FileEventListener.java index e4e4dcbf..aa621e6f 100644 --- a/app/src/main/java/foundation/e/drive/FileObservers/FileEventListener.java +++ b/app/src/main/java/foundation/e/drive/FileObservers/FileEventListener.java @@ -109,7 +109,7 @@ public class FileEventListener { final SyncRequestCollector syncManager = SyncProxy.INSTANCE; syncManager.queueSyncRequest(request, appContext.getApplicationContext()); - //todo service.startSynchronization(); + syncManager.startSynchronization(appContext); } diff --git a/app/src/main/java/foundation/e/drive/receivers/BootCompletedReceiver.java b/app/src/main/java/foundation/e/drive/receivers/BootCompletedReceiver.java index ce806ce1..1f67e5a7 100644 --- a/app/src/main/java/foundation/e/drive/receivers/BootCompletedReceiver.java +++ b/app/src/main/java/foundation/e/drive/receivers/BootCompletedReceiver.java @@ -8,6 +8,7 @@ package foundation.e.drive.receivers; +import android.app.Application; import android.content.BroadcastReceiver; import android.content.Context; import android.content.Intent; @@ -18,6 +19,7 @@ import androidx.annotation.NonNull; import foundation.e.drive.BuildConfig; import foundation.e.drive.database.DbHelper; +import foundation.e.drive.synchronization.SyncProxy; import foundation.e.drive.utils.AppConstants; import foundation.e.drive.utils.CommonUtils; import timber.log.Timber; @@ -51,6 +53,8 @@ public class BootCompletedReceiver extends BroadcastReceiver { Timber.e(exception); } } + + SyncProxy.INSTANCE.startListeningFiles((Application) context.getApplicationContext()); } } 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 147d9bee..fe8faf1a 100644 --- a/app/src/main/java/foundation/e/drive/services/ObserverService.java +++ b/app/src/main/java/foundation/e/drive/services/ObserverService.java @@ -65,7 +65,6 @@ public class ObserverService extends Service implements OnRemoteOperationListene private final static int INTERSYNC_MINIMUM_DELAY = 900000; // min delay execution two sync in ms. private List mSyncedFolders; //List of synced folder - private boolean isWorking = false; private Account mAccount; private HashMap syncRequests; //integer is SyncedFileState id; Parcelable is the operation @@ -108,6 +107,7 @@ public class ObserverService extends Service implements OnRemoteOperationListene this.syncRequests = new HashMap<>(); if (!checkStartCondition(prefs, forcedSync)) { + syncManager.startListeningFiles(getApplication()); stopSelf(); return START_NOT_STICKY; } @@ -149,11 +149,6 @@ public class ObserverService extends Service implements OnRemoteOperationListene return false; } - if (isWorking) { - Timber.d("ObserverService is already working"); - return false; - } - // Check minimum delay since last call & not forced sync /*@todo is it really usefull to check time beetween to start as it is started by WorkManager? it matters only if we want to consider forced sync */ @@ -172,9 +167,9 @@ public class ObserverService extends Service implements OnRemoteOperationListene } - final boolean isSyncServicePaused = syncManager.pauseSync(); - Timber.d("isSyncServicePaused ? %s", isSyncServicePaused); - return isSyncServicePaused; + final boolean startAllowed = syncManager.onPeriodicScanStart(getApplication()); + Timber.d("starting periodic scan is allowed ? %s", startAllowed); + return startAllowed; } /* Common methods */ @@ -184,7 +179,6 @@ public class ObserverService extends Service implements OnRemoteOperationListene */ // protected to avoid SyntheticAccessor protected void begin(){ - this.isWorking = true; clearCachedFile(); deleteOldestCrashlogs(); startScan(true); @@ -321,20 +315,19 @@ public class ObserverService extends Service implements OnRemoteOperationListene startScan(false); - syncManager.unpauseSync(); - if (!syncRequests.isEmpty()) { Timber.d("syncRequests contains %s", syncRequests.size()); syncManager.queueSyncRequests(syncRequests.values(), getApplicationContext()); + syncManager.startSynchronization(getApplicationContext()); } else { Timber.i("There is no file to sync."); getSharedPreferences(AppConstants.SHARED_PREFERENCE_NAME, Context.MODE_PRIVATE) .edit() .putLong(AppConstants.KEY_LAST_SYNC_TIME, System.currentTimeMillis()) .apply(); + syncManager.startListeningFiles(getApplication()); } - this.isWorking = false; this.stopSelf(); } diff --git a/app/src/main/java/foundation/e/drive/services/SynchronizationService.java b/app/src/main/java/foundation/e/drive/services/SynchronizationService.java index cc7b8512..42700dc3 100644 --- a/app/src/main/java/foundation/e/drive/services/SynchronizationService.java +++ b/app/src/main/java/foundation/e/drive/services/SynchronizationService.java @@ -78,6 +78,7 @@ public class SynchronizationService extends Service implements OnRemoteOperation if (account == null) { Timber.d("No account available"); + syncManager.startListeningFiles(getApplication()); stopSelf(); return START_NOT_STICKY; } @@ -93,6 +94,8 @@ public class SynchronizationService extends Service implements OnRemoteOperation handlerThread.start(); handler = new Handler(handlerThread.getLooper()); + startSynchronization(); + return START_REDELIVER_INTENT; } @@ -124,14 +127,18 @@ public class SynchronizationService extends Service implements OnRemoteOperation return; } - if (!canStart(threadIndex) || syncManager.isPaused()) { - Timber.d("Can't start thread #%s, Sync is paused or thread is already running", threadIndex); + if (!canStart(threadIndex)) { + Timber.d("Can't start thread #%s, thread is already running", threadIndex); return; } final SyncRequest request = this.syncManager.pollSyncRequest(); //return null if empty if (request == null) { Timber.d("Thread #%s: No more sync request to start.", threadIndex); + syncManager.removeStartedRequest(threadIndex); + if (!syncManager.isAnySyncRequestRunning()) { + syncManager.startListeningFiles(getApplication()); + } return; } @@ -147,7 +154,7 @@ public class SynchronizationService extends Service implements OnRemoteOperation final FileSyncDisabler fileSyncDisabler = new FileSyncDisabler(request.getSyncedFileState()); threadPool[threadIndex] = new Thread(fileSyncDisabler.getRunnable(handler, threadIndex, getApplicationContext(), this)); threadPool[threadIndex].start(); - syncManager.addStartedSync(threadIndex,syncWrapper); + syncManager.addStartedRequest(threadIndex,syncWrapper); return; } @@ -162,7 +169,7 @@ public class SynchronizationService extends Service implements OnRemoteOperation //noinspection deprecation threadPool[threadIndex] = operation.execute(ocClient, this, handler); - syncManager.addStartedSync(threadIndex, syncWrapper); + syncManager.addStartedRequest(threadIndex, syncWrapper); } /** @@ -171,7 +178,7 @@ public class SynchronizationService extends Service implements OnRemoteOperation * @return false if nogo */ private boolean canStart(int threadIndex) { - final SyncWrapper syncWrapper = syncManager.getStartedSync(threadIndex); + final SyncWrapper syncWrapper = syncManager.getStartedRequestOnThread(threadIndex); return (syncWrapper == null || !syncWrapper.isRunning()); } @@ -202,7 +209,11 @@ public class SynchronizationService extends Service implements OnRemoteOperation break; } - for (Map.Entry keyValue : syncManager.getStartedSync().entrySet()) { + if (isNetworkDisconnected) { + syncManager.clearRequestQueue(); + } + + for (Map.Entry keyValue : syncManager.getStartedRequests().entrySet()) { final SyncWrapper wrapper = keyValue.getValue(); final RemoteOperation wrapperOperation = wrapper.getRemoteOperation(); if (wrapperOperation != null && wrapperOperation.equals(callerOperation)) { @@ -212,10 +223,6 @@ public class SynchronizationService extends Service implements OnRemoteOperation break; } } - - if (isNetworkDisconnected) { - syncManager.clearRequestQueue(); - } } private void logSyncResult(@NonNull RemoteOperationResult.ResultCode resultCode, @NonNull RemoteOperation callerOperation) { @@ -255,7 +262,7 @@ public class SynchronizationService extends Service implements OnRemoteOperation @Override public void onSyncDisabled(int threadId, boolean succeed) { - final SyncWrapper wrapper = syncManager.getStartedSync(threadId); + final SyncWrapper wrapper = syncManager.getStartedRequestOnThread(threadId); if (wrapper != null) { final SyncRequest request = wrapper.getRequest(); Timber.d("%s sync disabled ? %s", request.getSyncedFileState().getLocalPath(), succeed); diff --git a/app/src/main/java/foundation/e/drive/synchronization/StateMachine.kt b/app/src/main/java/foundation/e/drive/synchronization/StateMachine.kt new file mode 100644 index 00000000..4407d6ce --- /dev/null +++ b/app/src/main/java/foundation/e/drive/synchronization/StateMachine.kt @@ -0,0 +1,78 @@ +/* + * Copyright © MURENA SAS 2023. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the GNU Public License v3.0 + * which accompanies this distribution, and is available at + * http://www.gnu.org/licenses/gpl.html + */ +package foundation.e.drive.synchronization + +import androidx.annotation.VisibleForTesting +import timber.log.Timber + +enum class SyncState { + PERIODIC_SCAN, + SYNCHRONIZING, + LISTENING_FILES, + IDLE +} + +/** + * This class handle Synchronization state of the app + * It can be synchronizing (transfering file) + * It can be listening for file event + * It can be Scanning cloud & device for missed files + * It can be Idle (i.e: after boot, before restart) + * @author Vincent Bourgmayer + */ +object StateMachine { + + + var currentState: SyncState = SyncState.IDLE + @VisibleForTesting + set + + private var lastUpdateTimeInMs = 0L //todo use it to prevent apps to be blocked in one state + + @Synchronized + fun changeState(newState: SyncState): Boolean { + val previousState = currentState + val result = when (newState) { + SyncState.PERIODIC_SCAN -> setPeriodicScanState() + SyncState.SYNCHRONIZING -> setSynchronizing() + SyncState.LISTENING_FILES -> setListeningFilesState() + SyncState.IDLE -> false + } + if (result) { + Timber.i("Change Sync state to %s from %s", newState, previousState) + lastUpdateTimeInMs = System.currentTimeMillis() + } else { + Timber.d("Failed to change state to %s, from %s", newState, previousState) + } + return result + } + + private fun setListeningFilesState(): Boolean { + currentState = SyncState.LISTENING_FILES + return true + } + + private fun setPeriodicScanState(): Boolean { + if (currentState == SyncState.SYNCHRONIZING) { + Timber.d("Cannot change state: files sync is running") + return false + } + if (currentState == SyncState.PERIODIC_SCAN) { + Timber.d("Cannot change state: Periodic scan is already running") + return false + } + + currentState = SyncState.PERIODIC_SCAN + return true + } + + private fun setSynchronizing(): Boolean { + currentState = SyncState.SYNCHRONIZING + return true + } +} \ No newline at end of file diff --git a/app/src/main/java/foundation/e/drive/synchronization/SyncProxy.kt b/app/src/main/java/foundation/e/drive/synchronization/SyncProxy.kt index 14c4c49b..463ada82 100644 --- a/app/src/main/java/foundation/e/drive/synchronization/SyncProxy.kt +++ b/app/src/main/java/foundation/e/drive/synchronization/SyncProxy.kt @@ -7,10 +7,14 @@ */ package foundation.e.drive.synchronization +import android.app.Application import android.content.Context +import android.content.Intent +import foundation.e.drive.EdriveApplication import foundation.e.drive.database.FailedSyncPrefsManager import foundation.e.drive.models.SyncRequest import foundation.e.drive.models.SyncWrapper +import foundation.e.drive.services.SynchronizationService import timber.log.Timber import java.util.concurrent.ConcurrentHashMap import java.util.concurrent.ConcurrentLinkedQueue @@ -22,8 +26,9 @@ import java.util.concurrent.ConcurrentLinkedQueue interface SyncRequestCollector { fun queueSyncRequest(request: SyncRequest, context: Context) fun queueSyncRequests(requests: MutableCollection, context: Context) - fun pauseSync(): Boolean - fun unpauseSync() + fun startSynchronization(context: Context) + fun onPeriodicScanStart(application: Application): Boolean + fun startListeningFiles(application: Application) } /** @@ -32,18 +37,18 @@ interface SyncRequestCollector { */ interface SyncManager { fun pollSyncRequest(): SyncRequest? - fun addStartedSync(threadId: Int, syncWrapper: SyncWrapper) + fun addStartedRequest(threadId: Int, syncWrapper: SyncWrapper) fun isQueueEmpty(): Boolean - fun isPaused(): Boolean fun clearRequestQueue() - fun getStartedSync(threadIndex: Int): SyncWrapper? - fun getStartedSync(): ConcurrentHashMap + fun getStartedRequestOnThread(threadIndex: Int): SyncWrapper? + fun getStartedRequests(): ConcurrentHashMap + fun isAnySyncRequestRunning(): Boolean + fun removeStartedRequest(threadIndex: Int) + fun startListeningFiles(application: Application) } /** * This class goals is to act as a proxy between file's change detection & performing synchronization - * todo 1. should I rename to SyncProxy ? I feel that it is less clear what it is - * todo 2. Shouldn't I use an Object instead of a class ? * todo 3. one improvement could be to handle file that has just been synced in order to prevent instant detection to trigger a syncRequest in reaction (i.e in case of a download) * * This object must allow concurrent access between (periodic | instant) file's change detection and synchronization @@ -52,9 +57,7 @@ interface SyncManager { */ object SyncProxy: SyncRequestCollector, SyncManager { private val syncRequestQueue: ConcurrentLinkedQueue = ConcurrentLinkedQueue() //could we use channel instead ? - private val startedSync: ConcurrentHashMap = ConcurrentHashMap() - private var pauseStartTime = 0L - private val maxPauseTimeInMs: Long = 300000 //5 minutes, might need to be adapted + private val startedRequest: ConcurrentHashMap = ConcurrentHashMap() /** * Add a SyncRequest into waiting queue if it matches some conditions: @@ -66,7 +69,7 @@ object SyncProxy: SyncRequestCollector, SyncManager { * @param context used to check previous failure of file sync */ override fun queueSyncRequest(request: SyncRequest, context: Context) { - for (syncWrapper in startedSync.values) { + for (syncWrapper in startedRequest.values) { if (syncWrapper.isRunning && syncWrapper == request) { return } @@ -88,7 +91,7 @@ object SyncProxy: SyncRequestCollector, SyncManager { * @param context used to check previous failure of file sync */ override fun queueSyncRequests(requests: MutableCollection, context: Context) { - for (syncWrapper in startedSync.values) { + for (syncWrapper in startedRequest.values) { if (syncWrapper.isRunning) { requests.removeIf { obj: SyncRequest? -> syncWrapper == obj @@ -103,22 +106,81 @@ object SyncProxy: SyncRequestCollector, SyncManager { syncRequestQueue.addAll(requests) } - override fun pauseSync(): Boolean { - pauseStartTime = System.currentTimeMillis() - val isQueueEmpty = syncRequestQueue.isEmpty() - val isNoStartedSync = - startedSync.values.stream().noneMatch { obj: SyncWrapper -> obj.isRunning } + /** + * Try to start synchronization + * + * some rules: + * - Restart FileEventListener if previous state what Periodic Scan + * - Start synchronization + * @param context use ApplicationContext + */ + override fun startSynchronization(context: Context) { + if (context !is EdriveApplication) { + Timber.d("Invalid parameter: startSynchronization(context)") + return + } + + if (syncRequestQueue.isEmpty()) { + Timber.d("Request queue is empty") + return + } + + val previousSyncState = StateMachine.currentState + val isStateChanged = StateMachine.changeState(SyncState.SYNCHRONIZING) + + if (!isStateChanged) return + + if (previousSyncState == SyncState.PERIODIC_SCAN) { + context.startRecursiveFileObserver() + } + + if (previousSyncState != SyncState.SYNCHRONIZING) { + val intent = Intent(context, SynchronizationService::class.java) + context.startService(intent) + } + } + + /** + * Called when periodic scan try to start + * @param application need EdriveApplication instance or will return false + * @return true if the periodic scan is allowed + */ + override fun onPeriodicScanStart(application: Application): Boolean { + if (application !is EdriveApplication) { + Timber.d("Invalid parameter: : startPeriodicScan(application)") + return false + } + + val isStateChanged = StateMachine.changeState(SyncState.PERIODIC_SCAN) + if (!isStateChanged) return false + + application.stopRecursiveFileObserver() - Timber.v("is queue empty ? %s ; is no started sync ? %s", isQueueEmpty, isNoStartedSync) - val canBePaused = isQueueEmpty && isNoStartedSync - if (!canBePaused) pauseStartTime = 0L - return canBePaused + return true } - override fun unpauseSync() { - pauseStartTime = 0L + /** + * File synchronization is finished + * restart FileObserver if required + * @param application need EdriveApplication instance or will exit + */ + override fun startListeningFiles(application: Application) { + if (application !is EdriveApplication) { + Timber.d("Invalid parameter: startListeningFiles(application)") + return + } + + val previousSyncState = StateMachine.currentState + val isStateChanged = StateMachine.changeState(SyncState.LISTENING_FILES) + + if (!isStateChanged) return + + if (previousSyncState == SyncState.IDLE || previousSyncState == SyncState.PERIODIC_SCAN) { + application.startRecursiveFileObserver() + } } + /** * Progressively delay synchronization of a file in case of failure * @param request SyncRequest for the given file @@ -142,28 +204,33 @@ object SyncProxy: SyncRequestCollector, SyncManager { return syncRequestQueue.poll() } - override fun addStartedSync(threadId: Int, syncWrapper: SyncWrapper) { - startedSync[threadId] = syncWrapper + override fun addStartedRequest(threadId: Int, syncWrapper: SyncWrapper) { + startedRequest[threadId] = syncWrapper } override fun isQueueEmpty(): Boolean { return syncRequestQueue.isEmpty() } - override fun isPaused(): Boolean { - return if (pauseStartTime == 0L) false - else System.currentTimeMillis() - pauseStartTime < maxPauseTimeInMs + override fun getStartedRequests(): ConcurrentHashMap { + return startedRequest } - override fun getStartedSync(): ConcurrentHashMap { - return startedSync + override fun isAnySyncRequestRunning(): Boolean { + return startedRequest.isNotEmpty() } override fun clearRequestQueue() { syncRequestQueue.clear() } - override fun getStartedSync(threadIndex: Int): SyncWrapper? { - return startedSync[threadIndex] + override fun getStartedRequestOnThread(threadIndex: Int): SyncWrapper? { + return startedRequest[threadIndex] + } + + override fun removeStartedRequest(threadIndex: Int) { + if (startedRequest[threadIndex]?.isRunning == false) { + startedRequest.remove(threadIndex) + } } } \ No newline at end of file diff --git a/app/src/main/java/foundation/e/drive/work/FirstStartWorker.java b/app/src/main/java/foundation/e/drive/work/FirstStartWorker.java index aaf7f8e5..0ebce10f 100644 --- a/app/src/main/java/foundation/e/drive/work/FirstStartWorker.java +++ b/app/src/main/java/foundation/e/drive/work/FirstStartWorker.java @@ -55,11 +55,8 @@ public class FirstStartWorker extends Worker { enqueuePeriodicFileScanWorkRequest(appContext); - getApplicationContext().startService(new Intent(getApplicationContext(), foundation.e.drive.services.SynchronizationService.class)); getApplicationContext().startService(new Intent(getApplicationContext(), foundation.e.drive.services.ObserverService.class)); - ((EdriveApplication) getApplicationContext()).startRecursiveFileObserver(); - return Result.success(); } catch (Exception exception) { Timber.e(exception); diff --git a/app/src/test/java/foundation/e/drive/synchronization/StateMachineTest.kt b/app/src/test/java/foundation/e/drive/synchronization/StateMachineTest.kt new file mode 100644 index 00000000..c1674fca --- /dev/null +++ b/app/src/test/java/foundation/e/drive/synchronization/StateMachineTest.kt @@ -0,0 +1,183 @@ +/* + * Copyright © MURENA SAS 2023. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the GNU Public License v3.0 + * which accompanies this distribution, and is available at + * http://www.gnu.org/licenses/gpl.html + */ +package foundation.e.drive.synchronization + +import foundation.e.drive.synchronization.SyncState.IDLE +import foundation.e.drive.synchronization.SyncState.LISTENING_FILES +import foundation.e.drive.synchronization.SyncState.PERIODIC_SCAN +import foundation.e.drive.synchronization.SyncState.SYNCHRONIZING +import org.junit.Assert +import org.junit.Test + +class StateMachineTest { + + @Test + fun `Changing State from PERIODIC_SCAN to LISTENING_FILES should return true`() { + StateMachine.currentState = PERIODIC_SCAN + + val result = StateMachine.changeState(LISTENING_FILES) + Assert.assertTrue("Changing state from PERIODIC_SCAN to LISTENING_FILES returned false", result) + + val currentState = StateMachine.currentState + Assert.assertEquals("New current state should be LISTENING_FILES but is $currentState", LISTENING_FILES, currentState) + } + + @Test + fun `Changing State from PERIODIC_SCAN to PERIODIC_SCAN should return false`() { + StateMachine.currentState = PERIODIC_SCAN + + val result = StateMachine.changeState(PERIODIC_SCAN) + Assert.assertFalse("Changing state from PERIODIC_SCAN to PERIODIC_SCAN returned true", result) + + val currentState = StateMachine.currentState + Assert.assertEquals("Current state should remain PERIODIC_SCAN but is $currentState", PERIODIC_SCAN, currentState) + } + + @Test + fun `Changing State from PERIODIC_SCAN to SYNCHRONIZING should return true`() { + StateMachine.currentState = PERIODIC_SCAN + + val result = StateMachine.changeState(SYNCHRONIZING) + Assert.assertTrue("Changing state from PERIODIC_SCAN to SYNCHRONIZING returned false", result) + + val currentState = StateMachine.currentState + Assert.assertEquals("New current state should be SYNCHRONIZING but is $currentState", SYNCHRONIZING, currentState) + } + + @Test + fun `Changing State from PERIODIC_SCAN to IDLE should return false`() { + StateMachine.currentState = PERIODIC_SCAN + + val result = StateMachine.changeState(IDLE) + Assert.assertFalse("Changing state from PERIODIC_SCAN to IDLE returned false", result) + + val currentState = StateMachine.currentState + Assert.assertEquals("Current state should remain PERIODIC_SCAN but is $currentState", PERIODIC_SCAN, currentState) + } + + @Test + fun `Changing State from LISTENING_FILES to PERIODIC_SCAN should return true`() { + StateMachine.currentState = LISTENING_FILES + + val result = StateMachine.changeState(PERIODIC_SCAN) + Assert.assertTrue("Changing state from LISTENING_FILES to PERIODIC_SCAN returned false", result) + + val currentState = StateMachine.currentState + Assert.assertEquals("New current state should be PERIODIC_SCAN but is $currentState", PERIODIC_SCAN, currentState) + } + + @Test + fun `Changing State from LISTENING_FILE to SYNCHRONIZING should return true`() { + StateMachine.currentState = LISTENING_FILES + + val result = StateMachine.changeState(SYNCHRONIZING) + Assert.assertTrue("Changing state from LISTENING_FILES to SYNCHRONIZING returned false", result) + + val currentState = StateMachine.currentState + Assert.assertEquals("New current state should be SYNCHRONIZING but is $currentState", SYNCHRONIZING, currentState) + } + + @Test + fun `Changing State from LISTENING_FILE to IDLE should return false`() { + StateMachine.currentState = LISTENING_FILES + + val result = StateMachine.changeState(IDLE) + Assert.assertFalse("Changing state from LISTENING_FILES to IDLE returned true", result) + + val currentState = StateMachine.currentState + Assert.assertEquals("Current state should remain LISTENING_FILES but is $currentState", LISTENING_FILES, currentState) + } + + @Test + fun `Changing State from LISTENING_FILES to LISTENING_FILES should return true`() { + StateMachine.currentState = LISTENING_FILES + + val result = StateMachine.changeState(LISTENING_FILES) + Assert.assertTrue("Changing state from LISTENING_FILES to LISTENING_FILES returned false", result) + + val currentState = StateMachine.currentState + Assert.assertEquals("New current state should be LISTENING_FILES but is $currentState", LISTENING_FILES, currentState) + } + + @Test + fun `Changing State from SYNCHRONIZING to LISTENING_FILE should return true`() { + StateMachine.currentState = SYNCHRONIZING + + val result = StateMachine.changeState(LISTENING_FILES) + Assert.assertTrue("Changing state from SYNCHRONIZING to LISTENING_FILES returned false", result) + + val currentState = StateMachine.currentState + Assert.assertEquals("New current state should be LISTENING_FILES but is $currentState", LISTENING_FILES, currentState) + } + + @Test + fun `Changing State from SYNCHRONIZING to PERIODIC_SCAN should return false`() { + StateMachine.currentState = SYNCHRONIZING + + val result = StateMachine.changeState(PERIODIC_SCAN) + Assert.assertFalse("Changing state from SYNCHRONIZING to PERIODIC_SCAN returned true", result) + + val currentState = StateMachine.currentState + Assert.assertEquals("Current state should remain SYNCHRONIZING but is $currentState", SYNCHRONIZING, currentState) + } + + @Test + fun `Changing State from SYNCHRONIZING to SYNCHRONIZING should return true`() { + StateMachine.currentState = SYNCHRONIZING + + val result = StateMachine.changeState(SYNCHRONIZING) + Assert.assertTrue("Changing state from SYNCHRONIZING to SYNCHRONIZING returned false", result) + + val currentState = StateMachine.currentState + Assert.assertEquals("New Current state should be SYNCHRONIZING but is $currentState", SYNCHRONIZING, currentState) + } + + @Test + fun `Changing State from SYNCHRONIZING to IDLE should return false`() { + StateMachine.currentState = SYNCHRONIZING + + val result = StateMachine.changeState(IDLE) + Assert.assertFalse("Changing state from SYNCHRONIZING to IDLE returned true", result) + + val currentState = StateMachine.currentState + Assert.assertEquals("Current state should remain SYNCHRONIZING but is $currentState", SYNCHRONIZING, currentState) + } + + @Test + fun `Changing State from IDLE to LISTENING_FILES should return true`() { + StateMachine.currentState = IDLE + + val result = StateMachine.changeState(LISTENING_FILES) + Assert.assertTrue("Changing state from IDLE to LISTENING_FILES returned false", result) + + val currentState = StateMachine.currentState + Assert.assertEquals("New current state should be LISTENING_FILES but is $currentState", LISTENING_FILES, currentState) + } + + @Test + fun `Changing State from IDLE to SYNCHRONIZING should return true`() { + StateMachine.currentState = IDLE + + val result = StateMachine.changeState(SYNCHRONIZING) + Assert.assertTrue("Changing state from IDLE to SYNCHRONIZING returned false", result) + + val currentState = StateMachine.currentState + Assert.assertEquals("New current state should be SYNCHRONIZING but is $currentState", SYNCHRONIZING, currentState) + } + + @Test + fun `Changing State from IDLE to PERIODIC_SCAN should return true`() { + StateMachine.currentState = IDLE + + val result = StateMachine.changeState(PERIODIC_SCAN) + Assert.assertTrue("Changing state from IDLE to PERIODIC_SCAN returned false", result) + + val currentState = StateMachine.currentState + Assert.assertEquals("New current state should be PERIODIC_SCAN but is $currentState", PERIODIC_SCAN, currentState) + } +} \ No newline at end of file -- GitLab From e242d579d13b5974ab0735061ee8930ece6b0b1f Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Tue, 24 Oct 2023 13:01:29 +0000 Subject: [PATCH 10/11] Apply 3 suggestion(s) to 2 file(s) --- .../java/foundation/e/drive/synchronization/StateMachine.kt | 1 + .../main/java/foundation/e/drive/synchronization/SyncProxy.kt | 2 ++ 2 files changed, 3 insertions(+) diff --git a/app/src/main/java/foundation/e/drive/synchronization/StateMachine.kt b/app/src/main/java/foundation/e/drive/synchronization/StateMachine.kt index 4407d6ce..81ad3b61 100644 --- a/app/src/main/java/foundation/e/drive/synchronization/StateMachine.kt +++ b/app/src/main/java/foundation/e/drive/synchronization/StateMachine.kt @@ -62,6 +62,7 @@ object StateMachine { Timber.d("Cannot change state: files sync is running") return false } + if (currentState == SyncState.PERIODIC_SCAN) { Timber.d("Cannot change state: Periodic scan is already running") return false diff --git a/app/src/main/java/foundation/e/drive/synchronization/SyncProxy.kt b/app/src/main/java/foundation/e/drive/synchronization/SyncProxy.kt index 463ada82..13c458e1 100644 --- a/app/src/main/java/foundation/e/drive/synchronization/SyncProxy.kt +++ b/app/src/main/java/foundation/e/drive/synchronization/SyncProxy.kt @@ -98,6 +98,7 @@ object SyncProxy: SyncRequestCollector, SyncManager { } } } + requests.removeIf { request: SyncRequest? -> skipBecauseOfPreviousFail(request!!, context) } @@ -196,6 +197,7 @@ object SyncProxy: SyncRequestCollector, SyncManager { 3 -> 18000 // 5h else -> return true } + val timeStamp = System.currentTimeMillis() / 1000 return timeStamp < failedSyncPref.getLastFailureTimeForFile(fileStateId) + delay } -- GitLab From 8c07171a5b081dcfab14dfe188554ded654c449f Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Wed, 25 Oct 2023 09:31:15 +0200 Subject: [PATCH 11/11] improve StateMachine.changeState(...) --- .../e/drive/synchronization/StateMachine.kt | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/synchronization/StateMachine.kt b/app/src/main/java/foundation/e/drive/synchronization/StateMachine.kt index 81ad3b61..7be44b40 100644 --- a/app/src/main/java/foundation/e/drive/synchronization/StateMachine.kt +++ b/app/src/main/java/foundation/e/drive/synchronization/StateMachine.kt @@ -37,19 +37,21 @@ object StateMachine { @Synchronized fun changeState(newState: SyncState): Boolean { val previousState = currentState - val result = when (newState) { + val isStateChanged = when (newState) { SyncState.PERIODIC_SCAN -> setPeriodicScanState() SyncState.SYNCHRONIZING -> setSynchronizing() SyncState.LISTENING_FILES -> setListeningFilesState() SyncState.IDLE -> false } - if (result) { - Timber.i("Change Sync state to %s from %s", newState, previousState) - lastUpdateTimeInMs = System.currentTimeMillis() - } else { + + if (!isStateChanged) { Timber.d("Failed to change state to %s, from %s", newState, previousState) + return false } - return result + + Timber.i("Change Sync state to %s from %s", newState, previousState) + lastUpdateTimeInMs = System.currentTimeMillis() + return true } private fun setListeningFilesState(): Boolean { -- GitLab