From 857d356615b86a690899bbde28dc41a46f82fb3a Mon Sep 17 00:00:00 2001 From: Julius Linus Date: Mon, 11 Dec 2023 11:10:01 -0600 Subject: [PATCH] Fixed bug with Room Overwriting old data - Created MigrationsTest - Created Manual Migration from 9 to 10 - New Database has additional primary key Signed-off-by: Julius Linus --- app/build.gradle | 8 ++- .../9.json | 11 +-- .../talk/migrations/MigrationsTest.kt | 69 +++++++++++++++++++ .../ArbitraryStorageManager.kt | 2 +- .../ConversationInfoActivity.kt | 1 - .../talk/data/source/local/Migrations.kt | 32 +++++++++ .../talk/data/source/local/TalkDatabase.kt | 11 ++- .../ArbitraryStoragesRepositoryImpl.kt | 4 +- .../data/storage/model/ArbitraryStorage.kt | 2 +- .../storage/model/ArbitraryStorageEntity.kt | 4 +- .../DatabaseStorageModule.java | 6 ++ 11 files changed, 135 insertions(+), 15 deletions(-) create mode 100644 app/src/androidTest/java/com/nextcloud/talk/migrations/MigrationsTest.kt diff --git a/app/build.gradle b/app/build.gradle index 99b086ee4c..256cc01a8a 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -23,9 +23,10 @@ * You should have received a copy of the GNU General Public License * along with this program. If not, see . */ -import com.github.spotbugs.snom.SpotBugsTask + import com.github.spotbugs.snom.Confidence import com.github.spotbugs.snom.Effort +import com.github.spotbugs.snom.SpotBugsTask apply plugin: 'com.android.application' apply plugin: 'kotlin-android' @@ -91,6 +92,10 @@ android { } } + sourceSets { + androidTest.assets.srcDirs += files("$projectDir/schemas".toString()) + } + testInstrumentationRunnerArgument "TEST_SERVER_URL", "${NC_TEST_SERVER_BASEURL}" testInstrumentationRunnerArgument "TEST_SERVER_USERNAME", "${NC_TEST_SERVER_USERNAME}" testInstrumentationRunnerArgument "TEST_SERVER_PASSWORD", "${NC_TEST_SERVER_PASSWORD}" @@ -239,6 +244,7 @@ dependencies { implementation "androidx.room:room-rxjava2:${roomVersion}" kapt "androidx.room:room-compiler:${roomVersion}" implementation "androidx.room:room-ktx:${roomVersion}" + androidTestImplementation "androidx.room:room-testing:2.6.1" implementation "org.parceler:parceler-api:$parcelerVersion" implementation 'eu.davidea:flexible-adapter:5.1.0' diff --git a/app/schemas/com.nextcloud.talk.data.source.local.TalkDatabase/9.json b/app/schemas/com.nextcloud.talk.data.source.local.TalkDatabase/9.json index 4b966b37e7..af743b7e3f 100644 --- a/app/schemas/com.nextcloud.talk.data.source.local.TalkDatabase/9.json +++ b/app/schemas/com.nextcloud.talk.data.source.local.TalkDatabase/9.json @@ -2,7 +2,7 @@ "formatVersion": 1, "database": { "version": 9, - "identityHash": "666fcc4bbbdf3ff121b8f1ace8fcbcb8", + "identityHash": "250a3a56f3943f0d72f7ca0aac08fd1e", "entities": [ { "tableName": "User", @@ -92,7 +92,7 @@ }, { "tableName": "ArbitraryStorage", - "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`accountIdentifier` INTEGER NOT NULL, `key` TEXT NOT NULL, `object` TEXT, `value` TEXT, PRIMARY KEY(`accountIdentifier`, `key`))", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`accountIdentifier` INTEGER NOT NULL, `key` TEXT NOT NULL, `object` TEXT NOT NULL, `value` TEXT, PRIMARY KEY(`accountIdentifier`, `key`))", "fields": [ { "fieldPath": "accountIdentifier", @@ -110,7 +110,7 @@ "fieldPath": "storageObject", "columnName": "object", "affinity": "TEXT", - "notNull": false + "notNull": true }, { "fieldPath": "value", @@ -123,7 +123,8 @@ "autoGenerate": false, "columnNames": [ "accountIdentifier", - "key" + "key", + "object" ] }, "indices": [], @@ -133,7 +134,7 @@ "views": [], "setupQueries": [ "CREATE TABLE IF NOT EXISTS room_master_table (id INTEGER PRIMARY KEY,identity_hash TEXT)", - "INSERT OR REPLACE INTO room_master_table (id,identity_hash) VALUES(42, '666fcc4bbbdf3ff121b8f1ace8fcbcb8')" + "INSERT OR REPLACE INTO room_master_table (id,identity_hash) VALUES(42, '250a3a56f3943f0d72f7ca0aac08fd1e')" ] } } \ No newline at end of file diff --git a/app/src/androidTest/java/com/nextcloud/talk/migrations/MigrationsTest.kt b/app/src/androidTest/java/com/nextcloud/talk/migrations/MigrationsTest.kt new file mode 100644 index 0000000000..af8e9bb970 --- /dev/null +++ b/app/src/androidTest/java/com/nextcloud/talk/migrations/MigrationsTest.kt @@ -0,0 +1,69 @@ +/* + * Nextcloud Talk application + * + * @author Julius Linus + * Copyright (C) 2023 Julius Linus + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +package com.nextcloud.talk.migrations + +import androidx.room.Room +import androidx.room.testing.MigrationTestHelper +import androidx.sqlite.db.framework.FrameworkSQLiteOpenHelperFactory +import androidx.test.platform.app.InstrumentationRegistry +import com.nextcloud.talk.data.source.local.Migrations +import com.nextcloud.talk.data.source.local.TalkDatabase +import org.junit.Rule +import org.junit.Test +import java.io.IOException + +class MigrationsTest { + + @get:Rule + val helper: MigrationTestHelper = MigrationTestHelper( + InstrumentationRegistry.getInstrumentation(), + TalkDatabase::class.java.canonicalName!!, + FrameworkSQLiteOpenHelperFactory() + ) + + @Test + @Throws(IOException::class) + fun migrateAll() { + // Create earliest version of the database. + helper.createDatabase(TEST_DB, 8).apply { + close() + } + + // Open latest version of the database. Room validates the schema + // once all migrations execute. + Room.databaseBuilder( + InstrumentationRegistry.getInstrumentation().targetContext, + TalkDatabase::class.java, + TEST_DB + ).addMigrations( + Migrations.MIGRATION_6_8, + Migrations.MIGRATION_7_8, + Migrations.MIGRATION_8_9, + Migrations.MIGRATION_9_10 + ).build().apply { + openHelper.writableDatabase.close() + } + } + + companion object { + private const val TEST_DB = "migration-test" + } +} diff --git a/app/src/main/java/com/nextcloud/talk/arbitrarystorage/ArbitraryStorageManager.kt b/app/src/main/java/com/nextcloud/talk/arbitrarystorage/ArbitraryStorageManager.kt index 27c2550e0e..dee554bf5b 100644 --- a/app/src/main/java/com/nextcloud/talk/arbitrarystorage/ArbitraryStorageManager.kt +++ b/app/src/main/java/com/nextcloud/talk/arbitrarystorage/ArbitraryStorageManager.kt @@ -25,7 +25,7 @@ import com.nextcloud.talk.data.storage.model.ArbitraryStorage import io.reactivex.Maybe class ArbitraryStorageManager(private val arbitraryStoragesRepository: ArbitraryStoragesRepository) { - fun storeStorageSetting(accountIdentifier: Long, key: String, value: String?, objectString: String?) { + fun storeStorageSetting(accountIdentifier: Long, key: String, value: String?, objectString: String) { arbitraryStoragesRepository.saveArbitraryStorage(ArbitraryStorage(accountIdentifier, key, objectString, value)) } diff --git a/app/src/main/java/com/nextcloud/talk/conversationinfo/ConversationInfoActivity.kt b/app/src/main/java/com/nextcloud/talk/conversationinfo/ConversationInfoActivity.kt index e04bd2ac6f..d72248409f 100644 --- a/app/src/main/java/com/nextcloud/talk/conversationinfo/ConversationInfoActivity.kt +++ b/app/src/main/java/com/nextcloud/talk/conversationinfo/ConversationInfoActivity.kt @@ -166,7 +166,6 @@ class ConversationInfoActivity : override fun onResume() { super.onResume() - if (databaseStorageModule == null) { databaseStorageModule = DatabaseStorageModule(conversationUser, conversationToken) } diff --git a/app/src/main/java/com/nextcloud/talk/data/source/local/Migrations.kt b/app/src/main/java/com/nextcloud/talk/data/source/local/Migrations.kt index aca16d0a6b..2704f4f568 100644 --- a/app/src/main/java/com/nextcloud/talk/data/source/local/Migrations.kt +++ b/app/src/main/java/com/nextcloud/talk/data/source/local/Migrations.kt @@ -47,6 +47,13 @@ object Migrations { } } + val MIGRATION_9_10 = object : Migration(9, 10) { + override fun migrate(db: SupportSQLiteDatabase) { + Log.i("Migrations", "Migrating 9 to 10") + migrateToTriplePrimaryKeyArbitraryStorage(db) + } + } + fun migrateToRoom(db: SupportSQLiteDatabase) { db.execSQL( "CREATE TABLE User_new (" + @@ -124,4 +131,29 @@ object Migrations { // Change the table name to the correct one db.execSQL("ALTER TABLE ArbitraryStorage_dualPK RENAME TO ArbitraryStorage") } + + fun migrateToTriplePrimaryKeyArbitraryStorage(db: SupportSQLiteDatabase) { + db.execSQL( + "CREATE TABLE ArbitraryStorage_triplePK (" + + "accountIdentifier INTEGER NOT NULL, " + + "value TEXT, " + + "\"key\" TEXT NOT NULL, " + + "object TEXT NOT NULL, " + + "PRIMARY KEY(accountIdentifier, \"key\", object)" + + ")" + ) + // Copy the data + db.execSQL( + "INSERT INTO ArbitraryStorage_triplePK (" + + "accountIdentifier, \"key\", object, value) " + + "SELECT " + + "accountIdentifier, \"key\", object, value " + + "FROM ArbitraryStorage" + ) + // Remove the old table + db.execSQL("DROP TABLE ArbitraryStorage") + + // Change the table name to the correct one + db.execSQL("ALTER TABLE ArbitraryStorage_triplePK RENAME TO ArbitraryStorage") + } } diff --git a/app/src/main/java/com/nextcloud/talk/data/source/local/TalkDatabase.kt b/app/src/main/java/com/nextcloud/talk/data/source/local/TalkDatabase.kt index 65eb7ee5fe..b043da2a62 100644 --- a/app/src/main/java/com/nextcloud/talk/data/source/local/TalkDatabase.kt +++ b/app/src/main/java/com/nextcloud/talk/data/source/local/TalkDatabase.kt @@ -45,7 +45,7 @@ import java.util.Locale @Database( entities = [UserEntity::class, ArbitraryStorageEntity::class], - version = 9, + version = 10, exportSchema = true ) @TypeConverters( @@ -94,9 +94,14 @@ abstract class TalkDatabase : RoomDatabase() { return Room .databaseBuilder(context.applicationContext, TalkDatabase::class.java, dbName) - // comment out openHelperFactory to view the database entries in Android Studio for debugging + // NOTE: comment out openHelperFactory to view the database entries in Android Studio for debugging .openHelperFactory(factory) - .addMigrations(Migrations.MIGRATION_6_8, Migrations.MIGRATION_7_8, Migrations.MIGRATION_8_9) + .addMigrations( + Migrations.MIGRATION_6_8, + Migrations.MIGRATION_7_8, + Migrations.MIGRATION_8_9, + Migrations.MIGRATION_9_10 + ) .allowMainThreadQueries() .addCallback( object : RoomDatabase.Callback() { diff --git a/app/src/main/java/com/nextcloud/talk/data/storage/ArbitraryStoragesRepositoryImpl.kt b/app/src/main/java/com/nextcloud/talk/data/storage/ArbitraryStoragesRepositoryImpl.kt index 6eb07cf761..558fa5ab2b 100644 --- a/app/src/main/java/com/nextcloud/talk/data/storage/ArbitraryStoragesRepositoryImpl.kt +++ b/app/src/main/java/com/nextcloud/talk/data/storage/ArbitraryStoragesRepositoryImpl.kt @@ -33,7 +33,9 @@ class ArbitraryStoragesRepositoryImpl(private val arbitraryStoragesDao: Arbitrar ): Maybe { return arbitraryStoragesDao .getStorageSetting(accountIdentifier, key, objectString) - .map { ArbitraryStorageMapper.toModel(it) } + .map { + ArbitraryStorageMapper.toModel(it) + } } override fun getAll(): Maybe> { diff --git a/app/src/main/java/com/nextcloud/talk/data/storage/model/ArbitraryStorage.kt b/app/src/main/java/com/nextcloud/talk/data/storage/model/ArbitraryStorage.kt index 56b93b38b4..7f2f4ae569 100644 --- a/app/src/main/java/com/nextcloud/talk/data/storage/model/ArbitraryStorage.kt +++ b/app/src/main/java/com/nextcloud/talk/data/storage/model/ArbitraryStorage.kt @@ -27,6 +27,6 @@ import kotlinx.parcelize.Parcelize data class ArbitraryStorage( var accountIdentifier: Long, var key: String, - var storageObject: String? = null, + var storageObject: String, var value: String? = null ) : Parcelable diff --git a/app/src/main/java/com/nextcloud/talk/data/storage/model/ArbitraryStorageEntity.kt b/app/src/main/java/com/nextcloud/talk/data/storage/model/ArbitraryStorageEntity.kt index ab5f7dc626..b54c3dc39c 100644 --- a/app/src/main/java/com/nextcloud/talk/data/storage/model/ArbitraryStorageEntity.kt +++ b/app/src/main/java/com/nextcloud/talk/data/storage/model/ArbitraryStorageEntity.kt @@ -26,7 +26,7 @@ import androidx.room.Entity import kotlinx.parcelize.Parcelize @Parcelize -@Entity(tableName = "ArbitraryStorage", primaryKeys = ["accountIdentifier", "key"]) +@Entity(tableName = "ArbitraryStorage", primaryKeys = ["accountIdentifier", "key", "object"]) data class ArbitraryStorageEntity( @ColumnInfo(name = "accountIdentifier") var accountIdentifier: Long = 0, @@ -35,7 +35,7 @@ data class ArbitraryStorageEntity( var key: String = "", @ColumnInfo(name = "object") - var storageObject: String? = null, + var storageObject: String = "", @ColumnInfo(name = "value") var value: String? = null diff --git a/app/src/main/java/com/nextcloud/talk/utils/preferences/preferencestorage/DatabaseStorageModule.java b/app/src/main/java/com/nextcloud/talk/utils/preferences/preferencestorage/DatabaseStorageModule.java index cc4365c55b..a93fa82910 100644 --- a/app/src/main/java/com/nextcloud/talk/utils/preferences/preferencestorage/DatabaseStorageModule.java +++ b/app/src/main/java/com/nextcloud/talk/utils/preferences/preferencestorage/DatabaseStorageModule.java @@ -251,4 +251,10 @@ public String getString(String key, String defaultVal) { public void setMessageExpiration(int messageExpiration) { this.messageExpiration = messageExpiration; } + + @androidx.annotation.NonNull + public String toString() { + return "Conversation token: " + conversationToken + + "\nAccount Number: " + accountIdentifier; + } }