From 3fd9c33587fe9b65970dd55fd37e08a0fdc5f00f Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Thu, 19 Oct 2023 21:04:31 +0200 Subject: [PATCH 01/14] implement a kind of state pattern in SyncProxy --- .../e/drive/synchronization/SyncProxy.kt | 93 ++++++++++++++----- .../e/drive/synchronization/SyncState.kt | 8 ++ 2 files changed, 79 insertions(+), 22 deletions(-) create mode 100644 app/src/main/java/foundation/e/drive/synchronization/SyncState.kt 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..e76455f5 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 startPeriodicScan(context: Context): Boolean + fun setStateToInstantScan(context: Context) } /** @@ -34,16 +39,15 @@ 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 + fun setStateToInstantScan(context: Context) } /** * 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 @@ -53,8 +57,8 @@ 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 var currentSyncState: SyncState = SyncState.WAITING //todo check if it can be some...concurrency issue /** * Add a SyncRequest into waiting queue if it matches some conditions: @@ -103,22 +107,72 @@ 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: + * - Do nothing if sync is already running + * - Restart FileEventListener if previous state what Periodic Scan + * - Start synchronization + */ + override fun startSynchronization(context: Context) { + + + if (currentSyncState == SyncState.SYNCHRONIZING) { + Timber.d("Synchronization is already running") + return + } + + if (syncRequestQueue.isEmpty()) { + Timber.d("Request queue is empty") + return + } + + if (currentSyncState == SyncState.PERIODIC_SCAN) { + val application = context as EdriveApplication + application.startRecursiveFileObserver() + } + + val intent = Intent(context, SynchronizationService::class.java) + context.startService(intent) + + currentSyncState = SyncState.SYNCHRONIZING + } + + /** + * try to start Periodic file's change detection + * Not allowed if some files are already being synced + */ + override fun startPeriodicScan(context: Context): Boolean { + if (currentSyncState == SyncState.SYNCHRONIZING) { + Timber.d("Starting Periodic scan while sync is running is not allowed") + return false + } + if (currentSyncState == SyncState.PERIODIC_SCAN) { + Timber.d("Starting Periodic scan while already running is not allowed") + return false + } + + val application = context as EdriveApplication + application.startRecursiveFileObserver() - Timber.v("is queue empty ? %s ; is no started sync ? %s", isQueueEmpty, isNoStartedSync) - val canBePaused = isQueueEmpty && isNoStartedSync - if (!canBePaused) pauseStartTime = 0L - return canBePaused + currentSyncState = SyncState.PERIODIC_SCAN + return true } - override fun unpauseSync() { - pauseStartTime = 0L + /** + * File synchronization is finished + * set sync state to Instant file change detection + */ + override fun setStateToInstantScan(context: Context) { + if (currentSyncState == SyncState.WAITING) { + val application = context as EdriveApplication + application.startRecursiveFileObserver() + } + currentSyncState = SyncState.INSTANT_SCAN } + /** * Progressively delay synchronization of a file in case of failure * @param request SyncRequest for the given file @@ -150,11 +204,6 @@ object SyncProxy: SyncRequestCollector, SyncManager { return syncRequestQueue.isEmpty() } - override fun isPaused(): Boolean { - return if (pauseStartTime == 0L) false - else System.currentTimeMillis() - pauseStartTime < maxPauseTimeInMs - } - override fun getStartedSync(): ConcurrentHashMap { return startedSync } diff --git a/app/src/main/java/foundation/e/drive/synchronization/SyncState.kt b/app/src/main/java/foundation/e/drive/synchronization/SyncState.kt new file mode 100644 index 00000000..981147a5 --- /dev/null +++ b/app/src/main/java/foundation/e/drive/synchronization/SyncState.kt @@ -0,0 +1,8 @@ +package foundation.e.drive.synchronization + +enum class SyncState { + PERIODIC_SCAN, + SYNCHRONIZING, + INSTANT_SCAN, + WAITING +} \ No newline at end of file -- GitLab From e9298009bf061aab54dbd15984ea6d57df471137 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Thu, 19 Oct 2023 21:04:56 +0200 Subject: [PATCH 02/14] use state pattern in other component --- .../foundation/e/drive/EdriveApplication.java | 2 +- .../drive/FileObservers/FileEventListener.java | 1 + .../e/drive/services/ObserverService.java | 18 +++++------------- .../drive/services/SynchronizationService.java | 16 ++++++++++------ 4 files changed, 17 insertions(+), 20 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/EdriveApplication.java b/app/src/main/java/foundation/e/drive/EdriveApplication.java index 1e9b0d32..2401b3a0 100644 --- a/app/src/main/java/foundation/e/drive/EdriveApplication.java +++ b/app/src/main/java/foundation/e/drive/EdriveApplication.java @@ -58,7 +58,7 @@ public class EdriveApplication extends Application { .apply(); } - startRecursiveFileObserver(); + //startRecursiveFileObserver(); FailedSyncPrefsManager.getInstance(getApplicationContext()).clearPreferences(); final Intent SynchronizationServiceIntent = new Intent(getApplicationContext(), SynchronizationService.class); 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..77a41958 100644 --- a/app/src/main/java/foundation/e/drive/FileObservers/FileEventListener.java +++ b/app/src/main/java/foundation/e/drive/FileObservers/FileEventListener.java @@ -109,6 +109,7 @@ public class FileEventListener { final SyncRequestCollector syncManager = SyncProxy.INSTANCE; syncManager.queueSyncRequest(request, appContext.getApplicationContext()); + syncManager.startSynchronization(appContext); //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 147d9bee..e4f6ffaa 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.setStateToInstantScan(); 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.startPeriodicScan(getApplicationContext()); + 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,11 +315,10 @@ 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) @@ -334,7 +327,6 @@ public class ObserverService extends Service implements OnRemoteOperationListene .apply(); } - 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..a1ab7838 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.setStateToInstantScan(getApplicationContext()); 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,15 @@ 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.setStateToInstantScan(getApplicationContext()); return; } @@ -202,6 +206,10 @@ public class SynchronizationService extends Service implements OnRemoteOperation break; } + if (isNetworkDisconnected) { + syncManager.clearRequestQueue(); + } + for (Map.Entry keyValue : syncManager.getStartedSync().entrySet()) { final SyncWrapper wrapper = keyValue.getValue(); final RemoteOperation wrapperOperation = wrapper.getRemoteOperation(); @@ -212,10 +220,6 @@ public class SynchronizationService extends Service implements OnRemoteOperation break; } } - - if (isNetworkDisconnected) { - syncManager.clearRequestQueue(); - } } private void logSyncResult(@NonNull RemoteOperationResult.ResultCode resultCode, @NonNull RemoteOperation callerOperation) { -- GitLab From bb18339be0113ae5c1479f79a2f0ac30a7047f45 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Thu, 19 Oct 2023 21:32:20 +0200 Subject: [PATCH 03/14] add debug instruction and trigger instant sync from bootCompleteReceiver --- app/src/main/java/foundation/e/drive/EdriveApplication.java | 6 ------ .../foundation/e/drive/receivers/BootCompletedReceiver.java | 3 +++ .../java/foundation/e/drive/services/ObserverService.java | 2 +- .../java/foundation/e/drive/synchronization/SyncProxy.kt | 5 +++-- 4 files changed, 7 insertions(+), 9 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/EdriveApplication.java b/app/src/main/java/foundation/e/drive/EdriveApplication.java index 2401b3a0..9b28a6ce 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,11 +56,7 @@ public class EdriveApplication extends Application { .apply(); } - //startRecursiveFileObserver(); FailedSyncPrefsManager.getInstance(getApplicationContext()).clearPreferences(); - - final Intent SynchronizationServiceIntent = new Intent(getApplicationContext(), SynchronizationService.class); - startService(SynchronizationServiceIntent); } /** 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..f6cd845b 100644 --- a/app/src/main/java/foundation/e/drive/receivers/BootCompletedReceiver.java +++ b/app/src/main/java/foundation/e/drive/receivers/BootCompletedReceiver.java @@ -18,6 +18,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 +52,8 @@ public class BootCompletedReceiver extends BroadcastReceiver { Timber.e(exception); } } + + SyncProxy.INSTANCE.setStateToInstantScan(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 e4f6ffaa..f0478267 100644 --- a/app/src/main/java/foundation/e/drive/services/ObserverService.java +++ b/app/src/main/java/foundation/e/drive/services/ObserverService.java @@ -107,7 +107,7 @@ public class ObserverService extends Service implements OnRemoteOperationListene this.syncRequests = new HashMap<>(); if (!checkStartCondition(prefs, forcedSync)) { - syncManager.setStateToInstantScan(); + syncManager.setStateToInstantScan(getApplicationContext()); stopSelf(); return START_NOT_STICKY; } 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 e76455f5..52c7cb62 100644 --- a/app/src/main/java/foundation/e/drive/synchronization/SyncProxy.kt +++ b/app/src/main/java/foundation/e/drive/synchronization/SyncProxy.kt @@ -116,8 +116,7 @@ object SyncProxy: SyncRequestCollector, SyncManager { * - Start synchronization */ override fun startSynchronization(context: Context) { - - + Timber.v("startSynchronization(context) from current sync State: %s", currentSyncState) if (currentSyncState == SyncState.SYNCHRONIZING) { Timber.d("Synchronization is already running") return @@ -144,6 +143,7 @@ object SyncProxy: SyncRequestCollector, SyncManager { * Not allowed if some files are already being synced */ override fun startPeriodicScan(context: Context): Boolean { + Timber.v("startPeriodicScan(context) from current syncState: %s", currentSyncState) if (currentSyncState == SyncState.SYNCHRONIZING) { Timber.d("Starting Periodic scan while sync is running is not allowed") return false @@ -165,6 +165,7 @@ object SyncProxy: SyncRequestCollector, SyncManager { * set sync state to Instant file change detection */ override fun setStateToInstantScan(context: Context) { + Timber.v("setStateToInstantScan(context) from currentSyncState %s", currentSyncState) if (currentSyncState == SyncState.WAITING) { val application = context as EdriveApplication application.startRecursiveFileObserver() -- GitLab From d46aba3fc5b4ffdd95085f4323d648d0fa2f466f Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Thu, 19 Oct 2023 21:48:41 +0200 Subject: [PATCH 04/14] add missing method for SynchronizationService and add missing call in ObserverService --- .../foundation/e/drive/services/ObserverService.java | 1 + .../e/drive/services/SynchronizationService.java | 5 ++++- .../foundation/e/drive/synchronization/SyncProxy.kt | 12 ++++++++++++ 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/foundation/e/drive/services/ObserverService.java b/app/src/main/java/foundation/e/drive/services/ObserverService.java index f0478267..f31dd072 100644 --- a/app/src/main/java/foundation/e/drive/services/ObserverService.java +++ b/app/src/main/java/foundation/e/drive/services/ObserverService.java @@ -325,6 +325,7 @@ public class ObserverService extends Service implements OnRemoteOperationListene .edit() .putLong(AppConstants.KEY_LAST_SYNC_TIME, System.currentTimeMillis()) .apply(); + syncManager.setStateToInstantScan(getApplicationContext()); } 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 a1ab7838..04f52b8d 100644 --- a/app/src/main/java/foundation/e/drive/services/SynchronizationService.java +++ b/app/src/main/java/foundation/e/drive/services/SynchronizationService.java @@ -135,7 +135,10 @@ public class SynchronizationService extends Service implements OnRemoteOperation 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.setStateToInstantScan(getApplicationContext()); + syncManager.removeStartedSync(threadIndex); + if (!syncManager.isAnyFileSyncRunning()) { + syncManager.setStateToInstantScan(getApplicationContext()); + } return; } 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 52c7cb62..be37123d 100644 --- a/app/src/main/java/foundation/e/drive/synchronization/SyncProxy.kt +++ b/app/src/main/java/foundation/e/drive/synchronization/SyncProxy.kt @@ -42,6 +42,8 @@ interface SyncManager { fun clearRequestQueue() fun getStartedSync(threadIndex: Int): SyncWrapper? fun getStartedSync(): ConcurrentHashMap + fun isAnyFileSyncRunning(): Boolean + fun removeStartedSync(threadIndex: Int) fun setStateToInstantScan(context: Context) } @@ -209,6 +211,10 @@ object SyncProxy: SyncRequestCollector, SyncManager { return startedSync } + override fun isAnyFileSyncRunning(): Boolean { + return startedSync.isNotEmpty() + } + override fun clearRequestQueue() { syncRequestQueue.clear() } @@ -216,4 +222,10 @@ object SyncProxy: SyncRequestCollector, SyncManager { override fun getStartedSync(threadIndex: Int): SyncWrapper? { return startedSync[threadIndex] } + + override fun removeStartedSync(threadIndex: Int) { + if (startedSync[threadIndex]?.isRunning == false) { + startedSync.remove(threadIndex) + } + } } \ No newline at end of file -- GitLab From d5ca0f2ac9aa7b149600f4c6e463c661093e0713 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Thu, 19 Oct 2023 22:27:20 +0200 Subject: [PATCH 05/14] fix error in code --- .../main/java/foundation/e/drive/synchronization/SyncProxy.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 be37123d..2efa037f 100644 --- a/app/src/main/java/foundation/e/drive/synchronization/SyncProxy.kt +++ b/app/src/main/java/foundation/e/drive/synchronization/SyncProxy.kt @@ -156,7 +156,7 @@ object SyncProxy: SyncRequestCollector, SyncManager { } val application = context as EdriveApplication - application.startRecursiveFileObserver() + application.stopRecursiveFileObserver() currentSyncState = SyncState.PERIODIC_SCAN return true -- GitLab From bfb064170e2c08ada2f72e5922d30335a91bf709 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Mon, 23 Oct 2023 12:45:21 +0200 Subject: [PATCH 06/14] rename one syncState & add test about context --- .../receivers/BootCompletedReceiver.java | 3 +- .../e/drive/services/ObserverService.java | 6 +- .../services/SynchronizationService.java | 4 +- .../e/drive/synchronization/SyncProxy.kt | 82 +++++++++++-------- .../e/drive/synchronization/SyncState.kt | 2 +- 5 files changed, 58 insertions(+), 39 deletions(-) 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 f6cd845b..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; @@ -53,7 +54,7 @@ public class BootCompletedReceiver extends BroadcastReceiver { } } - SyncProxy.INSTANCE.setStateToInstantScan(context.getApplicationContext()); + 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 f31dd072..464f4ae1 100644 --- a/app/src/main/java/foundation/e/drive/services/ObserverService.java +++ b/app/src/main/java/foundation/e/drive/services/ObserverService.java @@ -107,7 +107,7 @@ public class ObserverService extends Service implements OnRemoteOperationListene this.syncRequests = new HashMap<>(); if (!checkStartCondition(prefs, forcedSync)) { - syncManager.setStateToInstantScan(getApplicationContext()); + syncManager.startListeningFiles(getApplication()); stopSelf(); return START_NOT_STICKY; } @@ -167,7 +167,7 @@ public class ObserverService extends Service implements OnRemoteOperationListene } - final boolean startAllowed = syncManager.startPeriodicScan(getApplicationContext()); + final boolean startAllowed = syncManager.startPeriodicScan(getApplication()); Timber.d("starting periodic scan is allowed ? %s", startAllowed); return startAllowed; } @@ -325,7 +325,7 @@ public class ObserverService extends Service implements OnRemoteOperationListene .edit() .putLong(AppConstants.KEY_LAST_SYNC_TIME, System.currentTimeMillis()) .apply(); - syncManager.setStateToInstantScan(getApplicationContext()); + syncManager.startListeningFiles(getApplication()); } 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 04f52b8d..7c55dd4d 100644 --- a/app/src/main/java/foundation/e/drive/services/SynchronizationService.java +++ b/app/src/main/java/foundation/e/drive/services/SynchronizationService.java @@ -78,7 +78,7 @@ public class SynchronizationService extends Service implements OnRemoteOperation if (account == null) { Timber.d("No account available"); - syncManager.setStateToInstantScan(getApplicationContext()); + syncManager.startListeningFiles(getApplication()); stopSelf(); return START_NOT_STICKY; } @@ -137,7 +137,7 @@ public class SynchronizationService extends Service implements OnRemoteOperation Timber.d("Thread #%s: No more sync request to start.", threadIndex); syncManager.removeStartedSync(threadIndex); if (!syncManager.isAnyFileSyncRunning()) { - syncManager.setStateToInstantScan(getApplicationContext()); + syncManager.startListeningFiles(getApplication()); } return; } 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 2efa037f..b9096d1e 100644 --- a/app/src/main/java/foundation/e/drive/synchronization/SyncProxy.kt +++ b/app/src/main/java/foundation/e/drive/synchronization/SyncProxy.kt @@ -27,8 +27,8 @@ interface SyncRequestCollector { fun queueSyncRequest(request: SyncRequest, context: Context) fun queueSyncRequests(requests: MutableCollection, context: Context) fun startSynchronization(context: Context) - fun startPeriodicScan(context: Context): Boolean - fun setStateToInstantScan(context: Context) + fun startPeriodicScan(application: Application): Boolean + fun startListeningFiles(application: Application) } /** @@ -44,12 +44,11 @@ interface SyncManager { fun getStartedSync(): ConcurrentHashMap fun isAnyFileSyncRunning(): Boolean fun removeStartedSync(threadIndex: Int) - fun setStateToInstantScan(context: Context) + 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 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 @@ -116,49 +115,62 @@ object SyncProxy: SyncRequestCollector, SyncManager { * - Do nothing if sync is already running * - Restart FileEventListener if previous state what Periodic Scan * - Start synchronization + * @param context use ApplicationContext */ override fun startSynchronization(context: Context) { - Timber.v("startSynchronization(context) from current sync State: %s", currentSyncState) - if (currentSyncState == SyncState.SYNCHRONIZING) { - Timber.d("Synchronization is already running") + if (context !is EdriveApplication) { + Timber.d("Invalid parameter: startSynchronization(context)") return } - if (syncRequestQueue.isEmpty()) { - Timber.d("Request queue is empty") - return - } + synchronized(currentSyncState) { + Timber.v("startSynchronization(context) from current sync State: %s", currentSyncState) + if (currentSyncState == SyncState.SYNCHRONIZING) { + Timber.d("Synchronization is already running") + return + } - if (currentSyncState == SyncState.PERIODIC_SCAN) { - val application = context as EdriveApplication - application.startRecursiveFileObserver() + if (syncRequestQueue.isEmpty()) { + Timber.d("Request queue is empty") + return + } + + if (currentSyncState == SyncState.PERIODIC_SCAN) { + val application: EdriveApplication = context + application.startRecursiveFileObserver() + } + currentSyncState = SyncState.SYNCHRONIZING } val intent = Intent(context, SynchronizationService::class.java) context.startService(intent) - - currentSyncState = SyncState.SYNCHRONIZING } /** * try to start Periodic file's change detection * Not allowed if some files are already being synced */ - override fun startPeriodicScan(context: Context): Boolean { - Timber.v("startPeriodicScan(context) from current syncState: %s", currentSyncState) - if (currentSyncState == SyncState.SYNCHRONIZING) { - Timber.d("Starting Periodic scan while sync is running is not allowed") + override fun startPeriodicScan(application: Application): Boolean { + if (application !is EdriveApplication) { + Timber.d("Invalid parameter: : startPeriodicScan(application)") return false } - if (currentSyncState == SyncState.PERIODIC_SCAN) { - Timber.d("Starting Periodic scan while already running is not allowed") - return false + + synchronized(currentSyncState) { + Timber.v("startPeriodicScan(context) from current syncState: %s", currentSyncState) + if (currentSyncState == SyncState.SYNCHRONIZING) { + Timber.d("Starting Periodic scan while sync is running is not allowed") + return false + } + if (currentSyncState == SyncState.PERIODIC_SCAN) { + Timber.d("Starting Periodic scan while already running is not allowed") + return false + } + + currentSyncState = SyncState.PERIODIC_SCAN } - val application = context as EdriveApplication application.stopRecursiveFileObserver() - - currentSyncState = SyncState.PERIODIC_SCAN return true } @@ -166,13 +178,19 @@ object SyncProxy: SyncRequestCollector, SyncManager { * File synchronization is finished * set sync state to Instant file change detection */ - override fun setStateToInstantScan(context: Context) { - Timber.v("setStateToInstantScan(context) from currentSyncState %s", currentSyncState) - if (currentSyncState == SyncState.WAITING) { - val application = context as EdriveApplication - application.startRecursiveFileObserver() + override fun startListeningFiles(application: Application) { + if (application !is EdriveApplication) { + Timber.d("Invalid parameter: startListeningFiles(application)") + return + } + synchronized(currentSyncState) { + Timber.v("setStateToInstantScan(context) from currentSyncState %s", currentSyncState) + if (currentSyncState == SyncState.WAITING) { + val application: EdriveApplication = application + application.startRecursiveFileObserver() + } + currentSyncState = SyncState.LISTENING_FILES } - currentSyncState = SyncState.INSTANT_SCAN } diff --git a/app/src/main/java/foundation/e/drive/synchronization/SyncState.kt b/app/src/main/java/foundation/e/drive/synchronization/SyncState.kt index 981147a5..ff6a1d0b 100644 --- a/app/src/main/java/foundation/e/drive/synchronization/SyncState.kt +++ b/app/src/main/java/foundation/e/drive/synchronization/SyncState.kt @@ -3,6 +3,6 @@ package foundation.e.drive.synchronization enum class SyncState { PERIODIC_SCAN, SYNCHRONIZING, - INSTANT_SCAN, + LISTENING_FILES, WAITING } \ No newline at end of file -- GitLab From ca273d006f0b41f2b6f1dd9ab7eada32d4894d3d Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Mon, 23 Oct 2023 13:39:15 +0200 Subject: [PATCH 07/14] extract State handling code from SyncProxy to StateManager --- .../e/drive/services/ObserverService.java | 2 +- .../e/drive/synchronization/StateManager.kt | 80 +++++++++++++++++++ .../e/drive/synchronization/SyncProxy.kt | 70 +++++++--------- .../e/drive/synchronization/SyncState.kt | 8 -- 4 files changed, 109 insertions(+), 51 deletions(-) create mode 100644 app/src/main/java/foundation/e/drive/synchronization/StateManager.kt delete mode 100644 app/src/main/java/foundation/e/drive/synchronization/SyncState.kt 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 464f4ae1..fe8faf1a 100644 --- a/app/src/main/java/foundation/e/drive/services/ObserverService.java +++ b/app/src/main/java/foundation/e/drive/services/ObserverService.java @@ -167,7 +167,7 @@ public class ObserverService extends Service implements OnRemoteOperationListene } - final boolean startAllowed = syncManager.startPeriodicScan(getApplication()); + final boolean startAllowed = syncManager.onPeriodicScanStart(getApplication()); Timber.d("starting periodic scan is allowed ? %s", startAllowed); return startAllowed; } diff --git a/app/src/main/java/foundation/e/drive/synchronization/StateManager.kt b/app/src/main/java/foundation/e/drive/synchronization/StateManager.kt new file mode 100644 index 00000000..dca1a0dd --- /dev/null +++ b/app/src/main/java/foundation/e/drive/synchronization/StateManager.kt @@ -0,0 +1,80 @@ +/* + * 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 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 StateManager { + + private var currentState: SyncState = SyncState.IDLE + private var lastUpdateTimeInMs = 0L //todo use it to prevent apps to be blocked in one state + + fun getCurrentState(): SyncState { + return currentState + } + + @Synchronized fun changeState(newState: SyncState): Boolean { + 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, currentState) + lastUpdateTimeInMs = System.currentTimeMillis() + } else { + Timber.d("Failed to change state to %s, from %s", newState, currentState) + } + 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 { + if (currentState == SyncState.SYNCHRONIZING) { + Timber.d("Cannot change state : Synchronization is already running") + return false + } + + 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 b9096d1e..a9a7c8ba 100644 --- a/app/src/main/java/foundation/e/drive/synchronization/SyncProxy.kt +++ b/app/src/main/java/foundation/e/drive/synchronization/SyncProxy.kt @@ -27,7 +27,7 @@ interface SyncRequestCollector { fun queueSyncRequest(request: SyncRequest, context: Context) fun queueSyncRequests(requests: MutableCollection, context: Context) fun startSynchronization(context: Context) - fun startPeriodicScan(application: Application): Boolean + fun onPeriodicScanStart(application: Application): Boolean fun startListeningFiles(application: Application) } @@ -59,8 +59,6 @@ object SyncProxy: SyncRequestCollector, SyncManager { private val syncRequestQueue: ConcurrentLinkedQueue = ConcurrentLinkedQueue() //could we use channel instead ? private val startedSync: ConcurrentHashMap = ConcurrentHashMap() - private var currentSyncState: SyncState = SyncState.WAITING //todo check if it can be some...concurrency issue - /** * Add a SyncRequest into waiting queue if it matches some conditions: * - an equivalent sync Request (same file & same operation type) isn't already running @@ -123,23 +121,18 @@ object SyncProxy: SyncRequestCollector, SyncManager { return } - synchronized(currentSyncState) { - Timber.v("startSynchronization(context) from current sync State: %s", currentSyncState) - if (currentSyncState == SyncState.SYNCHRONIZING) { - Timber.d("Synchronization is already running") - return - } + if (syncRequestQueue.isEmpty()) { + Timber.d("Request queue is empty") + return + } - if (syncRequestQueue.isEmpty()) { - Timber.d("Request queue is empty") - return - } + val previousSyncState = StateManager.getCurrentState() + val isStateChanged = StateManager.changeState(SyncState.SYNCHRONIZING) - if (currentSyncState == SyncState.PERIODIC_SCAN) { - val application: EdriveApplication = context - application.startRecursiveFileObserver() - } - currentSyncState = SyncState.SYNCHRONIZING + if (!isStateChanged) return + + if (previousSyncState == SyncState.PERIODIC_SCAN) { + context.startRecursiveFileObserver() } val intent = Intent(context, SynchronizationService::class.java) @@ -147,49 +140,42 @@ object SyncProxy: SyncRequestCollector, SyncManager { } /** - * try to start Periodic file's change detection - * Not allowed if some files are already being synced + * 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 startPeriodicScan(application: Application): Boolean { + override fun onPeriodicScanStart(application: Application): Boolean { if (application !is EdriveApplication) { Timber.d("Invalid parameter: : startPeriodicScan(application)") return false } - synchronized(currentSyncState) { - Timber.v("startPeriodicScan(context) from current syncState: %s", currentSyncState) - if (currentSyncState == SyncState.SYNCHRONIZING) { - Timber.d("Starting Periodic scan while sync is running is not allowed") - return false - } - if (currentSyncState == SyncState.PERIODIC_SCAN) { - Timber.d("Starting Periodic scan while already running is not allowed") - return false - } - - currentSyncState = SyncState.PERIODIC_SCAN - } + val isStateChanged = StateManager.changeState(SyncState.SYNCHRONIZING) + if (!isStateChanged) return false application.stopRecursiveFileObserver() + return true } /** * File synchronization is finished - * set sync state to Instant file change detection + * 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 } - synchronized(currentSyncState) { - Timber.v("setStateToInstantScan(context) from currentSyncState %s", currentSyncState) - if (currentSyncState == SyncState.WAITING) { - val application: EdriveApplication = application - application.startRecursiveFileObserver() - } - currentSyncState = SyncState.LISTENING_FILES + + val previousSyncState = StateManager.getCurrentState() + val isStateChanged = StateManager.changeState(SyncState.LISTENING_FILES) + + if (!isStateChanged) return + + if (previousSyncState == SyncState.IDLE) { + application.startRecursiveFileObserver() } } diff --git a/app/src/main/java/foundation/e/drive/synchronization/SyncState.kt b/app/src/main/java/foundation/e/drive/synchronization/SyncState.kt deleted file mode 100644 index ff6a1d0b..00000000 --- a/app/src/main/java/foundation/e/drive/synchronization/SyncState.kt +++ /dev/null @@ -1,8 +0,0 @@ -package foundation.e.drive.synchronization - -enum class SyncState { - PERIODIC_SCAN, - SYNCHRONIZING, - LISTENING_FILES, - WAITING -} \ No newline at end of file -- GitLab From a3bc78f0a63b52244da72aa959ac00b37d257ff9 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Mon, 23 Oct 2023 14:17:59 +0200 Subject: [PATCH 08/14] apply Jonathan's suggestion --- .../services/SynchronizationService.java | 14 +++---- .../e/drive/synchronization/SyncProxy.kt | 38 +++++++++---------- 2 files changed, 26 insertions(+), 26 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/services/SynchronizationService.java b/app/src/main/java/foundation/e/drive/services/SynchronizationService.java index 7c55dd4d..42700dc3 100644 --- a/app/src/main/java/foundation/e/drive/services/SynchronizationService.java +++ b/app/src/main/java/foundation/e/drive/services/SynchronizationService.java @@ -135,8 +135,8 @@ public class SynchronizationService extends Service implements OnRemoteOperation 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.removeStartedSync(threadIndex); - if (!syncManager.isAnyFileSyncRunning()) { + syncManager.removeStartedRequest(threadIndex); + if (!syncManager.isAnySyncRequestRunning()) { syncManager.startListeningFiles(getApplication()); } return; @@ -154,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; } @@ -169,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); } /** @@ -178,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()); } @@ -213,7 +213,7 @@ public class SynchronizationService extends Service implements OnRemoteOperation syncManager.clearRequestQueue(); } - for (Map.Entry keyValue : syncManager.getStartedSync().entrySet()) { + for (Map.Entry keyValue : syncManager.getStartedRequests().entrySet()) { final SyncWrapper wrapper = keyValue.getValue(); final RemoteOperation wrapperOperation = wrapper.getRemoteOperation(); if (wrapperOperation != null && wrapperOperation.equals(callerOperation)) { @@ -262,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/SyncProxy.kt b/app/src/main/java/foundation/e/drive/synchronization/SyncProxy.kt index a9a7c8ba..23d4f33c 100644 --- a/app/src/main/java/foundation/e/drive/synchronization/SyncProxy.kt +++ b/app/src/main/java/foundation/e/drive/synchronization/SyncProxy.kt @@ -37,13 +37,13 @@ interface SyncRequestCollector { */ interface SyncManager { fun pollSyncRequest(): SyncRequest? - fun addStartedSync(threadId: Int, syncWrapper: SyncWrapper) + fun addStartedRequest(threadId: Int, syncWrapper: SyncWrapper) fun isQueueEmpty(): Boolean fun clearRequestQueue() - fun getStartedSync(threadIndex: Int): SyncWrapper? - fun getStartedSync(): ConcurrentHashMap - fun isAnyFileSyncRunning(): Boolean - fun removeStartedSync(threadIndex: Int) + fun getStartedRequestOnThread(threadIndex: Int): SyncWrapper? + fun getStartedRequests(): ConcurrentHashMap + fun isAnySyncRequestRunning(): Boolean + fun removeStartedRequest(threadIndex: Int) fun startListeningFiles(application: Application) } @@ -57,7 +57,7 @@ interface SyncManager { */ object SyncProxy: SyncRequestCollector, SyncManager { private val syncRequestQueue: ConcurrentLinkedQueue = ConcurrentLinkedQueue() //could we use channel instead ? - private val startedSync: ConcurrentHashMap = ConcurrentHashMap() + private val startedRequest: ConcurrentHashMap = ConcurrentHashMap() /** * Add a SyncRequest into waiting queue if it matches some conditions: @@ -69,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 } @@ -91,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 @@ -203,33 +203,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 getStartedSync(): ConcurrentHashMap { - return startedSync + override fun getStartedRequests(): ConcurrentHashMap { + return startedRequest } - override fun isAnyFileSyncRunning(): Boolean { - return startedSync.isNotEmpty() + 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 removeStartedSync(threadIndex: Int) { - if (startedSync[threadIndex]?.isRunning == false) { - startedSync.remove(threadIndex) + override fun removeStartedRequest(threadIndex: Int) { + if (startedRequest[threadIndex]?.isRunning == false) { + startedRequest.remove(threadIndex) } } } \ No newline at end of file -- GitLab From 3f262f017739a463c458fc75d25487571ea5d6d3 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Mon, 23 Oct 2023 15:19:26 +0200 Subject: [PATCH 09/14] fix bug --- .../java/foundation/e/drive/EdriveApplication.java | 4 ++-- .../e/drive/synchronization/StateManager.kt | 10 +++------- .../foundation/e/drive/synchronization/SyncProxy.kt | 11 ++++++----- .../foundation/e/drive/work/FirstStartWorker.java | 3 --- 4 files changed, 11 insertions(+), 17 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/EdriveApplication.java b/app/src/main/java/foundation/e/drive/EdriveApplication.java index 9b28a6ce..fe9b303f 100644 --- a/app/src/main/java/foundation/e/drive/EdriveApplication.java +++ b/app/src/main/java/foundation/e/drive/EdriveApplication.java @@ -62,14 +62,14 @@ public class EdriveApplication extends Application { /** * 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/synchronization/StateManager.kt b/app/src/main/java/foundation/e/drive/synchronization/StateManager.kt index dca1a0dd..b034ea5c 100644 --- a/app/src/main/java/foundation/e/drive/synchronization/StateManager.kt +++ b/app/src/main/java/foundation/e/drive/synchronization/StateManager.kt @@ -34,6 +34,7 @@ object StateManager { } @Synchronized fun changeState(newState: SyncState): Boolean { + val previousState = currentState val result = when (newState) { SyncState.PERIODIC_SCAN -> setPeriodicScanState() SyncState.SYNCHRONIZING -> setSynchronizing() @@ -41,10 +42,10 @@ object StateManager { SyncState.IDLE -> false } if (result) { - Timber.i("Change Sync state to %s from %s", newState, currentState) + 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, currentState) + Timber.d("Failed to change state to %s, from %s", newState, previousState) } return result } @@ -69,11 +70,6 @@ object StateManager { } private fun setSynchronizing(): Boolean { - if (currentState == SyncState.SYNCHRONIZING) { - Timber.d("Cannot change state : Synchronization is already running") - return false - } - currentState = SyncState.SYNCHRONIZING return true } 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 23d4f33c..ca238187 100644 --- a/app/src/main/java/foundation/e/drive/synchronization/SyncProxy.kt +++ b/app/src/main/java/foundation/e/drive/synchronization/SyncProxy.kt @@ -110,7 +110,6 @@ object SyncProxy: SyncRequestCollector, SyncManager { * Try to start synchronization * * some rules: - * - Do nothing if sync is already running * - Restart FileEventListener if previous state what Periodic Scan * - Start synchronization * @param context use ApplicationContext @@ -135,8 +134,10 @@ object SyncProxy: SyncRequestCollector, SyncManager { context.startRecursiveFileObserver() } - val intent = Intent(context, SynchronizationService::class.java) - context.startService(intent) + if (previousSyncState != SyncState.SYNCHRONIZING) { + val intent = Intent(context, SynchronizationService::class.java) + context.startService(intent) + } } /** @@ -150,7 +151,7 @@ object SyncProxy: SyncRequestCollector, SyncManager { return false } - val isStateChanged = StateManager.changeState(SyncState.SYNCHRONIZING) + val isStateChanged = StateManager.changeState(SyncState.PERIODIC_SCAN) if (!isStateChanged) return false application.stopRecursiveFileObserver() @@ -174,7 +175,7 @@ object SyncProxy: SyncRequestCollector, SyncManager { if (!isStateChanged) return - if (previousSyncState == SyncState.IDLE) { + if (previousSyncState == SyncState.IDLE || previousSyncState == SyncState.PERIODIC_SCAN) { application.startRecursiveFileObserver() } } 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); -- GitLab From 0f8357adbb559048d3bba78e438383ea7767c5d3 Mon Sep 17 00:00:00 2001 From: Jonathan Klee Date: Mon, 23 Oct 2023 16:44:44 +0000 Subject: [PATCH 10/14] Apply 1 suggestion(s) to 1 file(s) --- .../java/foundation/e/drive/FileObservers/FileEventListener.java | 1 - 1 file changed, 1 deletion(-) 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 77a41958..aa621e6f 100644 --- a/app/src/main/java/foundation/e/drive/FileObservers/FileEventListener.java +++ b/app/src/main/java/foundation/e/drive/FileObservers/FileEventListener.java @@ -110,7 +110,6 @@ public class FileEventListener { final SyncRequestCollector syncManager = SyncProxy.INSTANCE; syncManager.queueSyncRequest(request, appContext.getApplicationContext()); syncManager.startSynchronization(appContext); - //todo service.startSynchronization(); } -- GitLab From 28ab677255f75f431f7bf125a26fd2ba281e1477 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Tue, 24 Oct 2023 09:13:21 +0200 Subject: [PATCH 11/14] rename StateManager into StateMachine --- .../e/drive/synchronization/{StateManager.kt => StateMachine.kt} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename app/src/main/java/foundation/e/drive/synchronization/{StateManager.kt => StateMachine.kt} (100%) diff --git a/app/src/main/java/foundation/e/drive/synchronization/StateManager.kt b/app/src/main/java/foundation/e/drive/synchronization/StateMachine.kt similarity index 100% rename from app/src/main/java/foundation/e/drive/synchronization/StateManager.kt rename to app/src/main/java/foundation/e/drive/synchronization/StateMachine.kt -- GitLab From 697f327519f2656b5dbce9dd45ed7ea8e22b5c47 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Tue, 24 Oct 2023 11:11:50 +0200 Subject: [PATCH 12/14] add unit test for StateMachine --- .../e/drive/synchronization/StateMachine.kt | 13 ++-- .../e/drive/synchronization/SyncProxy.kt | 10 +-- .../drive/synchronization/StateMachineTest.kt | 74 +++++++++++++++++++ 3 files changed, 86 insertions(+), 11 deletions(-) create mode 100644 app/src/test/java/foundation/e/drive/synchronization/StateMachineTest.kt 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 b034ea5c..d64c2fe2 100644 --- a/app/src/main/java/foundation/e/drive/synchronization/StateMachine.kt +++ b/app/src/main/java/foundation/e/drive/synchronization/StateMachine.kt @@ -7,6 +7,7 @@ */ package foundation.e.drive.synchronization +import androidx.annotation.VisibleForTesting import timber.log.Timber enum class SyncState { @@ -24,14 +25,14 @@ enum class SyncState { * It can be Idle (i.e: after boot, before restart) * @author Vincent Bourgmayer */ -object StateManager { +object StateMachine { - private var currentState: SyncState = SyncState.IDLE - private var lastUpdateTimeInMs = 0L //todo use it to prevent apps to be blocked in one state - fun getCurrentState(): SyncState { - return currentState - } + 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 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 ca238187..463ada82 100644 --- a/app/src/main/java/foundation/e/drive/synchronization/SyncProxy.kt +++ b/app/src/main/java/foundation/e/drive/synchronization/SyncProxy.kt @@ -125,8 +125,8 @@ object SyncProxy: SyncRequestCollector, SyncManager { return } - val previousSyncState = StateManager.getCurrentState() - val isStateChanged = StateManager.changeState(SyncState.SYNCHRONIZING) + val previousSyncState = StateMachine.currentState + val isStateChanged = StateMachine.changeState(SyncState.SYNCHRONIZING) if (!isStateChanged) return @@ -151,7 +151,7 @@ object SyncProxy: SyncRequestCollector, SyncManager { return false } - val isStateChanged = StateManager.changeState(SyncState.PERIODIC_SCAN) + val isStateChanged = StateMachine.changeState(SyncState.PERIODIC_SCAN) if (!isStateChanged) return false application.stopRecursiveFileObserver() @@ -170,8 +170,8 @@ object SyncProxy: SyncRequestCollector, SyncManager { return } - val previousSyncState = StateManager.getCurrentState() - val isStateChanged = StateManager.changeState(SyncState.LISTENING_FILES) + val previousSyncState = StateMachine.currentState + val isStateChanged = StateMachine.changeState(SyncState.LISTENING_FILES) if (!isStateChanged) return 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..4c4252b9 --- /dev/null +++ b/app/src/test/java/foundation/e/drive/synchronization/StateMachineTest.kt @@ -0,0 +1,74 @@ +/* + * 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 org.junit.Test + + +class StateMachineTest { + + @Test + fun `Changing State from PERIODIC_SCAN to LISTENING_FILE should return true`() { + StateMachine + } + + @Test + fun `Changing State from PERIODIC_SCAN to PERIODIC_SCAN should return false`() { + + } + + @Test + fun `Changing State from PERIODIC_SCAN to SYNCHRONING should return true`() { + + } + + @Test + fun `Changing State from PERIODIC_SCAN to IDLE should return false`() { + + } + + @Test + fun `Changing State from LISTENING_FILE to PERIODIC_SCAN should return true`() { + + } + + @Test + fun `Changing State from LISTENING_FILE to SYNCHRONING should return true`() { + + } + + @Test + fun `Changing State from LISTENING_FILE to IDLE should return false`() { + + } + + @Test + fun `Changing State from LISTENING_FILE to LISTENING_FILE should return false`() { + + } + + @Test + fun `Changing State from SYNCHRONIZING to LISTENING_FILE should return true`() { + + } + + @Test + fun `Changing State from SYNCHRONIZING to PERIODIC_SCAN should return false`() { + + } + + @Test + fun `Changing State from SYNCHRONIZING to SYNCHRONIZING should return true`() { + + } + + @Test + fun `Changing State from SYNCHRONIZING to IDLE should return false`() { + + } +} \ No newline at end of file -- GitLab From 3c68d1b307487e7c510b91c51bc47fadf3fcb504 Mon Sep 17 00:00:00 2001 From: Jonathan Klee Date: Tue, 24 Oct 2023 09:13:54 +0000 Subject: [PATCH 13/14] Apply 1 suggestion(s) to 1 file(s) --- .../java/foundation/e/drive/synchronization/StateMachine.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 d64c2fe2..4407d6ce 100644 --- a/app/src/main/java/foundation/e/drive/synchronization/StateMachine.kt +++ b/app/src/main/java/foundation/e/drive/synchronization/StateMachine.kt @@ -34,7 +34,8 @@ object StateMachine { private var lastUpdateTimeInMs = 0L //todo use it to prevent apps to be blocked in one state - @Synchronized fun changeState(newState: SyncState): Boolean { + @Synchronized + fun changeState(newState: SyncState): Boolean { val previousState = currentState val result = when (newState) { SyncState.PERIODIC_SCAN -> setPeriodicScanState() -- GitLab From 5030667c786206a7bb3a065bbc92686787a1c1e3 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Tue, 24 Oct 2023 11:11:50 +0200 Subject: [PATCH 14/14] add unit test for StateMachine --- .../drive/synchronization/StateMachineTest.kt | 123 +++++++++++++++++- 1 file changed, 116 insertions(+), 7 deletions(-) diff --git a/app/src/test/java/foundation/e/drive/synchronization/StateMachineTest.kt b/app/src/test/java/foundation/e/drive/synchronization/StateMachineTest.kt index 4c4252b9..c1674fca 100644 --- a/app/src/test/java/foundation/e/drive/synchronization/StateMachineTest.kt +++ b/app/src/test/java/foundation/e/drive/synchronization/StateMachineTest.kt @@ -7,68 +7,177 @@ */ 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_FILE should return true`() { - StateMachine + 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 SYNCHRONING should return true`() { + 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_FILE to PERIODIC_SCAN should return true`() { + 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 SYNCHRONING should return true`() { + 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_FILE to LISTENING_FILE should return false`() { + 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