Skip to content

Commit d0dfdda

Browse files
grzesiek2010grzesiek2010
authored
Merge pull request #6402 from seadowg/alter-table
Fix bad queries after adding new propreties to entity list
2 parents c7f5b15 + a154665 commit d0dfdda

File tree

9 files changed

+235
-136
lines changed

9 files changed

+235
-136
lines changed

‎collect_app/src/main/java/org/odk/collect/android/database/entities/DatabaseEntitiesRepository.kt

Lines changed: 112 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -4,21 +4,20 @@ import android.content.ContentValues
44
import android.content.Context
55
import android.database.Cursor
66
import android.database.sqlite.SQLiteDatabase
7-
import android.database.sqlite.SQLiteException
87
import android.provider.BaseColumns._ID
9-
import androidx.core.database.sqlite.transaction
108
import org.odk.collect.db.sqlite.CursorExt.first
119
import org.odk.collect.db.sqlite.CursorExt.foldAndClose
1210
import org.odk.collect.db.sqlite.CursorExt.getInt
1311
import org.odk.collect.db.sqlite.CursorExt.getString
1412
import org.odk.collect.db.sqlite.CursorExt.getStringOrNull
1513
import org.odk.collect.db.sqlite.CursorExt.rowToMap
16-
import org.odk.collect.db.sqlite.DatabaseConnection
1714
import org.odk.collect.db.sqlite.DatabaseMigrator
1815
import org.odk.collect.db.sqlite.SQLiteColumns.ROW_ID
1916
import org.odk.collect.db.sqlite.SQLiteDatabaseExt.delete
2017
import org.odk.collect.db.sqlite.SQLiteDatabaseExt.doesColumnExist
18+
import org.odk.collect.db.sqlite.SQLiteDatabaseExt.getColumnNames
2119
import org.odk.collect.db.sqlite.SQLiteDatabaseExt.query
20+
import org.odk.collect.db.sqlite.SynchronizedDatabaseConnection
2221
import org.odk.collect.entities.storage.EntitiesRepository
2322
import org.odk.collect.entities.storage.Entity
2423

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

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

42-
private val databaseConnection: DatabaseConnection = DatabaseConnection(
41+
private val databaseConnection = SynchronizedDatabaseConnection(
4342
context,
4443
dbPath,
4544
"entities.db",
4645
EntitiesDatabaseMigrator(),
47-
1,
48-
true
46+
1
4947
)
5048

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

6159
updatePropertyColumns(list, entities.first())
6260

63-
databaseConnection.writeableDatabase.transaction {
61+
databaseConnection.transaction {
6462
entities.forEach { entity ->
6563
val existing = if (listExists) {
6664
query(
@@ -121,32 +119,34 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep
121119
}
122120

123121
override fun getLists(): Set<String> {
124-
return databaseConnection
125-
.readableDatabase
126-
.query(ListsTable.TABLE_NAME)
127-
.foldAndClose(emptySet()) { set, cursor -> set + cursor.getString(ListsTable.COLUMN_NAME) }
122+
return databaseConnection.withConnection {
123+
readableDatabase
124+
.query(ListsTable.TABLE_NAME)
125+
.foldAndClose(emptySet()) { set, cursor -> set + cursor.getString(ListsTable.COLUMN_NAME) }
126+
}
128127
}
129128

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

135-
databaseConnection
136-
.writeableDatabase
137-
.update(
134+
databaseConnection.withConnection {
135+
writableDatabase.update(
138136
ListsTable.TABLE_NAME,
139137
contentValues,
140138
"${ListsTable.COLUMN_NAME} = ?",
141139
arrayOf(list)
142140
)
141+
}
143142
}
144143

145144
override fun getListHash(list: String): String? {
146-
return databaseConnection
147-
.readableDatabase
148-
.query(ListsTable.TABLE_NAME, "${ListsTable.COLUMN_NAME} = ?", arrayOf(list))
149-
.first { it.getStringOrNull(ListsTable.COLUMN_HASH) }
145+
return databaseConnection.withConnection {
146+
readableDatabase
147+
.query(ListsTable.TABLE_NAME, "${ListsTable.COLUMN_NAME} = ?", arrayOf(list))
148+
.first { it.getStringOrNull(ListsTable.COLUMN_HASH) }
149+
}
150150
}
151151

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

171-
return databaseConnection.readableDatabase.rawQuery(
172-
"""
173-
SELECT COUNT(*)
174-
FROM $list
175-
""".trimIndent(),
176-
null
177-
).first {
178-
it.getInt(0)
179-
}!!
171+
return databaseConnection.withConnection {
172+
readableDatabase.rawQuery(
173+
"""
174+
SELECT COUNT(*)
175+
FROM $list
176+
""".trimIndent(),
177+
null
178+
).first {
179+
it.getInt(0)
180+
}!!
181+
}
180182
}
181183

182184
override fun clear() {
183-
getLists().forEach {
184-
databaseConnection.writeableDatabase.delete(it)
185-
}
185+
databaseConnection.withConnection {
186+
getLists().forEach {
187+
writableDatabase.delete(it)
188+
}
186189

187-
databaseConnection.writeableDatabase.delete(ListsTable.TABLE_NAME)
190+
writableDatabase.delete(ListsTable.TABLE_NAME)
191+
}
188192
}
189193

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

197201
override fun delete(id: String) {
198-
getLists().forEach {
199-
databaseConnection.writeableDatabase.delete(
200-
it,
201-
"${EntitiesTable.COLUMN_ID} = ?",
202-
arrayOf(id)
203-
)
202+
databaseConnection.withConnection {
203+
getLists().forEach {
204+
writableDatabase.delete(
205+
it,
206+
"${EntitiesTable.COLUMN_ID} = ?",
207+
arrayOf(id)
208+
)
209+
}
204210
}
205211

206212
updateRowIdTables()
@@ -229,7 +235,11 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep
229235
return emptyList()
230236
}
231237

232-
return if (databaseConnection.readableDatabase.doesColumnExist(list, property)) {
238+
val propertyExists = databaseConnection.withConnection {
239+
readableDatabase.doesColumnExist(list, property)
240+
}
241+
242+
return if (propertyExists) {
233243
queryWithAttachedRowId(
234244
list,
235245
selectionColumn = property,
@@ -251,45 +261,50 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep
251261
return null
252262
}
253263

254-
return databaseConnection.readableDatabase
255-
.rawQuery(
256-
"""
257-
SELECT *, i.$ROW_ID
258-
FROM $list e, ${getRowIdTableName(list)} i
259-
WHERE e._id = i._id AND i.$ROW_ID = ?
260-
""".trimIndent(),
261-
arrayOf((index + 1).toString())
262-
).first {
263-
mapCursorRowToEntity(list, it, it.getInt(ROW_ID))
264-
}
264+
return databaseConnection.withConnection {
265+
readableDatabase
266+
.rawQuery(
267+
"""
268+
SELECT *, i.$ROW_ID
269+
FROM $list e, ${getRowIdTableName(list)} i
270+
WHERE e._id = i._id AND i.$ROW_ID = ?
271+
""".trimIndent(),
272+
arrayOf((index + 1).toString())
273+
).first {
274+
mapCursorRowToEntity(list, it, it.getInt(ROW_ID))
275+
}
276+
}
265277
}
266278

267279
private fun queryWithAttachedRowId(list: String): Cursor {
268-
return databaseConnection.readableDatabase
269-
.rawQuery(
270-
"""
271-
SELECT *, i.$ROW_ID
272-
FROM $list e, ${getRowIdTableName(list)} i
273-
WHERE e._id = i._id
274-
""".trimIndent(),
275-
null
276-
)
280+
return databaseConnection.withConnection {
281+
readableDatabase
282+
.rawQuery(
283+
"""
284+
SELECT *, i.$ROW_ID
285+
FROM $list e, ${getRowIdTableName(list)} i
286+
WHERE e._id = i._id
287+
""".trimIndent(),
288+
null
289+
)
290+
}
277291
}
278292

279293
private fun queryWithAttachedRowId(
280294
list: String,
281295
selectionColumn: String,
282296
selectionArg: String
283297
): Cursor {
284-
return databaseConnection.readableDatabase
285-
.rawQuery(
298+
return databaseConnection.withConnection {
299+
readableDatabase.rawQuery(
286300
"""
287301
SELECT *, i.$ROW_ID
288302
FROM $list e, ${getRowIdTableName(list)} i
289303
WHERE e._id = i._id AND $selectionColumn = ?
290304
""".trimIndent(),
291305
arrayOf(selectionArg)
292306
)
307+
}
293308
}
294309

295310
/**
@@ -300,34 +315,38 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep
300315
* function, but that's not available in all the supported versions of Android.
301316
*/
302317
private fun updateRowIdTables() {
303-
getLists().forEach {
304-
databaseConnection.writeableDatabase.execSQL(
305-
"""
306-
DROP TABLE IF EXISTS ${getRowIdTableName(it)};
307-
""".trimIndent()
308-
)
318+
databaseConnection.withConnection {
319+
getLists().forEach {
320+
writableDatabase.execSQL(
321+
"""
322+
DROP TABLE IF EXISTS ${getRowIdTableName(it)};
323+
""".trimIndent()
324+
)
309325

310-
databaseConnection.writeableDatabase.execSQL(
311-
"""
312-
CREATE TABLE ${getRowIdTableName(it)} AS SELECT _id FROM $it;
313-
""".trimIndent()
314-
)
326+
writableDatabase.execSQL(
327+
"""
328+
CREATE TABLE ${getRowIdTableName(it)} AS SELECT _id FROM $it;
329+
""".trimIndent()
330+
)
331+
}
315332
}
316333
}
317334

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

320337
private fun listExists(list: String): Boolean {
321-
return databaseConnection.readableDatabase
322-
.query(
323-
ListsTable.TABLE_NAME,
324-
selection = "${ListsTable.COLUMN_NAME} = ?",
325-
selectionArgs = arrayOf(list)
326-
).use { it.count } > 0
338+
return databaseConnection.withConnection {
339+
readableDatabase
340+
.query(
341+
ListsTable.TABLE_NAME,
342+
selection = "${ListsTable.COLUMN_NAME} = ?",
343+
selectionArgs = arrayOf(list)
344+
).use { it.count } > 0
345+
}
327346
}
328347

329348
private fun createList(list: String) {
330-
databaseConnection.writeableDatabase.transaction {
349+
databaseConnection.transaction {
331350
val contentValues = ContentValues()
332351
contentValues.put(ListsTable.COLUMN_NAME, list)
333352
insertOrThrow(
@@ -359,15 +378,21 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep
359378
}
360379

361380
private fun updatePropertyColumns(list: String, entity: Entity) {
362-
entity.properties.map { it.first }.forEach {
363-
try {
364-
databaseConnection.writeableDatabase.execSQL(
365-
"""
366-
ALTER TABLE $list ADD "$it" text NOT NULL DEFAULT "";
367-
""".trimIndent()
368-
)
369-
} catch (e: SQLiteException) {
370-
// Ignored
381+
val columnNames = databaseConnection.withConnection {
382+
readableDatabase.getColumnNames(list)
383+
}
384+
385+
val missingColumns =
386+
entity.properties.map { it.first }.filterNot { columnNames.contains(it) }
387+
if (missingColumns.isNotEmpty()) {
388+
databaseConnection.resetTransaction {
389+
missingColumns.forEach {
390+
execSQL(
391+
"""
392+
ALTER TABLE $list ADD "$it" text NOT NULL DEFAULT "";
393+
""".trimIndent()
394+
)
395+
}
371396
}
372397
}
373398
}

‎collect_app/src/main/java/org/odk/collect/android/database/forms/DatabaseFormsRepository.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -238,13 +238,13 @@ private Cursor queryAndReturnCursor(Map<String, String> projectionMap, String[]
238238
}
239239

240240
private Long insertForm(ContentValues values) {
241-
SQLiteDatabase writeableDatabase = databaseConnection.getWriteableDatabase();
242-
return writeableDatabase.insertOrThrow(FORMS_TABLE_NAME, null, values);
241+
SQLiteDatabase writableDatabase = databaseConnection.getWritableDatabase();
242+
return writableDatabase.insertOrThrow(FORMS_TABLE_NAME, null, values);
243243
}
244244

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

250250
private void deleteForms(String selection, String[] selectionArgs) {
@@ -255,8 +255,8 @@ private void deleteForms(String selection, String[] selectionArgs) {
255255
deleteFilesForForm(form);
256256
}
257257

258-
SQLiteDatabase writeableDatabase = databaseConnection.getWriteableDatabase();
259-
writeableDatabase.delete(FORMS_TABLE_NAME, selection, selectionArgs);
258+
SQLiteDatabase writableDatabase = databaseConnection.getWritableDatabase();
259+
writableDatabase.delete(FORMS_TABLE_NAME, selection, selectionArgs);
260260
}
261261

262262
@NotNull

‎collect_app/src/main/java/org/odk/collect/android/database/instances/DatabaseInstancesRepository.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ public List<Instance> getAllNotDeletedByFormIdAndVersion(String jrFormId, String
144144
public void delete(Long id) {
145145
Instance instance = get(id);
146146

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

160-
databaseConnection.getWriteableDatabase().delete(
160+
databaseConnection.getWritableDatabase().delete(
161161
INSTANCES_TABLE_NAME,
162162
null,
163163
null
@@ -253,15 +253,15 @@ private Cursor query(String[] projection, String selection, String[] selectionAr
253253
}
254254

255255
private long insert(ContentValues values) {
256-
return databaseConnection.getWriteableDatabase().insertOrThrow(
256+
return databaseConnection.getWritableDatabase().insertOrThrow(
257257
INSTANCES_TABLE_NAME,
258258
null,
259259
values
260260
);
261261
}
262262

263263
private void update(Long instanceId, ContentValues values) {
264-
databaseConnection.getWriteableDatabase().update(
264+
databaseConnection.getWritableDatabase().update(
265265
INSTANCES_TABLE_NAME,
266266
values,
267267
_ID + "=?",

0 commit comments

Comments
 (0)