From f8b2f73f7b1e8fad8c64f16d96a5c334a10ae4cb Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Thu, 4 Aug 2022 13:25:46 +1000 Subject: [PATCH] Fixed a few issues found during QA Fixed an issue where quotes containing images wouldn't send Fixed an issue where a MessageSend job could get stuck in an infinite retry loop if it had an attachment in an invalid state Fixed an issue where quotes containing non-media files wouldn't contain the correct data Fixed an issue where the quote thumbnail was getting the wrong content mode set Fixed an issue where the local disappearing messages config wasn't getting generated correctly Fixed an issue where the format parameters for the disappearing message info message were the wrong way around in one case Updated the AttachmentUploadJob to try to support images which haven't completed downloading (untested as it's not supported via the UI) --- .../Content Views/QuoteView.swift | 39 ++++++++++--------- .../Database/Models/Attachment.swift | 22 ++++++++++- .../DisappearingMessageConfiguration.swift | 14 +++---- .../Database/Models/Quote.swift | 4 +- .../Jobs/Types/AttachmentUploadJob.swift | 12 ++++-- .../Jobs/Types/MessageSendJob.swift | 12 +++++- .../Quotes/QuotedReplyModel.swift | 2 +- 7 files changed, 71 insertions(+), 34 deletions(-) diff --git a/Session/Conversations/Message Cells/Content Views/QuoteView.swift b/Session/Conversations/Message Cells/Content Views/QuoteView.swift index 2a47a91d9..03aea60b5 100644 --- a/Session/Conversations/Message Cells/Content Views/QuoteView.swift +++ b/Session/Conversations/Message Cells/Content Views/QuoteView.swift @@ -135,25 +135,8 @@ final class QuoteView: UIView { let fallbackImageName: String = (isAudio ? "attachment_audio" : "actionsheet_document_black") let imageView: UIImageView = UIImageView( image: UIImage(named: fallbackImageName)? + .resizedImage(to: CGSize(width: iconSize, height: iconSize))? .withRenderingMode(.alwaysTemplate) - .resizedImage(to: CGSize(width: iconSize, height: iconSize)) - ) - - attachment.thumbnail( - size: .small, - success: { image, _ in - guard Thread.isMainThread else { - DispatchQueue.main.async { - imageView.image = image - imageView.contentMode = .scaleAspectFill - } - return - } - - imageView.image = image - imageView.contentMode = .scaleAspectFill - }, - failure: {} ) imageView.tintColor = .white @@ -171,6 +154,26 @@ final class QuoteView: UIView { (isAudio ? "Audio" : "Document") ) } + + // Generate the thumbnail if needed + if attachment.isVisualMedia { + attachment.thumbnail( + size: .small, + success: { image, _ in + guard Thread.isMainThread else { + DispatchQueue.main.async { + imageView.image = image + imageView.contentMode = .scaleAspectFill + } + return + } + + imageView.image = image + imageView.contentMode = .scaleAspectFill + }, + failure: {} + ) + } } else { mainStackView.addArrangedSubview(lineView) diff --git a/SessionMessagingKit/Database/Models/Attachment.swift b/SessionMessagingKit/Database/Models/Attachment.swift index e0ceef673..a62849bd7 100644 --- a/SessionMessagingKit/Database/Models/Attachment.swift +++ b/SessionMessagingKit/Database/Models/Attachment.swift @@ -717,6 +717,8 @@ extension Attachment { // MARK: - Convenience extension Attachment { + public static let nonMediaQuoteFileId: String = "NON_MEDIA_QUOTE_FILE_ID" + public enum ThumbnailSize { case small case medium @@ -869,7 +871,7 @@ extension Attachment { return existingImage } - public func cloneAsThumbnail() -> Attachment? { + public func cloneAsQuoteThumbnail() -> Attachment? { let cloneId: String = UUID().uuidString let thumbnailName: String = "quoted-thumbnail-\(sourceFilename ?? "null")" @@ -881,7 +883,22 @@ extension Attachment { mimeType: OWSMimeTypeImageJpeg, sourceFilename: thumbnailName ) - else { return nil } + else { + // Non-media files cannot have thumbnails but may be sent as quotes, in these cases we want + // to create an attachment in an 'uploaded' state with a hard-coded file id so the messageSend + // job doesn't try to upload the attachment (we include the original `serverId` as it's + // required for generating the protobuf) + return Attachment( + id: cloneId, + serverId: self.serverId, + variant: self.variant, + state: .uploaded, + contentType: self.contentType, + byteCount: 0, + downloadUrl: Attachment.nonMediaQuoteFileId, + isValid: self.isValid + ) + } // Try generate the thumbnail var thumbnailData: Data? @@ -922,6 +939,7 @@ extension Attachment { return Attachment( id: cloneId, variant: .standard, + state: .downloaded, contentType: OWSMimeTypeImageJpeg, byteCount: UInt(thumbnailData.count), sourceFilename: thumbnailName, diff --git a/SessionMessagingKit/Database/Models/DisappearingMessageConfiguration.swift b/SessionMessagingKit/Database/Models/DisappearingMessageConfiguration.swift index 08937d1e4..6d4b1cfbc 100644 --- a/SessionMessagingKit/Database/Models/DisappearingMessageConfiguration.swift +++ b/SessionMessagingKit/Database/Models/DisappearingMessageConfiguration.swift @@ -79,8 +79,8 @@ public extension DisappearingMessagesConfiguration { return String( format: "OTHER_UPDATED_DISAPPEARING_MESSAGES_CONFIGURATION".localized(), - NSString.formatDurationSeconds(UInt32(floor(durationSeconds)), useShortFormat: false), - senderName + senderName, + NSString.formatDurationSeconds(UInt32(floor(durationSeconds)), useShortFormat: false) ) } } @@ -192,14 +192,14 @@ public class SMKDisappearingMessagesConfiguration: NSObject { return } - let config: DisappearingMessagesConfiguration = (try DisappearingMessagesConfiguration - .fetchOne(db, id: threadId)? + let config: DisappearingMessagesConfiguration = try DisappearingMessagesConfiguration + .fetchOne(db, id: threadId) + .defaulting(to: DisappearingMessagesConfiguration.defaultWith(threadId)) .with( isEnabled: isEnabled, durationSeconds: durationSeconds ) - .saved(db)) - .defaulting(to: DisappearingMessagesConfiguration.defaultWith(threadId)) + .saved(db) let interaction: Interaction = try Interaction( threadId: threadId, @@ -214,7 +214,7 @@ public class SMKDisappearingMessagesConfiguration: NSObject { db, message: ExpirationTimerUpdate( syncTarget: nil, - duration: UInt32(floor(durationSeconds)) + duration: UInt32(floor(isEnabled ? durationSeconds : 0)) ), interactionId: interaction.id, in: thread diff --git a/SessionMessagingKit/Database/Models/Quote.swift b/SessionMessagingKit/Database/Models/Quote.swift index 9ac83853c..633676aa6 100644 --- a/SessionMessagingKit/Database/Models/Quote.swift +++ b/SessionMessagingKit/Database/Models/Quote.swift @@ -113,7 +113,7 @@ public extension Quote { .map { quotedInteraction -> Attachment? in // If the quotedInteraction has an attachment then try clone it if let attachment: Attachment = try? quotedInteraction.attachments.fetchOne(db) { - return attachment.cloneAsThumbnail() + return attachment.cloneAsQuoteThumbnail() } // Otherwise if the quotedInteraction has a link preview, try clone that @@ -121,7 +121,7 @@ public extension Quote { .fetchOne(db)? .attachment .fetchOne(db)? - .cloneAsThumbnail() + .cloneAsQuoteThumbnail() } .defaulting(to: Attachment(proto: attachment)) .inserted(db) diff --git a/SessionMessagingKit/Jobs/Types/AttachmentUploadJob.swift b/SessionMessagingKit/Jobs/Types/AttachmentUploadJob.swift index ae538be47..4692d48b1 100644 --- a/SessionMessagingKit/Jobs/Types/AttachmentUploadJob.swift +++ b/SessionMessagingKit/Jobs/Types/AttachmentUploadJob.swift @@ -43,9 +43,15 @@ public enum AttachmentUploadJob: JobExecutor { } } - // Note: In the AttachmentUploadJob we intentionally don't provide our own db instance to prevent reentrancy - // issues when the success/failure closures get called before the upload as the JobRunner will attempt to - // update the state of the job immediately + // If the attachment is still pending download the hold off on running this job + guard attachment.state != .pendingDownload && attachment.state != .downloading else { + deferred(job) + return + } + + // Note: In the AttachmentUploadJob we intentionally don't provide our own db instance to prevent + // reentrancy issues when the success/failure closures get called before the upload as the JobRunner + // will attempt to update the state of the job immediately attachment.upload( queue: queue, using: { db, data in diff --git a/SessionMessagingKit/Jobs/Types/MessageSendJob.swift b/SessionMessagingKit/Jobs/Types/MessageSendJob.swift index f7bbceb62..8e9aa3732 100644 --- a/SessionMessagingKit/Jobs/Types/MessageSendJob.swift +++ b/SessionMessagingKit/Jobs/Types/MessageSendJob.swift @@ -75,7 +75,17 @@ public enum MessageSendJob: JobExecutor { // but not on the message recipients device - both LinkPreview and Quote can // have this case) try allAttachmentStateInfo - .filter { $0.state == .uploading || $0.state == .failedUpload || $0.state == .downloaded } + .filter { attachment -> Bool in + // Non-media quotes won't have thumbnails so so don't try to upload them + guard attachment.downloadUrl != Attachment.nonMediaQuoteFileId else { return false } + + switch attachment.state { + case .uploading, .pendingDownload, .downloading, .failedUpload, .downloaded: + return true + + default: return false + } + } .filter { stateInfo in // Don't add a new job if there is one already in the queue !JobRunner.hasPendingOrRunningJob( diff --git a/SessionMessagingKit/Sending & Receiving/Quotes/QuotedReplyModel.swift b/SessionMessagingKit/Sending & Receiving/Quotes/QuotedReplyModel.swift index abde118f3..9e688faaa 100644 --- a/SessionMessagingKit/Sending & Receiving/Quotes/QuotedReplyModel.swift +++ b/SessionMessagingKit/Sending & Receiving/Quotes/QuotedReplyModel.swift @@ -69,7 +69,7 @@ public extension QuotedReplyModel { guard let sourceAttachment: Attachment = self.attachment else { return nil } return try sourceAttachment - .cloneAsThumbnail()? + .cloneAsQuoteThumbnail()? .inserted(db) .id }