diff --git a/SignalMessaging/groups/GroupV2UpdatesImpl.swift b/SignalMessaging/groups/GroupV2UpdatesImpl.swift index 89c105e8c7..fbff46687f 100644 --- a/SignalMessaging/groups/GroupV2UpdatesImpl.swift +++ b/SignalMessaging/groups/GroupV2UpdatesImpl.swift @@ -693,21 +693,16 @@ public class GroupV2UpdatesImpl: NSObject, GroupV2UpdatesSwift { } } - let diff = groupChange.diff - let changeActionsProto = diff.changeActionsProto - // 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 = changeActionsProto.sourceUuid else { - throw OWSAssertionError("Missing changeAuthorUuid.") - } - guard let oldGroupModel = groupThread.groupModel as? TSGroupModelV2 else { throw OWSAssertionError("Invalid group model.") } let isSingleRevisionUpdate = oldGroupModel.revision + 1 == changeRevision var groupUpdateSourceAddress: SignalServiceAddress? - if isSingleRevisionUpdate { + if isSingleRevisionUpdate, let changeAuthorUuidData = groupChange.changeActionsProto?.sourceUuid { + // 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. + // // The "group update" info message should only reflect // the "change author" if the change/diff reflects a // single revision. Eventually there will be gaps in @@ -717,6 +712,10 @@ public class GroupV2UpdatesImpl: NSObject, GroupV2UpdatesSwift { // the service. This is one. let changeAuthorUuid = try groupV2Params.uuid(forUserId: changeAuthorUuidData) groupUpdateSourceAddress = SignalServiceAddress(uuid: changeAuthorUuid) + } else { + owsAssertDebug(groupChange.changeActionsProto == nil + || groupChange.changeActionsProto?.sourceUuid != nil, + "explicit changes should always have authors") } // We should only replace placeholder models using @@ -750,15 +749,18 @@ public class GroupV2UpdatesImpl: NSObject, GroupV2UpdatesSwift { newGroupModel = try builder.build(transaction: transaction) newDisappearingMessageToken = snapshot.disappearingMessageToken newProfileKeys = snapshot.profileKeys - } else { + } else if let changeActionsProto = groupChange.changeActionsProto { let changedGroupModel = try GroupsV2IncomingChanges.applyChangesToGroupModel(groupThread: groupThread, changeActionsProto: changeActionsProto, - downloadedAvatars: diff.downloadedAvatars, + downloadedAvatars: groupChange.downloadedAvatars, groupModelOptions: groupModelOptions, transaction: transaction) newGroupModel = changedGroupModel.newGroupModel newDisappearingMessageToken = changedGroupModel.newDisappearingMessageToken newProfileKeys = changedGroupModel.profileKeys + } else { + owsFailDebug("neither a snapshot nor a change action (should have been validated earlier)") + continue } // We should only replace placeholder models using @@ -840,26 +842,30 @@ public class GroupV2UpdatesImpl: NSObject, GroupV2UpdatesSwift { guard let snapshot = firstGroupChange.snapshot else { throw OWSAssertionError("Missing snapshot.") } - // 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 = firstGroupChange.diff.changeActionsProto.sourceUuid else { - throw OWSAssertionError("Missing changeAuthorUuid.") + + var groupUpdateSourceAddress: SignalServiceAddress? + if let changeAuthorUuidData = firstGroupChange.changeActionsProto?.sourceUuid { + // 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. + // + // Some userIds/uuidCiphertexts can be validated by + // the service. This is one. + let changeAuthorUuid = try groupV2Params.uuid(forUserId: changeAuthorUuidData) + groupUpdateSourceAddress = SignalServiceAddress(uuid: changeAuthorUuid) + } else { + owsAssertDebug(firstGroupChange.changeActionsProto == nil + || firstGroupChange.changeActionsProto?.sourceUuid != nil, + "explicit changes should always have authors") } - // Some userIds/uuidCiphertexts can be validated by - // the service. This is one. - let changeAuthorUuid = try groupV2Params.uuid(forUserId: changeAuthorUuidData) var builder = try TSGroupModelBuilder.builderForSnapshot(groupV2Snapshot: snapshot, transaction: transaction) - if snapshot.revision == 0, - let localUuid = tsAccountManager.localUuid, - localUuid == changeAuthorUuid { + if snapshot.revision == 0, groupUpdateSourceAddress?.isLocalAddress == true { builder.wasJustCreatedByLocalUser = true } let newGroupModel = try builder.build(transaction: transaction) - let groupUpdateSourceAddress = SignalServiceAddress(uuid: changeAuthorUuid) let newDisappearingMessageToken = snapshot.disappearingMessageToken let didAddLocalUserToV2Group = self.didAddLocalUserToV2Group(groupChange: firstGroupChange, groupV2Params: groupV2Params) @@ -887,12 +893,15 @@ public class GroupV2UpdatesImpl: NSObject, GroupV2UpdatesSwift { guard let localUuid = tsAccountManager.localUuid else { return false } - if groupChange.diff.revision == 0 { + if groupChange.revision == 0 { // Revision 0 is a special case and won't have actions to // reflect the initial membership. return true } - let changeActionsProto = groupChange.diff.changeActionsProto + guard let changeActionsProto = groupChange.changeActionsProto else { + // We're missing a change here, so we can't assume this is how we got into the group. + return false + } for action in changeActionsProto.addMembers { do { diff --git a/SignalMessaging/groups/GroupsV2Protos.swift b/SignalMessaging/groups/GroupsV2Protos.swift index a8b32b9612..75e7ef839a 100644 --- a/SignalMessaging/groups/GroupsV2Protos.swift +++ b/SignalMessaging/groups/GroupsV2Protos.swift @@ -490,19 +490,27 @@ public class GroupsV2Protos { var result = [GroupV2Change]() for changeStateData in groupChangesProto.groupChanges { let changeStateProto = try GroupsProtoGroupChangesGroupChangeState(serializedData: changeStateData) + var snapshot: GroupV2Snapshot? if let snapshotProto = changeStateProto.groupState { snapshot = try parse(groupProto: snapshotProto, downloadedAvatars: downloadedAvatars, groupV2Params: groupV2Params) } - guard let changeProto = changeStateProto.groupChange else { - throw OWSAssertionError("Missing groupChange proto.") + + var changeActionsProto: GroupsProtoGroupChangeActions? + if let changeProto = changeStateProto.groupChange { + // We can ignoreSignature because these protos came from the service. + changeActionsProto = try parseAndVerifyChangeActionsProto(changeProto, ignoreSignature: true) } - // We can ignoreSignature because these protos came from the service. - let changeActionsProto: GroupsProtoGroupChangeActions = try parseAndVerifyChangeActionsProto(changeProto, ignoreSignature: true) - let diff = GroupV2Diff(changeActionsProto: changeActionsProto, downloadedAvatars: downloadedAvatars) - result.append(GroupV2Change(snapshot: snapshot, diff: diff)) + + guard snapshot != nil || changeActionsProto != nil else { + throw OWSAssertionError("both groupState and groupChange are absent") + } + + result.append(GroupV2Change(snapshot: snapshot, + changeActionsProto: changeActionsProto, + downloadedAvatars: downloadedAvatars)) } return result } @@ -542,13 +550,11 @@ public class GroupsV2Protos { } avatarUrlPaths += collectAvatarUrlPaths(groupProto: groupState) - guard let changeProto = changeStateProto.groupChange else { - owsFailDebug("Missing groupChange proto.") - throw GroupsV2Error.missingGroupChangeProtos + if let changeProto = changeStateProto.groupChange { + // We can ignoreSignature because these protos came from the service. + let changeActionsProto = try parseAndVerifyChangeActionsProto(changeProto, ignoreSignature: ignoreSignature) + avatarUrlPaths += self.collectAvatarUrlPaths(changeActionsProto: changeActionsProto) } - // We can ignoreSignature because these protos came from the service. - let changeActionsProto = try parseAndVerifyChangeActionsProto(changeProto, ignoreSignature: ignoreSignature) - avatarUrlPaths += self.collectAvatarUrlPaths(changeActionsProto: changeActionsProto) } return avatarUrlPaths } diff --git a/SignalServiceKit/src/groups/GroupsV2.swift b/SignalServiceKit/src/groups/GroupsV2.swift index b18d735ac3..486263753b 100644 --- a/SignalServiceKit/src/groups/GroupsV2.swift +++ b/SignalServiceKit/src/groups/GroupsV2.swift @@ -317,35 +317,22 @@ public protocol GroupV2Snapshot { // MARK: - -public struct GroupV2Diff { - public let changeActionsProto: GroupsProtoGroupChangeActions +public struct GroupV2Change { + public var snapshot: GroupV2Snapshot? + public var changeActionsProto: GroupsProtoGroupChangeActions? public let downloadedAvatars: GroupV2DownloadedAvatars - public init(changeActionsProto: GroupsProtoGroupChangeActions, + public init(snapshot: GroupV2Snapshot?, + changeActionsProto: GroupsProtoGroupChangeActions?, downloadedAvatars: GroupV2DownloadedAvatars) { + owsAssert(snapshot != nil || changeActionsProto != nil) + self.snapshot = snapshot self.changeActionsProto = changeActionsProto self.downloadedAvatars = downloadedAvatars } public var revision: UInt32 { - return changeActionsProto.revision - } -} - -// MARK: - - -public struct GroupV2Change { - public var snapshot: GroupV2Snapshot? - public var diff: GroupV2Diff - - public init(snapshot: GroupV2Snapshot?, - diff: GroupV2Diff) { - self.snapshot = snapshot - self.diff = diff - } - - public var revision: UInt32 { - return diff.revision + return changeActionsProto?.revision ?? snapshot!.revision } }