diff --git a/app/src/main/java/com/nextcloud/client/account/UserAccountManager.java b/app/src/main/java/com/nextcloud/client/account/UserAccountManager.java index 3317e180192d..89fb27b2e1f5 100644 --- a/app/src/main/java/com/nextcloud/client/account/UserAccountManager.java +++ b/app/src/main/java/com/nextcloud/client/account/UserAccountManager.java @@ -9,6 +9,7 @@ import android.accounts.Account; import android.accounts.AccountManager; import android.app.Activity; +import android.content.Context; import android.content.Intent; import com.owncloud.android.MainApp; @@ -30,6 +31,9 @@ public interface UserAccountManager extends CurrentAccountProvider { String ACCOUNT_USES_STANDARD_PASSWORD = "ACCOUNT_USES_STANDARD_PASSWORD"; String PENDING_FOR_REMOVAL = "PENDING_FOR_REMOVAL"; + @Nullable + Context getContext(); + @Nullable OwnCloudAccount getCurrentOwnCloudAccount(); diff --git a/app/src/main/java/com/nextcloud/client/account/UserAccountManagerImpl.java b/app/src/main/java/com/nextcloud/client/account/UserAccountManagerImpl.java index 3aec76281df3..8b941d9f70e6 100644 --- a/app/src/main/java/com/nextcloud/client/account/UserAccountManagerImpl.java +++ b/app/src/main/java/com/nextcloud/client/account/UserAccountManagerImpl.java @@ -55,7 +55,7 @@ public class UserAccountManagerImpl implements UserAccountManager { private static final String TAG = UserAccountManagerImpl.class.getSimpleName(); private static final String PREF_SELECT_OC_ACCOUNT = "select_oc_account"; - private Context context; + private final Context context; private final AccountManager accountManager; public static UserAccountManagerImpl fromContext(Context context) { @@ -305,6 +305,12 @@ public User getAnonymousUser() { return AnonymousUser.fromContext(context); } + @Nullable + @Override + public Context getContext() { + return context; + } + @Override @Nullable public OwnCloudAccount getCurrentOwnCloudAccount() { diff --git a/app/src/main/java/com/nextcloud/client/di/DispatcherModule.kt b/app/src/main/java/com/nextcloud/client/di/DispatcherModule.kt index ab15cdf6e541..8812ffbe428f 100644 --- a/app/src/main/java/com/nextcloud/client/di/DispatcherModule.kt +++ b/app/src/main/java/com/nextcloud/client/di/DispatcherModule.kt @@ -1,17 +1,23 @@ /* * Nextcloud - Android Client * + * SPDX-FileCopyrightText: 2026 Alper Ozturk * SPDX-FileCopyrightText: 2022 Álvaro Brey * SPDX-FileCopyrightText: 2022 Nextcloud GmbH * SPDX-License-Identifier: AGPL-3.0-or-later OR GPL-2.0-only */ package com.nextcloud.client.di +import com.owncloud.android.lib.common.utils.Log_OC import dagger.Module import dagger.Provides import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.CoroutineExceptionHandler +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.SupervisorJob import javax.inject.Qualifier +import javax.inject.Singleton @Retention(AnnotationRetention.BINARY) @Qualifier @@ -25,8 +31,15 @@ annotation class IoDispatcher @Qualifier annotation class MainDispatcher +@Retention(AnnotationRetention.BINARY) +@Qualifier +annotation class ApplicationScope + @Module object DispatcherModule { + + private const val APPLICATION_SCOPE_TAG = "ApplicationScope" + @DefaultDispatcher @Provides fun provideDefaultDispatcher(): CoroutineDispatcher = Dispatchers.Default @@ -38,4 +51,18 @@ object DispatcherModule { @MainDispatcher @Provides fun provideMainDispatcher(): CoroutineDispatcher = Dispatchers.Main + + /** + * A process-lifetime [CoroutineScope] for singletons that outlive any single Android component. + */ + @ApplicationScope + @Provides + @Singleton + fun provideApplicationScope(@IoDispatcher dispatcher: CoroutineDispatcher): CoroutineScope = CoroutineScope( + SupervisorJob() + + dispatcher + + CoroutineExceptionHandler { _, throwable -> + Log_OC.e(APPLICATION_SCOPE_TAG, "Uncaught exception in application coroutine scope", throwable) + } + ) } diff --git a/app/src/main/java/com/nextcloud/client/jobs/upload/FileUploadHelper.kt b/app/src/main/java/com/nextcloud/client/jobs/upload/FileUploadHelper.kt index 816c81898b24..714a7d0eb382 100644 --- a/app/src/main/java/com/nextcloud/client/jobs/upload/FileUploadHelper.kt +++ b/app/src/main/java/com/nextcloud/client/jobs/upload/FileUploadHelper.kt @@ -19,13 +19,14 @@ import com.nextcloud.client.database.entity.toOCUpload import com.nextcloud.client.database.entity.toUploadEntity import com.nextcloud.client.device.BatteryStatus import com.nextcloud.client.device.PowerManagementService +import com.nextcloud.client.di.ApplicationScope import com.nextcloud.client.jobs.BackgroundJobManager import com.nextcloud.client.network.Connectivity import com.nextcloud.client.network.ConnectivityService import com.nextcloud.client.notifications.AppWideNotificationManager import com.nextcloud.utils.extensions.checkWCFRestrictions +import com.nextcloud.utils.extensions.createOwncloudClient import com.nextcloud.utils.extensions.getUploadIds -import com.nextcloud.utils.extensions.isAnonymous import com.nextcloud.utils.extensions.isLastResultConflictError import com.nextcloud.utils.extensions.isSame import com.owncloud.android.MainApp @@ -38,7 +39,6 @@ import com.owncloud.android.db.OCUpload import com.owncloud.android.db.UploadResult import com.owncloud.android.files.services.NameCollisionPolicy import com.owncloud.android.lib.common.OwnCloudClient -import com.owncloud.android.lib.common.OwnCloudClientFactory import com.owncloud.android.lib.common.network.OnDatatransferProgressListener import com.owncloud.android.lib.common.operations.RemoteOperationResult import com.owncloud.android.lib.common.utils.Log_OC @@ -56,7 +56,7 @@ import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch import kotlinx.coroutines.withContext import java.io.File -import java.util.concurrent.Semaphore +import java.util.concurrent.atomic.AtomicBoolean import javax.inject.Inject @Suppress("TooManyFunctions") @@ -74,12 +74,16 @@ class FileUploadHelper { @Inject lateinit var fileStorageManager: FileDataStorageManager - private val ioScope = CoroutineScope(Dispatchers.IO) + @Inject + @ApplicationScope + lateinit var appScope: CoroutineScope init { MainApp.getAppComponent().inject(this) } + private val uploadActionHandler = UploadListAdapterActionHandler() + companion object { private val TAG = FileUploadWorker::class.java.simpleName @@ -87,13 +91,11 @@ class FileUploadHelper { val mBoundListeners = HashMap() - private var instance: FileUploadHelper? = null + private val retryInProgress = AtomicBoolean(false) - private val retryFailedUploadsSemaphore = Semaphore(1) + private val sharedInstance: FileUploadHelper by lazy { FileUploadHelper() } - fun instance(): FileUploadHelper = instance ?: synchronized(this) { - instance ?: FileUploadHelper().also { instance = it } - } + fun instance(): FileUploadHelper = sharedInstance fun buildRemoteName(accountName: String, remotePath: String): String = accountName + remotePath } @@ -122,21 +124,17 @@ class FileUploadHelper { connectivityService: ConnectivityService, accountManager: UserAccountManager, powerManagementService: PowerManagementService - ): Boolean { - if (!retryFailedUploadsSemaphore.tryAcquire()) { + ) { + if (!retryInProgress.compareAndSet(false, true)) { Log_OC.d(TAG, "skipping retryFailedUploads, already running") - return true + return } - var isUploadStarted = false val capability = fileStorageManager.getCapability(accountManager.user) - try { - ioScope.launch { + appScope.launch { + try { val uploads = getUploadsByStatus(null, UploadStatus.UPLOAD_FAILED, capability) - if (uploads.isNotEmpty()) { - isUploadStarted = true - } retryUploads( uploadsStorageManager, @@ -145,12 +143,12 @@ class FileUploadHelper { powerManagementService, uploads ) + } finally { + // Reset only after retry processing has completely finished so the guard covers + // coroutine execution, not just its launch. This keeps a single retry running at a time. + retryInProgress.set(false) } - } finally { - retryFailedUploadsSemaphore.release() } - - return isUploadStarted } suspend fun retryCancelledUploads( @@ -185,21 +183,13 @@ class FileUploadHelper { val batteryStatus = powerManagementService.battery val uploadsToRetry = mutableListOf() - - val currentAccount = accountManager.currentAccount - val context = MainApp.getAppContext() - var ownCloudClient: OwnCloudClient? = null - if (!currentAccount.isAnonymous(context)) { - ownCloudClient = - OwnCloudClientFactory.createOwnCloudClient(accountManager.currentAccount, MainApp.getAppContext()) - } - val uploadActionHandler = UploadListAdapterActionHandler() + val client = accountManager.createOwncloudClient() for (upload in uploads) { if (upload.isLastResultConflictError()) { - ownCloudClient?.let { + client?.let { conflictHandlingResult = - uploadActionHandler.handleConflict(upload, ownCloudClient, uploadsStorageManager) + uploadActionHandler.handleConflict(upload, client = it, uploadsStorageManager) } continue } @@ -214,7 +204,7 @@ class FileUploadHelper { if (uploadResult != UploadResult.UPLOADED) { if (upload.lastResult != uploadResult) { - // Setting Upload status else cancelled uploads will behave wrong, when retrying + // Setting Upload status else canceled uploads will behave wrong, when retrying // Needs to happen first since lastResult wil be overwritten by setter upload.uploadStatus = UploadStatus.UPLOAD_FAILED @@ -345,7 +335,7 @@ class FileUploadHelper { status: UploadStatus, onCompleted: () -> Unit = {} ) { - ioScope.launch { + appScope.launch { uploadsStorageManager.uploadDao.updateStatus(remotePath, accountName, status.value) onCompleted() } @@ -531,7 +521,7 @@ class FileUploadHelper { * @param user Needed for creating client */ fun removeDuplicatedFile(duplicatedFile: OCFile, client: OwnCloudClient, user: User, onCompleted: () -> Unit) { - ioScope.launch { + appScope.launch { val removeFileOperation = RemoveFileOperation( duplicatedFile, false, diff --git a/app/src/main/java/com/nextcloud/utils/extensions/UserAccountManagerExtensions.kt b/app/src/main/java/com/nextcloud/utils/extensions/UserAccountManagerExtensions.kt new file mode 100644 index 000000000000..51cffc0a7de2 --- /dev/null +++ b/app/src/main/java/com/nextcloud/utils/extensions/UserAccountManagerExtensions.kt @@ -0,0 +1,46 @@ +/* + * Nextcloud - Android Client + * + * SPDX-FileCopyrightText: 2026 Alper Ozturk + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +package com.nextcloud.utils.extensions + +import com.nextcloud.client.account.UserAccountManager +import com.owncloud.android.MainApp +import com.owncloud.android.lib.common.OwnCloudClient +import com.owncloud.android.lib.common.OwnCloudClientFactory +import com.owncloud.android.lib.common.accounts.AccountUtils +import com.owncloud.android.lib.common.utils.Log_OC + +private const val TAG = "UserAccountManagerExtensions" + +fun UserAccountManager.createOwncloudClient(): OwnCloudClient? = createOwncloudClient(currentAccount.name) + +@Suppress("TooGenericExceptionCaught", "ReturnCount", "DEPRECATION") +fun UserAccountManager.createOwncloudClient(accountName: String): OwnCloudClient? { + val context = context ?: MainApp.getAppContext() + if (context == null) { + Log_OC.e(TAG, "app context is null, cannot create client") + return null + } + + val user = getUser(accountName).orElse(null) + if (user == null || user.isAnonymous) { + Log_OC.e(TAG, "account is not registered, cannot create client for: $accountName") + return null + } + + return try { + val result = OwnCloudClientFactory.createOwnCloudClient(user.toPlatformAccount(), context) + Log_OC.i(TAG, "client created") + result + } catch (e: AccountUtils.AccountNotFoundException) { + Log_OC.e(TAG, "account removed while creating client for: $accountName", e) + null + } catch (e: Exception) { + Log_OC.e(TAG, "cannot create client: ", e) + null + } +} diff --git a/app/src/main/java/com/owncloud/android/ui/activity/UploadListActivity.kt b/app/src/main/java/com/owncloud/android/ui/activity/UploadListActivity.kt index fc742f2ac97f..b6ef36356a6b 100755 --- a/app/src/main/java/com/owncloud/android/ui/activity/UploadListActivity.kt +++ b/app/src/main/java/com/owncloud/android/ui/activity/UploadListActivity.kt @@ -168,16 +168,12 @@ class UploadListActivity : } private fun refresh() { - val isUploadStarted = FileUploadHelper.instance().retryFailedUploads( + FileUploadHelper.instance().retryFailedUploads( uploadsStorageManager, connectivityService, accountManager, powerManagementService ) - - if (!isUploadStarted) { - uploadListAdapter.loadUploadItemsFromDb { swipeListRefreshLayout?.isRefreshing = false } - } } override fun onStart() { diff --git a/app/src/test/java/com/nextcloud/utils/extensions/UserAccountManagerExtensionsTest.kt b/app/src/test/java/com/nextcloud/utils/extensions/UserAccountManagerExtensionsTest.kt new file mode 100644 index 000000000000..336a02157f79 --- /dev/null +++ b/app/src/test/java/com/nextcloud/utils/extensions/UserAccountManagerExtensionsTest.kt @@ -0,0 +1,120 @@ +/* + * Nextcloud - Android Client + * + * SPDX-FileCopyrightText: 2026 Alper Ozturk + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +package com.nextcloud.utils.extensions + +import android.accounts.Account +import android.content.Context +import com.nextcloud.client.account.User +import com.nextcloud.client.account.UserAccountManager +import com.owncloud.android.lib.common.OwnCloudClient +import com.owncloud.android.lib.common.OwnCloudClientFactory +import com.owncloud.android.lib.common.accounts.AccountUtils +import org.junit.After +import org.junit.Assert.assertNull +import org.junit.Assert.assertSame +import org.junit.Before +import org.junit.Test +import org.mockito.MockedStatic +import org.mockito.Mockito +import org.mockito.kotlin.mock +import org.mockito.kotlin.whenever +import java.util.Optional + +class UserAccountManagerExtensionsTest { + + private val accountName = "test@server.com" + + private lateinit var accountManager: UserAccountManager + private lateinit var context: Context + private lateinit var platformAccount: Account + private lateinit var client: OwnCloudClient + private lateinit var clientFactory: MockedStatic + + @Before + fun setUp() { + context = mock() + platformAccount = mock() + client = mock() + + accountManager = mock() + whenever(accountManager.context).thenReturn(context) + + clientFactory = Mockito.mockStatic(OwnCloudClientFactory::class.java) + } + + @After + fun tearDown() { + clientFactory.close() + } + + @Test + fun `client is created for a registered account`() { + givenRegisteredAccount() + givenClientIsCreatedFor(platformAccount, context) + + assertSame(client, accountManager.createOwncloudClient(accountName)) + } + + @Test + fun `client is created for the current account when no account name is given`() { + whenever(accountManager.currentAccount).thenReturn(accountNamed(accountName)) + givenRegisteredAccount() + givenClientIsCreatedFor(platformAccount, context) + + assertSame(client, accountManager.createOwncloudClient()) + } + + @Test + fun `no client is created for an unknown account`() { + whenever(accountManager.getUser(accountName)).thenReturn(Optional.empty()) + + assertNull(accountManager.createOwncloudClient(accountName)) + + clientFactory.verifyNoInteractions() + } + + @Test + fun `no client is created while the account has no base url yet`() { + val anonymousUser = mock() + whenever(anonymousUser.isAnonymous).thenReturn(true) + whenever(accountManager.getUser(accountName)).thenReturn(Optional.of(anonymousUser)) + + assertNull(accountManager.createOwncloudClient(accountName)) + + clientFactory.verifyNoInteractions() + } + + @Test + fun `no client is created when the account is removed while the client is created`() { + givenRegisteredAccount() + clientFactory.`when` { + OwnCloudClientFactory.createOwnCloudClient(platformAccount, context) + }.thenThrow(AccountUtils.AccountNotFoundException(platformAccount, "Account not found", null)) + + assertNull(accountManager.createOwncloudClient(accountName)) + } + + private fun givenClientIsCreatedFor(account: Account, appContext: Context) { + clientFactory.`when` { + OwnCloudClientFactory.createOwnCloudClient(account, appContext) + }.thenReturn(client) + } + + // Account.name is a public final field, so it cannot be stubbed and has to be written directly + private fun accountNamed(name: String): Account = mock().also { account -> + Account::class.java.getField("name").apply { isAccessible = true }.set(account, name) + } + + @Suppress("DEPRECATION") + private fun givenRegisteredAccount() { + val user = mock() + whenever(user.isAnonymous).thenReturn(false) + whenever(user.toPlatformAccount()).thenReturn(platformAccount) + whenever(accountManager.getUser(accountName)).thenReturn(Optional.of(user)) + } +}