From addc859c84d4f37beeb8b06e032a1e95d3a19553 Mon Sep 17 00:00:00 2001 From: nielsandriesse Date: Fri, 27 Nov 2020 15:13:42 +1100 Subject: [PATCH] Fix duplicate messages & debug --- .../Signal/MediaGalleryViewController.swift | 1 - .../Database/Storage+Messaging.swift | 21 +++++++++++++++++++ SessionMessagingKit/Jobs/MessageSendJob.swift | 1 - .../VisibleMessage+Quote.swift | 4 ++-- .../Visible Messages/VisibleMessage.swift | 8 +++++++ .../MessageReceiver+Handling.swift | 21 +++++++++++++------ .../Sending & Receiving/MessageReceiver.swift | 7 ++++++- SessionMessagingKit/Storage.swift | 2 ++ .../MessageSender+Handling.swift | 6 ++---- 9 files changed, 56 insertions(+), 15 deletions(-) diff --git a/Session/Signal/MediaGalleryViewController.swift b/Session/Signal/MediaGalleryViewController.swift index ccc9c253e..a8153b0c9 100644 --- a/Session/Signal/MediaGalleryViewController.swift +++ b/Session/Signal/MediaGalleryViewController.swift @@ -377,7 +377,6 @@ class MediaGallery: NSObject, MediaGalleryDataSource, MediaTileViewControllerDel } guard let initialDetailItem = galleryItem else { - owsFailDebug("unexpectedly failed to build initialDetailItem.") return } diff --git a/SessionMessagingKit/Database/Storage+Messaging.swift b/SessionMessagingKit/Database/Storage+Messaging.swift index 96a265d7d..7afb97b55 100644 --- a/SessionMessagingKit/Database/Storage+Messaging.swift +++ b/SessionMessagingKit/Database/Storage+Messaging.swift @@ -61,5 +61,26 @@ extension Storage { guard let tsIncomingMessage = TSIncomingMessage.fetch(uniqueId: tsIncomingMessageID, transaction: transaction) else { return } tsIncomingMessage.touch(with: transaction) } + + private static let receivedMessageTimestampsCollection = "ReceivedMessageTimestampsCollection" + + public func getReceivedMessageTimestamps(using transaction: Any) -> [UInt64] { + var result: [UInt64] = [] + let transaction = transaction as! YapDatabaseReadWriteTransaction + transaction.enumerateRows(inCollection: Storage.receivedMessageTimestampsCollection) { _, object, _, _ in + guard let timestamp = object as? UInt64 else { return } + result.append(timestamp) + } + return result + } + + public func addReceivedMessageTimestamp(_ timestamp: UInt64, using transaction: Any) { + var receivedMessageTimestamps = getReceivedMessageTimestamps(using: transaction) + // TODO: Do we need to sort the timestamps here? + if receivedMessageTimestamps.count > 1000 { receivedMessageTimestamps.remove(at: 0) } // Limit the size of the collection to 1000 + receivedMessageTimestamps.append(timestamp) + let transaction = transaction as! YapDatabaseReadWriteTransaction + transaction.setObject(receivedMessageTimestamps, forKey: "receivedMessageTimestamps", inCollection: Storage.receivedMessageTimestampsCollection) + } } diff --git a/SessionMessagingKit/Jobs/MessageSendJob.swift b/SessionMessagingKit/Jobs/MessageSendJob.swift index 73255910a..504203638 100644 --- a/SessionMessagingKit/Jobs/MessageSendJob.swift +++ b/SessionMessagingKit/Jobs/MessageSendJob.swift @@ -78,7 +78,6 @@ public final class MessageSendJob : NSObject, Job, NSCoding { // NSObject/NSCodi } if !attachmentsToUpload.isEmpty { return } // Wait for all attachments to upload before continuing } - // FIXME: This doesn't yet handle the attachment side of link previews, quotes, etc. storage.withAsync({ transaction in // Intentionally capture self MessageSender.send(self.message, to: self.destination, using: transaction).done(on: DispatchQueue.global(qos: .userInitiated)) { self.handleSuccess() diff --git a/SessionMessagingKit/Messages/Visible Messages/VisibleMessage+Quote.swift b/SessionMessagingKit/Messages/Visible Messages/VisibleMessage+Quote.swift index 43f51c63b..d19893194 100644 --- a/SessionMessagingKit/Messages/Visible Messages/VisibleMessage+Quote.swift +++ b/SessionMessagingKit/Messages/Visible Messages/VisibleMessage+Quote.swift @@ -46,12 +46,12 @@ public extension VisibleMessage { } public func toProto(using transaction: YapDatabaseReadWriteTransaction) -> SNProtoDataMessageQuote? { - guard let timestamp = timestamp, let publicKey = publicKey, let text = text else { + guard let timestamp = timestamp, let publicKey = publicKey else { SNLog("Couldn't construct quote proto from: \(self).") return nil } let quoteProto = SNProtoDataMessageQuote.builder(id: timestamp, author: publicKey) - quoteProto.setText(text) + if let text = text { quoteProto.setText(text) } addAttachmentsIfNeeded(to: quoteProto, using: transaction) do { return try quoteProto.build() diff --git a/SessionMessagingKit/Messages/Visible Messages/VisibleMessage.swift b/SessionMessagingKit/Messages/Visible Messages/VisibleMessage.swift index 3c85f1da1..88f7b7825 100644 --- a/SessionMessagingKit/Messages/Visible Messages/VisibleMessage.swift +++ b/SessionMessagingKit/Messages/Visible Messages/VisibleMessage.swift @@ -25,12 +25,20 @@ public final class VisibleMessage : Message { super.init(coder: coder) if let text = coder.decodeObject(forKey: "body") as! String? { self.text = text } if let attachmentIDs = coder.decodeObject(forKey: "attachments") as! [String]? { self.attachmentIDs = attachmentIDs } + if let quote = coder.decodeObject(forKey: "quote") as! Quote? { self.quote = quote } + if let linkPreview = coder.decodeObject(forKey: "linkPreview") as! LinkPreview? { self.linkPreview = linkPreview } + // TODO: Contact + if let profile = coder.decodeObject(forKey: "profile") as! Profile? { self.profile = profile } } public override func encode(with coder: NSCoder) { super.encode(with: coder) coder.encode(text, forKey: "body") coder.encode(attachmentIDs, forKey: "attachments") + coder.encode(quote, forKey: "quote") + coder.encode(linkPreview, forKey: "linkPreview") + // TODO: Contact + coder.encode(profile, forKey: "profile") } // MARK: Proto Conversion diff --git a/SessionMessagingKit/Sending & Receiving/MessageReceiver+Handling.swift b/SessionMessagingKit/Sending & Receiving/MessageReceiver+Handling.swift index 5bfa544a3..b5631a540 100644 --- a/SessionMessagingKit/Sending & Receiving/MessageReceiver+Handling.swift +++ b/SessionMessagingKit/Sending & Receiving/MessageReceiver+Handling.swift @@ -160,14 +160,23 @@ extension MessageReceiver { // Persist the message guard let (threadID, tsIncomingMessageID) = storage.persist(message, groupPublicKey: message.groupPublicKey, using: transaction) else { throw Error.noThread } message.threadID = threadID + // Handle quoted attachment if needed + if message.quote != nil && proto.dataMessage?.quote != nil, let thread = TSThread.fetch(uniqueId: threadID, transaction: transaction) { + let tsQuote = TSQuotedMessage(for: proto.dataMessage!, thread: thread, transaction: transaction) + if let thumbnailID = tsQuote?.thumbnailAttachmentStreamId() ?? tsQuote?.thumbnailAttachmentPointerId() { + message.quote?.attachmentID = thumbnailID + } + } // Start attachment downloads if needed storage.withAsync({ transaction in - attachmentIDs.forEach { attachmentID in - let downloadJob = AttachmentDownloadJob(attachmentID: attachmentID, tsIncomingMessageID: tsIncomingMessageID) - if CurrentAppContext().isMainAppAndActive { - JobQueue.shared.add(downloadJob, using: transaction) - } else { - JobQueue.shared.addWithoutExecuting(downloadJob, using: transaction) + DispatchQueue.main.async { + attachmentIDs.forEach { attachmentID in + let downloadJob = AttachmentDownloadJob(attachmentID: attachmentID, tsIncomingMessageID: tsIncomingMessageID) + if CurrentAppContext().isMainAppAndActive { // This has to be called from the main thread + JobQueue.shared.add(downloadJob, using: transaction) + } else { + JobQueue.shared.addWithoutExecuting(downloadJob, using: transaction) + } } } }, completion: { }) diff --git a/SessionMessagingKit/Sending & Receiving/MessageReceiver.swift b/SessionMessagingKit/Sending & Receiving/MessageReceiver.swift index 9d918ef45..a42a60bae 100644 --- a/SessionMessagingKit/Sending & Receiving/MessageReceiver.swift +++ b/SessionMessagingKit/Sending & Receiving/MessageReceiver.swift @@ -3,6 +3,7 @@ import SessionUtilitiesKit internal enum MessageReceiver { internal enum Error : LocalizedError { + case duplicateMessage case invalidMessage case unknownMessage case unknownEnvelopeType @@ -18,13 +19,14 @@ internal enum MessageReceiver { internal var isRetryable: Bool { switch self { - case .invalidMessage, .unknownMessage, .unknownEnvelopeType, .noData, .senderBlocked, .selfSend: return false + case .duplicateMessage, .invalidMessage, .unknownMessage, .unknownEnvelopeType, .noData, .senderBlocked, .selfSend: return false default: return true } } internal var errorDescription: String? { switch self { + case .duplicateMessage: return "Duplicate message." case .invalidMessage: return "Invalid message." case .unknownMessage: return "Unknown message type." case .unknownEnvelopeType: return "Unknown envelope type." @@ -45,6 +47,9 @@ internal enum MessageReceiver { let userPublicKey = Configuration.shared.storage.getUserPublicKey() // Parse the envelope let envelope = try SNProtoEnvelope.parseData(data) + let storage = Configuration.shared.storage + guard !Set(storage.getReceivedMessageTimestamps(using: transaction)).contains(envelope.timestamp) else { throw Error.duplicateMessage } + storage.addReceivedMessageTimestamp(envelope.timestamp, using: transaction) // Decrypt the contents let plaintext: Data let sender: String diff --git a/SessionMessagingKit/Storage.swift b/SessionMessagingKit/Storage.swift index 0f752fe00..5b2974483 100644 --- a/SessionMessagingKit/Storage.swift +++ b/SessionMessagingKit/Storage.swift @@ -71,6 +71,8 @@ public protocol SessionMessagingKitStorageProtocol { // MARK: - Message Handling + func getReceivedMessageTimestamps(using transaction: Any) -> [UInt64] + func addReceivedMessageTimestamp(_ timestamp: UInt64, using transaction: Any) /// Returns the ID of the thread the message was stored under along with the ID of the `TSIncomingMessage` that was constructed. func persist(_ message: VisibleMessage, groupPublicKey: String?, using transaction: Any) -> (String, String)? /// Returns the IDs of the saved attachments. diff --git a/SignalUtilitiesKit/Messaging/Sending & Receiving/MessageSender+Handling.swift b/SignalUtilitiesKit/Messaging/Sending & Receiving/MessageSender+Handling.swift index 9bb8752c8..f67ed9e3a 100644 --- a/SignalUtilitiesKit/Messaging/Sending & Receiving/MessageSender+Handling.swift +++ b/SignalUtilitiesKit/Messaging/Sending & Receiving/MessageSender+Handling.swift @@ -20,16 +20,14 @@ extension MessageSender : SharedSenderKeysDelegate { streams.append(stream) stream.write($0.dataSource) stream.save(with: transaction) - tsMessage.attachmentIds.add(stream.uniqueId!) - } - if let quotedMessageThumbnails = tsMessage.quotedMessage?.createThumbnailAttachmentsIfNecessary(with: transaction) { - streams += quotedMessageThumbnails } + tsMessage.quotedMessage?.createThumbnailAttachmentsIfNecessary(with: transaction) if let linkPreviewAttachmentID = tsMessage.linkPreview?.imageAttachmentId, let stream = TSAttachment.fetch(uniqueId: linkPreviewAttachmentID, transaction: transaction) as? TSAttachmentStream { streams.append(stream) } message.attachmentIDs = streams.map { $0.uniqueId! } + tsMessage.attachmentIds.addObjects(from: message.attachmentIDs) tsMessage.save(with: transaction) }