Decouple query structure from SQL translation logic

getodk · Dec 19, 2024 · 3766a57 · 3766a57
1 parent 8e9c25f
commit 3766a57
Show file tree

Hide file tree

Showing 13 changed files with 80 additions and 59 deletions.
diff --git a/...app/src/main/java/org/odk/collect/android/database/entities/DatabaseEntitiesRepository.kt b/...app/src/main/java/org/odk/collect/android/database/entities/DatabaseEntitiesRepository.kt
@@ -12,16 +12,18 @@ import org.odk.collect.db.sqlite.CursorExt.getString
 import org.odk.collect.db.sqlite.CursorExt.getStringOrNull
 import org.odk.collect.db.sqlite.CursorExt.rowToMap
 import org.odk.collect.db.sqlite.DatabaseMigrator
-import org.odk.collect.db.sqlite.Query
 import org.odk.collect.db.sqlite.SQLiteColumns.ROW_ID
 import org.odk.collect.db.sqlite.SQLiteDatabaseExt.delete
 import org.odk.collect.db.sqlite.SQLiteDatabaseExt.doesColumnExist
 import org.odk.collect.db.sqlite.SQLiteDatabaseExt.getColumnNames
 import org.odk.collect.db.sqlite.SQLiteDatabaseExt.query
 import org.odk.collect.db.sqlite.SynchronizedDatabaseConnection
+import org.odk.collect.db.sqlite.toSql
 import org.odk.collect.entities.javarosa.parse.EntityItemElement
 import org.odk.collect.entities.storage.EntitiesRepository
 import org.odk.collect.entities.storage.Entity
+import org.odk.collect.shared.Query
+import org.odk.collect.shared.mapColumns
 
 private object ListsTable {
     const val TABLE_NAME = "lists"
@@ -210,7 +212,7 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep
             return emptyList()
         }
 
-        return queryWithAttachedRowId(list, query?.copyWithMappedColumns { columnName ->
+        return queryWithAttachedRowId(list, query?.mapColumns { columnName ->
             when (columnName) {
                 EntityItemElement.ID -> EntitiesTable.COLUMN_ID
                 EntityItemElement.LABEL -> EntitiesTable.COLUMN_LABEL
@@ -277,15 +279,16 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep
             }
         } else {
             databaseConnection.withConnection {
+                val sqlQuery = query.toSql()
                 readableDatabase
                     .rawQuery(
                         """
                         SELECT *, i.$ROW_ID
                         FROM "$list" e, "${getRowIdTableName(list)}" i
-                        WHERE e._id = i._id AND ${query.selection}
+                        WHERE e._id = i._id AND ${sqlQuery.selection}
                         ORDER BY i.$ROW_ID
                         """.trimIndent(),
-                        query.selectionArgs
+                        sqlQuery.selectionArgs
                     )
             }
         }.foldAndClose {

diff --git a/collect_app/src/test/java/org/odk/collect/android/entities/EntitiesRepositoryTest.kt b/collect_app/src/test/java/org/odk/collect/android/entities/EntitiesRepositoryTest.kt
@@ -6,7 +6,7 @@ import org.hamcrest.Matchers.containsInAnyOrder
 import org.hamcrest.Matchers.equalTo
 import org.junit.Test
 import org.odk.collect.android.entities.support.EntitySameAsMatcher.Companion.sameEntityAs
-import org.odk.collect.db.sqlite.Query
+import org.odk.collect.shared.Query
 import org.odk.collect.entities.storage.EntitiesRepository
 import org.odk.collect.entities.storage.Entity
 

diff --git a/db/src/main/java/org/odk/collect/db/sqlite/Query.kt b/db/src/main/java/org/odk/collect/db/sqlite/Query.kt
diff --git a/db/src/main/java/org/odk/collect/db/sqlite/SqlQuery.kt b/db/src/main/java/org/odk/collect/db/sqlite/SqlQuery.kt
@@ -0,0 +1,31 @@
+package org.odk.collect.db.sqlite
+
+import org.odk.collect.shared.Query
+
+data class SqlQuery(
+    val selection: String,
+    val selectionArgs: Array<String>
+)
+
+fun Query.toSql(): SqlQuery {
+    return when (this) {
+        is Query.Eq -> SqlQuery("$column = ?", arrayOf(value))
+        is Query.NotEq -> SqlQuery("$column != ?", arrayOf(value))
+        is Query.And -> {
+            val sqlA = queryA.toSql()
+            val sqlB = queryB.toSql()
+            SqlQuery(
+                "(${sqlA.selection} AND ${sqlB.selection})",
+                sqlA.selectionArgs + sqlB.selectionArgs
+            )
+        }
+        is Query.Or -> {
+            val sqlA = queryA.toSql()
+            val sqlB = queryB.toSql()
+            SqlQuery(
+                "(${sqlA.selection} OR ${sqlB.selection})",
+                sqlA.selectionArgs + sqlB.selectionArgs
+            )
+        }
+    }
+}
diff --git a/...va/org/odk/collect/db/sqlite/QueryTest.kt → ...org/odk/collect/db/sqlite/SqlQueryTest.kt b/...va/org/odk/collect/db/sqlite/QueryTest.kt → ...org/odk/collect/db/sqlite/SqlQueryTest.kt
@@ -3,19 +3,20 @@ package org.odk.collect.db.sqlite
 import org.hamcrest.CoreMatchers.equalTo
 import org.hamcrest.MatcherAssert.assertThat
 import org.junit.Test
+import org.odk.collect.shared.Query
 
-class QueryTest {
+class SqlQueryTest {
     @Test
     fun `Eq query generates correct selection and arguments`() {
-        val query = Query.Eq("name", "John")
+        val query = Query.Eq("name", "John").toSql()
 
         assertThat(query.selection, equalTo("name = ?"))
         assertThat(query.selectionArgs, equalTo(arrayOf("John")))
     }
 
     @Test
     fun `NotEq query generates correct selection and arguments`() {
-        val query = Query.NotEq("age", "30")
+        val query = Query.NotEq("age", "30").toSql()
 
         assertThat(query.selection, equalTo("age != ?"))
         assertThat(query.selectionArgs, equalTo(arrayOf("30")))
@@ -25,7 +26,7 @@ class QueryTest {
     fun `And query generates correct selection and arguments`() {
         val queryA = Query.Eq("name", "John")
         val queryB = Query.NotEq("age", "30")
-        val combinedQuery = Query.And(queryA, queryB)
+        val combinedQuery = Query.And(queryA, queryB).toSql()
 
         assertThat(combinedQuery.selection, equalTo("(name = ? AND age != ?)"))
         assertThat(combinedQuery.selectionArgs, equalTo(arrayOf("John", "30")))
@@ -35,7 +36,7 @@ class QueryTest {
     fun `Or query generates correct selection and arguments`() {
         val queryA = Query.Eq("city", "New York")
         val queryB = Query.NotEq("country", "Canada")
-        val combinedQuery = Query.Or(queryA, queryB)
+        val combinedQuery = Query.Or(queryA, queryB).toSql()
 
         assertThat(combinedQuery.selection, equalTo("(city = ? OR country != ?)"))
         assertThat(combinedQuery.selectionArgs, equalTo(arrayOf("New York", "Canada")))
@@ -47,7 +48,7 @@ class QueryTest {
         val queryB = Query.NotEq("role", "admin")
         val queryC = Query.Eq("team", "engineering")
 
-        val combinedQuery = Query.And(Query.Or(queryA, queryB), queryC)
+        val combinedQuery = Query.And(Query.Or(queryA, queryB), queryC).toSql()
 
         assertThat(combinedQuery.selection, equalTo("((status = ? OR role != ?) AND team = ?)"))
         assertThat(combinedQuery.selectionArgs, equalTo(arrayOf("active", "admin", "engineering")))

diff --git a/entities/build.gradle.kts b/entities/build.gradle.kts
@@ -51,7 +51,6 @@ dependencies {
     implementation(project(":material"))
     implementation(project(":async"))
     implementation(project(":lists"))
-    implementation(project(":db"))
 
     implementation(libs.kotlinStdlib)
     implementation(libs.javarosa) {

diff --git a/...ies/src/main/java/org/odk/collect/entities/javarosa/filter/LocalEntitiesFilterStrategy.kt b/...ies/src/main/java/org/odk/collect/entities/javarosa/filter/LocalEntitiesFilterStrategy.kt
@@ -8,10 +8,10 @@ import org.javarosa.core.model.instance.TreeReference
 import org.javarosa.xpath.expr.XPathBoolExpr
 import org.javarosa.xpath.expr.XPathEqExpr
 import org.javarosa.xpath.expr.XPathExpression
-import org.odk.collect.db.sqlite.Query
 import org.odk.collect.entities.javarosa.intance.LocalEntitiesInstanceAdapter
 import org.odk.collect.entities.javarosa.intance.LocalEntitiesInstanceProvider
 import org.odk.collect.entities.storage.EntitiesRepository
+import org.odk.collect.shared.Query
 import java.util.function.Supplier
 
 /**

diff --git a/entities/src/main/java/org/odk/collect/entities/javarosa/filter/PullDataFunctionHandler.kt b/entities/src/main/java/org/odk/collect/entities/javarosa/filter/PullDataFunctionHandler.kt
@@ -3,9 +3,9 @@ package org.odk.collect.entities.javarosa.filter
 import org.javarosa.core.model.condition.EvaluationContext
 import org.javarosa.core.model.condition.IFunctionHandler
 import org.javarosa.xpath.expr.XPathFuncExpr
-import org.odk.collect.db.sqlite.Query
 import org.odk.collect.entities.javarosa.intance.LocalEntitiesInstanceAdapter
 import org.odk.collect.entities.storage.EntitiesRepository
+import org.odk.collect.shared.Query
 
 class PullDataFunctionHandler(
     entitiesRepository: EntitiesRepository,

diff --git a/...s/src/main/java/org/odk/collect/entities/javarosa/intance/LocalEntitiesInstanceAdapter.kt b/...s/src/main/java/org/odk/collect/entities/javarosa/intance/LocalEntitiesInstanceAdapter.kt
@@ -2,10 +2,10 @@ package org.odk.collect.entities.javarosa.intance
 
 import org.javarosa.core.model.data.StringData
 import org.javarosa.core.model.instance.TreeElement
-import org.odk.collect.db.sqlite.Query
 import org.odk.collect.entities.javarosa.parse.EntityItemElement
 import org.odk.collect.entities.storage.EntitiesRepository
 import org.odk.collect.entities.storage.Entity
+import org.odk.collect.shared.Query
 
 class LocalEntitiesInstanceAdapter(private val entitiesRepository: EntitiesRepository) {
 

diff --git a/entities/src/main/java/org/odk/collect/entities/storage/EntitiesRepository.kt b/entities/src/main/java/org/odk/collect/entities/storage/EntitiesRepository.kt
@@ -1,6 +1,6 @@
 package org.odk.collect.entities.storage
 
-import org.odk.collect.db.sqlite.Query
+import org.odk.collect.shared.Query
 
 interface EntitiesRepository {
     fun save(list: String, vararg entities: Entity)

diff --git a/entities/src/main/java/org/odk/collect/entities/storage/InMemEntitiesRepository.kt b/entities/src/main/java/org/odk/collect/entities/storage/InMemEntitiesRepository.kt
@@ -1,7 +1,7 @@
 package org.odk.collect.entities.storage
 
-import org.odk.collect.db.sqlite.Query
 import org.odk.collect.entities.javarosa.parse.EntityItemElement
+import org.odk.collect.shared.Query
 
 class InMemEntitiesRepository : EntitiesRepository {
 

diff --git a/entities/src/test/java/org/odk/collect/entities/LocalEntityUseCasesTest.kt b/entities/src/test/java/org/odk/collect/entities/LocalEntityUseCasesTest.kt
@@ -8,14 +8,14 @@ import org.hamcrest.Matchers.containsInAnyOrder
 import org.hamcrest.Matchers.equalTo
 import org.hamcrest.Matchers.not
 import org.junit.Test
-import org.odk.collect.db.sqlite.Query
 import org.odk.collect.entities.javarosa.finalization.EntitiesExtra
 import org.odk.collect.entities.javarosa.finalization.FormEntity
 import org.odk.collect.entities.javarosa.parse.EntityItemElement
 import org.odk.collect.entities.javarosa.spec.EntityAction
 import org.odk.collect.entities.storage.EntitiesRepository
 import org.odk.collect.entities.storage.Entity
 import org.odk.collect.entities.storage.InMemEntitiesRepository
+import org.odk.collect.shared.Query
 import org.odk.collect.shared.TempFiles
 import java.io.File
 

diff --git a/shared/src/main/java/org/odk/collect/shared/Query.kt b/shared/src/main/java/org/odk/collect/shared/Query.kt
@@ -0,0 +1,28 @@
+package org.odk.collect.shared
+
+import org.odk.collect.shared.Query.And
+import org.odk.collect.shared.Query.Eq
+import org.odk.collect.shared.Query.NotEq
+import org.odk.collect.shared.Query.Or
+
+sealed class Query {
+    class Eq(val column: String, val value: String) : Query()
+    class NotEq(val column: String, val value: String) : Query()
+    class And(val queryA: Query, val queryB: Query) : Query()
+    class Or(val queryA: Query, val queryB: Query) : Query()
+}
+
+fun Query.mapColumns(columnMapper: (String) -> String): Query {
+    return when (this) {
+        is Eq -> Eq(columnMapper(column), value)
+        is NotEq -> NotEq(columnMapper(column), value)
+        is And -> And(
+            queryA.mapColumns(columnMapper),
+            queryB.mapColumns(columnMapper)
+        )
+        is Or -> Or(
+            queryA.mapColumns(columnMapper),
+            queryB.mapColumns(columnMapper)
+        )
+    }
+}