Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix list names with special characters #6520

Merged
merged 3 commits into from
Nov 20, 2024
Merged

Fix list names with special characters #6520

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cda25c5
Fix list names with special characters
seadowg Nov 20, 2024
4810d08
Make sure all queries uses quoted list names
seadowg Nov 20, 2024
e742585
Fix adding new properties to lists with special characters
seadowg Nov 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 13 additions & 13 deletions ...app/src/main/java/org/odk/collect/android/database/entities/DatabaseEntitiesRepository.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep
entities.forEach { entity ->
val existing = if (listExists) {
query(
list,
"\"$list\"",
"${EntitiesTable.COLUMN_ID} = ?",
arrayOf(entity.id)
).first { mapCursorRowToEntity(it, 0) }
Expand All @@ -92,7 +92,7 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep
}

update(
list,
"\"$list\"",
contentValues,
"${EntitiesTable.COLUMN_ID} = ?",
arrayOf(entity.id)
Expand All @@ -110,7 +110,7 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep
}

insertOrThrow(
list,
"\"$list\"",
null,
contentValues
)
Expand Down Expand Up @@ -172,7 +172,7 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep
readableDatabase.rawQuery(
"""
SELECT COUNT(*)
FROM $list
FROM "$list"
""".trimIndent(),
null
).first {
Expand Down Expand Up @@ -262,7 +262,7 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep
.rawQuery(
"""
SELECT *, i.$ROW_ID
FROM $list e, ${getRowIdTableName(list)} i
FROM "$list" e, "${getRowIdTableName(list)}" i
WHERE e._id = i._id AND i.$ROW_ID = ?
""".trimIndent(),
arrayOf((index + 1).toString())
Expand All @@ -278,7 +278,7 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep
.rawQuery(
"""
SELECT *, i.$ROW_ID
FROM $list e, ${getRowIdTableName(list)} i
FROM "$list" e, "${getRowIdTableName(list)}" i
WHERE e._id = i._id
ORDER BY i.$ROW_ID
""".trimIndent(),
Expand All @@ -296,7 +296,7 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep
readableDatabase.rawQuery(
"""
SELECT *, i.$ROW_ID
FROM $list e, ${getRowIdTableName(list)} i
FROM "$list" e, "${getRowIdTableName(list)}" i
WHERE e._id = i._id AND $selectionColumn = ?
ORDER BY i.$ROW_ID
""".trimIndent(),
Expand All @@ -317,13 +317,13 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep
getLists().forEach {
writableDatabase.execSQL(
"""
DROP TABLE IF EXISTS ${getRowIdTableName(it)};
DROP TABLE IF EXISTS "${getRowIdTableName(it)}";
""".trimIndent()
)

writableDatabase.execSQL(
"""
CREATE TABLE ${getRowIdTableName(it)} AS SELECT _id FROM $it ORDER BY _id;
CREATE TABLE "${getRowIdTableName(it)}" AS SELECT _id FROM "$it" ORDER BY _id;
""".trimIndent()
)
}
Expand Down Expand Up @@ -355,7 +355,7 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep

execSQL(
"""
CREATE TABLE IF NOT EXISTS $list (
CREATE TABLE IF NOT EXISTS "$list" (
$_ID integer PRIMARY KEY,
${EntitiesTable.COLUMN_ID} text,
${EntitiesTable.COLUMN_LABEL} text,
Expand All @@ -369,15 +369,15 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep

execSQL(
"""
CREATE UNIQUE INDEX IF NOT EXISTS ${list}_unique_id_index ON $list (${EntitiesTable.COLUMN_ID});
CREATE UNIQUE INDEX IF NOT EXISTS "${list}_unique_id_index" ON "$list" (${EntitiesTable.COLUMN_ID});
""".trimIndent()
)
}
}

private fun updatePropertyColumns(list: String, entity: Entity) {
val columnNames = databaseConnection.withConnection {
readableDatabase.getColumnNames(list)
readableDatabase.getColumnNames("\"$list\"")
}

val missingColumns = entity.properties
Expand All @@ -390,7 +390,7 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep
missingColumns.forEach {
execSQL(
"""
ALTER TABLE $list ADD "$it" text NOT NULL DEFAULT "";
ALTER TABLE "$list" ADD "$it" text NOT NULL DEFAULT "";
""".trimIndent()
)
}
Expand Down
90 changes: 90 additions & 0 deletions collect_app/src/test/java/org/odk/collect/android/entities/EntitiesRepositoryTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,31 @@ abstract class EntitiesRepositoryTest {
assertThat(wines[0].properties, contains("window" to "2019-2038", "score" to "92"))
}

@Test
fun `#save adds new properties for lists with dashes`() {
val repository = buildSubject()

val wine = Entity.New(
"1",
"Léoville Barton 2008",
properties = listOf("window" to "2019-2038"),
version = 1
)
repository.save("favourite-wines", wine)

val updatedWine = Entity.New(
wine.id,
"Léoville Barton 2008",
properties = listOf("score" to "92"),
version = 2
)
repository.save("favourite-wines", updatedWine)

val wines = repository.getEntities("favourite-wines")
assertThat(wines.size, equalTo(1))
assertThat(wines[0].properties, contains("window" to "2019-2038", "score" to "92"))
}

@Test
fun `#save adds new properties to existing entities`() {
val repository = buildSubject()
Expand Down Expand Up @@ -275,6 +300,19 @@ abstract class EntitiesRepositoryTest {
assertThat(repository.getLists(), equalTo(emptySet()))
}

@Test
fun `#save supports creating list names with with dots and dashes`() {
val repository = buildSubject()

val wine = Entity.New("1", "Léoville Barton 2008")

repository.save("favourite-wines", wine)
assertThat(repository.getEntities("favourite-wines")[0], sameEntityAs(wine))

repository.save("favourite.wines", wine)
assertThat(repository.getEntities("favourite.wines")[0], sameEntityAs(wine))
}

@Test
fun `#clear deletes all entities`() {
val repository = buildSubject()
Expand Down Expand Up @@ -458,6 +496,25 @@ abstract class EntitiesRepositoryTest {
assertThat(repository.getById("wines", "3"), equalTo(null))
}

@Test
fun `#getById supports list names with dots and dashes`() {
val repository = buildSubject()

val leoville = Entity.New("1", "Léoville Barton 2008")
val canet = Entity.New("2", "Pontet-Canet 2014")
repository.save("favourite-wines", leoville)
repository.save("other.favourite.wines", canet)

val favouriteWines = repository.getEntities("favourite-wines")
val otherFavouriteWines = repository.getEntities("other.favourite.wines")

val queriedLeoville = repository.getById("favourite-wines", "1")
assertThat(queriedLeoville, equalTo(favouriteWines.first { it.id == "1" }))

val queriedCanet = repository.getById("other.favourite.wines", "2")
assertThat(queriedCanet, equalTo(otherFavouriteWines.first { it.id == "2" }))
}

@Test
fun `#getByAllByProperty returns entities with matching property value`() {
val repository = buildSubject()
Expand Down Expand Up @@ -610,6 +667,21 @@ abstract class EntitiesRepositoryTest {
assertThat(repository.getCount("whiskys"), equalTo(1))
}

@Test
fun `#getCount supports list names with dots and dashes`() {
val repository = buildSubject()

val leoville = Entity.New("1", "Léoville Barton 2008")
val dows = Entity.New("2", "Dow's 1983")
repository.save("favourite-wines", leoville, dows)

val springbank = Entity.New("1", "Springbank 10")
repository.save("favourite.whiskys", springbank)

assertThat(repository.getCount("favourite-wines"), equalTo(2))
assertThat(repository.getCount("favourite.whiskys"), equalTo(1))
}

@Test
fun `#getByIndex returns matching entity`() {
val repository = buildSubject()
Expand All @@ -636,6 +708,24 @@ abstract class EntitiesRepositoryTest {
assertThat(repository.getByIndex("wine", 0), equalTo(null))
}

@Test
fun `#getByIndex supports list names with dots and dashes`() {
val repository = buildSubject()

val leoville = Entity.New("1", "Léoville Barton 2008")
val canet = Entity.New("2", "Pontet-Canet 2014")
repository.save("favourite-wines", leoville)
repository.save("other.favourite.wines", canet)

val leovilleIndex =
repository.getEntities("favourite-wines").first { it.id == leoville.id }.index
assertThat(repository.getByIndex("favourite-wines", leovilleIndex), sameEntityAs(leoville))

val canetIndex =
repository.getEntities("other.favourite.wines").first { it.id == canet.id }.index
assertThat(repository.getByIndex("other.favourite.wines", canetIndex), sameEntityAs(canet))
}

@Test
fun `#getListVersion returns list version`() {
val repository = buildSubject()
Expand Down