Use subclass of outgoing message for group updates

Currently, group (v2) updates are just instances of `TSOutgoingMessage`. This works, but is inconsistent with other outgoing messages, which subclass it. This refactor changes that to make things consistent. I think this is a useful change on its own, but it will also make future changes easier.
2022-08-16 14:45:51 -05:00 · 2022-08-16 14:45:51 -05:00 · e858a0d916
commit e858a0d916
parent 4dffd80aae
5 changed files with 92 additions and 159 deletions
--- a/Signal.xcodeproj/project.pbxproj
+++ b/Signal.xcodeproj/project.pbxproj
@ -1174,6 +1174,7 @@
 		F94262A1289B1B5600460798 /* SignalServiceAddressTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = F9426239289B1B5500460798 /* SignalServiceAddressTest.swift */; };
 		F94262A2289B1B5600460798 /* SSKBaseTestSwift.swift in Sources */ = {isa = PBXBuildFile; fileRef = F942623A289B1B5500460798 /* SSKBaseTestSwift.swift */; };
 		F95427E6286E042200314EDA /* BadgeGiftingThanksSheet.swift in Sources */ = {isa = PBXBuildFile; fileRef = F95427E5286E042200314EDA /* BadgeGiftingThanksSheet.swift */; };
+		F96A534328A1AE7C003262D4 /* OutgoingGroupUpdateMessage.swift in Sources */ = {isa = PBXBuildFile; fileRef = F96A534228A1AE7B003262D4 /* OutgoingGroupUpdateMessage.swift */; };
 		F973DFBA0991C967C4934B08 /* Pods_SignalUITests.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = BD0D81F9D93CB5BADE6B9709 /* Pods_SignalUITests.framework */; };
 		F97A2EEA282578C000610669 /* BadgeExpirationSheetStateTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = F97A2EE828247C1300610669 /* BadgeExpirationSheetStateTest.swift */; };
 		F9844C492867936400B16DD4 /* SignalMeTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = F9844C482867936400B16DD4 /* SignalMeTest.swift */; };
@ -3534,6 +3535,7 @@
 		F942623A289B1B5500460798 /* SSKBaseTestSwift.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; name = SSKBaseTestSwift.swift; path = SignalServiceKit/tests/SSKBaseTestSwift.swift; sourceTree = SOURCE_ROOT; };
 		F95427E5286E042200314EDA /* BadgeGiftingThanksSheet.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BadgeGiftingThanksSheet.swift; sourceTree = "<group>"; };
 		F965C5B87BFDF8F86556C04C /* Pods-SignalNSE.profiling.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-SignalNSE.profiling.xcconfig"; path = "Pods/Target Support Files/Pods-SignalNSE/Pods-SignalNSE.profiling.xcconfig"; sourceTree = "<group>"; };
+		F96A534228A1AE7B003262D4 /* OutgoingGroupUpdateMessage.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = OutgoingGroupUpdateMessage.swift; sourceTree = "<group>"; };
 		F97A2EE828247C1300610669 /* BadgeExpirationSheetStateTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BadgeExpirationSheetStateTest.swift; sourceTree = "<group>"; };
 		F9844C482867936400B16DD4 /* SignalMeTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SignalMeTest.swift; sourceTree = "<group>"; };
 		F988DC12289DC8F2003B4B82 /* OWSOutgoingReactionMessageTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OWSOutgoingReactionMessageTest.swift; sourceTree = "<group>"; };
@ -7132,6 +7134,7 @@
 				F9C5C99E289453B100548EEE /* OWSUnknownProtocolVersionMessage.m */,
 				F9C5C99F289453B100548EEE /* OWSMessageManager.h */,
 				F9C5C9A0289453B100548EEE /* OWSMessageContentJob.m */,
+				F96A534228A1AE7B003262D4 /* OutgoingGroupUpdateMessage.swift */,
 			);
 			name = Messages;
 			path = src/Messages;
@ -10827,6 +10830,7 @@
 				F9C5CC7F289453B300548EEE /* TSAttachment.swift in Sources */,
 				F9C5CC0D289453B300548EEE /* StickerMetadata.swift in Sources */,
 				F9C5CC27289453B300548EEE /* TSInvalidIdentityKeySendingErrorMessage+SDS.swift in Sources */,
+				F96A534328A1AE7C003262D4 /* OutgoingGroupUpdateMessage.swift in Sources */,
 				F9C5CBF8289453B300548EEE /* TSInteraction.swift in Sources */,
 				F9C5CCAA289453B300548EEE /* TSAccountManager.m in Sources */,
 				F9C5CC97289453B300548EEE /* WebSocketProto.swift in Sources */,
--- a/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.h
+++ b/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.h
@ -186,17 +186,6 @@ NS_DESIGNATED_INITIALIZER NS_SWIFT_NAME(init(grdbId:uniqueId:receivedAtTimestamp
                            linkPreview:(nullable OWSLinkPreview *)linkPreview
                         messageSticker:(nullable MessageSticker *)messageSticker;

-+ (instancetype)outgoingMessageInThread:(TSThread *)thread
-                       groupMetaMessage:(TSGroupMetaMessage)groupMetaMessage
-                       expiresInSeconds:(uint32_t)expiresInSeconds
-                            transaction:(SDSAnyReadTransaction *)transaction;
-
-+ (instancetype)outgoingMessageInThread:(TSThread *)thread
-                       groupMetaMessage:(TSGroupMetaMessage)groupMetaMessage
-                       expiresInSeconds:(uint32_t)expiresInSeconds
-                 changeActionsProtoData:(nullable NSData *)changeActionsProtoData
-                            transaction:(SDSAnyReadTransaction *)transaction;
-
 - (void)removeTemporaryAttachmentsWithTransaction:(SDSAnyWriteTransaction *)transaction;

@property (nonatomic, readonly) TSOutgoingMessageState messageState;
--- a/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.m
+++ b/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.m
@ -272,30 +272,6 @@ NSUInteger const TSOutgoingMessageSchemaVersion = 1;
    return [builder buildWithSneakyTransaction];
 }

-+ (instancetype)outgoingMessageInThread:(TSThread *)thread
-                       groupMetaMessage:(TSGroupMetaMessage)groupMetaMessage
-                       expiresInSeconds:(uint32_t)expiresInSeconds
-                            transaction:(SDSAnyReadTransaction *)transaction
-{
-    TSOutgoingMessageBuilder *builder = [TSOutgoingMessageBuilder outgoingMessageBuilderWithThread:thread];
-    builder.groupMetaMessage = groupMetaMessage;
-    builder.expiresInSeconds = expiresInSeconds;
-    return [builder buildWithTransaction:transaction];
-}
-
-+ (instancetype)outgoingMessageInThread:(TSThread *)thread
-                       groupMetaMessage:(TSGroupMetaMessage)groupMetaMessage
-                       expiresInSeconds:(uint32_t)expiresInSeconds
-                 changeActionsProtoData:(nullable NSData *)changeActionsProtoData
-                            transaction:(SDSAnyReadTransaction *)transaction
-{
-    TSOutgoingMessageBuilder *builder = [TSOutgoingMessageBuilder outgoingMessageBuilderWithThread:thread];
-    builder.groupMetaMessage = groupMetaMessage;
-    builder.expiresInSeconds = expiresInSeconds;
-    builder.changeActionsProtoData = changeActionsProtoData;
-    return [builder buildWithTransaction:transaction];
-}
-
 - (instancetype)initOutgoingMessageWithBuilder:(TSOutgoingMessageBuilder *)outgoingMessageBuilder
                                   transaction:(SDSAnyReadTransaction *)transaction
 {
--- a/SignalServiceKit/src/Messages/OutgoingGroupUpdateMessage.swift
+++ b/SignalServiceKit/src/Messages/OutgoingGroupUpdateMessage.swift
@ -0,0 +1,50 @@
+//
+//  Copyright (c) 2022 Open Whisper Systems. All rights reserved.
+//
+
+import Foundation
+
+/// An outgoing group v2 update.
+public class OutgoingGroupUpdateMessage: TSOutgoingMessage {
+    public init<T: Sequence>(
+        in thread: TSGroupThread,
+        groupMetaMessage: TSGroupMetaMessage,
+        expiresInSeconds: UInt32 = 0,
+        changeActionsProtoData: Data? = nil,
+        additionalRecipients: T,
+        transaction: SDSAnyReadTransaction
+    ) where T.Element == SignalServiceAddress {
+        let builder = TSOutgoingMessageBuilder(thread: thread)
+        builder.groupMetaMessage = groupMetaMessage
+        builder.expiresInSeconds = expiresInSeconds
+        builder.changeActionsProtoData = changeActionsProtoData
+        builder.additionalRecipients = Array(additionalRecipients)
+
+        super.init(outgoingMessageWithBuilder: builder, transaction: transaction)
+    }
+
+    convenience init(
+        in thread: TSGroupThread,
+        groupMetaMessage: TSGroupMetaMessage,
+        expiresInSeconds: UInt32 = 0,
+        changeActionsProtoData: Data? = nil,
+        transaction: SDSAnyReadTransaction
+    ) {
+        self.init(
+            in: thread,
+            groupMetaMessage: groupMetaMessage,
+            expiresInSeconds: expiresInSeconds,
+            changeActionsProtoData: changeActionsProtoData,
+            additionalRecipients: [],
+            transaction: transaction
+        )
+    }
+
+    required init?(coder: NSCoder) {
+        super.init(coder: coder)
+    }
+
+    required init(dictionary: [String: Any]!) throws {
+        try super.init(dictionary: dictionary)
+    }
+}
--- a/SignalServiceKit/src/groups/GroupManager.swift
+++ b/SignalServiceKit/src/groups/GroupManager.swift
@ -182,11 +182,6 @@ public class GroupManager: NSObject {
            Logger.warn("Member without UUID.")
            return false
        }
-        // NOTE: We do consider users to support groups v2 even if:
-        //
-        // * We don't know their profile key.
-        // * They've never done a versioned profile update.
-        // * We don't have a profile key credential for them.
        return true
    }

@ -1287,56 +1282,25 @@ public class GroupManager: NSObject {
    public static func sendGroupUpdateMessage(thread: TSGroupThread,
                                              changeActionsProtoData: Data? = nil,
                                              singleRecipient: SignalServiceAddress? = nil) -> Promise<Void> {
-
-        // Only honor groupsV2dontSendUpdates for v2 groups.
-        let shouldSkipUpdate = thread.isGroupV2Thread && DebugFlags.groupsV2dontSendUpdates.get()
-        if shouldSkipUpdate {
+        guard thread.isGroupV2Thread, !DebugFlags.groupsV2dontSendUpdates.get() else {
            return Promise.value(())
        }

-        return databaseStorage.read(.promise) { transaction in
-            let expiresInSeconds = thread.disappearingMessagesDuration(with: transaction)
-            let messageBuilder = TSOutgoingMessageBuilder(thread: thread)
-            messageBuilder.expiresInSeconds = expiresInSeconds
-            // V2 group update messages mostly ignore groupMetaMessage,
-            // but we set it to get the right behavior in shouldBeSaved.
-            // i.e. we need to flag this message as a group update that
-            // is "durable but transient" - it should not be saved.
-            messageBuilder.groupMetaMessage = .update
-
-            if thread.isGroupV2Thread {
-                messageBuilder.changeActionsProtoData = changeActionsProtoData
-                if singleRecipient == nil {
-                    self.addAdditionalRecipients(to: messageBuilder,
-                                                 groupThread: thread)
-                }
-            }
-            return messageBuilder.build(transaction: transaction)
-        }.then(on: .global()) { (message: TSOutgoingMessage) throws -> Promise<Void> in
+        return databaseStorage.write(.promise) { transaction in
+            let message = OutgoingGroupUpdateMessage(
+                in: thread,
+                groupMetaMessage: .update,
+                expiresInSeconds: thread.disappearingMessagesDuration(with: transaction),
+                changeActionsProtoData: changeActionsProtoData,
+                additionalRecipients: singleRecipient == nil ? Self.invitedMembers(in: thread) : [],
+                transaction: transaction
+            )

            if let singleRecipient = singleRecipient {
-                Self.databaseStorage.write { transaction in
-                    message.updateWithSending(toSingleGroupRecipient: singleRecipient, transaction: transaction)
-                }
+                message.updateWithSending(toSingleGroupRecipient: singleRecipient, transaction: transaction)
            }

-            let groupModel = thread.groupModel
-            // V1 group updates need to include the group avatar (if any)
-            // as an attachment.
-            if thread.isGroupV1Thread,
-               let avatarData = groupModel.avatarData,
-               avatarData.count > 0 {
-                let imageFormat = (avatarData as NSData).imageMetadata(withPath: nil, mimeType: nil).imageFormat
-                let fileExtension = (imageFormat == .png) ? "png" : "jpg"
-                let mimeType = (imageFormat == .png) ? OWSMimeTypeImagePng : OWSMimeTypeImageJpeg
-
-                if let dataSource = DataSourceValue.dataSource(with: avatarData, fileExtension: fileExtension) {
-                    let attachment = GroupUpdateMessageAttachment(contentType: mimeType, dataSource: dataSource)
-                    return self.sendGroupUpdateMessage(message, thread: thread, attachment: attachment)
-                }
-            }
-
-            return self.sendGroupUpdateMessage(message, thread: thread)
+            Self.messageSenderJobQueue.add(message: message.asPreparer, transaction: transaction)
        }
    }

@ -1345,70 +1309,29 @@ public class GroupManager: NSObject {
        let dataSource: DataSource
    }

-    // v1 group update messages should be non-durable and have specific error handling.
-    // v2 group update messages should be durable.
-    private static func sendGroupUpdateMessage(_ message: TSOutgoingMessage,
-                                               thread: TSGroupThread,
-                                               attachment: GroupUpdateMessageAttachment? = nil) -> Promise<Void> {
-        if thread.isGroupV1Thread {
-            owsFailDebug("GV1 group updates no longer supported")
-            return Promise.value(())
-        } else {
-            // v2 group update.
-            //
-            // Enqueue the message for a durable send.
-            return databaseStorage.write(.promise) { transaction in
-                self.messageSenderJobQueue.add(message: message.asPreparer,
-                                               transaction: transaction)
-            }
-        }
-    }
-
    private static func sendDurableNewGroupMessage(forThread thread: TSGroupThread) -> Promise<Void> {
-        // Only honor groupsV2dontSendUpdates for v2 groups.
-        let shouldSkipUpdate = thread.isGroupV2Thread && DebugFlags.groupsV2dontSendUpdates.get()
-        if shouldSkipUpdate {
+        guard thread.isGroupV2Thread, !DebugFlags.groupsV2dontSendUpdates.get() else {
            return Promise.value(())
        }

-        return firstly {
-            databaseStorage.write(.promise) { transaction in
-                let expiresInSeconds = thread.disappearingMessagesDuration(with: transaction)
-                let messageBuilder = TSOutgoingMessageBuilder(thread: thread)
-                messageBuilder.groupMetaMessage = .new
-                messageBuilder.expiresInSeconds = expiresInSeconds
-                self.addAdditionalRecipients(to: messageBuilder,
-                                             groupThread: thread)
-                let message = messageBuilder.build(transaction: transaction)
-                self.messageSenderJobQueue.add(message: message.asPreparer,
-                                               transaction: transaction)
-            }
-        }.then(on: .global()) { _ -> Promise<Void> in
-            // The "new group" update message for v1 groups doesn't support avatars.
-            // So, if a new v1 group has an avatar, we need to send a group update
-            // message.
-            guard thread.groupModel.groupsVersion == .V1,
-                  thread.groupModel.avatarHash != nil else {
-                return Promise.value(())
-            }
-            return self.sendGroupUpdateMessage(thread: thread)
+        return databaseStorage.write(.promise) { transaction in
+            let message = OutgoingGroupUpdateMessage(
+                in: thread,
+                groupMetaMessage: .new,
+                expiresInSeconds: thread.disappearingMessagesDuration(with: transaction),
+                additionalRecipients: Self.invitedMembers(in: thread),
+                transaction: transaction
+            )
+            Self.messageSenderJobQueue.add(message: message.asPreparer, transaction: transaction)
        }
    }

-    private static func addAdditionalRecipients(to messageBuilder: TSOutgoingMessageBuilder,
-                                                groupThread: TSGroupThread) {
-        guard groupThread.groupModel.groupsVersion == .V2 else {
-            // No need to add "additional recipients" to v1 groups.
-            return
-        }
-        // We need to send v2 group updates to pending members
-        // as well.  Normal group sends only include "full members".
-        assert(messageBuilder.additionalRecipients == nil)
-        let groupMembership = groupThread.groupModel.groupMembership
-        let additionalRecipients = groupMembership.invitedMembers.filter { address in
-            return doesUserSupportGroupsV2(address: address)
-        }
-        messageBuilder.additionalRecipients = Array(additionalRecipients)
+    private static func invitedMembers(in thread: TSGroupThread) -> Set<SignalServiceAddress> {
+        thread.groupModel.groupMembership.invitedMembers.filter { doesUserSupportGroupsV2(address: $0) }
+    }
+
+    private static func invitedOrRequestedMembers(in thread: TSGroupThread) -> Set<SignalServiceAddress> {
+        thread.groupModel.groupMembership.invitedOrRequestMembers.filter { doesUserSupportGroupsV2(address: $0) }
    }

    @objc
@ -1432,10 +1355,9 @@ public class GroupManager: NSObject {
            return
        }

-        let message = TSOutgoingMessage(in: groupThread,
-                                        groupMetaMessage: .quit,
-                                        expiresInSeconds: 0,
-                                        transaction: transaction)
+        let message = OutgoingGroupUpdateMessage(in: groupThread,
+                                                 groupMetaMessage: .quit,
+                                                 transaction: transaction)
        messageSenderJobQueue.add(message: message.asPreparer, transaction: transaction)
    }

@ -1446,21 +1368,13 @@ public class GroupManager: NSObject {
            return
        }

-        let messageBuilder = TSOutgoingMessageBuilder(thread: groupThread)
-        // V2 group update messages mostly ignore groupMetaMessage,
-        // but we set it to get the right behavior in shouldBeSaved.
-        // i.e. we need to flag this message as a group update that
-        // is "durable but transient" - it should not be saved.
-        messageBuilder.groupMetaMessage = .update
-        // We need to send v2 group updates to pending members
-        // as well.  Normal group sends only include "full members".
-        assert(messageBuilder.additionalRecipients == nil)
-        let groupMembership = groupThread.groupModel.groupMembership
-        let additionalRecipients = groupMembership.invitedOrRequestMembers.filter { address in
-            return doesUserSupportGroupsV2(address: address)
-        }
-        messageBuilder.additionalRecipients = Array(additionalRecipients)
-        let message = messageBuilder.build(transaction: transaction)
+        let message = OutgoingGroupUpdateMessage(
+            in: groupThread,
+            groupMetaMessage: .update,
+            additionalRecipients: Self.invitedOrRequestedMembers(in: groupThread),
+            transaction: transaction
+        )
+
        messageSenderJobQueue.add(
            .promise,
            message: message.asPreparer,