Skip to content

Commit 19f8667

Browse files
ArnyminerZrfc2822
ArnyminerZ
and
rfc2822
authored
Implemented sort order for DAV documents (#1434)
* Implemented sort order Signed-off-by: Arnau Mora <arnyminerz@proton.me> * Fixed column name Signed-off-by: Arnau Mora <arnyminerz@proton.me> * Implemented sort order Signed-off-by: Arnau Mora <arnyminerz@proton.me> * Fixed column name Signed-off-by: Arnau Mora <arnyminerz@proton.me> * Improved issues Signed-off-by: Arnau Mora <arnyminerz@proton.me> * Add test for WebDavDocumentDao.getChildren for ORDER BY * Converted getChildren into a raw query Signed-off-by: Arnau Mora <arnyminerz@proton.me> * Fix formatting of SQL query in WebDavDocumentDao * Refactoring Signed-off-by: Arnau Mora <arnyminerz@proton.me> * Drop comment Signed-off-by: Arnau Mora <arnyminerz@proton.me> * Changed default sort to show directories first Signed-off-by: Arnau Mora <arnyminerz@proton.me> * Fixed tests Signed-off-by: Arnau Mora <arnyminerz@proton.me> * Switched to query constructor instead of in-place Signed-off-by: Arnau Mora <arnyminerz@proton.me> * Changed log method Signed-off-by: Arnau Mora <arnyminerz@proton.me> * Add DocumentSortByMapper to handle orderBy mapping for WebDavDocument queries * Rename DavDocumentsProviderTest to DocumentSortByMapperTest and update test method name * Refactor sorting and mapping * Add "order by name" as last criterion * Remove default sort order constant from DocumentSortByMapper and use WebDavDocumentDao.DEFAULT_ORDER instead * Adapt comments --------- Signed-off-by: Arnau Mora <arnyminerz@proton.me> Co-authored-by: Ricki Hirner <hirner@bitfire.at>
1 parent f74d14e commit 19f8667

File tree

7 files changed

+261
-8
lines changed

7 files changed

+261
-8
lines changed

‎app/src/androidTest/kotlin/at/bitfire/davdroid/db/WebDavDocumentDaoTest.kt

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
/*
2+
* Copyright © All Contributors. See LICENSE and AUTHORS in the root directory for details.
3+
*/
4+
5+
package at.bitfire.davdroid.db
6+
7+
import dagger.hilt.android.testing.HiltAndroidRule
8+
import dagger.hilt.android.testing.HiltAndroidTest
9+
import kotlinx.coroutines.test.runTest
10+
import okhttp3.HttpUrl.Companion.toHttpUrl
11+
import org.junit.Assert.assertEquals
12+
import org.junit.Before
13+
import org.junit.Rule
14+
import org.junit.Test
15+
import java.util.logging.Level
16+
import java.util.logging.Logger
17+
import javax.inject.Inject
18+
19+
@HiltAndroidTest
20+
class WebDavDocumentDaoTest {
21+
22+
@get:Rule
23+
val hiltRule = HiltAndroidRule(this)
24+
25+
@Inject
26+
lateinit var db: AppDatabase
27+
28+
@Inject
29+
lateinit var logger: Logger
30+
31+
@Before
32+
fun setUp() {
33+
hiltRule.inject()
34+
}
35+
36+
37+
@Test
38+
fun testGetChildren() = runTest {
39+
val mountDao = db.webDavMountDao()
40+
val dao = db.webDavDocumentDao()
41+
42+
val mount = WebDavMount(id = 1, name = "Test", url = "https://example.com/".toHttpUrl())
43+
db.webDavMountDao().insert(mount)
44+
45+
val root = WebDavDocument(
46+
id = 1,
47+
mountId = mount.id,
48+
parentId = null,
49+
name = "Root Document"
50+
)
51+
dao.insertOrReplace(root)
52+
dao.insertOrReplace(WebDavDocument(id = 0, mountId = mount.id, parentId = root.id, name = "Name 1", displayName = "DisplayName 2"))
53+
dao.insertOrReplace(WebDavDocument(id = 0, mountId = mount.id, parentId = root.id, name = "Name 2", displayName = "DisplayName 1"))
54+
dao.insertOrReplace(WebDavDocument(id = 0, mountId = mount.id, parentId = root.id, name = "Name 3", displayName = "Directory 1", isDirectory = true))
55+
try {
56+
dao.getChildren(root.id, orderBy = "name DESC").let { result ->
57+
logger.log(Level.INFO, "getChildren single sort Result", result)
58+
59+
assertEquals(listOf(
60+
"Name 3",
61+
"Name 2",
62+
"Name 1"
63+
), result.map { it.name })
64+
}
65+
66+
dao.getChildren(root.id, orderBy = "isDirectory DESC, name ASC").let { result ->
67+
logger.log(Level.INFO, "getChildren multiple sort Result", result)
68+
69+
assertEquals(listOf(
70+
"Name 3",
71+
"Name 1",
72+
"Name 2"
73+
), result.map { it.name })
74+
}
75+
} finally {
76+
mountDao.deleteAsync(mount)
77+
}
78+
}
79+
80+
}

‎app/src/androidTest/kotlin/at/bitfire/davdroid/webdav/DavDocumentsProviderTest.kt

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -101,9 +101,9 @@ class DavDocumentsProviderTest {
101101

102102
// Assert new children were inserted into db
103103
assertEquals(3, db.webDavDocumentDao().getChildren(parent.id).size)
104-
assertEquals("Secret_Document.pages", db.webDavDocumentDao().getChildren(parent.id)[0].displayName)
104+
assertEquals("Library", db.webDavDocumentDao().getChildren(parent.id)[0].displayName)
105105
assertEquals("MeowMeow_Cats.docx", db.webDavDocumentDao().getChildren(parent.id)[1].displayName)
106-
assertEquals("Library", db.webDavDocumentDao().getChildren(parent.id)[2].displayName)
106+
assertEquals("Secret_Document.pages", db.webDavDocumentDao().getChildren(parent.id)[2].displayName)
107107
}
108108

109109
@Test
@@ -137,8 +137,8 @@ class DavDocumentsProviderTest {
137137

138138
// Assert parent and children were updated in database
139139
assertEquals("Cats WebDAV", db.webDavDocumentDao().get(parent.id)!!.displayName)
140-
assertEquals("Library", db.webDavDocumentDao().getChildren(parent.id)[2].name)
141-
assertEquals("Library", db.webDavDocumentDao().getChildren(parent.id)[2].displayName)
140+
assertEquals("Library", db.webDavDocumentDao().getChildren(parent.id)[0].name)
141+
assertEquals("Library", db.webDavDocumentDao().getChildren(parent.id)[0].displayName)
142142

143143
}
144144

‎app/src/main/kotlin/at/bitfire/davdroid/db/WebDavDocument.kt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import java.time.Instant
3232
Index("parentId")
3333
]
3434
)
35+
// If any column name is modified, also change it in [DavDocumentsProvider$queryChildDocuments]
3536
data class WebDavDocument(
3637

3738
@PrimaryKey(autoGenerate = true)

‎app/src/main/kotlin/at/bitfire/davdroid/db/WebDavDocumentDao.kt

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ import androidx.room.Delete
99
import androidx.room.Insert
1010
import androidx.room.OnConflictStrategy
1111
import androidx.room.Query
12+
import androidx.room.RawQuery
13+
import androidx.room.RoomRawQuery
1214
import androidx.room.Transaction
1315
import androidx.room.Update
1416

@@ -21,8 +23,23 @@ interface WebDavDocumentDao {
2123
@Query("SELECT * FROM webdav_document WHERE mountId=:mountId AND (parentId=:parentId OR (parentId IS NULL AND :parentId IS NULL)) AND name=:name")
2224
fun getByParentAndName(mountId: Long, parentId: Long?, name: String): WebDavDocument?
2325

24-
@Query("SELECT * FROM webdav_document WHERE parentId=:parentId")
25-
fun getChildren(parentId: Long): List<WebDavDocument>
26+
@RawQuery
27+
fun query(query: RoomRawQuery): List<WebDavDocument>
28+
29+
/**
30+
* Gets all the child documents from a given parent id.
31+
*
32+
* @param parentId The id of the parent document to get the documents from.
33+
* @param orderBy If desired, a SQL clause to specify how to order the results.
34+
* **The caller is responsible for the correct formatting of this argument. Syntax won't be validated!**
35+
*/
36+
fun getChildren(parentId: Long, orderBy: String = DEFAULT_ORDER): List<WebDavDocument> {
37+
return query(
38+
RoomRawQuery("SELECT * FROM webdav_document WHERE parentId = ? ORDER BY $orderBy") {
39+
it.bindLong(1, parentId)
40+
}
41+
)
42+
}
2643

2744
@Insert(onConflict = OnConflictStrategy.REPLACE)
2845
fun insertOrReplace(document: WebDavDocument): Long
@@ -35,7 +52,7 @@ interface WebDavDocumentDao {
3552

3653
@Update
3754
fun update(document: WebDavDocument)
38-
55+
3956
@Delete
4057
fun delete(document: WebDavDocument)
4158

@@ -76,4 +93,15 @@ interface WebDavDocumentDao {
7693
return newDoc.copy(id = id)
7794
}
7895

96+
97+
companion object {
98+
99+
/**
100+
* Default ORDER BY value to use when content provider doesn't specify a sort order:
101+
* _sort by name (directories first)_
102+
*/
103+
const val DEFAULT_ORDER = "isDirectory DESC, name ASC"
104+
105+
}
106+
79107
}

‎app/src/main/kotlin/at/bitfire/davdroid/webdav/DavDocumentsProvider.kt

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ import at.bitfire.dav4jvm.property.webdav.ResourceType
4444
import at.bitfire.davdroid.R
4545
import at.bitfire.davdroid.db.AppDatabase
4646
import at.bitfire.davdroid.db.WebDavDocument
47+
import at.bitfire.davdroid.db.WebDavDocumentDao
4748
import at.bitfire.davdroid.di.IoDispatcher
4849
import at.bitfire.davdroid.network.HttpClient
4950
import at.bitfire.davdroid.network.MemoryCookieStore
@@ -95,6 +96,7 @@ class DavDocumentsProvider(
9596
interface DavDocumentsProviderEntryPoint {
9697
fun appDatabase(): AppDatabase
9798
fun davDocumentsActorFactory(): DavDocumentsActor.Factory
99+
fun documentSortByMapper(): DocumentSortByMapper
98100
fun logger(): Logger
99101
fun randomAccessCallbackWrapperFactory(): RandomAccessCallbackWrapper.Factory
100102
fun streamingFileDescriptorFactory(): StreamingFileDescriptor.Factory
@@ -127,6 +129,7 @@ class DavDocumentsProvider(
127129
fun notifyMountsChanged(context: Context) {
128130
context.contentResolver.notifyChange(buildRootsUri(context.getString(R.string.webdav_authority)), null)
129131
}
132+
130133
}
131134

132135
val documentProviderScope = CoroutineScope(SupervisorJob())
@@ -286,8 +289,16 @@ class DavDocumentsProvider(
286289
else // remove worker from list if done
287290
runningQueryChildren.remove(parentId)
288291

292+
// Prepare SORT BY clause
293+
val mapper = globalEntryPoint.documentSortByMapper()
294+
val sqlSortBy = if (sortOrder != null)
295+
mapper.mapContentProviderToSql(sortOrder)
296+
else
297+
WebDavDocumentDao.DEFAULT_ORDER
298+
289299
// Regardless of whether the worker is done, return the children we already have
290-
for (child in documentDao.getChildren(parentId)) {
300+
val children = documentDao.getChildren(parentId, sqlSortBy)
301+
for (child in children) {
291302
val bundle = child.toBundle(parent)
292303
result.addRow(bundle)
293304
}

‎app/src/main/kotlin/at/bitfire/davdroid/webdav/DocumentSortByMapper.kt

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
/*
2+
* Copyright © All Contributors. See LICENSE and AUTHORS in the root directory for details.
3+
*/
4+
5+
package at.bitfire.davdroid.webdav
6+
7+
import android.provider.DocumentsContract.Document
8+
import at.bitfire.davdroid.db.WebDavDocument
9+
import java.util.logging.Logger
10+
import javax.inject.Inject
11+
12+
class DocumentSortByMapper @Inject constructor(
13+
private val logger: Logger
14+
) {
15+
16+
/**
17+
* Contains a map that maps the column names from the documents provider to [WebDavDocument].
18+
*/
19+
private val columnsMap = mapOf(
20+
Document.COLUMN_DOCUMENT_ID to "id",
21+
Document.COLUMN_DISPLAY_NAME to "displayName",
22+
Document.COLUMN_MIME_TYPE to "mimeType",
23+
Document.COLUMN_SIZE to "size",
24+
Document.COLUMN_LAST_MODIFIED to "lastModified"
25+
)
26+
27+
/**
28+
* Maps an incoming `orderBy` column from a [android.content.ContentProvider] query to
29+
* a validated SQL ORDER BY-clause for [WebDavDocument]s.
30+
*
31+
* @param orderBy orderBy of content provider documents
32+
*
33+
* @return value of the ORDER BY-clause, like "name ASC"
34+
*/
35+
fun mapContentProviderToSql(orderBy: String): String {
36+
val requestedFields = orderBy
37+
// Split by commas to divide each order column
38+
.split(',')
39+
// Trim any leading or trailing spaces
40+
.map { it.trim() }
41+
42+
// Map incoming orderBy fields to a list of pairs of column and direction (true for ASC), like
43+
// [ Pair("displayName", Boolean), … ]
44+
val requestedCriteria = mutableListOf<Pair<String, Boolean>>()
45+
for (field in requestedFields) {
46+
val idx = field.indexOfFirst { it == ' ' }
47+
if (idx == -1)
48+
// no whitespace, only name → use ASC as default, like in SQL
49+
requestedCriteria += field to true
50+
else {
51+
// whitespace, name and sort order
52+
val name = field.substring(0, idx)
53+
val directionStr = field.substring(idx).trim()
54+
val ascending = directionStr.equals("ASC", true)
55+
requestedCriteria += name to ascending
56+
}
57+
}
58+
59+
// If displayName doesn't appear in sort order, append it in case that the other criteria generate
60+
// the same order. For instance, if files are ordered by ascending size and all have 0 bytes, then
61+
// another order by name is useful.
62+
if (!requestedCriteria.any { it.first == Document.COLUMN_DISPLAY_NAME })
63+
requestedCriteria += Document.COLUMN_DISPLAY_NAME to true
64+
65+
// Generate SQL
66+
val sqlSortBy = mutableListOf<String>() // list of valid SQL ORDER BY elements like "displayName ASC"
67+
for ((requestedColumn, ascending) in requestedCriteria) {
68+
// Only take columns that are registered in the columns map
69+
val sqlFieldName = columnsMap[requestedColumn]
70+
if (sqlFieldName == null) {
71+
logger.warning("Ignoring unknown column in sortOrder: $requestedColumn")
72+
continue
73+
}
74+
75+
// Finally, convert the column name from document to room, including the sort direction.
76+
sqlSortBy += "$sqlFieldName ${if (ascending) "ASC" else "DESC"}"
77+
}
78+
return sqlSortBy.joinToString(", ")
79+
}
80+
81+
}

‎app/src/test/kotlin/at/bitfire/davdroid/webdav/DocumentSortByMapperTest.kt

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
/*
2+
* Copyright © All Contributors. See LICENSE and AUTHORS in the root directory for details.
3+
*/
4+
5+
package at.bitfire.davdroid.webdav
6+
7+
import android.provider.DocumentsContract.Document
8+
import junit.framework.TestCase.assertEquals
9+
import org.junit.Test
10+
import java.util.logging.Logger
11+
12+
class DocumentSortByMapperTest {
13+
14+
private val mapper = DocumentSortByMapper(Logger.getGlobal())
15+
16+
@Test
17+
fun test_MapContentProviderToSql() {
18+
// Valid column without direction
19+
assertEquals(
20+
"displayName ASC",
21+
mapper.mapContentProviderToSql(Document.COLUMN_DISPLAY_NAME)
22+
)
23+
// Valid column with direction
24+
assertEquals(
25+
"displayName DESC",
26+
mapper.mapContentProviderToSql("${Document.COLUMN_DISPLAY_NAME} DESC")
27+
)
28+
// Valid column with direction and multiple spaces
29+
assertEquals(
30+
"displayName ASC",
31+
mapper.mapContentProviderToSql("${Document.COLUMN_DISPLAY_NAME} ASC")
32+
)
33+
// Invalid column without direction
34+
assertEquals(
35+
"displayName ASC",
36+
mapper.mapContentProviderToSql("invalid")
37+
)
38+
// Invalid column with direction
39+
assertEquals(
40+
"displayName ASC",
41+
mapper.mapContentProviderToSql("invalid ASC")
42+
)
43+
// Valid and invalid columns with and without directions
44+
assertEquals(
45+
"displayName ASC, mimeType DESC",
46+
mapper.mapContentProviderToSql(
47+
"${Document.COLUMN_DISPLAY_NAME}, invalid DESC, ${Document.COLUMN_MIME_TYPE} DESC"
48+
)
49+
)
50+
}
51+
52+
}

0 commit comments

Comments
 (0)