From 2e391bdd168a2924d7ce78a87882461873927d54 Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Mon, 9 Sep 2024 08:37:23 +0100 Subject: [PATCH 1/9] Spike out resetting DB connection after ALTER TABLE --- .../database/entities/DatabaseEntitiesRepository.kt | 7 +++---- .../java/org/odk/collect/db/sqlite/DatabaseConnection.kt | 5 +++++ 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/collect_app/src/main/java/org/odk/collect/android/database/entities/DatabaseEntitiesRepository.kt b/collect_app/src/main/java/org/odk/collect/android/database/entities/DatabaseEntitiesRepository.kt index bd5072a9b69..ef448c8fb41 100644 --- a/collect_app/src/main/java/org/odk/collect/android/database/entities/DatabaseEntitiesRepository.kt +++ b/collect_app/src/main/java/org/odk/collect/android/database/entities/DatabaseEntitiesRepository.kt @@ -4,7 +4,6 @@ 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 @@ -360,14 +359,14 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep private fun updatePropertyColumns(list: String, entity: Entity) { entity.properties.map { it.first }.forEach { - try { + if (!databaseConnection.readableDatabase.doesColumnExist(list, it)) { databaseConnection.writeableDatabase.execSQL( """ ALTER TABLE $list ADD "$it" text NOT NULL DEFAULT ""; """.trimIndent() ) - } catch (e: SQLiteException) { - // Ignored + + databaseConnection.reset() } } } diff --git a/db/src/main/java/org/odk/collect/db/sqlite/DatabaseConnection.kt b/db/src/main/java/org/odk/collect/db/sqlite/DatabaseConnection.kt index ff50599270e..9cdcfb42477 100644 --- a/db/src/main/java/org/odk/collect/db/sqlite/DatabaseConnection.kt +++ b/db/src/main/java/org/odk/collect/db/sqlite/DatabaseConnection.kt @@ -64,6 +64,11 @@ open class DatabaseConnection @JvmOverloads constructor( } } + fun reset() { + val databasePath = path + File.separator + name + openHelpers.remove(databasePath)?.close() + } + companion object { private val openHelpers = mutableMapOf() From 3f2607345db9e0f21a456fa667d7806c40ac9307 Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Mon, 9 Sep 2024 11:11:21 +0100 Subject: [PATCH 2/9] Synchronize access to the entities DB connection This should prevent issues with reset() being called while another transaction is running or a thread has a reference to a writable/ readable database. --- .../entities/DatabaseEntitiesRepository.kt | 300 ++++++++++-------- .../collect/db/sqlite/DatabaseConnection.kt | 6 + .../sqlite/SynchronizedDatabaseConnection.kt | 23 ++ 3 files changed, 193 insertions(+), 136 deletions(-) create mode 100644 db/src/main/java/org/odk/collect/db/sqlite/SynchronizedDatabaseConnection.kt diff --git a/collect_app/src/main/java/org/odk/collect/android/database/entities/DatabaseEntitiesRepository.kt b/collect_app/src/main/java/org/odk/collect/android/database/entities/DatabaseEntitiesRepository.kt index ef448c8fb41..93e9711ac55 100644 --- a/collect_app/src/main/java/org/odk/collect/android/database/entities/DatabaseEntitiesRepository.kt +++ b/collect_app/src/main/java/org/odk/collect/android/database/entities/DatabaseEntitiesRepository.kt @@ -12,12 +12,12 @@ 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.query +import org.odk.collect.db.sqlite.SynchronizedDatabaseConnection import org.odk.collect.entities.storage.EntitiesRepository import org.odk.collect.entities.storage.Entity @@ -38,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) { @@ -59,59 +58,61 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep updatePropertyColumns(list, entities.first()) - databaseConnection.writeableDatabase.transaction { - entities.forEach { entity -> - val existing = if (listExists) { - query( - list, - "${EntitiesTable.COLUMN_ID} = ?", - arrayOf(entity.id) - ).first { mapCursorRowToEntity(list, it, 0) } - } else { - null - } - - if (existing != null) { - val state = if (existing.state == Entity.State.OFFLINE) { - entity.state + databaseConnection.withConnection { + writeableDatabase.transaction { + entities.forEach { entity -> + val existing = if (listExists) { + query( + list, + "${EntitiesTable.COLUMN_ID} = ?", + arrayOf(entity.id) + ).first { mapCursorRowToEntity(list, it, 0) } } else { - Entity.State.ONLINE - } - - val contentValues = ContentValues().also { - it.put(EntitiesTable.COLUMN_ID, entity.id) - it.put(EntitiesTable.COLUMN_LABEL, entity.label ?: existing.label) - it.put(EntitiesTable.COLUMN_VERSION, entity.version) - it.put(EntitiesTable.COLUMN_TRUNK_VERSION, entity.trunkVersion) - it.put(EntitiesTable.COLUMN_BRANCH_ID, entity.branchId) - it.put(EntitiesTable.COLUMN_STATE, convertStateToInt(state)) - - addPropertiesToContentValues(it, entity) + null } - update( - list, - contentValues, - "${EntitiesTable.COLUMN_ID} = ?", - arrayOf(entity.id) - ) - } else { - val contentValues = ContentValues().also { - it.put(EntitiesTable.COLUMN_ID, entity.id) - it.put(EntitiesTable.COLUMN_LABEL, entity.label) - it.put(EntitiesTable.COLUMN_VERSION, entity.version) - it.put(EntitiesTable.COLUMN_TRUNK_VERSION, entity.trunkVersion) - it.put(EntitiesTable.COLUMN_BRANCH_ID, entity.branchId) - it.put(EntitiesTable.COLUMN_STATE, convertStateToInt(entity.state)) - - addPropertiesToContentValues(it, entity) + if (existing != null) { + val state = if (existing.state == Entity.State.OFFLINE) { + entity.state + } else { + Entity.State.ONLINE + } + + val contentValues = ContentValues().also { + it.put(EntitiesTable.COLUMN_ID, entity.id) + it.put(EntitiesTable.COLUMN_LABEL, entity.label ?: existing.label) + it.put(EntitiesTable.COLUMN_VERSION, entity.version) + it.put(EntitiesTable.COLUMN_TRUNK_VERSION, entity.trunkVersion) + it.put(EntitiesTable.COLUMN_BRANCH_ID, entity.branchId) + it.put(EntitiesTable.COLUMN_STATE, convertStateToInt(state)) + + addPropertiesToContentValues(it, entity) + } + + update( + list, + contentValues, + "${EntitiesTable.COLUMN_ID} = ?", + arrayOf(entity.id) + ) + } else { + val contentValues = ContentValues().also { + it.put(EntitiesTable.COLUMN_ID, entity.id) + it.put(EntitiesTable.COLUMN_LABEL, entity.label) + it.put(EntitiesTable.COLUMN_VERSION, entity.version) + it.put(EntitiesTable.COLUMN_TRUNK_VERSION, entity.trunkVersion) + it.put(EntitiesTable.COLUMN_BRANCH_ID, entity.branchId) + it.put(EntitiesTable.COLUMN_STATE, convertStateToInt(entity.state)) + + addPropertiesToContentValues(it, entity) + } + + insertOrThrow( + list, + null, + contentValues + ) } - - insertOrThrow( - list, - null, - contentValues - ) } } } @@ -120,10 +121,11 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep } override fun getLists(): Set { - 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) { @@ -131,21 +133,22 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep it.put(ListsTable.COLUMN_HASH, hash) } - databaseConnection - .writeableDatabase - .update( + databaseConnection.withConnection { + writeableDatabase.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 { @@ -167,23 +170,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 { + writeableDatabase.delete(it) + } - databaseConnection.writeableDatabase.delete(ListsTable.TABLE_NAME) + writeableDatabase.delete(ListsTable.TABLE_NAME) + } } override fun addList(list: String) { @@ -194,12 +201,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 { + writeableDatabase.delete( + it, + "${EntitiesTable.COLUMN_ID} = ?", + arrayOf(id) + ) + } } updateRowIdTables() @@ -228,7 +237,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, @@ -250,29 +263,33 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep return null } - 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 i.$ROW_ID = ? """.trimIndent(), - arrayOf((index + 1).toString()) - ).first { - mapCursorRowToEntity(list, it, it.getInt(ROW_ID)) - } + 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( @@ -280,8 +297,8 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep selectionColumn: String, selectionArg: String ): Cursor { - return databaseConnection.readableDatabase - .rawQuery( + return databaseConnection.withConnection { + readableDatabase.rawQuery( """ SELECT *, i.$ROW_ID FROM $list e, ${getRowIdTableName(list)} i @@ -289,6 +306,7 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep """.trimIndent(), arrayOf(selectionArg) ) + } } /** @@ -299,44 +317,51 @@ 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( - """ + databaseConnection.withConnection { + getLists().forEach { + writeableDatabase.execSQL( + """ DROP TABLE IF EXISTS ${getRowIdTableName(it)}; """.trimIndent() - ) + ) - databaseConnection.writeableDatabase.execSQL( - """ + writeableDatabase.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 { - val contentValues = ContentValues() - contentValues.put(ListsTable.COLUMN_NAME, list) - insertOrThrow( - ListsTable.TABLE_NAME, - null, - contentValues - ) + databaseConnection.withConnection { + writeableDatabase.transaction { + val contentValues = ContentValues() + contentValues.put(ListsTable.COLUMN_NAME, list) + insertOrThrow( + ListsTable.TABLE_NAME, + null, + contentValues + ) - execSQL( - """ + execSQL( + """ CREATE TABLE IF NOT EXISTS $list ( $_ID integer PRIMARY KEY, ${EntitiesTable.COLUMN_ID} text, @@ -347,26 +372,29 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep ${EntitiesTable.COLUMN_STATE} integer NOT NULL ); """.trimIndent() - ) + ) - execSQL( - """ + execSQL( + """ CREATE UNIQUE INDEX IF NOT EXISTS ${list}_unique_id_index ON $list (${EntitiesTable.COLUMN_ID}); """.trimIndent() - ) + ) + } } } private fun updatePropertyColumns(list: String, entity: Entity) { entity.properties.map { it.first }.forEach { - if (!databaseConnection.readableDatabase.doesColumnExist(list, it)) { - databaseConnection.writeableDatabase.execSQL( - """ - ALTER TABLE $list ADD "$it" text NOT NULL DEFAULT ""; - """.trimIndent() - ) + databaseConnection.withConnection { + if (!readableDatabase.doesColumnExist(list, it)) { + writeableDatabase.execSQL( + """ + ALTER TABLE $list ADD "$it" text NOT NULL DEFAULT ""; + """.trimIndent() + ) - databaseConnection.reset() + reset() + } } } } diff --git a/db/src/main/java/org/odk/collect/db/sqlite/DatabaseConnection.kt b/db/src/main/java/org/odk/collect/db/sqlite/DatabaseConnection.kt index 9cdcfb42477..313e2d7908d 100644 --- a/db/src/main/java/org/odk/collect/db/sqlite/DatabaseConnection.kt +++ b/db/src/main/java/org/odk/collect/db/sqlite/DatabaseConnection.kt @@ -69,6 +69,12 @@ open class DatabaseConnection @JvmOverloads constructor( openHelpers.remove(databasePath)?.close() } + fun withSynchronizedConnection(block: DatabaseConnection.() -> T): T { + return synchronized(dbHelper) { + block(this) + } + } + companion object { private val openHelpers = mutableMapOf() diff --git a/db/src/main/java/org/odk/collect/db/sqlite/SynchronizedDatabaseConnection.kt b/db/src/main/java/org/odk/collect/db/sqlite/SynchronizedDatabaseConnection.kt new file mode 100644 index 00000000000..a4065e3154d --- /dev/null +++ b/db/src/main/java/org/odk/collect/db/sqlite/SynchronizedDatabaseConnection.kt @@ -0,0 +1,23 @@ +package org.odk.collect.db.sqlite + +import android.content.Context + +class SynchronizedDatabaseConnection constructor( + context: Context, + path: String, + name: String, + migrator: DatabaseMigrator, + databaseVersion: Int +) { + private val databaseConnection = DatabaseConnection( + context, + path, + name, + migrator, + databaseVersion + ) + + fun withConnection(block: DatabaseConnection.() -> T): T { + return databaseConnection.withSynchronizedConnection(block) + } +} From b4d77663ff7fe7ef63e354431aa03cffa1168631 Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Mon, 9 Sep 2024 11:14:22 +0100 Subject: [PATCH 3/9] Synchronize SQLiteOpenHelper creation --- .../collect/db/sqlite/DatabaseConnection.kt | 42 ++++++++++--------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/db/src/main/java/org/odk/collect/db/sqlite/DatabaseConnection.kt b/db/src/main/java/org/odk/collect/db/sqlite/DatabaseConnection.kt index 313e2d7908d..5ffa8d1a27f 100644 --- a/db/src/main/java/org/odk/collect/db/sqlite/DatabaseConnection.kt +++ b/db/src/main/java/org/odk/collect/db/sqlite/DatabaseConnection.kt @@ -24,6 +24,8 @@ open class DatabaseConnection @JvmOverloads constructor( private val strict: Boolean = false ) { + private val databasePath = path + File.separator + name + val writeableDatabase: SQLiteDatabase get() { StrictMode.noteSlowCall("Accessing writable DB") @@ -40,32 +42,32 @@ open class DatabaseConnection @JvmOverloads constructor( private val dbHelper: SQLiteOpenHelper get() { - val databasePath = path + File.separator + name - if (openHelpers.containsKey(databasePath) && !File(databasePath).exists()) { - /** - * Ideally we should close the database here as well but it was causing crashes in - * our tests as DB connections seem to be getting used after being closed. These - * "removed" helpers will be closed in [closeAll] rather than when they are - * replaced. - */ - openHelpers.remove(databasePath)?.let { - toClose.add(it) + return synchronized(openHelpers) { + if (openHelpers.containsKey(databasePath) && !File(databasePath).exists()) { + /** + * Ideally we should close the database here as well but it was causing crashes in + * our tests as DB connections seem to be getting used after being closed. These + * "removed" helpers will be closed in [closeAll] rather than when they are + * replaced. + */ + openHelpers.remove(databasePath)?.let { + toClose.add(it) + } } - } - return openHelpers.getOrPut(databasePath) { - DatabaseMigratorSQLiteOpenHelper( - AltDatabasePathContext(path, context), - name, - null, - databaseVersion, - migrator - ) + openHelpers.getOrPut(databasePath) { + DatabaseMigratorSQLiteOpenHelper( + AltDatabasePathContext(path, context), + name, + null, + databaseVersion, + migrator + ) + } } } fun reset() { - val databasePath = path + File.separator + name openHelpers.remove(databasePath)?.close() } From d99a4a7a24c76523073cfa1ac0f2f3dd1983fff9 Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Mon, 9 Sep 2024 11:37:29 +0100 Subject: [PATCH 4/9] Add comments --- .../entities/DatabaseEntitiesRepository.kt | 42 +++++++++---------- .../collect/db/sqlite/DatabaseConnection.kt | 19 +++++++++ 2 files changed, 39 insertions(+), 22 deletions(-) diff --git a/collect_app/src/main/java/org/odk/collect/android/database/entities/DatabaseEntitiesRepository.kt b/collect_app/src/main/java/org/odk/collect/android/database/entities/DatabaseEntitiesRepository.kt index 93e9711ac55..ae3db4c64b4 100644 --- a/collect_app/src/main/java/org/odk/collect/android/database/entities/DatabaseEntitiesRepository.kt +++ b/collect_app/src/main/java/org/odk/collect/android/database/entities/DatabaseEntitiesRepository.kt @@ -267,10 +267,10 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep readableDatabase .rawQuery( """ - SELECT *, i.$ROW_ID - FROM $list e, ${getRowIdTableName(list)} i - WHERE e._id = i._id AND i.$ROW_ID = ? - """.trimIndent(), + 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)) @@ -321,18 +321,17 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep getLists().forEach { writeableDatabase.execSQL( """ - DROP TABLE IF EXISTS ${getRowIdTableName(it)}; - """.trimIndent() + DROP TABLE IF EXISTS ${getRowIdTableName(it)}; + """.trimIndent() ) writeableDatabase.execSQL( """ - CREATE TABLE ${getRowIdTableName(it)} AS SELECT _id FROM $it; - """.trimIndent() + CREATE TABLE ${getRowIdTableName(it)} AS SELECT _id FROM $it; + """.trimIndent() ) } } - } private fun getRowIdTableName(it: String) = "${it}_row_numbers" @@ -346,7 +345,6 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep selectionArgs = arrayOf(list) ).use { it.count } > 0 } - } private fun createList(list: String) { @@ -362,22 +360,22 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep execSQL( """ - CREATE TABLE IF NOT EXISTS $list ( - $_ID integer PRIMARY KEY, - ${EntitiesTable.COLUMN_ID} text, - ${EntitiesTable.COLUMN_LABEL} text, - ${EntitiesTable.COLUMN_VERSION} integer, - ${EntitiesTable.COLUMN_TRUNK_VERSION} integer, - ${EntitiesTable.COLUMN_BRANCH_ID} text, - ${EntitiesTable.COLUMN_STATE} integer NOT NULL - ); - """.trimIndent() + CREATE TABLE IF NOT EXISTS $list ( + $_ID integer PRIMARY KEY, + ${EntitiesTable.COLUMN_ID} text, + ${EntitiesTable.COLUMN_LABEL} text, + ${EntitiesTable.COLUMN_VERSION} integer, + ${EntitiesTable.COLUMN_TRUNK_VERSION} integer, + ${EntitiesTable.COLUMN_BRANCH_ID} text, + ${EntitiesTable.COLUMN_STATE} integer NOT NULL + ); + """.trimIndent() ) execSQL( """ - CREATE UNIQUE INDEX IF NOT EXISTS ${list}_unique_id_index ON $list (${EntitiesTable.COLUMN_ID}); - """.trimIndent() + CREATE UNIQUE INDEX IF NOT EXISTS ${list}_unique_id_index ON $list (${EntitiesTable.COLUMN_ID}); + """.trimIndent() ) } } diff --git a/db/src/main/java/org/odk/collect/db/sqlite/DatabaseConnection.kt b/db/src/main/java/org/odk/collect/db/sqlite/DatabaseConnection.kt index 5ffa8d1a27f..b709ed212e1 100644 --- a/db/src/main/java/org/odk/collect/db/sqlite/DatabaseConnection.kt +++ b/db/src/main/java/org/odk/collect/db/sqlite/DatabaseConnection.kt @@ -67,10 +67,29 @@ open class DatabaseConnection @JvmOverloads constructor( } } + /** + * Closes the underlying connection and clears state so that subsequent accesses will create + * a new connection. + * + * This can be dangerous if the database is being access by multiple threads as the connection + * might be closed while a transaction is running or while another thread is using a + * [SQLiteDatabase] instance obtained via [writeableDatabase] or [readableDatabase]. Using + * [SynchronizedDatabaseConnection] is recommended in those scenarios. + */ fun reset() { openHelpers.remove(databasePath)?.close() } + /** + * Access the database in a synchronized manner. This is not usually required, but should be + * used if a calls to [reset] will be made. + * + * Does not guarantee synchronized access if this or another [DatabaseConnection] for the + * same `.db` file uses [writeableDatabase] or [readableDatabase]. + * [SynchronizedDatabaseConnection] can be used to ensure synchronized writes/reads to the + * underlying DB. + * + */ fun withSynchronizedConnection(block: DatabaseConnection.() -> T): T { return synchronized(dbHelper) { block(this) From ed6f5ed635ed92495ada8ce3b42ee0964e80192f Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Mon, 9 Sep 2024 11:58:33 +0100 Subject: [PATCH 5/9] Only reset DB connection if adding columns --- .../entities/DatabaseEntitiesRepository.kt | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/collect_app/src/main/java/org/odk/collect/android/database/entities/DatabaseEntitiesRepository.kt b/collect_app/src/main/java/org/odk/collect/android/database/entities/DatabaseEntitiesRepository.kt index ae3db4c64b4..c9447404330 100644 --- a/collect_app/src/main/java/org/odk/collect/android/database/entities/DatabaseEntitiesRepository.kt +++ b/collect_app/src/main/java/org/odk/collect/android/database/entities/DatabaseEntitiesRepository.kt @@ -17,6 +17,7 @@ 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.query +import org.odk.collect.db.sqlite.SQLiteUtils import org.odk.collect.db.sqlite.SynchronizedDatabaseConnection import org.odk.collect.entities.storage.EntitiesRepository import org.odk.collect.entities.storage.Entity @@ -382,17 +383,23 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep } private fun updatePropertyColumns(list: String, entity: Entity) { - entity.properties.map { it.first }.forEach { + val columnNames = databaseConnection.withConnection { + SQLiteUtils.getColumnNames(readableDatabase, list) + } + + val missingColumns = + entity.properties.map { it.first }.filterNot { columnNames.contains(it) } + if (missingColumns.isNotEmpty()) { databaseConnection.withConnection { - if (!readableDatabase.doesColumnExist(list, it)) { + missingColumns.forEach { writeableDatabase.execSQL( """ ALTER TABLE $list ADD "$it" text NOT NULL DEFAULT ""; """.trimIndent() ) - - reset() } + + reset() } } } From 24da9b68011df2232f7134bdd3dc7a5e68519b43 Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Mon, 9 Sep 2024 11:59:54 +0100 Subject: [PATCH 6/9] Add columns in one transaction rather than many --- .../entities/DatabaseEntitiesRepository.kt | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/collect_app/src/main/java/org/odk/collect/android/database/entities/DatabaseEntitiesRepository.kt b/collect_app/src/main/java/org/odk/collect/android/database/entities/DatabaseEntitiesRepository.kt index c9447404330..f6699021489 100644 --- a/collect_app/src/main/java/org/odk/collect/android/database/entities/DatabaseEntitiesRepository.kt +++ b/collect_app/src/main/java/org/odk/collect/android/database/entities/DatabaseEntitiesRepository.kt @@ -391,14 +391,17 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep entity.properties.map { it.first }.filterNot { columnNames.contains(it) } if (missingColumns.isNotEmpty()) { databaseConnection.withConnection { - missingColumns.forEach { - writeableDatabase.execSQL( - """ - ALTER TABLE $list ADD "$it" text NOT NULL DEFAULT ""; - """.trimIndent() - ) + writeableDatabase.transaction { + missingColumns.forEach { + execSQL( + """ + ALTER TABLE $list ADD "$it" text NOT NULL DEFAULT ""; + """.trimIndent() + ) + } } + reset() } } From 1cc25406ce7d4282b014093602644b0abf032edf Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Mon, 9 Sep 2024 12:09:10 +0100 Subject: [PATCH 7/9] Add helpers to reduce indenting --- .../entities/DatabaseEntitiesRepository.kt | 174 +++++++++--------- .../sqlite/SynchronizedDatabaseConnection.kt | 25 ++- 2 files changed, 106 insertions(+), 93 deletions(-) diff --git a/collect_app/src/main/java/org/odk/collect/android/database/entities/DatabaseEntitiesRepository.kt b/collect_app/src/main/java/org/odk/collect/android/database/entities/DatabaseEntitiesRepository.kt index f6699021489..029da0530e8 100644 --- a/collect_app/src/main/java/org/odk/collect/android/database/entities/DatabaseEntitiesRepository.kt +++ b/collect_app/src/main/java/org/odk/collect/android/database/entities/DatabaseEntitiesRepository.kt @@ -5,7 +5,6 @@ import android.content.Context import android.database.Cursor import android.database.sqlite.SQLiteDatabase 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 @@ -59,61 +58,59 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep updatePropertyColumns(list, entities.first()) - databaseConnection.withConnection { - writeableDatabase.transaction { - entities.forEach { entity -> - val existing = if (listExists) { - query( - list, - "${EntitiesTable.COLUMN_ID} = ?", - arrayOf(entity.id) - ).first { mapCursorRowToEntity(list, it, 0) } + databaseConnection.transaction { + entities.forEach { entity -> + val existing = if (listExists) { + query( + list, + "${EntitiesTable.COLUMN_ID} = ?", + arrayOf(entity.id) + ).first { mapCursorRowToEntity(list, it, 0) } + } else { + null + } + + if (existing != null) { + val state = if (existing.state == Entity.State.OFFLINE) { + entity.state } else { - null + Entity.State.ONLINE } - if (existing != null) { - val state = if (existing.state == Entity.State.OFFLINE) { - entity.state - } else { - Entity.State.ONLINE - } - - val contentValues = ContentValues().also { - it.put(EntitiesTable.COLUMN_ID, entity.id) - it.put(EntitiesTable.COLUMN_LABEL, entity.label ?: existing.label) - it.put(EntitiesTable.COLUMN_VERSION, entity.version) - it.put(EntitiesTable.COLUMN_TRUNK_VERSION, entity.trunkVersion) - it.put(EntitiesTable.COLUMN_BRANCH_ID, entity.branchId) - it.put(EntitiesTable.COLUMN_STATE, convertStateToInt(state)) - - addPropertiesToContentValues(it, entity) - } - - update( - list, - contentValues, - "${EntitiesTable.COLUMN_ID} = ?", - arrayOf(entity.id) - ) - } else { - val contentValues = ContentValues().also { - it.put(EntitiesTable.COLUMN_ID, entity.id) - it.put(EntitiesTable.COLUMN_LABEL, entity.label) - it.put(EntitiesTable.COLUMN_VERSION, entity.version) - it.put(EntitiesTable.COLUMN_TRUNK_VERSION, entity.trunkVersion) - it.put(EntitiesTable.COLUMN_BRANCH_ID, entity.branchId) - it.put(EntitiesTable.COLUMN_STATE, convertStateToInt(entity.state)) - - addPropertiesToContentValues(it, entity) - } - - insertOrThrow( - list, - null, - contentValues - ) + val contentValues = ContentValues().also { + it.put(EntitiesTable.COLUMN_ID, entity.id) + it.put(EntitiesTable.COLUMN_LABEL, entity.label ?: existing.label) + it.put(EntitiesTable.COLUMN_VERSION, entity.version) + it.put(EntitiesTable.COLUMN_TRUNK_VERSION, entity.trunkVersion) + it.put(EntitiesTable.COLUMN_BRANCH_ID, entity.branchId) + it.put(EntitiesTable.COLUMN_STATE, convertStateToInt(state)) + + addPropertiesToContentValues(it, entity) + } + + update( + list, + contentValues, + "${EntitiesTable.COLUMN_ID} = ?", + arrayOf(entity.id) + ) + } else { + val contentValues = ContentValues().also { + it.put(EntitiesTable.COLUMN_ID, entity.id) + it.put(EntitiesTable.COLUMN_LABEL, entity.label) + it.put(EntitiesTable.COLUMN_VERSION, entity.version) + it.put(EntitiesTable.COLUMN_TRUNK_VERSION, entity.trunkVersion) + it.put(EntitiesTable.COLUMN_BRANCH_ID, entity.branchId) + it.put(EntitiesTable.COLUMN_STATE, convertStateToInt(entity.state)) + + addPropertiesToContentValues(it, entity) } + + insertOrThrow( + list, + null, + contentValues + ) } } } @@ -349,36 +346,34 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep } private fun createList(list: String) { - databaseConnection.withConnection { - writeableDatabase.transaction { - val contentValues = ContentValues() - contentValues.put(ListsTable.COLUMN_NAME, list) - insertOrThrow( - ListsTable.TABLE_NAME, - null, - contentValues - ) + databaseConnection.transaction { + val contentValues = ContentValues() + contentValues.put(ListsTable.COLUMN_NAME, list) + insertOrThrow( + ListsTable.TABLE_NAME, + null, + contentValues + ) - execSQL( - """ - CREATE TABLE IF NOT EXISTS $list ( - $_ID integer PRIMARY KEY, - ${EntitiesTable.COLUMN_ID} text, - ${EntitiesTable.COLUMN_LABEL} text, - ${EntitiesTable.COLUMN_VERSION} integer, - ${EntitiesTable.COLUMN_TRUNK_VERSION} integer, - ${EntitiesTable.COLUMN_BRANCH_ID} text, - ${EntitiesTable.COLUMN_STATE} integer NOT NULL - ); - """.trimIndent() - ) + execSQL( + """ + CREATE TABLE IF NOT EXISTS $list ( + $_ID integer PRIMARY KEY, + ${EntitiesTable.COLUMN_ID} text, + ${EntitiesTable.COLUMN_LABEL} text, + ${EntitiesTable.COLUMN_VERSION} integer, + ${EntitiesTable.COLUMN_TRUNK_VERSION} integer, + ${EntitiesTable.COLUMN_BRANCH_ID} text, + ${EntitiesTable.COLUMN_STATE} integer NOT NULL + ); + """.trimIndent() + ) - execSQL( - """ - CREATE UNIQUE INDEX IF NOT EXISTS ${list}_unique_id_index ON $list (${EntitiesTable.COLUMN_ID}); - """.trimIndent() - ) - } + execSQL( + """ + CREATE UNIQUE INDEX IF NOT EXISTS ${list}_unique_id_index ON $list (${EntitiesTable.COLUMN_ID}); + """.trimIndent() + ) } } @@ -390,19 +385,14 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep val missingColumns = entity.properties.map { it.first }.filterNot { columnNames.contains(it) } if (missingColumns.isNotEmpty()) { - databaseConnection.withConnection { - writeableDatabase.transaction { - missingColumns.forEach { - execSQL( - """ - ALTER TABLE $list ADD "$it" text NOT NULL DEFAULT ""; - """.trimIndent() - ) - } + databaseConnection.resetTransaction { + missingColumns.forEach { + execSQL( + """ + ALTER TABLE $list ADD "$it" text NOT NULL DEFAULT ""; + """.trimIndent() + ) } - - - reset() } } } diff --git a/db/src/main/java/org/odk/collect/db/sqlite/SynchronizedDatabaseConnection.kt b/db/src/main/java/org/odk/collect/db/sqlite/SynchronizedDatabaseConnection.kt index a4065e3154d..1400d3f30eb 100644 --- a/db/src/main/java/org/odk/collect/db/sqlite/SynchronizedDatabaseConnection.kt +++ b/db/src/main/java/org/odk/collect/db/sqlite/SynchronizedDatabaseConnection.kt @@ -1,8 +1,10 @@ package org.odk.collect.db.sqlite import android.content.Context +import android.database.sqlite.SQLiteDatabase +import androidx.core.database.sqlite.transaction -class SynchronizedDatabaseConnection constructor( +class SynchronizedDatabaseConnection( context: Context, path: String, name: String, @@ -20,4 +22,25 @@ class SynchronizedDatabaseConnection constructor( fun withConnection(block: DatabaseConnection.() -> T): T { return databaseConnection.withSynchronizedConnection(block) } + + fun transaction( + body: SQLiteDatabase.() -> T + ) { + return withConnection { + writeableDatabase.transaction { + body() + } + } + } + + /** + * Runs a transaction and then calls [DatabaseConnection.reset]. Useful for transactions + * that will mutate the DB schema. + */ + fun resetTransaction( + body: SQLiteDatabase.() -> T + ) { + transaction(body) + databaseConnection.reset() + } } From 4128ec432bec8fb2cc5e2b7999ada57e3ec0e389 Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Mon, 9 Sep 2024 13:34:26 +0100 Subject: [PATCH 8/9] Replace SQLiteUtils.getColumnNames --- .../entities/DatabaseEntitiesRepository.kt | 4 ++-- .../instances/InstanceDatabaseMigrator.java | 3 ++- .../org/odk/collect/db/sqlite/SQLiteDatabaseExt.kt | 12 +++++++++++- .../org/odk/collect/db/sqlite/SQLiteUtils.java | 14 -------------- 4 files changed, 15 insertions(+), 18 deletions(-) diff --git a/collect_app/src/main/java/org/odk/collect/android/database/entities/DatabaseEntitiesRepository.kt b/collect_app/src/main/java/org/odk/collect/android/database/entities/DatabaseEntitiesRepository.kt index 029da0530e8..62400fe974c 100644 --- a/collect_app/src/main/java/org/odk/collect/android/database/entities/DatabaseEntitiesRepository.kt +++ b/collect_app/src/main/java/org/odk/collect/android/database/entities/DatabaseEntitiesRepository.kt @@ -15,8 +15,8 @@ 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.SQLiteUtils import org.odk.collect.db.sqlite.SynchronizedDatabaseConnection import org.odk.collect.entities.storage.EntitiesRepository import org.odk.collect.entities.storage.Entity @@ -379,7 +379,7 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep private fun updatePropertyColumns(list: String, entity: Entity) { val columnNames = databaseConnection.withConnection { - SQLiteUtils.getColumnNames(readableDatabase, list) + readableDatabase.getColumnNames(list) } val missingColumns = diff --git a/collect_app/src/main/java/org/odk/collect/android/database/instances/InstanceDatabaseMigrator.java b/collect_app/src/main/java/org/odk/collect/android/database/instances/InstanceDatabaseMigrator.java index f4cbcb4ce33..22b6ff150ab 100644 --- a/collect_app/src/main/java/org/odk/collect/android/database/instances/InstanceDatabaseMigrator.java +++ b/collect_app/src/main/java/org/odk/collect/android/database/instances/InstanceDatabaseMigrator.java @@ -19,6 +19,7 @@ import android.database.sqlite.SQLiteDatabase; import org.odk.collect.db.sqlite.DatabaseMigrator; +import org.odk.collect.db.sqlite.SQLiteDatabaseExt; import org.odk.collect.db.sqlite.SQLiteUtils; import org.odk.collect.forms.instances.Instance; @@ -118,7 +119,7 @@ private void upgradeToVersion5(SQLiteDatabase db) { * @param temporaryTableName the name of the temporary table to use and then drop */ private void dropObsoleteColumns(SQLiteDatabase db, String[] relevantColumns, String temporaryTableName) { - List columns = SQLiteUtils.getColumnNames(db, INSTANCES_TABLE_NAME); + List columns = SQLiteDatabaseExt.getColumnNames(db, INSTANCES_TABLE_NAME); columns.retainAll(Arrays.asList(relevantColumns)); String[] columnsToKeep = columns.toArray(new String[0]); diff --git a/db/src/main/java/org/odk/collect/db/sqlite/SQLiteDatabaseExt.kt b/db/src/main/java/org/odk/collect/db/sqlite/SQLiteDatabaseExt.kt index 3f10577f071..642545eced6 100644 --- a/db/src/main/java/org/odk/collect/db/sqlite/SQLiteDatabaseExt.kt +++ b/db/src/main/java/org/odk/collect/db/sqlite/SQLiteDatabaseExt.kt @@ -23,6 +23,16 @@ object SQLiteDatabaseExt { @JvmStatic fun SQLiteDatabase.doesColumnExist(table: String, column: String): Boolean { - return SQLiteUtils.getColumnNames(this, table).contains(column) + return this.getColumnNames(table).contains(column) + } + + @JvmStatic + fun SQLiteDatabase.getColumnNames(table: String): List { + var columnNames: Array + this.query(table, null, null, null, null, null, null).use { c -> + columnNames = c.columnNames + } + + return columnNames.toList() } } diff --git a/db/src/main/java/org/odk/collect/db/sqlite/SQLiteUtils.java b/db/src/main/java/org/odk/collect/db/sqlite/SQLiteUtils.java index f6248f3f1a9..df3d57ec180 100644 --- a/db/src/main/java/org/odk/collect/db/sqlite/SQLiteUtils.java +++ b/db/src/main/java/org/odk/collect/db/sqlite/SQLiteUtils.java @@ -5,10 +5,6 @@ import android.database.Cursor; import android.database.sqlite.SQLiteDatabase; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; - /** * @deprecated use {@link SQLiteDatabaseExt} instead. */ @@ -36,16 +32,6 @@ public static boolean doesTableExist(SQLiteDatabase db, String tableName) { return foundTable; } - public static List getColumnNames(SQLiteDatabase db, String tableName) { - String[] columnNames; - try (Cursor c = db.query(tableName, null, null, null, null, null, null)) { - columnNames = c.getColumnNames(); - } - - // Build a full-featured ArrayList rather than the limited array-backed List from asList - return new ArrayList<>(Arrays.asList(columnNames)); - } - public static void addColumn(SQLiteDatabase db, String table, String column, String type) { if (!doesColumnExist(db, table, column)) { CustomSQLiteQueryExecutor.begin(db) From a1546651b88714ca8a06aec7a95b76493d3c7233 Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Tue, 10 Sep 2024 11:56:20 +0100 Subject: [PATCH 9/9] Fix typo --- .../database/entities/DatabaseEntitiesRepository.kt | 12 ++++++------ .../database/forms/DatabaseFormsRepository.java | 12 ++++++------ .../instances/DatabaseInstancesRepository.java | 8 ++++---- .../savepoints/DatabaseSavepointsRepository.kt | 6 +++--- .../org/odk/collect/db/sqlite/DatabaseConnection.kt | 7 +++---- .../db/sqlite/SynchronizedDatabaseConnection.kt | 2 +- 6 files changed, 23 insertions(+), 24 deletions(-) diff --git a/collect_app/src/main/java/org/odk/collect/android/database/entities/DatabaseEntitiesRepository.kt b/collect_app/src/main/java/org/odk/collect/android/database/entities/DatabaseEntitiesRepository.kt index 62400fe974c..273c89f44c1 100644 --- a/collect_app/src/main/java/org/odk/collect/android/database/entities/DatabaseEntitiesRepository.kt +++ b/collect_app/src/main/java/org/odk/collect/android/database/entities/DatabaseEntitiesRepository.kt @@ -132,7 +132,7 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep } databaseConnection.withConnection { - writeableDatabase.update( + writableDatabase.update( ListsTable.TABLE_NAME, contentValues, "${ListsTable.COLUMN_NAME} = ?", @@ -184,10 +184,10 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep override fun clear() { databaseConnection.withConnection { getLists().forEach { - writeableDatabase.delete(it) + writableDatabase.delete(it) } - writeableDatabase.delete(ListsTable.TABLE_NAME) + writableDatabase.delete(ListsTable.TABLE_NAME) } } @@ -201,7 +201,7 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep override fun delete(id: String) { databaseConnection.withConnection { getLists().forEach { - writeableDatabase.delete( + writableDatabase.delete( it, "${EntitiesTable.COLUMN_ID} = ?", arrayOf(id) @@ -317,13 +317,13 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep private fun updateRowIdTables() { databaseConnection.withConnection { getLists().forEach { - writeableDatabase.execSQL( + writableDatabase.execSQL( """ DROP TABLE IF EXISTS ${getRowIdTableName(it)}; """.trimIndent() ) - writeableDatabase.execSQL( + writableDatabase.execSQL( """ CREATE TABLE ${getRowIdTableName(it)} AS SELECT _id FROM $it; """.trimIndent() diff --git a/collect_app/src/main/java/org/odk/collect/android/database/forms/DatabaseFormsRepository.java b/collect_app/src/main/java/org/odk/collect/android/database/forms/DatabaseFormsRepository.java index 3797a7f5a0e..db305178f92 100644 --- a/collect_app/src/main/java/org/odk/collect/android/database/forms/DatabaseFormsRepository.java +++ b/collect_app/src/main/java/org/odk/collect/android/database/forms/DatabaseFormsRepository.java @@ -238,13 +238,13 @@ private Cursor queryAndReturnCursor(Map 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) { @@ -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 diff --git a/collect_app/src/main/java/org/odk/collect/android/database/instances/DatabaseInstancesRepository.java b/collect_app/src/main/java/org/odk/collect/android/database/instances/DatabaseInstancesRepository.java index 18f06367c03..8c9d33c3a44 100644 --- a/collect_app/src/main/java/org/odk/collect/android/database/instances/DatabaseInstancesRepository.java +++ b/collect_app/src/main/java/org/odk/collect/android/database/instances/DatabaseInstancesRepository.java @@ -144,7 +144,7 @@ public List 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)} @@ -157,7 +157,7 @@ public void delete(Long id) { public void deleteAll() { List instances = getAll(); - databaseConnection.getWriteableDatabase().delete( + databaseConnection.getWritableDatabase().delete( INSTANCES_TABLE_NAME, null, null @@ -253,7 +253,7 @@ 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 @@ -261,7 +261,7 @@ private long insert(ContentValues values) { } private void update(Long instanceId, ContentValues values) { - databaseConnection.getWriteableDatabase().update( + databaseConnection.getWritableDatabase().update( INSTANCES_TABLE_NAME, values, _ID + "=?", diff --git a/collect_app/src/main/java/org/odk/collect/android/database/savepoints/DatabaseSavepointsRepository.kt b/collect_app/src/main/java/org/odk/collect/android/database/savepoints/DatabaseSavepointsRepository.kt index b9029fee1ad..6b126b04acd 100644 --- a/collect_app/src/main/java/org/odk/collect/android/database/savepoints/DatabaseSavepointsRepository.kt +++ b/collect_app/src/main/java/org/odk/collect/android/database/savepoints/DatabaseSavepointsRepository.kt @@ -66,7 +66,7 @@ class DatabaseSavepointsRepository( val values = getValuesFromSavepoint(savepoint, cachePath, instancesPath) databaseConnection - .writeableDatabase + .writableDatabase .insertOrThrow(SAVEPOINTS_TABLE_NAME, null, values) } @@ -86,7 +86,7 @@ class DatabaseSavepointsRepository( } databaseConnection - .writeableDatabase + .writableDatabase .delete(SAVEPOINTS_TABLE_NAME, selection, selectionArgs) File(savepoint.savepointFilePath).delete() @@ -98,7 +98,7 @@ class DatabaseSavepointsRepository( } databaseConnection - .writeableDatabase + .writableDatabase .delete(SAVEPOINTS_TABLE_NAME) } diff --git a/db/src/main/java/org/odk/collect/db/sqlite/DatabaseConnection.kt b/db/src/main/java/org/odk/collect/db/sqlite/DatabaseConnection.kt index b709ed212e1..b5690057d99 100644 --- a/db/src/main/java/org/odk/collect/db/sqlite/DatabaseConnection.kt +++ b/db/src/main/java/org/odk/collect/db/sqlite/DatabaseConnection.kt @@ -26,7 +26,7 @@ open class DatabaseConnection @JvmOverloads constructor( private val databasePath = path + File.separator + name - val writeableDatabase: SQLiteDatabase + val writableDatabase: SQLiteDatabase get() { StrictMode.noteSlowCall("Accessing writable DB") return dbHelper.writableDatabase @@ -73,7 +73,7 @@ open class DatabaseConnection @JvmOverloads constructor( * * This can be dangerous if the database is being access by multiple threads as the connection * might be closed while a transaction is running or while another thread is using a - * [SQLiteDatabase] instance obtained via [writeableDatabase] or [readableDatabase]. Using + * [SQLiteDatabase] instance obtained via [writableDatabase] or [readableDatabase]. Using * [SynchronizedDatabaseConnection] is recommended in those scenarios. */ fun reset() { @@ -85,10 +85,9 @@ open class DatabaseConnection @JvmOverloads constructor( * used if a calls to [reset] will be made. * * Does not guarantee synchronized access if this or another [DatabaseConnection] for the - * same `.db` file uses [writeableDatabase] or [readableDatabase]. + * same `.db` file uses [writableDatabase] or [readableDatabase]. * [SynchronizedDatabaseConnection] can be used to ensure synchronized writes/reads to the * underlying DB. - * */ fun withSynchronizedConnection(block: DatabaseConnection.() -> T): T { return synchronized(dbHelper) { diff --git a/db/src/main/java/org/odk/collect/db/sqlite/SynchronizedDatabaseConnection.kt b/db/src/main/java/org/odk/collect/db/sqlite/SynchronizedDatabaseConnection.kt index 1400d3f30eb..21e63608aa4 100644 --- a/db/src/main/java/org/odk/collect/db/sqlite/SynchronizedDatabaseConnection.kt +++ b/db/src/main/java/org/odk/collect/db/sqlite/SynchronizedDatabaseConnection.kt @@ -27,7 +27,7 @@ class SynchronizedDatabaseConnection( body: SQLiteDatabase.() -> T ) { return withConnection { - writeableDatabase.transaction { + writableDatabase.transaction { body() } }