From 68284315a6f0c314370c6a69b27e362fcdb86fb0 Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Thu, 28 Nov 2024 11:40:16 +1100 Subject: [PATCH] Fixed some issues with message deletion and the input field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit • Fixed an issue where the input view wouldn't reappear when deleting messages for everyone in a one-to-one conversation • Fixed an issue where the input view would be visible while the loading indicator was visible when deleting from a community • Fixed an issue where notifications weren't being removed after processing an UnsendRequest • Fixed an issue where legacy groups had a "Clear for everyone" option but it didn't do anything • Updated the copy for legacy groups to say "Delete for everyone" instead of "Clear for everyone" --- .../ConversationVC+Interaction.swift | 35 +++++--- .../Database/Models/Interaction.swift | 79 +++++++++++++------ .../MessageReceiver+UnsendRequests.swift | 56 ++++++++++--- .../Sending & Receiving/MessageReceiver.swift | 3 +- 4 files changed, 123 insertions(+), 50 deletions(-) diff --git a/Session/Conversations/ConversationVC+Interaction.swift b/Session/Conversations/ConversationVC+Interaction.swift index ba1669fcb..a6e02db32 100644 --- a/Session/Conversations/ConversationVC+Interaction.swift +++ b/Session/Conversations/ConversationVC+Interaction.swift @@ -2109,6 +2109,7 @@ extension ConversationVC: } // Delete the message from the open group + self.hideInputAccessoryView() deleteRemotely( from: self, request: Storage.shared @@ -2133,13 +2134,14 @@ extension ConversationVC: userPublicKey : cellViewModel.threadId ) - let serverHash: String? = Storage.shared.read { db -> String? in - try Interaction - .select(.serverHash) - .filter(id: cellViewModel.id) - .asRequest(of: String.self) - .fetchOne(db) - } + let serverHashes: Set = Storage.shared + .read { db -> Set in + try Interaction.serverHashesForDeletion( + db, + interactionIds: [cellViewModel.id] + ) + } + .defaulting(to: []) let unsendRequest: UnsendRequest = UnsendRequest( timestamp: UInt64(cellViewModel.timestampMs), author: (cellViewModel.variant == .standardOutgoing ? @@ -2153,7 +2155,7 @@ extension ConversationVC: ) // For incoming interactions or interactions with no serverHash just delete them locally - guard cellViewModel.variant == .standardOutgoing, let serverHash: String = serverHash else { + guard cellViewModel.variant == .standardOutgoing, !serverHashes.isEmpty else { Storage.shared.writeAsync { db in _ = try Interaction .filter(id: cellViewModel.id) @@ -2161,7 +2163,7 @@ extension ConversationVC: // No need to send the unsendRequest if there is no serverHash (ie. the message // was outgoing but never got to the server) - guard serverHash != nil else { return } + guard !serverHashes.isEmpty else { return } MessageSender .send( @@ -2209,7 +2211,6 @@ extension ConversationVC: actionSheet.addAction(UIAlertAction( title: { switch (cellViewModel.threadVariant, cellViewModel.threadId) { - case (.legacyGroup, _), (.group, _): return "clearMessagesForEveryone".localized() case (_, userPublicKey): return "deleteMessageDevicesAll".localized() default: return "deleteMessageEveryone".localized() } @@ -2219,6 +2220,10 @@ extension ConversationVC: ) { [weak self] _ in let completeServerDeletion = { Storage.shared.writeAsync { db in + _ = try Interaction + .filter(id: cellViewModel.id) + .deleteAll(db) + try MessageSender .send( db, @@ -2228,7 +2233,15 @@ extension ConversationVC: threadVariant: cellViewModel.threadVariant, using: dependencies ) + + /// We should also remove the `SnodeReceivedMessageInfo` entries for the hashes (otherwise we + /// might try to poll for a hash which no longer exists, resulting in fetching the last 14 days of messages) + try SnodeReceivedMessageInfo.handlePotentialDeletedOrInvalidHash( + db, + potentiallyInvalidHashes: Array(serverHashes) + ) } + self?.showInputAccessoryView() } // We can only delete messages on the server for `contact` and `group` conversations @@ -2241,7 +2254,7 @@ extension ConversationVC: request: SnodeAPI .deleteMessages( swarmPublicKey: targetPublicKey, - serverHashes: [serverHash] + serverHashes: Array(serverHashes) ) .map { _ in () } .eraseToAnyPublisher() diff --git a/SessionMessagingKit/Database/Models/Interaction.swift b/SessionMessagingKit/Database/Models/Interaction.swift index e87ce2b5c..20fe4c9e8 100644 --- a/SessionMessagingKit/Database/Models/Interaction.swift +++ b/SessionMessagingKit/Database/Models/Interaction.swift @@ -799,32 +799,6 @@ public extension Interaction { return "\(threadId)-\(id)" } - func markingAsDeleted() -> Interaction { - return Interaction( - id: id, - serverHash: nil, - messageUuid: messageUuid, - threadId: threadId, - authorId: authorId, - variant: .standardIncomingDeleted, - body: nil, - timestampMs: timestampMs, - receivedAtTimestampMs: receivedAtTimestampMs, - wasRead: true, // Never consider deleted messages unread - hasMention: false, - expiresInSeconds: expiresInSeconds, - expiresStartedAtMs: expiresStartedAtMs, - linkPreviewUrl: nil, - openGroupServerMessageId: openGroupServerMessageId, - openGroupWhisper: openGroupWhisper, - openGroupWhisperMods: openGroupWhisperMods, - openGroupWhisperTo: openGroupWhisperTo, - state: .deleted, - recipientReadTimestampMs: nil, - mostRecentFailureText: nil - ) - } - static func isUserMentioned( _ db: Database, threadId: String, @@ -1146,3 +1120,56 @@ public extension Interaction.State { } } } + +// MARK: - Deletion + +public extension Interaction { + /// When deleting a message we should also delete any reactions which were on the message, so fetch and + /// return those hashes as well + static func serverHashesForDeletion( + _ db: Database, + interactionIds: Set, + additionalServerHashesToRemove: [String] = [] + ) throws -> Set { + let messageHashes: [String] = try Interaction + .filter(ids: interactionIds) + .filter(Interaction.Columns.serverHash != nil) + .select(.serverHash) + .asRequest(of: String.self) + .fetchAll(db) + let reactionHashes: [String] = try Reaction + .filter(interactionIds.contains(Reaction.Columns.interactionId)) + .filter(Reaction.Columns.serverHash != nil) + .select(.serverHash) + .asRequest(of: String.self) + .fetchAll(db) + + return Set(messageHashes + reactionHashes + additionalServerHashesToRemove) + } + + func markingAsDeleted() -> Interaction { + return Interaction( + id: id, + serverHash: nil, + messageUuid: messageUuid, + threadId: threadId, + authorId: authorId, + variant: .standardIncomingDeleted, + body: nil, + timestampMs: timestampMs, + receivedAtTimestampMs: receivedAtTimestampMs, + wasRead: true, // Never consider deleted messages unread + hasMention: false, + expiresInSeconds: expiresInSeconds, + expiresStartedAtMs: expiresStartedAtMs, + linkPreviewUrl: nil, + openGroupServerMessageId: openGroupServerMessageId, + openGroupWhisper: openGroupWhisper, + openGroupWhisperMods: openGroupWhisperMods, + openGroupWhisperTo: openGroupWhisperTo, + state: .deleted, + recipientReadTimestampMs: nil, + mostRecentFailureText: nil + ) + } +} diff --git a/SessionMessagingKit/Sending & Receiving/Message Handling/MessageReceiver+UnsendRequests.swift b/SessionMessagingKit/Sending & Receiving/Message Handling/MessageReceiver+UnsendRequests.swift index 8f7d7065c..e9a010384 100644 --- a/SessionMessagingKit/Sending & Receiving/Message Handling/MessageReceiver+UnsendRequests.swift +++ b/SessionMessagingKit/Sending & Receiving/Message Handling/MessageReceiver+UnsendRequests.swift @@ -10,7 +10,8 @@ extension MessageReceiver { _ db: Database, threadId: String, threadVariant: SessionThread.Variant, - message: UnsendRequest + message: UnsendRequest, + using dependencies: Dependencies ) throws { let userPublicKey: String = getUserHexEncodedPublicKey(db) @@ -27,7 +28,7 @@ extension MessageReceiver { let interaction: Interaction = maybeInteraction else { return } - // Mark incoming messages as read and remove any of their notifications + /// Mark incoming messages as read and remove any of their notifications if interaction.variant == .standardIncoming { try Interaction.markAsRead( db, @@ -42,16 +43,12 @@ extension MessageReceiver { UNUserNotificationCenter.current().removePendingNotificationRequests(withIdentifiers: interaction.notificationIdentifiers) } - if author == message.sender, let serverHash: String = interaction.serverHash { - SnodeAPI - .deleteMessages( - swarmPublicKey: author, - serverHashes: [serverHash] - ) - .subscribe(on: DispatchQueue.global(qos: .background)) - .sinkUntilComplete() - } - + /// Retrieve the hashes which should be deleted first (these will be removed by marking the message as deleted) + let hashes: Set = try Interaction.serverHashesForDeletion( + db, + interactionIds: [interactionId] + ) + switch (interaction.variant, (author == message.sender)) { case (.standardOutgoing, _), (_, false): _ = try interaction.delete(db) @@ -71,5 +68,40 @@ extension MessageReceiver { ) } } + + /// Can't delete from the legacy group swarm so only bother for contact conversations + switch threadVariant { + case .legacyGroup, .group, .community: break + case .contact: + dependencies.storage + .readPublisher { db in + try SnodeAPI.preparedDeleteMessages( + db, + swarmPublicKey: userPublicKey, + serverHashes: Array(hashes), + requireSuccessfulDeletion: false, + using: dependencies + ) + } + .flatMap { $0.send(using: dependencies) } + .subscribe(on: DispatchQueue.global(qos: .background), using: dependencies) + .sinkUntilComplete( + receiveCompletion: { result in + switch result { + case .failure: break + case .finished: + /// Since the server deletion was successful we should also remove the `SnodeReceivedMessageInfo` + /// entries for the hashes (otherwise we might try to poll for a hash which no longer exists, resulting in fetching + /// the last 14 days of messages) + dependencies.storage.writeAsync { db in + try SnodeReceivedMessageInfo.handlePotentialDeletedOrInvalidHash( + db, + potentiallyInvalidHashes: Array(hashes) + ) + } + } + } + ) + } } } diff --git a/SessionMessagingKit/Sending & Receiving/MessageReceiver.swift b/SessionMessagingKit/Sending & Receiving/MessageReceiver.swift index 6b699fc8f..7dcc5ce45 100644 --- a/SessionMessagingKit/Sending & Receiving/MessageReceiver.swift +++ b/SessionMessagingKit/Sending & Receiving/MessageReceiver.swift @@ -335,7 +335,8 @@ public enum MessageReceiver { db, threadId: threadId, threadVariant: threadVariant, - message: message + message: message, + using: dependencies ) case let message as CallMessage: