Move back to manually implementing secure-delete.

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

@@ -112,6 +112,7 @@ import org.thoughtcrime.securesms.database.model.databaseprotos.SessionSwitchove
 import org.thoughtcrime.securesms.database.model.databaseprotos.ThreadMergeEvent
 import org.thoughtcrime.securesms.dependencies.AppDependencies
 import org.thoughtcrime.securesms.groups.GroupMigrationMembershipChange
+import org.thoughtcrime.securesms.jobs.OptimizeMessageSearchIndexJob
 import org.thoughtcrime.securesms.jobs.ThreadUpdateJob
 import org.thoughtcrime.securesms.jobs.TrimThreadJob
 import org.thoughtcrime.securesms.keyvalue.SignalStore
@@ -1653,7 +1654,7 @@ open class MessageTable(context: Context?, databaseHelper: SignalDatabase) : Dat
         .readToList { RecipientId.from(it.requireLong(FROM_RECIPIENT_ID)) }
         .forEach { id -> AppDependencies.databaseObserver.notifyStoryObservers(id) }
 
-      db.select(ID)
+      val deletedStoryCount = db.select(ID)
         .from(TABLE_NAME)
         .where(storiesBeforeTimestampWhere, sharedArgs)
         .run()
@@ -1664,6 +1665,12 @@ open class MessageTable(context: Context?, databaseHelper: SignalDatabase) : Dat
 
           cursor.count
         }
+
+      if (deletedStoryCount > 0) {
+        OptimizeMessageSearchIndexJob.enqueue()
+      }
+
+      deletedStoryCount
     }
   }
 
@@ -1706,7 +1713,7 @@ open class MessageTable(context: Context?, databaseHelper: SignalDatabase) : Dat
 
       AppDependencies.databaseObserver.notifyStoryObservers(recipientId)
 
-      db.select(ID)
+      val deletedStoryCount = db.select(ID)
         .from(TABLE_NAME)
         .where(storesInRecipientThread, sharedArgs)
         .run()
@@ -1717,6 +1724,12 @@ open class MessageTable(context: Context?, databaseHelper: SignalDatabase) : Dat
 
           cursor.count
         }
+
+      if (deletedStoryCount > 0) {
+        OptimizeMessageSearchIndexJob.enqueue()
+      }
+
+      deletedStoryCount
     }
   }
 
@@ -2097,6 +2110,7 @@ open class MessageTable(context: Context?, databaseHelper: SignalDatabase) : Dat
       threads.update(threadId, false)
     }
 
+    OptimizeMessageSearchIndexJob.enqueue()
     AppDependencies.databaseObserver.notifyMessageUpdateObservers(MessageId(messageId))
     AppDependencies.databaseObserver.notifyConversationListListeners()
 
@@ -3299,6 +3313,7 @@ open class MessageTable(context: Context?, databaseHelper: SignalDatabase) : Dat
       notifyConversationListeners(threadId)
       notifyStickerListeners()
       notifyStickerPackListeners()
+      OptimizeMessageSearchIndexJob.enqueue()
     }
 
     return threadDeleted
@@ -3518,6 +3533,7 @@ open class MessageTable(context: Context?, databaseHelper: SignalDatabase) : Dat
     notifyConversationListListeners()
     notifyStickerListeners()
     notifyStickerPackListeners()
+    OptimizeMessageSearchIndexJob.enqueue()
 
     return unhandled
   }
@@ -3539,6 +3555,8 @@ open class MessageTable(context: Context?, databaseHelper: SignalDatabase) : Dat
     mentions.deleteAllMentions()
     writableDatabase.deleteAll(TABLE_NAME)
     calls.updateCallEventDeletionTimestamps()
+
+    OptimizeMessageSearchIndexJob.enqueue()
   }
 
   fun getNearestExpiringViewOnceMessage(): ViewOnceExpirationInfo? {

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

@@ -7,7 +7,9 @@ import android.text.TextUtils
 import net.zetetic.database.sqlcipher.SQLiteDatabase
 import org.intellij.lang.annotations.Language
 import org.signal.core.util.SqlUtil
+import org.signal.core.util.ThreadUtil
 import org.signal.core.util.logging.Log
+import org.signal.core.util.withinTransaction
 import org.thoughtcrime.securesms.jobs.RebuildMessageSearchIndexJob
 
 /**
@@ -34,10 +36,7 @@ class SearchTable(context: Context, databaseHelper: SignalDatabase) : DatabaseTa
       // We've taken the default of tokenize value of "unicode61 categories 'L* N* Co'" and added the Sc (currency) and So (emoji) categories to allow searching for those characters.
       // https://www.sqlite.org/fts5.html#tokenizers
       // https://www.compart.com/en/unicode/category
-      """CREATE VIRTUAL TABLE $FTS_TABLE_NAME USING fts5($BODY, $THREAD_ID UNINDEXED, content=${MessageTable.TABLE_NAME}, content_rowid=${MessageTable.ID}, tokenize = "unicode61 categories 'L* N* Co Sc So'")""",
-
-      // Not technically a `CREATE` statement, but it's part of table creation. FTS5 just has weird configuration syntax. See https://www.sqlite.org/fts5.html#the_secure_delete_configuration_option
-      """INSERT INTO $FTS_TABLE_NAME ($FTS_TABLE_NAME, rank) VALUES('secure-delete', 1);"""
+      """CREATE VIRTUAL TABLE $FTS_TABLE_NAME USING fts5($BODY, $THREAD_ID UNINDEXED, content=${MessageTable.TABLE_NAME}, content_rowid=${MessageTable.ID}, tokenize = "unicode61 categories 'L* N* Co Sc So'")"""
     )
 
     private const val TRIGGER_AFTER_INSERT = "message_ai"
@@ -172,6 +171,73 @@ class SearchTable(context: Context, databaseHelper: SignalDatabase) : DatabaseTa
     }
   }
 
+  /**
+   * This performs the same thing as the `optimize` command in SQLite, but broken into iterative stages to avoid locking up the database for too long.
+   * If what's going on in this method seems weird, that's because it is, but please read the sqlite docs -- we're following their algorithm:
+   * https://www.sqlite.org/fts5.html#the_optimize_command
+   *
+   * Note that in order for the [SqlUtil.getTotalChanges] call to work, we have to be within a transaction, or else the connection pool screws everything up
+   * (the stats are on a per-connection basis).
+   *
+   * There's this double-batching mechanism happening here to strike a balance between making individual transactions short while also not hammering the
+   * database with a ton of independent transactions.
+   *
+   * To give you some ballpark numbers, on a large database (~400k messages), it takes ~75 iterations to fully optimize everything.
+   */
+  fun optimizeIndex(timeout: Long): Boolean {
+    val pageSize = 64 // chosen through experimentation
+    val batchSize = 10 // chosen through experimentation
+    val noChangeThreshold = 2 // if less changes occurred than this, operation is considered no-op (see sqlite docs ref'd in kdoc)
+
+    val startTime = System.currentTimeMillis()
+    var totalIterations = 0
+    var totalBatches = 0
+    var actualWorkTime = 0L
+    var finished = false
+
+    while (!finished) {
+      var batchIterations = 0
+      val batchStartTime = System.currentTimeMillis()
+
+      writableDatabase.withinTransaction { db ->
+        // Note the negative page size -- see sqlite docs ref'd in kdoc
+        db.execSQL("INSERT INTO $FTS_TABLE_NAME ($FTS_TABLE_NAME, rank) values ('merge', -$pageSize)")
+        var previousCount = SqlUtil.getTotalChanges(db)
+
+        val iterativeStatement = db.compileStatement("INSERT INTO $FTS_TABLE_NAME ($FTS_TABLE_NAME, rank) values ('merge', $pageSize)")
+        iterativeStatement.execute()
+        var count = SqlUtil.getTotalChanges(db)
+
+        while (batchIterations < batchSize && count - previousCount >= noChangeThreshold) {
+          previousCount = count
+          iterativeStatement.execute()
+
+          count = SqlUtil.getTotalChanges(db)
+          batchIterations++
+        }
+
+        if (count - previousCount < noChangeThreshold) {
+          finished = true
+        }
+      }
+
+      totalIterations += batchIterations
+      totalBatches++
+      actualWorkTime += System.currentTimeMillis() - batchStartTime
+
+      if (actualWorkTime >= timeout) {
+        Log.w(TAG, "Timed out during optimization! We did $totalIterations iterations across $totalBatches batches, taking ${System.currentTimeMillis() - startTime} ms. Bailed out to avoid database lockup.")
+        return false
+      }
+
+      // We want to sleep in between batches to give other db operations a chance to run
+      ThreadUtil.sleep(50)
+    }
+
+    Log.d(TAG, "Took ${System.currentTimeMillis() - startTime} ms and $totalIterations iterations across $totalBatches batches to optimize. Of that time, $actualWorkTime ms were spent actually working (~${actualWorkTime / totalBatches} ms/batch). The rest was spent sleeping.")
+    return true
+  }
+
   /**
    * Drops all tables and recreates them.
    */

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

@@ -51,6 +51,7 @@ import org.thoughtcrime.securesms.dependencies.AppDependencies
 import org.thoughtcrime.securesms.groups.BadGroupIdException
 import org.thoughtcrime.securesms.groups.GroupId
 import org.thoughtcrime.securesms.jobs.MultiDeviceDeleteSyncJob
+import org.thoughtcrime.securesms.jobs.OptimizeMessageSearchIndexJob
 import org.thoughtcrime.securesms.keyvalue.SignalStore
 import org.thoughtcrime.securesms.mms.SlideDeck
 import org.thoughtcrime.securesms.mms.StickerSlide
@@ -362,6 +363,7 @@ class ThreadTable(context: Context, databaseHelper: SignalDatabase) : DatabaseTa
 
     notifyAttachmentListeners()
     notifyStickerPackListeners()
+    OptimizeMessageSearchIndexJob.enqueue()
   }
 
   fun trimThread(
@@ -395,6 +397,7 @@ class ThreadTable(context: Context, databaseHelper: SignalDatabase) : DatabaseTa
 
     notifyAttachmentListeners()
     notifyStickerPackListeners()
+    OptimizeMessageSearchIndexJob.enqueue()
   }
 
   private fun trimThreadInternal(
@@ -1185,6 +1188,8 @@ class ThreadTable(context: Context, databaseHelper: SignalDatabase) : DatabaseTa
     AppDependencies.databaseObserver.notifyConversationDeleteListeners(selectedConversations)
 
     ConversationUtil.clearShortcuts(context, recipientIds)
+
+    OptimizeMessageSearchIndexJob.enqueue()
   }
 
   @SuppressLint("DiscouragedApi")

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

@@ -100,6 +100,7 @@ import org.thoughtcrime.securesms.database.helpers.migration.V239_MessageFullTex
 import org.thoughtcrime.securesms.database.helpers.migration.V240_MessageFullTextSearchSecureDelete
 import org.thoughtcrime.securesms.database.helpers.migration.V241_ExpireTimerVersion
 import org.thoughtcrime.securesms.database.helpers.migration.V242_MessageFullTextSearchEmojiSupportV2
+import org.thoughtcrime.securesms.database.helpers.migration.V243_MessageFullTextSearchDisableSecureDelete
 
 /**
  * Contains all of the database migrations for [SignalDatabase]. Broken into a separate file for cleanliness.
@@ -202,10 +203,11 @@ object SignalDatabaseMigrations {
     239 to V239_MessageFullTextSearchEmojiSupport,
     240 to V240_MessageFullTextSearchSecureDelete,
     241 to V241_ExpireTimerVersion,
-    242 to V242_MessageFullTextSearchEmojiSupportV2
+    242 to V242_MessageFullTextSearchEmojiSupportV2,
+    243 to V243_MessageFullTextSearchDisableSecureDelete
   )
 
-  const val DATABASE_VERSION = 242
+  const val DATABASE_VERSION = 243
 
   @JvmStatic
   fun migrate(context: Application, db: SQLiteDatabase, oldVersion: Int, newVersion: Int) {

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

@@ -0,0 +1,21 @@
+/*
+ * Copyright 2024 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.thoughtcrime.securesms.database.helpers.migration.V240_MessageFullTextSearchSecureDelete.FTS_TABLE_NAME
+
+/**
+ * This undoes [V240_MessageFullTextSearchSecureDelete] by disabling secure-delete on our FTS table.
+ * Unfortunately the performance overhead was too high. Thankfully, our old approach, while more
+ * manual, provides the same safety guarantees, while also allowing us to optimize bulk deletes.
+ */
+object V243_MessageFullTextSearchDisableSecureDelete : SignalDatabaseMigration {
+  override fun migrate(context: Application, db: SQLiteDatabase, oldVersion: Int, newVersion: Int) {
+    db.execSQL("""INSERT INTO $FTS_TABLE_NAME ($FTS_TABLE_NAME, rank) VALUES('secure-delete', 0);""")
+  }
+}