diff --git a/app/src/androidTest/java/com/nextcloud/client/jobs/BackgroundJobManagerTest.kt b/app/src/androidTest/java/com/nextcloud/client/jobs/BackgroundJobManagerTest.kt index d59256f786c0..025c817af319 100644 --- a/app/src/androidTest/java/com/nextcloud/client/jobs/BackgroundJobManagerTest.kt +++ b/app/src/androidTest/java/com/nextcloud/client/jobs/BackgroundJobManagerTest.kt @@ -16,10 +16,13 @@ import androidx.work.ExistingPeriodicWorkPolicy import androidx.work.ExistingWorkPolicy import androidx.work.OneTimeWorkRequest import androidx.work.PeriodicWorkRequest +import androidx.work.WorkContinuation import androidx.work.WorkInfo import androidx.work.WorkManager import com.nextcloud.client.account.User import com.nextcloud.client.core.Clock +import com.nextcloud.client.jobs.upload.FileUploadWorker +import com.nextcloud.client.preferences.AppPreferences import com.nextcloud.utils.extensions.toByteArray import com.owncloud.android.lib.common.utils.Log_OC import org.apache.commons.io.FileUtils @@ -42,6 +45,7 @@ import org.mockito.kotlin.argThat import org.mockito.kotlin.argumentCaptor import org.mockito.kotlin.eq import org.mockito.kotlin.mock +import org.mockito.kotlin.timeout import org.mockito.kotlin.verify import org.mockito.kotlin.whenever import java.io.File @@ -64,6 +68,8 @@ import java.util.concurrent.TimeoutException BackgroundJobManagerTest.PeriodicContactsBackup::class, BackgroundJobManagerTest.ImmediateContactsBackup::class, BackgroundJobManagerTest.ImmediateContactsImport::class, + BackgroundJobManagerTest.FilesUpload::class, + BackgroundJobManagerTest.FilesUploadNonBeta::class, BackgroundJobManagerTest.Tags::class ) class BackgroundJobManagerTest { @@ -90,6 +96,7 @@ class BackgroundJobManagerTest { internal lateinit var user: User internal lateinit var workManager: WorkManager internal lateinit var clock: Clock + internal lateinit var preferences: AppPreferences internal lateinit var backgroundJobManager: BackgroundJobManagerImpl internal lateinit var context: Context @@ -100,9 +107,10 @@ class BackgroundJobManagerTest { whenever(user.accountName).thenReturn(USER_ACCOUNT_NAME) workManager = mock() clock = mock() + preferences = mock() whenever(clock.currentTime).thenReturn(TIMESTAMP) whenever(clock.currentDate).thenReturn(Date(TIMESTAMP)) - backgroundJobManager = BackgroundJobManagerImpl(workManager, clock, mock()) + backgroundJobManager = BackgroundJobManagerImpl(workManager, clock, preferences, context) } fun assertHasRequiredTags(tags: Set, jobName: String, user: User? = null) { @@ -385,6 +393,103 @@ class BackgroundJobManagerTest { } } + class FilesUpload : Fixture() { + + @Before + fun setUpFilesUploadTest() { + // Mock context resources to return true for is_beta to test the parallel upload logic + val resources = mock() + whenever(resources.getBoolean(com.owncloud.android.R.bool.is_beta)).thenReturn(true) + whenever(context.resources).thenReturn(resources) + } + + @Test + fun start_files_upload_job_enqueues_batches() { + val uploadIds = longArrayOf(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11) + + val continuation: WorkContinuation = mock() + whenever( + workManager.beginUniqueWork(any(), any(), any>()) + ).thenReturn(continuation) + + backgroundJobManager.startFilesUploadJob(user, uploadIds, true) + + val tagCaptor = argumentCaptor() + val requestsCaptor = argumentCaptor>() + + verify(workManager, timeout(1000)).enqueueUniqueWork( + tagCaptor.capture(), + eq(ExistingWorkPolicy.KEEP), + requestsCaptor.capture() + ) + + val tag = tagCaptor.firstValue + assertTrue(tag.startsWith(BackgroundJobManagerImpl.JOB_FILES_UPLOAD + USER_ACCOUNT_NAME + "_")) + + val requests = requestsCaptor.firstValue + assertEquals(6, requests.size) + + // Check first batch [1, 2] + val data1 = requests[0].workSpec.input + assertEquals(true, data1.getBoolean(FileUploadWorker.SHOW_SAME_FILE_ALREADY_EXISTS_NOTIFICATION, false)) + assertEquals(USER_ACCOUNT_NAME, data1.getString(FileUploadWorker.ACCOUNT)) + assertEquals(2, data1.getInt(FileUploadWorker.TOTAL_UPLOAD_SIZE, 0)) + assertTrue(longArrayOf(1, 2).contentEquals(data1.getLongArray(FileUploadWorker.UPLOAD_IDS)!!)) + assertEquals(0, data1.getInt(FileUploadWorker.CURRENT_BATCH_INDEX, -1)) + + // Check second batch [3, 4] + val data2 = requests[1].workSpec.input + assertTrue(longArrayOf(3, 4).contentEquals(data2.getLongArray(FileUploadWorker.UPLOAD_IDS)!!)) + assertEquals(1, data2.getInt(FileUploadWorker.CURRENT_BATCH_INDEX, -1)) + + // Check third batch [5] + val data3 = requests[2].workSpec.input + assertTrue(longArrayOf(5, 6).contentEquals(data3.getLongArray(FileUploadWorker.UPLOAD_IDS)!!)) + assertEquals(2, data3.getInt(FileUploadWorker.CURRENT_BATCH_INDEX, -1)) + } + + @Test + fun start_files_upload_job_does_nothing_when_empty() { + val uploadIds = longArrayOf() + + backgroundJobManager.startFilesUploadJob(user, uploadIds, true) + + verify(workManager, timeout(1000).times(0)).beginUniqueWork(any(), any(), any>()) + } + } + + class FilesUploadNonBeta : Fixture() { + + @Before + fun setUpFilesUploadNonBetaTest() { + // Mock context resources to return false for is_beta to test the sequential upload logic + val resources = mock() + whenever(resources.getBoolean(com.owncloud.android.R.bool.is_beta)).thenReturn(false) + whenever(context.resources).thenReturn(resources) + } + + @Test + fun start_files_upload_job_chains_sequentially_when_not_beta() { + val uploadIds = longArrayOf(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11) + + val continuation: WorkContinuation = mock() + whenever( + workManager.beginUniqueWork(any(), any(), any()) + ).thenReturn(continuation) + whenever(continuation.then(any())).thenReturn(continuation) + whenever(continuation.enqueue()).thenReturn(mock()) + + backgroundJobManager.startFilesUploadJob(user, uploadIds, true) + + // Verify that beginUniqueWork was called (sequential chaining starts here) + verify(workManager, timeout(1000)).beginUniqueWork( + any(), + eq(ExistingWorkPolicy.APPEND_OR_REPLACE), + any() + ) + } + } + class Tags { @Test fun split_tag_key_and_value() { diff --git a/app/src/androidTest/java/com/nextcloud/client/jobs/ContactsBackupIT.kt b/app/src/androidTest/java/com/nextcloud/client/jobs/ContactsBackupIT.kt index 79197a12a034..24b4e7504657 100644 --- a/app/src/androidTest/java/com/nextcloud/client/jobs/ContactsBackupIT.kt +++ b/app/src/androidTest/java/com/nextcloud/client/jobs/ContactsBackupIT.kt @@ -37,7 +37,7 @@ import java.io.IOException class ContactsBackupIT : AbstractOnServerIT() { private val workManager = WorkManager.getInstance(targetContext) private val preferences: AppPreferences = AppPreferencesImpl.fromContext(targetContext) - private val backgroundJobManager = BackgroundJobManagerImpl(workManager, ClockImpl(), preferences) + private val backgroundJobManager = BackgroundJobManagerImpl(workManager, ClockImpl(), preferences, targetContext) @get:Rule val writeContactsRule: GrantPermissionRule = GrantPermissionRule.grant(Manifest.permission.WRITE_CONTACTS) diff --git a/app/src/main/java/com/nextcloud/client/jobs/BackgroundJobFactory.kt b/app/src/main/java/com/nextcloud/client/jobs/BackgroundJobFactory.kt index 1fe963ad7c34..f4b76c5ab106 100644 --- a/app/src/main/java/com/nextcloud/client/jobs/BackgroundJobFactory.kt +++ b/app/src/main/java/com/nextcloud/client/jobs/BackgroundJobFactory.kt @@ -34,6 +34,7 @@ import com.nextcloud.client.jobs.metadata.MetadataWorker import com.nextcloud.client.jobs.offlineOperations.OfflineOperationsWorker import com.nextcloud.client.jobs.upload.AlbumFileUploadWorker import com.nextcloud.client.jobs.upload.FileUploadWorker +import com.nextcloud.client.jobs.upload.UploadNotificationManager import com.nextcloud.client.logger.Logger import com.nextcloud.client.network.ConnectivityService import com.nextcloud.client.preferences.AppPreferences @@ -45,6 +46,7 @@ import com.owncloud.android.utils.theme.ViewThemeUtils import org.greenrobot.eventbus.EventBus import javax.inject.Inject import javax.inject.Provider +import kotlin.random.Random /** * This factory is responsible for creating all background jobs and for injecting worker dependencies. @@ -266,6 +268,7 @@ class BackgroundJobFactory @Inject constructor( syncedFolderProvider, context, uploadFileOperationFactory, + UploadNotificationManager(context, viewThemeUtils.get(), Random.nextInt()), params ) diff --git a/app/src/main/java/com/nextcloud/client/jobs/BackgroundJobManagerImpl.kt b/app/src/main/java/com/nextcloud/client/jobs/BackgroundJobManagerImpl.kt index 12b6ffc52ced..0f14e4e6bb10 100644 --- a/app/src/main/java/com/nextcloud/client/jobs/BackgroundJobManagerImpl.kt +++ b/app/src/main/java/com/nextcloud/client/jobs/BackgroundJobManagerImpl.kt @@ -6,6 +6,7 @@ */ package com.nextcloud.client.jobs +import android.content.Context import android.provider.MediaStore import androidx.lifecycle.LiveData import androidx.lifecycle.MutableLiveData @@ -46,6 +47,7 @@ import com.owncloud.android.datamodel.OCFile import com.owncloud.android.datamodel.SyncedFolder import com.owncloud.android.lib.common.utils.Log_OC import com.owncloud.android.operations.DownloadType +import com.owncloud.android.R import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch @@ -73,7 +75,8 @@ import kotlin.reflect.KClass internal class BackgroundJobManagerImpl( private val workManager: WorkManager, private val clock: Clock, - private val preferences: AppPreferences + private val preferences: AppPreferences, + private val context: Context ) : BackgroundJobManager, Injectable { @@ -128,6 +131,11 @@ internal class BackgroundJobManagerImpl( private const val KEEP_LOG_MILLIS = 1000 * 60 * 60 * 24 * 3L + /** + * The maximum number of concurrent parallel uploads + */ + const val MAX_CONCURRENT_UPLOADS = 5 + fun formatNameTag(name: String, user: User? = null): String = if (user == null) { "$TAG_PREFIX_NAME:$name" } else { @@ -644,6 +652,111 @@ internal class BackgroundJobManagerImpl( private fun startFileUploadJobTag(accountName: String): String = JOB_FILES_UPLOAD + accountName + private fun isParallelUploadBetaEnabled(): Boolean = context.resources.getBoolean(R.bool.is_beta) + + private fun buildUploadBatchData( + accountName: String, + uploadIds: List, + batchIndex: Int, + totalUploadSize: Int, + showSameFileAlreadyExistsNotification: Boolean, + skipAutoUploadCheck: Boolean + ): Data = Data.Builder() + .putBoolean( + FileUploadWorker.SHOW_SAME_FILE_ALREADY_EXISTS_NOTIFICATION, + showSameFileAlreadyExistsNotification + ) + .putBoolean(FileUploadWorker.SKIP_AUTO_UPLOAD_CHECK, skipAutoUploadCheck) + .putString(FileUploadWorker.ACCOUNT, accountName) + .putInt(FileUploadWorker.TOTAL_UPLOAD_SIZE, totalUploadSize) + .putLongArray(FileUploadWorker.UPLOAD_IDS, uploadIds.toLongArray()) + .putInt(FileUploadWorker.CURRENT_BATCH_INDEX, batchIndex) + .build() + + private fun createFileUploadWorkRequest( + user: User, + tag: String, + data: Data, + constraints: Constraints + ): OneTimeWorkRequest = oneTimeRequestBuilder(FileUploadWorker::class, JOB_FILES_UPLOAD, user) + .addTag(tag) + .setInputData(data) + .setConstraints(constraints) + .setExpedited(OutOfQuotaPolicy.RUN_AS_NON_EXPEDITED_WORK_REQUEST) + .build() + + private fun enqueueParallelUploadBatches( + user: User, + uploadIds: LongArray, + showSameFileAlreadyExistsNotification: Boolean, + skipAutoUploadCheck: Boolean + ) { + if (uploadIds.isEmpty()) return + + val maxConcurrentUploads = MAX_CONCURRENT_UPLOADS + val chunkSize = ((uploadIds.size + maxConcurrentUploads - 1) / maxConcurrentUploads).coerceAtLeast(1) + val batches = uploadIds.toList().chunked(chunkSize) + val executionId = System.currentTimeMillis() + val tag = "${startFileUploadJobTag(user.accountName)}_$executionId" + val constraints = SupportedNetworkTransports.getConstraints() + + val workRequests = batches.mapIndexed { index, batch -> + val data = buildUploadBatchData( + accountName = user.accountName, + uploadIds = batch, + batchIndex = index, + totalUploadSize = chunkSize, + showSameFileAlreadyExistsNotification = showSameFileAlreadyExistsNotification, + skipAutoUploadCheck = skipAutoUploadCheck + ) + createFileUploadWorkRequest(user, tag, data, constraints) + } + + if (workRequests.isNotEmpty()) { + workManager.enqueueUniqueWork(tag, ExistingWorkPolicy.KEEP, workRequests) + } + } + + private fun enqueueSequentialUploadBatches( + user: User, + uploadIds: LongArray, + showSameFileAlreadyExistsNotification: Boolean, + skipAutoUploadCheck: Boolean + ) { + if (uploadIds.isEmpty()) return + + val batchSize = FileUploadHelper.MAX_FILE_COUNT + val batches = uploadIds.toList().chunked(batchSize) + val tag = startFileUploadJobTag(user.accountName) + val constraints = SupportedNetworkTransports.getConstraints() + + val workRequests = batches.mapIndexed { index, batch -> + val data = buildUploadBatchData( + accountName = user.accountName, + uploadIds = batch, + batchIndex = index, + totalUploadSize = uploadIds.size, + showSameFileAlreadyExistsNotification = showSameFileAlreadyExistsNotification, + skipAutoUploadCheck = skipAutoUploadCheck + ) + createFileUploadWorkRequest(user, tag, data, constraints) + } + + if (workRequests.isEmpty()) return + + var workChain = workManager.beginUniqueWork( + tag, + ExistingWorkPolicy.APPEND_OR_REPLACE, + workRequests.first() + ) + + workRequests.drop(1).forEach { request -> + workChain = workChain.then(request) + } + + workChain.enqueue() + } + override fun isStartFileUploadJobScheduled(accountName: String): Boolean = workManager.isWorkScheduled(startFileUploadJobTag(accountName)) @@ -666,47 +779,20 @@ internal class BackgroundJobManagerImpl( skipAutoUploadCheck: Boolean ) { defaultDispatcherScope.launch { - val batchSize = FileUploadHelper.MAX_FILE_COUNT - val batches = uploadIds.toList().chunked(batchSize) - val tag = startFileUploadJobTag(user.accountName) - - val constraints = SupportedNetworkTransports.getConstraints() - - val dataBuilder = Data.Builder() - .putBoolean( - FileUploadWorker.SHOW_SAME_FILE_ALREADY_EXISTS_NOTIFICATION, - showSameFileAlreadyExistsNotification + if (isParallelUploadBetaEnabled()) { + enqueueParallelUploadBatches( + user, + uploadIds, + showSameFileAlreadyExistsNotification, + skipAutoUploadCheck ) - .putBoolean(FileUploadWorker.SKIP_AUTO_UPLOAD_CHECK, skipAutoUploadCheck) - .putString(FileUploadWorker.ACCOUNT, user.accountName) - .putInt(FileUploadWorker.TOTAL_UPLOAD_SIZE, uploadIds.size) - - val workRequests = batches.mapIndexed { index, batch -> - dataBuilder - .putLongArray(FileUploadWorker.UPLOAD_IDS, batch.toLongArray()) - .putInt(FileUploadWorker.CURRENT_BATCH_INDEX, index) - - oneTimeRequestBuilder(FileUploadWorker::class, JOB_FILES_UPLOAD, user) - .addTag(tag) - .setInputData(dataBuilder.build()) - .setConstraints(constraints) - .setExpedited(OutOfQuotaPolicy.RUN_AS_NON_EXPEDITED_WORK_REQUEST) - .build() - } - - // Chain the work requests sequentially - if (workRequests.isNotEmpty()) { - var workChain = workManager.beginUniqueWork( - tag, - ExistingWorkPolicy.APPEND_OR_REPLACE, - workRequests.first() + } else { + enqueueSequentialUploadBatches( + user, + uploadIds, + showSameFileAlreadyExistsNotification, + skipAutoUploadCheck ) - - workRequests.drop(1).forEach { request -> - workChain = workChain.then(request) - } - - workChain.enqueue() } } } @@ -722,6 +808,7 @@ internal class BackgroundJobManagerImpl( * within the worker. * @param albumName Album on which selected files should be copy after upload */ + override fun startAlbumFilesUploadJob(user: User, uploadIds: LongArray, albumName: String) { defaultDispatcherScope.launch { val batchSize = FileUploadHelper.MAX_FILE_COUNT diff --git a/app/src/main/java/com/nextcloud/client/jobs/JobsModule.kt b/app/src/main/java/com/nextcloud/client/jobs/JobsModule.kt index 3f331453c7a1..f8382ca29e43 100644 --- a/app/src/main/java/com/nextcloud/client/jobs/JobsModule.kt +++ b/app/src/main/java/com/nextcloud/client/jobs/JobsModule.kt @@ -39,6 +39,7 @@ class JobsModule { fun backgroundJobManager( workManager: WorkManager, clock: Clock, - preferences: AppPreferences - ): BackgroundJobManager = BackgroundJobManagerImpl(workManager, clock, preferences) + preferences: AppPreferences, + context: Context + ): BackgroundJobManager = BackgroundJobManagerImpl(workManager, clock, preferences, context) } 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 128a544b7fdb..1dd14521df5b 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 @@ -21,9 +21,10 @@ 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.jobs.upload.FileUploadWorker.Companion.activeOperations +import com.nextcloud.client.notifications.AppWideNotificationManager import com.nextcloud.client.network.Connectivity import com.nextcloud.client.network.ConnectivityService -import com.nextcloud.client.notifications.AppWideNotificationManager import com.nextcloud.model.OCUploadLocalPathData import com.nextcloud.utils.SnackbarUtil import com.nextcloud.utils.extensions.checkWCFRestrictions @@ -473,17 +474,18 @@ class FileUploadHelper { @Suppress("ReturnCount") fun isUploadingNow(upload: OCUpload?): Boolean { - val currentUploadFileOperation = FileUploadWorker.getCurrentUpload(upload?.uploadId) - if (currentUploadFileOperation == null || currentUploadFileOperation.user == null) return false - if (upload == null || upload.accountName != currentUploadFileOperation.user.accountName) return false + upload ?: return false - return if (currentUploadFileOperation.oldFile != null) { - // For file conflicts check old file remote path - upload.remotePath == currentUploadFileOperation.remotePath || - upload.remotePath == currentUploadFileOperation.oldFile!! - .remotePath - } else { - upload.remotePath == currentUploadFileOperation.remotePath + val currentUploadFileOperation = FileUploadWorker.getCurrentUpload(upload.uploadId) + if (currentUploadFileOperation == null || currentUploadFileOperation.user == null) return false + if (upload.accountName != currentUploadFileOperation.user.accountName) return false + + return activeOperations.values.any { operation -> + operation.user?.accountName == upload.accountName && + ( + upload.remotePath == operation.remotePath || + upload.remotePath == operation.oldFile?.remotePath + ) } } diff --git a/app/src/main/java/com/nextcloud/client/jobs/upload/FileUploadWorker.kt b/app/src/main/java/com/nextcloud/client/jobs/upload/FileUploadWorker.kt index e698f3850869..4d32571881f0 100644 --- a/app/src/main/java/com/nextcloud/client/jobs/upload/FileUploadWorker.kt +++ b/app/src/main/java/com/nextcloud/client/jobs/upload/FileUploadWorker.kt @@ -55,7 +55,6 @@ import kotlinx.coroutines.ensureActive import kotlinx.coroutines.withContext import java.io.File import java.util.concurrent.ConcurrentHashMap -import kotlin.random.Random import kotlin.time.Duration.Companion.milliseconds @Suppress("LongParameterList", "TooGenericExceptionCaught") @@ -72,6 +71,7 @@ class FileUploadWorker( val syncedFolderProvider: SyncedFolderProvider, val context: Context, val uploadFileOperationFactory: UploadFileOperationFactory, + val notificationManager: UploadNotificationManager, params: WorkerParameters ) : CoroutineWorker(context, params), OnDatatransferProgressListener { @@ -87,6 +87,7 @@ class FileUploadWorker( const val TOTAL_UPLOAD_SIZE = "total_upload_size" const val SHOW_SAME_FILE_ALREADY_EXISTS_NOTIFICATION = "show_same_file_already_exists_notification" const val SKIP_AUTO_UPLOAD_CHECK = "skip_auto_upload_check" + private const val BATCH_SIZE = 100 const val EXTRA_ACCOUNT_NAME = "ACCOUNT_NAME" @@ -96,7 +97,7 @@ class FileUploadWorker( const val LOCAL_BEHAVIOUR_FORGET = 2 const val LOCAL_BEHAVIOUR_DELETE = 3 - private val activeOperations = ConcurrentHashMap() + val activeOperations = ConcurrentHashMap() @JvmOverloads fun cancelUpload(remotePath: String?, accountName: String?, onCompleted: () -> Unit = {}) { @@ -155,9 +156,9 @@ class FileUploadWorker( } } - private var lastPercent = 0 - private val notificationId = Random.nextInt() - private val notificationManager = UploadNotificationManager(context, viewThemeUtils, notificationId) + private val lastPercents = ConcurrentHashMap() + private val lastUpdateTimes = ConcurrentHashMap() + private val intents = FileUploaderIntents(context) private val fileUploadEventBroadcaster = FileUploadEventBroadcaster(localBroadcastManager) private val retryPolicy = UploadDelayPolicy() @@ -203,7 +204,7 @@ class FileUploadWorker( val notification = createNotification(notificationTitle) return ForegroundServiceHelper.createWorkerForegroundInfo( - notificationId, + notificationManager.getId(), notification, ForegroundServiceType.DataSync ) @@ -211,7 +212,7 @@ class FileUploadWorker( private suspend fun updateForegroundInfo(notification: Notification) { val foregroundInfo = ForegroundServiceHelper.createWorkerForegroundInfo( - notificationId, + notificationManager.getId(), notification, ForegroundServiceType.DataSync ) @@ -333,11 +334,11 @@ class FileUploadWorker( activeOperations[upload.uploadId] = operation val currentIndex = (index + 1) - val currentUploadIndex = (currentIndex + previouslyUploadedFileSize) + notificationManager.prepareForStart( operation, startIntent = intents.openUploadListIntent(operation), - currentUploadIndex = currentUploadIndex, + currentUploadIndex = currentIndex, totalUploadSize = totalUploadSize ) @@ -362,7 +363,7 @@ class FileUploadWorker( } } - sendUploadFinishEvent(totalUploadSize, currentUploadIndex, operation, result) + sendUploadFinishEvent(totalUploadSize, currentIndex, operation, result) if (result.code == ResultCode.UNAUTHORIZED) { Log_OC.e(TAG, "credentials are no longer valid, stopping uploads") @@ -498,14 +499,17 @@ class FileUploadWorker( totalToTransfer: Long, fileAbsoluteName: String ) { + val currentUploadFileOperation = + activeOperations.values.find { it.originalStoragePath == fileAbsoluteName } + val percent = getPercent(totalTransferredSoFar, totalToTransfer) val currentTime = System.currentTimeMillis() + val lastPercent = lastPercents[fileAbsoluteName] ?: 0 + val lastUpdateTime = lastUpdateTimes[fileAbsoluteName] ?: 0L + if (percent != lastPercent && (currentTime - lastUpdateTime) >= minProgressUpdateInterval) { notificationManager.run { - val currentUploadFileOperation = - activeOperations.values.find { it.originalStoragePath == fileAbsoluteName } - val accountName = currentUploadFileOperation?.user?.accountName val remotePath = currentUploadFileOperation?.remotePath @@ -526,9 +530,8 @@ class FileUploadWorker( dismissOldErrorNotification(currentUploadFileOperation) } - lastUpdateTime = currentTime + lastUpdateTimes[fileAbsoluteName] = currentTime + lastPercents[fileAbsoluteName] = percent } - - lastPercent = percent } } diff --git a/app/src/test/java/com/nextcloud/client/jobs/upload/FileUploadHelperTest.kt b/app/src/test/java/com/nextcloud/client/jobs/upload/FileUploadHelperTest.kt new file mode 100644 index 000000000000..7d26d5ad8192 --- /dev/null +++ b/app/src/test/java/com/nextcloud/client/jobs/upload/FileUploadHelperTest.kt @@ -0,0 +1,148 @@ +/* + * Nextcloud - Android Client + * + * SPDX-FileCopyrightText: 2026 Raphael Vieira raphaelecv.projects@gmail.com + * SPDX-License-Identifier: AGPL-3.0-or-later OR GPL-2.0-only + */ + +package com.nextcloud.client.jobs.upload + +import com.nextcloud.client.account.User +import com.nextcloud.client.di.AppComponent +import com.owncloud.android.MainApp +import com.owncloud.android.datamodel.OCFile +import com.owncloud.android.db.OCUpload +import com.owncloud.android.operations.UploadFileOperation +import io.mockk.every +import io.mockk.mockk +import io.mockk.mockkStatic +import io.mockk.unmockkAll +import org.junit.After +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test + +class FileUploadHelperTest { + + private lateinit var fileUploadHelper: FileUploadHelper + + @Before + fun setUp() { + mockkStatic(MainApp::class) + val appComponent = mockk(relaxed = true) + every { MainApp.getAppComponent() } returns appComponent + + fileUploadHelper = FileUploadHelper() + FileUploadWorker.activeOperations.clear() + } + + @After + fun tearDown() { + unmockkAll() + FileUploadWorker.activeOperations.clear() + } + + @Test + fun `isUploadingNow returns false for null upload`() { + assertFalse(fileUploadHelper.isUploadingNow(null)) + } + + @Test + fun `isUploadingNow returns false when no active operations`() { + val upload = mockk() + every { upload.uploadId } returns 1 + + assertFalse(fileUploadHelper.isUploadingNow(upload)) + } + + @Test + fun `isUploadingNow returns true when remotePath matches`() { + val accountName = "account" + val remotePath = "/file.txt" + + val upload = mockk() + every { upload.accountName } returns accountName + every { upload.remotePath } returns remotePath + every { upload.uploadId } returns 1 + + val operation = mockk() + val user = mockk() + every { user.accountName } returns accountName + every { operation.user } returns user + every { operation.remotePath } returns remotePath + every { operation.oldFile } returns null + + FileUploadWorker.activeOperations[1] = operation + + assertTrue(fileUploadHelper.isUploadingNow(upload)) + } + + @Test + fun `isUploadingNow returns true when old remotePath matches`() { + val accountName = "account" + val remotePath = "/file_renamed.txt" + val oldRemotePath = "/file.txt" + + val upload = mockk() + every { upload.accountName } returns accountName + every { upload.remotePath } returns oldRemotePath + every { upload.uploadId } returns 1 + + val operation = mockk() + val user = mockk() + every { user.accountName } returns accountName + every { operation.user } returns user + every { operation.remotePath } returns remotePath + + val oldFile = mockk() + every { oldFile.remotePath } returns oldRemotePath + every { operation.oldFile } returns oldFile + + FileUploadWorker.activeOperations[1] = operation + + assertTrue(fileUploadHelper.isUploadingNow(upload)) + } + + @Test + fun `isUploadingNow returns false when accountName does not match`() { + val remotePath = "/file.txt" + + val upload = mockk() + every { upload.accountName } returns "account1" + every { upload.remotePath } returns remotePath + every { upload.uploadId } returns 1 + + val operation = mockk() + val user = mockk() + every { user.accountName } returns "account2" + every { operation.user } returns user + every { operation.remotePath } returns remotePath + every { operation.oldFile } returns null + + FileUploadWorker.activeOperations[1] = operation + + assertFalse(fileUploadHelper.isUploadingNow(upload)) + } + + @Test + fun `isUploadingNow returns false when paths do not match`() { + val accountName = "account" + + val upload = mockk() + every { upload.accountName } returns accountName + every { upload.remotePath } returns "/other.txt" + every { upload.uploadId } returns 1 + + val operation = mockk() + val user = mockk() + every { user.accountName } returns accountName + every { operation.user } returns user + every { operation.remotePath } returns "/file.txt" + every { operation.oldFile } returns null + + FileUploadWorker.activeOperations[1] = operation + + assertFalse(fileUploadHelper.isUploadingNow(upload)) + } +} diff --git a/app/src/test/java/com/nextcloud/client/jobs/upload/FileUploadWorkerTest.kt b/app/src/test/java/com/nextcloud/client/jobs/upload/FileUploadWorkerTest.kt new file mode 100644 index 000000000000..5fbe83b144fe --- /dev/null +++ b/app/src/test/java/com/nextcloud/client/jobs/upload/FileUploadWorkerTest.kt @@ -0,0 +1,226 @@ +/* + * Nextcloud - Android Client + * + * SPDX-FileCopyrightText: 2026 Raphael Vieira raphaelecv.projects@gmail.com + * SPDX-License-Identifier: AGPL-3.0-or-later OR GPL-2.0-only + */ + +package com.nextcloud.client.jobs.upload + +import android.app.NotificationManager +import android.content.Context +import androidx.localbroadcastmanager.content.LocalBroadcastManager +import androidx.work.ListenableWorker +import androidx.work.WorkerParameters +import com.nextcloud.android.common.ui.theme.MaterialSchemes +import com.nextcloud.client.account.UserAccountManager +import com.nextcloud.client.device.PowerManagementService +import com.nextcloud.client.jobs.BackgroundJobManager +import com.nextcloud.client.jobs.autoUpload.FileSystemRepository +import com.nextcloud.client.network.Connectivity +import com.nextcloud.client.network.ConnectivityService +import com.nextcloud.client.preferences.AppPreferences +import com.owncloud.android.datamodel.SyncedFolderProvider +import com.owncloud.android.datamodel.UploadsStorageManager +import com.owncloud.android.lib.common.operations.RemoteOperationResult.ResultCode +import com.owncloud.android.operations.UploadFileOperation +import com.owncloud.android.operations.factory.UploadFileOperationFactory +import com.owncloud.android.utils.theme.ViewThemeUtils +import io.mockk.every +import io.mockk.mockk +import io.mockk.unmockkAll +import io.mockk.verify +import kotlinx.coroutines.runBlocking +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import java.util.Optional + +class FileUploadWorkerTest { + + private lateinit var worker: FileUploadWorker + private val uploadsStorageManager: UploadsStorageManager = mockk(relaxed = true) + private val connectivityService: ConnectivityService = mockk(relaxed = true) + private val powerManagementService: PowerManagementService = mockk(relaxed = true) + private val userAccountManager: UserAccountManager = mockk(relaxed = true) + private val localBroadcastManager: LocalBroadcastManager = mockk(relaxed = true) + private val backgroundJobManager: BackgroundJobManager = mockk(relaxed = true) + private val preferences: AppPreferences = mockk(relaxed = true) + private val filesystemRepository: FileSystemRepository = mockk(relaxed = true) + private val syncedFolderProvider: SyncedFolderProvider = mockk(relaxed = true) + private val context: Context = mockk(relaxed = true) + private val uploadFileOperationFactory: UploadFileOperationFactory = mockk(relaxed = true) + private val params: WorkerParameters = mockk(relaxed = true) + private val systemNotificationManager: NotificationManager = mockk(relaxed = true) + private val uploadNotificationManager: UploadNotificationManager = mockk(relaxed = true) + + @Before + fun setUp() { + every { context.getSystemService(Context.NOTIFICATION_SERVICE) } returns systemNotificationManager + + val materialSchemes = mockk(relaxed = true) + val viewThemeUtils = ViewThemeUtils(materialSchemes, mockk(relaxed = true)) + + val connectivity = mockk() + every { connectivity.isConnected } returns true + every { connectivityService.connectivity } returns connectivity + every { connectivityService.isConnected } returns true + every { connectivityService.isInternetWalled() } returns false + + worker = FileUploadWorker( + uploadsStorageManager, + connectivityService, + powerManagementService, + userAccountManager, + viewThemeUtils, + localBroadcastManager, + backgroundJobManager, + preferences, + filesystemRepository, + syncedFolderProvider, + context, + uploadFileOperationFactory, + uploadNotificationManager, + params + ) + } + + @After + fun tearDown() { + unmockkAll() + FileUploadWorker.activeOperations.clear() + } + + @Test + fun `doWork returns failure when account name is missing`() = runBlocking { + // GIVEN + every { params.inputData.getString(FileUploadWorker.ACCOUNT) } returns null + + // WHEN + val result = worker.doWork() + + // THEN + assertEquals(ListenableWorker.Result.failure(), result) + } + + @Test + fun `doWork returns failure when upload ids are missing`() = runBlocking { + // GIVEN + every { params.inputData.getString(FileUploadWorker.ACCOUNT) } returns "account" + every { params.inputData.getLongArray(FileUploadWorker.UPLOAD_IDS) } returns null + + // WHEN + val result = worker.doWork() + + // THEN + assertEquals(ListenableWorker.Result.failure(), result) + } + + @Test + fun `doWork returns failure when batch index is missing`() = runBlocking { + // GIVEN + every { params.inputData.getString(FileUploadWorker.ACCOUNT) } returns "account" + every { params.inputData.getLongArray(FileUploadWorker.UPLOAD_IDS) } returns longArrayOf(1L) + every { params.inputData.getInt(FileUploadWorker.CURRENT_BATCH_INDEX, -1) } returns -1 + + // WHEN + val result = worker.doWork() + + // THEN + assertEquals(ListenableWorker.Result.failure(), result) + } + + @Test + fun `doWork returns failure when user is not found`() = runBlocking { + // GIVEN + val accountName = "account" + every { params.inputData.getString(FileUploadWorker.ACCOUNT) } returns accountName + every { params.inputData.getLongArray(FileUploadWorker.UPLOAD_IDS) } returns longArrayOf(1L) + every { params.inputData.getInt(FileUploadWorker.CURRENT_BATCH_INDEX, any()) } returns 0 + every { params.inputData.getInt(FileUploadWorker.TOTAL_UPLOAD_SIZE, any()) } returns 1 + every { userAccountManager.getUser(accountName) } returns Optional.empty() + + // WHEN + val result = worker.doWork() + + // THEN + assertEquals(ListenableWorker.Result.failure(), result) + } + + @Test + fun `onTransferProgress updates notification manager`() { + // GIVEN + val fileName = "testFile" + val operation = mockk(relaxed = true) + + every { operation.getOriginalStoragePath() } returns fileName + every { operation.getOCUploadId() } returns 1L + FileUploadWorker.registerActiveUpload(operation) + + // WHEN + worker.onTransferProgress(100, 50, 100, fileName) + + // THEN + verify { uploadNotificationManager.updateUploadProgress(50, match { true }) } + assertEquals(operation, FileUploadWorker.getCurrentUpload(1L)) + } + + @Test + fun `cancelCurrentUpload cancels matching operations`() { + // GIVEN + val remotePath = "path" + val accountName = "account" + val operation = mockk(relaxed = true) + every { operation.remotePath } returns remotePath + every { operation.user.accountName } returns accountName + FileUploadWorker.registerActiveUpload(operation) + + // WHEN + var completed = false + FileUploadWorker.cancelUpload(remotePath, accountName) { + completed = true + } + + // THEN + verify { operation.cancel(ResultCode.USER_CANCELLED) } + assertTrue(completed) + } + + @Test + fun `isUploading returns true when operation exists`() { + // GIVEN + val remotePath = "path" + val accountName = "account" + val operation = mockk(relaxed = true) + every { operation.remotePath } returns remotePath + every { operation.user.accountName } returns accountName + FileUploadWorker.registerActiveUpload(operation) + + // WHEN & THEN + assertTrue(FileUploadWorker.isUploading(remotePath, accountName)) + assertFalse(FileUploadWorker.isUploading("other", accountName)) + } + + @Test + fun `getUploadAction returns correct values`() { + assertEquals( + FileUploadWorker.LOCAL_BEHAVIOUR_FORGET, + FileUploadWorker.getUploadAction("LOCAL_BEHAVIOUR_FORGET") + ) + assertEquals( + FileUploadWorker.LOCAL_BEHAVIOUR_MOVE, + FileUploadWorker.getUploadAction("LOCAL_BEHAVIOUR_MOVE") + ) + assertEquals( + FileUploadWorker.LOCAL_BEHAVIOUR_DELETE, + FileUploadWorker.getUploadAction("LOCAL_BEHAVIOUR_DELETE") + ) + assertEquals( + FileUploadWorker.LOCAL_BEHAVIOUR_FORGET, + FileUploadWorker.getUploadAction("UNKNOWN") + ) + } +}