Ensure that remapped records are valid.

Fixes #12691

app/src/main/java/org/thoughtcrime/securesms/database/RemappedRecordTables.kt

@@ -3,6 +3,8 @@ package org.thoughtcrime.securesms.database
 import android.content.Context
 import android.database.Cursor
 import androidx.core.content.contentValuesOf
+import org.signal.core.util.delete
+import org.signal.core.util.logging.Log
 import org.signal.core.util.readToList
 import org.signal.core.util.requireLong
 import org.signal.core.util.select
@@ -19,6 +21,8 @@ import java.util.HashMap
 class RemappedRecordTables internal constructor(context: Context?, databaseHelper: SignalDatabase?) : DatabaseTable(context, databaseHelper) {
 
   companion object {
+    val TAG = Log.tag(RemappedRecordTables::class.java)
+
     val CREATE_TABLE = arrayOf(Recipients.CREATE_TABLE, Threads.CREATE_TABLE)
   }
 
@@ -51,6 +55,8 @@ class RemappedRecordTables internal constructor(context: Context?, databaseHelpe
   }
 
   fun getAllRecipientMappings(): Map<RecipientId, RecipientId> {
+    clearInvalidRecipientMappings()
+
     val recipientMap: MutableMap<RecipientId, RecipientId> = HashMap()
 
     readableDatabase.withinTransaction { db ->
@@ -66,6 +72,8 @@ class RemappedRecordTables internal constructor(context: Context?, databaseHelpe
   }
 
   fun getAllThreadMappings(): Map<Long, Long> {
+    clearInvalidThreadMappings()
+
     val threadMap: MutableMap<Long, Long> = HashMap()
 
     readableDatabase.withinTransaction { db ->
@@ -87,6 +95,7 @@ class RemappedRecordTables internal constructor(context: Context?, databaseHelpe
   }
 
   fun getAllRecipients(): Cursor {
+    clearInvalidRecipientMappings()
     return readableDatabase
       .select()
       .from(Recipients.TABLE_NAME)
@@ -94,6 +103,7 @@ class RemappedRecordTables internal constructor(context: Context?, databaseHelpe
   }
 
   fun getAllThreads(): Cursor {
+    clearInvalidThreadMappings()
     return readableDatabase
       .select()
       .from(Threads.TABLE_NAME)
@@ -120,5 +130,33 @@ class RemappedRecordTables internal constructor(context: Context?, databaseHelpe
     databaseHelper.signalWritableDatabase.insert(table, null, values)
   }
 
+  /**
+   * The old_id should never exist -- this class is intended to remap from IDs that were deleted.
+   */
+  private fun clearInvalidRecipientMappings() {
+    val count = writableDatabase
+      .delete(Recipients.TABLE_NAME)
+      .where("$OLD_ID IN (SELECT ${RecipientTable.ID} FROM ${RecipientTable.TABLE_NAME})")
+      .run()
+
+    if (count > 0) {
+      Log.w(TAG, "Deleted $count invalid recipient mappings!", true)
+    }
+  }
+
+  /**
+   * The old_id should never exist -- this class is intended to remap from IDs that were deleted.
+   */
+  private fun clearInvalidThreadMappings() {
+    val count = writableDatabase
+      .delete(Threads.TABLE_NAME)
+      .where("$OLD_ID IN (SELECT ${ThreadTable.ID} FROM ${ThreadTable.TABLE_NAME})")
+      .run()
+
+    if (count > 0) {
+      Log.w(TAG, "Deleted $count invalid recipient mappings!", true)
+    }
+  }
+
   private class Mapping(val oldId: Long, val newId: Long)
 }

app/src/main/java/org/thoughtcrime/securesms/database/RemappedRecords.java

@@ -20,7 +20,7 @@ import java.util.Set;
  *
  * There should be very few of these, so we keep them in a fast, lazily-loaded memory cache.
  *
- * One important thing to note is that this class will often be accesses inside of database
+ * One important thing to note is that this class will often be accessed inside of database
  * transactions. As a result, it cannot attempt to acquire a database lock while holding a
  * separate lock. Instead, we use the database lock itself as a locking mechanism.
  */

app/src/main/java/org/thoughtcrime/securesms/database/helpers/migration/V166_ThreadAndMessageForeignKeys.kt

@@ -23,7 +23,7 @@ object V166_ThreadAndMessageForeignKeys : SignalDatabaseMigration {
     // Some crashes we were seeing indicated that we may have been running this migration twice on some unlucky devices, likely due
     // to some gaps that were left between some transactions during the upgrade path.
     if (!SqlUtil.columnExists(db, "thread", "thread_recipient_id")) {
-      Log.w(TAG, "Migration must have already run! Skipping.")
+      Log.w(TAG, "Migration must have already run! Skipping.", true)
       return
     }
 
@@ -63,7 +63,7 @@ object V166_ThreadAndMessageForeignKeys : SignalDatabaseMigration {
       while (cursor.moveToNext()) {
         val recipientId = cursor.requireLong("thread_recipient_id")
         val count = cursor.requireLong("thread_count")
-        Log.w(TAG, "There were $count threads for RecipientId::$recipientId. Merging.")
+        Log.w(TAG, "There were $count threads for RecipientId::$recipientId. Merging.", true)
 
         val threads: List<ThreadInfo> = getThreadsByRecipientId(db, cursor.requireLong("thread_recipient_id"))
         mergeThreads(db, threads)
@@ -112,6 +112,14 @@ object V166_ThreadAndMessageForeignKeys : SignalDatabaseMigration {
       .where("thread_id = ?", secondaryId)
       .run()
 
+    // We're dealing with threads that exist, so we don't need to remap old_ids
+
+    val count = db.update("remapped_threads")
+      .values("new_id" to primaryId)
+      .where("new_id = ?", secondaryId)
+      .run()
+    Log.w(TAG, "Remapped $count remapped_threads new_ids from $secondaryId to $primaryId", true)
+
     db.delete("thread")
       .where("_id = ?", secondaryId)
       .run()