signalapp/Signal-iOS
signalapp/Signal-iOS
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases 4 Wiki Activity

IOS-445: Do CDS lookup during message send for recipients without UUID

Browse Source 
This adds a new async operation in the message send flow. MessageSender
will now verify that the intended recipients of a message all have UUIDs
before proceeding. If any recipients are missing a UUID, it'll kick off
a CDS lookup for the invalid recipients.

While this code appears to work, this is a first pass. Looking to get
some inital feedback in a pull request. It's unclear to me whether or
not this pattern fits in with the existing error propogation flow in
MessagSender promises. It's also unclear whether or not the error I'm
using is correct.

Some other changes that come along for the ride:
- Add a -perform() method to ContactDiscoveryService to automatically
  submit itself and its dependencies to its static operation queue
This commit is contained in:
Michelle Linington 2020-07-14 19:12:21 -07:00
parent 6e0b388834
commit b276e8eb02
5 changed files with 97 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
SignalServiceKit/src/Contacts/OWSContactDiscoveryOperation.swift
View File

@ -95,8 +95,7 @@ public class LegacyContactDiscoveryOperation: OWSOperation {
// Compare against new CDS service
let modernContactDiscoveryOperation = ContactDiscoveryOperation(phoneNumbersToLookup: self.phoneNumbersToLookup)
let operations = modernContactDiscoveryOperation.dependencies + [modernContactDiscoveryOperation]
ContactDiscoveryOperation.operationQueue.addOperations(operations, waitUntilFinished: false)
modernContactDiscoveryOperation.perform()
guard let legacyRegisteredPhoneNumbers = self.registeredPhoneNumbers else {
owsFailDebug("legacyRegisteredPhoneNumbers was unexpectedly nil")
@ -187,6 +186,12 @@ public class ContactDiscoveryOperation: OWSOperation {
}
}
/// Asynchronously start the operation and its dependencies
@objc public func perform() {
let operationSet = self.dependencies + [self]
Self.operationQueue.addOperations(operationSet, waitUntilFinished: false)
}
// MARK: Mandatory overrides
// Called every retry, this is where the bulk of the operation's work should go.

3
SignalServiceKit/src/Contacts/UUIDBackfillTask.swift
View File

@ -210,8 +210,7 @@ extension UUIDBackfillTask {
operation.completionBlock = {
completion(operation.registeredContacts, operation.failingError)
}
ContactDiscoveryOperation.operationQueue.addOperations(operation.dependencies, waitUntilFinished: false)
ContactDiscoveryOperation.operationQueue.addOperation(operation)
operation.perform()
}
}

56
SignalServiceKit/src/Messages/OWSMessageSender.m
View File

@ -683,22 +683,6 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException";
return recipientAddresses.allObjects;
}
- (NSArray<SignalRecipient *> *)recipientsForAddresses:(NSArray<SignalServiceAddress *> *)addresses
{
OWSAssertDebug(!NSThread.isMainThread);
OWSAssertDebug(addresses.count > 0);
NSMutableArray<SignalRecipient *> *recipients = [NSMutableArray new];
[self.databaseStorage readWithBlock:^(SDSAnyReadTransaction *transaction) {
for (SignalServiceAddress *address in addresses) {
SignalRecipient *recipient = [SignalRecipient getOrBuildUnsavedRecipientForAddress:address
transaction:transaction];
[recipients addObject:recipient];
}
}];
return [recipients copy];
}
- (AnyPromise *)unlockPreKeyUpdateFailuresPromise
{
OWSAssertDebug(!NSThread.isMainThread);
@ -742,11 +726,23 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException";
OWSAssertDebug(message);
OWSAssertDebug(thread);
BOOL disallowUuidlessRecipients = SSKFeatureFlags.useOnlyModernContactDiscovery;
NSMutableArray *validRecipients = [[NSMutableArray alloc] init];
NSMutableArray *invalidRecipients = [[NSMutableArray alloc] init];
for (SignalRecipient *recipient in recipients) {
if (recipient.address.uuid == nil && disallowUuidlessRecipients) {
[invalidRecipients addObject:recipient];
} else {
[validRecipients addObject:recipient];
}
}
// 1. gather "ud sending access" using a single write transaction.
NSMutableDictionary<SignalServiceAddress *, OWSUDSendingAccess *> *sendingAccessMap = [NSMutableDictionary new];
if (senderCertificate != nil) {
if (senderCertificate != nil && validRecipients.count > 0) {
DatabaseStorageWrite(self.databaseStorage, ^(SDSAnyWriteTransaction *transaction) {
for (SignalRecipient *recipient in recipients) {
for (SignalRecipient *recipient in validRecipients) {
if (!recipient.address.isLocalAddress) {
sendingAccessMap[recipient.address] = [self.udManager udSendingAccessForAddress:recipient.address
requireSyncAccess:YES
@ -759,7 +755,7 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException";
// 2. Build a "OWSMessageSend" for each recipient.
NSMutableArray<OWSMessageSend *> *messageSends = [NSMutableArray new];
for (SignalRecipient *recipient in recipients) {
for (SignalRecipient *recipient in validRecipients) {
OWSUDSendingAccess *_Nullable udSendingAccess = sendingAccessMap[recipient.address];
OWSMessageSend *messageSend = [[OWSMessageSend alloc] initWithMessage:message
thread:thread
@ -777,17 +773,28 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException";
return
[MessageSending ensureSessionsforMessageSendsObjc:messageSends ignoreErrors:YES].thenInBackground(^(id value) {
// 4. Perform the per-recipient message sends.
NSMutableArray<AnyPromise *> *sendPromises = [NSMutableArray array];
NSMutableArray<AnyPromise *> *allPromises = [NSMutableArray array];
for (OWSMessageSend *messageSend in messageSends) {
[self sendMessageToRecipient:messageSend];
[sendPromises addObject:messageSend.asAnyPromise];
[allPromises addObject:messageSend.asAnyPromise];
}
// 5. Fail any message sends to invalid recipients
for (SignalRecipient *invalidRecipient in invalidRecipients) {
NSError *invalidRecipientError = OWSErrorMakeNoSuchSignalRecipientError();
// Retryable because we can refetch from CDS
[invalidRecipientError setIsRetryable:YES];
sendErrorBlock(invalidRecipient, invalidRecipientError);
AnyPromise *failedPromise = [AnyPromise promiseWithValue:invalidRecipientError];
[allPromises addObject:failedPromise];
}
// We use PMKJoin(), not PMKWhen(), because we don't want the
// completion promise to execute until _all_ send promises
// have either succeeded or failed. PMKWhen() executes as
// soon as any of its input promises fail.
return PMKJoin(sendPromises);
return PMKJoin(allPromises);
});
}
@ -897,14 +904,15 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException";
return;
}
NSArray<SignalRecipient *> *recipients = [self recipientsForAddresses:recipientAddresses];
BOOL isGroupSend = thread.isGroupThread;
NSMutableArray<NSError *> *sendErrors = [NSMutableArray array];
NSMutableDictionary<SignalServiceAddress *, NSError *> *sendErrorPerRecipient = [NSMutableDictionary dictionary];
[self unlockPreKeyUpdateFailuresPromise]
.thenInBackground(^(id value) {
return [self recipientPromiseForAddressesObjcWithAddressList:recipientAddresses];
})
.thenInBackground(^(NSArray<SignalRecipient *> *recipients) {
return [self
sendPromiseForRecipients:recipients
message:message

56
SignalServiceKit/src/Messages/OWSMessageSender.swift
View File

@ -474,3 +474,59 @@ fileprivate extension MessageSending {
}
}
}
// MARK: - SignalServiceAddress fetching
extension MessageSender {
@objc func recipientPromiseForAddressesObjc(addressList: [SignalServiceAddress]) -> AnyPromise {
return AnyPromise(recipientPromiseForAddresses(addressList: addressList))
}
func recipientPromiseForAddresses(addressList: [SignalServiceAddress]) -> Promise<[SignalRecipient]> {
return firstly { () -> Promise<[SignalServiceAddress]> in
Promise { resolver in
let invalidAddresses = addressList.filter { $0.uuid == nil }
if invalidAddresses.count > 0 && FeatureFlags.useOnlyModernContactDiscovery {
// If we have any invalid addresses, block on a one-shot CDS fetch to get their UUIDs
// We'll fail the send later for any UUID-less addresses
let phoneNumbersToFetch = invalidAddresses.compactMap { $0.phoneNumber }
let operation = ContactDiscoveryOperation(phoneNumbersToLookup: phoneNumbersToFetch)
operation.completionBlock = {
let discoveredContactMap = operation.registeredContacts.reduce(into: [:], { (builder, contact) in
builder[contact.e164PhoneNumber] = contact.signalUuid
})
let rawNumberToUUIDMap: [String: UUID] = operation.phoneNumbersToLookup.reduce(into: [:], { (builder, rawNumber) in
if let e164 = PhoneNumber.tryParsePhoneNumber(fromUserSpecifiedText: rawNumber)?.toE164() {
builder[rawNumber] = discoveredContactMap[e164]
}
})
let updatedAddressList = addressList.map { (address: SignalServiceAddress) -> SignalServiceAddress in
guard address.uuid == nil,
let phoneNumber = address.phoneNumber,
let uuid = rawNumberToUUIDMap[phoneNumber]
else { return address }
return SignalServiceAddress(uuid: uuid, phoneNumber: phoneNumber)
}
resolver.fulfill(updatedAddressList)
}
operation.perform()
} else {
resolver.fulfill(addressList)
}
}
}.map { (addresses: [SignalServiceAddress]) -> [SignalRecipient] in
return SDSDatabaseStorage.shared.read { (readTx) in
addresses.map {
SignalRecipient.getOrBuildUnsavedRecipient(for: $0, transaction: readTx)
}
}
}
}
}

1
SignalServiceKit/src/SignalServiceKit.h
View File

@ -28,6 +28,7 @@
#import <SignalServiceKit/OWSIncomingContactSyncJobRecord.h>
#import <SignalServiceKit/OWSIncomingGroupSyncJobRecord.h>
#import <SignalServiceKit/OWSMessageReceiver.h>
#import <SignalServiceKit/OWSMessageSender.h>
#import <SignalServiceKit/OWSOperation.h>
#import <SignalServiceKit/OWSOutgoingSyncMessage.h>
#import <SignalServiceKit/OWSReaction.h>