diff --git a/SignalServiceKit/src/Contacts/Contact.m b/SignalServiceKit/src/Contacts/Contact.m index e461824da..5f6265a64 100644 --- a/SignalServiceKit/src/Contacts/Contact.m +++ b/SignalServiceKit/src/Contacts/Contact.m @@ -212,8 +212,9 @@ NS_ASSUME_NONNULL_BEGIN __block NSMutableArray *result = [NSMutableArray array]; for (PhoneNumber *number in [self.parsedPhoneNumbers sortedArrayUsingSelector:@selector(compare:)]) { - SignalRecipient *_Nullable signalRecipient = - [SignalRecipient registeredRecipientForRecipientId:number.toE164 transaction:transaction]; + SignalRecipient *_Nullable signalRecipient = [SignalRecipient registeredRecipientForRecipientId:number.toE164 + mustHaveDevices:YES + transaction:transaction]; if (signalRecipient) { [result addObject:signalRecipient]; } diff --git a/SignalServiceKit/src/Contacts/ContactsUpdater.m b/SignalServiceKit/src/Contacts/ContactsUpdater.m index 16baf3511..287483558 100644 --- a/SignalServiceKit/src/Contacts/ContactsUpdater.m +++ b/SignalServiceKit/src/Contacts/ContactsUpdater.m @@ -103,7 +103,7 @@ NS_ASSUME_NONNULL_BEGIN [SignalRecipient markRecipientAsRegisteredAndGet:recipientId transaction:transaction]; [recipients addObject:recipient]; } else { - [SignalRecipient removeUnregisteredRecipient:recipientId transaction:transaction]; + [SignalRecipient markRecipientAsUnregistered:recipientId transaction:transaction]; } } }]; diff --git a/SignalServiceKit/src/Contacts/SignalRecipient.h b/SignalServiceKit/src/Contacts/SignalRecipient.h index 9826f0475..6dcfb51a0 100644 --- a/SignalServiceKit/src/Contacts/SignalRecipient.h +++ b/SignalServiceKit/src/Contacts/SignalRecipient.h @@ -10,10 +10,11 @@ NS_ASSUME_NONNULL_BEGIN // // a) It serves as a cache of "known" Signal accounts. When the service indicates // that an account exists, we make sure that an instance of SignalRecipient exists -// for that recipient id (using mark as registered). -// When the service indicates that an account does not exist, we remove any -// SignalRecipient. -// b) We hang the "known device list" for known signal accounts on this entity. +// for that recipient id (using mark as registered) and has at least one device. +// When the service indicates that an account does not exist, we remove any devices +// from that SignalRecipient - but do not remove it from the database. +// Note that SignalRecipients without any devices are not considered registered. +//// b) We hang the "known device list" for known signal accounts on this entity. @interface SignalRecipient : TSYapDatabaseObject @property (nonatomic, readonly) NSOrderedSet *devices; @@ -21,6 +22,7 @@ NS_ASSUME_NONNULL_BEGIN - (instancetype)init NS_UNAVAILABLE; + (nullable instancetype)registeredRecipientForRecipientId:(NSString *)recipientId + mustHaveDevices:(BOOL)mustHaveDevices transaction:(YapDatabaseReadTransaction *)transaction; + (instancetype)getOrBuildUnsavedRecipientForRecipientId:(NSString *)recipientId transaction:(YapDatabaseReadTransaction *)transaction; @@ -40,7 +42,7 @@ NS_ASSUME_NONNULL_BEGIN + (void)markRecipientAsRegistered:(NSString *)recipientId deviceId:(UInt32)deviceId transaction:(YapDatabaseReadWriteTransaction *)transaction; -+ (void)removeUnregisteredRecipient:(NSString *)recipientId transaction:(YapDatabaseReadWriteTransaction *)transaction; ++ (void)markRecipientAsUnregistered:(NSString *)recipientId transaction:(YapDatabaseReadWriteTransaction *)transaction; @end diff --git a/SignalServiceKit/src/Contacts/SignalRecipient.m b/SignalServiceKit/src/Contacts/SignalRecipient.m index 3ce3bebb3..4ce907298 100644 --- a/SignalServiceKit/src/Contacts/SignalRecipient.m +++ b/SignalServiceKit/src/Contacts/SignalRecipient.m @@ -41,8 +41,9 @@ NS_ASSUME_NONNULL_BEGIN { OWSAssertDebug(transaction); OWSAssertDebug(recipientId.length > 0); - - SignalRecipient *_Nullable recipient = [self registeredRecipientForRecipientId:recipientId transaction:transaction]; + + SignalRecipient *_Nullable recipient = + [self registeredRecipientForRecipientId:recipientId mustHaveDevices:NO transaction:transaction]; if (!recipient) { recipient = [[self alloc] initWithTextSecureIdentifier:recipientId]; } @@ -95,14 +96,18 @@ NS_ASSUME_NONNULL_BEGIN return self; } - + (nullable instancetype)registeredRecipientForRecipientId:(NSString *)recipientId + mustHaveDevices:(BOOL)mustHaveDevices transaction:(YapDatabaseReadTransaction *)transaction { OWSAssertDebug(transaction); OWSAssertDebug(recipientId.length > 0); - return [self fetchObjectWithUniqueID:recipientId transaction:transaction]; + SignalRecipient *_Nullable signalRecipient = [self fetchObjectWithUniqueID:recipientId transaction:transaction]; + if (mustHaveDevices && signalRecipient.devices.count < 1) { + return nil; + } + return signalRecipient; } - (void)addDevices:(NSSet *)devices @@ -136,7 +141,7 @@ NS_ASSUME_NONNULL_BEGIN OWSAssertDebug(devicesToAdd.count > 0 || devicesToRemove.count > 0); // Add before we remove, since removeDevicesFromRecipient:... - // can removeUnregisteredRecipient:... if the recipient has + // can markRecipientAsUnregistered:... if the recipient has // no devices left. if (devicesToAdd.count > 0) { [self addDevicesToRegisteredRecipient:[NSSet setWithArray:devicesToAdd] transaction:transaction]; @@ -172,12 +177,7 @@ NS_ASSUME_NONNULL_BEGIN OWSLogDebug(@"removing devices: %@, from registered recipient: %@", devices, self); [self reloadWithTransaction:transaction]; [self removeDevices:devices]; - - if (self.devices.count > 0) { - [self saveWithTransaction_internal:transaction]; - } else { - [SignalRecipient removeUnregisteredRecipient:self.recipientId transaction:transaction]; - } + [self saveWithTransaction_internal:transaction]; } - (NSString *)recipientId @@ -190,6 +190,19 @@ NS_ASSUME_NONNULL_BEGIN return [self.recipientId compare:other.recipientId]; } +- (void)removeWithTransaction:(YapDatabaseReadWriteTransaction *)transaction +{ + // We need to distinguish between "users we know to be unregistered" and + // "users whose registration status is unknown". The former correspond to + // instances of SignalRecipient with no devices. The latter do not + // correspond to an instance of SignalRecipient in the database (although + // they may correspond to an "unsaved" instance of SignalRecipient built + // by getOrBuildUnsavedRecipientForRecipientId. + OWSFailDebug(@"Don't call removeWithTransaction."); + + [super removeWithTransaction:transaction]; +} + - (void)saveWithTransaction:(YapDatabaseReadWriteTransaction *)transaction { // We only want to mutate the persisted SignalRecipients in the database @@ -199,7 +212,7 @@ NS_ASSUME_NONNULL_BEGIN // reflect "last known registration status". Forcing our codebase to // use those methods helps ensure that we update the cache deliberately. OWSFailDebug(@"Don't call saveWithTransaction from outside this class."); - + [self saveWithTransaction_internal:transaction]; } @@ -207,13 +220,12 @@ NS_ASSUME_NONNULL_BEGIN { [super saveWithTransaction:transaction]; - OWSLogVerbose(@"saved signal recipient: %@", self.recipientId); + OWSLogVerbose(@"saved signal recipient: %@ (%lu)", self.recipientId, (unsigned long) self.devices.count); } + (BOOL)isRegisteredRecipient:(NSString *)recipientId transaction:(YapDatabaseReadTransaction *)transaction { - SignalRecipient *_Nullable instance = [self registeredRecipientForRecipientId:recipientId transaction:transaction]; - return instance != nil; + return nil != [self registeredRecipientForRecipientId:recipientId mustHaveDevices:YES transaction:transaction]; } + (SignalRecipient *)markRecipientAsRegisteredAndGet:(NSString *)recipientId @@ -222,7 +234,8 @@ NS_ASSUME_NONNULL_BEGIN OWSAssertDebug(transaction); OWSAssertDebug(recipientId.length > 0); - SignalRecipient *_Nullable instance = [self registeredRecipientForRecipientId:recipientId transaction:transaction]; + SignalRecipient *_Nullable instance = + [self registeredRecipientForRecipientId:recipientId mustHaveDevices:YES transaction:transaction]; if (!instance) { OWSLogDebug(@"creating recipient: %@", recipientId); @@ -249,17 +262,18 @@ NS_ASSUME_NONNULL_BEGIN } } -+ (void)removeUnregisteredRecipient:(NSString *)recipientId transaction:(YapDatabaseReadWriteTransaction *)transaction ++ (void)markRecipientAsUnregistered:(NSString *)recipientId transaction:(YapDatabaseReadWriteTransaction *)transaction { OWSAssertDebug(transaction); OWSAssertDebug(recipientId.length > 0); - SignalRecipient *_Nullable instance = [self registeredRecipientForRecipientId:recipientId transaction:transaction]; - if (!instance) { - return; + SignalRecipient *instance = [self getOrBuildUnsavedRecipientForRecipientId:recipientId + transaction:transaction]; + OWSLogDebug(@"Marking recipient as not registered: %@", recipientId); + if (instance.devices.count > 0) { + [instance removeDevices:instance.devices.set]; } - OWSLogDebug(@"removing recipient: %@", recipientId); - [instance removeWithTransaction:transaction]; + [instance saveWithTransaction_internal:transaction]; } @end diff --git a/SignalServiceKit/src/Messages/OWSMessageManager.m b/SignalServiceKit/src/Messages/OWSMessageManager.m index 8e2180790..68a20d28c 100644 --- a/SignalServiceKit/src/Messages/OWSMessageManager.m +++ b/SignalServiceKit/src/Messages/OWSMessageManager.m @@ -1456,7 +1456,7 @@ NS_ASSUME_NONNULL_BEGIN // Consult the device list cache we use for message sending // whether or not we know about this linked device. SignalRecipient *_Nullable recipient = - [SignalRecipient registeredRecipientForRecipientId:localNumber transaction:transaction]; + [SignalRecipient registeredRecipientForRecipientId:localNumber mustHaveDevices:NO transaction:transaction]; if (!recipient) { OWSFailDebug(@"No local SignalRecipient."); } else { diff --git a/SignalServiceKit/src/Messages/OWSMessageSender.m b/SignalServiceKit/src/Messages/OWSMessageSender.m index c65d29bc0..b304d9605 100644 --- a/SignalServiceKit/src/Messages/OWSMessageSender.m +++ b/SignalServiceKit/src/Messages/OWSMessageSender.m @@ -817,7 +817,7 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; return; } - [SignalRecipient removeUnregisteredRecipient:recipient.recipientId transaction:transaction]; + [SignalRecipient markRecipientAsUnregistered:recipient.recipientId transaction:transaction]; [[TSInfoMessage userNotRegisteredMessageInThread:thread recipientId:recipient.recipientId] saveWithTransaction:transaction]; @@ -1056,7 +1056,11 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; OWSFailDebug(@"sync message has device messages for unknown secondary devices."); } } else { - OWSAssertDebug(deviceMessages.count > 0); + // This can happen for users who have unregistered. + // We still want to try sending to them in case they have re-registered. + if (deviceMessages.count < 1) { + OWSLogWarn(@"Message send attempt with no device messages."); + } } if (deviceMessages.count == 0) {