Improve safety of update and delete database methods.

greyson-signal · greyson-signal · commit 750fd4efe16e · 2024-01-23T11:52:03.000-05:00
diff --git a/app/src/androidTest/java/org/thoughtcrime/securesms/database/GroupTableTest.kt b/app/src/androidTest/java/org/thoughtcrime/securesms/database/GroupTableTest.kt
@@ -7,7 +7,7 @@ import org.junit.Assert.assertTrue
 import org.junit.Before
 import org.junit.Rule
 import org.junit.Test
-import org.signal.core.util.delete
+import org.signal.core.util.deleteAll
 import org.signal.core.util.readToList
 import org.signal.core.util.requireLong
 import org.signal.core.util.withinTransaction
@@ -33,8 +33,8 @@ class GroupTableTest {
   fun setUp() {
     groupTable = SignalDatabase.groups
 
-    groupTable.writableDatabase.delete(GroupTable.TABLE_NAME).run()
-    groupTable.writableDatabase.delete(GroupTable.MembershipTable.TABLE_NAME).run()
+    groupTable.writableDatabase.deleteAll(GroupTable.TABLE_NAME)
+    groupTable.writableDatabase.deleteAll(GroupTable.MembershipTable.TABLE_NAME)
   }
 
   @Test
diff --git a/app/src/androidTest/java/org/thoughtcrime/securesms/database/LogDatabaseTest.kt b/app/src/androidTest/java/org/thoughtcrime/securesms/database/LogDatabaseTest.kt
@@ -10,7 +10,7 @@ import org.signal.core.util.forEach
 import org.signal.core.util.requireLong
 import org.signal.core.util.requireNonNullString
 import org.signal.core.util.select
-import org.signal.core.util.update
+import org.signal.core.util.updateAll
 import org.thoughtcrime.securesms.crash.CrashConfig
 import org.thoughtcrime.securesms.dependencies.ApplicationDependencies
 import org.thoughtcrime.securesms.testing.assertIs
@@ -220,7 +220,7 @@ class LogDatabaseTest {
     )
 
     db.writableDatabase
-      .update(LogDatabase.CrashTable.TABLE_NAME)
+      .updateAll(LogDatabase.CrashTable.TABLE_NAME)
       .values(LogDatabase.CrashTable.LAST_PROMPTED_AT to currentTime)
       .run()
 
diff --git a/app/src/androidTest/java/org/thoughtcrime/securesms/testing/SignalDatabaseRule.kt b/app/src/androidTest/java/org/thoughtcrime/securesms/testing/SignalDatabaseRule.kt
@@ -2,7 +2,9 @@ package org.thoughtcrime.securesms.testing
 
 import org.junit.rules.TestWatcher
 import org.junit.runner.Description
+import org.signal.core.util.deleteAll
 import org.thoughtcrime.securesms.database.SignalDatabase
+import org.thoughtcrime.securesms.database.ThreadTable
 import org.thoughtcrime.securesms.keyvalue.SignalStore
 import org.whispersystems.signalservice.api.push.ServiceId.ACI
 import org.whispersystems.signalservice.api.push.ServiceId.PNI
@@ -34,7 +36,8 @@ class SignalDatabaseRule(
 
   private fun deleteAllThreads() {
     if (deleteAllThreadsOnEachRun) {
-      SignalDatabase.threads.clearForTests()
+      SignalDatabase.threads.deleteAllConversations()
+      SignalDatabase.rawDatabase.deleteAll(ThreadTable.TABLE_NAME)
     }
   }
 }
diff --git a/app/src/main/java/org/thoughtcrime/securesms/backup/v2/database/AttachmentTableBackupExtensions.kt b/app/src/main/java/org/thoughtcrime/securesms/backup/v2/database/AttachmentTableBackupExtensions.kt
@@ -5,9 +5,9 @@
 
 package org.thoughtcrime.securesms.backup.v2.database
 
-import org.signal.core.util.delete
+import org.signal.core.util.deleteAll
 import org.thoughtcrime.securesms.database.AttachmentTable
 
 fun AttachmentTable.clearAllDataForBackupRestore() {
-  writableDatabase.delete(AttachmentTable.TABLE_NAME).run()
+  writableDatabase.deleteAll(AttachmentTable.TABLE_NAME)
 }
diff --git a/app/src/main/java/org/thoughtcrime/securesms/backup/v2/database/DistributionListTablesBackupExtensions.kt b/app/src/main/java/org/thoughtcrime/securesms/backup/v2/database/DistributionListTablesBackupExtensions.kt
@@ -7,7 +7,7 @@ package org.thoughtcrime.securesms.backup.v2.database
 
 import okio.ByteString.Companion.toByteString
 import org.signal.core.util.CursorUtil
-import org.signal.core.util.delete
+import org.signal.core.util.deleteAll
 import org.signal.core.util.logging.Log
 import org.signal.core.util.readToList
 import org.signal.core.util.requireLong
@@ -88,12 +88,10 @@ fun DistributionListTables.restoreFromBackup(dlist: BackupDistributionList, back
 
 fun DistributionListTables.clearAllDataForBackupRestore() {
   writableDatabase
-    .delete(DistributionListTables.ListTable.TABLE_NAME)
-    .run()
+    .deleteAll(DistributionListTables.ListTable.TABLE_NAME)
 
   writableDatabase
-    .delete(DistributionListTables.MembershipTable.TABLE_NAME)
-    .run()
+    .deleteAll(DistributionListTables.MembershipTable.TABLE_NAME)
 }
 
 private fun DistributionListPrivacyMode.toBackupPrivacyMode(): BackupDistributionList.PrivacyMode {
diff --git a/app/src/main/java/org/thoughtcrime/securesms/backup/v2/database/RecipientTableBackupExtensions.kt b/app/src/main/java/org/thoughtcrime/securesms/backup/v2/database/RecipientTableBackupExtensions.kt
@@ -10,7 +10,7 @@ import android.database.Cursor
 import okio.ByteString.Companion.toByteString
 import org.signal.core.util.Base64
 import org.signal.core.util.SqlUtil
-import org.signal.core.util.delete
+import org.signal.core.util.deleteAll
 import org.signal.core.util.logging.Log
 import org.signal.core.util.nullIfBlank
 import org.signal.core.util.requireBoolean
@@ -155,7 +155,7 @@ fun RecipientTable.restoreSelfFromBackup(accountData: AccountData, selfId: Recip
 }
 
 fun RecipientTable.clearAllDataForBackupRestore() {
-  writableDatabase.delete(RecipientTable.TABLE_NAME).run()
+  writableDatabase.deleteAll(RecipientTable.TABLE_NAME)
   SqlUtil.resetAutoIncrementValue(writableDatabase, RecipientTable.TABLE_NAME)
 
   RecipientId.clearCache()
diff --git a/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/changenumber/ChangeNumberRepository.kt b/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/changenumber/ChangeNumberRepository.kt
@@ -216,7 +216,7 @@ class ChangeNumberRepository(
   @WorkerThread
   fun changeLocalNumber(e164: String, pni: PNI): Single<Unit> {
     val oldStorageId: ByteArray? = Recipient.self().storageServiceId
-    SignalDatabase.recipients.updateSelfPhone(e164, pni)
+    SignalDatabase.recipients.updateSelfE164(e164, pni)
     val newStorageId: ByteArray? = Recipient.self().storageServiceId
 
     if (e164 != SignalStore.account().requireE164() && MessageDigest.isEqual(oldStorageId, newStorageId)) {
diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/AttachmentTable.kt b/app/src/main/java/org/thoughtcrime/securesms/database/AttachmentTable.kt
@@ -39,6 +39,7 @@ import org.signal.core.util.SqlUtil.buildSingleCollectionQuery
 import org.signal.core.util.StreamUtil
 import org.signal.core.util.ThreadUtil
 import org.signal.core.util.delete
+import org.signal.core.util.deleteAll
 import org.signal.core.util.exists
 import org.signal.core.util.forEach
 import org.signal.core.util.groupBy
@@ -475,9 +476,7 @@ class AttachmentTable(
   fun deleteAllAttachments() {
     Log.d(TAG, "[deleteAllAttachments]")
 
-    writableDatabase
-      .delete(TABLE_NAME)
-      .run()
+    writableDatabase.deleteAll(TABLE_NAME)
 
     FileUtils.deleteDirectoryContents(context.getDir(DIRECTORY, Context.MODE_PRIVATE))
 
diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/CallTable.kt b/app/src/main/java/org/thoughtcrime/securesms/database/CallTable.kt
@@ -8,6 +8,7 @@ import org.signal.core.util.IntSerializer
 import org.signal.core.util.Serializer
 import org.signal.core.util.SqlUtil
 import org.signal.core.util.delete
+import org.signal.core.util.deleteAll
 import org.signal.core.util.flatten
 import org.signal.core.util.insertInto
 import org.signal.core.util.logging.Log
@@ -1014,9 +1015,7 @@ class CallTable(context: Context, databaseHelper: SignalDatabase) : DatabaseTabl
   @Discouraged("Using this method is generally considered an error. Utilize other deletion methods instead of this.")
   fun deleteAllCalls() {
     Log.w(TAG, "Deleting all calls from the local database.")
-    writableDatabase
-      .delete(TABLE_NAME)
-      .run()
+    writableDatabase.deleteAll(TABLE_NAME)
   }
 
   private fun getCallSelectionQuery(callId: Long, recipientId: RecipientId): SqlUtil.Query {
diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/CdsTable.kt b/app/src/main/java/org/thoughtcrime/securesms/database/CdsTable.kt
@@ -5,6 +5,7 @@ import android.content.Context
 import androidx.core.content.contentValuesOf
 import org.signal.core.util.SqlUtil
 import org.signal.core.util.delete
+import org.signal.core.util.deleteAll
 import org.signal.core.util.logging.Log
 import org.signal.core.util.requireNonNullString
 import org.signal.core.util.select
@@ -106,8 +107,6 @@ class CdsTable(context: Context, databaseHelper: SignalDatabase) : DatabaseTable
    * Wipes the entire table.
    */
   fun clearAll() {
-    writableDatabase
-      .delete(TABLE_NAME)
-      .run()
+    writableDatabase.deleteAll(TABLE_NAME)
   }
 }
diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/DraftTable.kt b/app/src/main/java/org/thoughtcrime/securesms/database/DraftTable.kt
@@ -5,6 +5,7 @@ import android.net.Uri
 import androidx.core.content.contentValuesOf
 import org.signal.core.util.SqlUtil
 import org.signal.core.util.delete
+import org.signal.core.util.deleteAll
 import org.signal.core.util.logging.Log
 import org.signal.core.util.readToList
 import org.signal.core.util.requireNonNullString
@@ -71,9 +72,7 @@ class DraftTable(context: Context?, databaseHelper: SignalDatabase?) : DatabaseT
   }
 
   fun clearAllDrafts() {
-    writableDatabase
-      .delete(TABLE_NAME)
-      .run()
+    writableDatabase.deleteAll(TABLE_NAME)
   }
 
   fun getDrafts(threadId: Long): Drafts {
diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/GroupReceiptTable.kt b/app/src/main/java/org/thoughtcrime/securesms/database/GroupReceiptTable.kt
@@ -6,6 +6,7 @@ import android.database.Cursor
 import androidx.core.content.contentValuesOf
 import org.signal.core.util.SqlUtil
 import org.signal.core.util.delete
+import org.signal.core.util.deleteAll
 import org.signal.core.util.forEach
 import org.signal.core.util.readToList
 import org.signal.core.util.requireBoolean
@@ -172,7 +173,7 @@ class GroupReceiptTable(context: Context?, databaseHelper: SignalDatabase?) : Da
   }
 
   fun deleteAllRows() {
-    writableDatabase.delete(TABLE_NAME).run()
+    writableDatabase.deleteAll(TABLE_NAME)
   }
 
   override fun remapRecipient(fromId: RecipientId, toId: RecipientId) {
diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/JobDatabase.kt b/app/src/main/java/org/thoughtcrime/securesms/database/JobDatabase.kt
@@ -21,6 +21,7 @@ import org.signal.core.util.requireNonNullString
 import org.signal.core.util.requireString
 import org.signal.core.util.select
 import org.signal.core.util.update
+import org.signal.core.util.updateAll
 import org.signal.core.util.withinTransaction
 import org.thoughtcrime.securesms.crypto.DatabaseSecret
 import org.thoughtcrime.securesms.crypto.DatabaseSecretProvider
@@ -235,7 +236,7 @@ class JobDatabase(
   @Synchronized
   fun updateAllJobsToBePending() {
     writableDatabase
-      .update(Jobs.TABLE_NAME)
+      .updateAll(Jobs.TABLE_NAME)
       .values(Jobs.IS_RUNNING to 0)
       .run()
   }
diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/LogDatabase.kt b/app/src/main/java/org/thoughtcrime/securesms/database/LogDatabase.kt
@@ -9,6 +9,7 @@ import org.signal.core.util.CursorUtil
 import org.signal.core.util.SqlUtil
 import org.signal.core.util.Stopwatch
 import org.signal.core.util.delete
+import org.signal.core.util.deleteAll
 import org.signal.core.util.exists
 import org.signal.core.util.getTableRowCount
 import org.signal.core.util.insertInto
@@ -268,9 +269,7 @@ class LogDatabase private constructor(
     }
 
     fun clearAll() {
-      writableDatabase
-        .delete(TABLE_NAME)
-        .run()
+      writableDatabase.deleteAll(TABLE_NAME)
     }
 
     private fun getSize(query: String?, args: Array<String>?): Long {
@@ -403,9 +402,7 @@ class LogDatabase private constructor(
     }
 
     fun clear() {
-      writableDatabase
-        .delete(TABLE_NAME)
-        .run()
+      writableDatabase.deleteAll(TABLE_NAME)
     }
 
     private fun CrashConfig.CrashPattern.asLikeQuery(): Pair<String, Array<String>> {
@@ -494,9 +491,7 @@ class LogDatabase private constructor(
     }
 
     fun clear() {
-      writableDatabase
-        .delete(TABLE_NAME)
-        .run()
+      writableDatabase.deleteAll(TABLE_NAME)
     }
 
     data class AnrRecord(
diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/MessageTable.kt b/app/src/main/java/org/thoughtcrime/securesms/database/MessageTable.kt
@@ -40,6 +40,7 @@ import org.signal.core.util.SqlUtil.getNextAutoIncrementId
 import org.signal.core.util.Stopwatch
 import org.signal.core.util.count
 import org.signal.core.util.delete
+import org.signal.core.util.deleteAll
 import org.signal.core.util.exists
 import org.signal.core.util.forEach
 import org.signal.core.util.insertInto
@@ -3390,7 +3391,7 @@ open class MessageTable(context: Context?, databaseHelper: SignalDatabase) : Dat
     attachments.deleteAllAttachments()
     groupReceipts.deleteAllRows()
     mentions.deleteAllMentions()
-    writableDatabase.delete(TABLE_NAME).run()
+    writableDatabase.deleteAll(TABLE_NAME)
     calls.updateCallEventDeletionTimestamps()
 
     OptimizeMessageSearchIndexJob.enqueue()
diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/PendingPniSignatureMessageTable.kt b/app/src/main/java/org/thoughtcrime/securesms/database/PendingPniSignatureMessageTable.kt
@@ -3,6 +3,7 @@ package org.thoughtcrime.securesms.database
 import android.content.Context
 import androidx.core.content.contentValuesOf
 import org.signal.core.util.delete
+import org.signal.core.util.deleteAll
 import org.signal.core.util.exists
 import org.signal.core.util.logging.Log
 import org.signal.core.util.update
@@ -117,7 +118,7 @@ class PendingPniSignatureMessageTable(context: Context, databaseHelper: SignalDa
    * Deletes all record of pending PNI verification messages. Should only be called after the user changes their number.
    */
   fun deleteAll() {
-    writableDatabase.delete(TABLE_NAME).run()
+    writableDatabase.deleteAll(TABLE_NAME)
   }
 
   override fun remapRecipient(oldId: RecipientId, newId: RecipientId) {
diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/RecipientTable.kt b/app/src/main/java/org/thoughtcrime/securesms/database/RecipientTable.kt
@@ -33,6 +33,7 @@ import org.signal.core.util.requireNonNullString
 import org.signal.core.util.requireString
 import org.signal.core.util.select
 import org.signal.core.util.update
+import org.signal.core.util.updateAll
 import org.signal.core.util.withinTransaction
 import org.signal.libsignal.protocol.IdentityKey
 import org.signal.libsignal.protocol.InvalidKeyException
@@ -2011,7 +2012,7 @@ open class RecipientTable(context: Context, databaseHelper: SignalDatabase) : Da
   /**
    * Does *not* handle clearing the recipient cache. It is assumed the caller handles this.
    */
-  fun updateSelfPhone(e164: String, pni: PNI) {
+  fun updateSelfE164(e164: String, pni: PNI) {
     val db = writableDatabase
 
     db.beginTransaction()
@@ -2022,11 +2023,10 @@ open class RecipientTable(context: Context, databaseHelper: SignalDatabase) : Da
       if (id == newId) {
         Log.i(TAG, "[updateSelfPhone] Phone updated for self")
       } else {
-        throw AssertionError("[updateSelfPhone] Self recipient id changed when updating phone. old: $id new: $newId")
+        throw AssertionError("[updateSelfPhone] Self recipient id changed when updating e164. old: $id new: $newId")
       }
 
-      db
-        .update(TABLE_NAME)
+      db.updateAll(TABLE_NAME)
         .values(NEEDS_PNI_SIGNATURE to 0)
         .run()
 
diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/RemoteMegaphoneTable.kt b/app/src/main/java/org/thoughtcrime/securesms/database/RemoteMegaphoneTable.kt
@@ -8,7 +8,7 @@ import androidx.core.content.contentValuesOf
 import androidx.core.net.toUri
 import org.json.JSONException
 import org.json.JSONObject
-import org.signal.core.util.delete
+import org.signal.core.util.deleteAll
 import org.signal.core.util.logging.Log
 import org.signal.core.util.readToList
 import org.signal.core.util.requireInt
@@ -194,9 +194,7 @@ class RemoteMegaphoneTable(context: Context, databaseHelper: SignalDatabase) : D
 
   /** Only call from internal settings */
   fun debugRemoveAll() {
-    writableDatabase
-      .delete(TABLE_NAME)
-      .run()
+    writableDatabase.deleteAll(TABLE_NAME)
   }
 
   private fun RemoteMegaphoneRecord.toContentValues(): ContentValues {
diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/SenderKeySharedTable.kt b/app/src/main/java/org/thoughtcrime/securesms/database/SenderKeySharedTable.kt
@@ -4,6 +4,7 @@ import android.content.Context
 import android.database.Cursor
 import androidx.core.content.contentValuesOf
 import org.signal.core.util.delete
+import org.signal.core.util.deleteAll
 import org.signal.core.util.logging.Log
 import org.signal.core.util.readToSet
 import org.signal.core.util.requireInt
@@ -137,9 +138,7 @@ class SenderKeySharedTable internal constructor(context: Context?, databaseHelpe
    * Clears all database content.
    */
   fun deleteAll() {
-    writableDatabase
-      .delete(TABLE_NAME)
-      .run()
+    writableDatabase.deleteAll(TABLE_NAME)
   }
 
   /**
diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/SenderKeyTable.kt b/app/src/main/java/org/thoughtcrime/securesms/database/SenderKeyTable.kt
@@ -5,6 +5,7 @@ import android.database.Cursor
 import androidx.core.content.contentValuesOf
 import org.signal.core.util.CursorUtil
 import org.signal.core.util.delete
+import org.signal.core.util.deleteAll
 import org.signal.core.util.firstOrNull
 import org.signal.core.util.logging.Log
 import org.signal.core.util.requireLong
@@ -123,8 +124,6 @@ class SenderKeyTable internal constructor(context: Context?, databaseHelper: Sig
    * Deletes all database state.
    */
   fun deleteAll() {
-    writableDatabase
-      .delete(TABLE_NAME)
-      .run()
+    writableDatabase.deleteAll(TABLE_NAME)
   }
 }
diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/ThreadTable.kt b/app/src/main/java/org/thoughtcrime/securesms/database/ThreadTable.kt
diff --git a/core-util/src/main/java/org/signal/core/util/SQLiteDatabaseExtensions.kt b/core-util/src/main/java/org/signal/core/util/SQLiteDatabaseExtensions.kt

Original file line number	Diff line number	Diff line change
`@@ -2,7 +2,9 @@ package org.thoughtcrime.securesms.testing`
`2`	`2`
`3`	`3`	`import org.junit.rules.TestWatcher`
`4`	`4`	`import org.junit.runner.Description`
	`5`	`+import org.signal.core.util.deleteAll`
`5`	`6`	`import org.thoughtcrime.securesms.database.SignalDatabase`
	`7`	`+import org.thoughtcrime.securesms.database.ThreadTable`
`6`	`8`	`import org.thoughtcrime.securesms.keyvalue.SignalStore`
`7`	`9`	`import org.whispersystems.signalservice.api.push.ServiceId.ACI`
`8`	`10`	`import org.whispersystems.signalservice.api.push.ServiceId.PNI`
`@@ -34,7 +36,8 @@ class SignalDatabaseRule(`
`34`	`36`
`35`	`37`	`private fun deleteAllThreads() {`
`36`	`38`	`if (deleteAllThreadsOnEachRun) {`
`37`		`- SignalDatabase.threads.clearForTests()`
	`39`	`+ SignalDatabase.threads.deleteAllConversations()`
	`40`	`+ SignalDatabase.rawDatabase.deleteAll(ThreadTable.TABLE_NAME)`
`38`	`41`	`}`
`39`	`42`	`}`
`40`	`43`	`}`
Original file line number	Diff line number	Diff line change
`@@ -5,9 +5,9 @@`
`5`	`5`
`6`	`6`	`package org.thoughtcrime.securesms.backup.v2.database`
`7`	`7`
`8`		`-import org.signal.core.util.delete`
	`8`	`+import org.signal.core.util.deleteAll`
`9`	`9`	`import org.thoughtcrime.securesms.database.AttachmentTable`
`10`	`10`
`11`	`11`	`fun AttachmentTable.clearAllDataForBackupRestore() {`
`12`		`- writableDatabase.delete(AttachmentTable.TABLE_NAME).run()`
	`12`	`+ writableDatabase.deleteAll(AttachmentTable.TABLE_NAME)`
`13`	`13`	`}`