Skip to content

Commit d7c1c73

Browse files
seadowgseadowg
authored
Merge pull request #6539 from grzesiek2010/COLLECT-6498
Optimize more entity filter expressions
2 parents 467c317 + 870c23d commit d7c1c73

File tree

17 files changed

+505
-258
lines changed

17 files changed

+505
-258
lines changed

‎collect_app/src/main/java/org/odk/collect/android/database/entities/DatabaseEntitiesRepository.kt

+55-64
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,12 @@ import org.odk.collect.db.sqlite.SQLiteDatabaseExt.doesColumnExist
1818
import org.odk.collect.db.sqlite.SQLiteDatabaseExt.getColumnNames
1919
import org.odk.collect.db.sqlite.SQLiteDatabaseExt.query
2020
import org.odk.collect.db.sqlite.SynchronizedDatabaseConnection
21+
import org.odk.collect.db.sqlite.toSql
22+
import org.odk.collect.entities.javarosa.parse.EntitySchema
2123
import org.odk.collect.entities.storage.EntitiesRepository
2224
import org.odk.collect.entities.storage.Entity
25+
import org.odk.collect.shared.Query
26+
import org.odk.collect.shared.mapColumns
2327

2428
private object ListsTable {
2529
const val TABLE_NAME = "lists"
@@ -155,12 +159,7 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep
155159
return emptyList()
156160
}
157161

158-
return queryWithAttachedRowId(list).foldAndClose {
159-
mapCursorRowToEntity(
160-
it,
161-
it.getInt(ROW_ID)
162-
)
163-
}
162+
return queryWithAttachedRowId(list, null)
164163
}
165164

166165
override fun getCount(list: String): Int {
@@ -208,18 +207,27 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep
208207
updateRowIdTables()
209208
}
210209

210+
override fun query(list: String, query: Query): List<Entity.Saved> {
211+
if (!listExists(list)) {
212+
return emptyList()
213+
}
214+
215+
return queryWithAttachedRowId(list, query.mapColumns { columnName ->
216+
when (columnName) {
217+
EntitySchema.ID -> EntitiesTable.COLUMN_ID
218+
EntitySchema.LABEL -> EntitiesTable.COLUMN_LABEL
219+
EntitySchema.VERSION -> EntitiesTable.COLUMN_VERSION
220+
else -> EntitiesTable.getPropertyColumn(columnName)
221+
}
222+
})
223+
}
224+
211225
override fun getById(list: String, id: String): Entity.Saved? {
212226
if (!listExists(list)) {
213227
return null
214228
}
215229

216-
return queryWithAttachedRowId(
217-
list,
218-
selectionColumn = EntitiesTable.COLUMN_ID,
219-
selectionArg = id
220-
).first {
221-
mapCursorRowToEntity(it, it.getInt(ROW_ID))
222-
}
230+
return queryWithAttachedRowId(list, Query.Eq(EntitiesTable.COLUMN_ID, id)).firstOrNull()
223231
}
224232

225233
override fun getAllByProperty(
@@ -238,15 +246,10 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep
238246
return if (propertyExists) {
239247
queryWithAttachedRowId(
240248
list,
241-
selectionColumn = EntitiesTable.getPropertyColumn(property),
242-
selectionArg = value
243-
).foldAndClose {
244-
mapCursorRowToEntity(it, it.getInt(ROW_ID))
245-
}
249+
Query.Eq(EntitiesTable.getPropertyColumn(property), value)
250+
)
246251
} else if (value == "") {
247-
queryWithAttachedRowId(list).foldAndClose {
248-
mapCursorRowToEntity(it, it.getInt(ROW_ID))
249-
}
252+
queryWithAttachedRowId(list, null)
250253
} else {
251254
emptyList()
252255
}
@@ -257,51 +260,39 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep
257260
return null
258261
}
259262

260-
return databaseConnection.withConnection {
261-
readableDatabase
262-
.rawQuery(
263-
"""
264-
SELECT *, i.$ROW_ID
265-
FROM "$list" e, "${getRowIdTableName(list)}" i
266-
WHERE e._id = i._id AND i.$ROW_ID = ?
267-
""".trimIndent(),
268-
arrayOf((index + 1).toString())
269-
).first {
270-
mapCursorRowToEntity(it, it.getInt(ROW_ID))
271-
}
272-
}
263+
return queryWithAttachedRowId(list, Query.Eq("i.$ROW_ID", (index + 1).toString())).firstOrNull()
273264
}
274265

275-
private fun queryWithAttachedRowId(list: String): Cursor {
276-
return databaseConnection.withConnection {
277-
readableDatabase
278-
.rawQuery(
279-
"""
280-
SELECT *, i.$ROW_ID
281-
FROM "$list" e, "${getRowIdTableName(list)}" i
282-
WHERE e._id = i._id
283-
ORDER BY i.$ROW_ID
284-
""".trimIndent(),
285-
null
286-
)
287-
}
288-
}
289-
290-
private fun queryWithAttachedRowId(
291-
list: String,
292-
selectionColumn: String,
293-
selectionArg: String
294-
): Cursor {
295-
return databaseConnection.withConnection {
296-
readableDatabase.rawQuery(
297-
"""
298-
SELECT *, i.$ROW_ID
299-
FROM "$list" e, "${getRowIdTableName(list)}" i
300-
WHERE e._id = i._id AND $selectionColumn = ?
301-
ORDER BY i.$ROW_ID
302-
""".trimIndent(),
303-
arrayOf(selectionArg)
304-
)
266+
private fun queryWithAttachedRowId(list: String, query: Query?): List<Entity.Saved> {
267+
return if (query == null) {
268+
databaseConnection.withConnection {
269+
readableDatabase
270+
.rawQuery(
271+
"""
272+
SELECT *, i.$ROW_ID
273+
FROM "$list" e, "${getRowIdTableName(list)}" i
274+
WHERE e._id = i._id
275+
ORDER BY i.$ROW_ID
276+
""".trimIndent(),
277+
null
278+
)
279+
}
280+
} else {
281+
databaseConnection.withConnection {
282+
val sqlQuery = query.toSql()
283+
readableDatabase
284+
.rawQuery(
285+
"""
286+
SELECT *, i.$ROW_ID
287+
FROM "$list" e, "${getRowIdTableName(list)}" i
288+
WHERE e._id = i._id AND ${sqlQuery.selection}
289+
ORDER BY i.$ROW_ID
290+
""".trimIndent(),
291+
sqlQuery.selectionArgs
292+
)
293+
}
294+
}.foldAndClose {
295+
mapCursorRowToEntity(it, it.getInt(ROW_ID))
305296
}
306297
}
307298

‎collect_app/src/test/java/org/odk/collect/android/entities/EntitiesRepositoryTest.kt

+77
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import org.junit.Test
88
import org.odk.collect.android.entities.support.EntitySameAsMatcher.Companion.sameEntityAs
99
import org.odk.collect.entities.storage.EntitiesRepository
1010
import org.odk.collect.entities.storage.Entity
11+
import org.odk.collect.shared.Query
1112

1213
abstract class EntitiesRepositoryTest {
1314

@@ -769,4 +770,80 @@ abstract class EntitiesRepositoryTest {
769770
assertThat(savedEntities[0].properties.size, equalTo(1))
770771
assertThat(savedEntities[0].properties[0].first, equalTo("prop"))
771772
}
773+
774+
@Test
775+
fun `#query returns matching entities`() {
776+
val repository = buildSubject()
777+
778+
val leoville = Entity.New(
779+
"1",
780+
"Léoville Barton 2008",
781+
version = 1,
782+
properties = listOf("vintage" to "2008")
783+
)
784+
785+
val canet = Entity.New(
786+
"2",
787+
"Pontet-Canet 2014",
788+
version = 2,
789+
properties = listOf("vintage" to "2009")
790+
)
791+
792+
repository.save("wines", leoville, canet)
793+
794+
val wines = repository.query("wines", Query.Eq("name", "2"))
795+
assertThat(wines, containsInAnyOrder(sameEntityAs(canet)))
796+
}
797+
798+
@Test
799+
fun `#query returns empty list when there are no matches`() {
800+
val repository = buildSubject()
801+
802+
val leoville = Entity.New(
803+
"1",
804+
"Léoville Barton 2008",
805+
version = 1,
806+
properties = listOf("vintage" to "2008")
807+
)
808+
809+
repository.save("wines", leoville)
810+
811+
val wines = repository.query("wines", Query.Eq("name", "3"))
812+
assertThat(wines, equalTo(emptyList()))
813+
}
814+
815+
@Test
816+
fun `#query returns empty list when there is a match in a different list`() {
817+
val repository = buildSubject()
818+
819+
val leoville = Entity.New("1", "Léoville Barton 2008")
820+
val ardbeg = Entity.New("2", "Ardbeg 10",)
821+
822+
repository.save("wines", leoville)
823+
repository.save("whisky", ardbeg)
824+
825+
assertThat(repository.query("wines", Query.Eq("label", "Ardbeg 10")), equalTo(emptyList()))
826+
}
827+
828+
@Test
829+
fun `#query returns empty list where there are no entities in the list`() {
830+
val repository = buildSubject()
831+
assertThat(repository.query("wines", Query.Eq("label", "Léoville Barton 2008")), equalTo(emptyList()))
832+
}
833+
834+
@Test
835+
fun `#query supports list names with dots and dashes`() {
836+
val repository = buildSubject()
837+
838+
val leoville = Entity.New("1", "Léoville Barton 2008")
839+
val canet = Entity.New("2", "Pontet-Canet 2014")
840+
repository.save("favourite-wines", leoville)
841+
repository.save("other.favourite.wines", canet)
842+
843+
val queriedLeoville = repository.query("favourite-wines", Query.Eq("label", "Léoville Barton 2008"))
844+
assertThat(queriedLeoville, containsInAnyOrder(sameEntityAs(leoville)))
845+
846+
val queriedCanet = repository.query("other.favourite.wines", Query.Eq("label", "Pontet-Canet 2014"))
847+
assertThat(queriedCanet, containsInAnyOrder(sameEntityAs(canet)))
848+
}
772849
}

‎db/src/main/java/org/odk/collect/db/sqlite/SqlQuery.kt

+31
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
package org.odk.collect.db.sqlite
2+
3+
import org.odk.collect.shared.Query
4+
5+
data class SqlQuery(
6+
val selection: String,
7+
val selectionArgs: Array<String>
8+
)
9+
10+
fun Query.toSql(): SqlQuery {
11+
return when (this) {
12+
is Query.Eq -> SqlQuery("$column = ?", arrayOf(value))
13+
is Query.NotEq -> SqlQuery("$column != ?", arrayOf(value))
14+
is Query.And -> {
15+
val sqlA = queryA.toSql()
16+
val sqlB = queryB.toSql()
17+
SqlQuery(
18+
"(${sqlA.selection} AND ${sqlB.selection})",
19+
sqlA.selectionArgs + sqlB.selectionArgs
20+
)
21+
}
22+
is Query.Or -> {
23+
val sqlA = queryA.toSql()
24+
val sqlB = queryB.toSql()
25+
SqlQuery(
26+
"(${sqlA.selection} OR ${sqlB.selection})",
27+
sqlA.selectionArgs + sqlB.selectionArgs
28+
)
29+
}
30+
}
31+
}

‎db/src/test/java/org/odk/collect/db/sqlite/SqlQueryTest.kt

+56
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
package org.odk.collect.db.sqlite
2+
3+
import org.hamcrest.CoreMatchers.equalTo
4+
import org.hamcrest.MatcherAssert.assertThat
5+
import org.junit.Test
6+
import org.odk.collect.shared.Query
7+
8+
class SqlQueryTest {
9+
@Test
10+
fun `Eq query generates correct selection and arguments`() {
11+
val query = Query.Eq("name", "John").toSql()
12+
13+
assertThat(query.selection, equalTo("name = ?"))
14+
assertThat(query.selectionArgs, equalTo(arrayOf("John")))
15+
}
16+
17+
@Test
18+
fun `NotEq query generates correct selection and arguments`() {
19+
val query = Query.NotEq("age", "30").toSql()
20+
21+
assertThat(query.selection, equalTo("age != ?"))
22+
assertThat(query.selectionArgs, equalTo(arrayOf("30")))
23+
}
24+
25+
@Test
26+
fun `And query generates correct selection and arguments`() {
27+
val queryA = Query.Eq("name", "John")
28+
val queryB = Query.NotEq("age", "30")
29+
val combinedQuery = Query.And(queryA, queryB).toSql()
30+
31+
assertThat(combinedQuery.selection, equalTo("(name = ? AND age != ?)"))
32+
assertThat(combinedQuery.selectionArgs, equalTo(arrayOf("John", "30")))
33+
}
34+
35+
@Test
36+
fun `Or query generates correct selection and arguments`() {
37+
val queryA = Query.Eq("city", "New York")
38+
val queryB = Query.NotEq("country", "Canada")
39+
val combinedQuery = Query.Or(queryA, queryB).toSql()
40+
41+
assertThat(combinedQuery.selection, equalTo("(city = ? OR country != ?)"))
42+
assertThat(combinedQuery.selectionArgs, equalTo(arrayOf("New York", "Canada")))
43+
}
44+
45+
@Test
46+
fun `nested And and Or queries generates correct selection and arguments`() {
47+
val queryA = Query.Eq("status", "active")
48+
val queryB = Query.NotEq("role", "admin")
49+
val queryC = Query.Eq("team", "engineering")
50+
51+
val combinedQuery = Query.And(Query.Or(queryA, queryB), queryC).toSql()
52+
53+
assertThat(combinedQuery.selection, equalTo("((status = ? OR role != ?) AND team = ?)"))
54+
assertThat(combinedQuery.selectionArgs, equalTo(arrayOf("active", "admin", "engineering")))
55+
}
56+
}

‎entities/src/main/java/org/odk/collect/entities/LocalEntityUseCases.kt

+4-4
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ package org.odk.collect.entities
33
import org.apache.commons.csv.CSVRecord
44
import org.javarosa.core.model.instance.SecondaryInstanceCSVParserBuilder
55
import org.odk.collect.entities.javarosa.finalization.EntitiesExtra
6-
import org.odk.collect.entities.javarosa.parse.EntityItemElement
6+
import org.odk.collect.entities.javarosa.parse.EntitySchema
77
import org.odk.collect.entities.javarosa.spec.EntityAction
88
import org.odk.collect.entities.storage.EntitiesRepository
99
import org.odk.collect.entities.storage.Entity
@@ -125,9 +125,9 @@ object LocalEntityUseCases {
125125
private fun parseEntityFromRecord(record: CSVRecord): ServerEntity? {
126126
val map = record.toMap()
127127

128-
val id = map.remove(EntityItemElement.ID)
129-
val label = map.remove(EntityItemElement.LABEL)
130-
val version = map.remove(EntityItemElement.VERSION)?.toInt()
128+
val id = map.remove(EntitySchema.ID)
129+
val label = map.remove(EntitySchema.LABEL)
130+
val version = map.remove(EntitySchema.VERSION)?.toInt()
131131
if (id == null || label == null || version == null) {
132132
return null
133133
}

0 commit comments

Comments
 (0)