Don't cache CNContact.

Signal/src/ViewControllers/AddContactShareToExistingContactViewController.swift

@@ -55,24 +55,21 @@ class AddContactShareToExistingContactViewController: ContactsPicker, ContactsPi
         navigationController.popViewController(animated: true)
     }
 
-    func contactsPicker(_: ContactsPicker, didSelectContact newContact: Contact) {
+    func contactsPicker(_: ContactsPicker, didSelectContact oldContact: Contact) {
         Logger.debug("\(self.logTag) in \(#function)")
 
         let contactsManager = Environment.current().contactsManager
-        contactsManager?.cnContact(withId: self.contactShare.cnContactId,
-                                   success: { [weak self] oldCNContact in
-                                    contactsManager?.cnContact(withId: newContact.cnContactId,
-                                                               success: { newCNContact in
-                                                                guard let strongSelf = weakSelf else {
-                                                                    return
-                                                                }
-                                                                strongSelf.merge(oldCNContact: oldCNContact, newCNContact: newCNContact)
-                                    }, failure: {
-                                        // TODO: Alert?
-                                    })
-        }, failure: {
+        guard let oldCNContact = contactsManager?.cnContact(withId: oldContact.cnContactId) else {
             // TODO: Alert?
-        })
+            Logger.warn("\(logTag) could not load old CNContact.")
+            return
+        }
+        guard let newCNContact = OWSContacts.systemContact(for: self.contactShare.dbRecord, imageData: self.contactShare.avatarImageData) else {
+            // TODO: Alert?
+            Logger.warn("\(logTag) could not load new CNContact.")
+            return
+        }
+        merge(oldCNContact: oldCNContact, newCNContact: newCNContact)
     }
 
     func merge(oldCNContact: CNContact, newCNContact: CNContact) {

Signal/src/ViewControllers/ContactsPicker.swift

@@ -237,7 +237,7 @@ public class ContactsPicker: OWSViewController, UITableViewDelegate, UITableView
         let contact = Contact(systemContact: cnContact)
 
         cell.configure(contact: contact, subtitleType: subtitleCellType, showsWhenSelected: self.allowsMultipleSelection, contactsManager: self.contactsManager)
-        let isSelected = selectedContacts.contains(where: { $0.uniqueId == contact.uniqueId })
+        let isSelected = selectedContacts.contains(where: { $0.cnContactId == contact.cnContactId })
         cell.isSelected = isSelected
 
         // Make sure we preserve selection across tableView.reloadData which happens when toggling between 
@@ -256,7 +256,7 @@ public class ContactsPicker: OWSViewController, UITableViewDelegate, UITableView
         let deselectedContact = cell.contact!
 
         selectedContacts = selectedContacts.filter {
-            return $0.uniqueId != deselectedContact.uniqueId
+            return $0.cnContactId != deselectedContact.cnContactId
         }
     }
 

Signal/src/ViewControllers/ConversationView/ConversationViewController.m

@@ -5118,13 +5118,18 @@ interactionControllerForAnimationController:(id<UIViewControllerAnimatedTransiti
 - (void)contactsPicker:(ContactsPicker *)contactsPicker didSelectContact:(Contact *)contact
 {
     OWSAssert(contact);
-    OWSAssert(contact.cnContact);
+
+    CNContact *_Nullable cnContact = [self.contactsManager cnContactWithId:contact.cnContactId];
+    if (!cnContact) {
+        OWSFail(@"%@ Could not load system contact.", self.logTag);
+        return;
+    }
 
     DDLogDebug(@"%@ in %s with contact: %@", self.logTag, __PRETTY_FUNCTION__, contact);
 
-    OWSContact *_Nullable contactShareRecord = [OWSContacts contactForSystemContact:contact.cnContact];
+    OWSContact *_Nullable contactShareRecord = [OWSContacts contactForSystemContact:cnContact];
     if (!contactShareRecord) {
-        DDLogError(@"%@ Could not convert system contact.", self.logTag);
+        OWSFail(@"%@ Could not convert system contact.", self.logTag);
         return;
     }
 

Signal/src/ViewControllers/ThreadSettings/OWSAddToContactViewController.m

@@ -161,8 +161,6 @@ NS_ASSUME_NONNULL_BEGIN
         @"EDIT_GROUP_CONTACTS_SECTION_TITLE", @"a title for the contacts section of the 'new/update group' view.");
 
     for (Contact *contact in self.contactsViewHelper.contactsManager.allContacts) {
-        OWSAssert(contact.cnContact);
-
         NSString *_Nullable displayName = [self displayNameForContact:contact];
         if (displayName.length < 1) {
             continue;
@@ -198,10 +196,14 @@ NS_ASSUME_NONNULL_BEGIN
         OWSFail(@"%@ Contact editing not supported", self.logTag);
         return;
     }
+    CNContact *_Nullable cnContact = [self.contactsManager cnContactWithId:contact.cnContactId];
+    if (!cnContact) {
+        // TODO:
+    }
     [self.contactsViewHelper presentContactViewControllerForRecipientId:self.recipientId
                                                      fromViewController:self
                                                         editImmediately:YES
-                                                 addToExistingCnContact:contact.cnContact];
+                                                 addToExistingCnContact:cnContact];
 }
 
 @end

Signal/src/views/ContactCell.swift

@@ -74,7 +74,8 @@ class ContactCell: UITableViewCell {
         self.contact = contact
         self.showsWhenSelected = showsWhenSelected
 
-        titleLabel.attributedText = contact.cnContact?.formattedFullName(font: titleLabel.font)
+        let cnContact = contactsManager.cnContact(withId: contact.cnContactId)
+        titleLabel.attributedText = cnContact?.formattedFullName(font: titleLabel.font)
         updateSubtitle(subtitleType: subtitleType, contact: contact)
 
         if let contactImage = contact.image {

SignalMessaging/ViewModels/ContactShareViewModel.swift

@@ -148,21 +148,6 @@ public class ContactShareViewModel: NSObject {
         return dbRecord.isProfileAvatar
     }
 
-//    @objc
-//    public func cnContact(mergedWithExistingContact existingContact: Contact) -> CNContact? {
-//    public func cnContact(mergedWithExistingContact existingContact: Contact) -> CNContact? {
-////        success successParam: @escaping (CNContact) -> Void,
-////        failure failureParam: @escaping () -> Void) {
-//
-//        guard let newCNContact = OWSContacts.systemContact(for: self.dbRecord, imageData: self.avatarImageData) else {
-//            owsFail("\(logTag) in \(#function) newCNContact was unexpectedly nil")
-//            return nil
-//        }
-//
-//        let mergedCNContact = Contact.merge(cnContact: <#T##CNContact#>, newCNContact: <#T##CNContact#>)
-//        return existingContact.buildCNContact(mergedWithNewContact: newCNContact)
-//    }
-
     @objc
     public func copy(withName name: OWSContactName) -> ContactShareViewModel {
 
@@ -181,5 +166,4 @@ public class ContactShareViewModel: NSObject {
         // If we want to keep the avatar image, the caller will need to re-apply it.
         return ContactShareViewModel(contactShareRecord: newDbRecord, avatarImageData: nil)
     }
-
 }

SignalMessaging/Views/ContactsViewHelper.m

@@ -355,7 +355,7 @@ NS_ASSUME_NONNULL_BEGIN
         cnContact = updatedContact;
     }
     if (signalAccount && !cnContact) {
-        cnContact = signalAccount.contact.cnContact;
+        cnContact = [self.contactsManager cnContactWithId:signalAccount.contact.cnContactId];
     }
     if (cnContact) {
         if (shouldEditImmediately) {

SignalMessaging/contacts/OWSContactsManager.h

@@ -9,9 +9,6 @@ NS_ASSUME_NONNULL_BEGIN
 
 extern NSString *const OWSContactsManagerSignalAccountsDidChangeNotification;
 
-typedef void (^CNContactFetchSuccess)(CNContact *cnContact);
-typedef void (^CNContactFetchFailure)(void);
-
 @class ImageCache;
 @class SignalAccount;
 @class UIFont;
@@ -62,9 +59,7 @@ typedef void (^CNContactFetchFailure)(void);
 // contacts haven't changed, and will clear out any stale cached SignalAccounts
 - (void)userRequestedSystemContactsRefreshWithCompletion:(void (^)(NSError *_Nullable error))completionHandler;
 
-- (void)cnContactWithId:(NSString *)contactId
-                success:(CNContactFetchSuccess)success
-                failure:(CNContactFetchFailure)failure;
+- (nullable CNContact *)cnContactWithId:(nullable NSString *)contactId;
 
 #pragma mark - Util
 

SignalMessaging/contacts/OWSContactsManager.m

@@ -36,6 +36,7 @@ NSString *const OWSContactsManagerSignalAccountsDidChangeNotification
 @property (nonatomic, readonly) SystemContactsFetcher *systemContactsFetcher;
 @property (nonatomic, readonly) YapDatabaseConnection *dbReadConnection;
 @property (nonatomic, readonly) YapDatabaseConnection *dbWriteConnection;
+@property (nonatomic, readonly) NSCache<NSString *, CNContact *> *cnContactCache;
 
 @end
 
@@ -60,6 +61,8 @@ NSString *const OWSContactsManagerSignalAccountsDidChangeNotification
     _signalAccounts = @[];
     _systemContactsFetcher = [SystemContactsFetcher new];
     _systemContactsFetcher.delegate = self;
+    _cnContactCache = [NSCache new];
+    _cnContactCache.countLimit = 50;
 
     OWSSingletonAssert();
 
@@ -133,15 +136,24 @@ NSString *const OWSContactsManagerSignalAccountsDidChangeNotification
     return self.systemContactsFetcher.supportsContactEditing;
 }
 
-- (void)cnContactWithId:(NSString *)contactId
-                success:(CNContactFetchSuccess)success
-                failure:(CNContactFetchFailure)failure
+- (nullable CNContact *)cnContactWithId:(nullable NSString *)contactId
 {
     OWSAssert(contactId.length > 0);
-    OWSAssert(success);
-    OWSAssert(failure);
+    OWSAssert(self.cnContactCache);
 
-    return [self.systemContactsFetcher fetchCNContactWithContactId:contactId success:success failure:failure];
+    if (!contactId) {
+        return nil;
+    }
+
+    CNContact *_Nullable cnContact = [self.cnContactCache objectForKey:contactId];
+    if (!cnContact) {
+        cnContact = [self.systemContactsFetcher fetchCNContactWithContactId:contactId];
+        if (cnContact) {
+            [self.cnContactCache setObject:cnContact forKey:contactId];
+        }
+    }
+
+    return cnContact;
 }
 
 #pragma mark - SystemContactsFetcherDelegate
@@ -595,7 +607,8 @@ NSString *const OWSContactsManagerSignalAccountsDidChangeNotification
         NSAttributedString *lastName =
             [[NSAttributedString alloc] initWithString:cachedLastName attributes:lastNameAttributes];
 
-        CNContact *_Nullable cnContact = self.allContactsMap[recipientId].cnContactForFormatting;
+        NSString *_Nullable cnContactId = self.allContactsMap[recipientId].cnContactId;
+        CNContact *_Nullable cnContact = [self cnContactWithId:cnContactId];
         if (!cnContact) {
             // If we don't have a CNContact for this recipient id, make one.
             // Presumably [CNContactFormatter nameOrderForContact:] tries

SignalMessaging/contacts/SystemContactsFetcher.swift

@@ -395,63 +395,15 @@ public class SystemContactsFetcher: NSObject {
     }
 
     @objc
-    public func fetchCNContact(contactId: String,
-                               success successParam: @escaping (CNContact) -> Void,
-                               failure failureParam: @escaping () -> Void) {
+    public func fetchCNContact(contactId: String) -> CNContact? {
         SwiftAssertIsOnMainThread(#function)
 
         guard authorizationStatus == .authorized else {
             Logger.error("\(logTag) contact fetch failed; no access.")
-            failureParam()
-            return
-        }
-
-        var backgroundTask: OWSBackgroundTask? = OWSBackgroundTask(label: "\(#function)", completionBlock: { [weak self] status in
-            SwiftAssertIsOnMainThread(#function)
-
-            guard status == .expired else {
-                return
-            }
-
-            guard let _ = self else {
-                return
-            }
-            Logger.error("background task time ran out before contact fetch completed.")
-        })
-
-        // Ensure success is invoked on main thread.
-        let success: (CNContact) -> Void = { contact in
-            DispatchMainThreadSafe({
-                successParam(contact)
-
-                assert(backgroundTask != nil)
-                backgroundTask = nil
-            })
-        }
-
-        // Ensure success is invoked on main thread.
-        let failure: () -> Void = {
-            DispatchMainThreadSafe({
-                failureParam()
-
-                assert(backgroundTask != nil)
-                backgroundTask = nil
-            })
+            return nil
         }
 
-        // Don't use the serial queue.
-        DispatchQueue.global().async {
-
-            Logger.info("\(self.logTag) fetching contact")
-
-            if let cnContact = self.contactStoreAdapter.fetchCNContact(contactId: contactId) {
-                Logger.info("\(self.logTag) contact found")
-                success(cnContact)
-            } else {
-                Logger.info("\(self.logTag) contact not found")
-                failure()
-            }
-        }
+        return contactStoreAdapter.fetchCNContact(contactId: contactId)
     }
 }
 

SignalServiceKit/src/Contacts/Contact.h

@@ -40,6 +40,7 @@ NS_ASSUME_NONNULL_BEGIN
 #if TARGET_OS_IOS
 
 - (instancetype)initWithSystemContact:(CNContact *)contact NS_AVAILABLE_IOS(9_0);
++ (nullable Contact *)contactWithVCardData:(NSData *)data;
 + (nullable CNContact *)cnContactWithVCardData:(NSData *)data;
 
 - (NSString *)nameForPhoneNumber:(NSString *)recipientId;
@@ -53,8 +54,6 @@ NS_ASSUME_NONNULL_BEGIN
 + (CNContact *)mergeCNContact:(CNContact *)oldCNContact
                  newCNContact:(CNContact *)newCNContact NS_SWIFT_NAME(merge(cnContact:newCNContact:));
 
-- (CNContact *)cnContactForFormatting;
-
 @end
 
 NS_ASSUME_NONNULL_END

SignalServiceKit/src/Contacts/Contact.m

@@ -113,6 +113,17 @@ NS_ASSUME_NONNULL_BEGIN
     return self;
 }
 
++ (nullable Contact *)contactWithVCardData:(NSData *)data
+{
+    CNContact *_Nullable cnContact = [self cnContactWithVCardData:data];
+
+    if (!cnContact) {
+        return nil;
+    }
+
+    return [[self alloc] initWithSystemContact:cnContact];
+}
+
 - (nullable UIImage *)image
 {
     if (_image) {
@@ -409,14 +420,6 @@ NS_ASSUME_NONNULL_BEGIN
     return localizedLabel;
 }
 
-- (CNContact *)cnContactForFormatting
-{
-    CNMutableContact *cnContact = [CNMutableContact new];
-    cnContact.givenName = self.firstName;
-    cnContact.familyName = self.lastName;
-    return cnContact;
-}
-
 @end
 
 NS_ASSUME_NONNULL_END