From 3e129edfcd0b75ccb4e08f5cc6bf76db0d02c955 Mon Sep 17 00:00:00 2001 From: Max Radermacher Date: Mon, 17 Oct 2022 15:29:42 -0700 Subject: [PATCH] Limit scope of undiscoverable gv2 phone numbers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If we’ve recently discovered that a phone number can’t be discovered, there isn’t much benefit to checking again. However, for other parts of the migration flow, we still want to consider such users. For example, sometimes we want to require all users to have a UUID to perform a migration. Before this change, we might return a false positive after performing a CDS lookup since we wouldn’t check whether or not recently undiscoverable users have a UUID. In another case, we’re intentionally excluding users without UUIDs. In this case, the end result is the same whether we drop them during the first pass or the second pass, but the intent is more clear if we drop all such users during the second pass. Lastly, we want to fetch profiles even for undiscoverable users, even though this operation is likely to fail. --- SignalMessaging/groups/GroupsV2Migration.swift | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/SignalMessaging/groups/GroupsV2Migration.swift b/SignalMessaging/groups/GroupsV2Migration.swift index 85d0591538..822aa99de9 100644 --- a/SignalMessaging/groups/GroupsV2Migration.swift +++ b/SignalMessaging/groups/GroupsV2Migration.swift @@ -245,10 +245,10 @@ fileprivate extension GroupsV2Migration { } let groupMembership = unmigratedState.groupThread.groupModel.groupMembership - let membersToMigrate = membersToTryToMigrate(groupMembership: groupMembership) return firstly(on: .global()) { () -> Promise in - let phoneNumbersWithoutUuids = membersToMigrate.compactMap { (address: SignalServiceAddress) -> String? in + let membersToFetchUuids = membersToTryToMigrate(groupMembership: groupMembership) + let phoneNumbersWithoutUuids = membersToFetchUuids.compactMap { (address: SignalServiceAddress) -> String? in if address.uuid != nil { return nil } @@ -271,7 +271,7 @@ fileprivate extension GroupsV2Migration { let membersToFetchProfiles = Self.databaseStorage.read { transaction in // Both the capability and a profile key are required to migrate // If a user doesn't have both, we need to refetch their profile - membersToMigrate.filter { address in + groupMembership.allMembersOfAnyKind.filter { address in let hasProfileKey = groupsV2.hasProfileKeyCredential( for: address, transaction: transaction) @@ -522,8 +522,7 @@ fileprivate extension GroupsV2Migration { // The group creator is an administrator; // the other members are normal users. var v2MembershipBuilder = GroupMembership.Builder() - let membersToMigrate = membersToTryToMigrate(groupMembership: v1GroupModel.groupMembership) - for address in membersToMigrate { + for address in v1GroupModel.groupMembership.allMembersOfAnyKind { if DebugFlags.groupsV2migrationsDropOtherMembers.get(), !address.isLocalAddress { Logger.warn("Dropping non-local user.") @@ -623,9 +622,6 @@ fileprivate extension GroupsV2Migration { let isGroupInProfileWhitelist = profileManager.isThread(inProfileWhitelist: groupThread, transaction: transaction) - let groupMembership = groupThread.groupModel.groupMembership - let membersToMigrate = membersToTryToMigrate(groupMembership: groupMembership) - // Inspect member list. // // The group creator is an administrator; @@ -633,7 +629,7 @@ fileprivate extension GroupsV2Migration { var membersWithoutUuids = [SignalServiceAddress]() var membersWithoutProfileKeys = [SignalServiceAddress]() var membersMigrated = [SignalServiceAddress]() - for address in membersToMigrate { + for address in groupThread.groupModel.groupMembership.allMembersOfAnyKind { if address.isEqualToAddress(localAddress) { continue }