Fix bug where username may be put in e164 column.

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

@@ -3,6 +3,7 @@ package org.thoughtcrime.securesms.database.helpers
 import android.app.Application
 import android.content.Context
 import net.zetetic.database.sqlcipher.SQLiteDatabase
+import org.signal.core.util.areForeignKeyConstraintsEnabled
 import org.signal.core.util.logging.Log
 import org.signal.core.util.withinTransaction
 import org.thoughtcrime.securesms.database.helpers.migration.SignalDatabaseMigration
@@ -69,6 +70,7 @@ import org.thoughtcrime.securesms.database.helpers.migration.V209_ClearRecipient
 import org.thoughtcrime.securesms.database.helpers.migration.V210_FixPniPossibleColumns
 import org.thoughtcrime.securesms.database.helpers.migration.V211_ReceiptColumnRenames
 import org.thoughtcrime.securesms.database.helpers.migration.V212_RemoveDistributionListUniqueConstraint
+import org.thoughtcrime.securesms.database.helpers.migration.V213_FixUsernameInE164Column
 
 /**
  * Contains all of the database migrations for [SignalDatabase]. Broken into a separate file for cleanliness.
@@ -77,8 +79,6 @@ object SignalDatabaseMigrations {
 
   val TAG: String = Log.tag(SignalDatabaseMigrations.javaClass)
 
-  const val DATABASE_VERSION = 212
-
   private val migrations: List<Pair<Int, SignalDatabaseMigration>> = listOf(
     149 to V149_LegacyMigrations,
     150 to V150_UrgentMslFlagMigration,
@@ -143,23 +143,34 @@ object SignalDatabaseMigrations {
     209 to V209_ClearRecipientPniFromAciColumn,
     210 to V210_FixPniPossibleColumns,
     211 to V211_ReceiptColumnRenames,
-    212 to V212_RemoveDistributionListUniqueConstraint
+    212 to V212_RemoveDistributionListUniqueConstraint,
+    213 to V213_FixUsernameInE164Column
   )
 
+  const val DATABASE_VERSION = 213
+
   @JvmStatic
   fun migrate(context: Application, db: SQLiteDatabase, oldVersion: Int, newVersion: Int) {
+    val initialForeignKeyState = db.areForeignKeyConstraintsEnabled()
+
     for (migrationData in migrations) {
       val (version, migration) = migrationData
 
       if (oldVersion < version) {
-        Log.i(TAG, "Running migration for version $version: ${migration.javaClass.simpleName}")
+        Log.i(TAG, "Running migration for version $version: ${migration.javaClass.simpleName}. Foreign keys: ${migration.enableForeignKeys}")
+        val startTime = System.currentTimeMillis()
+
+        db.setForeignKeyConstraintsEnabled(migration.enableForeignKeys)
         db.withinTransaction {
           migration.migrate(context, db, oldVersion, newVersion)
           db.version = version
         }
-        Log.i(TAG, "Successfully completed migration for version $version.")
+
+        Log.i(TAG, "Successfully completed migration for version $version in ${System.currentTimeMillis() - startTime} ms")
       }
     }
+
+    db.setForeignKeyConstraintsEnabled(initialForeignKeyState)
   }
 
   @JvmStatic

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

@@ -7,5 +7,9 @@ import net.zetetic.database.sqlcipher.SQLiteDatabase
  * Simple interface for allowing database migrations to live outside of [org.thoughtcrime.securesms.database.helpers.SignalDatabaseMigrations].
  */
 interface SignalDatabaseMigration {
+  /** True if you want foreign key constraints to be enforced during a migration, otherwise false. Defaults to false. */
+  val enableForeignKeys: Boolean
+    get() = false
+
   fun migrate(context: Application, db: SQLiteDatabase, oldVersion: Int, newVersion: Int)
 }

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

@@ -0,0 +1,56 @@
+/*
+ * Copyright 2023 Signal Messenger, LLC
+ * SPDX-License-Identifier: AGPL-3.0-only
+ */
+
+package org.thoughtcrime.securesms.database.helpers.migration
+
+import android.app.Application
+import net.zetetic.database.sqlcipher.SQLiteDatabase
+import org.signal.core.util.logging.Log
+
+/**
+ * There was a bug where adding a member to a group by username could put that username in the e164 column.
+ * We have to clean it up and move that value to the username column.
+ */
+object V213_FixUsernameInE164Column : SignalDatabaseMigration {
+
+  private val TAG = Log.tag(V213_FixUsernameInE164Column::class.java)
+
+  /** We rely on foreign keys to clean up data from bad recipients */
+  override val enableForeignKeys: Boolean = true
+
+  override fun migrate(context: Application, db: SQLiteDatabase, oldVersion: Int, newVersion: Int) {
+    // In order to avoid unique constraint violations, we run this query once to move over everything that doesn't break any violations...
+    db.execSQL(
+      """
+      UPDATE 
+        recipient
+      SET 
+        username = e164,
+        e164 = NULL
+      WHERE 
+        e164 GLOB '[a-zA-Z][a-zA-Z0-9][a-zA-Z0-9]*.[0-9][0-9]*'
+          AND e164 NOT IN (SELECT username FROM recipient WHERE username IS NOT NULL)
+      """
+    )
+
+    // ...and again to just clear out any remaining bad data. This should only clear data that would otherwise violate the unique constraint.
+    db.execSQL(
+      """
+      UPDATE 
+        recipient
+      SET 
+        e164 = NULL
+      WHERE 
+        e164 GLOB '[a-zA-Z][a-zA-Z0-9][a-zA-Z0-9]*.[0-9][0-9]*'
+      """
+    )
+
+    // Finally, the above queries may have created recipients that have a username but no ACI. These are invalid entries that need to be trimmed.
+    // Given that usernames are not public, we'll rely on cascading deletes here to clean things up (foreign keys are enabled for this migration).
+    db.delete("recipient", "username IS NOT NULL AND aci IS NULL", null).let { deleteCount ->
+      Log.i(TAG, "Deleted $deleteCount username-only recipients.")
+    }
+  }
+}