Skip to content

Commit 8d51240

Browse files
committed
fixed notifications manager and uploads list. Added relevant unit tests
Signed-off-by: Raphael Vieira <raphaelecv@hotmail.com>
1 parent a8d3079 commit 8d51240

4 files changed

Lines changed: 162 additions & 14 deletions

File tree

app/src/androidTest/java/com/nextcloud/client/jobs/BackgroundJobManagerTest.kt

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,11 @@ import androidx.lifecycle.LiveData
1111
import androidx.lifecycle.MutableLiveData
1212
import androidx.lifecycle.Observer
1313
import androidx.test.annotation.UiThreadTest
14+
import androidx.test.runner.screenshot.Screenshot
1415
import androidx.work.Data
1516
import androidx.work.ExistingPeriodicWorkPolicy
1617
import androidx.work.ExistingWorkPolicy
18+
import androidx.work.NetworkType
1719
import androidx.work.OneTimeWorkRequest
1820
import androidx.work.PeriodicWorkRequest
1921
import androidx.work.WorkContinuation
@@ -25,7 +27,12 @@ import com.nextcloud.client.jobs.upload.FileUploadWorker
2527
import com.nextcloud.client.preferences.AppPreferences
2628
import com.nextcloud.utils.extensions.toByteArray
2729
import com.owncloud.android.lib.common.utils.Log_OC
30+
import io.mockk.every
31+
import io.mockk.mockk
32+
import io.mockk.slot
2833
import org.apache.commons.io.FileUtils
34+
import org.bouncycastle.util.test.SimpleTest.runTest
35+
import org.junit.Assert
2936
import org.junit.Assert.assertEquals
3037
import org.junit.Assert.assertFalse
3138
import org.junit.Assert.assertNotNull
@@ -396,8 +403,7 @@ class BackgroundJobManagerTest {
396403

397404
@Test
398405
fun start_files_upload_job_enqueues_batches() {
399-
val uploadIds = longArrayOf(1, 2, 3, 4, 5)
400-
whenever(preferences.maxConcurrentUploads).thenReturn(2)
406+
val uploadIds = longArrayOf(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11)
401407

402408
val continuation: WorkContinuation = mock()
403409
whenever(
@@ -409,7 +415,7 @@ class BackgroundJobManagerTest {
409415
val tagCaptor = argumentCaptor<String>()
410416
val requestsCaptor = argumentCaptor<List<OneTimeWorkRequest>>()
411417

412-
verify(workManager, timeout(1000)).beginUniqueWork(
418+
verify(workManager, timeout(1000)).enqueueUniqueWork(
413419
tagCaptor.capture(),
414420
eq(ExistingWorkPolicy.KEEP),
415421
requestsCaptor.capture()
@@ -419,13 +425,13 @@ class BackgroundJobManagerTest {
419425
assertTrue(tag.startsWith(BackgroundJobManagerImpl.JOB_FILES_UPLOAD + USER_ACCOUNT_NAME + "_"))
420426

421427
val requests = requestsCaptor.firstValue
422-
assertEquals(3, requests.size)
428+
assertEquals(6, requests.size)
423429

424430
// Check first batch [1, 2]
425431
val data1 = requests[0].workSpec.input
426432
assertEquals(true, data1.getBoolean(FileUploadWorker.SHOW_SAME_FILE_ALREADY_EXISTS_NOTIFICATION, false))
427433
assertEquals(USER_ACCOUNT_NAME, data1.getString(FileUploadWorker.ACCOUNT))
428-
assertEquals(5, data1.getInt(FileUploadWorker.TOTAL_UPLOAD_SIZE, 0))
434+
assertEquals(2, data1.getInt(FileUploadWorker.TOTAL_UPLOAD_SIZE, 0))
429435
assertTrue(longArrayOf(1, 2).contentEquals(data1.getLongArray(FileUploadWorker.UPLOAD_IDS)!!))
430436
assertEquals(0, data1.getInt(FileUploadWorker.CURRENT_BATCH_INDEX, -1))
431437

@@ -436,16 +442,13 @@ class BackgroundJobManagerTest {
436442

437443
// Check third batch [5]
438444
val data3 = requests[2].workSpec.input
439-
assertTrue(longArrayOf(5).contentEquals(data3.getLongArray(FileUploadWorker.UPLOAD_IDS)!!))
445+
assertTrue(longArrayOf(5, 6).contentEquals(data3.getLongArray(FileUploadWorker.UPLOAD_IDS)!!))
440446
assertEquals(2, data3.getInt(FileUploadWorker.CURRENT_BATCH_INDEX, -1))
441-
442-
verify(continuation).enqueue()
443447
}
444448

445449
@Test
446450
fun start_files_upload_job_does_nothing_when_empty() {
447451
val uploadIds = longArrayOf()
448-
whenever(preferences.maxConcurrentUploads).thenReturn(2)
449452

450453
backgroundJobManager.startFilesUploadJob(user, uploadIds, true)
451454

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -657,7 +657,7 @@ internal class BackgroundJobManagerImpl(
657657
*/
658658
override fun startFilesUploadJob(user: User, uploadIds: LongArray, showSameFileAlreadyExistsNotification: Boolean) {
659659
defaultDispatcherScope.launch {
660-
val chunkSize = (uploadIds.size / 7).coerceAtLeast(1)
660+
val chunkSize = (uploadIds.size / 5).coerceAtLeast(1)
661661
val batches = uploadIds.toList().chunked(chunkSize)
662662
val executionId = System.currentTimeMillis()
663663
val tag = "${startFileUploadJobTag(user.accountName)}_$executionId"
@@ -673,7 +673,7 @@ internal class BackgroundJobManagerImpl(
673673
showSameFileAlreadyExistsNotification
674674
)
675675
.putString(FileUploadWorker.ACCOUNT, user.accountName)
676-
.putInt(FileUploadWorker.TOTAL_UPLOAD_SIZE, uploadIds.size)
676+
.putInt(FileUploadWorker.TOTAL_UPLOAD_SIZE, chunkSize)
677677
.putLongArray(FileUploadWorker.UPLOAD_IDS, batch.toLongArray())
678678
.putInt(FileUploadWorker.CURRENT_BATCH_INDEX, index)
679679
.build()

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -271,11 +271,11 @@ class FileUploadWorker(
271271
activeUploadFileOperations[operation.originalStoragePath] = operation
272272

273273
val currentIndex = (index + 1)
274-
val currentUploadIndex = (currentIndex + previouslyUploadedFileSize)
274+
275275
notificationManager.prepareForStart(
276276
operation,
277277
startIntent = intents.openUploadListIntent(operation),
278-
currentUploadIndex = currentUploadIndex,
278+
currentUploadIndex = currentIndex,
279279
totalUploadSize = totalUploadSize
280280
)
281281

@@ -292,7 +292,7 @@ class FileUploadWorker(
292292
break
293293
}
294294

295-
sendUploadFinishEvent(totalUploadSize, currentUploadIndex, operation, result)
295+
sendUploadFinishEvent(totalUploadSize, currentIndex, operation, result)
296296
}
297297

298298
return@withContext Result.success()
Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,145 @@
1+
/*
2+
* Nextcloud - Android Client
3+
*
4+
* SPDX-FileCopyrightText: 2026 Raphael Vieira raphaelecv.projects@gmail.com
5+
* SPDX-License-Identifier: AGPL-3.0-or-later OR GPL-2.0-only
6+
*/
7+
8+
package com.nextcloud.client.jobs.upload
9+
10+
import com.nextcloud.client.account.User
11+
import com.nextcloud.client.di.AppComponent
12+
import com.owncloud.android.MainApp
13+
import com.owncloud.android.datamodel.OCFile
14+
import com.owncloud.android.db.OCUpload
15+
import com.owncloud.android.operations.UploadFileOperation
16+
import io.mockk.every
17+
import io.mockk.mockk
18+
import io.mockk.mockkStatic
19+
import io.mockk.unmockkAll
20+
import org.junit.After
21+
import org.junit.Assert.assertFalse
22+
import org.junit.Assert.assertTrue
23+
import org.junit.Before
24+
import org.junit.Test
25+
26+
class FileUploadHelperTest {
27+
28+
private lateinit var fileUploadHelper: FileUploadHelper
29+
30+
@Before
31+
fun setUp() {
32+
mockkStatic(MainApp::class)
33+
val appComponent = mockk<AppComponent>(relaxed = true)
34+
every { MainApp.getAppComponent() } returns appComponent
35+
36+
fileUploadHelper = FileUploadHelper()
37+
FileUploadWorker.activeUploadFileOperations.clear()
38+
}
39+
40+
@After
41+
fun tearDown() {
42+
unmockkAll()
43+
FileUploadWorker.activeUploadFileOperations.clear()
44+
}
45+
46+
@Test
47+
fun `isUploadingNow returns false for null upload`() {
48+
assertFalse(fileUploadHelper.isUploadingNow(null))
49+
}
50+
51+
@Test
52+
fun `isUploadingNow returns false when no active operations`() {
53+
val upload = mockk<OCUpload>()
54+
every { upload.accountName } returns "account"
55+
every { upload.remotePath } returns "/file.txt"
56+
57+
assertFalse(fileUploadHelper.isUploadingNow(upload))
58+
}
59+
60+
@Test
61+
fun `isUploadingNow returns true when remotePath matches`() {
62+
val accountName = "account"
63+
val remotePath = "/file.txt"
64+
65+
val upload = mockk<OCUpload>()
66+
every { upload.accountName } returns accountName
67+
every { upload.remotePath } returns remotePath
68+
69+
val operation = mockk<UploadFileOperation>()
70+
val user = mockk<User>()
71+
every { user.accountName } returns accountName
72+
every { operation.user } returns user
73+
every { operation.remotePath } returns remotePath
74+
every { operation.oldFile } returns null
75+
76+
FileUploadWorker.activeUploadFileOperations["key"] = operation
77+
78+
assertTrue(fileUploadHelper.isUploadingNow(upload))
79+
}
80+
81+
@Test
82+
fun `isUploadingNow returns true when old remotePath matches`() {
83+
val accountName = "account"
84+
val remotePath = "/file_renamed.txt"
85+
val oldRemotePath = "/file.txt"
86+
87+
val upload = mockk<OCUpload>()
88+
every { upload.accountName } returns accountName
89+
every { upload.remotePath } returns oldRemotePath
90+
91+
val operation = mockk<UploadFileOperation>()
92+
val user = mockk<User>()
93+
every { user.accountName } returns accountName
94+
every { operation.user } returns user
95+
every { operation.remotePath } returns remotePath
96+
97+
val oldFile = mockk<OCFile>()
98+
every { oldFile.remotePath } returns oldRemotePath
99+
every { operation.oldFile } returns oldFile
100+
101+
FileUploadWorker.activeUploadFileOperations["key"] = operation
102+
103+
assertTrue(fileUploadHelper.isUploadingNow(upload))
104+
}
105+
106+
@Test
107+
fun `isUploadingNow returns false when accountName does not match`() {
108+
val remotePath = "/file.txt"
109+
110+
val upload = mockk<OCUpload>()
111+
every { upload.accountName } returns "account1"
112+
every { upload.remotePath } returns remotePath
113+
114+
val operation = mockk<UploadFileOperation>()
115+
val user = mockk<User>()
116+
every { user.accountName } returns "account2"
117+
every { operation.user } returns user
118+
every { operation.remotePath } returns remotePath
119+
every { operation.oldFile } returns null
120+
121+
FileUploadWorker.activeUploadFileOperations["key"] = operation
122+
123+
assertFalse(fileUploadHelper.isUploadingNow(upload))
124+
}
125+
126+
@Test
127+
fun `isUploadingNow returns false when paths do not match`() {
128+
val accountName = "account"
129+
130+
val upload = mockk<OCUpload>()
131+
every { upload.accountName } returns accountName
132+
every { upload.remotePath } returns "/other.txt"
133+
134+
val operation = mockk<UploadFileOperation>()
135+
val user = mockk<User>()
136+
every { user.accountName } returns accountName
137+
every { operation.user } returns user
138+
every { operation.remotePath } returns "/file.txt"
139+
every { operation.oldFile } returns null
140+
141+
FileUploadWorker.activeUploadFileOperations["key"] = operation
142+
143+
assertFalse(fileUploadHelper.isUploadingNow(upload))
144+
}
145+
}

0 commit comments

Comments
 (0)