Parallelise uploads

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,7 @@ import java.util.concurrent.TimeoutException
     BackgroundJobManagerTest.PeriodicContactsBackup::class,
     BackgroundJobManagerTest.ImmediateContactsBackup::class,
     BackgroundJobManagerTest.ImmediateContactsImport::class,
+    BackgroundJobManagerTest.FilesUpload::class,
     BackgroundJobManagerTest.Tags::class
 )
 class BackgroundJobManagerTest {
@@ -90,6 +95,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 +106,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)
         }
 
         fun assertHasRequiredTags(tags: Set<String>, jobName: String, user: User? = null) {
@@ -385,6 +392,63 @@ class BackgroundJobManagerTest {
         }
     }
 
+    class FilesUpload : Fixture() {
+
+        @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<List<OneTimeWorkRequest>>())
+            ).thenReturn(continuation)
+
+            backgroundJobManager.startFilesUploadJob(user, uploadIds, true)
+
+            val tagCaptor = argumentCaptor<String>()
+            val requestsCaptor = argumentCaptor<List<OneTimeWorkRequest>>()
+
+            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<List<OneTimeWorkRequest>>())
+        }
+    }
+
     class Tags {
         @Test
         fun split_tag_key_and_value() {

app/src/main/java/com/nextcloud/client/jobs/BackgroundJobFactory.kt

@@ -29,6 +29,7 @@ import com.nextcloud.client.jobs.metadata.MetadataWorker
 import com.nextcloud.client.jobs.offlineOperations.OfflineOperationsWorker
 import com.nextcloud.client.jobs.folderDownload.FolderDownloadWorker
 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
@@ -39,6 +40,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.
@@ -238,6 +240,7 @@ class BackgroundJobFactory @Inject constructor(
             backgroundJobManager.get(),
             preferences,
             context,
+            UploadNotificationManager(context, viewThemeUtils.get(), Random.nextInt()),
             params
         )
 

app/src/main/java/com/nextcloud/client/jobs/BackgroundJobManagerImpl.kt

@@ -657,48 +657,41 @@ internal class BackgroundJobManagerImpl(
      */
     override fun startFilesUploadJob(user: User, uploadIds: LongArray, showSameFileAlreadyExistsNotification: Boolean) {
         defaultDispatcherScope.launch {
-            val batchSize = FileUploadHelper.MAX_FILE_COUNT
-            val batches = uploadIds.toList().chunked(batchSize)
-            val tag = startFileUploadJobTag(user.accountName)
+            val chunkSize = (uploadIds.size / 5).coerceAtLeast(1)
+            val batches = uploadIds.toList().chunked(chunkSize)
+            val executionId = System.currentTimeMillis()
+            val tag = "${startFileUploadJobTag(user.accountName)}_$executionId"
 
             val constraints = Constraints.Builder()
                 .setRequiredNetworkType(NetworkType.CONNECTED)
                 .build()
 
-            val dataBuilder = Data.Builder()
-                .putBoolean(
-                    FileUploadWorker.SHOW_SAME_FILE_ALREADY_EXISTS_NOTIFICATION,
-                    showSameFileAlreadyExistsNotification
-                )
-                .putString(FileUploadWorker.ACCOUNT, user.accountName)
-                .putInt(FileUploadWorker.TOTAL_UPLOAD_SIZE, uploadIds.size)
-
             val workRequests = batches.mapIndexed { index, batch ->
-                dataBuilder
+                val data = Data.Builder()
+                    .putBoolean(
+                        FileUploadWorker.SHOW_SAME_FILE_ALREADY_EXISTS_NOTIFICATION,
+                        showSameFileAlreadyExistsNotification
+                    )
+                    .putString(FileUploadWorker.ACCOUNT, user.accountName)
+                    .putInt(FileUploadWorker.TOTAL_UPLOAD_SIZE, chunkSize)
                     .putLongArray(FileUploadWorker.UPLOAD_IDS, batch.toLongArray())
                     .putInt(FileUploadWorker.CURRENT_BATCH_INDEX, index)
+                    .build()
 
                 oneTimeRequestBuilder(FileUploadWorker::class, JOB_FILES_UPLOAD, user)
                     .addTag(tag)
-                    .setInputData(dataBuilder.build())
+                    .setInputData(data)
                     .setConstraints(constraints)
                     .setExpedited(OutOfQuotaPolicy.RUN_AS_NON_EXPEDITED_WORK_REQUEST)
                     .build()
             }
 
-            // Chain the work requests sequentially
             if (workRequests.isNotEmpty()) {
-                var workChain = workManager.beginUniqueWork(
+                workManager.enqueueUniqueWork(
                     tag,
-                    ExistingWorkPolicy.APPEND_OR_REPLACE,
-                    workRequests.first()
+                    ExistingWorkPolicy.KEEP,
+                    workRequests
                 )
-
-                workRequests.drop(1).forEach { request ->
-                    workChain = workChain.then(request)
-                }
-
-                workChain.enqueue()
             }
         }
     }

app/src/main/java/com/nextcloud/client/jobs/upload/FileUploadHelper.kt

@@ -20,8 +20,10 @@ import com.nextcloud.client.device.PowerManagementService
 import com.nextcloud.client.jobs.BackgroundJobManager
 import com.nextcloud.client.jobs.upload.FileUploadWorker.Companion.currentUploadFileOperation
 import com.nextcloud.client.notifications.AppWideNotificationManager
+import com.nextcloud.client.jobs.upload.FileUploadWorker.Companion.activeUploadFileOperations
 import com.nextcloud.client.network.Connectivity
 import com.nextcloud.client.network.ConnectivityService
+import com.nextcloud.client.notifications.AppWideNotificationManager
 import com.nextcloud.utils.extensions.getUploadIds
 import com.owncloud.android.MainApp
 import com.owncloud.android.R
@@ -372,17 +374,14 @@ class FileUploadHelper {
 
     @Suppress("ReturnCount")
     fun isUploadingNow(upload: OCUpload?): Boolean {
-        val currentUploadFileOperation = currentUploadFileOperation
-        if (currentUploadFileOperation == null || currentUploadFileOperation.user == null) return false
-        if (upload == null || upload.accountName != currentUploadFileOperation.user.accountName) 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
+        upload ?: return false
+
+        return activeUploadFileOperations.values.any { operation ->
+            operation.user?.accountName == upload.accountName &&
+                (
+                    upload.remotePath == operation.remotePath ||
+                        upload.remotePath == operation.oldFile?.remotePath
+                    )
         }
     }