diff --git a/Signal/src/ViewControllers/NewContactThreadViewController.m b/Signal/src/ViewControllers/NewContactThreadViewController.m
index 137228bcf..acd419397 100644
--- a/Signal/src/ViewControllers/NewContactThreadViewController.m
+++ b/Signal/src/ViewControllers/NewContactThreadViewController.m
@@ -148,7 +148,7 @@ NS_ASSUME_NONNULL_BEGIN
OWSAssert([NSThread isMainThread]);
[self.contactsViewHelper.contactsManager
- fetchSystemContactsIfAlreadyAuthorizedAndAlwaysNotifyWithCompletion:^(NSError *_Nullable error) {
+ userRequestedSystemContactsRefreshWithCompletion:^(NSError *_Nullable error) {
if (error) {
DDLogError(@"%@ refreshing contacts failed with error: %@", self.logTag, error);
}
diff --git a/Signal/src/contact/OWSContactsManager.h b/Signal/src/contact/OWSContactsManager.h
index 01d56dc35..2e829309d 100644
--- a/Signal/src/contact/OWSContactsManager.h
+++ b/Signal/src/contact/OWSContactsManager.h
@@ -54,9 +54,9 @@ extern NSString *const OWSContactsManagerSignalAccountsDidChangeNotification;
- (void)fetchSystemContactsOnceIfAlreadyAuthorized;
// This variant will fetch system contacts if contact access has already been granted,
-// but not prompt for contact access. Also, it will always fire a notification.
-- (void)fetchSystemContactsIfAlreadyAuthorizedAndAlwaysNotifyWithCompletion:
- (void (^)(NSError *_Nullable error))completionHandler;
+// but not prompt for contact access. Also, it will always notify delegates, even if
+// contacts haven't changed, and will clear out any stale cached SignalAccount's
+- (void)userRequestedSystemContactsRefreshWithCompletion:(void (^)(NSError *_Nullable error))completionHandler;
#pragma mark - Util
diff --git a/Signal/src/contact/OWSContactsManager.m b/Signal/src/contact/OWSContactsManager.m
index ac3c27a7a..05a037731 100644
--- a/Signal/src/contact/OWSContactsManager.m
+++ b/Signal/src/contact/OWSContactsManager.m
@@ -77,6 +77,17 @@ NSString *const OWSContactsManagerSignalAccountsDidChangeNotification
[self updateSignalAccounts:signalAccounts];
}
+- (dispatch_queue_t)serialQueue
+{
+ static dispatch_queue_t _serialQueue;
+ static dispatch_once_t onceToken;
+ dispatch_once(&onceToken, ^{
+ _serialQueue = dispatch_queue_create("org.whispersystems.contacts.buildSignalAccount", DISPATCH_QUEUE_SERIAL);
+ });
+
+ return _serialQueue;
+}
+
#pragma mark - System Contact Fetching
// Request contacts access if you haven't asked recently.
@@ -95,10 +106,9 @@ NSString *const OWSContactsManagerSignalAccountsDidChangeNotification
[self.systemContactsFetcher fetchOnceIfAlreadyAuthorized];
}
-- (void)fetchSystemContactsIfAlreadyAuthorizedAndAlwaysNotifyWithCompletion:
- (void (^)(NSError *_Nullable error))completionHandler
+- (void)userRequestedSystemContactsRefreshWithCompletion:(void (^)(NSError *_Nullable error))completionHandler
{
- [self.systemContactsFetcher fetchIfAlreadyAuthorizedAndAlwaysNotifyWithCompletion:completionHandler];
+ [self.systemContactsFetcher userRequestedRefreshWithCompletion:completionHandler];
}
- (BOOL)isSystemContactsAuthorized
@@ -120,8 +130,9 @@ NSString *const OWSContactsManagerSignalAccountsDidChangeNotification
- (void)systemContactsFetcher:(SystemContactsFetcher *)systemsContactsFetcher
updatedContacts:(NSArray<Contact *> *)contacts
+ userRequested:(BOOL)userRequested
{
- [self updateWithContacts:contacts];
+ [self updateWithContacts:contacts clearStaleCache:userRequested];
}
#pragma mark - Intersection
@@ -179,10 +190,9 @@ NSString *const OWSContactsManagerSignalAccountsDidChangeNotification
[self.avatarCache removeAllImagesForKey:recipientId];
}
-- (void)updateWithContacts:(NSArray<Contact *> *)contacts
+- (void)updateWithContacts:(NSArray<Contact *> *)contacts clearStaleCache:(BOOL)clearStaleCache
{
- dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
-
+ dispatch_async(self.serialQueue, ^{
NSMutableDictionary<NSString *, Contact *> *allContactsMap = [NSMutableDictionary new];
for (Contact *contact in contacts) {
for (PhoneNumber *phoneNumber in contact.parsedPhoneNumbers) {
@@ -200,22 +210,15 @@ NSString *const OWSContactsManagerSignalAccountsDidChangeNotification
[self.avatarCache removeAllImages];
[self intersectContactsWithCompletion:^(NSError *_Nullable error) {
- [self buildSignalAccounts];
+ [self buildSignalAccountsAndClearStaleCache:clearStaleCache];
}];
});
});
}
-- (void)buildSignalAccounts
+- (void)buildSignalAccountsAndClearStaleCache:(BOOL)clearStaleCache;
{
- // Ensure we're not running concurrently since one invocation could affect the other.
- static dispatch_queue_t _serialQueue;
- static dispatch_once_t onceToken;
- dispatch_once(&onceToken, ^{
- _serialQueue = dispatch_queue_create("org.whispersystems.contacts.buildSignalAccount", DISPATCH_QUEUE_SERIAL);
- });
-
- dispatch_async(_serialQueue, ^{
+ dispatch_async(self.serialQueue, ^{
NSMutableArray<SignalAccount *> *signalAccounts = [NSMutableArray new];
NSArray<Contact *> *contacts = self.allContacts;
@@ -291,16 +294,43 @@ NSString *const OWSContactsManagerSignalAccountsDidChangeNotification
// Update cached SignalAccounts on disk
[self.dbWriteConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *_Nonnull transaction) {
- DDLogInfo(@"%@ Saving %lu new SignalAccounts", self.logTag, (unsigned long)accountsToSave.count);
+ DDLogInfo(@"%@ Saving %lu SignalAccounts", self.logTag, (unsigned long)accountsToSave.count);
for (SignalAccount *signalAccount in accountsToSave) {
- DDLogVerbose(@"%@ Adding new SignalAccount: %@", self.logTag, signalAccount);
+ DDLogVerbose(@"%@ Saving SignalAccount: %@", self.logTag, signalAccount);
[signalAccount saveWithTransaction:transaction];
}
- DDLogInfo(@"%@ Removing %lu old SignalAccounts.", self.logTag, (unsigned long)oldSignalAccounts.count);
- for (SignalAccount *signalAccount in oldSignalAccounts.allValues) {
- DDLogVerbose(@"%@ Removing old SignalAccount: %@", self.logTag, signalAccount);
- [signalAccount removeWithTransaction:transaction];
+ if (clearStaleCache) {
+ DDLogInfo(@"%@ Removing %lu old SignalAccounts.", self.logTag, (unsigned long)oldSignalAccounts.count);
+ for (SignalAccount *signalAccount in oldSignalAccounts.allValues) {
+ DDLogVerbose(@"%@ Removing old SignalAccount: %@", self.logTag, signalAccount);
+ [signalAccount removeWithTransaction:transaction];
+ }
+ } else {
+ // In theory we want to remove SignalAccounts if the user deletes the corresponding system contact.
+ // However, as of iOS11.2 CNContactStore occasionally gives us only a subset of the system contacts.
+ // Because of that, it's not safe to clear orphaned accounts.
+ // Because we still want to give users a way to clear their stale accounts, if they pull-to-refresh
+ // their contacts we'll clear the cached ones.
+ // RADAR: https://bugreport.apple.com/web/?problemID=36082946
+ if (oldSignalAccounts.allValues.count > 0) {
+ DDLogWarn(@"%@ NOT Removing %lu old SignalAccounts.",
+ self.logTag,
+ (unsigned long)oldSignalAccounts.count);
+ for (SignalAccount *signalAccount in oldSignalAccounts.allValues) {
+ DDLogVerbose(
+ @"%@ Ensuring old SignalAccount is not inadvertently lost: %@", self.logTag, signalAccount);
+ [signalAccounts addObject:signalAccount];
+ }
+
+ // re-sort signal accounts since we've appended some orphans
+ [signalAccounts sortUsingComparator:^NSComparisonResult(SignalAccount *left, SignalAccount *right) {
+ NSString *leftName = [self comparableNameForSignalAccount:left];
+ NSString *rightName = [self comparableNameForSignalAccount:right];
+
+ return [leftName compare:rightName];
+ }];
+ }
}
}];
diff --git a/Signal/src/contact/SystemContactsFetcher.swift b/Signal/src/contact/SystemContactsFetcher.swift
index b1621c57f..41baba6b8 100644
--- a/Signal/src/contact/SystemContactsFetcher.swift
+++ b/Signal/src/contact/SystemContactsFetcher.swift
@@ -185,11 +185,11 @@ class AddressBookContactStoreAdaptee: ContactStoreAdaptee {
}
}
- return Contact(contactWithFirstName: firstName,
- andLastName: lastName,
- andUserTextPhoneNumbers: phoneNumbers,
- andImage: addressBookRecord.image,
- andContactID: addressBookRecord.recordId)
+ return Contact(firstName: firstName,
+ lastName: lastName,
+ userTextPhoneNumbers: phoneNumbers,
+ imageData: addressBookRecord.imageData,
+ contactID: addressBookRecord.recordId)
}
}
@@ -243,14 +243,14 @@ struct OWSABRecord {
}
}
- var image: UIImage? {
+ var imageData: Data? {
guard ABPersonHasImageData(abRecord) else {
return nil
}
guard let data = ABPersonCopyImageData(abRecord)?.takeRetainedValue() else {
return nil
}
- return UIImage(data: data as Data)
+ return data as Data
}
private func extractProperty<T>(_ propertyName: ABPropertyID) -> T? {
@@ -316,7 +316,7 @@ class ContactStoreAdapter: ContactStoreAdaptee {
}
@objc protocol SystemContactsFetcherDelegate: class {
- func systemContactsFetcher(_ systemContactsFetcher: SystemContactsFetcher, updatedContacts contacts: [Contact])
+ func systemContactsFetcher(_ systemContactsFetcher: SystemContactsFetcher, updatedContacts contacts: [Contact], userRequested: Bool)
}
@objc
@@ -361,7 +361,7 @@ class SystemContactsFetcher: NSObject {
hasSetupObservation = true
self.contactStoreAdapter.startObservingChanges { [weak self] in
DispatchQueue.main.async {
- self?.updateContacts(completion: nil, alwaysNotify: false)
+ self?.updateContacts(completion: nil, userRequested: false)
}
}
}
@@ -431,19 +431,20 @@ class SystemContactsFetcher: NSObject {
return
}
- updateContacts(completion: nil, alwaysNotify: false)
+ updateContacts(completion: nil, userRequested: false)
}
- public func fetchIfAlreadyAuthorizedAndAlwaysNotify(completion: ((Error?) -> Void)?) {
+ public func userRequestedRefresh(completion: @escaping (Error?) -> Void) {
AssertIsOnMainThread()
guard authorizationStatus == .authorized else {
+ owsFail("should have already requested contact access")
return
}
- updateContacts(completion: completion, alwaysNotify: true)
+ updateContacts(completion: completion, userRequested: true)
}
- private func updateContacts(completion completionParam: ((Error?) -> Void)?, alwaysNotify: Bool = false) {
+ private func updateContacts(completion completionParam: ((Error?) -> Void)?, userRequested: Bool = false) {
AssertIsOnMainThread()
// Ensure completion is invoked on main thread.
@@ -483,8 +484,8 @@ class SystemContactsFetcher: NSObject {
if self.lastContactUpdateHash != contactsHash {
Logger.info("\(self.TAG) contact hash changed. new contactsHash: \(contactsHash)")
shouldNotifyDelegate = true
- } else if alwaysNotify {
- Logger.info("\(self.TAG) ignoring debounce.")
+ } else if userRequested {
+ Logger.info("\(self.TAG) ignoring debounce due to user request")
shouldNotifyDelegate = true
} else {
@@ -516,7 +517,7 @@ class SystemContactsFetcher: NSObject {
self.lastDelegateNotificationDate = Date()
self.lastContactUpdateHash = contactsHash
- self.delegate?.systemContactsFetcher(self, updatedContacts: contacts)
+ self.delegate?.systemContactsFetcher(self, updatedContacts: contacts, userRequested: userRequested)
completion(nil)
}
}
diff --git a/SignalServiceKit/src/Contacts/Contact.h b/SignalServiceKit/src/Contacts/Contact.h
index 94fbf42b4..8c7729aec 100644
--- a/SignalServiceKit/src/Contacts/Contact.h
+++ b/SignalServiceKit/src/Contacts/Contact.h
@@ -44,11 +44,11 @@ NS_ASSUME_NONNULL_BEGIN
#if TARGET_OS_IOS
-- (instancetype)initWithContactWithFirstName:(nullable NSString *)firstName
- andLastName:(nullable NSString *)lastName
- andUserTextPhoneNumbers:(NSArray<NSString *> *)phoneNumbers
- andImage:(nullable UIImage *)image
- andContactID:(ABRecordID)record;
+- (instancetype)initWithFirstName:(nullable NSString *)firstName
+ lastName:(nullable NSString *)lastName
+ userTextPhoneNumbers:(NSArray<NSString *> *)phoneNumbers
+ imageData:(nullable NSData *)imageData
+ contactID:(ABRecordID)record;
- (instancetype)initWithSystemContact:(CNContact *)contact;
diff --git a/SignalServiceKit/src/Contacts/Contact.m b/SignalServiceKit/src/Contacts/Contact.m
index af98a4cdc..c0c291c16 100644
--- a/SignalServiceKit/src/Contacts/Contact.m
+++ b/SignalServiceKit/src/Contacts/Contact.m
@@ -27,13 +27,14 @@ NS_ASSUME_NONNULL_BEGIN
@synthesize fullName = _fullName;
@synthesize comparableNameFirstLast = _comparableNameFirstLast;
@synthesize comparableNameLastFirst = _comparableNameLastFirst;
+@synthesize image = _image;
#if TARGET_OS_IOS
-- (instancetype)initWithContactWithFirstName:(nullable NSString *)firstName
- andLastName:(nullable NSString *)lastName
- andUserTextPhoneNumbers:(NSArray *)phoneNumbers
- andImage:(nullable UIImage *)image
- andContactID:(ABRecordID)record
+- (instancetype)initWithFirstName:(nullable NSString *)firstName
+ lastName:(nullable NSString *)lastName
+ userTextPhoneNumbers:(NSArray<NSString *> *)phoneNumbers
+ imageData:(nullable NSData *)imageData
+ contactID:(ABRecordID)record
{
self = [super init];
if (!self) {
@@ -47,7 +48,7 @@ NS_ASSUME_NONNULL_BEGIN
_userTextPhoneNumbers = phoneNumbers;
_phoneNumberNameMap = [NSMutableDictionary new];
_parsedPhoneNumbers = [self parsedPhoneNumbersFromUserTextPhoneNumbers:phoneNumbers phoneNumberNameMap:@{}];
- _image = image;
+ _imageData = imageData;
// Not using emails for old AB style contacts.
_emails = [NSMutableArray new];
@@ -128,27 +129,24 @@ NS_ASSUME_NONNULL_BEGIN
_emails = [emailAddresses copy];
if (contact.thumbnailImageData) {
- _image = [UIImage imageWithData:contact.thumbnailImageData];
+ _imageData = contact.thumbnailImageData;
}
return self;
}
-- (instancetype)initWithCoder:(NSCoder *)coder
+- (nullable UIImage *)image
{
- self = [super initWithCoder:coder];
- if (_imageData) {
- _image = [UIImage imageWithData:_imageData];
+ if (_image) {
+ return _image;
}
- return self;
-}
-- (void)encodeWithCoder:(NSCoder *)coder
-{
- if (_image) {
- _imageData = UIImagePNGRepresentation(_image);
+ if (!self.imageData) {
+ return nil;
}
- [super encodeWithCoder:coder];
+
+ _image = [UIImage imageWithData:self.imageData];
+ return _image;
}
- (NSString *)trimName:(NSString *)name