Skip to content

Commit

Permalink
Merge pull request #6495 from seadowg/prop-prefix
Browse files Browse the repository at this point in the history 
Fix property names clashing with DB column names
  • Loading branch information
@grzesiek2010
grzesiek2010 authored Nov 6, 2024
2 parents e54753f + f0a4c3e commit 90d41ae
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 36 deletions.
84 changes: 48 additions & 36 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 @@ -34,6 +34,9 @@ private object EntitiesTable {
const val COLUMN_TRUNK_VERSION = "trunk_version"
const val COLUMN_BRANCH_ID = "branch_id"
const val COLUMN_STATE = "state"
const val COLUMN_PROPERTY_PREFIX = "p_"

fun getPropertyColumn(property: String) = "$COLUMN_PROPERTY_PREFIX$property"
}

class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRepository {
Expand All @@ -43,7 +46,7 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep
dbPath,
"entities.db",
EntitiesDatabaseMigrator(),
1
DATABASE_VERSION
)

override fun save(list: String, vararg entities: Entity) {
Expand All @@ -65,7 +68,7 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep
list,
"${EntitiesTable.COLUMN_ID} = ?",
arrayOf(entity.id)
).first { mapCursorRowToEntity(list, it, 0) }
).first { mapCursorRowToEntity(it, 0) }
} else {
null
}
Expand Down Expand Up @@ -120,9 +123,7 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep

override fun getLists(): Set<String> {
return databaseConnection.withConnection {
readableDatabase
.query(ListsTable.TABLE_NAME)
.foldAndClose(emptySet()) { set, cursor -> set + cursor.getString(ListsTable.COLUMN_NAME) }
getListsFromDB(readableDatabase)
}
}

Expand Down Expand Up @@ -156,7 +157,6 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep

return queryWithAttachedRowId(list).foldAndClose {
mapCursorRowToEntity(
list,
it,
it.getInt(ROW_ID)
)
Expand All @@ -183,11 +183,7 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep

override fun clear() {
databaseConnection.withConnection {
getLists().forEach {
writableDatabase.delete(it)
}

writableDatabase.delete(ListsTable.TABLE_NAME)
dropAllTablesFromDB(writableDatabase)
}
}

Expand Down Expand Up @@ -222,7 +218,7 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep
selectionColumn = EntitiesTable.COLUMN_ID,
selectionArg = id
).first {
mapCursorRowToEntity(list, it, it.getInt(ROW_ID))
mapCursorRowToEntity(it, it.getInt(ROW_ID))
}
}

Expand All @@ -236,20 +232,20 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep
}

val propertyExists = databaseConnection.withConnection {
readableDatabase.doesColumnExist(list, property)
readableDatabase.doesColumnExist(list, EntitiesTable.getPropertyColumn(property))
}

return if (propertyExists) {
queryWithAttachedRowId(
list,
selectionColumn = property,
selectionColumn = EntitiesTable.getPropertyColumn(property),
selectionArg = value
).foldAndClose {
mapCursorRowToEntity(list, it, it.getInt(ROW_ID))
mapCursorRowToEntity(it, it.getInt(ROW_ID))
}
} else if (value == "") {
queryWithAttachedRowId(list).foldAndClose {
mapCursorRowToEntity(list, it, it.getInt(ROW_ID))
mapCursorRowToEntity(it, it.getInt(ROW_ID))
}
} else {
emptyList()
Expand All @@ -271,7 +267,7 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep
""".trimIndent(),
arrayOf((index + 1).toString())
).first {
mapCursorRowToEntity(list, it, it.getInt(ROW_ID))
mapCursorRowToEntity(it, it.getInt(ROW_ID))
}
}
}
Expand Down Expand Up @@ -384,8 +380,10 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep
readableDatabase.getColumnNames(list)
}

val missingColumns =
entity.properties.map { it.first }.filterNot { columnNames.contains(it) }
val missingColumns = entity.properties
.map { EntitiesTable.getPropertyColumn(it.first) }
.filterNot { columnNames.contains(it) }

if (missingColumns.isNotEmpty()) {
databaseConnection.resetTransaction {
missingColumns.forEach {
Expand All @@ -399,29 +397,28 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep
}
}

private fun addPropertiesToContentValues(contentValues: ContentValues, entity: Entity) {
entity.properties.forEach { (name, value) ->
contentValues.put("\"${EntitiesTable.getPropertyColumn(name)}\"", value)
}
}

private fun mapCursorRowToEntity(
list: String,
cursor: Cursor,
rowId: Int
): Entity.Saved {
val map = cursor.rowToMap()

val propertyColumns = map.keys.filter {
!listOf(
_ID,
EntitiesTable.COLUMN_ID,
EntitiesTable.COLUMN_LABEL,
EntitiesTable.COLUMN_VERSION,
EntitiesTable.COLUMN_TRUNK_VERSION,
EntitiesTable.COLUMN_BRANCH_ID,
EntitiesTable.COLUMN_STATE,
ROW_ID
).contains(it)
it.startsWith(EntitiesTable.COLUMN_PROPERTY_PREFIX)
}

val properties =
propertyColumns.fold(emptyList<Pair<String, String>>()) { accum, property ->
accum + Pair(property, map[property] ?: "")
accum + Pair(
property.removePrefix(EntitiesTable.COLUMN_PROPERTY_PREFIX),
map[property] ?: ""
)
}

val state = if (map[EntitiesTable.COLUMN_STATE]!!.toInt() == 0) {
Expand Down Expand Up @@ -453,15 +450,14 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep
}
}

private fun addPropertiesToContentValues(contentValues: ContentValues, entity: Entity) {
entity.properties.forEach { (name, value) ->
contentValues.put("\"$name\"", value)
}
companion object {
private const val DATABASE_VERSION = 2
}
}

private class EntitiesDatabaseMigrator :
DatabaseMigrator {

override fun onCreate(db: SQLiteDatabase) {
db.execSQL(
"""
Expand All @@ -474,5 +470,21 @@ private class EntitiesDatabaseMigrator :
)
}

override fun onUpgrade(db: SQLiteDatabase?, oldVersion: Int) = Unit
override fun onUpgrade(db: SQLiteDatabase, oldVersion: Int) {
dropAllTablesFromDB(db)
}
}

private fun dropAllTablesFromDB(db: SQLiteDatabase) {
getListsFromDB(db).forEach {
db.delete(it)
}

db.delete(ListsTable.TABLE_NAME)
}

private fun getListsFromDB(db: SQLiteDatabase): Set<String> {
return db
.query(ListsTable.TABLE_NAME)
.foldAndClose(emptySet()) { set, cursor -> set + cursor.getString(ListsTable.COLUMN_NAME) }
}
21 changes: 21 additions & 0 deletions collect_app/src/test/java/org/odk/collect/android/entities/DatabaseEntitiesRepositoryTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,13 @@ package org.odk.collect.android.entities

import androidx.test.core.app.ApplicationProvider
import androidx.test.ext.junit.runners.AndroidJUnit4
import org.hamcrest.MatcherAssert.assertThat
import org.junit.Test
import org.junit.runner.RunWith
import org.odk.collect.android.database.entities.DatabaseEntitiesRepository
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.shared.TempFiles

@RunWith(AndroidJUnit4::class)
Expand All @@ -15,4 +19,21 @@ class DatabaseEntitiesRepositoryTest : EntitiesRepositoryTest() {
TempFiles.createTempDir().absolutePath
)
}

@Test
fun `#save supports properties with db column names saving new entities and updating existing ones`() {
val repository = buildSubject()
val entity = Entity.New(
"1",
"One",
properties = listOf(Pair("_id", "value"), Pair("version", "value"))
)

repository.save("things", entity)
val savedEntity = repository.getEntities("things")[0]
assertThat(savedEntity, sameEntityAs(entity))

repository.save("things", savedEntity)
assertThat(repository.getEntities("things")[0], sameEntityAs(savedEntity))
}
}

0 comments on commit 90d41ae

Please sign in to comment.