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 <julius.linus@nextcloud.com>

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 <http://www.gnu.org/licenses/>.
  */
-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'

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')"
     ]
   }
 }

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 <julius.linus@nextcloud.com>
+ *
+ * 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 <http://www.gnu.org/licenses/>.
+ */
+
+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"
+    }
+}

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))
     }
 

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)
         }

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")
+    }
 }

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() {

app/src/main/java/com/nextcloud/talk/data/storage/ArbitraryStoragesRepositoryImpl.kt

@@ -33,7 +33,9 @@ class ArbitraryStoragesRepositoryImpl(private val arbitraryStoragesDao: Arbitrar
     ): Maybe<ArbitraryStorage> {
         return arbitraryStoragesDao
             .getStorageSetting(accountIdentifier, key, objectString)
-            .map { ArbitraryStorageMapper.toModel(it) }
+            .map {
+                ArbitraryStorageMapper.toModel(it)
+            }
     }
 
     override fun getAll(): Maybe<List<ArbitraryStorageEntity>> {

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

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

app/src/main/java/com/nextcloud/talk/utils/preferences/preferencestorage/DatabaseStorageModule.java

@@ -251,4 +251,10 @@ public class DatabaseStorageModule {
     public void setMessageExpiration(int messageExpiration) {
         this.messageExpiration = messageExpiration;
     }
+
+    @androidx.annotation.NonNull
+    public String toString() {
+        return "Conversation token: " + conversationToken
+            + "\nAccount Number: " + accountIdentifier;
+    }
 }