From eb38e1fd91b4a9e3c6879e8250d456b1cc9a78ad Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Thu, 5 Jan 2023 15:06:21 +0100 Subject: [PATCH 01/14] Add two missing debug instructions --- .../foundation/e/drive/contentScanner/RemoteContentScanner.java | 1 + .../foundation/e/drive/services/SynchronizationService.java | 2 ++ 2 files changed, 3 insertions(+) diff --git a/app/src/main/java/foundation/e/drive/contentScanner/RemoteContentScanner.java b/app/src/main/java/foundation/e/drive/contentScanner/RemoteContentScanner.java index 342d9505..1cb57ae0 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/RemoteContentScanner.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/RemoteContentScanner.java @@ -82,6 +82,7 @@ public class RemoteContentScanner extends AbstractContentScanner { @Override protected void onMissingFile(SyncedFileState fileState) { + Timber.d("Add local deletion request for file: %s", fileState.getLocalPath()); this.syncRequests.put(fileState.getId(), new SyncRequest(fileState, LOCAL_DELETE)); } 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 77aa340e..b337b411 100644 --- a/app/src/main/java/foundation/e/drive/services/SynchronizationService.java +++ b/app/src/main/java/foundation/e/drive/services/SynchronizationService.java @@ -210,6 +210,8 @@ public class SynchronizationService extends Service implements OnRemoteOperation final SyncWrapper syncWrapper = new SyncWrapper(request, account, getApplicationContext()); if (request.getOperationType().equals(SyncRequest.Type.LOCAL_DELETE)) { + Timber.v(" starts " + request.getSyncedFileState().getName() + + " local deletion on thread " + threadIndex); final LocalFileDeleter fileDeleter = new LocalFileDeleter(request.getSyncedFileState()); threadPool[threadIndex] = new Thread(fileDeleter.getRunnable( handler, threadIndex, getApplicationContext(), this)); threadPool[threadIndex].start(); -- GitLab From ed761db0be9d22418a89b54cd6b0fd6f643a6649 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Thu, 5 Jan 2023 19:28:46 +0100 Subject: [PATCH 02/14] fix one error in a log message --- .../java/foundation/e/drive/operations/UploadFileOperation.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/foundation/e/drive/operations/UploadFileOperation.java b/app/src/main/java/foundation/e/drive/operations/UploadFileOperation.java index 5e30d2e6..caa1874b 100644 --- a/app/src/main/java/foundation/e/drive/operations/UploadFileOperation.java +++ b/app/src/main/java/foundation/e/drive/operations/UploadFileOperation.java @@ -133,7 +133,7 @@ public class UploadFileOperation extends RemoteOperation { //If file already up-to-date & synced if (syncedState.isLastEtagStored() && syncedState.getLocalLastModified() == file.lastModified()) { - Timber.d("Synchronization conflict because: last modified from DB(%s) and from file (%s) are different ", syncedState.getLocalLastModified(), file.lastModified()); + Timber.d("Synchronization conflict because: last modified from DB(%s) and from file (%s) are the same ", syncedState.getLocalLastModified(), file.lastModified()); return ResultCode.SYNC_CONFLICT; } -- GitLab From 25f6b71bd7619e71a0eae18dea7e57130574df92 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Thu, 5 Jan 2023 19:30:13 +0100 Subject: [PATCH 03/14] RemoteContentScanner: check if file has already been synced before to declare that the remote file is missing --- .../e/drive/contentScanner/RemoteContentScanner.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/src/main/java/foundation/e/drive/contentScanner/RemoteContentScanner.java b/app/src/main/java/foundation/e/drive/contentScanner/RemoteContentScanner.java index 1cb57ae0..9ff7c18c 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/RemoteContentScanner.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/RemoteContentScanner.java @@ -82,6 +82,10 @@ public class RemoteContentScanner extends AbstractContentScanner { @Override protected void onMissingFile(SyncedFileState fileState) { + if (!fileState.hasBeenSynchronizedOnce()) { + return; + } + Timber.d("Add local deletion request for file: %s", fileState.getLocalPath()); this.syncRequests.put(fileState.getId(), new SyncRequest(fileState, LOCAL_DELETE)); } -- GitLab From 53d5ea9bee6b4de5c5310c43be35976d18b94409 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Fri, 6 Jan 2023 13:26:21 +0100 Subject: [PATCH 04/14] add a description of syncedFileState in comment --- .../java/foundation/e/drive/models/SyncedFileState.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/foundation/e/drive/models/SyncedFileState.java b/app/src/main/java/foundation/e/drive/models/SyncedFileState.java index 38ee3331..952fafbe 100644 --- a/app/src/main/java/foundation/e/drive/models/SyncedFileState.java +++ b/app/src/main/java/foundation/e/drive/models/SyncedFileState.java @@ -15,8 +15,14 @@ import android.os.Parcelable; /** * @author Vincent Bourgmayer * Describe a file state which will be Synchronized (= Synced) or which has already been synced one times + * + * 1. file state: no etag & LM <=0 : new local file discovered + * todo: shouldn't it be more case 2 below ? would break lot of codes + * 2. filestate: no etag but LM > 0 : ??? + * When does it happens ?, and what does it correspond to ? what side effect of such case ? + * 3. fileState: etag but LM <= 0 : new remote file discovered + * 4. fileState: etag & LM > 0: file synchronized once */ - public class SyncedFileState implements Parcelable { public static final int NOT_SCANNABLE=0; public static final int ECLOUD_SCANNABLE=1; -- GitLab From 869541c717c4e7d64b1c490fd85ab236999492a6 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Mon, 9 Jan 2023 12:49:36 +0100 Subject: [PATCH 05/14] only ignore local file deletion request is file has not been synchronized once --- .../e/drive/contentScanner/LocalContentScanner.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/contentScanner/LocalContentScanner.java b/app/src/main/java/foundation/e/drive/contentScanner/LocalContentScanner.java index 0187ac57..f8f81583 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/LocalContentScanner.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/LocalContentScanner.java @@ -34,9 +34,10 @@ public class LocalContentScanner extends AbstractContentScanner{ @Override protected void onMissingFile(SyncedFileState fileState) { - if (!fileState.hasBeenSynchronizedOnce()) { + /*if (fileState.getLocalLastModified() <= 0) { return; - } + }*/ + if (!fileState.hasBeenSynchronizedOnce()) return; final File file = new File(fileState.getLocalPath()); -- GitLab From 8e7c9324345ab0fbdc37cf802dae41b65c0cc270 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Mon, 23 Jan 2023 12:14:55 +0100 Subject: [PATCH 06/14] add start condition to ObserverService: SyncService shouldn't be working --- .../e/drive/services/ObserverService.java | 7 ++++++- .../e/drive/services/SynchronizationService.java | 13 +++++++++++++ 2 files changed, 19 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 3d201f7b..e422aee7 100644 --- a/app/src/main/java/foundation/e/drive/services/ObserverService.java +++ b/app/src/main/java/foundation/e/drive/services/ObserverService.java @@ -177,7 +177,12 @@ public class ObserverService extends Service implements OnRemoteOperationListene Timber.w("There is no allowed internet connexion."); return false; } - return true; + + + if (synchronizationServiceConnection.isBoundToSynchronizationService()) { + return synchronizationServiceConnection.getSynchronizationService().isIdle(); + } + return false; } /* Common methods */ 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 b337b411..76808395 100644 --- a/app/src/main/java/foundation/e/drive/services/SynchronizationService.java +++ b/app/src/main/java/foundation/e/drive/services/SynchronizationService.java @@ -116,6 +116,19 @@ public class SynchronizationService extends Service implements OnRemoteOperation } + /** + * Check if synchronizationService idle + * meaning sync request is empty and + * no synchronization is running. + * + * It has to be used by periodic scan + * to know if it can start or not. + */ + public boolean isIdle() { + return syncRequestQueue.isEmpty() + && startedSync.values().stream().noneMatch(predicate -> predicate.isRunning()); + } + /** * 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 8f4d4bc40569b3460c3246ab27f3e921cd5d478b Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Mon, 23 Jan 2023 16:14:33 +0100 Subject: [PATCH 07/14] Add feature to pause SynchronizationService: block execution of syncRequest while periodic scan is running --- .../e/drive/services/ObserverService.java | 7 +++++-- .../e/drive/services/SynchronizationService.java | 15 ++++++++++++++- 2 files changed, 19 insertions(+), 3 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 e422aee7..63e73441 100644 --- a/app/src/main/java/foundation/e/drive/services/ObserverService.java +++ b/app/src/main/java/foundation/e/drive/services/ObserverService.java @@ -179,8 +179,10 @@ public class ObserverService extends Service implements OnRemoteOperationListene } - if (synchronizationServiceConnection.isBoundToSynchronizationService()) { - return synchronizationServiceConnection.getSynchronizationService().isIdle(); + if (synchronizationServiceConnection.isBoundToSynchronizationService() + && synchronizationServiceConnection.getSynchronizationService().isIdle()) { + synchronizationServiceConnection.getSynchronizationService().setPaused(true); + return true; } return false; } @@ -347,6 +349,7 @@ public class ObserverService extends Service implements OnRemoteOperationListene private void passSyncRequestsToSynchronizationService() { if (synchronizationServiceConnection.isBoundToSynchronizationService()) { synchronizationServiceConnection.getSynchronizationService().queueSyncRequests(syncRequests.values()); + synchronizationServiceConnection.getSynchronizationService().setPaused(false); synchronizationServiceConnection.getSynchronizationService().startSynchronization(); } else { Timber.e("ERROR: impossible to bind ObserverService to SynchronizationService"); 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 76808395..8c57622b 100644 --- a/app/src/main/java/foundation/e/drive/services/SynchronizationService.java +++ b/app/src/main/java/foundation/e/drive/services/SynchronizationService.java @@ -63,6 +63,9 @@ public class SynchronizationService extends Service implements OnRemoteOperation private NextcloudClient ncClient; private Handler handler; private HandlerThread handlerThread; + private boolean paused = false; + + @Override public void onCreate() { super.onCreate(); @@ -129,6 +132,16 @@ public class SynchronizationService extends Service implements OnRemoteOperation && startedSync.values().stream().noneMatch(predicate -> predicate.isRunning()); } + + /** + * Set value of a paused boolean to prevent starting of synchronization while + * Method used by ObserverService to prevent synchronization from FileListener to start + * @param paused + */ + public void setPaused(boolean paused) { + this.paused = paused; + } + /** * Add a SyncRequest into waiting queue if it matches some conditions: * - an equivalent sync Request (same file & same operation type) isn't already running @@ -211,7 +224,7 @@ public class SynchronizationService extends Service implements OnRemoteOperation } private void startWorker(Integer threadIndex){ - if (!canStart(threadIndex)) return; + if (!canStart(threadIndex) || paused) return; final SyncRequest request = this.syncRequestQueue.poll(); //return null if empty if (request == null -- GitLab From af141418c7603978dc2550d049fc6617841c444d Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Tue, 24 Jan 2023 11:51:53 +0100 Subject: [PATCH 08/14] update pause mechanism --- .../e/drive/services/ObserverService.java | 10 +++----- .../services/SynchronizationService.java | 25 ++++++++----------- 2 files changed, 14 insertions(+), 21 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 63e73441..27cab6b3 100644 --- a/app/src/main/java/foundation/e/drive/services/ObserverService.java +++ b/app/src/main/java/foundation/e/drive/services/ObserverService.java @@ -179,12 +179,8 @@ public class ObserverService extends Service implements OnRemoteOperationListene } - if (synchronizationServiceConnection.isBoundToSynchronizationService() - && synchronizationServiceConnection.getSynchronizationService().isIdle()) { - synchronizationServiceConnection.getSynchronizationService().setPaused(true); - return true; - } - return false; + return synchronizationServiceConnection.isBoundToSynchronizationService() + && synchronizationServiceConnection.getSynchronizationService().pauseSync(); } /* Common methods */ @@ -349,7 +345,7 @@ public class ObserverService extends Service implements OnRemoteOperationListene private void passSyncRequestsToSynchronizationService() { if (synchronizationServiceConnection.isBoundToSynchronizationService()) { synchronizationServiceConnection.getSynchronizationService().queueSyncRequests(syncRequests.values()); - synchronizationServiceConnection.getSynchronizationService().setPaused(false); + synchronizationServiceConnection.getSynchronizationService().unPauseSync(); synchronizationServiceConnection.getSynchronizationService().startSynchronization(); } else { Timber.e("ERROR: impossible to bind ObserverService to SynchronizationService"); 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 8c57622b..09e2d740 100644 --- a/app/src/main/java/foundation/e/drive/services/SynchronizationService.java +++ b/app/src/main/java/foundation/e/drive/services/SynchronizationService.java @@ -120,26 +120,23 @@ public class SynchronizationService extends Service implements OnRemoteOperation /** - * Check if synchronizationService idle - * meaning sync request is empty and - * no synchronization is running. - * - * It has to be used by periodic scan - * to know if it can start or not. + * 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 isIdle() { - return syncRequestQueue.isEmpty() + public boolean pauseSync() { + this.paused = true; + final boolean isPausable = syncRequestQueue.isEmpty() && startedSync.values().stream().noneMatch(predicate -> predicate.isRunning()); + paused = isPausable; + return paused; } - /** - * Set value of a paused boolean to prevent starting of synchronization while - * Method used by ObserverService to prevent synchronization from FileListener to start - * @param paused + * unpause synchronization */ - public void setPaused(boolean paused) { - this.paused = paused; + public void unPauseSync() { + this.paused = false; } /** -- GitLab From 14d9c75b39437707320a65475a6d7822decee6ea Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Tue, 24 Jan 2023 16:28:32 +0100 Subject: [PATCH 09/14] Add a time limit for SynchronizationService's pause --- .../e/drive/services/ObserverService.java | 3 --- .../services/SynchronizationService.java | 19 ++++++++++++------- 2 files changed, 12 insertions(+), 10 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 27cab6b3..842e25f2 100644 --- a/app/src/main/java/foundation/e/drive/services/ObserverService.java +++ b/app/src/main/java/foundation/e/drive/services/ObserverService.java @@ -86,7 +86,6 @@ public class ObserverService extends Service implements OnRemoteOperationListene this.mSyncedFolders = null; } - @Override public void onCreate() { super.onCreate(); @@ -170,7 +169,6 @@ public class ObserverService extends Service implements OnRemoteOperationListene return false; } - final boolean meteredNetworkAllowed = CommonUtils.isMeteredNetworkAllowed(mAccount); //check for the case where intent has been launched by initializerService if (!CommonUtils.haveNetworkConnection(this, meteredNetworkAllowed)) { @@ -178,7 +176,6 @@ public class ObserverService extends Service implements OnRemoteOperationListene return false; } - return synchronizationServiceConnection.isBoundToSynchronizationService() && synchronizationServiceConnection.getSynchronizationService().pauseSync(); } 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 09e2d740..129e5af9 100644 --- a/app/src/main/java/foundation/e/drive/services/SynchronizationService.java +++ b/app/src/main/java/foundation/e/drive/services/SynchronizationService.java @@ -51,6 +51,7 @@ import timber.log.Timber; */ public class SynchronizationService extends Service implements OnRemoteOperationListener, LocalFileDeleter.LocalFileDeletionListener { private final SynchronizationBinder binder = new SynchronizationBinder(); + private final static long maxPauseTime = 300000; //5 minutes, might need to be adapted private ConcurrentLinkedDeque syncRequestQueue; private ConcurrentHashMap startedSync; //Integer is thread index (1 to workerAmount) @@ -63,8 +64,7 @@ public class SynchronizationService extends Service implements OnRemoteOperation private NextcloudClient ncClient; private Handler handler; private HandlerThread handlerThread; - private boolean paused = false; - + private long pauseStartTime = 0L; @Override public void onCreate() { @@ -125,18 +125,23 @@ public class SynchronizationService extends Service implements OnRemoteOperation * @return true if service successfully paused */ public boolean pauseSync() { - this.paused = true; + pauseStartTime = System.currentTimeMillis(); final boolean isPausable = syncRequestQueue.isEmpty() && startedSync.values().stream().noneMatch(predicate -> predicate.isRunning()); - paused = isPausable; - return paused; + if (!isPausable) pauseStartTime = 0L; + return isPausable; } /** * unpause synchronization */ public void unPauseSync() { - this.paused = false; + pauseStartTime = 0L; + } + + private boolean isPaused() { + if (pauseStartTime == 0L) return false; + return System.currentTimeMillis() - pauseStartTime < maxPauseTime; } /** @@ -221,7 +226,7 @@ public class SynchronizationService extends Service implements OnRemoteOperation } private void startWorker(Integer threadIndex){ - if (!canStart(threadIndex) || paused) return; + if (!canStart(threadIndex) || isPaused()) return; final SyncRequest request = this.syncRequestQueue.poll(); //return null if empty if (request == null -- GitLab From e92f70cc36d06ee7393cfec3420655c3728a6a7f Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Tue, 24 Jan 2023 16:39:00 +0100 Subject: [PATCH 10/14] remove commented code and linebreak --- .../e/drive/contentScanner/LocalContentScanner.java | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/contentScanner/LocalContentScanner.java b/app/src/main/java/foundation/e/drive/contentScanner/LocalContentScanner.java index f8f81583..65ad9d4e 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/LocalContentScanner.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/LocalContentScanner.java @@ -34,9 +34,6 @@ public class LocalContentScanner extends AbstractContentScanner{ @Override protected void onMissingFile(SyncedFileState fileState) { - /*if (fileState.getLocalLastModified() <= 0) { - return; - }*/ if (!fileState.hasBeenSynchronizedOnce()) return; final File file = new File(fileState.getLocalPath()); @@ -90,6 +87,4 @@ public class LocalContentScanner extends AbstractContentScanner{ final String filePath = CommonUtils.getLocalPath(file); return fileState.getLocalPath().equals(filePath); } -} - - +} \ No newline at end of file -- GitLab From 938594a012033ce6578bce8705bdf1d00b75e64c Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Wed, 25 Jan 2023 15:13:03 +0100 Subject: [PATCH 11/14] Fix bound service issue Add debug information for pause mechanism --- .../e/drive/services/ObserverService.java | 48 ++++++++++++------- .../services/SynchronizationService.java | 10 +++- 2 files changed, 38 insertions(+), 20 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 842e25f2..59f406ca 100644 --- a/app/src/main/java/foundation/e/drive/services/ObserverService.java +++ b/app/src/main/java/foundation/e/drive/services/ObserverService.java @@ -15,6 +15,7 @@ import static foundation.e.drive.utils.AppConstants.INITIALIZATION_HAS_BEEN_DONE 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; @@ -29,23 +30,19 @@ import com.owncloud.android.lib.common.OwnCloudClient; import com.owncloud.android.lib.common.operations.OnRemoteOperationListener; import com.owncloud.android.lib.common.operations.RemoteOperation; import com.owncloud.android.lib.common.operations.RemoteOperationResult; -import com.owncloud.android.lib.resources.files.FileUtils; import com.owncloud.android.lib.resources.files.model.RemoteFile; import java.io.File; -import java.io.FileFilter; import java.io.FileOutputStream; import java.util.ArrayList; import java.util.HashMap; import java.util.List; -import java.util.ListIterator; import foundation.e.drive.contentScanner.LocalContentScanner; import foundation.e.drive.contentScanner.LocalFileLister; import foundation.e.drive.contentScanner.RemoteContentScanner; import foundation.e.drive.database.DbHelper; import foundation.e.drive.fileFilters.CrashlogsFileFilter; -import foundation.e.drive.fileFilters.FileFilterFactory; import foundation.e.drive.fileFilters.OnlyFileFilter; import foundation.e.drive.models.SyncRequest; import foundation.e.drive.models.SyncedFileState; @@ -72,15 +69,31 @@ public class ObserverService extends Service implements OnRemoteOperationListene private boolean isWorking = false; private Account mAccount; private HashMap syncRequests; //integer is SyncedFileState id; Parcelable is the operation - private SynchronizationServiceConnection synchronizationServiceConnection = new SynchronizationServiceConnection(); + private SynchronizationServiceConnection synchronizationServiceConnection = new SynchronizationServiceConnection() { + @Override + public void onServiceConnected(ComponentName componentName, 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 Handler handler; private HandlerThread handlerThread; + private boolean forcedSync = false; /* Lifecycle Methods */ @Override public void onDestroy(){ Timber.v("onDestroy()"); - unbindService(synchronizationServiceConnection); + if (synchronizationServiceConnection.isBoundToSynchronizationService()) { + getApplicationContext().unbindService(synchronizationServiceConnection); + } handlerThread.quitSafely(); super.onDestroy(); this.mSyncedFolders = null; @@ -90,13 +103,13 @@ public class ObserverService extends Service implements OnRemoteOperationListene public void onCreate() { super.onCreate(); Timber.tag(ObserverService.class.getSimpleName()); + final Intent SynchronizationServiceIntent = new Intent(this, SynchronizationService.class); + getApplicationContext().bindService(SynchronizationServiceIntent, synchronizationServiceConnection, Context.BIND_AUTO_CREATE); } @Override public int onStartCommand(Intent intent, int flags, int startId) { Timber.i("onStartCommand(%s)", startId); - final Intent SynchronizationServiceIntent = new Intent(this, SynchronizationService.class); - bindService(SynchronizationServiceIntent, synchronizationServiceConnection, Context.BIND_AUTO_CREATE); CommonUtils.setServiceUnCaughtExceptionHandler(this); @@ -105,18 +118,12 @@ public class ObserverService extends Service implements OnRemoteOperationListene final String accountType = prefs.getString(AccountManager.KEY_ACCOUNT_TYPE, ""); this.mAccount = CommonUtils.getAccount(accountName, accountType, AccountManager.get(this)); - final boolean forcedSync = intent != null && DebugCmdReceiver.ACTION_FORCE_SYNC.equals(intent.getAction()); - - if (!checkStartCondition(prefs, forcedSync)) { - return super.onStartCommand(intent, flags, startId); - } + forcedSync = intent != null && DebugCmdReceiver.ACTION_FORCE_SYNC.equals(intent.getAction()); this.syncRequests = new HashMap<>(); - handlerThread = new HandlerThread("syncService_onResponse"); - handlerThread.start(); - handler = new Handler(handlerThread.getLooper()); - begin(); + + //begin(); return START_NOT_STICKY; } @@ -176,8 +183,10 @@ public class ObserverService extends Service implements OnRemoteOperationListene return false; } - return synchronizationServiceConnection.isBoundToSynchronizationService() + final boolean isSyncServicePaused = synchronizationServiceConnection.isBoundToSynchronizationService() && synchronizationServiceConnection.getSynchronizationService().pauseSync(); + Timber.d("isSyncServicePaused ? %s", isSyncServicePaused); + return isSyncServicePaused; } /* Common methods */ @@ -262,6 +271,9 @@ public class ObserverService extends Service implements OnRemoteOperationListene } try { + handlerThread = new HandlerThread("syncService_onResponse"); + handlerThread.start(); + handler = new Handler(handlerThread.getLooper()); final ListFileRemoteOperation loadOperation = new ListFileRemoteOperation(this.mSyncedFolders, this); loadOperation.execute(client, this, handler); } catch (IllegalArgumentException exception) { 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 129e5af9..f82731ec 100644 --- a/app/src/main/java/foundation/e/drive/services/SynchronizationService.java +++ b/app/src/main/java/foundation/e/drive/services/SynchronizationService.java @@ -126,9 +126,15 @@ public class SynchronizationService extends Service implements OnRemoteOperation */ public boolean pauseSync() { pauseStartTime = System.currentTimeMillis(); - final boolean isPausable = syncRequestQueue.isEmpty() - && startedSync.values().stream().noneMatch(predicate -> predicate.isRunning()); + + final boolean isQueueEmpty = syncRequestQueue.isEmpty(); + final boolean isNoStartedSync = startedSync.values().stream().noneMatch(predicate -> predicate.isRunning()); + + Timber.v("is queue empty ? %s ; is no started sync ? %s", isQueueEmpty, isNoStartedSync); + final boolean isPausable = isQueueEmpty + && isNoStartedSync; if (!isPausable) pauseStartTime = 0L; + Timber.d("SyncService.pauseSync(): is pausable: %s", isPausable); return isPausable; } -- GitLab From cae13c6abea3d0fedfc8fca26b63bead713eb155 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Thu, 26 Jan 2023 11:46:25 +0100 Subject: [PATCH 12/14] Update SynchronizationServiceConnection to make safe binding and unbinding Update ObserverService.onDestroy() to prevent NPE when killing the service --- .../FileObservers/FileEventListener.java | 10 ++--- .../e/drive/services/ObserverService.java | 19 ++++----- .../SynchronizationServiceConnection.java | 39 ++++++++++++++++--- 3 files changed, 43 insertions(+), 25 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 91463c3c..57c342c8 100644 --- a/app/src/main/java/foundation/e/drive/FileObservers/FileEventListener.java +++ b/app/src/main/java/foundation/e/drive/FileObservers/FileEventListener.java @@ -103,7 +103,7 @@ public class FileEventListener { */ private void sendSyncRequestToSynchronizationService(SyncRequest request) { Timber.d("Sending a SyncRequest for %s", request.getSyncedFileState().getName()); - if (serviceConnection.isBoundToSynchronizationService()) { + if (serviceConnection.isBound()) { serviceConnection.getSynchronizationService().queueSyncRequest(request); serviceConnection.getSynchronizationService().startSynchronization(); }else{ @@ -233,14 +233,10 @@ public class FileEventListener { } public void unbindFromSynchronizationService(){ - if (serviceConnection.isBoundToSynchronizationService()) - appContext.unbindService(serviceConnection); - else - Timber.w("Not bound to SynchronizationService: can't unbind."); + serviceConnection.unBind(appContext); } public void bindToSynchronizationService(){ - final Intent SynchronizationServiceIntent = new Intent(appContext, SynchronizationService.class); - appContext.bindService(SynchronizationServiceIntent, serviceConnection, Context.BIND_AUTO_CREATE); + serviceConnection.bindService(appContext); } } 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 59f406ca..c791bb21 100644 --- a/app/src/main/java/foundation/e/drive/services/ObserverService.java +++ b/app/src/main/java/foundation/e/drive/services/ObserverService.java @@ -91,20 +91,17 @@ public class ObserverService extends Service implements OnRemoteOperationListene @Override public void onDestroy(){ Timber.v("onDestroy()"); - if (synchronizationServiceConnection.isBoundToSynchronizationService()) { - getApplicationContext().unbindService(synchronizationServiceConnection); - } - handlerThread.quitSafely(); + synchronizationServiceConnection.unBind(getApplicationContext()); + if (handlerThread != null) handlerThread.quitSafely(); + mSyncedFolders = null; super.onDestroy(); - this.mSyncedFolders = null; } @Override public void onCreate() { super.onCreate(); Timber.tag(ObserverService.class.getSimpleName()); - final Intent SynchronizationServiceIntent = new Intent(this, SynchronizationService.class); - getApplicationContext().bindService(SynchronizationServiceIntent, synchronizationServiceConnection, Context.BIND_AUTO_CREATE); + synchronizationServiceConnection.bindService(getApplicationContext()); } @Override @@ -122,9 +119,7 @@ public class ObserverService extends Service implements OnRemoteOperationListene this.syncRequests = new HashMap<>(); - - //begin(); - return START_NOT_STICKY; + return START_STICKY; } /** @@ -183,7 +178,7 @@ public class ObserverService extends Service implements OnRemoteOperationListene return false; } - final boolean isSyncServicePaused = synchronizationServiceConnection.isBoundToSynchronizationService() + final boolean isSyncServicePaused = synchronizationServiceConnection.isBound() && synchronizationServiceConnection.getSynchronizationService().pauseSync(); Timber.d("isSyncServicePaused ? %s", isSyncServicePaused); return isSyncServicePaused; @@ -352,7 +347,7 @@ public class ObserverService extends Service implements OnRemoteOperationListene * Send all gathered SyncRequest to SynchronizationService */ private void passSyncRequestsToSynchronizationService() { - if (synchronizationServiceConnection.isBoundToSynchronizationService()) { + if (synchronizationServiceConnection.isBound()) { synchronizationServiceConnection.getSynchronizationService().queueSyncRequests(syncRequests.values()); synchronizationServiceConnection.getSynchronizationService().unPauseSync(); synchronizationServiceConnection.getSynchronizationService().startSynchronization(); diff --git a/app/src/main/java/foundation/e/drive/utils/SynchronizationServiceConnection.java b/app/src/main/java/foundation/e/drive/utils/SynchronizationServiceConnection.java index 5a11626c..f8aaab6d 100644 --- a/app/src/main/java/foundation/e/drive/utils/SynchronizationServiceConnection.java +++ b/app/src/main/java/foundation/e/drive/utils/SynchronizationServiceConnection.java @@ -9,6 +9,8 @@ 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; @@ -21,26 +23,28 @@ import timber.log.Timber; public class SynchronizationServiceConnection implements ServiceConnection { private SynchronizationService synchronizationService; - private boolean boundToSynchronizationService = false; + private boolean isBound = false; + private boolean attemptingToBind = false; @Override public void onServiceConnected(ComponentName componentName, IBinder iBinder) { Timber.tag(SynchronizationServiceConnection.class.getSimpleName()) .v("binding to SynchronizationService"); - SynchronizationService.SynchronizationBinder binder = (SynchronizationService.SynchronizationBinder) iBinder; + attemptingToBind = false; + isBound = true; + final SynchronizationService.SynchronizationBinder binder = (SynchronizationService.SynchronizationBinder) iBinder; synchronizationService = binder.getService(); - boundToSynchronizationService = true; } @Override public void onServiceDisconnected(ComponentName componentName) { Timber.tag(SynchronizationServiceConnection.class.getSimpleName()) .v("Unbinding from SynchronizationService"); - boundToSynchronizationService = false; + isBound = false; } - public boolean isBoundToSynchronizationService() { - return boundToSynchronizationService; + public boolean isBound() { + return isBound; } /** @@ -50,4 +54,27 @@ public class SynchronizationServiceConnection implements ServiceConnection { public SynchronizationService getSynchronizationService() { return synchronizationService; } + + /** + * Sources: https://stackoverflow.com/questions/8614335/android-how-to-safely-unbind-a-service + * @param context + */ + public void unBind(Context context) { + attemptingToBind = false; + if (isBound) { + context.unbindService(this); + isBound = false; + } + } + + /** + * Sources: https://stackoverflow.com/questions/8614335/android-how-to-safely-unbind-a-service + * @param context + */ + public void bindService(Context context) { + if (!attemptingToBind) { + attemptingToBind = true; + context.bindService(new Intent(context, SynchronizationService.class), this, Context.BIND_AUTO_CREATE); + } + } } -- GitLab From 3ffd012f4485123a6fb02b93f0932b136330c43d Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Thu, 26 Jan 2023 13:15:22 +0100 Subject: [PATCH 13/14] unpause Synchronization service even if no requests to send and update some log --- .../java/foundation/e/drive/services/ObserverService.java | 8 ++++++-- .../e/drive/services/SynchronizationService.java | 4 +--- .../e/drive/utils/SynchronizationServiceConnection.java | 2 +- 3 files changed, 8 insertions(+), 6 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 c791bb21..418b28df 100644 --- a/app/src/main/java/foundation/e/drive/services/ObserverService.java +++ b/app/src/main/java/foundation/e/drive/services/ObserverService.java @@ -329,6 +329,10 @@ public class ObserverService extends Service implements OnRemoteOperationListene startScan(false); + if (synchronizationServiceConnection.isBound()) { + synchronizationServiceConnection.getSynchronizationService().unPauseSync(); + } + if (!syncRequests.isEmpty()) { Timber.d("syncRequests contains %s", syncRequests.size()); passSyncRequestsToSynchronizationService(); @@ -339,6 +343,7 @@ public class ObserverService extends Service implements OnRemoteOperationListene .putLong(AppConstants.KEY_LAST_SYNC_TIME, System.currentTimeMillis()) .apply(); } + this.isWorking = false; this.stopSelf(); } @@ -349,10 +354,9 @@ public class ObserverService extends Service implements OnRemoteOperationListene private void passSyncRequestsToSynchronizationService() { if (synchronizationServiceConnection.isBound()) { synchronizationServiceConnection.getSynchronizationService().queueSyncRequests(syncRequests.values()); - synchronizationServiceConnection.getSynchronizationService().unPauseSync(); synchronizationServiceConnection.getSynchronizationService().startSynchronization(); } else { - Timber.e("ERROR: impossible to bind ObserverService to SynchronizationService"); + Timber.e("ERROR: binding to SynchronizationService lost"); } } 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 f82731ec..fe7ecc87 100644 --- a/app/src/main/java/foundation/e/drive/services/SynchronizationService.java +++ b/app/src/main/java/foundation/e/drive/services/SynchronizationService.java @@ -131,10 +131,8 @@ public class SynchronizationService extends Service implements OnRemoteOperation final boolean isNoStartedSync = startedSync.values().stream().noneMatch(predicate -> predicate.isRunning()); Timber.v("is queue empty ? %s ; is no started sync ? %s", isQueueEmpty, isNoStartedSync); - final boolean isPausable = isQueueEmpty - && isNoStartedSync; + final boolean isPausable = isQueueEmpty && isNoStartedSync; if (!isPausable) pauseStartTime = 0L; - Timber.d("SyncService.pauseSync(): is pausable: %s", isPausable); return isPausable; } diff --git a/app/src/main/java/foundation/e/drive/utils/SynchronizationServiceConnection.java b/app/src/main/java/foundation/e/drive/utils/SynchronizationServiceConnection.java index f8aaab6d..bd92f6da 100644 --- a/app/src/main/java/foundation/e/drive/utils/SynchronizationServiceConnection.java +++ b/app/src/main/java/foundation/e/drive/utils/SynchronizationServiceConnection.java @@ -39,7 +39,7 @@ public class SynchronizationServiceConnection implements ServiceConnection { @Override public void onServiceDisconnected(ComponentName componentName) { Timber.tag(SynchronizationServiceConnection.class.getSimpleName()) - .v("Unbinding from SynchronizationService"); + .v("onServiceDisconnected()"); isBound = false; } -- GitLab From 8d80127b7a92c38ff84377b013a789f02222c740 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Fri, 27 Jan 2023 09:03:39 +0100 Subject: [PATCH 14/14] add NonNull annotation & rename static variable in SynchronizationService --- .../foundation/e/drive/services/SynchronizationService.java | 4 ++-- .../e/drive/utils/SynchronizationServiceConnection.java | 6 ++++-- 2 files changed, 6 insertions(+), 4 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 fe7ecc87..0eaa40ff 100644 --- a/app/src/main/java/foundation/e/drive/services/SynchronizationService.java +++ b/app/src/main/java/foundation/e/drive/services/SynchronizationService.java @@ -51,7 +51,7 @@ import timber.log.Timber; */ public class SynchronizationService extends Service implements OnRemoteOperationListener, LocalFileDeleter.LocalFileDeletionListener { private final SynchronizationBinder binder = new SynchronizationBinder(); - private final static long maxPauseTime = 300000; //5 minutes, might need to be adapted + 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) @@ -145,7 +145,7 @@ public class SynchronizationService extends Service implements OnRemoteOperation private boolean isPaused() { if (pauseStartTime == 0L) return false; - return System.currentTimeMillis() - pauseStartTime < maxPauseTime; + return System.currentTimeMillis() - pauseStartTime < maxPauseTimeInMs; } /** diff --git a/app/src/main/java/foundation/e/drive/utils/SynchronizationServiceConnection.java b/app/src/main/java/foundation/e/drive/utils/SynchronizationServiceConnection.java index bd92f6da..370ed76e 100644 --- a/app/src/main/java/foundation/e/drive/utils/SynchronizationServiceConnection.java +++ b/app/src/main/java/foundation/e/drive/utils/SynchronizationServiceConnection.java @@ -14,6 +14,8 @@ import android.content.Intent; import android.content.ServiceConnection; import android.os.IBinder; +import androidx.annotation.NonNull; + import foundation.e.drive.services.SynchronizationService; import timber.log.Timber; @@ -59,7 +61,7 @@ public class SynchronizationServiceConnection implements ServiceConnection { * Sources: https://stackoverflow.com/questions/8614335/android-how-to-safely-unbind-a-service * @param context */ - public void unBind(Context context) { + public void unBind(@NonNull Context context) { attemptingToBind = false; if (isBound) { context.unbindService(this); @@ -71,7 +73,7 @@ public class SynchronizationServiceConnection implements ServiceConnection { * Sources: https://stackoverflow.com/questions/8614335/android-how-to-safely-unbind-a-service * @param context */ - public void bindService(Context context) { + public void bindService(@NonNull Context context) { if (!attemptingToBind) { attemptingToBind = true; context.bindService(new Intent(context, SynchronizationService.class), this, Context.BIND_AUTO_CREATE); -- GitLab