diff --git a/app/src/main/java/org/thoughtcrime/securesms/avatar/AvatarPickerStorage.kt b/app/src/main/java/org/thoughtcrime/securesms/avatar/AvatarPickerStorage.kt index 9f87a80a41..4304206c27 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/avatar/AvatarPickerStorage.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/avatar/AvatarPickerStorage.kt @@ -38,8 +38,8 @@ object AvatarPickerStorage { .getAllAvatars() .filterIsInstance() - val inDatabaseFileNames = photoAvatars.map { PartAuthority.getAvatarPickerFilename(it.uri) } - val onDiskFileNames = avatarFiles.map { it.name } + val inDatabaseFileNames = photoAvatars.mapTo(mutableSetOf()) { PartAuthority.getAvatarPickerFilename(it.uri) } + val onDiskFileNames = avatarFiles.mapTo(mutableSetOf()) { it.name } val inDatabaseButNotOnDisk = inDatabaseFileNames - onDiskFileNames val onDiskButNotInDatabase = onDiskFileNames - inDatabaseFileNames diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/AttachmentMetadataTable.kt b/app/src/main/java/org/thoughtcrime/securesms/database/AttachmentMetadataTable.kt index 7b5ccc6745..fc44986131 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/AttachmentMetadataTable.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/AttachmentMetadataTable.kt @@ -7,6 +7,7 @@ package org.thoughtcrime.securesms.database import android.content.Context import android.database.Cursor +import org.signal.core.util.SqlUtil import org.signal.core.util.Util import org.signal.core.util.delete import org.signal.core.util.insertInto @@ -84,6 +85,23 @@ class AttachmentMetadataTable(context: Context, databaseHelper: SignalDatabase) .run() } + fun cleanupById(ids: Collection) { + if (ids.isEmpty()) { + return + } + + val idClause = SqlUtil.buildSingleCollectionQuery(ID, ids) + val attachmentReferenceClause = SqlUtil.buildSingleCollectionQuery(AttachmentTable.METADATA_ID, ids) + + writableDatabase + .delete(TABLE_NAME) + .where( + "${idClause.where} AND $ID NOT IN (SELECT ${AttachmentTable.METADATA_ID} FROM ${AttachmentTable.TABLE_NAME} WHERE ${attachmentReferenceClause.where})", + idClause.whereArgs + attachmentReferenceClause.whereArgs + ) + .run() + } + fun insertNewKeysForExistingAttachments() { writableDatabase.withinTransaction { do { diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/AttachmentTable.kt b/app/src/main/java/org/thoughtcrime/securesms/database/AttachmentTable.kt index ef9bb1f715..4a7d4088f1 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/AttachmentTable.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/AttachmentTable.kt @@ -299,7 +299,9 @@ class AttachmentTable( "CREATE INDEX IF NOT EXISTS attachment_remote_digest_index ON $TABLE_NAME ($REMOTE_DIGEST);", "CREATE INDEX IF NOT EXISTS attachment_metadata_id ON $TABLE_NAME ($METADATA_ID);", "CREATE INDEX IF NOT EXISTS attachment_media_overview_size ON $TABLE_NAME ($DATA_SIZE DESC, $DISPLAY_ORDER DESC) WHERE $QUOTE = 0 AND $STICKER_PACK_ID IS NULL AND $DATA_FILE IS NOT NULL", - "CREATE INDEX IF NOT EXISTS attachment_archive_thumbnail_transfer_state ON $TABLE_NAME ($ARCHIVE_THUMBNAIL_TRANSFER_STATE)" + "CREATE INDEX IF NOT EXISTS attachment_archive_thumbnail_transfer_state ON $TABLE_NAME ($ARCHIVE_THUMBNAIL_TRANSFER_STATE)", + "CREATE INDEX IF NOT EXISTS attachment_thumbnail_file_index ON $TABLE_NAME ($THUMBNAIL_FILE) WHERE $THUMBNAIL_FILE IS NOT NULL", + "CREATE INDEX IF NOT EXISTS attachment_uuid_index ON $TABLE_NAME ($ATTACHMENT_UUID) WHERE $ATTACHMENT_UUID IS NOT NULL" ) private val DATA_FILE_INFO_PROJECTION = arrayOf( @@ -373,7 +375,7 @@ class AttachmentTable( val hashEnd = Base64.encodeWithPadding(hash) val (existingFile: String?, existingSize: Long?, existingRandom: ByteArray?) = db.select(DATA_FILE, DATA_SIZE, DATA_RANDOM) - .from(TABLE_NAME) + .from("$TABLE_NAME INDEXED BY $DATA_HASH_REMOTE_KEY_INDEX") .where("$DATA_HASH_END = ? AND $TRANSFER_STATE = $TRANSFER_PROGRESS_DONE AND $DATA_FILE NOT NULL AND $DATA_FILE != ?", hashEnd, file.absolutePath) .limit(1) .run() @@ -1302,7 +1304,9 @@ class AttachmentTable( val contentTypesToDelete: MutableSet = mutableSetOf() val deleteCount = writableDatabase.withinTransaction { db -> - db.select(DATA_FILE, CONTENT_TYPE, ID) + val metadataIdsToCleanup: MutableSet = mutableSetOf() + + db.select(DATA_FILE, CONTENT_TYPE, ID, METADATA_ID) .from(TABLE_NAME) .where("$MESSAGE_ID = ?", mmsId) .run() @@ -1314,6 +1318,8 @@ class AttachmentTable( val filePath = cursor.requireString(DATA_FILE) val contentType = cursor.requireString(CONTENT_TYPE) + cursor.requireLongOrNull(METADATA_ID)?.let { metadataIdsToCleanup += it } + if (filePath != null && isSafeToDeleteDataFile(filePath, attachmentId)) { filePathsToDelete += filePath contentType?.let { contentTypesToDelete += it } @@ -1324,7 +1330,7 @@ class AttachmentTable( .where("$MESSAGE_ID = ?", mmsId) .run() - SignalDatabase.attachmentMetadata.cleanup() + SignalDatabase.attachmentMetadata.cleanupById(metadataIdsToCleanup) AppDependencies.databaseObserver.notifyAttachmentDeletedObservers() @@ -1368,7 +1374,9 @@ class AttachmentTable( var threadId: Long = -1 writableDatabase.withinTransaction { db -> - db.select(DATA_FILE, CONTENT_TYPE, ID) + val metadataIdsToCleanup: MutableSet = mutableSetOf() + + db.select(DATA_FILE, CONTENT_TYPE, ID, METADATA_ID) .from(TABLE_NAME) .where("$MESSAGE_ID = ?", messageId) .run() @@ -1376,6 +1384,7 @@ class AttachmentTable( val filePath = cursor.requireString(DATA_FILE) val contentType = cursor.requireString(CONTENT_TYPE) val id = AttachmentId(cursor.requireLong(ID)) + cursor.requireLongOrNull(METADATA_ID)?.let { metadataIdsToCleanup += it } if (filePath != null && isSafeToDeleteDataFile(filePath, id)) { filePathsToDelete += filePath @@ -1409,7 +1418,7 @@ class AttachmentTable( .where("$MESSAGE_ID = ?", messageId) .run() - SignalDatabase.attachmentMetadata.cleanup() + SignalDatabase.attachmentMetadata.cleanupById(metadataIdsToCleanup) AppDependencies.databaseObserver.notifyAttachmentDeletedObservers() @@ -1434,7 +1443,7 @@ class AttachmentTable( var deletedMessageId: Long? = null writableDatabase.withinTransaction { db -> - db.select(DATA_FILE, CONTENT_TYPE, MESSAGE_ID) + db.select(DATA_FILE, CONTENT_TYPE, MESSAGE_ID, METADATA_ID) .from(TABLE_NAME) .where("$ID = ?", id.id) .run() @@ -1447,12 +1456,13 @@ class AttachmentTable( val filePath = cursor.requireString(DATA_FILE) val contentType = cursor.requireString(CONTENT_TYPE) deletedMessageId = cursor.requireLong(MESSAGE_ID) + val metadataId = cursor.requireLongOrNull(METADATA_ID) db.delete(TABLE_NAME) .where("$ID = ?", id.id) .run() - SignalDatabase.attachmentMetadata.cleanup() + SignalDatabase.attachmentMetadata.cleanupById(listOfNotNull(metadataId)) if (filePath != null && isSafeToDeleteDataFile(filePath, id)) { filePathsToDelete += filePath @@ -1749,7 +1759,7 @@ class AttachmentTable( // the quality of the attachment we received. val hashMatch: DataFileInfo? = readableDatabase .select(*DATA_FILE_INFO_PROJECTION) - .from(TABLE_NAME) + .from("$TABLE_NAME INDEXED BY $DATA_HASH_REMOTE_KEY_INDEX") .where("$DATA_HASH_END = ? AND $DATA_HASH_END NOT NULL AND $TRANSFER_STATE = $TRANSFER_PROGRESS_DONE AND $DATA_FILE NOT NULL", fileWriteResult.hash) .run() .readToList { it.readDataFileInfo() } diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/MessageTable.kt b/app/src/main/java/org/thoughtcrime/securesms/database/MessageTable.kt index d3ae87fde4..001e8e62d6 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/MessageTable.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/MessageTable.kt @@ -315,9 +315,11 @@ open class MessageTable(context: Context?, databaseHelper: SignalDatabase) : Dat private const val INDEX_THREAD_COUNT = "message_thread_count_index" private const val INDEX_THREAD_UNREAD_COUNT = "message_thread_unread_count_index" private const val INDEX_STORY_TYPE = "message_story_type_index" + private const val INDEX_PARENT_STORY_ID = "message_parent_story_id_index" private const val INDEX_ARCHIVED_STORY = "message_story_archived_index" private const val INDEX_STARRED = "message_starred_index" private const val INDEX_NOTIFICATION_STATE = "message_notification_state_index" + private const val INDEX_RATE_LIMITED = "message_rate_limited_index" @JvmField val CREATE_INDEXS = arrayOf( @@ -353,7 +355,8 @@ open class MessageTable(context: Context?, databaseHelper: SignalDatabase) : Dat "CREATE INDEX IF NOT EXISTS message_collapsed_head_id_index ON $TABLE_NAME ($COLLAPSED_HEAD_ID)", "CREATE INDEX IF NOT EXISTS $INDEX_NOTIFICATION_STATE ON $TABLE_NAME ($DATE_RECEIVED) WHERE $NOTIFIED = 0 AND $STORY_TYPE = 0 AND $LATEST_REVISION_ID IS NULL", "CREATE INDEX IF NOT EXISTS message_expire_started_index ON $TABLE_NAME ($EXPIRE_STARTED) WHERE $EXPIRE_STARTED > 0", - "CREATE INDEX IF NOT EXISTS message_view_once_index ON $TABLE_NAME ($VIEW_ONCE) WHERE $VIEW_ONCE > 0" + "CREATE INDEX IF NOT EXISTS message_view_once_index ON $TABLE_NAME ($VIEW_ONCE) WHERE $VIEW_ONCE > 0", + "CREATE INDEX IF NOT EXISTS $INDEX_RATE_LIMITED ON $TABLE_NAME ($ID) WHERE ($TYPE & ${MessageTypes.MESSAGE_RATE_LIMITED_BIT}) != 0" ) private val MMS_PROJECTION_BASE = arrayOf( @@ -1723,76 +1726,45 @@ open class MessageTable(context: Context?, databaseHelper: SignalDatabase) : Dat return writableDatabase.withinTransaction { db -> val releaseChannelThreadId = getReleaseChannelThreadId(hasSeenReleaseChannelStories) - val storiesBeforeTimestampWhere = "$IS_STORY_CLAUSE AND $DATE_SENT < ? AND $THREAD_ID != ?" - val sharedArgs = buildArgs(timestamp, releaseChannelThreadId) + data class ExpiredStory(val id: Long, val fromRecipientId: Long) - val deleteStoryRepliesQuery = """ - DELETE FROM $TABLE_NAME INDEXED BY $INDEX_STORY_TYPE - WHERE - $PARENT_STORY_ID > 0 AND - $PARENT_STORY_ID IN ( - SELECT $ID - FROM $TABLE_NAME - WHERE $storiesBeforeTimestampWhere - ) - """ + val expiredStories = db + .select(ID, FROM_RECIPIENT_ID) + .from("$TABLE_NAME INDEXED BY $INDEX_STORY_TYPE") + .where("$IS_STORY_CLAUSE AND $DATE_SENT < ? AND $THREAD_ID != ?", buildArgs(timestamp, releaseChannelThreadId)) + .run() + .readToList { ExpiredStory(it.requireLong(ID), it.requireLong(FROM_RECIPIENT_ID)) } - val disassociateQuoteQuery = """ - UPDATE $TABLE_NAME - SET - $QUOTE_MISSING = 1, - $QUOTE_BODY = '' - WHERE - $PARENT_STORY_ID < 0 AND - ABS($PARENT_STORY_ID) IN ( - SELECT $ID - FROM $TABLE_NAME - WHERE $storiesBeforeTimestampWhere - ) - """ - - val storyRepliesQuery = """ - SELECT $ID FROM $TABLE_NAME - WHERE - $PARENT_STORY_ID < 0 AND - ABS($PARENT_STORY_ID) IN ( - SELECT $ID - FROM $TABLE_NAME - WHERE $storiesBeforeTimestampWhere - ) - """ - - db.execSQL(deleteStoryRepliesQuery, sharedArgs) - db.execSQL(disassociateQuoteQuery, sharedArgs) - db.rawQuery(storyRepliesQuery, sharedArgs).forEach { cursor: Cursor -> - val mmsId = cursor.requireLong(ID) - attachments.deleteAttachmentsForMessage(mmsId) + if (expiredStories.isEmpty()) { + return@withinTransaction 0 } - db.select(FROM_RECIPIENT_ID) - .from(TABLE_NAME) - .where(storiesBeforeTimestampWhere, sharedArgs) + val storyIds = expiredStories.map { it.id } + val directReplyClause = buildSingleCollectionQuery(PARENT_STORY_ID, storyIds) + val quotedReplyClause = buildSingleCollectionQuery(PARENT_STORY_ID, storyIds.map { -it }) + + db.delete("$TABLE_NAME INDEXED BY $INDEX_PARENT_STORY_ID") + .where(directReplyClause.where, directReplyClause.whereArgs) .run() - .readToList { RecipientId.from(it.requireLong(FROM_RECIPIENT_ID)) } - .forEach { id -> AppDependencies.databaseObserver.notifyStoryObservers(id) } - val deletedStoryCount = db.select(ID) - .from(TABLE_NAME) - .where(storiesBeforeTimestampWhere, sharedArgs) + db.update("$TABLE_NAME INDEXED BY $INDEX_PARENT_STORY_ID") + .values(QUOTE_MISSING to 1, QUOTE_BODY to "") + .where(quotedReplyClause.where, quotedReplyClause.whereArgs) .run() - .use { cursor -> - while (cursor.moveToNext()) { - deleteMessage(cursor.requireLong(ID)) - } - cursor.count - } + db.select(ID) + .from("$TABLE_NAME INDEXED BY $INDEX_PARENT_STORY_ID") + .where(quotedReplyClause.where, quotedReplyClause.whereArgs) + .run() + .forEach { cursor -> attachments.deleteAttachmentsForMessage(cursor.requireLong(ID)) } - if (deletedStoryCount > 0) { - OptimizeMessageSearchIndexJob.enqueue() - } + expiredStories.forEach { AppDependencies.databaseObserver.notifyStoryObservers(RecipientId.from(it.fromRecipientId)) } - deletedStoryCount + storyIds.forEach { deleteMessage(it) } + + OptimizeMessageSearchIndexJob.enqueue() + + storyIds.size } } @@ -1863,71 +1835,40 @@ open class MessageTable(context: Context?, databaseHelper: SignalDatabase) : Dat fun deleteStoriesForRecipient(recipientId: RecipientId): Int { return writableDatabase.withinTransaction { db -> val threadId = threads.getThreadIdFor(recipientId) ?: return@withinTransaction 0 - val storiesInRecipientThread = "$IS_STORY_CLAUSE AND $THREAD_ID = ?" - val sharedArgs = buildArgs(threadId) - val deleteStoryRepliesQuery = """ - DELETE FROM $TABLE_NAME INDEXED BY $INDEX_STORY_TYPE - WHERE - $PARENT_STORY_ID > 0 AND - $PARENT_STORY_ID IN ( - SELECT $ID - FROM $TABLE_NAME - WHERE $storiesInRecipientThread - ) - """ + val storyIds = db.select(ID) + .from("$TABLE_NAME INDEXED BY $INDEX_STORY_TYPE") + .where("$IS_STORY_CLAUSE AND $THREAD_ID = ?", threadId) + .run() + .readToList { it.requireLong(ID) } - val disassociateQuoteQuery = """ - UPDATE $TABLE_NAME - SET - $QUOTE_MISSING = 1, - $QUOTE_BODY = '' - WHERE - $PARENT_STORY_ID < 0 AND - ABS($PARENT_STORY_ID) IN ( - SELECT $ID - FROM $TABLE_NAME - WHERE $storiesInRecipientThread - ) - """ + if (storyIds.isEmpty()) return@withinTransaction 0 - val storyRepliesQuery = """ - SELECT $ID FROM $TABLE_NAME - WHERE - $PARENT_STORY_ID < 0 AND - ABS($PARENT_STORY_ID) IN ( - SELECT $ID - FROM $TABLE_NAME - WHERE $storiesInRecipientThread - ) - """ + val directReplyClause = buildSingleCollectionQuery(PARENT_STORY_ID, storyIds) + val quotedReplyClause = buildSingleCollectionQuery(PARENT_STORY_ID, storyIds.map { -it }) - db.execSQL(deleteStoryRepliesQuery, sharedArgs) - db.execSQL(disassociateQuoteQuery, sharedArgs) - db.rawQuery(storyRepliesQuery, sharedArgs).forEach { cursor: Cursor -> - val mmsId = cursor.requireLong(ID) - attachments.deleteAttachmentsForMessage(mmsId) - } + db.delete("$TABLE_NAME INDEXED BY $INDEX_PARENT_STORY_ID") + .where(directReplyClause.where, directReplyClause.whereArgs) + .run() + + db.update("$TABLE_NAME INDEXED BY $INDEX_PARENT_STORY_ID") + .values(QUOTE_MISSING to 1, QUOTE_BODY to "") + .where(quotedReplyClause.where, quotedReplyClause.whereArgs) + .run() + + db.select(ID) + .from("$TABLE_NAME INDEXED BY $INDEX_PARENT_STORY_ID") + .where(quotedReplyClause.where, quotedReplyClause.whereArgs) + .run() + .forEach { cursor -> attachments.deleteAttachmentsForMessage(cursor.requireLong(ID)) } AppDependencies.databaseObserver.notifyStoryObservers(recipientId) - val deletedStoryCount = db.select(ID) - .from("$TABLE_NAME INDEXED BY $INDEX_STORY_TYPE") - .where(storiesInRecipientThread, sharedArgs) - .run() - .use { cursor -> - while (cursor.moveToNext()) { - deleteMessage(cursor.requireLong(ID)) - } + storyIds.forEach { deleteMessage(it) } - cursor.count - } + OptimizeMessageSearchIndexJob.enqueue() - if (deletedStoryCount > 0) { - OptimizeMessageSearchIndexJob.enqueue() - } - - deletedStoryCount + storyIds.size } } @@ -4285,15 +4226,12 @@ open class MessageTable(context: Context?, databaseHelper: SignalDatabase) : Dat } fun getAllRateLimitedMessageIds(): Set { - val db = databaseHelper.signalReadableDatabase - val where = "(" + TYPE + " & " + MessageTypes.TOTAL_MASK + " & " + MessageTypes.MESSAGE_RATE_LIMITED_BIT + ") > 0" - val ids: MutableSet = HashSet() - db.query(TABLE_NAME, arrayOf(ID), where, null, null, null, null).use { cursor -> - while (cursor.moveToNext()) { - ids.add(CursorUtil.requireLong(cursor, ID)) - } - } - return ids + return readableDatabase + .select(ID) + .from("$TABLE_NAME INDEXED BY $INDEX_RATE_LIMITED") + .where("($TYPE & ${MessageTypes.MESSAGE_RATE_LIMITED_BIT}) != 0") + .run() + .readToSet { it.requireLong(ID) } } fun deleteMessagesInThreadBeforeDate(threadId: Long, date: Long, inclusive: Boolean): Int { diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/SQLiteDatabase.java b/app/src/main/java/org/thoughtcrime/securesms/database/SQLiteDatabase.java index 1a8664c8af..803909cd0f 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/SQLiteDatabase.java +++ b/app/src/main/java/org/thoughtcrime/securesms/database/SQLiteDatabase.java @@ -41,6 +41,7 @@ public class SQLiteDatabase implements SupportSQLiteDatabase { private static final long SLOW_WRITE_LOCK_WAIT_MS = TimeUnit.SECONDS.toMillis(3); private static final long SLOW_TRANSACTION_HOLD_MS = 750; private static final long SLOW_DIRECT_WRITE_MS = 250; + private static final long SLOW_DIRECT_DELETE_MS = TimeUnit.SECONDS.toMillis(1); private static final long SLOW_QUERY_MS = TimeUnit.SECONDS.toMillis(1); public static final int CONFLICT_ROLLBACK = 1; @@ -668,7 +669,9 @@ public class SQLiteDatabase implements SupportSQLiteDatabase { long elapsedMs = (System.nanoTime() - startNs) / 1_000_000L; - if (elapsedMs >= SLOW_DIRECT_WRITE_MS) { + long threshold = "delete()".equals(methodName) ? SLOW_DIRECT_DELETE_MS : SLOW_DIRECT_WRITE_MS; + + if (elapsedMs >= threshold) { Log.w(TAG, "Slow direct write: " + methodName + " on " + table + " took " + elapsedMs + "ms (query=" + query + ")", new Throwable()); logQueryPlan(methodName, queryPlanSql, queryPlanArgs); } diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/SlowTransactionInternalNotifier.kt b/app/src/main/java/org/thoughtcrime/securesms/database/SlowTransactionInternalNotifier.kt index abdc40ca13..45e54d0906 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/SlowTransactionInternalNotifier.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/SlowTransactionInternalNotifier.kt @@ -26,8 +26,7 @@ import kotlin.time.Duration.Companion.milliseconds import kotlin.time.Duration.Companion.minutes /** - * Notifier that surfaces SQLite write-lock contention. Gated behind the - * [RemoteConfig.slowDatabaseNotifications] flag. + * Notifier that surfaces SQLite write-lock contention. Gated behind the [RemoteConfig.slowDatabaseNotifications] flag. */ object SlowTransactionInternalNotifier { @@ -35,6 +34,13 @@ object SlowTransactionInternalNotifier { private val NOTIFY_INTERVAL = 30.minutes + private val IGNORED_STACK_TRACE_CLASSES = listOf( + "BackupRepository", + "BackupMessagesJob", + "ArchiveAttachmentReconciliationJob", + "SubmitDebugLogRepository" + ) + private val count = AtomicInteger(0) @Volatile @@ -46,6 +52,10 @@ object SlowTransactionInternalNotifier { return } + if (isExpectedSlowOperation()) { + return + } + if (count.incrementAndGet() < THRESHOLD) { return } @@ -72,4 +82,15 @@ object SlowTransactionInternalNotifier { NotificationManagerCompat.from(context).notify(NotificationIds.INTERNAL_ERROR, notification) } + + private fun isExpectedSlowOperation(): Boolean { + return Thread + .currentThread() + .stackTrace + .any { element -> + IGNORED_STACK_TRACE_CLASSES.any { + element.className.contains(it) + } + } + } } diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/helpers/SignalDatabaseMigrations.kt b/app/src/main/java/org/thoughtcrime/securesms/database/helpers/SignalDatabaseMigrations.kt index 931caecd37..2d5180c8fd 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/helpers/SignalDatabaseMigrations.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/helpers/SignalDatabaseMigrations.kt @@ -172,6 +172,7 @@ import org.thoughtcrime.securesms.database.helpers.migration.V316_AddVerifiedGro import org.thoughtcrime.securesms.database.helpers.migration.V317_AddMessageThreadDateReceivedUnreadIndex import org.thoughtcrime.securesms.database.helpers.migration.V318_AddMessageNotificationStateIndex import org.thoughtcrime.securesms.database.helpers.migration.V319_AddAttachmentAndMessageIndexes +import org.thoughtcrime.securesms.database.helpers.migration.V320_AddAttachmentThumbnailFileAndUuidIndexes import org.thoughtcrime.securesms.database.SQLiteDatabase as SignalSqliteDatabase /** @@ -351,10 +352,11 @@ object SignalDatabaseMigrations { 316 to V316_AddVerifiedGroupNameHashMigration, 317 to V317_AddMessageThreadDateReceivedUnreadIndex, 318 to V318_AddMessageNotificationStateIndex, - 319 to V319_AddAttachmentAndMessageIndexes + 319 to V319_AddAttachmentAndMessageIndexes, + 320 to V320_AddAttachmentThumbnailFileAndUuidIndexes ) - const val DATABASE_VERSION = 319 + const val DATABASE_VERSION = 320 @JvmStatic fun migrate(context: Application, db: SignalSqliteDatabase, oldVersion: Int, newVersion: Int) { diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/helpers/migration/V320_AddAttachmentThumbnailFileAndUuidIndexes.kt b/app/src/main/java/org/thoughtcrime/securesms/database/helpers/migration/V320_AddAttachmentThumbnailFileAndUuidIndexes.kt new file mode 100644 index 0000000000..acd7b69c36 --- /dev/null +++ b/app/src/main/java/org/thoughtcrime/securesms/database/helpers/migration/V320_AddAttachmentThumbnailFileAndUuidIndexes.kt @@ -0,0 +1,13 @@ +package org.thoughtcrime.securesms.database.helpers.migration + +import android.app.Application +import org.thoughtcrime.securesms.database.SQLiteDatabase + +@Suppress("ClassName") +object V320_AddAttachmentThumbnailFileAndUuidIndexes : SignalDatabaseMigration { + override fun migrate(context: Application, db: SQLiteDatabase, oldVersion: Int, newVersion: Int) { + db.execSQL("CREATE INDEX IF NOT EXISTS attachment_thumbnail_file_index ON attachment (thumbnail_file) WHERE thumbnail_file IS NOT NULL") + db.execSQL("CREATE INDEX IF NOT EXISTS attachment_uuid_index ON attachment (attachment_uuid) WHERE attachment_uuid IS NOT NULL") + db.execSQL("CREATE INDEX IF NOT EXISTS message_rate_limited_index ON message (_id) WHERE (type & 128) != 0") + } +} diff --git a/app/src/test/java/org/thoughtcrime/securesms/avatar/AvatarPickerStorageTest_cleanOrphans.kt b/app/src/test/java/org/thoughtcrime/securesms/avatar/AvatarPickerStorageTest_cleanOrphans.kt new file mode 100644 index 0000000000..0d2ea7c5cb --- /dev/null +++ b/app/src/test/java/org/thoughtcrime/securesms/avatar/AvatarPickerStorageTest_cleanOrphans.kt @@ -0,0 +1,137 @@ +/* + * Copyright 2026 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + +package org.thoughtcrime.securesms.avatar + +import android.app.Application +import androidx.test.core.app.ApplicationProvider +import assertk.assertThat +import assertk.assertions.isFalse +import assertk.assertions.isTrue +import org.junit.BeforeClass +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config +import org.signal.core.util.logging.Log +import org.thoughtcrime.securesms.database.SignalDatabase +import org.thoughtcrime.securesms.mms.PartAuthority +import org.thoughtcrime.securesms.testutil.MockAppDependenciesRule +import org.thoughtcrime.securesms.testutil.SignalDatabaseRule +import org.thoughtcrime.securesms.testutil.SystemOutLogger +import java.io.File + +@Suppress("ClassName") +@RunWith(RobolectricTestRunner::class) +@Config(manifest = Config.NONE, application = Application::class) +class AvatarPickerStorageTest_cleanOrphans { + + @get:Rule + val signalDatabaseRule = SignalDatabaseRule() + + @get:Rule + val appDependencies = MockAppDependenciesRule() + + companion object { + @BeforeClass + @JvmStatic + fun setUpClass() { + Log.initialize(SystemOutLogger()) + } + } + + @Test + fun givenFileOnDiskAndInDatabase_whenCleanOrphans_thenNothingIsDeleted() { + val context = ApplicationProvider.getApplicationContext() + val filename = createDiskFile("avatar_matched.jpg").name + val savedAvatar = insertPhotoAvatar(filename) + + AvatarPickerStorage.cleanOrphans(context) + + assertThat(diskFileExists(filename)).isTrue() + assertThat(dbRowExists(savedAvatar)).isTrue() + } + + @Test + fun givenFileOnDiskOnly_whenCleanOrphans_thenFileIsDeleted() { + val context = ApplicationProvider.getApplicationContext() + val filename = createDiskFile("avatar_diskonly.jpg").name + + AvatarPickerStorage.cleanOrphans(context) + + assertThat(diskFileExists(filename)).isFalse() + } + + @Test + fun givenInDatabaseOnly_whenCleanOrphans_thenDatabaseRowIsDeleted() { + val context = ApplicationProvider.getApplicationContext() + val savedAvatar = insertPhotoAvatar("avatar_dbonly.jpg") + + AvatarPickerStorage.cleanOrphans(context) + + assertThat(dbRowExists(savedAvatar)).isFalse() + } + + @Test + fun givenOrphanFileAndOrphanDbRow_whenCleanOrphans_thenBothDeleted() { + val context = ApplicationProvider.getApplicationContext() + val diskOnlyFilename = createDiskFile("avatar_diskorphan.jpg").name + val dbOnlyAvatar = insertPhotoAvatar("avatar_dborphan.jpg") + + AvatarPickerStorage.cleanOrphans(context) + + assertThat(diskFileExists(diskOnlyFilename)).isFalse() + assertThat(dbRowExists(dbOnlyAvatar)).isFalse() + } + + @Test + fun givenTextAvatarAndOrphanFile_whenCleanOrphans_thenOrphanFileDeletedAndTextAvatarUntouched() { + val context = ApplicationProvider.getApplicationContext() + val textAvatar = SignalDatabase.avatarPicker.saveAvatarForSelf( + Avatar.Text("AB", Avatars.colors[0], Avatar.DatabaseId.NotSet) + ) + val diskOnlyFilename = createDiskFile("avatar_orphan_with_text.jpg").name + + AvatarPickerStorage.cleanOrphans(context) + + assertThat(diskFileExists(diskOnlyFilename)).isFalse() + assertThat(dbRowExists(textAvatar)).isTrue() + } + + @Test + fun givenNoFilesAndNoDatabase_whenCleanOrphans_thenNothingExplodes() { + val context = ApplicationProvider.getApplicationContext() + AvatarPickerStorage.cleanOrphans(context) + } + + // region helpers + + private fun createDiskFile(name: String): File { + val dir = ApplicationProvider.getApplicationContext().getDir("avatar_picker", android.content.Context.MODE_PRIVATE) + return File(dir, name).also { it.createNewFile() } + } + + private fun insertPhotoAvatar(filename: String): Avatar { + val uri = PartAuthority.getAvatarPickerUri(filename) + return SignalDatabase.avatarPicker.saveAvatarForSelf( + Avatar.Photo(uri, 0L, Avatar.DatabaseId.NotSet) + ) + } + + private fun diskFileExists(filename: String): Boolean { + val dir = ApplicationProvider.getApplicationContext().getDir("avatar_picker", android.content.Context.MODE_PRIVATE) + return File(dir, filename).exists() + } + + private fun dbRowExists(avatar: Avatar): Boolean { + val id = (avatar.databaseId as? Avatar.DatabaseId.Saved)?.id ?: return false + return SignalDatabase.avatarPicker.getAllAvatars().any { + (it.databaseId as? Avatar.DatabaseId.Saved)?.id == id + } + } + + // endregion +} diff --git a/app/src/test/java/org/thoughtcrime/securesms/database/AttachmentTableTest_deleteAttachmentsForMessage.kt b/app/src/test/java/org/thoughtcrime/securesms/database/AttachmentTableTest_deleteAttachmentsForMessage.kt new file mode 100644 index 0000000000..e33aa5bdbd --- /dev/null +++ b/app/src/test/java/org/thoughtcrime/securesms/database/AttachmentTableTest_deleteAttachmentsForMessage.kt @@ -0,0 +1,135 @@ +/* + * Copyright 2026 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + +package org.thoughtcrime.securesms.database + +import android.app.Application +import android.content.ContentValues +import assertk.assertThat +import assertk.assertions.isFalse +import assertk.assertions.isTrue +import org.junit.BeforeClass +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config +import org.signal.core.util.logging.Log +import org.signal.core.util.select +import org.thoughtcrime.securesms.testutil.MockAppDependenciesRule +import org.thoughtcrime.securesms.testutil.SignalDatabaseRule +import org.thoughtcrime.securesms.testutil.SystemOutLogger + +@Suppress("ClassName") +@RunWith(RobolectricTestRunner::class) +@Config(manifest = Config.NONE, application = Application::class) +class AttachmentTableTest_deleteAttachmentsForMessage { + + @get:Rule + val signalDatabaseRule = SignalDatabaseRule() + + @get:Rule + val appDependencies = MockAppDependenciesRule() + + companion object { + @BeforeClass + @JvmStatic + fun setUpClass() { + Log.initialize(SystemOutLogger()) + } + } + + @Test + fun givenAttachmentWithMetadata_whenMessageDeleted_thenMetadataRowIsCleanedUp() { + // GIVEN + val metadataId = insertMetadata("hash_unique_1") + val messageId = insertMessage() + insertAttachment(messageId = messageId, metadataId = metadataId) + + // WHEN + SignalDatabase.attachments.deleteAttachmentsForMessage(messageId) + + // THEN + assertThat(metadataExists(metadataId)).isFalse() + } + + @Test + fun givenAttachmentWithNoMetadata_whenMessageDeleted_thenNothingExplodes() { + // GIVEN + val messageId = insertMessage() + insertAttachment(messageId = messageId, metadataId = null) + + // WHEN + SignalDatabase.attachments.deleteAttachmentsForMessage(messageId) + + // THEN — no assertion needed, just verifying no crash + } + + @Test + fun givenTwoAttachmentsReferencingSameMetadata_whenFirstMessageDeleted_thenMetadataRowIsPreserved() { + // GIVEN — two attachments sharing the same deduped metadata row + val metadataId = insertMetadata("hash_shared") + val messageId1 = insertMessage() + val messageId2 = insertMessage() + insertAttachment(messageId = messageId1, metadataId = metadataId) + insertAttachment(messageId = messageId2, metadataId = metadataId) + + // WHEN + SignalDatabase.attachments.deleteAttachmentsForMessage(messageId1) + + // THEN + assertThat(metadataExists(metadataId)).isTrue() + } + + @Test + fun givenTwoAttachmentsReferencingSameMetadata_whenBothMessagesDeleted_thenMetadataRowIsCleanedUp() { + // GIVEN + val metadataId = insertMetadata("hash_shared_both") + val messageId1 = insertMessage() + val messageId2 = insertMessage() + insertAttachment(messageId = messageId1, metadataId = metadataId) + insertAttachment(messageId = messageId2, metadataId = metadataId) + + // WHEN + SignalDatabase.attachments.deleteAttachmentsForMessage(messageId1) + SignalDatabase.attachments.deleteAttachmentsForMessage(messageId2) + + // THEN + assertThat(metadataExists(metadataId)).isFalse() + } + + // region helpers + + private fun insertMetadata(plaintextHash: String): Long { + return SignalDatabase.attachmentMetadata.insert(plaintextHash, ByteArray(64) { it.toByte() }) + } + + private fun insertMessage(): Long { + return TestSms.insert(signalDatabaseRule.writeableDatabase) + } + + private fun insertAttachment(messageId: Long, metadataId: Long?): Long { + return SignalDatabase.writableDatabase.insert( + AttachmentTable.TABLE_NAME, + null, + ContentValues().apply { + put(AttachmentTable.MESSAGE_ID, messageId) + put(AttachmentTable.TRANSFER_STATE, AttachmentTable.TRANSFER_PROGRESS_DONE) + put(AttachmentTable.CONTENT_TYPE, "image/jpeg") + if (metadataId != null) put(AttachmentTable.METADATA_ID, metadataId) + } + ) + } + + private fun metadataExists(metadataId: Long): Boolean { + return SignalDatabase.writableDatabase.select(AttachmentMetadataTable.ID) + .from(AttachmentMetadataTable.TABLE_NAME) + .where("${AttachmentMetadataTable.ID} = ?", metadataId) + .run() + .use { it.moveToFirst() } + } + + // endregion +} diff --git a/app/src/test/java/org/thoughtcrime/securesms/database/MessageTableTest_stories.kt b/app/src/test/java/org/thoughtcrime/securesms/database/MessageTableTest_stories.kt index a98b0f9743..29ee1936f6 100644 --- a/app/src/test/java/org/thoughtcrime/securesms/database/MessageTableTest_stories.kt +++ b/app/src/test/java/org/thoughtcrime/securesms/database/MessageTableTest_stories.kt @@ -18,10 +18,12 @@ import org.junit.runner.RunWith import org.robolectric.RobolectricTestRunner import org.robolectric.annotation.Config import org.thoughtcrime.securesms.database.model.DistributionListId +import org.thoughtcrime.securesms.database.model.MmsMessageRecord import org.thoughtcrime.securesms.database.model.ParentStoryId import org.thoughtcrime.securesms.database.model.StoryType import org.thoughtcrime.securesms.mms.IncomingMessage import org.thoughtcrime.securesms.mms.OutgoingMessage +import org.thoughtcrime.securesms.mms.QuoteModel import org.thoughtcrime.securesms.recipients.Recipient import org.thoughtcrime.securesms.recipients.RecipientId import org.thoughtcrime.securesms.testutil.RecipientTestRule @@ -280,6 +282,164 @@ class MessageTableTest_stories { assertEquals(expected, oldestTimestamp) } + // region deleteUnarchivedStoriesOlderThan + + @Test + fun givenExpiredStory_whenIDeleteUnarchivedStoriesOlderThan_thenStoryIsDeleted() { + // GIVEN + val sender = others[0] + val storyId = insertIncomingStory(from = sender, sentTimeMillis = 100L).get().messageId + + // WHEN + val deleted = mms.deleteUnarchivedStoriesOlderThan(150L, hasSeenReleaseChannelStories = true) + + // THEN + assertEquals(1, deleted) + assertNull(mms.getMessageRecordOrNull(storyId)) + } + + @Test + fun givenFreshStory_whenIDeleteUnarchivedStoriesOlderThan_thenStoryIsPreserved() { + // GIVEN + val sender = others[0] + val storyId = insertIncomingStory(from = sender, sentTimeMillis = 200L).get().messageId + + // WHEN + val deleted = mms.deleteUnarchivedStoriesOlderThan(150L, hasSeenReleaseChannelStories = true) + + // THEN + assertEquals(0, deleted) + assertFalse(mms.getMessageRecordOrNull(storyId) == null) + } + + @Test + fun givenExpiredStoryWithGroupReply_whenIDeleteUnarchivedStoriesOlderThan_thenGroupReplyIsDeleted() { + // GIVEN + val sender = others[0] + val storyId = insertIncomingStory(from = sender, sentTimeMillis = 100L).get().messageId + val threadId = SignalDatabase.threads.getOrCreateThreadIdFor(Recipient.resolved(sender), ThreadTable.DistributionTypes.DEFAULT) + val replyId = insertIncomingGroupReply(from = sender, sentTimeMillis = 101L, parentStoryId = storyId, threadId = threadId) + + // WHEN + mms.deleteUnarchivedStoriesOlderThan(150L, hasSeenReleaseChannelStories = true) + + // THEN + assertNull(mms.getMessageRecordOrNull(replyId)) + } + + @Test + fun givenExpiredStoryWithDirectReply_whenIDeleteUnarchivedStoriesOlderThan_thenDirectReplyIsDisassociated() { + // GIVEN + val sender = others[0] + val storyId = insertIncomingStory(from = sender, sentTimeMillis = 100L).get().messageId + val threadId = SignalDatabase.threads.getOrCreateThreadIdFor(Recipient.resolved(sender), ThreadTable.DistributionTypes.DEFAULT) + val replyId = insertIncomingDirectReply(from = sender, sentTimeMillis = 101L, parentStoryId = storyId, threadId = threadId) + + // WHEN + mms.deleteUnarchivedStoriesOlderThan(150L, hasSeenReleaseChannelStories = true) + + // THEN + assertTrue(getQuote(replyId)!!.isOriginalMissing) + assertEquals("", getQuote(replyId)!!.displayText.toString()) + } + + @Test + fun givenFreshStoryWithReplies_whenIDeleteUnarchivedStoriesOlderThan_thenRepliesArePreserved() { + // GIVEN + val sender = others[0] + val storyId = insertIncomingStory(from = sender, sentTimeMillis = 200L).get().messageId + val threadId = SignalDatabase.threads.getOrCreateThreadIdFor(Recipient.resolved(sender), ThreadTable.DistributionTypes.DEFAULT) + val groupReplyId = insertIncomingGroupReply(from = sender, sentTimeMillis = 201L, parentStoryId = storyId, threadId = threadId) + val directReplyId = insertIncomingDirectReply(from = sender, sentTimeMillis = 202L, parentStoryId = storyId, threadId = threadId) + + // WHEN + mms.deleteUnarchivedStoriesOlderThan(150L, hasSeenReleaseChannelStories = true) + + // THEN + assertFalse(mms.getMessageRecordOrNull(groupReplyId) == null) + assertFalse(getQuote(directReplyId)!!.isOriginalMissing) + } + + // endregion + + // region deleteStoriesForRecipient + + @Test + fun givenStoriesForRecipient_whenIDeleteStoriesForRecipient_thenStoriesAreDeleted() { + // GIVEN + val sender = others[0] + val story1 = insertIncomingStory(from = sender, sentTimeMillis = 100L).get().messageId + val story2 = insertIncomingStory(from = sender, sentTimeMillis = 200L).get().messageId + + // WHEN + val deleted = mms.deleteStoriesForRecipient(sender) + + // THEN + assertEquals(2, deleted) + assertNull(mms.getMessageRecordOrNull(story1)) + assertNull(mms.getMessageRecordOrNull(story2)) + } + + @Test + fun givenStoriesForMultipleRecipients_whenIDeleteStoriesForRecipient_thenOtherStoriesArePreserved() { + // GIVEN + val sender = others[0] + val otherSender = others[1] + insertIncomingStory(from = sender, sentTimeMillis = 100L) + val otherStory = insertIncomingStory(from = otherSender, sentTimeMillis = 100L).get().messageId + + // WHEN + mms.deleteStoriesForRecipient(sender) + + // THEN + assertFalse(mms.getMessageRecordOrNull(otherStory) == null) + } + + @Test + fun givenStoryWithGroupReply_whenIDeleteStoriesForRecipient_thenGroupReplyIsDeleted() { + // GIVEN + val sender = others[0] + val storyId = insertIncomingStory(from = sender, sentTimeMillis = 100L).get().messageId + val threadId = SignalDatabase.threads.getOrCreateThreadIdFor(Recipient.resolved(sender), ThreadTable.DistributionTypes.DEFAULT) + val replyId = insertIncomingGroupReply(from = sender, sentTimeMillis = 101L, parentStoryId = storyId, threadId = threadId) + + // WHEN + mms.deleteStoriesForRecipient(sender) + + // THEN + assertNull(mms.getMessageRecordOrNull(replyId)) + } + + @Test + fun givenStoryWithDirectReply_whenIDeleteStoriesForRecipient_thenDirectReplyIsDisassociated() { + // GIVEN + val sender = others[0] + val storyId = insertIncomingStory(from = sender, sentTimeMillis = 100L).get().messageId + val threadId = SignalDatabase.threads.getOrCreateThreadIdFor(Recipient.resolved(sender), ThreadTable.DistributionTypes.DEFAULT) + val replyId = insertIncomingDirectReply(from = sender, sentTimeMillis = 101L, parentStoryId = storyId, threadId = threadId) + + // WHEN + mms.deleteStoriesForRecipient(sender) + + // THEN + assertTrue(getQuote(replyId)!!.isOriginalMissing) + assertEquals("", getQuote(replyId)!!.displayText.toString()) + } + + @Test + fun givenNoThread_whenIDeleteStoriesForRecipient_thenReturnZero() { + // GIVEN + val recipient = recipients.createRecipient("No Thread") + + // WHEN + val deleted = mms.deleteStoriesForRecipient(recipient) + + // THEN + assertEquals(0, deleted) + } + + // endregion + private fun insertOutgoingStory( recipient: Recipient, sentTimeMillis: Long, @@ -327,4 +487,35 @@ class MessageTableTest_stories { -1L ) } + + private fun insertIncomingGroupReply(from: RecipientId, sentTimeMillis: Long, parentStoryId: Long, threadId: Long): Long { + return mms.insertMessageInbox( + IncomingMessage( + type = MessageType.NORMAL, + from = from, + sentTimeMillis = sentTimeMillis, + serverTimeMillis = sentTimeMillis, + receivedTimeMillis = sentTimeMillis, + parentStoryId = ParentStoryId.GroupReply(parentStoryId) + ), + threadId + ).get().messageId + } + + private fun insertIncomingDirectReply(from: RecipientId, sentTimeMillis: Long, parentStoryId: Long, threadId: Long): Long { + return mms.insertMessageInbox( + IncomingMessage( + type = MessageType.NORMAL, + from = from, + sentTimeMillis = sentTimeMillis, + serverTimeMillis = sentTimeMillis, + receivedTimeMillis = sentTimeMillis, + parentStoryId = ParentStoryId.DirectReply(parentStoryId), + quote = QuoteModel(parentStoryId, from, "original story text", false, null, null, QuoteModel.Type.NORMAL, null) + ), + threadId + ).get().messageId + } + + private fun getQuote(messageId: Long) = (mms.getMessageRecord(messageId) as MmsMessageRecord).quote }