From a1bf4d62ab7045d2c1c4c0bb81690b0eb9d6207f Mon Sep 17 00:00:00 2001 From: Cody Henthorne Date: Wed, 18 Sep 2024 10:09:59 -0400 Subject: [PATCH] Fix thumbnail rendering and refreshing on full download. --- .../securesms/database/MediaTable.kt | 17 +---------------- .../securesms/jobs/ArchiveThumbnailUploadJob.kt | 1 - .../securesms/jobs/AttachmentDownloadJob.kt | 9 +++++++-- .../securesms/jobs/RestoreAttachmentJob.kt | 2 +- .../jobs/RestoreAttachmentThumbnailJob.kt | 7 +++++++ .../mediapreview/MediaIntentFactory.kt | 2 +- .../mediapreview/MediaPreviewRepository.kt | 7 +++++-- .../mediapreview/MediaPreviewV2Adapter.kt | 6 ++++++ .../mediapreview/MediaPreviewV2Fragment.kt | 2 +- .../mediapreview/MediaPreviewV2ViewModel.kt | 11 +++++++++++ .../securesms/mms/PartAuthority.java | 4 +++- 11 files changed, 43 insertions(+), 25 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/MediaTable.kt b/app/src/main/java/org/thoughtcrime/securesms/database/MediaTable.kt index e377d1c39a..12448a8588 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/MediaTable.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/MediaTable.kt @@ -11,7 +11,6 @@ import org.thoughtcrime.securesms.attachments.DatabaseAttachment import org.thoughtcrime.securesms.recipients.RecipientId import org.thoughtcrime.securesms.util.MediaUtil import org.thoughtcrime.securesms.util.MediaUtil.SlideType -import org.thoughtcrime.securesms.util.RemoteConfig @SuppressLint("RecipientIdDatabaseReferenceUsage", "ThreadIdDatabaseReferenceUsage") // Not a real table, just a view class MediaTable internal constructor(context: Context?, databaseHelper: SignalDatabase?) : DatabaseTable(context, databaseHelper) { @@ -109,16 +108,6 @@ class MediaTable internal constructor(context: Context?, databaseHelper: SignalD ) private val GALLERY_MEDIA_QUERY_INCLUDING_TEMP_VIDEOS = String.format( - BASE_MEDIA_QUERY, - """ - (${AttachmentTable.DATA_FILE} IS NOT NULL OR (${AttachmentTable.CONTENT_TYPE} LIKE 'video/%' AND ${AttachmentTable.REMOTE_INCREMENTAL_DIGEST} IS NOT NULL)) AND - ${AttachmentTable.CONTENT_TYPE} NOT LIKE 'image/svg%' AND - (${AttachmentTable.CONTENT_TYPE} LIKE 'image/%' OR ${AttachmentTable.CONTENT_TYPE} LIKE 'video/%') AND - ${MessageTable.LINK_PREVIEWS} IS NULL - """ - ) - - private val GALLERY_MEDIA_QUERY_INCLUDING_TEMP_VIDEOS_AND_THUMBNAILS = String.format( BASE_MEDIA_QUERY, """ (${AttachmentTable.DATA_FILE} IS NOT NULL OR (${AttachmentTable.CONTENT_TYPE} LIKE 'video/%' AND ${AttachmentTable.REMOTE_INCREMENTAL_DIGEST} IS NOT NULL) OR (${AttachmentTable.THUMBNAIL_FILE} IS NOT NULL)) AND @@ -167,11 +156,7 @@ class MediaTable internal constructor(context: Context?, databaseHelper: SignalD @JvmOverloads fun getGalleryMediaForThread(threadId: Long, sorting: Sorting, limit: Int = 0): Cursor { - var query = if (RemoteConfig.messageBackups) { - sorting.applyToQuery(applyEqualityOperator(threadId, GALLERY_MEDIA_QUERY_INCLUDING_TEMP_VIDEOS_AND_THUMBNAILS)) - } else { - sorting.applyToQuery(applyEqualityOperator(threadId, GALLERY_MEDIA_QUERY_INCLUDING_TEMP_VIDEOS)) - } + var query = sorting.applyToQuery(applyEqualityOperator(threadId, GALLERY_MEDIA_QUERY_INCLUDING_TEMP_VIDEOS)) val args = arrayOf(threadId.toString() + "") if (limit > 0) { diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/ArchiveThumbnailUploadJob.kt b/app/src/main/java/org/thoughtcrime/securesms/jobs/ArchiveThumbnailUploadJob.kt index bb180b8434..34182981e4 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/ArchiveThumbnailUploadJob.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/ArchiveThumbnailUploadJob.kt @@ -131,7 +131,6 @@ class ArchiveThumbnailUploadJob private constructor( val archiveMediaId = attachment.archiveMediaId ?: backupKey.deriveMediaId(attachment.getMediaName()).encode() SignalDatabase.attachments.finalizeAttachmentThumbnailAfterUpload(attachmentId, archiveMediaId, mediaSecrets.id, thumbnailResult.data) - Log.i(RestoreAttachmentJob.TAG, "Restore: Thumbnail mediaId=${mediaSecrets.id.encode()} backupDir=${backupDirectories.backupDir} mediaDir=${backupDirectories.mediaDir}") Log.d(TAG, "Successfully archived thumbnail for $attachmentId mediaName=${attachment.getThumbnailMediaName()}") Result.success() } diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/AttachmentDownloadJob.kt b/app/src/main/java/org/thoughtcrime/securesms/jobs/AttachmentDownloadJob.kt index cb30352d1e..a83fa4983f 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/AttachmentDownloadJob.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/AttachmentDownloadJob.kt @@ -84,6 +84,8 @@ class AttachmentDownloadJob private constructor( @JvmStatic fun downloadAttachmentIfNeeded(databaseAttachment: DatabaseAttachment): String? { return when (val transferState = databaseAttachment.transferState) { + AttachmentTable.TRANSFER_PROGRESS_DONE -> null + AttachmentTable.TRANSFER_RESTORE_OFFLOADED, AttachmentTable.TRANSFER_NEEDS_RESTORE -> RestoreAttachmentJob.restoreAttachment(databaseAttachment) @@ -109,10 +111,9 @@ class AttachmentDownloadJob private constructor( } AttachmentTable.TRANSFER_RESTORE_IN_PROGRESS, - AttachmentTable.TRANSFER_PROGRESS_DONE, AttachmentTable.TRANSFER_PROGRESS_STARTED, AttachmentTable.TRANSFER_PROGRESS_PERMANENT_FAILURE -> { - Log.d(TAG, "$databaseAttachment is downloading, downloaded already or permanently failed, transferState: $transferState") + Log.d(TAG, "${databaseAttachment.attachmentId} is downloading or permanently failed, transferState: $transferState") null } @@ -215,6 +216,7 @@ class AttachmentDownloadJob private constructor( retrieveAttachmentForReleaseChannel(messageId, attachmentId, attachment) false } + else -> { retrieveAttachment(messageId, attachmentId, attachment) } @@ -225,14 +227,17 @@ class AttachmentDownloadJob private constructor( attachment.archiveTransferState == AttachmentTable.ArchiveTransferState.FINISHED -> { Log.i(TAG, "[$attachmentId] Already archived. Skipping.") } + digestChanged -> { Log.i(TAG, "[$attachmentId] Digest for attachment changed after download. Re-uploading to archive.") AppDependencies.jobManager.add(UploadAttachmentToArchiveJob(attachmentId)) } + attachment.cdn !in CopyAttachmentToArchiveJob.ALLOWED_SOURCE_CDNS -> { Log.i(TAG, "[$attachmentId] Attachment CDN doesn't support copying to archive. Re-uploading to archive.") AppDependencies.jobManager.add(UploadAttachmentToArchiveJob(attachmentId)) } + else -> { Log.i(TAG, "[$attachmentId] Enqueuing job to copy to archive.") AppDependencies.jobManager.add(CopyAttachmentToArchiveJob(attachmentId)) diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/RestoreAttachmentJob.kt b/app/src/main/java/org/thoughtcrime/securesms/jobs/RestoreAttachmentJob.kt index 5cdebe41f4..906b1fef1b 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/RestoreAttachmentJob.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/RestoreAttachmentJob.kt @@ -48,7 +48,7 @@ class RestoreAttachmentJob private constructor( companion object { const val KEY = "RestoreAttachmentJob" - val TAG = Log.tag(RestoreAttachmentJob::class.java) + private val TAG = Log.tag(RestoreAttachmentJob::class.java) @JvmStatic fun constructQueueString(): String { diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/RestoreAttachmentThumbnailJob.kt b/app/src/main/java/org/thoughtcrime/securesms/jobs/RestoreAttachmentThumbnailJob.kt index b4b706fb67..6c20a3909b 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/RestoreAttachmentThumbnailJob.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/RestoreAttachmentThumbnailJob.kt @@ -22,6 +22,7 @@ import org.thoughtcrime.securesms.keyvalue.SignalStore import org.thoughtcrime.securesms.util.RemoteConfig import org.whispersystems.signalservice.api.messages.SignalServiceAttachment import org.whispersystems.signalservice.api.push.exceptions.MissingConfigurationException +import org.whispersystems.signalservice.api.push.exceptions.NonSuccessfulResponseCodeException import java.io.File import java.io.IOException import java.util.concurrent.TimeUnit @@ -142,6 +143,12 @@ class RestoreAttachmentThumbnailJob private constructor( } override fun onShouldRetry(exception: Exception): Boolean { + if (exception is NonSuccessfulResponseCodeException) { + if (exception.code == 404) { + Log.w(TAG, "[$attachmentId-thumbnail] Unable to find file") + return false + } + } return exception is IOException } diff --git a/app/src/main/java/org/thoughtcrime/securesms/mediapreview/MediaIntentFactory.kt b/app/src/main/java/org/thoughtcrime/securesms/mediapreview/MediaIntentFactory.kt index 8be7a3b859..3a945709ab 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/mediapreview/MediaIntentFactory.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/mediapreview/MediaIntentFactory.kt @@ -70,7 +70,7 @@ object MediaIntentFactory { MediaPreviewArgs( threadId = mediaRecord.threadId, date = mediaRecord.date, - initialMediaUri = attachment.uri!!, + initialMediaUri = attachment.displayUri!!, initialMediaType = attachment.contentType, initialMediaSize = attachment.size, initialCaption = attachment.caption, diff --git a/app/src/main/java/org/thoughtcrime/securesms/mediapreview/MediaPreviewRepository.kt b/app/src/main/java/org/thoughtcrime/securesms/mediapreview/MediaPreviewRepository.kt index 092cd33cbc..6ab08c0b8d 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/mediapreview/MediaPreviewRepository.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/mediapreview/MediaPreviewRepository.kt @@ -64,7 +64,10 @@ class MediaPreviewRepository { for (i in 0..limit) { val element = MediaTable.MediaRecord.from(cursor) - if (element.attachment?.transferState == AttachmentTable.TRANSFER_PROGRESS_DONE || element.attachment?.transferState == AttachmentTable.TRANSFER_PROGRESS_STARTED) { + if (element.attachment?.transferState == AttachmentTable.TRANSFER_PROGRESS_DONE || + element.attachment?.transferState == AttachmentTable.TRANSFER_PROGRESS_STARTED || + element.attachment?.thumbnailUri != null + ) { mediaRecords.add(element) if (startingAttachmentId.id == cursor.requireLong(AttachmentTable.ID)) { @@ -87,7 +90,7 @@ class MediaPreviewRepository { .map { it as MmsMessageRecord } .associate { it.id to it.resolveBody(context).getDisplayBody(context) } - Result(itemPosition, mediaRecords.toList(), messages) + Result(if (mediaRecords.isNotEmpty()) itemPosition.coerceIn(mediaRecords.indices) else itemPosition, mediaRecords, messages) } }.subscribeOn(Schedulers.io()).toFlowable() } diff --git a/app/src/main/java/org/thoughtcrime/securesms/mediapreview/MediaPreviewV2Adapter.kt b/app/src/main/java/org/thoughtcrime/securesms/mediapreview/MediaPreviewV2Adapter.kt index 27969cdc23..845c64b808 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/mediapreview/MediaPreviewV2Adapter.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/mediapreview/MediaPreviewV2Adapter.kt @@ -5,6 +5,8 @@ import androidx.fragment.app.Fragment import androidx.viewpager2.adapter.FragmentStateAdapter import org.signal.core.util.logging.Log import org.thoughtcrime.securesms.attachments.Attachment +import org.thoughtcrime.securesms.attachments.DatabaseAttachment +import org.thoughtcrime.securesms.jobs.AttachmentDownloadJob import org.thoughtcrime.securesms.mediasend.Media import org.thoughtcrime.securesms.util.MediaUtil import org.thoughtcrime.securesms.util.adapter.StableIdGenerator @@ -44,6 +46,10 @@ class MediaPreviewV2Adapter(fragment: Fragment) : FragmentStateAdapter(fragment) fragment.arguments = args + if (attachment is DatabaseAttachment) { + AttachmentDownloadJob.downloadAttachmentIfNeeded(attachment) + } + return fragment } diff --git a/app/src/main/java/org/thoughtcrime/securesms/mediapreview/MediaPreviewV2Fragment.kt b/app/src/main/java/org/thoughtcrime/securesms/mediapreview/MediaPreviewV2Fragment.kt index 0cfe531798..81c668b36c 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/mediapreview/MediaPreviewV2Fragment.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/mediapreview/MediaPreviewV2Fragment.kt @@ -151,7 +151,7 @@ class MediaPreviewV2Fragment : LoggingFragment(R.layout.fragment_media_preview_v val startingAttachmentId = PartAuthority.requireAttachmentId(args.initialMediaUri) val threadId = args.threadId viewModel.fetchAttachments(requireContext(), startingAttachmentId, threadId, sorting) - val dbObserver = DatabaseObserver.Observer { viewModel.fetchAttachments(requireContext(), startingAttachmentId, threadId, sorting, true) } + val dbObserver = DatabaseObserver.Observer { viewModel.refetchAttachments(requireContext(), startingAttachmentId, threadId, sorting) } AppDependencies.databaseObserver.registerAttachmentUpdatedObserver(dbObserver) this.dbChangeObserver = dbObserver } diff --git a/app/src/main/java/org/thoughtcrime/securesms/mediapreview/MediaPreviewV2ViewModel.kt b/app/src/main/java/org/thoughtcrime/securesms/mediapreview/MediaPreviewV2ViewModel.kt index 347989efba..04e8c48a7f 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/mediapreview/MediaPreviewV2ViewModel.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/mediapreview/MediaPreviewV2ViewModel.kt @@ -68,6 +68,17 @@ class MediaPreviewV2ViewModel : ViewModel() { } } + fun refetchAttachments(context: Context, startingAttachmentId: AttachmentId, threadId: Long, sorting: MediaTable.Sorting) { + val state = store.state + val currentAttachmentId = if (state.position in state.mediaRecords.indices) { + state.mediaRecords[state.position].attachment?.attachmentId + } else { + null + } + + fetchAttachments(context, currentAttachmentId ?: startingAttachmentId, threadId, sorting, true) + } + fun initialize(showThread: Boolean, allMediaInAlbumRail: Boolean, leftIsRecent: Boolean) { if (store.state.loadState == MediaPreviewV2State.LoadState.INIT) { store.update { oldState -> diff --git a/app/src/main/java/org/thoughtcrime/securesms/mms/PartAuthority.java b/app/src/main/java/org/thoughtcrime/securesms/mms/PartAuthority.java index db14123ecf..70ac37e718 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/mms/PartAuthority.java +++ b/app/src/main/java/org/thoughtcrime/securesms/mms/PartAuthority.java @@ -218,6 +218,7 @@ public class PartAuthority { int match = uriMatcher.match(uri); switch (match) { case PART_ROW: + case THUMBNAIL_ROW: case PERSISTENT_ROW: case BLOB_ROW: return true; @@ -226,7 +227,8 @@ public class PartAuthority { } public static boolean isAttachmentUri(@NonNull Uri uri) { - return uriMatcher.match(uri) == PART_ROW; + int match = uriMatcher.match(uri); + return match == PART_ROW || match == THUMBNAIL_ROW; } public static @NonNull AttachmentId requireAttachmentId(@NonNull Uri uri) {