Skip to content

Commit e80ebd8

Browse files
greyson-signalcody-signal
greyson-signal
authored and
cody-signal
committed
Refactor and simplify attachment archiving.
1 parent 816006c commit e80ebd8

23 files changed

Lines changed: 626 additions & 480 deletions

File tree

‎app/src/androidTest/java/org/thoughtcrime/securesms/messages/SyncMessageProcessorTest_synchronizeDeleteForMe.kt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -706,6 +706,7 @@ class SyncMessageProcessorTest_synchronizeDeleteForMe {
706706
archiveMediaName = this.archiveMediaName,
707707
archiveMediaId = this.archiveMediaId,
708708
thumbnailRestoreState = this.thumbnailRestoreState,
709+
archiveTransferState = this.archiveTransferState,
709710
uuid = uuid
710711
)
711712
}

‎app/src/main/java/org/thoughtcrime/securesms/attachments/DatabaseAttachment.kt‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,9 @@ class DatabaseAttachment : Attachment {
3838
@JvmField
3939
val thumbnailRestoreState: AttachmentTable.ThumbnailRestoreState
4040

41+
@JvmField
42+
val archiveTransferState: AttachmentTable.ArchiveTransferState
43+
4144
private val hasArchiveThumbnail: Boolean
4245
private val hasThumbnail: Boolean
4346
val displayOrder: Int
@@ -78,6 +81,7 @@ class DatabaseAttachment : Attachment {
7881
archiveMediaName: String?,
7982
archiveMediaId: String?,
8083
thumbnailRestoreState: AttachmentTable.ThumbnailRestoreState,
84+
archiveTransferState: AttachmentTable.ArchiveTransferState,
8185
uuid: UUID?
8286
) : super(
8387
contentType = contentType,
@@ -116,6 +120,7 @@ class DatabaseAttachment : Attachment {
116120
this.archiveMediaName = archiveMediaName
117121
this.archiveMediaId = archiveMediaId
118122
this.thumbnailRestoreState = thumbnailRestoreState
123+
this.archiveTransferState = archiveTransferState
119124
}
120125

121126
constructor(parcel: Parcel) : super(parcel) {
@@ -130,6 +135,7 @@ class DatabaseAttachment : Attachment {
130135
archiveMediaId = parcel.readString()
131136
hasArchiveThumbnail = ParcelUtil.readBoolean(parcel)
132137
thumbnailRestoreState = AttachmentTable.ThumbnailRestoreState.deserialize(parcel.readInt())
138+
archiveTransferState = AttachmentTable.ArchiveTransferState.deserialize(parcel.readInt())
133139
}
134140

135141
override fun writeToParcel(dest: Parcel, flags: Int) {
@@ -145,6 +151,7 @@ class DatabaseAttachment : Attachment {
145151
dest.writeString(archiveMediaId)
146152
ParcelUtil.writeBoolean(dest, hasArchiveThumbnail)
147153
dest.writeInt(thumbnailRestoreState.value)
154+
dest.writeInt(archiveTransferState.value)
148155
}
149156

150157
override val uri: Uri?

‎app/src/main/java/org/thoughtcrime/securesms/backup/v2/BackupRepository.kt‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -541,7 +541,7 @@ object BackupRepository {
541541
*
542542
* @return True if successful, otherwise false.
543543
*/
544-
fun uploadBackupFile(backupStream: InputStream, backupStreamLength: Long): Boolean {
544+
fun uploadBackupFile(backupStream: InputStream, backupStreamLength: Long): NetworkResult<Unit> {
545545
val backupKey = SignalStore.svr.getOrCreateMasterKey().deriveBackupKey()
546546

547547
return initBackupAndFetchAuth(backupKey)
@@ -559,7 +559,6 @@ object BackupRepository {
559559
SignalNetwork.archive.uploadBackupFile(form, resumableUploadUrl, backupStream, backupStreamLength)
560560
.also { Log.i(TAG, "UploadBackupFileResult: $it") }
561561
}
562-
.also { Log.i(TAG, "OverallResult: $it") } is NetworkResult.Success
563562
}
564563

565564
fun downloadBackupFile(destination: File, listener: ProgressListener? = null): Boolean {

‎app/src/main/java/org/thoughtcrime/securesms/components/settings/app/internal/backup/InternalBackupPlaygroundViewModel.kt‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,11 @@ import org.thoughtcrime.securesms.backup.v2.local.SnapshotFileSystem
3131
import org.thoughtcrime.securesms.database.MessageType
3232
import org.thoughtcrime.securesms.database.SignalDatabase
3333
import org.thoughtcrime.securesms.dependencies.AppDependencies
34-
import org.thoughtcrime.securesms.jobs.ArchiveAttachmentJob
3534
import org.thoughtcrime.securesms.jobs.AttachmentUploadJob
3635
import org.thoughtcrime.securesms.jobs.BackupMessagesJob
3736
import org.thoughtcrime.securesms.jobs.BackupRestoreJob
3837
import org.thoughtcrime.securesms.jobs.BackupRestoreMediaJob
38+
import org.thoughtcrime.securesms.jobs.CopyAttachmentToArchiveJob
3939
import org.thoughtcrime.securesms.jobs.RestoreAttachmentJob
4040
import org.thoughtcrime.securesms.jobs.RestoreAttachmentThumbnailJob
4141
import org.thoughtcrime.securesms.jobs.RestoreLocalAttachmentJob
@@ -175,7 +175,7 @@ class InternalBackupPlaygroundViewModel : ViewModel() {
175175
_state.value = _state.value.copy(uploadState = BackupUploadState.UPLOAD_IN_PROGRESS)
176176

177177
disposables += Single
178-
.fromCallable { BackupRepository.uploadBackupFile(backupData!!.inputStream(), backupData!!.size.toLong()) }
178+
.fromCallable { BackupRepository.uploadBackupFile(backupData!!.inputStream(), backupData!!.size.toLong()) is NetworkResult.Success }
179179
.subscribeOn(Schedulers.io())
180180
.subscribe { success ->
181181
_state.value = _state.value.copy(uploadState = if (success) BackupUploadState.UPLOAD_DONE else BackupUploadState.UPLOAD_FAILED)
@@ -295,7 +295,7 @@ class InternalBackupPlaygroundViewModel : ViewModel() {
295295
AppDependencies
296296
.jobManager
297297
.startChain(AttachmentUploadJob(attachmentId))
298-
.then(ArchiveAttachmentJob(attachmentId))
298+
.then(CopyAttachmentToArchiveJob(attachmentId))
299299
.enqueueAndBlockUntilCompletion(15.seconds.inWholeMilliseconds)
300300
}
301301
.subscribeOn(Schedulers.io())

‎app/src/main/java/org/thoughtcrime/securesms/database/AttachmentTable.kt‎

Lines changed: 35 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,7 @@ class AttachmentTable(
224224
ARCHIVE_TRANSFER_FILE,
225225
THUMBNAIL_FILE,
226226
THUMBNAIL_RESTORE_STATE,
227+
ARCHIVE_TRANSFER_STATE,
227228
ATTACHMENT_UUID
228229
)
229230

@@ -526,30 +527,26 @@ class AttachmentTable(
526527
}
527528

528529
/**
529-
* Finds the next eligible attachment that needs to be uploaded to the archive service.
530-
* If it exists, it'll also atomically be marked as [ArchiveTransferState.BACKFILL_UPLOAD_IN_PROGRESS].
530+
* Finds all of the attachmentIds of attachments that need to be uploaded to the archive cdn.
531531
*/
532-
fun getNextAttachmentToArchiveAndMarkUploadInProgress(): DatabaseAttachment? {
533-
return writableDatabase.withinTransaction {
534-
val record: DatabaseAttachment? = readableDatabase
535-
.select(*PROJECTION)
536-
.from(TABLE_NAME)
537-
.where("$ARCHIVE_TRANSFER_STATE = ? AND $DATA_FILE NOT NULL AND $TRANSFER_STATE = $TRANSFER_PROGRESS_DONE", ArchiveTransferState.NONE.value)
538-
.orderBy("$ID DESC")
539-
.limit(1)
540-
.run()
541-
.readToSingleObject { it.readAttachment() }
542-
543-
if (record != null) {
544-
writableDatabase
545-
.update(TABLE_NAME)
546-
.values(ARCHIVE_TRANSFER_STATE to ArchiveTransferState.BACKFILL_UPLOAD_IN_PROGRESS.value)
547-
.where("$ID = ?", record.attachmentId)
548-
.run()
549-
}
532+
fun getAttachmentsThatNeedArchiveUpload(): List<AttachmentId> {
533+
return readableDatabase
534+
.select(ID)
535+
.from(TABLE_NAME)
536+
.where("$ARCHIVE_TRANSFER_STATE = ? AND $DATA_FILE NOT NULL AND $TRANSFER_STATE = $TRANSFER_PROGRESS_DONE", ArchiveTransferState.NONE.value)
537+
.orderBy("$ID DESC")
538+
.run()
539+
.readToList { AttachmentId(it.requireLong(ID)) }
540+
}
550541

551-
record
552-
}
542+
/**
543+
* Similar to [getAttachmentsThatNeedArchiveUpload], but returns if the list would be non-null in a more efficient way.
544+
*/
545+
fun doAnyAttachmentsNeedArchiveUpload(): Boolean {
546+
return readableDatabase
547+
.exists(TABLE_NAME)
548+
.where("$ARCHIVE_TRANSFER_STATE = ? AND $DATA_FILE NOT NULL AND $TRANSFER_STATE = $TRANSFER_PROGRESS_DONE", ArchiveTransferState.NONE.value)
549+
.run()
553550
}
554551

555552
/**
@@ -584,19 +581,6 @@ class AttachmentTable(
584581
}
585582
}
586583

587-
/**
588-
* Resets any in-progress archive backfill states to [ArchiveTransferState.NONE], returning the number that had to be reset.
589-
* This should only be called if you believe the backfill process has finished. In this case, if this returns a value > 0,
590-
* it indicates that state was mis-tracked and you should try uploading again.
591-
*/
592-
fun resetPendingArchiveBackfills(): Int {
593-
return writableDatabase
594-
.update(TABLE_NAME)
595-
.values(ARCHIVE_TRANSFER_STATE to ArchiveTransferState.NONE.value)
596-
.where("$ARCHIVE_TRANSFER_STATE == ${ArchiveTransferState.BACKFILL_UPLOAD_IN_PROGRESS.value} || $ARCHIVE_TRANSFER_STATE == ${ArchiveTransferState.BACKFILL_UPLOADED.value}")
597-
.run()
598-
}
599-
600584
fun deleteAttachmentsForMessage(mmsId: Long): Boolean {
601585
Log.d(TAG, "[deleteAttachmentsForMessage] mmsId: $mmsId")
602586

@@ -940,9 +924,11 @@ class AttachmentTable(
940924
* When we find out about a new inbound attachment pointer, we insert a row for it that contains all the info we need to download it via [insertAttachmentWithData].
941925
* Later, we download the data for that pointer. Call this method once you have the data to associate it with the attachment. At this point, it is assumed
942926
* that the content of the attachment will never change.
927+
*
928+
* @return True if we had to change the digest as part of saving the file, otherwise false.
943929
*/
944930
@Throws(MmsException::class)
945-
fun finalizeAttachmentAfterDownload(mmsId: Long, attachmentId: AttachmentId, inputStream: LimitedInputStream, iv: ByteArray?) {
931+
fun finalizeAttachmentAfterDownload(mmsId: Long, attachmentId: AttachmentId, inputStream: LimitedInputStream, iv: ByteArray?): Boolean {
946932
Log.i(TAG, "[finalizeAttachmentAfterDownload] Finalizing downloaded data for $attachmentId. (MessageId: $mmsId, $attachmentId)")
947933

948934
val existingPlaceholder: DatabaseAttachment = getAttachment(attachmentId) ?: throw MmsException("No attachment found for id: $attachmentId")
@@ -969,6 +955,8 @@ class AttachmentTable(
969955
cipherOutputStream.transmittedDigest
970956
}
971957

958+
val digestChanged = !digest.contentEquals(existingPlaceholder.remoteDigest)
959+
972960
val foundDuplicate = writableDatabase.withinTransaction { db ->
973961
// We can look and see if we have any exact matches on hash_ends and dedupe the file if we see one.
974962
// We don't look at hash_start here because that could result in us matching on a file that got compressed down to something smaller, effectively lowering
@@ -1048,6 +1036,8 @@ class AttachmentTable(
10481036
if (MediaUtil.isAudio(existingPlaceholder)) {
10491037
GenerateAudioWaveFormJob.enqueue(existingPlaceholder.attachmentId)
10501038
}
1039+
1040+
return digestChanged
10511041
}
10521042

10531043
@Throws(IOException::class)
@@ -1673,6 +1663,7 @@ class AttachmentTable(
16731663
archiveMediaId = jsonObject.getString(ARCHIVE_MEDIA_ID),
16741664
hasArchiveThumbnail = !TextUtils.isEmpty(jsonObject.getString(THUMBNAIL_FILE)),
16751665
thumbnailRestoreState = ThumbnailRestoreState.deserialize(jsonObject.getInt(THUMBNAIL_RESTORE_STATE)),
1666+
archiveTransferState = ArchiveTransferState.deserialize(jsonObject.getInt(ARCHIVE_TRANSFER_STATE)),
16761667
uuid = UuidUtil.parseOrNull(jsonObject.getString(ATTACHMENT_UUID))
16771668
)
16781669
}
@@ -2273,6 +2264,7 @@ class AttachmentTable(
22732264
archiveMediaId = cursor.requireString(ARCHIVE_MEDIA_ID),
22742265
hasArchiveThumbnail = !cursor.isNull(THUMBNAIL_FILE),
22752266
thumbnailRestoreState = ThumbnailRestoreState.deserialize(cursor.requireInt(THUMBNAIL_RESTORE_STATE)),
2267+
archiveTransferState = ArchiveTransferState.deserialize(cursor.requireInt(ARCHIVE_TRANSFER_STATE)),
22762268
uuid = UuidUtil.parseOrNull(cursor.requireString(ATTACHMENT_UUID))
22772269
)
22782270
}
@@ -2513,33 +2505,30 @@ class AttachmentTable(
25132505
*
25142506
* The first is the backfill process, which will happen after newly-enabling backups. That process will go:
25152507
* 1. [NONE]
2516-
* 2. [BACKFILL_UPLOAD_IN_PROGRESS]
2517-
* 3. [BACKFILL_UPLOADED]
2508+
* 2. [UPLOAD_IN_PROGRESS]
2509+
* 3. [COPY_PENDING]
25182510
* 4. [FINISHED] or [PERMANENT_FAILURE]
25192511
*
25202512
* The second is when newly sending/receiving an attachment after enabling backups. That process will go:
25212513
* 1. [NONE]
2522-
* 2. [ATTACHMENT_TRANSFER_PENDING]
2514+
* 2. [COPY_PENDING]
25232515
* 3. [FINISHED] or [PERMANENT_FAILURE]
25242516
*/
25252517
enum class ArchiveTransferState(val value: Int) {
25262518
/** Not backed up at all. */
25272519
NONE(0),
25282520

25292521
/** The upload to the attachment service is in progress. */
2530-
BACKFILL_UPLOAD_IN_PROGRESS(1),
2522+
UPLOAD_IN_PROGRESS(1),
25312523

2532-
/** Successfully uploaded to the attachment service during the backfill process. Still need to tell the service to move the file over to the archive service. */
2533-
BACKFILL_UPLOADED(2),
2524+
/** We sent/received this attachment after enabling backups, but still need to transfer the file to the archive service. */
2525+
COPY_PENDING(2),
25342526

25352527
/** Completely finished backing up the attachment. */
25362528
FINISHED(3),
25372529

25382530
/** It is impossible to upload this attachment. */
2539-
PERMANENT_FAILURE(4),
2540-
2541-
/** We sent/received this attachment after enabling backups, but still need to transfer the file to the archive service. */
2542-
ATTACHMENT_TRANSFER_PENDING(5);
2531+
PERMANENT_FAILURE(4);
25432532

25442533
companion object {
25452534
fun deserialize(value: Int): ArchiveTransferState {

‎app/src/main/java/org/thoughtcrime/securesms/database/MediaTable.kt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ class MediaTable internal constructor(context: Context?, databaseHelper: SignalD
5757
${AttachmentTable.TABLE_NAME}.${AttachmentTable.ARCHIVE_MEDIA_NAME},
5858
${AttachmentTable.TABLE_NAME}.${AttachmentTable.ARCHIVE_MEDIA_ID},
5959
${AttachmentTable.TABLE_NAME}.${AttachmentTable.THUMBNAIL_RESTORE_STATE},
60+
${AttachmentTable.TABLE_NAME}.${AttachmentTable.ARCHIVE_TRANSFER_STATE},
6061
${AttachmentTable.TABLE_NAME}.${AttachmentTable.ATTACHMENT_UUID},
6162
${MessageTable.TABLE_NAME}.${MessageTable.TYPE},
6263
${MessageTable.TABLE_NAME}.${MessageTable.DATE_SENT},

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -391,6 +391,7 @@ open class MessageTable(context: Context?, databaseHelper: SignalDatabase) : Dat
391391
'${AttachmentTable.ARCHIVE_MEDIA_NAME}', ${AttachmentTable.TABLE_NAME}.${AttachmentTable.ARCHIVE_MEDIA_NAME},
392392
'${AttachmentTable.ARCHIVE_MEDIA_ID}', ${AttachmentTable.TABLE_NAME}.${AttachmentTable.ARCHIVE_MEDIA_ID},
393393
'${AttachmentTable.THUMBNAIL_RESTORE_STATE}', ${AttachmentTable.TABLE_NAME}.${AttachmentTable.THUMBNAIL_RESTORE_STATE},
394+
'${AttachmentTable.ARCHIVE_TRANSFER_STATE}', ${AttachmentTable.TABLE_NAME}.${AttachmentTable.ARCHIVE_TRANSFER_STATE},
394395
'${AttachmentTable.ATTACHMENT_UUID}', ${AttachmentTable.TABLE_NAME}.${AttachmentTable.ATTACHMENT_UUID}
395396
)
396397
) AS ${AttachmentTable.ATTACHMENT_JSON_ALIAS}

0 commit comments

Comments
 (0)