From 415e6c230c9f00935577d59cdc39b85af57963cc Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Fri, 28 Feb 2020 18:19:45 -0300 Subject: [PATCH] Resolve groups v2-related TODOs. --- .../groups/GroupV2UpdatesImpl.swift | 86 ++++++++++++++----- .../groups/GroupsV2ChangeSetImpl.swift | 2 +- 2 files changed, 66 insertions(+), 22 deletions(-) diff --git a/SignalMessaging/groups/GroupV2UpdatesImpl.swift b/SignalMessaging/groups/GroupV2UpdatesImpl.swift index 63e3b12464..48f0d67944 100644 --- a/SignalMessaging/groups/GroupV2UpdatesImpl.swift +++ b/SignalMessaging/groups/GroupV2UpdatesImpl.swift @@ -286,26 +286,7 @@ public class GroupV2UpdatesImpl: NSObject, GroupV2UpdatesSwift { return firstly { return GroupManager.ensureLocalProfileHasCommitmentIfNecessary() - }.map(on: .global()) { () throws -> Bool in - guard let localAddress = self.tsAccountManager.localAddress else { - throw OWSAssertionError("Missing localAddress.") - } - let groupId = try self.groupsV2.groupId(forGroupSecretParamsData: groupSecretParamsData) - let thread = self.databaseStorage.read { transaction in - return TSGroupThread.fetch(groupId: groupId, transaction: transaction) - } - var canFetchChangeActions = false - if let groupThread = thread { - // Pending members can fetch snapshots but not change actions. - let groupMembership = groupThread.groupModel.groupMembership - canFetchChangeActions = groupMembership.isNonPendingMember(localAddress) - } - return canFetchChangeActions - }.then(on: DispatchQueue.global()) { (canFetchChangeActions: Bool) throws -> Promise in - guard canFetchChangeActions else { - return self.fetchAndApplyCurrentGroupV2SnapshotFromService(groupSecretParamsData: groupSecretParamsData, - groupUpdateMode: groupUpdateMode) - } + }.then(on: DispatchQueue.global()) { () throws -> Promise in // Try to use individual changes. return self.fetchAndApplyChangeActionsFromService(groupSecretParamsData: groupSecretParamsData, groupUpdateMode: groupUpdateMode) @@ -403,6 +384,7 @@ public class GroupV2UpdatesImpl: NSObject, GroupV2UpdatesSwift { }.then(on: DispatchQueue.global()) { (groupChanges: [GroupV2Change]) throws -> Promise in let groupId = try self.groupsV2.groupId(forGroupSecretParamsData: groupSecretParamsData) return self.tryToApplyGroupChangesFromService(groupId: groupId, + groupSecretParamsData: groupSecretParamsData, groupChanges: groupChanges, groupUpdateMode: groupUpdateMode) }.timeout(seconds: GroupManager.KGroupUpdateTimeoutDuration) { @@ -411,15 +393,18 @@ public class GroupV2UpdatesImpl: NSObject, GroupV2UpdatesSwift { } private func tryToApplyGroupChangesFromService(groupId: Data, + groupSecretParamsData: Data, groupChanges: [GroupV2Change], groupUpdateMode: GroupUpdateMode) -> Promise { switch groupUpdateMode { case .upToCurrentRevisionImmediately: return tryToApplyGroupChangesFromServiceNow(groupId: groupId, + groupSecretParamsData: groupSecretParamsData, groupChanges: groupChanges, upToRevision: nil) case .upToSpecificRevisionImmediately(let upToRevision): return tryToApplyGroupChangesFromServiceNow(groupId: groupId, + groupSecretParamsData: groupSecretParamsData, groupChanges: groupChanges, upToRevision: upToRevision) case .upToCurrentRevisionAfterMessageProcessWithThrottling: @@ -427,6 +412,7 @@ public class GroupV2UpdatesImpl: NSObject, GroupV2UpdatesSwift { return self.messageProcessing.allMessageFetchingAndProcessingPromise() }.then(on: .global()) { _ in return self.tryToApplyGroupChangesFromServiceNow(groupId: groupId, + groupSecretParamsData: groupSecretParamsData, groupChanges: groupChanges, upToRevision: nil) } @@ -434,6 +420,7 @@ public class GroupV2UpdatesImpl: NSObject, GroupV2UpdatesSwift { } private func tryToApplyGroupChangesFromServiceNow(groupId: Data, + groupSecretParamsData: Data, groupChanges: [GroupV2Change], upToRevision: UInt32?) -> Promise { @@ -443,7 +430,11 @@ public class GroupV2UpdatesImpl: NSObject, GroupV2UpdatesSwift { } return databaseStorage.write(.promise) { (transaction: SDSAnyWriteTransaction) throws -> TSGroupThread in - guard let oldGroupThread = TSGroupThread.fetch(groupId: groupId, transaction: transaction) else { + // See comment on getOrCreateThreadForGroupChanges(...). + guard let oldGroupThread = self.getOrCreateThreadForGroupChanges(groupId: groupId, + groupSecretParamsData: groupSecretParamsData, + groupChanges: groupChanges, + transaction: transaction) else { throw OWSAssertionError("Missing group thread.") } guard let oldGroupModel = oldGroupThread.groupModel as? TSGroupModelV2 else { @@ -519,6 +510,59 @@ public class GroupV2UpdatesImpl: NSObject, GroupV2UpdatesSwift { } } + // When learning about v2 groups for the first time, we can always + // insert them into the database using a snapshot. However we prefer + // to create them from group changes if possible, because group + // changes have more information. Critically, we can usually determine + // who create the group or who added or invited us to the group. + // + // Therefore, before starting to apply group changes we use this + // method to insert the group into the database if necessary. + private func getOrCreateThreadForGroupChanges(groupId: Data, + groupSecretParamsData: Data, + groupChanges: [GroupV2Change], + transaction: SDSAnyWriteTransaction) -> TSGroupThread? { + if let groupThread = TSGroupThread.fetch(groupId: groupId, transaction: transaction) { + return groupThread + } + + do { + let groupV2Params = try GroupV2Params(groupSecretParamsData: groupSecretParamsData) + + guard let groupChange = groupChanges.first else { + return nil + } + + let builder = try TSGroupModelBuilder(groupV2Snapshot: groupChange.snapshot) + let newGroupModel = try builder.build(transaction: transaction) + + // Many change actions have author info, e.g. addedByUserID. But we can + // safely assume that all actions in the "change actions" have the same author. + guard let changeAuthorUuidData = groupChange.changeActionsProto.sourceUuid else { + throw OWSAssertionError("Missing changeAuthorUuid.") + } + let changeAuthorUuid = try groupV2Params.uuid(forUserId: changeAuthorUuidData) + let groupUpdateSourceAddress = SignalServiceAddress(uuid: changeAuthorUuid) + + let newDisappearingMessageToken = groupChange.snapshot.disappearingMessageToken + + let result = try GroupManager.tryToUpsertExistingGroupThreadInDatabaseAndCreateInfoMessage(newGroupModel: newGroupModel, + newDisappearingMessageToken: newDisappearingMessageToken, + groupUpdateSourceAddress: groupUpdateSourceAddress, + canInsert: true, + transaction: transaction) + + // NOTE: We don't need to worry about profile keys here. This method is + // only used by tryToApplyGroupChangesFromServiceNow() which will take + // care of that. + + return result.groupThread + } catch { + owsFailDebug("Error: \(error)") + return nil + } + } + // MARK: - Current Snapshot private func fetchAndApplyCurrentGroupV2SnapshotFromService(groupSecretParamsData: Data, diff --git a/SignalMessaging/groups/GroupsV2ChangeSetImpl.swift b/SignalMessaging/groups/GroupsV2ChangeSetImpl.swift index e8ce3c444a..4a34efbaa7 100644 --- a/SignalMessaging/groups/GroupsV2ChangeSetImpl.swift +++ b/SignalMessaging/groups/GroupsV2ChangeSetImpl.swift @@ -376,7 +376,7 @@ public class GroupsV2ChangeSetImpl: NSObject, GroupsV2ChangeSet { } if shouldUpdateAvatar { - if newAvatarUrlPath != currentGroupModel.avatarUrlPath { + if newAvatarUrlPath == currentGroupModel.avatarUrlPath { // Redundant change, not a conflict. owsFailDebug("This should never occur.") } else {