Skip to content

Commit 02bb99a

Browse files
alperozturk96alperozturk96
committed
fix(file-sort): concurrent modification exception
Signed-off-by: alperozturk <[email protected]>
1 parent 7a5e283 commit 02bb99a

File tree

4 files changed

+284
-21
lines changed

4 files changed

+284
-21
lines changed

‎app/src/androidTest/java/com/nextcloud/utils/FileSortOrderTests.kt‎

Lines changed: 251 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,251 @@
1+
/*
2+
* Nextcloud - Android Client
3+
*
4+
* SPDX-FileCopyrightText: 2025 Alper Ozturk <[email protected]>
5+
* SPDX-License-Identifier: AGPL-3.0-or-later
6+
*/
7+
package com.nextcloud.utils
8+
9+
import com.owncloud.android.datamodel.OCFile
10+
import com.owncloud.android.utils.FileSortOrder.Companion.SORT_A_TO_Z_ID
11+
import com.owncloud.android.utils.FileSortOrder.Companion.SORT_BIG_TO_SMALL_ID
12+
import com.owncloud.android.utils.FileSortOrder.Companion.SORT_NEW_TO_OLD_ID
13+
import com.owncloud.android.utils.FileSortOrder.Companion.SORT_OLD_TO_NEW_ID
14+
import com.owncloud.android.utils.FileSortOrder.Companion.SORT_SMALL_TO_BIG_ID
15+
import com.owncloud.android.utils.FileSortOrder.Companion.SORT_Z_TO_A_ID
16+
import com.owncloud.android.utils.FileSortOrderByDate
17+
import com.owncloud.android.utils.FileSortOrderByName
18+
import com.owncloud.android.utils.FileSortOrderBySize
19+
import org.junit.Assert.assertEquals
20+
import org.junit.Assert.assertTrue
21+
import org.junit.Test
22+
import java.io.File
23+
import java.util.concurrent.Executors
24+
import java.util.concurrent.TimeUnit
25+
26+
class FileSortOrderTests {
27+
28+
private fun createTempFile(prefix: String, lastModified: Long? = null, sizeBytes: Int? = null): File {
29+
return File.createTempFile(prefix, ".txt").apply {
30+
lastModified?.let { setLastModified(it) }
31+
sizeBytes?.let { writeBytes(ByteArray(it)) }
32+
}
33+
}
34+
35+
private fun createTempFolder(prefix: String): File {
36+
return File.createTempFile(prefix, "").apply {
37+
delete()
38+
mkdir()
39+
}
40+
}
41+
42+
private fun createOCFile(path: String, modTime: Long? = null, size: Long? = null): OCFile {
43+
return OCFile(path).apply {
44+
modTime?.let { modificationTimestamp = it }
45+
size?.let { fileLength = it }
46+
}
47+
}
48+
49+
private fun testConcurrentModification(
50+
files: MutableList<File>,
51+
sorter: com.owncloud.android.utils.FileSortOrder,
52+
iterations: Int = 50
53+
) {
54+
val executor = Executors.newFixedThreadPool(2)
55+
try {
56+
repeat(iterations) { i ->
57+
// modifying and sorting files
58+
executor.submit { sorter.sortLocalFiles(files) }
59+
executor.submit {
60+
files.add(createTempFile("temp$i", lastModified = i.toLong()))
61+
if (files.size > 20) files.removeAt(0)
62+
}
63+
}
64+
} finally {
65+
executor.shutdown()
66+
executor.awaitTermination(5, TimeUnit.SECONDS)
67+
}
68+
assertTrue(true)
69+
}
70+
71+
@Test
72+
fun testSortLocalFilesAscending() {
73+
val file1 = createTempFile("file1", lastModified = 1000)
74+
val file2 = createTempFile("file2", lastModified = 2000)
75+
val file3 = createTempFile("file3", lastModified = 1500)
76+
77+
val files = mutableListOf(file1, file2, file3)
78+
val sorter = FileSortOrderByDate(SORT_OLD_TO_NEW_ID, ascending = true)
79+
80+
val sorted = sorter.sortLocalFiles(files)
81+
82+
assertEquals(listOf(file1, file3, file2), sorted)
83+
}
84+
85+
@Test
86+
fun testSortLocalFilesAscendingFalse() {
87+
val file1 = createTempFile("file1", lastModified = 1000)
88+
val file2 = createTempFile("file2", lastModified = 2000)
89+
val file3 = createTempFile("file3", lastModified = 1500)
90+
91+
val files = mutableListOf(file1, file2, file3)
92+
val sorter = FileSortOrderByDate(SORT_OLD_TO_NEW_ID, ascending = false)
93+
94+
val sorted = sorter.sortLocalFiles(files)
95+
96+
assertEquals(listOf(file2, file3, file1), sorted)
97+
}
98+
99+
@Test
100+
fun testSortLocalFilesDescending() {
101+
val file1 = createTempFile("file1", lastModified = 1000)
102+
val file2 = createTempFile("file2", lastModified = 2000)
103+
val file3 = createTempFile("file3", lastModified = 1500)
104+
105+
val files = mutableListOf(file1, file2, file3)
106+
val sorter = FileSortOrderByDate(SORT_NEW_TO_OLD_ID, ascending = false)
107+
108+
val sorted = sorter.sortLocalFiles(files)
109+
110+
assertEquals(listOf(file2, file3, file1), sorted)
111+
}
112+
113+
@Test
114+
fun testSortLocalFilesNoConcurrentModification() {
115+
val files = mutableListOf(
116+
createTempFile("file1", lastModified = 1000),
117+
createTempFile("file2", lastModified = 2000),
118+
createTempFile("file3", lastModified = 1500)
119+
)
120+
val sorter = FileSortOrderByDate(SORT_OLD_TO_NEW_ID, ascending = true)
121+
122+
testConcurrentModification(files, sorter, iterations = 100)
123+
}
124+
125+
@Test
126+
fun testSortCloudFilesByDate() {
127+
val f1 = createOCFile("/123.txt", modTime = 1000)
128+
val f2 = createOCFile("/124.txt", modTime = 3000)
129+
val f3 = createOCFile("/125.txt", modTime = 2000)
130+
131+
val files = mutableListOf(f1, f2, f3)
132+
val sorter = FileSortOrderByDate(SORT_OLD_TO_NEW_ID, ascending = true)
133+
134+
val sorted = sorter.sortCloudFiles(files, foldersBeforeFiles = false, favoritesFirst = false)
135+
136+
assertEquals(listOf(f1, f3, f2), sorted)
137+
}
138+
139+
@Test
140+
fun testSortLocalFilesByNameAscending() {
141+
val folder = createTempFolder("folder")
142+
val file1 = createTempFile("apple")
143+
val file2 = createTempFile("banana")
144+
val file3 = createTempFile("cherry")
145+
146+
val files = mutableListOf(file3, folder, file1, file2)
147+
val sorter = FileSortOrderByName(SORT_A_TO_Z_ID, ascending = true)
148+
149+
val sorted = sorter.sortLocalFiles(files)
150+
151+
assertEquals(listOf(folder, file1, file2, file3), sorted)
152+
}
153+
154+
@Test
155+
fun testSortLocalFilesByNameDescending() {
156+
val file1 = createTempFile("apple")
157+
val file2 = createTempFile("banana")
158+
val file3 = createTempFile("cherry")
159+
160+
val files = mutableListOf(file1, file2, file3)
161+
val sorter = FileSortOrderByName(SORT_Z_TO_A_ID, ascending = false)
162+
163+
val sorted = sorter.sortLocalFiles(files)
164+
165+
assertEquals(listOf(file3, file2, file1), sorted)
166+
}
167+
168+
@Test
169+
fun testSortCloudFilesByName() {
170+
val f1 = createOCFile("/b.txt")
171+
val f2 = createOCFile("/a.txt")
172+
val f3 = createOCFile("/c.txt")
173+
174+
val files = mutableListOf(f1, f2, f3)
175+
val sorter = FileSortOrderByName(SORT_A_TO_Z_ID, ascending = true)
176+
177+
val sorted = sorter.sortCloudFiles(files, foldersBeforeFiles = false, favoritesFirst = false)
178+
179+
assertEquals(listOf(f2, f1, f3), sorted)
180+
}
181+
182+
@Test
183+
fun testSortLocalFilesByNameNoConcurrentModification() {
184+
val files = mutableListOf(
185+
createTempFile("apple"),
186+
createTempFile("banana"),
187+
createTempFile("cherry"),
188+
createTempFolder("folder")
189+
)
190+
val sorter = FileSortOrderByName(SORT_A_TO_Z_ID, ascending = true)
191+
192+
testConcurrentModification(files, sorter)
193+
}
194+
195+
@Test
196+
fun testSortLocalFilesBySizeAscending() {
197+
val file1 = createTempFile("file1", sizeBytes = 100)
198+
val file2 = createTempFile("file2", sizeBytes = 300)
199+
val file3 = createTempFile("file3", sizeBytes = 200)
200+
201+
val files = mutableListOf(file1, file2, file3)
202+
val sorter = FileSortOrderBySize(SORT_SMALL_TO_BIG_ID, ascending = true)
203+
204+
val sorted = sorter.sortLocalFiles(files)
205+
206+
assertEquals(listOf(file1, file3, file2), sorted)
207+
}
208+
209+
@Test
210+
fun testSortLocalFilesBySizeDescending() {
211+
val file1 = createTempFile("file1", sizeBytes = 100)
212+
val file2 = createTempFile("file2", sizeBytes = 300)
213+
val file3 = createTempFile("file3", sizeBytes = 200)
214+
val folder1 = createTempFolder("folder1")
215+
val folder2 = createTempFolder("folder2")
216+
217+
val files = mutableListOf(file1, file2, file3, folder1, folder2)
218+
val sorter = FileSortOrderBySize(SORT_BIG_TO_SMALL_ID, ascending = false)
219+
220+
val sorted = sorter.sortLocalFiles(files)
221+
222+
assertEquals(listOf(folder1, folder2, file2, file3, file1), sorted)
223+
}
224+
225+
@Test
226+
fun testSortCloudFilesBySize() {
227+
val f1 = createOCFile("/file1.txt", size = 100)
228+
val f2 = createOCFile("/file2.txt", size = 300)
229+
val f3 = createOCFile("/file3.txt", size = 200)
230+
231+
val files = mutableListOf(f1, f2, f3)
232+
val sorter = FileSortOrderBySize(SORT_SMALL_TO_BIG_ID, ascending = true)
233+
234+
val sorted = sorter.sortCloudFiles(files, foldersBeforeFiles = false, favoritesFirst = false)
235+
236+
assertEquals(listOf(f1, f3, f2), sorted)
237+
}
238+
239+
@Test
240+
fun testSortLocalFilesBySizeNoConcurrentModification() {
241+
val files = mutableListOf(
242+
createTempFile("file1", sizeBytes = 100),
243+
createTempFile("file2", sizeBytes = 200),
244+
createTempFile("file3", sizeBytes = 300),
245+
createTempFolder("folder")
246+
)
247+
val sorter = FileSortOrderBySize(SORT_SMALL_TO_BIG_ID, ascending = true)
248+
249+
testConcurrentModification(files, sorter)
250+
}
251+
}

‎app/src/main/java/com/owncloud/android/utils/FileSortOrderByDate.kt‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,11 @@ class FileSortOrderByDate(name: String, ascending: Boolean) : FileSortOrder(name
2626
favoritesFirst: Boolean
2727
): MutableList<OCFile> {
2828
val multiplier = if (isAscending) 1 else -1
29-
files.sortWith { o1: OCFile, o2: OCFile ->
29+
val copy = files.toMutableList()
30+
copy.sortWith { o1: OCFile, o2: OCFile ->
3031
multiplier * o1.modificationTimestamp.compareTo(o2.modificationTimestamp)
3132
}
32-
return super.sortCloudFiles(files, foldersBeforeFiles, favoritesFirst)
33+
return super.sortCloudFiles(copy, foldersBeforeFiles, favoritesFirst)
3334
}
3435

3536
/**
@@ -39,10 +40,11 @@ class FileSortOrderByDate(name: String, ascending: Boolean) : FileSortOrder(name
3940
*/
4041
override fun sortTrashbinFiles(files: MutableList<TrashbinFile>): List<TrashbinFile> {
4142
val multiplier = if (isAscending) 1 else -1
42-
files.sortWith { o1: TrashbinFile, o2: TrashbinFile ->
43+
val copy = files.toMutableList()
44+
copy.sortWith { o1: TrashbinFile, o2: TrashbinFile ->
4345
multiplier * o1.deletionTimestamp.compareTo(o2.deletionTimestamp)
4446
}
45-
return super.sortTrashbinFiles(files)
47+
return super.sortTrashbinFiles(copy)
4648
}
4749

4850
/**
@@ -52,9 +54,10 @@ class FileSortOrderByDate(name: String, ascending: Boolean) : FileSortOrder(name
5254
*/
5355
override fun sortLocalFiles(files: MutableList<File>): List<File> {
5456
val multiplier = if (isAscending) 1 else -1
55-
files.sortWith { o1: File, o2: File ->
57+
val copy = files.toMutableList()
58+
copy.sortWith { o1: File, o2: File ->
5659
multiplier * o1.lastModified().compareTo(o2.lastModified())
5760
}
58-
return files
61+
return copy
5962
}
6063
}

‎app/src/main/java/com/owncloud/android/utils/FileSortOrderByName.kt‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,8 @@ class FileSortOrderByName internal constructor(name: String?, ascending: Boolean
2929
foldersBeforeFiles: Boolean,
3030
favoritesFirst: Boolean
3131
): MutableList<OCFile> {
32-
val sortedByName = sortOnlyByName(files)
32+
val copy = files.toMutableList()
33+
val sortedByName = sortOnlyByName(copy)
3334
return super.sortCloudFiles(sortedByName, foldersBeforeFiles, favoritesFirst)
3435
}
3536

@@ -39,25 +40,28 @@ class FileSortOrderByName internal constructor(name: String?, ascending: Boolean
3940
* @param files files to sort
4041
*/
4142
override fun sortTrashbinFiles(files: MutableList<TrashbinFile>): List<TrashbinFile> {
42-
val sortedByName = sortServerFiles(files)
43+
val copy = files.toMutableList()
44+
val sortedByName = sortServerFiles(copy)
4345
return super.sortTrashbinFiles(sortedByName)
4446
}
4547

4648
private fun <T : ServerFileInterface> sortServerFiles(files: MutableList<T>): MutableList<T> {
47-
files.sortWith { o1: ServerFileInterface, o2: ServerFileInterface ->
49+
val copy = files.toMutableList()
50+
copy.sortWith { o1: ServerFileInterface, o2: ServerFileInterface ->
4851
when {
4952
o1.isFolder && o2.isFolder -> sortMultiplier * AlphanumComparator.compare(o1, o2)
5053
o1.isFolder -> -1
5154
o2.isFolder -> 1
5255
else -> sortMultiplier * AlphanumComparator.compare(o1, o2)
5356
}
5457
}
55-
return files
58+
return copy
5659
}
5760

5861
private fun sortOnlyByName(files: MutableList<OCFile>): MutableList<OCFile> {
59-
files.sortWith { o1: OCFile, o2: OCFile -> sortMultiplier * AlphanumComparator.compare(o1, o2) }
60-
return files
62+
val copy = files.toMutableList()
63+
copy.sortWith { o1: OCFile, o2: OCFile -> sortMultiplier * AlphanumComparator.compare(o1, o2) }
64+
return copy
6165
}
6266

6367
/**
@@ -66,7 +70,8 @@ class FileSortOrderByName internal constructor(name: String?, ascending: Boolean
6670
* @param files files to sort
6771
*/
6872
override fun sortLocalFiles(files: MutableList<File>): List<File> {
69-
files.sortWith { o1: File, o2: File ->
73+
val copy = files.toMutableList()
74+
copy.sortWith { o1: File, o2: File ->
7075
when {
7176
o1.isDirectory && o2.isDirectory -> sortMultiplier * o1.path.lowercase(Locale.getDefault())
7277
.compareTo(o2.path.lowercase(Locale.getDefault()))
@@ -78,6 +83,6 @@ class FileSortOrderByName internal constructor(name: String?, ascending: Boolean
7883
)
7984
}
8085
}
81-
return files
86+
return copy
8287
}
8388
}

0 commit comments

Comments
 (0)