From 0d390769d4530631b7d4266c3027f38cf84fa55e Mon Sep 17 00:00:00 2001 From: Cody Henthorne Date: Fri, 22 Aug 2025 13:18:07 -0400 Subject: [PATCH] Add key reuse to create keys operation in backup job. --- .../securesms/database/AttachmentTable.kt | 88 ++++- .../helpers/SignalDatabaseMigrations.kt | 6 +- .../migration/V287_FixInvalidArchiveState.kt | 19 + .../securesms/jobs/BackupMessagesJob.kt | 33 +- ...eKeyForAttachmentsThatNeedArchiveUpload.kt | 340 ++++++++++++++++++ .../V287_FixInvalidArchiveStateTest.kt | 256 +++++++++++++ .../testing/TestSignalSQLiteDatabase.kt | 14 +- .../core/util/SQLiteDatabaseExtensions.kt | 2 +- 8 files changed, 739 insertions(+), 19 deletions(-) create mode 100644 app/src/main/java/org/thoughtcrime/securesms/database/helpers/migration/V287_FixInvalidArchiveState.kt create mode 100644 app/src/test/java/org/thoughtcrime/securesms/database/AttachmentTableTest_createRemoteKeyForAttachmentsThatNeedArchiveUpload.kt create mode 100644 app/src/test/java/org/thoughtcrime/securesms/database/helpers/migration/V287_FixInvalidArchiveStateTest.kt 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 5e50d005d7..3290b46ac3 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/AttachmentTable.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/AttachmentTable.kt @@ -172,7 +172,6 @@ class AttachmentTable( const val DISPLAY_ORDER = "display_order" const val UPLOAD_TIMESTAMP = "upload_timestamp" const val ARCHIVE_CDN = "archive_cdn" - const val ARCHIVE_TRANSFER_FILE = "archive_transfer_file" const val ARCHIVE_TRANSFER_STATE = "archive_transfer_state" const val THUMBNAIL_RESTORE_STATE = "thumbnail_restore_state" const val ATTACHMENT_UUID = "attachment_uuid" @@ -652,10 +651,13 @@ class AttachmentTable( * At archive creation time, we need to ensure that all relevant attachments have populated [REMOTE_KEY]s. * This does that. */ - fun createRemoteKeyForAttachmentsThatNeedArchiveUpload(): Int { - var count = 0 + fun createRemoteKeyForAttachmentsThatNeedArchiveUpload(): CreateRemoteKeyResult { + var totalCount = 0 + var notQuoteOrStickerDupeNotFoundCount = 0 + var notQuoteOrStickerDupeFoundCount = 0 - writableDatabase.select(ID, REMOTE_KEY, DATA_FILE, DATA_RANDOM) + val missingKeys = readableDatabase + .select(ID, DATA_FILE, QUOTE, STICKER_ID) .from(TABLE_NAME) .where( """ @@ -666,21 +668,75 @@ class AttachmentTable( """ ) .run() - .forEach { cursor -> - val attachmentId = AttachmentId(cursor.requireLong(ID)) - Log.w(TAG, "[createRemoteKeyForAttachmentsThatNeedArchiveUpload][$attachmentId] Missing key. Generating.") + .readToList { Triple(AttachmentId(it.requireLong(ID)), it.requireBoolean(QUOTE), it.requireInt(STICKER_ID) >= 0) to it.requireNonNullString(DATA_FILE) } + .groupBy({ (_, dataFile) -> dataFile }, { (record, _) -> record }) - val key = cursor.requireString(REMOTE_KEY)?.let { Base64.decode(it) } ?: Util.getSecretBytes(64) + missingKeys.forEach { dataFile, ids -> + val duplicateAttachmentWithRemoteData = readableDatabase + .select() + .from(TABLE_NAME) + .where("$DATA_FILE = ? AND $DATA_RANDOM NOT NULL AND $REMOTE_KEY NOT NULL AND $REMOTE_LOCATION NOT NULL AND $REMOTE_DIGEST NOT NULL", dataFile) + .orderBy("$ID DESC") + .limit(1) + .run() + .readToSingleObject { cursor -> + val duplicateAttachment = cursor.readAttachment() + val dataFileInfo = cursor.readDataFileInfo()!! - writableDatabase.update(TABLE_NAME) - .values(REMOTE_KEY to Base64.encodeWithPadding(key)) - .where("$ID = ?", attachmentId.id) - .run() + duplicateAttachment to dataFileInfo + } - count++ + if (duplicateAttachmentWithRemoteData != null) { + val (duplicateAttachment, duplicateAttachmentDataInfo) = duplicateAttachmentWithRemoteData + + ids.forEach { (attachmentId, isQuote, isSticker) -> + Log.w(TAG, "[createRemoteKeyForAttachmentsThatNeedArchiveUpload][$attachmentId] Missing key but found same data file with remote data. Updating. isQuote:$isQuote isSticker:$isSticker") + + writableDatabase + .update(TABLE_NAME) + .values( + REMOTE_KEY to duplicateAttachment.remoteKey, + REMOTE_LOCATION to duplicateAttachment.remoteLocation, + REMOTE_DIGEST to duplicateAttachment.remoteDigest, + REMOTE_INCREMENTAL_DIGEST to duplicateAttachment.incrementalDigest?.takeIf { it.isNotEmpty() }, + REMOTE_INCREMENTAL_DIGEST_CHUNK_SIZE to duplicateAttachment.incrementalMacChunkSize, + UPLOAD_TIMESTAMP to duplicateAttachment.uploadTimestamp, + ARCHIVE_CDN to duplicateAttachment.archiveCdn, + ARCHIVE_TRANSFER_STATE to duplicateAttachment.archiveTransferState.value, + THUMBNAIL_FILE to duplicateAttachmentDataInfo.thumbnailFile, + THUMBNAIL_RANDOM to duplicateAttachmentDataInfo.thumbnailRandom, + THUMBNAIL_RESTORE_STATE to duplicateAttachmentDataInfo.thumbnailRestoreState + ) + .where("$ID = ?", attachmentId.id) + .run() + + if (!isQuote && !isSticker) { + notQuoteOrStickerDupeFoundCount++ + } + + totalCount++ + } + } else { + ids.forEach { (attachmentId, isQuote, isSticker) -> + Log.w(TAG, "[createRemoteKeyForAttachmentsThatNeedArchiveUpload][$attachmentId] Missing key. Generating. isQuote:$isQuote isSticker:$isSticker") + + val key = Util.getSecretBytes(64) + + writableDatabase.update(TABLE_NAME) + .values(REMOTE_KEY to Base64.encodeWithPadding(key)) + .where("$ID = ?", attachmentId.id) + .run() + + totalCount++ + + if (!isQuote && !isSticker) { + notQuoteOrStickerDupeNotFoundCount++ + } + } } + } - return count + return CreateRemoteKeyResult(totalCount, notQuoteOrStickerDupeNotFoundCount, notQuoteOrStickerDupeFoundCount) } /** @@ -3088,4 +3144,8 @@ class AttachmentTable( val validForArchiveTransferStateCounts: Map, val estimatedThumbnailCount: Long ) + + data class CreateRemoteKeyResult(val totalCount: Int, val notQuoteOrSickerDupeNotFoundCount: Int, val notQuoteOrSickerDupeFoundCount: Int) { + val unexpectedKeyCreation = notQuoteOrSickerDupeFoundCount > 0 || notQuoteOrSickerDupeNotFoundCount > 0 + } } 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 6622127b68..fa60639234 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 @@ -141,6 +141,7 @@ import org.thoughtcrime.securesms.database.helpers.migration.V283_ViewOnceRemote import org.thoughtcrime.securesms.database.helpers.migration.V284_SetPlaceholderGroupFlag import org.thoughtcrime.securesms.database.helpers.migration.V285_AddEpochToCallLinksTable import org.thoughtcrime.securesms.database.helpers.migration.V286_FixRemoteKeyEncoding +import org.thoughtcrime.securesms.database.helpers.migration.V287_FixInvalidArchiveState import org.thoughtcrime.securesms.database.SQLiteDatabase as SignalSqliteDatabase /** @@ -287,10 +288,11 @@ object SignalDatabaseMigrations { 283 to V283_ViewOnceRemoteDataCleanup, 284 to V284_SetPlaceholderGroupFlag, 285 to V285_AddEpochToCallLinksTable, - 286 to V286_FixRemoteKeyEncoding + 286 to V286_FixRemoteKeyEncoding, + 287 to V287_FixInvalidArchiveState ) - const val DATABASE_VERSION = 286 + const val DATABASE_VERSION = 287 @JvmStatic fun migrate(context: Application, db: SignalSqliteDatabase, oldVersion: Int, newVersion: Int) { diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/helpers/migration/V287_FixInvalidArchiveState.kt b/app/src/main/java/org/thoughtcrime/securesms/database/helpers/migration/V287_FixInvalidArchiveState.kt new file mode 100644 index 0000000000..4a52c9302e --- /dev/null +++ b/app/src/main/java/org/thoughtcrime/securesms/database/helpers/migration/V287_FixInvalidArchiveState.kt @@ -0,0 +1,19 @@ +/* + * Copyright 2024 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + +package org.thoughtcrime.securesms.database.helpers.migration + +import android.app.Application +import org.thoughtcrime.securesms.database.SQLiteDatabase + +/** + * Ensure archive_transfer_state is clear if an attachment is missing a remote_key. + */ +@Suppress("ClassName") +object V287_FixInvalidArchiveState : SignalDatabaseMigration { + override fun migrate(context: Application, db: SQLiteDatabase, oldVersion: Int, newVersion: Int) { + db.execSQL("UPDATE attachment SET archive_cdn = null, archive_transfer_state = 0 WHERE remote_key IS NULL AND archive_transfer_state = 3") + } +} diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/BackupMessagesJob.kt b/app/src/main/java/org/thoughtcrime/securesms/jobs/BackupMessagesJob.kt index 63bc26490f..83b01622e4 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/BackupMessagesJob.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/BackupMessagesJob.kt @@ -5,6 +5,12 @@ package org.thoughtcrime.securesms.jobs +import android.app.Notification +import android.app.PendingIntent +import android.content.Intent +import androidx.core.app.NotificationCompat +import androidx.core.app.NotificationManagerCompat +import org.signal.core.util.PendingIntentFlags import org.signal.core.util.Stopwatch import org.signal.core.util.isNotNullOrBlank import org.signal.core.util.logging.Log @@ -12,6 +18,7 @@ import org.signal.core.util.logging.logW import org.signal.libsignal.messagebackup.BackupForwardSecrecyToken import org.signal.libsignal.net.SvrBStoreResponse import org.signal.protos.resumableuploads.ResumableUpload +import org.thoughtcrime.securesms.R import org.thoughtcrime.securesms.backup.ArchiveUploadProgress import org.thoughtcrime.securesms.backup.RestoreState import org.thoughtcrime.securesms.backup.v2.ArchiveMediaItemIterator @@ -25,7 +32,10 @@ import org.thoughtcrime.securesms.jobmanager.impl.BackupMessagesConstraint import org.thoughtcrime.securesms.jobs.protos.BackupMessagesJobData import org.thoughtcrime.securesms.keyvalue.SignalStore import org.thoughtcrime.securesms.keyvalue.isDecisionPending +import org.thoughtcrime.securesms.logsubmit.SubmitDebugLogActivity import org.thoughtcrime.securesms.net.SignalNetwork +import org.thoughtcrime.securesms.notifications.NotificationChannels +import org.thoughtcrime.securesms.notifications.NotificationIds import org.thoughtcrime.securesms.providers.BlobProvider import org.thoughtcrime.securesms.recipients.Recipient import org.thoughtcrime.securesms.storage.StorageSyncHelper @@ -179,7 +189,13 @@ class BackupMessagesJob private constructor( Log.i(TAG, "Successfully stored data on SVRB.") stopwatch.split("svrb") - SignalDatabase.attachments.createRemoteKeyForAttachmentsThatNeedArchiveUpload().takeIf { it > 0 }?.let { count -> Log.w(TAG, "Needed to create $count remote keys.") } + val createKeyResult = SignalDatabase.attachments.createRemoteKeyForAttachmentsThatNeedArchiveUpload() + if (createKeyResult.totalCount > 0) { + Log.w(TAG, "Needed to create remote keys. $createKeyResult") + if (createKeyResult.unexpectedKeyCreation) { + maybePostRemoteKeyMissingNotification() + } + } stopwatch.split("keygen") SignalDatabase.attachments.clearIncrementalMacsForAttachmentsThatNeedArchiveUpload().takeIf { it > 0 }?.let { count -> Log.w(TAG, "Needed to clear $count incrementalMacs.") } @@ -409,6 +425,21 @@ class BackupMessagesJob private constructor( } } + private fun maybePostRemoteKeyMissingNotification() { + if (!RemoteConfig.internalUser || !SignalStore.backup.backsUpMedia) { + return + } + + val notification: Notification = NotificationCompat.Builder(context, NotificationChannels.getInstance().FAILURES) + .setSmallIcon(R.drawable.ic_notification) + .setContentTitle("[Internal-only] Unexpected remote key missing!") + .setContentText("Tap to send a debug log") + .setContentIntent(PendingIntent.getActivity(context, 0, Intent(context, SubmitDebugLogActivity::class.java), PendingIntentFlags.mutable())) + .build() + + NotificationManagerCompat.from(context).notify(NotificationIds.INTERNAL_ERROR, notification) + } + class Factory : Job.Factory { override fun create(parameters: Parameters, serializedData: ByteArray?): BackupMessagesJob { val jobData = if (serializedData != null) { diff --git a/app/src/test/java/org/thoughtcrime/securesms/database/AttachmentTableTest_createRemoteKeyForAttachmentsThatNeedArchiveUpload.kt b/app/src/test/java/org/thoughtcrime/securesms/database/AttachmentTableTest_createRemoteKeyForAttachmentsThatNeedArchiveUpload.kt new file mode 100644 index 0000000000..6e36704996 --- /dev/null +++ b/app/src/test/java/org/thoughtcrime/securesms/database/AttachmentTableTest_createRemoteKeyForAttachmentsThatNeedArchiveUpload.kt @@ -0,0 +1,340 @@ +/* + * Copyright 2025 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.isEqualTo +import assertk.assertions.isNotNull +import assertk.assertions.isNull +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.Base64 +import org.signal.core.util.logging.Log +import org.signal.core.util.update +import org.thoughtcrime.securesms.attachments.AttachmentId +import org.thoughtcrime.securesms.testutil.MockAppDependenciesRule +import org.thoughtcrime.securesms.testutil.SignalDatabaseRule +import org.thoughtcrime.securesms.testutil.SystemOutLogger +import java.util.UUID + +@Suppress("ClassName") +@RunWith(RobolectricTestRunner::class) +@Config(manifest = Config.NONE, application = Application::class) +class AttachmentTableTest_createRemoteKeyForAttachmentsThatNeedArchiveUpload { + + @get:Rule val signalDatabaseRule = SignalDatabaseRule() + + @get:Rule val applicationDependencies = MockAppDependenciesRule() + + companion object { + @BeforeClass + @JvmStatic + fun setUpClass() { + Log.initialize(SystemOutLogger()) + } + } + + @Test + fun whenNoEligibleAttachments_returnsZero() { + val result = SignalDatabase.attachments.createRemoteKeyForAttachmentsThatNeedArchiveUpload() + assertThat(result.totalCount).isEqualTo(0) + } + + @Test + fun whenAttachmentHasArchiveTransferStateInProgress_returnsZero() { + val attachmentId = insertWithData() + SignalDatabase.attachments.setArchiveTransferState(attachmentId, AttachmentTable.ArchiveTransferState.UPLOAD_IN_PROGRESS) + + val result = SignalDatabase.attachments.createRemoteKeyForAttachmentsThatNeedArchiveUpload() + assertThat(result.totalCount).isEqualTo(0) + } + + @Test + fun whenAttachmentMissingDataFile_returnsZero() { + val attachmentId = insertWithoutData() // No data file set + + val result = SignalDatabase.attachments.createRemoteKeyForAttachmentsThatNeedArchiveUpload() + assertThat(result.totalCount).isEqualTo(0) + } + + @Test + fun whenTransferStateNotDone_returnsZero() { + val attachmentId = insertWithData() + SignalDatabase.attachments.setTransferState(1L, attachmentId, 1) // Not TRANSFER_PROGRESS_DONE (0) + + val result = SignalDatabase.attachments.createRemoteKeyForAttachmentsThatNeedArchiveUpload() + assertThat(result.totalCount).isEqualTo(0) + } + + @Test + fun whenAttachmentAlreadyHasRemoteKey_returnsZero() { + val attachmentId = insertWithData() + // Set a remote key + val remoteKey = Base64.encodeWithPadding(byteArrayOf(1, 2, 3, 4)) + setRemoteKey(attachmentId, remoteKey) + + val result = SignalDatabase.attachments.createRemoteKeyForAttachmentsThatNeedArchiveUpload() + assertThat(result.totalCount).isEqualTo(0) + } + + @Test + fun whenOneEligibleAttachment_returnsOneAndCreatesRemoteKey() { + val attachmentId = insertWithData() + + // Verify attachment has no remote key initially + val attachmentBefore = SignalDatabase.attachments.getAttachment(attachmentId) + assertThat(attachmentBefore?.remoteKey).isNull() + + val result = SignalDatabase.attachments.createRemoteKeyForAttachmentsThatNeedArchiveUpload() + assertThat(result.totalCount).isEqualTo(1) + + // Verify remote key was created + val attachmentAfter = SignalDatabase.attachments.getAttachment(attachmentId) + assertThat(attachmentAfter?.remoteKey).isNotNull() + } + + @Test + fun whenMultipleEligibleAttachments_returnsCorrectCountAndCreatesKeys() { + val attachmentId1 = insertWithData() + val attachmentId2 = insertWithData() + val attachmentId3 = insertWithData() + + // Verify all attachments have no remote keys initially + assertThat(SignalDatabase.attachments.getAttachment(attachmentId1)?.remoteKey).isNull() + assertThat(SignalDatabase.attachments.getAttachment(attachmentId2)?.remoteKey).isNull() + assertThat(SignalDatabase.attachments.getAttachment(attachmentId3)?.remoteKey).isNull() + + val result = SignalDatabase.attachments.createRemoteKeyForAttachmentsThatNeedArchiveUpload() + assertThat(result.totalCount).isEqualTo(3) + + // Verify all remote keys were created + assertThat(SignalDatabase.attachments.getAttachment(attachmentId1)?.remoteKey).isNotNull() + assertThat(SignalDatabase.attachments.getAttachment(attachmentId2)?.remoteKey).isNotNull() + assertThat(SignalDatabase.attachments.getAttachment(attachmentId3)?.remoteKey).isNotNull() + } + + @Test + fun whenMixedScenarios_returnsCorrectCount() { + // Eligible attachment - has data file, transfer done, archive state NONE, no remote key + val eligibleAttachmentId = insertWithData() + + // Ineligible - has remote key already + val attachmentWithKeyId = insertWithData() + setRemoteKey(attachmentWithKeyId, Base64.encodeWithPadding(byteArrayOf(1, 2, 3, 4))) + + // Ineligible - archive transfer state is not NONE + val inProgressAttachmentId = insertWithData() + SignalDatabase.attachments.setArchiveTransferState(inProgressAttachmentId, AttachmentTable.ArchiveTransferState.UPLOAD_IN_PROGRESS) + + // Ineligible - no data file + val noDataFileAttachmentId = insertWithoutData() + + val result = SignalDatabase.attachments.createRemoteKeyForAttachmentsThatNeedArchiveUpload() + assertThat(result.totalCount).isEqualTo(1) + + // Verify only the eligible attachment got a remote key + assertThat(SignalDatabase.attachments.getAttachment(eligibleAttachmentId)?.remoteKey).isNotNull() + assertThat(SignalDatabase.attachments.getAttachment(attachmentWithKeyId)?.remoteKey).isNotNull() // Already had one + assertThat(SignalDatabase.attachments.getAttachment(inProgressAttachmentId)?.remoteKey).isNull() + assertThat(SignalDatabase.attachments.getAttachment(noDataFileAttachmentId)?.remoteKey).isNull() + } + + @Test + fun whenCalledTwice_secondCallReturnsZero() { + val attachmentId = insertWithData() + + // First call should create remote key + val firstResult = SignalDatabase.attachments.createRemoteKeyForAttachmentsThatNeedArchiveUpload() + assertThat(firstResult.totalCount).isEqualTo(1) + + // Second call should find no eligible attachments since remote key now exists + val secondResult = SignalDatabase.attachments.createRemoteKeyForAttachmentsThatNeedArchiveUpload() + assertThat(secondResult.totalCount).isEqualTo(0) + + // Verify attachment still has remote key + assertThat(SignalDatabase.attachments.getAttachment(attachmentId)?.remoteKey).isNotNull() + } + + @Test + fun whenMatchingDataFileAndHashExists_reusesRemoteKey() { + val dataFile = "/shared/path/attachment.jpg" + val dataHashEnd = "shared_hash_end" + val existingRemoteKey = Base64.encodeWithPadding(byteArrayOf(1, 2, 3, 4, 5, 6, 7, 8)) + + // Create source attachment with remote key, location, digest + val sourceAttachmentId = insertAttachmentDirectly( + dataFile = dataFile, + transferState = AttachmentTable.TRANSFER_PROGRESS_DONE, + archiveTransferState = AttachmentTable.ArchiveTransferState.FINISHED.value, + remoteKey = existingRemoteKey, + dataHashEnd = dataHashEnd, + remoteLocation = "cdn-location-123", + remoteDigest = byteArrayOf(9, 10, 11, 12) + ) + + // Create target attachment with same data file and hash but no remote key + val targetAttachmentId = insertAttachmentDirectly( + dataFile = dataFile, + transferState = AttachmentTable.TRANSFER_PROGRESS_DONE, + archiveTransferState = AttachmentTable.ArchiveTransferState.NONE.value, + remoteKey = null, + dataHashEnd = dataHashEnd + ) + + val result = SignalDatabase.attachments.createRemoteKeyForAttachmentsThatNeedArchiveUpload() + assertThat(result.totalCount).isEqualTo(1) + + // Verify target attachment reused the remote key, location, and digest + val targetAttachment = SignalDatabase.attachments.getAttachment(targetAttachmentId)!! + assertThat(targetAttachment.remoteKey).isEqualTo(existingRemoteKey) + assertThat(targetAttachment.remoteLocation).isEqualTo("cdn-location-123") + assertThat(targetAttachment.remoteDigest.contentEquals(byteArrayOf(9, 10, 11, 12))).isTrue() + } + + @Test + fun whenMultipleMatchesExist_reusesFromLatestMatch() { + val dataFile = "/shared/path/attachment.jpg" + val firstRemoteKey = Base64.encodeWithPadding(byteArrayOf(1, 2, 3, 4)) + val secondRemoteKey = Base64.encodeWithPadding(byteArrayOf(5, 6, 7, 8)) + + // Create first source attachment + val firstSourceId = insertAttachmentDirectly( + dataFile = dataFile, + transferState = AttachmentTable.TRANSFER_PROGRESS_DONE, + archiveTransferState = AttachmentTable.ArchiveTransferState.FINISHED.value, + remoteKey = firstRemoteKey, + remoteLocation = "first-location", + remoteDigest = byteArrayOf(9, 10, 11, 12) + ) + + // Create second source attachment (inserted later) + val secondSourceId = insertAttachmentDirectly( + dataFile = dataFile, + transferState = AttachmentTable.TRANSFER_PROGRESS_DONE, + archiveTransferState = AttachmentTable.ArchiveTransferState.FINISHED.value, + remoteKey = secondRemoteKey, + remoteLocation = "second-location", + remoteDigest = byteArrayOf(13, 14, 15, 16) + ) + + // Create target attachment + val targetAttachmentId = insertAttachmentDirectly( + dataFile = dataFile, + transferState = AttachmentTable.TRANSFER_PROGRESS_DONE, + archiveTransferState = AttachmentTable.ArchiveTransferState.NONE.value, + remoteKey = null + ) + + val result = SignalDatabase.attachments.createRemoteKeyForAttachmentsThatNeedArchiveUpload() + assertThat(result.totalCount).isEqualTo(1) + + // Verify target attachment reused from the first match (by ID order desc) + val targetAttachment = SignalDatabase.attachments.getAttachment(targetAttachmentId)!! + assertThat(targetAttachment.remoteKey).isEqualTo(secondRemoteKey) + assertThat(targetAttachment.remoteLocation).isEqualTo("second-location") + assertThat(targetAttachment.remoteDigest.contentEquals(byteArrayOf(13, 14, 15, 16))).isTrue() + } + + @Test + fun whenSourceHasNoRemoteData_generatesNewKey() { + val dataFile = "/shared/path/attachment.jpg" + + // Create source attachment without remote key (should not be used for reuse) + val sourceAttachmentId = insertAttachmentDirectly( + dataFile = dataFile, + transferState = AttachmentTable.TRANSFER_PROGRESS_DONE, + archiveTransferState = AttachmentTable.ArchiveTransferState.NONE.value, + remoteKey = null + ) + + // Create target attachment + val targetAttachmentId = insertAttachmentDirectly( + dataFile = dataFile, + transferState = AttachmentTable.TRANSFER_PROGRESS_DONE, + archiveTransferState = AttachmentTable.ArchiveTransferState.NONE.value, + remoteKey = null + ) + + val result = SignalDatabase.attachments.createRemoteKeyForAttachmentsThatNeedArchiveUpload() + assertThat(result.totalCount).isEqualTo(2) // Both should get new keys + + // Verify both attachments got new keys + assertThat(SignalDatabase.attachments.getAttachment(sourceAttachmentId)?.remoteKey).isNotNull() + assertThat(SignalDatabase.attachments.getAttachment(targetAttachmentId)?.remoteKey).isNotNull() + } + + /** + * Creates an attachment that meets all criteria for archive upload: + * - ARCHIVE_TRANSFER_STATE = NONE (0) + * - DATA_FILE is not null + * - TRANSFER_STATE = TRANSFER_PROGRESS_DONE (0) + * - REMOTE_KEY is null + */ + fun insertWithData(dataFile: String = "/fake/path/attachment-${UUID.randomUUID()}.jpg"): AttachmentId { + return insertAttachmentDirectly( + dataFile = dataFile, + transferState = AttachmentTable.TRANSFER_PROGRESS_DONE, + archiveTransferState = AttachmentTable.ArchiveTransferState.NONE.value, + remoteKey = null + ) + } + + /** + * Creates an attachment without a data file (ineligible for archive upload) + */ + fun insertWithoutData(): AttachmentId { + return insertAttachmentDirectly( + dataFile = null, + transferState = AttachmentTable.TRANSFER_PROGRESS_DONE, + archiveTransferState = AttachmentTable.ArchiveTransferState.NONE.value, + remoteKey = null + ) + } + + /** + * Directly inserts an attachment with minimal required columns for testing + */ + private fun insertAttachmentDirectly( + dataFile: String?, + transferState: Int, + archiveTransferState: Int, + remoteKey: String?, + dataHashEnd: String? = null, + remoteLocation: String? = null, + remoteDigest: ByteArray? = null + ): AttachmentId { + val db = SignalDatabase.attachments.writableDatabase + + val values = ContentValues().apply { + put(AttachmentTable.DATA_FILE, dataFile) + put(AttachmentTable.DATA_RANDOM, dataFile?.toByteArray()) + put(AttachmentTable.TRANSFER_STATE, transferState) + put(AttachmentTable.ARCHIVE_TRANSFER_STATE, archiveTransferState) + put(AttachmentTable.REMOTE_KEY, remoteKey) + put(AttachmentTable.DATA_HASH_END, dataHashEnd) + put(AttachmentTable.REMOTE_LOCATION, remoteLocation) + put(AttachmentTable.REMOTE_DIGEST, remoteDigest) + } + + val id = db.insert(AttachmentTable.TABLE_NAME, null, values) + return AttachmentId(id) + } + + private fun setRemoteKey(attachmentId: AttachmentId, remoteKey: String) { + SignalDatabase.attachments.writableDatabase + .update(AttachmentTable.TABLE_NAME) + .values(AttachmentTable.REMOTE_KEY to remoteKey) + .where("${AttachmentTable.ID} = ?", attachmentId.id) + .run() + } +} diff --git a/app/src/test/java/org/thoughtcrime/securesms/database/helpers/migration/V287_FixInvalidArchiveStateTest.kt b/app/src/test/java/org/thoughtcrime/securesms/database/helpers/migration/V287_FixInvalidArchiveStateTest.kt new file mode 100644 index 0000000000..af6cdfb14c --- /dev/null +++ b/app/src/test/java/org/thoughtcrime/securesms/database/helpers/migration/V287_FixInvalidArchiveStateTest.kt @@ -0,0 +1,256 @@ +/* + * Copyright 2025 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + +package org.thoughtcrime.securesms.database.helpers.migration + +import android.app.Application +import android.content.ContentValues +import androidx.test.core.app.ApplicationProvider +import assertk.assertThat +import assertk.assertions.isEqualTo +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config +import org.thoughtcrime.securesms.database.AttachmentTable +import org.thoughtcrime.securesms.testutil.SignalDatabaseMigrationRule + +@Suppress("ClassName") +@RunWith(RobolectricTestRunner::class) +@Config(manifest = Config.NONE, application = Application::class) +class V287_FixInvalidArchiveStateTest { + + @get:Rule val signalDatabaseRule = SignalDatabaseMigrationRule(286) + + @Test + fun migrate_whenArchiveTransferStateIsFinishedAndRemoteKeyIsNull_clearsArchiveCdnAndSetsStateToNone() { + val attachmentId = insertAttachmentWithArchiveState( + archiveTransferState = AttachmentTable.ArchiveTransferState.FINISHED.value, + archiveCdn = 2, + remoteKey = null + ) + + val db = signalDatabaseRule.database + V287_FixInvalidArchiveState.migrate(ApplicationProvider.getApplicationContext(), db, 286, 287) + + val cursor = db.query( + AttachmentTable.TABLE_NAME, + arrayOf(AttachmentTable.ARCHIVE_CDN, AttachmentTable.ARCHIVE_TRANSFER_STATE), + "${AttachmentTable.ID} = ?", + arrayOf(attachmentId.toString()), + null, + null, + null + ) + + cursor.use { + assertThat(it.moveToFirst()).isEqualTo(true) + assertThat(it.isNull(it.getColumnIndexOrThrow(AttachmentTable.ARCHIVE_CDN))).isEqualTo(true) + assertThat(it.getInt(it.getColumnIndexOrThrow(AttachmentTable.ARCHIVE_TRANSFER_STATE))) + .isEqualTo(AttachmentTable.ArchiveTransferState.NONE.value) + } + } + + @Test + fun migrate_whenArchiveTransferStateIsFinishedButHasRemoteKey_noChanges() { + val attachmentId = insertAttachmentWithArchiveState( + archiveTransferState = AttachmentTable.ArchiveTransferState.FINISHED.value, + archiveCdn = 2, + remoteKey = "some-remote-key" + ) + + val db = signalDatabaseRule.database + V287_FixInvalidArchiveState.migrate(ApplicationProvider.getApplicationContext(), db, 286, 287) + + val cursor = db.query( + AttachmentTable.TABLE_NAME, + arrayOf(AttachmentTable.ARCHIVE_CDN, AttachmentTable.ARCHIVE_TRANSFER_STATE), + "${AttachmentTable.ID} = ?", + arrayOf(attachmentId.toString()), + null, + null, + null + ) + + cursor.use { + assertThat(it.moveToFirst()).isEqualTo(true) + assertThat(it.getInt(it.getColumnIndexOrThrow(AttachmentTable.ARCHIVE_CDN))).isEqualTo(2) + assertThat(it.getInt(it.getColumnIndexOrThrow(AttachmentTable.ARCHIVE_TRANSFER_STATE))) + .isEqualTo(AttachmentTable.ArchiveTransferState.FINISHED.value) + } + } + + @Test + fun migrate_whenArchiveTransferStateIsNoneAndRemoteKeyIsNull_noChanges() { + val attachmentId = insertAttachmentWithArchiveState( + archiveTransferState = AttachmentTable.ArchiveTransferState.NONE.value, + archiveCdn = 2, + remoteKey = null + ) + + val db = signalDatabaseRule.database + V287_FixInvalidArchiveState.migrate(ApplicationProvider.getApplicationContext(), db, 286, 287) + + val cursor = db.query( + AttachmentTable.TABLE_NAME, + arrayOf(AttachmentTable.ARCHIVE_CDN, AttachmentTable.ARCHIVE_TRANSFER_STATE), + "${AttachmentTable.ID} = ?", + arrayOf(attachmentId.toString()), + null, + null, + null + ) + + cursor.use { + assertThat(it.moveToFirst()).isEqualTo(true) + assertThat(it.getInt(it.getColumnIndexOrThrow(AttachmentTable.ARCHIVE_CDN))).isEqualTo(2) + assertThat(it.getInt(it.getColumnIndexOrThrow(AttachmentTable.ARCHIVE_TRANSFER_STATE))) + .isEqualTo(AttachmentTable.ArchiveTransferState.NONE.value) + } + } + + @Test + fun migrate_whenArchiveTransferStateIsUploadInProgressAndRemoteKeyIsNull_noChanges() { + val attachmentId = insertAttachmentWithArchiveState( + archiveTransferState = AttachmentTable.ArchiveTransferState.UPLOAD_IN_PROGRESS.value, + archiveCdn = 2, + remoteKey = null + ) + + val db = signalDatabaseRule.database + V287_FixInvalidArchiveState.migrate(ApplicationProvider.getApplicationContext(), db, 286, 287) + + val cursor = db.query( + AttachmentTable.TABLE_NAME, + arrayOf(AttachmentTable.ARCHIVE_CDN, AttachmentTable.ARCHIVE_TRANSFER_STATE), + "${AttachmentTable.ID} = ?", + arrayOf(attachmentId.toString()), + null, + null, + null + ) + + cursor.use { + assertThat(it.moveToFirst()).isEqualTo(true) + assertThat(it.getInt(it.getColumnIndexOrThrow(AttachmentTable.ARCHIVE_CDN))).isEqualTo(2) + assertThat(it.getInt(it.getColumnIndexOrThrow(AttachmentTable.ARCHIVE_TRANSFER_STATE))) + .isEqualTo(AttachmentTable.ArchiveTransferState.UPLOAD_IN_PROGRESS.value) + } + } + + @Test + fun migrate_whenMultipleAttachmentsWithMixedStates_onlyUpdatesFinishedStateWithNullRemoteKey() { + // These should be updated (FINISHED state + null remote_key) + val finishedNoKeyId1 = insertAttachmentWithArchiveState( + archiveTransferState = AttachmentTable.ArchiveTransferState.FINISHED.value, + archiveCdn = 1, + remoteKey = null + ) + val finishedNoKeyId2 = insertAttachmentWithArchiveState( + archiveTransferState = AttachmentTable.ArchiveTransferState.FINISHED.value, + archiveCdn = 3, + remoteKey = null + ) + + // These should NOT be updated + val finishedWithKeyId = insertAttachmentWithArchiveState( + archiveTransferState = AttachmentTable.ArchiveTransferState.FINISHED.value, + archiveCdn = 2, + remoteKey = "some-key" + ) + val noneId = insertAttachmentWithArchiveState( + archiveTransferState = AttachmentTable.ArchiveTransferState.NONE.value, + archiveCdn = 2, + remoteKey = null + ) + val inProgressId = insertAttachmentWithArchiveState( + archiveTransferState = AttachmentTable.ArchiveTransferState.UPLOAD_IN_PROGRESS.value, + archiveCdn = 1, + remoteKey = null + ) + + val db = signalDatabaseRule.database + V287_FixInvalidArchiveState.migrate(ApplicationProvider.getApplicationContext(), db, 286, 287) + + // Check finished attachments with null remote_key were updated + assertArchiveState(finishedNoKeyId1, expectedCdn = null, expectedState = AttachmentTable.ArchiveTransferState.NONE.value) + assertArchiveState(finishedNoKeyId2, expectedCdn = null, expectedState = AttachmentTable.ArchiveTransferState.NONE.value) + + // Check other states were not changed + assertArchiveState(finishedWithKeyId, expectedCdn = 2, expectedState = AttachmentTable.ArchiveTransferState.FINISHED.value) + assertArchiveState(noneId, expectedCdn = 2, expectedState = AttachmentTable.ArchiveTransferState.NONE.value) + assertArchiveState(inProgressId, expectedCdn = 1, expectedState = AttachmentTable.ArchiveTransferState.UPLOAD_IN_PROGRESS.value) + } + + @Test + fun migrate_whenNoAttachmentsMatchCriteria_noChanges() { + val noneId = insertAttachmentWithArchiveState( + archiveTransferState = AttachmentTable.ArchiveTransferState.NONE.value, + archiveCdn = 2, + remoteKey = null + ) + val inProgressId = insertAttachmentWithArchiveState( + archiveTransferState = AttachmentTable.ArchiveTransferState.UPLOAD_IN_PROGRESS.value, + archiveCdn = 1, + remoteKey = null + ) + val finishedWithKeyId = insertAttachmentWithArchiveState( + archiveTransferState = AttachmentTable.ArchiveTransferState.FINISHED.value, + archiveCdn = 3, + remoteKey = "has-key" + ) + + val db = signalDatabaseRule.database + V287_FixInvalidArchiveState.migrate(ApplicationProvider.getApplicationContext(), db, 286, 287) + + // Check no changes were made + assertArchiveState(noneId, expectedCdn = 2, expectedState = AttachmentTable.ArchiveTransferState.NONE.value) + assertArchiveState(inProgressId, expectedCdn = 1, expectedState = AttachmentTable.ArchiveTransferState.UPLOAD_IN_PROGRESS.value) + assertArchiveState(finishedWithKeyId, expectedCdn = 3, expectedState = AttachmentTable.ArchiveTransferState.FINISHED.value) + } + + private fun insertAttachmentWithArchiveState(archiveTransferState: Int, archiveCdn: Int?, remoteKey: String?): Long { + val db = signalDatabaseRule.database + + val values = ContentValues().apply { + put(AttachmentTable.DATA_FILE, "/fake/path/attachment.jpg") + put(AttachmentTable.DATA_RANDOM, "/fake/path/attachment.jpg".toByteArray()) + put(AttachmentTable.TRANSFER_STATE, AttachmentTable.TRANSFER_PROGRESS_DONE) + put(AttachmentTable.ARCHIVE_TRANSFER_STATE, archiveTransferState) + if (archiveCdn != null) { + put(AttachmentTable.ARCHIVE_CDN, archiveCdn) + } + put(AttachmentTable.REMOTE_KEY, remoteKey) + } + + return db.insert(AttachmentTable.TABLE_NAME, null, values) + } + + private fun assertArchiveState(attachmentId: Long, expectedCdn: Int?, expectedState: Int) { + val db = signalDatabaseRule.database + val cursor = db.query( + AttachmentTable.TABLE_NAME, + arrayOf(AttachmentTable.ARCHIVE_CDN, AttachmentTable.ARCHIVE_TRANSFER_STATE), + "${AttachmentTable.ID} = ?", + arrayOf(attachmentId.toString()), + null, + null, + null + ) + + cursor.use { + assertThat(it.moveToFirst()).isEqualTo(true) + + if (expectedCdn == null) { + assertThat(it.isNull(it.getColumnIndexOrThrow(AttachmentTable.ARCHIVE_CDN))).isEqualTo(true) + } else { + assertThat(it.getInt(it.getColumnIndexOrThrow(AttachmentTable.ARCHIVE_CDN))).isEqualTo(expectedCdn) + } + + assertThat(it.getInt(it.getColumnIndexOrThrow(AttachmentTable.ARCHIVE_TRANSFER_STATE))).isEqualTo(expectedState) + } + } +} diff --git a/app/src/test/java/org/thoughtcrime/securesms/testing/TestSignalSQLiteDatabase.kt b/app/src/test/java/org/thoughtcrime/securesms/testing/TestSignalSQLiteDatabase.kt index bfecc08918..57cd6db751 100644 --- a/app/src/test/java/org/thoughtcrime/securesms/testing/TestSignalSQLiteDatabase.kt +++ b/app/src/test/java/org/thoughtcrime/securesms/testing/TestSignalSQLiteDatabase.kt @@ -5,6 +5,8 @@ import android.database.Cursor import android.database.SQLException import androidx.sqlite.db.SupportSQLiteDatabase import androidx.sqlite.db.SupportSQLiteQuery +import io.mockk.every +import io.mockk.mockk import net.zetetic.database.sqlcipher.SQLiteQueryBuilder import java.util.Locale import android.database.sqlite.SQLiteDatabase as AndroidSQLiteDatabase @@ -228,7 +230,17 @@ class TestSignalSQLiteDatabase(private val database: SupportSQLiteDatabase) : Si } override fun compileStatement(sql: String): SQLCipherSQLiteStatement { - throw UnsupportedOperationException() + val statement = database.compileStatement(sql) + return mockk { + every { bindNull(any()) } answers { statement.bindNull(firstArg()) } + every { bindLong(any(), any()) } answers { statement.bindLong(firstArg(), secondArg()) } + every { bindDouble(any(), any()) } answers { statement.bindDouble(firstArg(), secondArg()) } + every { bindString(any(), any()) } answers { statement.bindString(firstArg(), secondArg()) } + every { bindBlob(any(), any()) } answers { statement.bindBlob(firstArg(), secondArg()) } + every { clearBindings() } answers { statement.clearBindings() } + every { executeUpdateDelete() } answers { statement.executeUpdateDelete() } + every { close() } answers { statement.close() } + } } override val isReadOnly: Boolean 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 index f30692de7b..1b7108a3d2 100644 --- a/core-util/src/main/java/org/signal/core/util/SQLiteDatabaseExtensions.kt +++ b/core-util/src/main/java/org/signal/core/util/SQLiteDatabaseExtensions.kt @@ -265,7 +265,7 @@ fun SupportSQLiteStatement.bindValue(index: Int, value: Any?) { is Boolean -> this.bindLong(index, value.toInt().toLong()) is ByteArray -> this.bindBlob(index, value) is Number -> { - if (value.toLong() == value) { + if (value.toLong() == value || value.toInt() == value || value.toShort() == value || value.toByte() == value) { this.bindLong(index, value.toLong()) } else { this.bindDouble(index, value.toDouble())