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

Jump to bottom

Fix bad queries after adding new propreties to entity list #6402

Merged
merged 9 commits into from
Sep 10, 2024
Merged

Fix bad queries after adding new propreties to entity list #6402

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
2e391bd
Spike out resetting DB connection after ALTER TABLE
seadowg Sep 9, 2024
3f26073
Synchronize access to the entities DB connection
seadowg Sep 9, 2024
b4d7766
Synchronize SQLiteOpenHelper creation
seadowg Sep 9, 2024
d99a4a7
Add comments
seadowg Sep 9, 2024
ed6f5ed
Only reset DB connection if adding columns
seadowg Sep 9, 2024
24da9b6
Add columns in one transaction rather than many
seadowg Sep 9, 2024
1cc2540
Add helpers to reduce indenting
seadowg Sep 9, 2024
4128ec4
Replace SQLiteUtils.getColumnNames
seadowg Sep 9, 2024
a154665
Fix typo
seadowg Sep 10, 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
199 changes: 112 additions & 87 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 @@ -4,21 +4,20 @@ 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 androidx.core.database.sqlite.transaction
import org.odk.collect.db.sqlite.CursorExt.first
import org.odk.collect.db.sqlite.CursorExt.foldAndClose
import org.odk.collect.db.sqlite.CursorExt.getInt
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.DatabaseConnection
import org.odk.collect.db.sqlite.DatabaseMigrator
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.entities.storage.EntitiesRepository
import org.odk.collect.entities.storage.Entity

Expand All @@ -39,13 +38,12 @@ private object EntitiesTable {

class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRepository {

private val databaseConnection: DatabaseConnection = DatabaseConnection(
private val databaseConnection = SynchronizedDatabaseConnection(
context,
dbPath,
"entities.db",
EntitiesDatabaseMigrator(),
1,
true
1
)

override fun save(list: String, vararg entities: Entity) {
Expand All @@ -60,7 +58,7 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep

updatePropertyColumns(list, entities.first())

databaseConnection.writeableDatabase.transaction {
databaseConnection.transaction {
entities.forEach { entity ->
val existing = if (listExists) {
query(
Expand Down Expand Up @@ -121,32 +119,34 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep
}

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

override fun updateListHash(list: String, hash: String) {
val contentValues = ContentValues().also {
it.put(ListsTable.COLUMN_HASH, hash)
}

databaseConnection
.writeableDatabase
.update(
databaseConnection.withConnection {
writableDatabase.update(
ListsTable.TABLE_NAME,
contentValues,
"${ListsTable.COLUMN_NAME} = ?",
arrayOf(list)
)
}
}

override fun getListHash(list: String): String? {
return databaseConnection
.readableDatabase
.query(ListsTable.TABLE_NAME, "${ListsTable.COLUMN_NAME} = ?", arrayOf(list))
.first { it.getStringOrNull(ListsTable.COLUMN_HASH) }
return databaseConnection.withConnection {
readableDatabase
.query(ListsTable.TABLE_NAME, "${ListsTable.COLUMN_NAME} = ?", arrayOf(list))
.first { it.getStringOrNull(ListsTable.COLUMN_HASH) }
}
}

override fun getEntities(list: String): List<Entity.Saved> {
Expand All @@ -168,23 +168,27 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep
return 0
}

return databaseConnection.readableDatabase.rawQuery(
"""
SELECT COUNT(*)
FROM $list
""".trimIndent(),
null
).first {
it.getInt(0)
}!!
return databaseConnection.withConnection {
readableDatabase.rawQuery(
"""
SELECT COUNT(*)
FROM $list
""".trimIndent(),
null
).first {
it.getInt(0)
}!!
}
}

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

databaseConnection.writeableDatabase.delete(ListsTable.TABLE_NAME)
writableDatabase.delete(ListsTable.TABLE_NAME)
}
}

override fun addList(list: String) {
Expand All @@ -195,12 +199,14 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep
}

override fun delete(id: String) {
getLists().forEach {
databaseConnection.writeableDatabase.delete(
it,
"${EntitiesTable.COLUMN_ID} = ?",
arrayOf(id)
)
databaseConnection.withConnection {
getLists().forEach {
writableDatabase.delete(
it,
"${EntitiesTable.COLUMN_ID} = ?",
arrayOf(id)
)
}
}

updateRowIdTables()
Expand Down Expand Up @@ -229,7 +235,11 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep
return emptyList()
}

return if (databaseConnection.readableDatabase.doesColumnExist(list, property)) {
val propertyExists = databaseConnection.withConnection {
readableDatabase.doesColumnExist(list, property)
}

return if (propertyExists) {
queryWithAttachedRowId(
list,
selectionColumn = property,
Expand All @@ -251,45 +261,50 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep
return null
}

return databaseConnection.readableDatabase
.rawQuery(
"""
SELECT *, i.$ROW_ID
FROM $list e, ${getRowIdTableName(list)} i
WHERE e._id = i._id AND i.$ROW_ID = ?
""".trimIndent(),
arrayOf((index + 1).toString())
).first {
mapCursorRowToEntity(list, it, it.getInt(ROW_ID))
}
return databaseConnection.withConnection {
readableDatabase
.rawQuery(
"""
SELECT *, i.$ROW_ID
FROM $list e, ${getRowIdTableName(list)} i
WHERE e._id = i._id AND i.$ROW_ID = ?
""".trimIndent(),
arrayOf((index + 1).toString())
).first {
mapCursorRowToEntity(list, it, it.getInt(ROW_ID))
}
}
}

private fun queryWithAttachedRowId(list: String): Cursor {
return databaseConnection.readableDatabase
.rawQuery(
"""
SELECT *, i.$ROW_ID
FROM $list e, ${getRowIdTableName(list)} i
WHERE e._id = i._id
""".trimIndent(),
null
)
return databaseConnection.withConnection {
readableDatabase
.rawQuery(
"""
SELECT *, i.$ROW_ID
FROM $list e, ${getRowIdTableName(list)} i
WHERE e._id = i._id
""".trimIndent(),
null
)
}
}

private fun queryWithAttachedRowId(
list: String,
selectionColumn: String,
selectionArg: String
): Cursor {
return databaseConnection.readableDatabase
.rawQuery(
return databaseConnection.withConnection {
readableDatabase.rawQuery(
"""
SELECT *, i.$ROW_ID
FROM $list e, ${getRowIdTableName(list)} i
WHERE e._id = i._id AND $selectionColumn = ?
""".trimIndent(),
arrayOf(selectionArg)
)
}
}

/**
Expand All @@ -300,34 +315,38 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep
* function, but that's not available in all the supported versions of Android.
*/
private fun updateRowIdTables() {
getLists().forEach {
databaseConnection.writeableDatabase.execSQL(
"""
DROP TABLE IF EXISTS ${getRowIdTableName(it)};
""".trimIndent()
)
databaseConnection.withConnection {
getLists().forEach {
writableDatabase.execSQL(
"""
DROP TABLE IF EXISTS ${getRowIdTableName(it)};
""".trimIndent()
)

databaseConnection.writeableDatabase.execSQL(
"""
CREATE TABLE ${getRowIdTableName(it)} AS SELECT _id FROM $it;
""".trimIndent()
)
writableDatabase.execSQL(
"""
CREATE TABLE ${getRowIdTableName(it)} AS SELECT _id FROM $it;
""".trimIndent()
)
}
}
}

private fun getRowIdTableName(it: String) = "${it}_row_numbers"

private fun listExists(list: String): Boolean {
return databaseConnection.readableDatabase
.query(
ListsTable.TABLE_NAME,
selection = "${ListsTable.COLUMN_NAME} = ?",
selectionArgs = arrayOf(list)
).use { it.count } > 0
return databaseConnection.withConnection {
readableDatabase
.query(
ListsTable.TABLE_NAME,
selection = "${ListsTable.COLUMN_NAME} = ?",
selectionArgs = arrayOf(list)
).use { it.count } > 0
}
}

private fun createList(list: String) {
databaseConnection.writeableDatabase.transaction {
databaseConnection.transaction {
val contentValues = ContentValues()
contentValues.put(ListsTable.COLUMN_NAME, list)
insertOrThrow(
Expand Down Expand Up @@ -359,15 +378,21 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep
}

private fun updatePropertyColumns(list: String, entity: Entity) {
entity.properties.map { it.first }.forEach {
try {
databaseConnection.writeableDatabase.execSQL(
"""
ALTER TABLE $list ADD "$it" text NOT NULL DEFAULT "";
""".trimIndent()
)
} catch (e: SQLiteException) {
// Ignored
val columnNames = databaseConnection.withConnection {
readableDatabase.getColumnNames(list)
}

val missingColumns =
entity.properties.map { it.first }.filterNot { columnNames.contains(it) }
if (missingColumns.isNotEmpty()) {
databaseConnection.resetTransaction {
missingColumns.forEach {
execSQL(
"""
ALTER TABLE $list ADD "$it" text NOT NULL DEFAULT "";
""".trimIndent()
)
}
}
}
}
Expand Down
12 changes: 6 additions & 6 deletions ...ect_app/src/main/java/org/odk/collect/android/database/forms/DatabaseFormsRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,13 +238,13 @@ private Cursor queryAndReturnCursor(Map<String, String> projectionMap, String[]
}

private Long insertForm(ContentValues values) {
SQLiteDatabase writeableDatabase = databaseConnection.getWriteableDatabase();
return writeableDatabase.insertOrThrow(FORMS_TABLE_NAME, null, values);
SQLiteDatabase writableDatabase = databaseConnection.getWritableDatabase();
return writableDatabase.insertOrThrow(FORMS_TABLE_NAME, null, values);
}

private void updateForm(Long id, ContentValues values) {
SQLiteDatabase writeableDatabase = databaseConnection.getWriteableDatabase();
writeableDatabase.update(FORMS_TABLE_NAME, values, _ID + "=?", new String[]{String.valueOf(id)});
SQLiteDatabase writableDatabase = databaseConnection.getWritableDatabase();
writableDatabase.update(FORMS_TABLE_NAME, values, _ID + "=?", new String[]{String.valueOf(id)});
}

private void deleteForms(String selection, String[] selectionArgs) {
Expand All @@ -255,8 +255,8 @@ private void deleteForms(String selection, String[] selectionArgs) {
deleteFilesForForm(form);
}

SQLiteDatabase writeableDatabase = databaseConnection.getWriteableDatabase();
writeableDatabase.delete(FORMS_TABLE_NAME, selection, selectionArgs);
SQLiteDatabase writableDatabase = databaseConnection.getWritableDatabase();
writableDatabase.delete(FORMS_TABLE_NAME, selection, selectionArgs);
}

@NotNull
Expand Down
8 changes: 4 additions & 4 deletions ...src/main/java/org/odk/collect/android/database/instances/DatabaseInstancesRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ public List<Instance> getAllNotDeletedByFormIdAndVersion(String jrFormId, String
public void delete(Long id) {
Instance instance = get(id);

databaseConnection.getWriteableDatabase().delete(
databaseConnection.getWritableDatabase().delete(
INSTANCES_TABLE_NAME,
_ID + "=?",
new String[]{String.valueOf(id)}
Expand All @@ -157,7 +157,7 @@ public void delete(Long id) {
public void deleteAll() {
List<Instance> instances = getAll();

databaseConnection.getWriteableDatabase().delete(
databaseConnection.getWritableDatabase().delete(
INSTANCES_TABLE_NAME,
null,
null
Expand Down Expand Up @@ -253,15 +253,15 @@ private Cursor query(String[] projection, String selection, String[] selectionAr
}

private long insert(ContentValues values) {
return databaseConnection.getWriteableDatabase().insertOrThrow(
return databaseConnection.getWritableDatabase().insertOrThrow(
INSTANCES_TABLE_NAME,
null,
values
);
}

private void update(Long instanceId, ContentValues values) {
databaseConnection.getWriteableDatabase().update(
databaseConnection.getWritableDatabase().update(
INSTANCES_TABLE_NAME,
values,
_ID + "=?",
Expand Down
Loading