From 29c6ce7a94bafca5c772c8bc73ccf9443fc8a425 Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Tue, 13 Feb 2024 07:22:07 +0600 Subject: [PATCH 1/7] feat: enable cookie sharing in DAV requests - replace nc-android-lib with eOS fork to support session sharing - refactor DavClientProvider; instead of maintaining singleton client ourselves, use nc-android-lib functionality - update DavClient instance after every successful usage --- app/build.gradle | 2 +- .../drive/account/AccountUserInfoWorker.java | 4 + .../account/receivers/AccountAddedReceiver.kt | 2 +- .../AccountRemoveCallbackReceiver.java | 21 ++- .../account/setup/RootFolderSetupWorker.java | 3 + .../e/drive/activity/AccountsActivity.java | 8 +- .../e/drive/periodicScan/FullScanWorker.kt | 2 + .../e/drive/synchronization/SyncWorker.kt | 2 + .../tasks/UploadFileOperation.java | 6 +- .../e/drive/utils/DavClientProvider.java | 121 +++++++----------- .../foundation/e/drive/utils/WorkerUtils.kt | 2 - 11 files changed, 88 insertions(+), 85 deletions(-) diff --git a/app/build.gradle b/app/build.gradle index ca4b9865..fde6fff4 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -94,7 +94,7 @@ android { } dependencies { - implementation 'com.github.nextcloud:android-library:2.13.0' + implementation 'foundation.e:Nextcloud-Android-Library:1.0.1-alpha' implementation "commons-httpclient:commons-httpclient:3.1@jar" implementation fileTree(include: ['*.jar'], dir: 'libs') api 'androidx.annotation:annotation:1.6.0' diff --git a/app/src/main/java/foundation/e/drive/account/AccountUserInfoWorker.java b/app/src/main/java/foundation/e/drive/account/AccountUserInfoWorker.java index ef376d0c..c1361c95 100644 --- a/app/src/main/java/foundation/e/drive/account/AccountUserInfoWorker.java +++ b/app/src/main/java/foundation/e/drive/account/AccountUserInfoWorker.java @@ -88,6 +88,7 @@ public class AccountUserInfoWorker extends Worker { .diskCacheStrategy(DiskCacheStrategy.ALL) .preload(); ViewUtils.updateWidgetView(mContext); + DavClientProvider.getInstance().saveAccounts(mContext); return Result.success(); } else { return Result.retry(); @@ -235,6 +236,9 @@ public class AccountUserInfoWorker extends Worker { } accountManager.setUserData(account, ACCOUNT_DATA_ALIAS_KEY, aliases); Timber.d("fetchAliases(): success"); + + DavClientProvider.getInstance().saveAccounts(mContext); + return true; } } diff --git a/app/src/main/java/foundation/e/drive/account/receivers/AccountAddedReceiver.kt b/app/src/main/java/foundation/e/drive/account/receivers/AccountAddedReceiver.kt index a95d15c6..704a780e 100644 --- a/app/src/main/java/foundation/e/drive/account/receivers/AccountAddedReceiver.kt +++ b/app/src/main/java/foundation/e/drive/account/receivers/AccountAddedReceiver.kt @@ -73,7 +73,7 @@ class AccountAddedReceiver() : BroadcastReceiver() { } if (!isExistingAccount(accountName, accountType, context)) { - Timber.w("No account exist for username: %s ", accountType, accountName) + Timber.w("No account exist for type: %s, username: %s", accountType, accountName) return false } return true diff --git a/app/src/main/java/foundation/e/drive/account/receivers/AccountRemoveCallbackReceiver.java b/app/src/main/java/foundation/e/drive/account/receivers/AccountRemoveCallbackReceiver.java index a04a6b18..89a8c846 100644 --- a/app/src/main/java/foundation/e/drive/account/receivers/AccountRemoveCallbackReceiver.java +++ b/app/src/main/java/foundation/e/drive/account/receivers/AccountRemoveCallbackReceiver.java @@ -11,6 +11,7 @@ package foundation.e.drive.account.receivers; import static foundation.e.drive.utils.AppConstants.INITIAL_FOLDER_NUMBER; import static foundation.e.drive.utils.AppConstants.SETUP_COMPLETED; +import android.accounts.Account; import android.accounts.AccountManager; import android.annotation.SuppressLint; import android.app.NotificationManager; @@ -56,7 +57,7 @@ public class AccountRemoveCallbackReceiver extends BroadcastReceiver { cleanSharedPreferences(applicationContext, preferences); removeCachedFiles(applicationContext); deleteNotificationChannels(applicationContext); - DavClientProvider.getInstance().cleanUp(); + clearDavClient(intent, applicationContext); ViewUtils.updateWidgetView(applicationContext); } @@ -78,6 +79,24 @@ public class AccountRemoveCallbackReceiver extends BroadcastReceiver { } } + private void clearDavClient(@NonNull Intent intent, @NonNull Context context) { + if (intent.getExtras() == null) { + return; + } + + final String accountType = intent.getExtras().getString(AccountManager.KEY_ACCOUNT_TYPE); + final String accountName = intent.getExtras().getString(AccountManager.KEY_ACCOUNT_NAME); + + AccountManager accountManager = AccountManager.get(context); + + for (Account account : accountManager.getAccountsByType(accountType)) { + if (account.name.equals(accountName)) { + DavClientProvider.getInstance().cleanUp(context, account); + break; + } + } + } + private boolean shouldProceedWithRemoval(@NonNull Intent intent, @NonNull SharedPreferences preferences, @NonNull Context context) { if (isInvalidAction(intent) || intent.getExtras() == null) { Timber.w("Invalid account removal request"); diff --git a/app/src/main/java/foundation/e/drive/account/setup/RootFolderSetupWorker.java b/app/src/main/java/foundation/e/drive/account/setup/RootFolderSetupWorker.java index 6488eeb5..b028653c 100644 --- a/app/src/main/java/foundation/e/drive/account/setup/RootFolderSetupWorker.java +++ b/app/src/main/java/foundation/e/drive/account/setup/RootFolderSetupWorker.java @@ -94,6 +94,9 @@ public class RootFolderSetupWorker extends Worker { new CreateFolderRemoteOperation(syncedFolder.getRemoteFolder(), true); @SuppressWarnings("deprecation") final RemoteOperationResult result = mkcolRequest.execute(client); + + DavClientProvider.getInstance().saveAccounts(context); + if (result.isSuccess() || result.getCode() == RemoteOperationResult.ResultCode.FOLDER_ALREADY_EXISTS) { DbHelper.insertSyncedFolder(syncedFolder, context); return Result.success(); diff --git a/app/src/main/java/foundation/e/drive/activity/AccountsActivity.java b/app/src/main/java/foundation/e/drive/activity/AccountsActivity.java index 5929f370..641b3c47 100644 --- a/app/src/main/java/foundation/e/drive/activity/AccountsActivity.java +++ b/app/src/main/java/foundation/e/drive/activity/AccountsActivity.java @@ -166,4 +166,10 @@ public class AccountsActivity extends AppCompatActivity { binding.avatar.setVisibility(View.VISIBLE); } } -} \ No newline at end of file + + @Override + protected void onDestroy() { + DavClientProvider.getInstance().saveAccounts(this); + super.onDestroy(); + } +} diff --git a/app/src/main/java/foundation/e/drive/periodicScan/FullScanWorker.kt b/app/src/main/java/foundation/e/drive/periodicScan/FullScanWorker.kt index 49cc9c53..08415ed9 100644 --- a/app/src/main/java/foundation/e/drive/periodicScan/FullScanWorker.kt +++ b/app/src/main/java/foundation/e/drive/periodicScan/FullScanWorker.kt @@ -191,6 +191,8 @@ class FullScanWorker(private val context: Context, private val workerParams: Wor val syncedFoldersIds = listRemoteFilesOperation.syncedFoldersId val syncedFileStates = DbHelper.getSyncedFileStatesByFolders(context, syncedFoldersIds) + DavClientProvider.getInstance().saveAccounts(applicationContext) + if (remoteFiles.isNotEmpty() || syncedFileStates.isNotEmpty()) { val scanner = RemoteContentScanner(context, syncedFolders) return scanner.scanContent(remoteFiles, syncedFileStates) diff --git a/app/src/main/java/foundation/e/drive/synchronization/SyncWorker.kt b/app/src/main/java/foundation/e/drive/synchronization/SyncWorker.kt index eda410b2..2eaa32fc 100644 --- a/app/src/main/java/foundation/e/drive/synchronization/SyncWorker.kt +++ b/app/src/main/java/foundation/e/drive/synchronization/SyncWorker.kt @@ -80,6 +80,8 @@ class SyncWorker( notificationManager.cancel(NOTIFICATION_ID) syncManager.startListeningFiles(applicationContext as EdriveApplication) + + DavClientProvider.getInstance().saveAccounts(applicationContext) } catch (exception: Exception) { Timber.w(exception) } diff --git a/app/src/main/java/foundation/e/drive/synchronization/tasks/UploadFileOperation.java b/app/src/main/java/foundation/e/drive/synchronization/tasks/UploadFileOperation.java index e141198c..10b79d59 100644 --- a/app/src/main/java/foundation/e/drive/synchronization/tasks/UploadFileOperation.java +++ b/app/src/main/java/foundation/e/drive/synchronization/tasks/UploadFileOperation.java @@ -165,6 +165,8 @@ public class UploadFileOperation extends RemoteOperation { } final RemoteOperationResult checkQuotaResult = checkAvailableSpace(ncClient, file.length()); + DavClientProvider.getInstance().saveAccounts(context); + if (checkQuotaResult.getCode() != ResultCode.OK) { Timber.d("Impossible to check quota. Cancels upload of %s", syncedState.getLocalPath()); return checkQuotaResult.getCode(); @@ -264,7 +266,7 @@ public class UploadFileOperation extends RemoteOperation { @VisibleForTesting() @NonNull public RemoteOperationResult uploadChunkedFile(@NonNull final File file, @NonNull final OwnCloudClient client) { - final String timeStamp = formatTimestampToMatchCloud(file.lastModified()); + final long timeStamp = file.lastModified()/1000; final String mimeType = getMimeType(file); final ChunkedFileUploadRemoteOperation uploadOperation = new ChunkedFileUploadRemoteOperation(syncedState.getLocalPath(), syncedState.getRemotePath(), @@ -321,7 +323,7 @@ public class UploadFileOperation extends RemoteOperation { @VisibleForTesting() @NonNull public RemoteOperationResult uploadFile(@NonNull final File file, @NonNull final OwnCloudClient client, boolean checkEtag) { - final String timeStamp = formatTimestampToMatchCloud(file.lastModified()); + final long timeStamp = file.lastModified()/1000; final String eTag = checkEtag ? syncedState.getLastEtag() : null; final UploadFileRemoteOperation uploadOperation = new UploadFileRemoteOperation(syncedState.getLocalPath(), diff --git a/app/src/main/java/foundation/e/drive/utils/DavClientProvider.java b/app/src/main/java/foundation/e/drive/utils/DavClientProvider.java index fc8ce000..47e0226b 100644 --- a/app/src/main/java/foundation/e/drive/utils/DavClientProvider.java +++ b/app/src/main/java/foundation/e/drive/utils/DavClientProvider.java @@ -11,124 +11,91 @@ package foundation.e.drive.utils; import android.accounts.Account; -import android.accounts.AccountManager; +import android.accounts.AuthenticatorException; +import android.accounts.OperationCanceledException; import android.content.Context; -import android.net.Uri; import androidx.annotation.NonNull; import androidx.annotation.Nullable; import com.nextcloud.common.NextcloudClient; -import com.nextcloud.common.OkHttpCredentialsUtil; -import com.owncloud.android.lib.common.OwnCloudBasicCredentials; +import com.owncloud.android.lib.common.OwnCloudAccount; import com.owncloud.android.lib.common.OwnCloudClient; -import com.owncloud.android.lib.common.OwnCloudClientFactory; +import com.owncloud.android.lib.common.OwnCloudClientManager; import com.owncloud.android.lib.common.OwnCloudClientManagerFactory; import com.owncloud.android.lib.common.accounts.AccountUtils; +import java.io.IOException; + +import foundation.e.drive.R; import timber.log.Timber; /** * @author Vincent Bourgmayer */ -@SuppressWarnings({"deprecation", "DeprecatedIsStillUsed"}) +@SuppressWarnings({"deprecation"}) public class DavClientProvider { private final static DavClientProvider instance = new DavClientProvider(); private DavClientProvider() { - Timber.tag(DavClientProvider.class.getSimpleName()); } - @Deprecated - private OwnCloudClient ocClientInstance; - private NextcloudClient ncClientInstance; - @Nullable public OwnCloudClient getClientInstance(@Nullable final Account account, @NonNull final Context context) { - if (account == null) return null; - - if (ocClientInstance == null) { - ocClientInstance = createOcClient(account, context); + if (account == null) { + return null; } - return ocClientInstance; - } - - - @Nullable - public NextcloudClient getNcClientInstance(@Nullable final Account account, @NonNull final Context context) { - if (account == null) return null; + OwnCloudClientManagerFactory.setUserAgent(AppConstants.USER_AGENT); - if (ncClientInstance == null) { - ncClientInstance = createNcClient(account, context); + try { + OwnCloudClientManager ownCloudClientManager = OwnCloudClientManagerFactory.getDefaultSingleton(); + OwnCloudAccount ocAccount = new OwnCloudAccount(account, context); + return ownCloudClientManager.getClientFor(ocAccount, context); + } catch (AccountUtils.AccountNotFoundException | OperationCanceledException | + AuthenticatorException | IOException exception) { + Timber.e(exception); + return null; } - return ncClientInstance; - } - - public void cleanUp() { - ncClientInstance = null; - ocClientInstance = null; - } - - @NonNull - public static DavClientProvider getInstance() { - return instance; } @Nullable - private OwnCloudBasicCredentials getOcCredentials(@NonNull Account account, @NonNull Context context) throws AccountUtils.AccountNotFoundException { - final String pwd = AccountManager.get(context).getPassword(account); - if (pwd == null) return null; - return new OwnCloudBasicCredentials(account.name, pwd); - } + public NextcloudClient getNcClientInstance(@Nullable final Account account, @NonNull final Context context) { + if (account == null) { + return null; + } - @Nullable - private OwnCloudClient createOcClient(@NonNull Account account, @NonNull Context context) { OwnCloudClientManagerFactory.setUserAgent(AppConstants.USER_AGENT); - final OwnCloudClient result; - try { - final OwnCloudBasicCredentials credentials = getOcCredentials(account, context); - if (credentials == null) return null; - final Uri serverUri = Uri.parse(AccountUtils.getBaseUrlForAccount(context, account)); - result = OwnCloudClientFactory.createOwnCloudClient(serverUri, context, true); - result.setCredentials(credentials); - } catch (AccountUtils.AccountNotFoundException exception) { + try { + OwnCloudClientManager ownCloudClientManager = OwnCloudClientManagerFactory.getDefaultSingleton(); + OwnCloudAccount ocAccount = new OwnCloudAccount(account, context); + return ownCloudClientManager.getNextcloudClientFor(ocAccount, context); + } catch (AccountUtils.AccountNotFoundException | OperationCanceledException | + AuthenticatorException | IOException exception) { Timber.e(exception); return null; } - if (result.getUserId() == null) { - final String userId = AccountManager.get(context).getUserData(account, AppConstants.ACCOUNT_USER_ID_KEY); - result.setUserId(userId); - } - - return result; } - - @Nullable - private String getNcCredentials(@NonNull Account account, @NonNull Context context) throws AccountUtils.AccountNotFoundException { - final String pwd = AccountManager.get(context).getPassword(account); - if (pwd == null) return null; - return OkHttpCredentialsUtil.basic(account.name, pwd); + public void cleanUp(@NonNull Context context, @NonNull Account account) { + OwnCloudClientManager ownCloudClientManager = OwnCloudClientManagerFactory.getDefaultSingleton(); + try { + OwnCloudAccount ocAccount = new OwnCloudAccount(account, context); + ownCloudClientManager.removeClientFor(ocAccount); + } catch (AccountUtils.AccountNotFoundException e) { + Timber.e(e); + } } - @Nullable - private NextcloudClient createNcClient(@NonNull Account account, @NonNull Context context) { - final NextcloudClient result; - try { - final String credentials = getNcCredentials(account, context); - if (credentials == null) return null; + public void saveAccounts(@NonNull Context context) { + OwnCloudClientManagerFactory.getDefaultSingleton().saveAllClients(context, context.getString(R.string.eelo_account_type)); + } - final Uri serverUri = Uri.parse(AccountUtils.getBaseUrlForAccount(context, account)); - result = OwnCloudClientFactory.createNextcloudClient(serverUri, account.name, credentials, context, true); - } catch (AccountUtils.AccountNotFoundException exception) { - Timber.e("Can't get server URI for account: %s\n%s", account.name, exception.getMessage()); - return null; - } - result.setUserId(account.name); - return result; + @NonNull + public static DavClientProvider getInstance() { + return instance; } -} \ No newline at end of file +} diff --git a/app/src/main/java/foundation/e/drive/utils/WorkerUtils.kt b/app/src/main/java/foundation/e/drive/utils/WorkerUtils.kt index 84a48585..eff2c404 100644 --- a/app/src/main/java/foundation/e/drive/utils/WorkerUtils.kt +++ b/app/src/main/java/foundation/e/drive/utils/WorkerUtils.kt @@ -16,8 +16,6 @@ import foundation.e.drive.work.WorkRequestFactory object WorkerUtils { fun registerSetupWorkers(context: Context) { - DavClientProvider.getInstance().cleanUp() - val rootFolderSetupWorkers = generateRootFolderSetupWorkers(context) if (rootFolderSetupWorkers.isEmpty()) { return -- GitLab From 61ac15ddec216e3fef6a88f9c8b97d2bbb04b6a2 Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Wed, 14 Feb 2024 18:52:13 +0600 Subject: [PATCH 2/7] fix: race condition on updating cookie - as the rootFolderSetup calls are executing concurrently, the cookie update sometime faces race-codition. As a result some requests are facing 401. We have to execute the request one after one to prevent this. - simplify the davClient cleanup implementation --- app/build.gradle | 2 +- .../account/receivers/AccountAddedReceiver.kt | 2 ++ .../AccountRemoveCallbackReceiver.java | 23 ++----------------- .../e/drive/utils/DavClientProvider.java | 9 ++------ .../foundation/e/drive/utils/WorkerUtils.kt | 11 ++++++--- 5 files changed, 15 insertions(+), 32 deletions(-) diff --git a/app/build.gradle b/app/build.gradle index fde6fff4..faedbf84 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -94,7 +94,7 @@ android { } dependencies { - implementation 'foundation.e:Nextcloud-Android-Library:1.0.1-alpha' + implementation 'foundation.e:Nextcloud-Android-Library:1.0.5-alpha' implementation "commons-httpclient:commons-httpclient:3.1@jar" implementation fileTree(include: ['*.jar'], dir: 'libs') api 'androidx.annotation:annotation:1.6.0' diff --git a/app/src/main/java/foundation/e/drive/account/receivers/AccountAddedReceiver.kt b/app/src/main/java/foundation/e/drive/account/receivers/AccountAddedReceiver.kt index 704a780e..0459bf3b 100644 --- a/app/src/main/java/foundation/e/drive/account/receivers/AccountAddedReceiver.kt +++ b/app/src/main/java/foundation/e/drive/account/receivers/AccountAddedReceiver.kt @@ -16,6 +16,7 @@ import foundation.e.drive.R import foundation.e.drive.utils.AccountUtils import foundation.e.drive.utils.AppConstants import foundation.e.drive.utils.CommonUtils +import foundation.e.drive.utils.DavClientProvider import foundation.e.drive.utils.WorkerUtils import timber.log.Timber @@ -44,6 +45,7 @@ class AccountAddedReceiver() : BroadcastReceiver() { .putString(AccountManager.KEY_ACCOUNT_NAME, accountName) .apply() + DavClientProvider.getInstance().cleanUp(); WorkerUtils.registerSetupWorkers(context) } diff --git a/app/src/main/java/foundation/e/drive/account/receivers/AccountRemoveCallbackReceiver.java b/app/src/main/java/foundation/e/drive/account/receivers/AccountRemoveCallbackReceiver.java index 89a8c846..5d58377d 100644 --- a/app/src/main/java/foundation/e/drive/account/receivers/AccountRemoveCallbackReceiver.java +++ b/app/src/main/java/foundation/e/drive/account/receivers/AccountRemoveCallbackReceiver.java @@ -11,7 +11,6 @@ package foundation.e.drive.account.receivers; import static foundation.e.drive.utils.AppConstants.INITIAL_FOLDER_NUMBER; import static foundation.e.drive.utils.AppConstants.SETUP_COMPLETED; -import android.accounts.Account; import android.accounts.AccountManager; import android.annotation.SuppressLint; import android.app.NotificationManager; @@ -30,7 +29,6 @@ import foundation.e.drive.EdriveApplication; import foundation.e.drive.R; import foundation.e.drive.database.DbHelper; import foundation.e.drive.database.FailedSyncPrefsManager; -import foundation.e.drive.synchronization.SyncWorker; import foundation.e.drive.utils.AppConstants; import foundation.e.drive.utils.DavClientProvider; import foundation.e.drive.utils.ViewUtils; @@ -57,7 +55,8 @@ public class AccountRemoveCallbackReceiver extends BroadcastReceiver { cleanSharedPreferences(applicationContext, preferences); removeCachedFiles(applicationContext); deleteNotificationChannels(applicationContext); - clearDavClient(intent, applicationContext); + + DavClientProvider.getInstance().cleanUp(); ViewUtils.updateWidgetView(applicationContext); } @@ -79,24 +78,6 @@ public class AccountRemoveCallbackReceiver extends BroadcastReceiver { } } - private void clearDavClient(@NonNull Intent intent, @NonNull Context context) { - if (intent.getExtras() == null) { - return; - } - - final String accountType = intent.getExtras().getString(AccountManager.KEY_ACCOUNT_TYPE); - final String accountName = intent.getExtras().getString(AccountManager.KEY_ACCOUNT_NAME); - - AccountManager accountManager = AccountManager.get(context); - - for (Account account : accountManager.getAccountsByType(accountType)) { - if (account.name.equals(accountName)) { - DavClientProvider.getInstance().cleanUp(context, account); - break; - } - } - } - private boolean shouldProceedWithRemoval(@NonNull Intent intent, @NonNull SharedPreferences preferences, @NonNull Context context) { if (isInvalidAction(intent) || intent.getExtras() == null) { Timber.w("Invalid account removal request"); diff --git a/app/src/main/java/foundation/e/drive/utils/DavClientProvider.java b/app/src/main/java/foundation/e/drive/utils/DavClientProvider.java index 47e0226b..ade32429 100644 --- a/app/src/main/java/foundation/e/drive/utils/DavClientProvider.java +++ b/app/src/main/java/foundation/e/drive/utils/DavClientProvider.java @@ -80,14 +80,9 @@ public class DavClientProvider { } - public void cleanUp(@NonNull Context context, @NonNull Account account) { + public void cleanUp() { OwnCloudClientManager ownCloudClientManager = OwnCloudClientManagerFactory.getDefaultSingleton(); - try { - OwnCloudAccount ocAccount = new OwnCloudAccount(account, context); - ownCloudClientManager.removeClientFor(ocAccount); - } catch (AccountUtils.AccountNotFoundException e) { - Timber.e(e); - } + ownCloudClientManager.removeClientForByName(null); } public void saveAccounts(@NonNull Context context) { diff --git a/app/src/main/java/foundation/e/drive/utils/WorkerUtils.kt b/app/src/main/java/foundation/e/drive/utils/WorkerUtils.kt index eff2c404..8dcd6c59 100644 --- a/app/src/main/java/foundation/e/drive/utils/WorkerUtils.kt +++ b/app/src/main/java/foundation/e/drive/utils/WorkerUtils.kt @@ -15,7 +15,7 @@ import foundation.e.drive.models.SyncedFolder import foundation.e.drive.work.WorkRequestFactory object WorkerUtils { - fun registerSetupWorkers(context: Context) { + fun registerSetupWorkers(context: Context) { val rootFolderSetupWorkers = generateRootFolderSetupWorkers(context) if (rootFolderSetupWorkers.isEmpty()) { return @@ -30,9 +30,14 @@ object WorkerUtils { null ) - WorkManager.getInstance(context) + var workContinuation = WorkManager.getInstance(context) .beginWith(getUserInfoRequest) - .then(rootFolderSetupWorkers) + + rootFolderSetupWorkers.forEach { + workContinuation = workContinuation.then(it) + } + + workContinuation .then(finishSetupRequest) .enqueue() } -- GitLab From f1c78b3b5d4e2cf3f706ea31b22305d1e85116f8 Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Thu, 15 Feb 2024 12:15:27 +0600 Subject: [PATCH 3/7] fix: stateMachine is not updating after account logout After user logged out from the account, stateMachine doesn't move to idle state, but the jobs are canceled. Which create some issue until user reboot the device. example, if the state IS `PERIODIC_SCAN`, & account logged-out, FullScanWorker will be stopped, but state won't update. So if user re-login, the FullScannWorker won't run again, until user reboot device. --- .../AccountRemoveCallbackReceiver.java | 11 +++------- .../e/drive/synchronization/StateMachine.kt | 9 +++++++-- .../e/drive/synchronization/SyncProxy.kt | 20 +++++++++++++++++++ 3 files changed, 30 insertions(+), 10 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/account/receivers/AccountRemoveCallbackReceiver.java b/app/src/main/java/foundation/e/drive/account/receivers/AccountRemoveCallbackReceiver.java index 5d58377d..4ff1ed1e 100644 --- a/app/src/main/java/foundation/e/drive/account/receivers/AccountRemoveCallbackReceiver.java +++ b/app/src/main/java/foundation/e/drive/account/receivers/AccountRemoveCallbackReceiver.java @@ -13,6 +13,7 @@ import static foundation.e.drive.utils.AppConstants.SETUP_COMPLETED; import android.accounts.AccountManager; import android.annotation.SuppressLint; +import android.app.Application; import android.app.NotificationManager; import android.content.BroadcastReceiver; import android.content.Context; @@ -25,10 +26,10 @@ import androidx.work.WorkManager; import java.io.File; -import foundation.e.drive.EdriveApplication; import foundation.e.drive.R; import foundation.e.drive.database.DbHelper; import foundation.e.drive.database.FailedSyncPrefsManager; +import foundation.e.drive.synchronization.SyncProxy; import foundation.e.drive.utils.AppConstants; import foundation.e.drive.utils.DavClientProvider; import foundation.e.drive.utils.ViewUtils; @@ -50,7 +51,7 @@ public class AccountRemoveCallbackReceiver extends BroadcastReceiver { } cancelWorkers(applicationContext); - stopRecursiveFileObserver(applicationContext); + SyncProxy.INSTANCE.moveToIdle((Application) applicationContext); deleteDatabase(applicationContext); cleanSharedPreferences(applicationContext, preferences); removeCachedFiles(applicationContext); @@ -72,12 +73,6 @@ public class AccountRemoveCallbackReceiver extends BroadcastReceiver { Timber.d("Remove Database: %s", result); } - private void stopRecursiveFileObserver(@NonNull Context applicationContext) { - if (applicationContext instanceof EdriveApplication) { - ((EdriveApplication) applicationContext).stopRecursiveFileObserver(); - } - } - private boolean shouldProceedWithRemoval(@NonNull Intent intent, @NonNull SharedPreferences preferences, @NonNull Context context) { if (isInvalidAction(intent) || intent.getExtras() == null) { Timber.w("Invalid account removal request"); 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 7be44b40..90f923b9 100644 --- a/app/src/main/java/foundation/e/drive/synchronization/StateMachine.kt +++ b/app/src/main/java/foundation/e/drive/synchronization/StateMachine.kt @@ -22,7 +22,7 @@ enum class SyncState { * 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) + * It can be Idle (i.e: after boot, before restart, after logout) * @author Vincent Bourgmayer */ object StateMachine { @@ -41,7 +41,7 @@ object StateMachine { SyncState.PERIODIC_SCAN -> setPeriodicScanState() SyncState.SYNCHRONIZING -> setSynchronizing() SyncState.LISTENING_FILES -> setListeningFilesState() - SyncState.IDLE -> false + SyncState.IDLE -> setIdleFilesState() } if (!isStateChanged) { @@ -59,6 +59,11 @@ object StateMachine { return true } + private fun setIdleFilesState(): Boolean { + currentState = SyncState.IDLE + return true + } + private fun setPeriodicScanState(): Boolean { if (currentState == SyncState.SYNCHRONIZING) { Timber.d("Cannot change state: files sync is running") 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 ef829bda..0befa246 100644 --- a/app/src/main/java/foundation/e/drive/synchronization/SyncProxy.kt +++ b/app/src/main/java/foundation/e/drive/synchronization/SyncProxy.kt @@ -31,6 +31,7 @@ interface SyncRequestCollector { fun startSynchronization(context: Context) fun onPeriodicScanStart(application: Application): Boolean fun startListeningFiles(application: Application) + fun moveToIdle(application: Application) } /** @@ -180,6 +181,25 @@ object SyncProxy: SyncRequestCollector, SyncManager { } } + /** + * called after account logged out + * update the stateMachine's state & stop recursive fileObserver + */ + override fun moveToIdle(application: Application) { + if (application !is EdriveApplication) { + Timber.d("Invalid parameter: : moveToIdle(application)") + return + } + + val isStateChanged = StateMachine.changeState(SyncState.IDLE) + if (!isStateChanged) { + Timber.d("failed to change state. moveToIdle") + return + } + + application.stopRecursiveFileObserver() + } + /** * Progressively delay synchronization of a file in case of failure -- GitLab From 1a30a034cffa711d64335da9b44b1196659976d7 Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Thu, 15 Feb 2024 12:53:17 +0600 Subject: [PATCH 4/7] chore: update nc-android-lib version --- app/build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/build.gradle b/app/build.gradle index faedbf84..9ed8b30b 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -94,7 +94,7 @@ android { } dependencies { - implementation 'foundation.e:Nextcloud-Android-Library:1.0.5-alpha' + implementation 'foundation.e:Nextcloud-Android-Library:1.0.5-release' implementation "commons-httpclient:commons-httpclient:3.1@jar" implementation fileTree(include: ['*.jar'], dir: 'libs') api 'androidx.annotation:annotation:1.6.0' -- GitLab From 6b66c49d3dcd3e727a63b233183ed139e6f5d6d2 Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Thu, 15 Feb 2024 12:59:31 +0600 Subject: [PATCH 5/7] fix: StateMchineTests are failing for the latest state logic change --- .../drive/synchronization/StateMachineTest.kt | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 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 c1674fca..ed7b47d7 100644 --- a/app/src/test/java/foundation/e/drive/synchronization/StateMachineTest.kt +++ b/app/src/test/java/foundation/e/drive/synchronization/StateMachineTest.kt @@ -50,14 +50,14 @@ class StateMachineTest { } @Test - fun `Changing State from PERIODIC_SCAN to IDLE should return false`() { + fun `Changing State from PERIODIC_SCAN to IDLE should return true`() { StateMachine.currentState = PERIODIC_SCAN val result = StateMachine.changeState(IDLE) - Assert.assertFalse("Changing state from PERIODIC_SCAN to IDLE returned false", result) + Assert.assertTrue("Changing state from PERIODIC_SCAN to IDLE returned true", result) val currentState = StateMachine.currentState - Assert.assertEquals("Current state should remain PERIODIC_SCAN but is $currentState", PERIODIC_SCAN, currentState) + Assert.assertEquals("Current state should be but is $currentState", IDLE, currentState) } @Test @@ -83,14 +83,14 @@ class StateMachineTest { } @Test - fun `Changing State from LISTENING_FILE to IDLE should return false`() { + fun `Changing State from LISTENING_FILE to IDLE should return true`() { StateMachine.currentState = LISTENING_FILES val result = StateMachine.changeState(IDLE) - Assert.assertFalse("Changing state from LISTENING_FILES to IDLE returned true", result) + Assert.assertTrue("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) + Assert.assertEquals("Current state should be IDLE but is $currentState", IDLE, currentState) } @Test @@ -138,14 +138,14 @@ class StateMachineTest { } @Test - fun `Changing State from SYNCHRONIZING to IDLE should return false`() { + fun `Changing State from SYNCHRONIZING to IDLE should return true`() { StateMachine.currentState = SYNCHRONIZING val result = StateMachine.changeState(IDLE) - Assert.assertFalse("Changing state from SYNCHRONIZING to IDLE returned true", result) + Assert.assertTrue("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) + Assert.assertEquals("Current state should be IDLE but is $currentState", IDLE, currentState) } @Test -- GitLab From 671381b37dd1dd382973b39d9b6da1e849bb595d Mon Sep 17 00:00:00 2001 From: Jonathan Klee Date: Thu, 15 Feb 2024 17:30:35 +0000 Subject: [PATCH 6/7] chore: apply refactor according to review --- .../main/java/foundation/e/drive/synchronization/SyncProxy.kt | 2 +- .../e/drive/synchronization/tasks/UploadFileOperation.java | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) 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 0befa246..568e37f6 100644 --- a/app/src/main/java/foundation/e/drive/synchronization/SyncProxy.kt +++ b/app/src/main/java/foundation/e/drive/synchronization/SyncProxy.kt @@ -181,7 +181,7 @@ object SyncProxy: SyncRequestCollector, SyncManager { } } - /** + /* * called after account logged out * update the stateMachine's state & stop recursive fileObserver */ diff --git a/app/src/main/java/foundation/e/drive/synchronization/tasks/UploadFileOperation.java b/app/src/main/java/foundation/e/drive/synchronization/tasks/UploadFileOperation.java index 10b79d59..5f5ec859 100644 --- a/app/src/main/java/foundation/e/drive/synchronization/tasks/UploadFileOperation.java +++ b/app/src/main/java/foundation/e/drive/synchronization/tasks/UploadFileOperation.java @@ -266,7 +266,7 @@ public class UploadFileOperation extends RemoteOperation { @VisibleForTesting() @NonNull public RemoteOperationResult uploadChunkedFile(@NonNull final File file, @NonNull final OwnCloudClient client) { - final long timeStamp = file.lastModified()/1000; + final long timeStamp = file.lastModified() / 1000; final String mimeType = getMimeType(file); final ChunkedFileUploadRemoteOperation uploadOperation = new ChunkedFileUploadRemoteOperation(syncedState.getLocalPath(), syncedState.getRemotePath(), @@ -323,7 +323,7 @@ public class UploadFileOperation extends RemoteOperation { @VisibleForTesting() @NonNull public RemoteOperationResult uploadFile(@NonNull final File file, @NonNull final OwnCloudClient client, boolean checkEtag) { - final long timeStamp = file.lastModified()/1000; + final long timeStamp = file.lastModified() / 1000; final String eTag = checkEtag ? syncedState.getLastEtag() : null; final UploadFileRemoteOperation uploadOperation = new UploadFileRemoteOperation(syncedState.getLocalPath(), -- GitLab From 25dfbcb5a3e7843f84d65d0529045fa834c249ed Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Tue, 20 Feb 2024 09:11:05 +0000 Subject: [PATCH 7/7] chore: refactor according to review --- .../foundation/e/drive/utils/DavClientProvider.java | 10 +++++----- .../e/drive/synchronization/StateMachineTest.kt | 8 ++++---- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/utils/DavClientProvider.java b/app/src/main/java/foundation/e/drive/utils/DavClientProvider.java index ade32429..842db665 100644 --- a/app/src/main/java/foundation/e/drive/utils/DavClientProvider.java +++ b/app/src/main/java/foundation/e/drive/utils/DavClientProvider.java @@ -50,8 +50,8 @@ public class DavClientProvider { OwnCloudClientManagerFactory.setUserAgent(AppConstants.USER_AGENT); try { - OwnCloudClientManager ownCloudClientManager = OwnCloudClientManagerFactory.getDefaultSingleton(); - OwnCloudAccount ocAccount = new OwnCloudAccount(account, context); + final OwnCloudClientManager ownCloudClientManager = OwnCloudClientManagerFactory.getDefaultSingleton(); + final OwnCloudAccount ocAccount = new OwnCloudAccount(account, context); return ownCloudClientManager.getClientFor(ocAccount, context); } catch (AccountUtils.AccountNotFoundException | OperationCanceledException | AuthenticatorException | IOException exception) { @@ -69,8 +69,8 @@ public class DavClientProvider { OwnCloudClientManagerFactory.setUserAgent(AppConstants.USER_AGENT); try { - OwnCloudClientManager ownCloudClientManager = OwnCloudClientManagerFactory.getDefaultSingleton(); - OwnCloudAccount ocAccount = new OwnCloudAccount(account, context); + final OwnCloudClientManager ownCloudClientManager = OwnCloudClientManagerFactory.getDefaultSingleton(); + final OwnCloudAccount ocAccount = new OwnCloudAccount(account, context); return ownCloudClientManager.getNextcloudClientFor(ocAccount, context); } catch (AccountUtils.AccountNotFoundException | OperationCanceledException | AuthenticatorException | IOException exception) { @@ -81,7 +81,7 @@ public class DavClientProvider { } public void cleanUp() { - OwnCloudClientManager ownCloudClientManager = OwnCloudClientManagerFactory.getDefaultSingleton(); + final OwnCloudClientManager ownCloudClientManager = OwnCloudClientManagerFactory.getDefaultSingleton(); ownCloudClientManager.removeClientForByName(null); } 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 ed7b47d7..a4a765df 100644 --- a/app/src/test/java/foundation/e/drive/synchronization/StateMachineTest.kt +++ b/app/src/test/java/foundation/e/drive/synchronization/StateMachineTest.kt @@ -54,10 +54,10 @@ class StateMachineTest { StateMachine.currentState = PERIODIC_SCAN val result = StateMachine.changeState(IDLE) - Assert.assertTrue("Changing state from PERIODIC_SCAN to IDLE returned true", result) + Assert.assertTrue("Changing state from PERIODIC_SCAN to IDLE returned false", result) val currentState = StateMachine.currentState - Assert.assertEquals("Current state should be but is $currentState", IDLE, currentState) + Assert.assertEquals("Current state should be IDLE but is $currentState", IDLE, currentState) } @Test @@ -87,7 +87,7 @@ class StateMachineTest { StateMachine.currentState = LISTENING_FILES val result = StateMachine.changeState(IDLE) - Assert.assertTrue("Changing state from LISTENING_FILES to IDLE returned true", result) + Assert.assertTrue("Changing state from LISTENING_FILES to IDLE returned false", result) val currentState = StateMachine.currentState Assert.assertEquals("Current state should be IDLE but is $currentState", IDLE, currentState) @@ -142,7 +142,7 @@ class StateMachineTest { StateMachine.currentState = SYNCHRONIZING val result = StateMachine.changeState(IDLE) - Assert.assertTrue("Changing state from SYNCHRONIZING to IDLE returned true", result) + Assert.assertTrue("Changing state from SYNCHRONIZING to IDLE returned false", result) val currentState = StateMachine.currentState Assert.assertEquals("Current state should be IDLE but is $currentState", IDLE, currentState) -- GitLab