From e858a0d9167e3eb4e42870e6688cfd61383d46cd Mon Sep 17 00:00:00 2001 From: Evan Hahn <69474926+EvanHahn-Signal@users.noreply.github.com> Date: Tue, 16 Aug 2022 14:45:51 -0500 Subject: [PATCH] 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. --- Signal.xcodeproj/project.pbxproj | 4 + .../Messages/Interactions/TSOutgoingMessage.h | 11 -- .../Messages/Interactions/TSOutgoingMessage.m | 24 --- .../Messages/OutgoingGroupUpdateMessage.swift | 50 ++++++ .../src/groups/GroupManager.swift | 162 ++++-------------- 5 files changed, 92 insertions(+), 159 deletions(-) create mode 100644 SignalServiceKit/src/Messages/OutgoingGroupUpdateMessage.swift diff --git a/Signal.xcodeproj/project.pbxproj b/Signal.xcodeproj/project.pbxproj index 869545cd96..76d5a9b1fe 100644 --- 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 = ""; }; 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 = ""; }; + F96A534228A1AE7B003262D4 /* OutgoingGroupUpdateMessage.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = OutgoingGroupUpdateMessage.swift; sourceTree = ""; }; F97A2EE828247C1300610669 /* BadgeExpirationSheetStateTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BadgeExpirationSheetStateTest.swift; sourceTree = ""; }; F9844C482867936400B16DD4 /* SignalMeTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SignalMeTest.swift; sourceTree = ""; }; F988DC12289DC8F2003B4B82 /* OWSOutgoingReactionMessageTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OWSOutgoingReactionMessageTest.swift; sourceTree = ""; }; @@ -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 */, diff --git a/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.h b/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.h index 998dc4a072..8789ace2cb 100644 --- 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; diff --git a/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.m b/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.m index 804fd48cfb..f5358069bf 100644 --- 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 { diff --git a/SignalServiceKit/src/Messages/OutgoingGroupUpdateMessage.swift b/SignalServiceKit/src/Messages/OutgoingGroupUpdateMessage.swift new file mode 100644 index 0000000000..b1bfd312ba --- /dev/null +++ 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( + 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) + } +} diff --git a/SignalServiceKit/src/groups/GroupManager.swift b/SignalServiceKit/src/groups/GroupManager.swift index b973162d7d..812b44f258 100644 --- 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 { - - // 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 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 { - 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 { - // 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 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 { + thread.groupModel.groupMembership.invitedMembers.filter { doesUserSupportGroupsV2(address: $0) } + } + + private static func invitedOrRequestedMembers(in thread: TSGroupThread) -> Set { + 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,