Implemented sort order for DAV documents

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

@@ -0,0 +1,80 @@
+/*
+ * Copyright © All Contributors. See LICENSE and AUTHORS in the root directory for details.
+ */
+
+package at.bitfire.davdroid.db
+
+import dagger.hilt.android.testing.HiltAndroidRule
+import dagger.hilt.android.testing.HiltAndroidTest
+import kotlinx.coroutines.test.runTest
+import okhttp3.HttpUrl.Companion.toHttpUrl
+import org.junit.Assert.assertEquals
+import org.junit.Before
+import org.junit.Rule
+import org.junit.Test
+import java.util.logging.Level
+import java.util.logging.Logger
+import javax.inject.Inject
+
+@HiltAndroidTest
+class WebDavDocumentDaoTest {
+
+    @get:Rule
+    val hiltRule = HiltAndroidRule(this)
+
+    @Inject
+    lateinit var db: AppDatabase
+
+    @Inject
+    lateinit var logger: Logger
+
+    @Before
+    fun setUp() {
+        hiltRule.inject()
+    }
+
+
+    @Test
+    fun testGetChildren() = runTest {
+        val mountDao = db.webDavMountDao()
+        val dao = db.webDavDocumentDao()
+
+        val mount = WebDavMount(id = 1, name = "Test", url = "https://example.com/".toHttpUrl())
+        db.webDavMountDao().insert(mount)
+
+        val root = WebDavDocument(
+            id = 1,
+            mountId = mount.id,
+            parentId = null,
+            name = "Root Document"
+        )
+        dao.insertOrReplace(root)
+        dao.insertOrReplace(WebDavDocument(id = 0, mountId = mount.id, parentId = root.id, name = "Name 1", displayName = "DisplayName 2"))
+        dao.insertOrReplace(WebDavDocument(id = 0, mountId = mount.id, parentId = root.id, name = "Name 2", displayName = "DisplayName 1"))
+        dao.insertOrReplace(WebDavDocument(id = 0, mountId = mount.id, parentId = root.id, name = "Name 3", displayName = "Directory 1", isDirectory = true))
+        try {
+            dao.getChildren(root.id, orderBy = "name DESC").let { result ->
+                logger.log(Level.INFO, "getChildren single sort Result", result)
+
+                assertEquals(listOf(
+                    "Name 3",
+                    "Name 2",
+                    "Name 1"
+                ), result.map { it.name })
+            }
+
+            dao.getChildren(root.id, orderBy = "isDirectory DESC, name ASC").let { result ->
+                logger.log(Level.INFO, "getChildren multiple sort Result", result)
+
+                assertEquals(listOf(
+                    "Name 3",
+                    "Name 1",
+                    "Name 2"
+                ), result.map { it.name })
+            }
+        } finally {
+            mountDao.deleteAsync(mount)
+        }
+    }
+
+}

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

@@ -101,9 +101,9 @@ class DavDocumentsProviderTest {
 
         // Assert new children were inserted into db
         assertEquals(3, db.webDavDocumentDao().getChildren(parent.id).size)
-        assertEquals("Secret_Document.pages", db.webDavDocumentDao().getChildren(parent.id)[0].displayName)
+        assertEquals("Library", db.webDavDocumentDao().getChildren(parent.id)[0].displayName)
         assertEquals("MeowMeow_Cats.docx", db.webDavDocumentDao().getChildren(parent.id)[1].displayName)
-        assertEquals("Library", db.webDavDocumentDao().getChildren(parent.id)[2].displayName)
+        assertEquals("Secret_Document.pages", db.webDavDocumentDao().getChildren(parent.id)[2].displayName)
     }
 
     @Test
@@ -137,8 +137,8 @@ class DavDocumentsProviderTest {
 
         // Assert parent and children were updated in database
         assertEquals("Cats WebDAV", db.webDavDocumentDao().get(parent.id)!!.displayName)
-        assertEquals("Library", db.webDavDocumentDao().getChildren(parent.id)[2].name)
-        assertEquals("Library", db.webDavDocumentDao().getChildren(parent.id)[2].displayName)
+        assertEquals("Library", db.webDavDocumentDao().getChildren(parent.id)[0].name)
+        assertEquals("Library", db.webDavDocumentDao().getChildren(parent.id)[0].displayName)
 
     }
 

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

@@ -32,6 +32,7 @@ import java.time.Instant
         Index("parentId")
     ]
 )
+// If any column name is modified, also change it in [DavDocumentsProvider$queryChildDocuments]
 data class WebDavDocument(
 
     @PrimaryKey(autoGenerate = true)

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

@@ -9,6 +9,8 @@ import androidx.room.Delete
 import androidx.room.Insert
 import androidx.room.OnConflictStrategy
 import androidx.room.Query
+import androidx.room.RawQuery
+import androidx.room.RoomRawQuery
 import androidx.room.Transaction
 import androidx.room.Update
 
@@ -21,8 +23,23 @@ interface WebDavDocumentDao {
     @Query("SELECT * FROM webdav_document WHERE mountId=:mountId AND (parentId=:parentId OR (parentId IS NULL AND :parentId IS NULL)) AND name=:name")
     fun getByParentAndName(mountId: Long, parentId: Long?, name: String): WebDavDocument?
 
-    @Query("SELECT * FROM webdav_document WHERE parentId=:parentId")
-    fun getChildren(parentId: Long): List<WebDavDocument>
+    @RawQuery
+    fun query(query: RoomRawQuery): List<WebDavDocument>
+
+    /**
+     * Gets all the child documents from a given parent id.
+     *
+     * @param parentId The id of the parent document to get the documents from.
+     * @param orderBy If desired, a SQL clause to specify how to order the results.
+     *                **The caller is responsible for the correct formatting of this argument. Syntax won't be validated!**
+     */
+    fun getChildren(parentId: Long, orderBy: String = DEFAULT_ORDER): List<WebDavDocument> {
+        return query(
+            RoomRawQuery("SELECT * FROM webdav_document WHERE parentId = ? ORDER BY $orderBy") {
+                it.bindLong(1, parentId)
+            }
+        )
+    }
 
     @Insert(onConflict = OnConflictStrategy.REPLACE)
     fun insertOrReplace(document: WebDavDocument): Long
@@ -35,7 +52,7 @@ interface WebDavDocumentDao {
 
     @Update
     fun update(document: WebDavDocument)
-    
+
     @Delete
     fun delete(document: WebDavDocument)
 
@@ -76,4 +93,15 @@ interface WebDavDocumentDao {
         return newDoc.copy(id = id)
     }
 
+
+    companion object {
+
+        /**
+         * Default ORDER BY value to use when content provider doesn't specify a sort order:
+         * _sort by name (directories first)_
+         */
+        const val DEFAULT_ORDER = "isDirectory DESC, name ASC"
+
+    }
+
 }

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

@@ -44,6 +44,7 @@ import at.bitfire.dav4jvm.property.webdav.ResourceType
 import at.bitfire.davdroid.R
 import at.bitfire.davdroid.db.AppDatabase
 import at.bitfire.davdroid.db.WebDavDocument
+import at.bitfire.davdroid.db.WebDavDocumentDao
 import at.bitfire.davdroid.di.IoDispatcher
 import at.bitfire.davdroid.network.HttpClient
 import at.bitfire.davdroid.network.MemoryCookieStore
@@ -95,6 +96,7 @@ class DavDocumentsProvider(
     interface DavDocumentsProviderEntryPoint {
         fun appDatabase(): AppDatabase
         fun davDocumentsActorFactory(): DavDocumentsActor.Factory
+        fun documentSortByMapper(): DocumentSortByMapper
         fun logger(): Logger
         fun randomAccessCallbackWrapperFactory(): RandomAccessCallbackWrapper.Factory
         fun streamingFileDescriptorFactory(): StreamingFileDescriptor.Factory
@@ -127,6 +129,7 @@ class DavDocumentsProvider(
         fun notifyMountsChanged(context: Context) {
             context.contentResolver.notifyChange(buildRootsUri(context.getString(R.string.webdav_authority)), null)
         }
+
     }
 
     val documentProviderScope = CoroutineScope(SupervisorJob())
@@ -286,8 +289,16 @@ class DavDocumentsProvider(
         else                // remove worker from list if done
             runningQueryChildren.remove(parentId)
 
+        // Prepare SORT BY clause
+        val mapper = globalEntryPoint.documentSortByMapper()
+        val sqlSortBy = if (sortOrder != null)
+            mapper.mapContentProviderToSql(sortOrder)
+        else
+            WebDavDocumentDao.DEFAULT_ORDER
+
         // Regardless of whether the worker is done, return the children we already have
-        for (child in documentDao.getChildren(parentId)) {
+        val children = documentDao.getChildren(parentId, sqlSortBy)
+        for (child in children) {
             val bundle = child.toBundle(parent)
             result.addRow(bundle)
         }

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

@@ -0,0 +1,81 @@
+/*
+ * Copyright © All Contributors. See LICENSE and AUTHORS in the root directory for details.
+ */
+
+package at.bitfire.davdroid.webdav
+
+import android.provider.DocumentsContract.Document
+import at.bitfire.davdroid.db.WebDavDocument
+import java.util.logging.Logger
+import javax.inject.Inject
+
+class DocumentSortByMapper @Inject constructor(
+    private val logger: Logger
+) {
+
+    /**
+     * Contains a map that maps the column names from the documents provider to [WebDavDocument].
+     */
+    private val columnsMap = mapOf(
+        Document.COLUMN_DOCUMENT_ID to "id",
+        Document.COLUMN_DISPLAY_NAME to "displayName",
+        Document.COLUMN_MIME_TYPE to "mimeType",
+        Document.COLUMN_SIZE to "size",
+        Document.COLUMN_LAST_MODIFIED to "lastModified"
+    )
+
+    /**
+     * Maps an incoming `orderBy` column from a [android.content.ContentProvider] query to
+     * a validated SQL ORDER BY-clause for [WebDavDocument]s.
+     *
+     * @param orderBy   orderBy of content provider documents
+     *
+     * @return value of the ORDER BY-clause, like "name ASC"
+     */
+    fun mapContentProviderToSql(orderBy: String): String {
+        val requestedFields = orderBy
+            // Split by commas to divide each order column
+            .split(',')
+            // Trim any leading or trailing spaces
+            .map { it.trim() }
+
+        // Map incoming orderBy fields to a list of pairs of column and direction (true for ASC), like
+        // [ Pair("displayName", Boolean), … ]
+        val requestedCriteria = mutableListOf<Pair<String, Boolean>>()
+        for (field in requestedFields) {
+            val idx = field.indexOfFirst { it == ' ' }
+            if (idx == -1)
+                // no whitespace, only name → use ASC as default, like in SQL
+                requestedCriteria += field to true
+            else {
+                // whitespace, name and sort order
+                val name = field.substring(0, idx)
+                val directionStr = field.substring(idx).trim()
+                val ascending = directionStr.equals("ASC", true)
+                requestedCriteria += name to ascending
+            }
+        }
+
+        // If displayName doesn't appear in sort order, append it in case that the other criteria generate
+        // the same order. For instance, if files are ordered by ascending size and all have 0 bytes, then
+        // another order by name is useful.
+        if (!requestedCriteria.any { it.first == Document.COLUMN_DISPLAY_NAME })
+            requestedCriteria += Document.COLUMN_DISPLAY_NAME to true
+
+        // Generate SQL
+        val sqlSortBy = mutableListOf<String>()     // list of valid SQL ORDER BY elements like "displayName ASC"
+        for ((requestedColumn, ascending) in requestedCriteria) {
+            // Only take columns that are registered in the columns map
+            val sqlFieldName = columnsMap[requestedColumn]
+            if (sqlFieldName == null) {
+                logger.warning("Ignoring unknown column in sortOrder: $requestedColumn")
+                continue
+            }
+
+            // Finally, convert the column name from document to room, including the sort direction.
+            sqlSortBy += "$sqlFieldName ${if (ascending) "ASC" else "DESC"}"
+        }
+        return sqlSortBy.joinToString(", ")
+    }
+
+}

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

@@ -0,0 +1,52 @@
+/*
+ * Copyright © All Contributors. See LICENSE and AUTHORS in the root directory for details.
+ */
+
+package at.bitfire.davdroid.webdav
+
+import android.provider.DocumentsContract.Document
+import junit.framework.TestCase.assertEquals
+import org.junit.Test
+import java.util.logging.Logger
+
+class DocumentSortByMapperTest {
+
+    private val mapper = DocumentSortByMapper(Logger.getGlobal())
+
+    @Test
+    fun test_MapContentProviderToSql() {
+        // Valid column without direction
+        assertEquals(
+            "displayName ASC",
+            mapper.mapContentProviderToSql(Document.COLUMN_DISPLAY_NAME)
+        )
+        // Valid column with direction
+        assertEquals(
+            "displayName DESC",
+            mapper.mapContentProviderToSql("${Document.COLUMN_DISPLAY_NAME} DESC")
+        )
+        // Valid column with direction and multiple spaces
+        assertEquals(
+            "displayName ASC",
+            mapper.mapContentProviderToSql("${Document.COLUMN_DISPLAY_NAME}   ASC")
+        )
+        // Invalid column without direction
+        assertEquals(
+            "displayName ASC",
+            mapper.mapContentProviderToSql("invalid")
+        )
+        // Invalid column with direction
+        assertEquals(
+            "displayName ASC",
+            mapper.mapContentProviderToSql("invalid ASC")
+        )
+        // Valid and invalid columns with and without directions
+        assertEquals(
+            "displayName ASC, mimeType DESC",
+            mapper.mapContentProviderToSql(
+                "${Document.COLUMN_DISPLAY_NAME}, invalid DESC, ${Document.COLUMN_MIME_TYPE} DESC"
+            )
+        )
+    }
+
+}