Merge pull request #2264 from vector-im/feature/ons/fix_riotx_crypto_migration

Fix crpyto store migration from RiotX.

CHANGES.md

@@ -59,6 +59,7 @@ Bugfix 🐛:
  - Don't set presence when handling a push notification or polling (#2156)
  - Be robust against `StrandHogg` task injection
  - Clear alerts if user sign out
+ - Messages encrypted with no way to decrypt after SDK update from 0.18 to 1.0.0 (#2252)
  - Fix rows are hidden in Textinput (#2234)
  - Uploading a file to a room caused it to have a info.size of -1 (#2141)
 

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/db/RealmCryptoStoreMigration.kt

@@ -43,6 +43,7 @@ import org.matrix.android.sdk.internal.crypto.store.db.model.WithHeldSessionEnti
 import org.matrix.android.sdk.internal.di.SerializeNulls
 import io.realm.DynamicRealm
 import io.realm.RealmMigration
+import io.realm.RealmObjectSchema
 import org.matrix.androidsdk.crypto.data.MXOlmInboundGroupSession2
 import timber.log.Timber
 import javax.inject.Inject
@@ -57,6 +58,27 @@ internal class RealmCryptoStoreMigration @Inject constructor(private val crossSi
         const val CRYPTO_STORE_SCHEMA_VERSION = 11L
     }
 
+    private fun RealmObjectSchema.addFieldIfNotExists(fieldName: String, fieldType: Class<*>): RealmObjectSchema {
+        if (!hasField(fieldName)) {
+            addField(fieldName, fieldType)
+        }
+        return this
+    }
+
+    private fun RealmObjectSchema.removeFieldIfExists(fieldName: String): RealmObjectSchema {
+        if (hasField(fieldName)) {
+            removeField(fieldName)
+        }
+        return this
+    }
+
+    private fun RealmObjectSchema.setRequiredIfNotAlready(fieldName: String, isRequired: Boolean): RealmObjectSchema {
+        if (isRequired != isRequired(fieldName)) {
+            setRequired(fieldName, isRequired)
+        }
+        return this
+    }
+
     override fun migrate(realm: DynamicRealm, oldVersion: Long, newVersion: Long) {
         Timber.v("Migrating Realm Crypto from $oldVersion to $newVersion")
 
@@ -89,13 +111,13 @@ internal class RealmCryptoStoreMigration @Inject constructor(private val crossSi
         Timber.d("Update IncomingRoomKeyRequestEntity format: requestBodyString field is exploded into several fields")
 
         realm.schema.get("IncomingRoomKeyRequestEntity")
-                ?.addField("requestBodyAlgorithm", String::class.java)
+                ?.addFieldIfNotExists("requestBodyAlgorithm", String::class.java)
-                ?.addField("requestBodyRoomId", String::class.java)
+                ?.addFieldIfNotExists("requestBodyRoomId", String::class.java)
-                ?.addField("requestBodySenderKey", String::class.java)
+                ?.addFieldIfNotExists("requestBodySenderKey", String::class.java)
-                ?.addField("requestBodySessionId", String::class.java)
+                ?.addFieldIfNotExists("requestBodySessionId", String::class.java)
                 ?.transform { dynamicObject ->
-                    val requestBodyString = dynamicObject.getString("requestBodyString")
                     try {
+                        val requestBodyString = dynamicObject.getString("requestBodyString")
                         // It was a map before
                         val map: Map<String, String>? = deserializeFromRealm(requestBodyString)
 
@@ -109,18 +131,18 @@ internal class RealmCryptoStoreMigration @Inject constructor(private val crossSi
                         Timber.e(e, "Error")
                     }
                 }
-                ?.removeField("requestBodyString")
+                ?.removeFieldIfExists("requestBodyString")
 
         Timber.d("Update IncomingRoomKeyRequestEntity format: requestBodyString field is exploded into several fields")
 
         realm.schema.get("OutgoingRoomKeyRequestEntity")
-                ?.addField("requestBodyAlgorithm", String::class.java)
+                ?.addFieldIfNotExists("requestBodyAlgorithm", String::class.java)
-                ?.addField("requestBodyRoomId", String::class.java)
+                ?.addFieldIfNotExists("requestBodyRoomId", String::class.java)
-                ?.addField("requestBodySenderKey", String::class.java)
+                ?.addFieldIfNotExists("requestBodySenderKey", String::class.java)
-                ?.addField("requestBodySessionId", String::class.java)
+                ?.addFieldIfNotExists("requestBodySessionId", String::class.java)
                 ?.transform { dynamicObject ->
-                    val requestBodyString = dynamicObject.getString("requestBodyString")
                     try {
+                        val requestBodyString = dynamicObject.getString("requestBodyString")
                         // It was a map before
                         val map: Map<String, String>? = deserializeFromRealm(requestBodyString)
 
@@ -134,16 +156,18 @@ internal class RealmCryptoStoreMigration @Inject constructor(private val crossSi
                         Timber.e(e, "Error")
                     }
                 }
-                ?.removeField("requestBodyString")
+                ?.removeFieldIfExists("requestBodyString")
 
         Timber.d("Create KeysBackupDataEntity")
 
-        realm.schema.create("KeysBackupDataEntity")
+        if (!realm.schema.contains("KeysBackupDataEntity")) {
-                .addField(KeysBackupDataEntityFields.PRIMARY_KEY, Integer::class.java)
+            realm.schema.create("KeysBackupDataEntity")
-                .addPrimaryKey(KeysBackupDataEntityFields.PRIMARY_KEY)
+                    .addField(KeysBackupDataEntityFields.PRIMARY_KEY, Integer::class.java)
-                .setRequired(KeysBackupDataEntityFields.PRIMARY_KEY, true)
+                    .addPrimaryKey(KeysBackupDataEntityFields.PRIMARY_KEY)
-                .addField(KeysBackupDataEntityFields.BACKUP_LAST_SERVER_HASH, String::class.java)
+                    .setRequired(KeysBackupDataEntityFields.PRIMARY_KEY, true)
-                .addField(KeysBackupDataEntityFields.BACKUP_LAST_SERVER_NUMBER_OF_KEYS, Integer::class.java)
+                    .addField(KeysBackupDataEntityFields.BACKUP_LAST_SERVER_HASH, String::class.java)
+                    .addField(KeysBackupDataEntityFields.BACKUP_LAST_SERVER_NUMBER_OF_KEYS, Integer::class.java)
+        }
     }
 
     private fun migrateTo3RiotX(realm: DynamicRealm) {
@@ -151,8 +175,8 @@ internal class RealmCryptoStoreMigration @Inject constructor(private val crossSi
         Timber.d("Migrate to RiotX model")
 
         realm.schema.get("CryptoRoomEntity")
-                ?.addField(CryptoRoomEntityFields.SHOULD_ENCRYPT_FOR_INVITED_MEMBERS, Boolean::class.java)
+                ?.addFieldIfNotExists(CryptoRoomEntityFields.SHOULD_ENCRYPT_FOR_INVITED_MEMBERS, Boolean::class.java)
-                ?.setRequired(CryptoRoomEntityFields.SHOULD_ENCRYPT_FOR_INVITED_MEMBERS, false)
+                ?.setRequiredIfNotAlready(CryptoRoomEntityFields.SHOULD_ENCRYPT_FOR_INVITED_MEMBERS, false)
 
         // Convert format of MXDeviceInfo, package has to be the same.
         realm.schema.get("DeviceInfoEntity")
@@ -204,8 +228,13 @@ internal class RealmCryptoStoreMigration @Inject constructor(private val crossSi
     // Version 4L added Cross Signing info persistence
     private fun migrateTo4(realm: DynamicRealm) {
         Timber.d("Step 3 -> 4")
-        Timber.d("Create KeyInfoEntity")
 
+        if (realm.schema.contains("TrustLevelEntity")) {
+            Timber.d("Skipping Step 3 -> 4 because entities already exist")
+            return
+        }
+
+        Timber.d("Create KeyInfoEntity")
         val trustLevelEntityEntitySchema = realm.schema.create("TrustLevelEntity")
                 .addField(TrustLevelEntityFields.CROSS_SIGNED_VERIFIED, Boolean::class.java)
                 .setNullable(TrustLevelEntityFields.CROSS_SIGNED_VERIFIED, true)