Skip to content

Commit

Permalink
Merge pull request #6601 from grzesiek2010/COLLECT-6600
Browse files Browse the repository at this point in the history 
Catch sqlite exceptions when filter entities
  • Loading branch information
@seadowg
seadowg authored Feb 21, 2025
2 parents 572b326 + eb66fa8 commit 532c842
Show file tree
Hide file tree
Showing 6 changed files with 214 additions and 31 deletions.
62 changes: 34 additions & 28 deletions ...app/src/main/java/org/odk/collect/android/database/entities/DatabaseEntitiesRepository.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import android.content.ContentValues
import android.content.Context
import android.database.Cursor
import android.database.sqlite.SQLiteDatabase
import android.database.sqlite.SQLiteException
import android.provider.BaseColumns._ID
import org.odk.collect.db.sqlite.CursorExt.first
import org.odk.collect.db.sqlite.CursorExt.foldAndClose
Expand All @@ -22,6 +23,7 @@ import org.odk.collect.db.sqlite.toSql
import org.odk.collect.entities.javarosa.parse.EntitySchema
import org.odk.collect.entities.storage.EntitiesRepository
import org.odk.collect.entities.storage.Entity
import org.odk.collect.entities.storage.QueryException
import org.odk.collect.shared.Query
import org.odk.collect.shared.mapColumns

Expand Down Expand Up @@ -258,35 +260,39 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep
}

private fun queryWithAttachedRowId(list: String, query: Query?): List<Entity.Saved> {
return if (query == null) {
databaseConnection.withConnection {
readableDatabase
.rawQuery(
"""
SELECT *, i.$ROW_ID
FROM "$list" e, "${getRowIdTableName(list)}" i
WHERE e._id = i._id
ORDER BY i.$ROW_ID
""".trimIndent(),
null
)
}
} 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 ${sqlQuery.selection}
ORDER BY i.$ROW_ID
""".trimIndent(),
sqlQuery.selectionArgs
)
try {
return if (query == null) {
databaseConnection.withConnection {
readableDatabase
.rawQuery(
"""
SELECT *, i.$ROW_ID
FROM "$list" e, "${getRowIdTableName(list)}" i
WHERE e._id = i._id
ORDER BY i.$ROW_ID
""".trimIndent(),
null
)
}
} 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 ${sqlQuery.selection}
ORDER BY i.$ROW_ID
""".trimIndent(),
sqlQuery.selectionArgs
)
}
}.foldAndClose {
mapCursorRowToEntity(it, it.getInt(ROW_ID))
}
}.foldAndClose {
mapCursorRowToEntity(it, it.getInt(ROW_ID))
} catch (e: SQLiteException) {
throw QueryException(e.message)
}
}

Expand Down
9 changes: 9 additions & 0 deletions collect_app/src/test/java/org/odk/collect/android/entities/EntitiesRepositoryTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import org.junit.Test
import org.odk.collect.android.entities.support.EntitySameAsMatcher.Companion.sameEntityAs
import org.odk.collect.entities.storage.EntitiesRepository
import org.odk.collect.entities.storage.Entity
import org.odk.collect.entities.storage.QueryException
import org.odk.collect.shared.Query

abstract class EntitiesRepositoryTest {
Expand Down Expand Up @@ -872,4 +873,12 @@ abstract class EntitiesRepositoryTest {
val queriedCanet = repository.query("other.favourite.wines", Query.Eq("label", "Pontet-Canet 2014"))
assertThat(queriedCanet, containsInAnyOrder(sameEntityAs(canet)))
}

@Test(expected = QueryException::class)
fun `#query throws an exception when not existing property is used`() {
val repository = buildSubject()
repository.save("wines", Entity.New("1", "Léoville Barton 2008",))

repository.query("wines", Query.Eq("score", "92"))
}
}
7 changes: 6 additions & 1 deletion ...ies/src/main/java/org/odk/collect/entities/javarosa/filter/LocalEntitiesFilterStrategy.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import org.javarosa.xpath.expr.XPathExpression
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.entities.storage.QueryException
import org.odk.collect.shared.Query
import java.util.function.Supplier

Expand Down Expand Up @@ -41,7 +42,11 @@ class LocalEntitiesFilterStrategy(entitiesRepository: EntitiesRepository) :
val query = xPathExpressionToQuery(predicate, sourceInstance, evaluationContext)

return if (query != null) {
queryToTreeReferences(query, sourceInstance)
try {
queryToTreeReferences(query, sourceInstance)
} catch (e: QueryException) {
next.get()
}
} else {
next.get()
}
Expand Down
4 changes: 2 additions & 2 deletions entities/src/main/java/org/odk/collect/entities/storage/InMemEntitiesRepository.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ class InMemEntitiesRepository : EntitiesRepository {
EntitySchema.VERSION -> it.version.toString()
else -> it.properties.find { propertyName ->
propertyName.first == query.column
}?.second
}?.second ?: throw QueryException("No such column: ${query.column}")
}
fieldName == query.value
}
Expand All @@ -69,7 +69,7 @@ class InMemEntitiesRepository : EntitiesRepository {
EntitySchema.VERSION -> it.version.toString()
else -> it.properties.find { propertyName ->
propertyName.first == query.column
}?.second
}?.second ?: throw QueryException("No such column: ${query.column}")
}
fieldName != query.value
}
Expand Down
3 changes: 3 additions & 0 deletions entities/src/main/java/org/odk/collect/entities/storage/QueryException.kt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
package org.odk.collect.entities.storage

class QueryException(message: String?) : RuntimeException(message)
160 changes: 160 additions & 0 deletions ...src/test/java/org/odk/collect/entities/javarosa/filter/LocalEntitiesFilterStrategyTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -541,6 +541,166 @@ class LocalEntitiesFilterStrategyTest {
assertThat(fallthroughFilterStrategy.fellThrough, equalTo(false))
assertThat(instanceProvider.fullParsePerformed, equalTo(false))
}

@Test
fun `works correctly but not in the optimized way with unanswered question = ''`() {
entitiesRepository.save("things", Entity.New("thing1", "Thing1"))

val scenario = Scenario.init(
"Secondary instance form",
html(
head(
title("Secondary instance form"),
model(
mainInstance(
t(
"data id=\"create-entity-form\"",
t("ref_question"),
t("question")
)
),
t("instance id=\"things\" src=\"jr://file-csv/things.csv\""),
bind("/data/ref_question").type("string"),
bind("/data/question").type("string")
)
),
body(
input("/data/ref_question"),
select1Dynamic(
"/data/question",
"instance('things')/root/item[/data/ref_question='']",
"name",
"label"
)
)
),
controllerSupplier
)

val choices = scenario.choicesOf("/data/question").map { it.value }
assertThat(choices, containsInAnyOrder("thing1"))

assertThat(fallthroughFilterStrategy.fellThrough, equalTo(true))
}

@Test
fun `works correctly but not in the optimized way with answered question = value`() {
entitiesRepository.save("things", Entity.New("thing1", "Thing1"))

val scenario = Scenario.init(
"Secondary instance form",
html(
head(
title("Secondary instance form"),
model(
mainInstance(
t(
"data id=\"create-entity-form\"",
t("ref_question"),
t("question")
)
),
t("instance id=\"things\" src=\"jr://file-csv/things.csv\""),
bind("/data/ref_question").type("string"),
bind("/data/question").type("string")
)
),
body(
input("/data/ref_question"),
select1Dynamic(
"/data/question",
"instance('things')/root/item[/data/ref_question='']",
"name",
"label"
)
)
),
controllerSupplier
)
scenario.next()
scenario.answer("blah")

val choices = scenario.choicesOf("/data/question").map { it.value }
assertThat(choices.isEmpty(), equalTo(true))

assertThat(fallthroughFilterStrategy.fellThrough, equalTo(true))
}

@Test
fun `works correctly but not in the optimized way with non existing property = ''`() {
entitiesRepository.save("things", Entity.New("thing1", "Thing1"))

val scenario = Scenario.init(
"Secondary instance form",
html(
head(
title("Secondary instance form"),
model(
mainInstance(
t(
"data id=\"create-entity-form\"",
t("question")
)
),
t("instance id=\"things\" src=\"jr://file-csv/things.csv\""),
bind("/data/question").type("string")
)
),
body(
select1Dynamic(
"/data/question",
"instance('things')/root/item[not_existing_property='']",
"name",
"label"
)
)
),
controllerSupplier
)

val choices = scenario.choicesOf("/data/question").map { it.value }
assertThat(choices, containsInAnyOrder("thing1"))

assertThat(fallthroughFilterStrategy.fellThrough, equalTo(true))
}

@Test
fun `works correctly but not in the optimized way with non existing property = value`() {
entitiesRepository.save("things", Entity.New("thing1", "Thing1"))

val scenario = Scenario.init(
"Secondary instance form",
html(
head(
title("Secondary instance form"),
model(
mainInstance(
t(
"data id=\"create-entity-form\"",
t("question")
)
),
t("instance id=\"things\" src=\"jr://file-csv/things.csv\""),
bind("/data/question").type("string")
)
),
body(
select1Dynamic(
"/data/question",
"instance('things')/root/item[not_existing_property='value']",
"name",
"label"
)
)
),
controllerSupplier
)

val choices = scenario.choicesOf("/data/question").map { it.value }
assertThat(choices.isEmpty(), equalTo(true))

assertThat(fallthroughFilterStrategy.fellThrough, equalTo(true))
}
}

private class FallthroughFilterStrategy : FilterStrategy {
Expand Down

0 comments on commit 532c842

Please sign in to comment.