Merge branch 'charlesmchen/lazyContact'

Signal/src/ViewControllers/AddContactShareToExistingContactViewController.swift

@@ -55,17 +55,29 @@ class AddContactShareToExistingContactViewController: ContactsPicker, ContactsPi
         navigationController.popViewController(animated: true)
     }
 
-    func contactsPicker(_: ContactsPicker, didSelectContact contact: Contact) {
+    func contactsPicker(_: ContactsPicker, didSelectContact oldContact: Contact) {
         Logger.debug("\(self.logTag) in \(#function)")
 
-        guard let mergedContact: CNContact = self.contactShare.cnContact(mergedWithExistingContact: contact) else {
+        let contactsManager = Environment.current().contactsManager
-            owsFail("\(logTag) in \(#function) mergedContact was unexpectedly nil")
+        guard let oldCNContact = contactsManager?.cnContact(withId: oldContact.cnContactId) else {
+            owsFail("\(logTag) could not load old CNContact.")
             return
         }
+        guard let newCNContact = OWSContacts.systemContact(for: self.contactShare.dbRecord, imageData: self.contactShare.avatarImageData) else {
+            owsFail("\(logTag) could not load new CNContact.")
+            return
+        }
+        merge(oldCNContact: oldCNContact, newCNContact: newCNContact)
+    }
+
+    func merge(oldCNContact: CNContact, newCNContact: CNContact) {
+        Logger.debug("\(self.logTag) in \(#function)")
+
+        let mergedCNContact: CNContact = Contact.merge(cnContact: oldCNContact, newCNContact: newCNContact)
 
         // Not actually a "new" contact, but this brings up the edit form rather than the "Read" form
         // saving our users a tap in some cases when we already know they want to edit.
-        let contactViewController: CNContactViewController = CNContactViewController(forNewContact: mergedContact)
+        let contactViewController: CNContactViewController = CNContactViewController(forNewContact: mergedCNContact)
 
         // Default title is "New Contact". We could give a more descriptive title, but anything
         // seems redundant - the context is sufficiently clear.

Signal/src/ViewControllers/ConversationView/ConversationViewController.m

@@ -5118,18 +5118,23 @@ 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;
     }
 
     BOOL isProfileAvatar = NO;
-    NSData *_Nullable avatarImageData = contact.imageData;
+    NSData *_Nullable avatarImageData = [self.contactsManager avatarDataForCNContactId:cnContact.identifier];
     for (NSString *recipientId in contact.textSecureIdentifiers) {
         if (avatarImageData) {
             break;

Signal/src/ViewControllers/DebugUI/DebugUIContacts.m

@@ -1,5 +1,5 @@
 //
-//  Copyright (c) 2017 Open Whisper Systems. All rights reserved.
+//  Copyright (c) 2018 Open Whisper Systems. All rights reserved.
 //
 
 #import "DebugUIContacts.h"
@@ -1164,7 +1164,7 @@ NS_ASSUME_NONNULL_BEGIN
     OWSAssert(count > 0);
     OWSAssert(batchCompletionHandler);
 
-    DDLogDebug(@"createRandomContactsBatch: %zd", count);
+    DDLogDebug(@"createRandomContactsBatch: %zu", count);
 
     CNAuthorizationStatus status = [CNContactStore authorizationStatusForEntityType:CNEntityTypeContacts];
     if (status == CNAuthorizationStatusDenied || status == CNAuthorizationStatusRestricted) {
@@ -1219,7 +1219,7 @@ NS_ASSUME_NONNULL_BEGIN
                          }
                      }
 
-                     DDLogError(@"Saving fake contacts: %zd", contacts.count);
+                     DDLogError(@"Saving fake contacts: %zu", contacts.count);
 
                      NSError *saveError = nil;
                      if (![store executeSaveRequest:request error:&saveError]) {

Signal/src/ViewControllers/DebugUI/DebugUIMisc.m

@@ -133,6 +133,11 @@ NS_ASSUME_NONNULL_BEGIN
                             }
                         }]];
 
+    [items addObject:[OWSTableItem itemWithTitle:@"Fetch system contacts"
+                                     actionBlock:^() {
+                                         [Environment.current.contactsManager requestSystemContactsOnce];
+                                     }]];
+
     return [OWSTableSection sectionWithTitle:self.name items:items];
 }
 

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,15 @@ NS_ASSUME_NONNULL_BEGIN
         OWSFail(@"%@ Contact editing not supported", self.logTag);
         return;
     }
+    CNContact *_Nullable cnContact = [self.contactsManager cnContactWithId:contact.cnContactId];
+    if (!cnContact) {
+        OWSFail(@"%@ Could not load system contact.", self.logTag);
+        return;
+    }
     [self.contactsViewHelper presentContactViewControllerForRecipientId:self.recipientId
                                                      fromViewController:self
                                                         editImmediately:YES
-                                                 addToExistingCnContact:contact.cnContact];
+                                                 addToExistingCnContact:cnContact];
 }
 
 @end

Signal/src/views/ContactCell.swift

@@ -74,10 +74,11 @@ 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 {
+        if let contactImage = contactsManager.avatarImage(forCNContactId: contact.cnContactId) {
             contactImageView.image = contactImage
         } else {
             let contactIdForDeterminingBackgroundColor: String

SignalMessaging/ViewModels/ContactShareViewModel.swift

@@ -148,17 +148,6 @@ public class ContactShareViewModel: NSObject {
         return dbRecord.isProfileAvatar
     }
 
-    @objc
-    public func cnContact(mergedWithExistingContact existingContact: Contact) -> CNContact? {
-
-        guard let newCNContact = OWSContacts.systemContact(for: self.dbRecord, imageData: self.avatarImageData) else {
-            owsFail("\(logTag) in \(#function) newCNContact was unexpectedly nil")
-            return nil
-        }
-
-        return existingContact.buildCNContact(mergedWithNewContact: newCNContact)
-    }
-
     @objc
     public func copy(withName name: OWSContactName) -> ContactShareViewModel {
 
@@ -177,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/attachments/SharingThreadPickerViewController.m

@@ -149,17 +149,45 @@ typedef void (^SendMessageBlock)(SendCompletionBlock completion);
     self.thread = thread;
 
     if (self.attachment.isConvertibleToContactShare) {
+        [self showContactShareApproval];
+        return;
+    }
+
+    NSString *_Nullable messageText = [self convertAttachmentToMessageTextIfPossible];
+
+    if (messageText) {
+        MessageApprovalViewController *approvalVC =
+            [[MessageApprovalViewController alloc] initWithMessageText:messageText
+                                                                thread:thread
+                                                       contactsManager:self.contactsManager
+                                                              delegate:self];
+
+        [self.navigationController pushViewController:approvalVC animated:YES];
+    } else {
+        OWSNavigationController *approvalModal =
+            [AttachmentApprovalViewController wrappedInNavControllerWithAttachment:self.attachment delegate:self];
+        [self presentViewController:approvalModal animated:YES completion:nil];
+    }
+}
+
+- (void)showContactShareApproval
+{
+    OWSAssert(self.attachment);
+    OWSAssert(self.thread);
+    OWSAssert(self.attachment.isConvertibleToContactShare);
+
     NSData *data = self.attachment.data;
 
-        Contact *_Nullable contact = [Contact contactWithVCardData:data];
+    CNContact *_Nullable cnContact = [Contact cnContactWithVCardData:data];
-        OWSContact *_Nullable contactShareRecord = [OWSContacts contactForSystemContact:contact.cnContact];
+    Contact *_Nullable contact = [[Contact alloc] initWithSystemContact:cnContact];
+    OWSContact *_Nullable contactShareRecord = [OWSContacts contactForSystemContact:cnContact];
     if (!contactShareRecord) {
         DDLogError(@"%@ Could not convert system contact.", self.logTag);
         return;
     }
 
     BOOL isProfileAvatar = NO;
-        NSData *_Nullable avatarImageData = contact.imageData;
+    NSData *_Nullable avatarImageData = [self.contactsManager avatarDataForCNContactId:contact.cnContactId];
     for (NSString *recipientId in contact.textSecureIdentifiers) {
         if (avatarImageData) {
             break;
@@ -172,32 +200,13 @@ typedef void (^SendMessageBlock)(SendCompletionBlock completion);
     contactShareRecord.isProfileAvatar = isProfileAvatar;
 
     ContactShareViewModel *contactShare =
-            [[ContactShareViewModel alloc] initWithContactShareRecord:contactShareRecord
+        [[ContactShareViewModel alloc] initWithContactShareRecord:contactShareRecord avatarImageData:avatarImageData];
-                                                      avatarImageData:avatarImageData];
 
     ContactShareApprovalViewController *approvalVC =
         [[ContactShareApprovalViewController alloc] initWithContactShare:contactShare
                                                          contactsManager:self.contactsManager
                                                                 delegate:self];
     [self.navigationController pushViewController:approvalVC animated:YES];
-        return;
-    }
-
-    NSString *_Nullable messageText = [self convertAttachmentToMessageTextIfPossible];
-
-    if (messageText) {
-        MessageApprovalViewController *approvalVC =
-            [[MessageApprovalViewController alloc] initWithMessageText:messageText
-                                                                thread:thread
-                                                       contactsManager:self.contactsManager
-                                                              delegate:self];
-
-        [self.navigationController pushViewController:approvalVC animated:YES];
-    } else {
-        OWSNavigationController *approvalModal =
-            [AttachmentApprovalViewController wrappedInNavControllerWithAttachment:self.attachment delegate:self];
-        [self presentViewController:approvalModal animated:YES completion:nil];
-    }
 }
 
 // override

SignalMessaging/contacts/OWSContactsManager.m

@@ -36,6 +36,8 @@ 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;
+@property (nonatomic, readonly) NSCache<NSString *, UIImage *> *cnContactAvatarCache;
 
 @end
 
@@ -60,6 +62,10 @@ NSString *const OWSContactsManagerSignalAccountsDidChangeNotification
     _signalAccounts = @[];
     _systemContactsFetcher = [SystemContactsFetcher new];
     _systemContactsFetcher.delegate = self;
+    _cnContactCache = [NSCache new];
+    _cnContactCache.countLimit = 50;
+    _cnContactAvatarCache = [NSCache new];
+    _cnContactAvatarCache.countLimit = 25;
 
     OWSSingletonAssert();
 
@@ -133,6 +139,63 @@ NSString *const OWSContactsManagerSignalAccountsDidChangeNotification
     return self.systemContactsFetcher.supportsContactEditing;
 }
 
+#pragma mark - CNContacts
+
+- (nullable CNContact *)cnContactWithId:(nullable NSString *)contactId
+{
+    OWSAssert(contactId.length > 0);
+    OWSAssert(self.cnContactCache);
+
+    if (!contactId) {
+        return nil;
+    }
+
+    CNContact *_Nullable cnContact;
+    @synchronized(self.cnContactCache) {
+        cnContact = [self.cnContactCache objectForKey:contactId];
+        if (!cnContact) {
+            cnContact = [self.systemContactsFetcher fetchCNContactWithContactId:contactId];
+            if (cnContact) {
+                [self.cnContactCache setObject:cnContact forKey:contactId];
+            }
+        }
+    }
+
+    return cnContact;
+}
+
+- (nullable NSData *)avatarDataForCNContactId:(nullable NSString *)contactId
+{
+    // Don't bother to cache avatar data.
+    CNContact *_Nullable cnContact = [self cnContactWithId:contactId];
+    return [Contact avatarDataForCNContact:cnContact];
+}
+
+- (nullable UIImage *)avatarImageForCNContactId:(nullable NSString *)contactId
+{
+    OWSAssert(self.cnContactAvatarCache);
+
+    if (!contactId) {
+        return nil;
+    }
+
+    UIImage *_Nullable avatarImage;
+    @synchronized(self.cnContactAvatarCache) {
+        avatarImage = [self.cnContactAvatarCache objectForKey:contactId];
+        if (!avatarImage) {
+            NSData *_Nullable avatarData = [self avatarDataForCNContactId:contactId];
+            if (avatarData) {
+                avatarImage = [UIImage imageWithData:avatarData];
+            }
+            if (avatarImage) {
+                [self.cnContactAvatarCache setObject:avatarImage forKey:contactId];
+            }
+        }
+    }
+
+    return avatarImage;
+}
+
 #pragma mark - SystemContactsFetcherDelegate
 
 - (void)systemContactsFetcher:(SystemContactsFetcher *)systemsContactsFetcher
@@ -232,6 +295,8 @@ NSString *const OWSContactsManagerSignalAccountsDidChangeNotification
         dispatch_async(dispatch_get_main_queue(), ^{
             self.allContacts = contacts;
             self.allContactsMap = [allContactsMap copy];
+            [self.cnContactCache removeAllObjects];
+            [self.cnContactAvatarCache removeAllObjects];
 
             [self.avatarCache removeAllImages];
 
@@ -584,7 +649,8 @@ NSString *const OWSContactsManagerSignalAccountsDidChangeNotification
         NSAttributedString *lastName =
             [[NSAttributedString alloc] initWithString:cachedLastName attributes:lastNameAttributes];
 
-        CNContact *_Nullable cnContact = self.allContactsMap[recipientId].cnContact;
+        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
@@ -765,7 +831,7 @@ NSString *const OWSContactsManagerSignalAccountsDidChangeNotification
         contact = [self signalAccountForRecipientId:identifier].contact;
     }
 
-    return contact.image;
+    return [self avatarImageForCNContactId:contact.cnContactId];
 }
 
 - (nullable UIImage *)profileImageForPhoneIdentifier:(nullable NSString *)identifier

SignalMessaging/contacts/SystemContactsFetcher.swift

@@ -17,12 +17,12 @@ protocol ContactStoreAdaptee {
     var supportsContactEditing: Bool { get }
     func requestAccess(completionHandler: @escaping (Bool, Error?) -> Void)
     func fetchContacts() -> Result<[Contact], Error>
+    func fetchCNContact(contactId: String) -> CNContact?
     func startObservingChanges(changeHandler: @escaping () -> Void)
 }
 
 public
-class ContactsFrameworkContactStoreAdaptee: ContactStoreAdaptee {
+class ContactsFrameworkContactStoreAdaptee: NSObject, ContactStoreAdaptee {
-    let TAG = "[ContactsFrameworkContactStoreAdaptee]"
     private let contactStore = CNContactStore()
     private var changeHandler: (() -> Void)?
     private var initializedObserver = false
@@ -72,7 +72,7 @@ class ContactsFrameworkContactStoreAdaptee: ContactStoreAdaptee {
                 return
             }
 
-            Logger.info("\(self.TAG) sort order changed: \(String(describing: self.lastSortOrder)) -> \(String(describing: currentSortOrder))")
+            Logger.info("\(self.logTag) sort order changed: \(String(describing: self.lastSortOrder)) -> \(String(describing: currentSortOrder))")
             self.lastSortOrder = currentSortOrder
             self.runChangeHandler()
         }
@@ -81,7 +81,7 @@ class ContactsFrameworkContactStoreAdaptee: ContactStoreAdaptee {
     @objc
     func runChangeHandler() {
         guard let changeHandler = self.changeHandler else {
-            owsFail("\(TAG) trying to run change handler before it was registered")
+            owsFail("\(self.logTag) trying to run change handler before it was registered")
             return
         }
         changeHandler()
@@ -100,13 +100,35 @@ class ContactsFrameworkContactStoreAdaptee: ContactStoreAdaptee {
                 systemContacts.append(contact)
             }
         } catch let error as NSError {
-            owsFail("\(self.TAG) Failed to fetch contacts with error:\(error)")
+            owsFail("\(self.logTag) Failed to fetch contacts with error:\(error)")
             return .error(error)
         }
 
         let contacts = systemContacts.map { Contact(systemContact: $0) }
         return .success(contacts)
     }
+
+    func fetchCNContact(contactId: String) -> CNContact? {
+        var result: CNContact?
+        do {
+            let contactFetchRequest = CNContactFetchRequest(keysToFetch: ContactsFrameworkContactStoreAdaptee.allowedContactKeys)
+            contactFetchRequest.sortOrder = .userDefault
+            contactFetchRequest.predicate = CNContact.predicateForContacts(withIdentifiers: [contactId])
+
+            try self.contactStore.enumerateContacts(with: contactFetchRequest) { (contact, _) -> Void in
+                guard result == nil else {
+                    owsFail("\(self.logTag) More than one contact with contact id.")
+                    return
+                }
+                result = contact
+            }
+        } catch let error as NSError {
+            owsFail("\(self.logTag) Failed to fetch contact with error:\(error)")
+            return nil
+        }
+
+        return result
+    }
 }
 
 @objc
@@ -125,8 +147,6 @@ public enum ContactStoreAuthorizationStatus: UInt {
 @objc
 public class SystemContactsFetcher: NSObject {
 
-    private let TAG = "[SystemContactsFetcher]"
-
     private let serialQueue = DispatchQueue(label: "SystemContactsFetcherQueue")
 
     var lastContactUpdateHash: Int?
@@ -207,20 +227,20 @@ public class SystemContactsFetcher: NSObject {
         switch authorizationStatus {
         case .notDetermined:
             if CurrentAppContext().isInBackground() {
-                Logger.error("\(self.TAG) do not request contacts permission when app is in background")
+                Logger.error("\(self.logTag) do not request contacts permission when app is in background")
                 completion(nil)
                 return
             }
             self.contactStoreAdapter.requestAccess { (granted, error) in
                 if let error = error {
-                    Logger.error("\(self.TAG) error fetching contacts: \(error)")
+                    Logger.error("\(self.logTag) error fetching contacts: \(error)")
                     completion(error)
                     return
                 }
 
                 guard granted else {
                     // This case should have been caught by the error guard a few lines up.
-                    owsFail("\(self.TAG) declined contact access.")
+                    owsFail("\(self.logTag) declined contact access.")
                     completion(nil)
                     return
                 }
@@ -232,7 +252,7 @@ public class SystemContactsFetcher: NSObject {
         case .authorized:
             self.updateContacts(completion: completion)
         case .denied, .restricted:
-            Logger.debug("\(TAG) contacts were \(self.authorizationStatus)")
+            Logger.debug("\(logTag) contacts were \(self.authorizationStatus)")
             self.delegate?.systemContactsFetcher(self, hasAuthorizationStatus: authorizationStatus)
             completion(nil)
         }
@@ -292,7 +312,7 @@ public class SystemContactsFetcher: NSObject {
             guard let _ = self else {
                 return
             }
-            Logger.error("background task time ran out contacts fetch completed.")
+            Logger.error("background task time ran out before contacts fetch completed.")
         })
 
         // Ensure completion is invoked on main thread.
@@ -310,7 +330,7 @@ public class SystemContactsFetcher: NSObject {
 
         serialQueue.async {
 
-            Logger.info("\(self.TAG) fetching contacts")
+            Logger.info("\(self.logTag) fetching contacts")
 
             var fetchedContacts: [Contact]?
             switch self.contactStoreAdapter.fetchContacts() {
@@ -322,21 +342,21 @@ public class SystemContactsFetcher: NSObject {
             }
 
             guard let contacts = fetchedContacts else {
-                owsFail("\(self.TAG) contacts was unexpectedly not set.")
+                owsFail("\(self.logTag) contacts was unexpectedly not set.")
                 completion(nil)
             }
 
-            Logger.info("\(self.TAG) fetched \(contacts.count) contacts.")
+            Logger.info("\(self.logTag) fetched \(contacts.count) contacts.")
             let contactsHash  = HashableArray(contacts).hashValue
 
             DispatchQueue.main.async {
                 var shouldNotifyDelegate = false
 
                 if self.lastContactUpdateHash != contactsHash {
-                    Logger.info("\(self.TAG) contact hash changed. new contactsHash: \(contactsHash)")
+                    Logger.info("\(self.logTag) contact hash changed. new contactsHash: \(contactsHash)")
                     shouldNotifyDelegate = true
                 } else if isUserRequested {
-                    Logger.info("\(self.TAG) ignoring debounce due to user request")
+                    Logger.info("\(self.logTag) ignoring debounce due to user request")
                     shouldNotifyDelegate = true
                 } else {
 
@@ -346,19 +366,19 @@ public class SystemContactsFetcher: NSObject {
 
                         let expiresAtDate = Date(timeInterval: kDebounceInterval, since: lastDelegateNotificationDate)
                         if  Date() > expiresAtDate {
-                            Logger.info("\(self.TAG) debounce interval expired at: \(expiresAtDate)")
+                            Logger.info("\(self.logTag) debounce interval expired at: \(expiresAtDate)")
                             shouldNotifyDelegate = true
                         } else {
-                            Logger.info("\(self.TAG) ignoring since debounce interval hasn't expired")
+                            Logger.info("\(self.logTag) ignoring since debounce interval hasn't expired")
                         }
                     } else {
-                        Logger.info("\(self.TAG) first contact fetch. contactsHash: \(contactsHash)")
+                        Logger.info("\(self.logTag) first contact fetch. contactsHash: \(contactsHash)")
                         shouldNotifyDelegate = true
                     }
                 }
 
                 guard shouldNotifyDelegate else {
-                    Logger.info("\(self.TAG) no reason to notify delegate.")
+                    Logger.info("\(self.logTag) no reason to notify delegate.")
 
                     completion(nil)
 
@@ -373,6 +393,18 @@ public class SystemContactsFetcher: NSObject {
             }
         }
     }
+
+    @objc
+    public func fetchCNContact(contactId: String) -> CNContact? {
+        SwiftAssertIsOnMainThread(#function)
+
+        guard authorizationStatus == .authorized else {
+            Logger.error("\(logTag) contact fetch failed; no access.")
+            return nil
+        }
+
+        return contactStoreAdapter.fetchCNContact(contactId: contactId)
+    }
 }
 
 struct HashableArray<Element: Hashable>: Hashable {

SignalMessaging/utils/LRUCache.swift

@@ -5,7 +5,7 @@
 @objc
 public class AnyLRUCache: NSObject {
 
-    let backingCache: LRUCache<NSObject, NSObject>
+    private let backingCache: LRUCache<NSObject, NSObject>
 
     @objc
     public init(maxSize: Int) {
@@ -21,29 +21,63 @@ public class AnyLRUCache: NSObject {
     public func set(key: NSObject, value: NSObject) {
         self.backingCache.set(key: key, value: value)
     }
+
+    @objc
+    public func clear() {
+        self.backingCache.clear()
+    }
 }
 
 // A simple LRU cache bounded by the number of entries.
-//
-// TODO: We might want to observe memory pressure notifications.
 public class LRUCache<KeyType: Hashable & Equatable, ValueType> {
 
     private var cacheMap: [KeyType: ValueType] = [:]
     private var cacheOrder: [KeyType] = []
     private let maxSize: Int
 
+    @objc
     public init(maxSize: Int) {
         self.maxSize = maxSize
+
+        NotificationCenter.default.addObserver(self,
+                                               selector: #selector(didReceiveMemoryWarning),
+                                               name: NSNotification.Name.UIApplicationDidReceiveMemoryWarning,
+                                               object: nil)
+        NotificationCenter.default.addObserver(self,
+                                               selector: #selector(didEnterBackground),
+                                               name: NSNotification.Name.OWSApplicationDidEnterBackground,
+                                               object: nil)
+    }
+
+    deinit {
+        NotificationCenter.default.removeObserver(self)
+    }
+
+    @objc func didEnterBackground() {
+        SwiftAssertIsOnMainThread(#function)
+
+        clear()
+    }
+
+    @objc func didReceiveMemoryWarning() {
+        SwiftAssertIsOnMainThread(#function)
+
+        clear()
+    }
+
+    private func updateCacheOrder(key: KeyType) {
+        cacheOrder = cacheOrder.filter { $0 != key }
+        cacheOrder.append(key)
     }
 
     public func get(key: KeyType) -> ValueType? {
         guard let value = cacheMap[key] else {
+            // Miss
             return nil
         }
 
-        // Update cache order.
+        // Hit
-        cacheOrder = cacheOrder.filter { $0 != key }
+        updateCacheOrder(key: key)
-        cacheOrder.append(key)
 
         return value
     }
@@ -51,9 +85,7 @@ public class LRUCache<KeyType: Hashable & Equatable, ValueType> {
     public func set(key: KeyType, value: ValueType) {
         cacheMap[key] = value
 
-        // Update cache order.
+        updateCacheOrder(key: key)
-        cacheOrder = cacheOrder.filter { $0 != key }
-        cacheOrder.append(key)
 
         while cacheOrder.count > maxSize {
             guard let staleKey = cacheOrder.first else {
@@ -64,4 +96,10 @@ public class LRUCache<KeyType: Hashable & Equatable, ValueType> {
             cacheMap.removeValue(forKey: staleKey)
         }
     }
+
+    @objc
+    public func clear() {
+        cacheMap.removeAll()
+        cacheOrder.removeAll()
+    }
 }

SignalServiceKit/src/Contacts/Contact.h

@@ -28,11 +28,7 @@ NS_ASSUME_NONNULL_BEGIN
 @property (readonly, nonatomic) NSArray<NSString *> *emails;
 @property (readonly, nonatomic) NSString *uniqueId;
 @property (nonatomic, readonly) BOOL isSignalContact;
-#if TARGET_OS_IOS
+@property (nonatomic, readonly) NSString *cnContactId;
-@property (nullable, readonly, nonatomic) UIImage *image;
-@property (nullable, readonly, nonatomic) NSData *imageData;
-@property (nullable, nonatomic, readonly) CNContact *cnContact;
-#endif // TARGET_OS_IOS
 
 - (NSArray<SignalRecipient *> *)signalRecipientsWithTransaction:(YapDatabaseReadTransaction *)transaction;
 // TODO: Remove this method.
@@ -40,8 +36,9 @@ NS_ASSUME_NONNULL_BEGIN
 
 #if TARGET_OS_IOS
 
-- (instancetype)initWithSystemContact:(CNContact *)contact NS_AVAILABLE_IOS(9_0);
+- (instancetype)initWithSystemContact:(CNContact *)cnContact NS_AVAILABLE_IOS(9_0);
 + (nullable Contact *)contactWithVCardData:(NSData *)data;
++ (nullable CNContact *)cnContactWithVCardData:(NSData *)data;
 
 - (NSString *)nameForPhoneNumber:(NSString *)recipientId;
 
@@ -51,7 +48,10 @@ NS_ASSUME_NONNULL_BEGIN
 + (NSString *)formattedFullNameWithCNContact:(CNContact *)cnContact NS_SWIFT_NAME(formattedFullName(cnContact:));
 + (nullable NSString *)localizedStringForCNLabel:(nullable NSString *)cnLabel;
 
-- (CNContact *)buildCNContactMergedWithNewContact:(CNContact *)newCNContact NS_SWIFT_NAME(buildCNContact(mergedWithNewContact:));
++ (CNContact *)mergeCNContact:(CNContact *)oldCNContact
+                 newCNContact:(CNContact *)newCNContact NS_SWIFT_NAME(merge(cnContact:newCNContact:));
+
++ (nullable NSData *)avatarDataForCNContact:(nullable CNContact *)cnContact;
 
 @end
 

SignalServiceKit/src/Contacts/Contact.m

@@ -9,6 +9,7 @@
 #import "PhoneNumber.h"
 #import "SignalRecipient.h"
 #import "TSAccountManager.h"
+#import "TextSecureKitEnv.h"
 
 @import Contacts;
 
@@ -16,7 +17,8 @@ NS_ASSUME_NONNULL_BEGIN
 
 @interface Contact ()
 
-@property (readonly, nonatomic) NSMutableDictionary<NSString *, NSString *> *phoneNumberNameMap;
+@property (nonatomic, readonly) NSMutableDictionary<NSString *, NSString *> *phoneNumberNameMap;
+@property (nonatomic, readonly) NSUInteger imageHash;
 
 @end
 
@@ -26,26 +28,24 @@ NS_ASSUME_NONNULL_BEGIN
 
 @synthesize comparableNameFirstLast = _comparableNameFirstLast;
 @synthesize comparableNameLastFirst = _comparableNameLastFirst;
-@synthesize image = _image;
 
 #if TARGET_OS_IOS
 
-- (instancetype)initWithSystemContact:(CNContact *)contact
+- (instancetype)initWithSystemContact:(CNContact *)cnContact
 {
     self = [super init];
     if (!self) {
         return self;
     }
 
-    _cnContact = contact;
+    _cnContactId = cnContact.identifier;
-    _firstName = contact.givenName.ows_stripped;
+    _firstName = cnContact.givenName.ows_stripped;
-    _lastName = contact.familyName.ows_stripped;
+    _lastName = cnContact.familyName.ows_stripped;
-    _fullName = [Contact formattedFullNameWithCNContact:contact];
+    _fullName = [Contact formattedFullNameWithCNContact:cnContact];
-    _uniqueId = contact.identifier;
 
     NSMutableArray<NSString *> *phoneNumbers = [NSMutableArray new];
     NSMutableDictionary<NSString *, NSString *> *phoneNumberNameMap = [NSMutableDictionary new];
-    for (CNLabeledValue *phoneNumberField in contact.phoneNumbers) {
+    for (CNLabeledValue *phoneNumberField in cnContact.phoneNumbers) {
         if ([phoneNumberField.value isKindOfClass:[CNPhoneNumber class]]) {
             CNPhoneNumber *phoneNumber = (CNPhoneNumber *)phoneNumberField.value;
             [phoneNumbers addObject:phoneNumber.stringValue];
@@ -97,23 +97,31 @@ NS_ASSUME_NONNULL_BEGIN
         [self parsedPhoneNumbersFromUserTextPhoneNumbers:phoneNumbers phoneNumberNameMap:phoneNumberNameMap];
 
     NSMutableArray<NSString *> *emailAddresses = [NSMutableArray new];
-    for (CNLabeledValue *emailField in contact.emailAddresses) {
+    for (CNLabeledValue *emailField in cnContact.emailAddresses) {
         if ([emailField.value isKindOfClass:[NSString class]]) {
             [emailAddresses addObject:(NSString *)emailField.value];
         }
     }
     _emails = [emailAddresses copy];
 
-    if (contact.thumbnailImageData) {
+    NSData *_Nullable avatarData = [Contact avatarDataForCNContact:cnContact];
-        _imageData = [contact.thumbnailImageData copy];
+    if (avatarData) {
-    } else if (contact.imageData) {
+        NSUInteger hashValue = 0;
-        // This only occurs when sharing a contact via the share extension
+        NSData *hashData = [Cryptography computeSHA256Digest:avatarData truncatedToBytes:sizeof(hashValue)];
-        _imageData = [contact.imageData copy];
+        [hashData getBytes:&hashValue length:sizeof(hashValue)];
+        _imageHash = hashValue;
+    } else {
+        _imageHash = 0;
     }
 
     return self;
 }
 
+- (NSString *)uniqueId
+{
+    return self.cnContactId;
+}
+
 + (nullable Contact *)contactWithVCardData:(NSData *)data
 {
     CNContact *_Nullable cnContact = [self cnContactWithVCardData:data];
@@ -125,29 +133,6 @@ NS_ASSUME_NONNULL_BEGIN
     return [[self alloc] initWithSystemContact:cnContact];
 }
 
-- (nullable UIImage *)image
-{
-    if (_image) {
-        return _image;
-    }
-
-    if (!self.imageData) {
-        return nil;
-    }
-
-    _image = [UIImage imageWithData:self.imageData];
-    return _image;
-}
-
-+ (MTLPropertyStorage)storageBehaviorForPropertyWithKey:(NSString *)propertyKey
-{
-    if ([propertyKey isEqualToString:@"cnContact"] || [propertyKey isEqualToString:@"image"]) {
-        return MTLPropertyStorageTransitory;
-    } else {
-        return [super storageBehaviorForPropertyWithKey:propertyKey];
-    }
-}
-
 #endif // TARGET_OS_IOS
 
 - (NSArray<PhoneNumber *> *)parsedPhoneNumbersFromUserTextPhoneNumbers:(NSArray<NSString *> *)userTextPhoneNumbers
@@ -278,6 +263,20 @@ NS_ASSUME_NONNULL_BEGIN
     return value;
 }
 
++ (nullable NSData *)avatarDataForCNContact:(nullable CNContact *)cnContact
+{
+    if (cnContact.thumbnailImageData) {
+        return cnContact.thumbnailImageData.copy;
+    } else if (cnContact.imageData) {
+        // This only occurs when sharing a contact via the share extension
+        return cnContact.imageData.copy;
+    } else {
+        return nil;
+    }
+}
+
+// This method is used to de-bounce system contact fetch notifications
+// by checking for changes in the contact data.
 - (NSUInteger)hash
 {
     // base hash is some arbitrary number
@@ -285,13 +284,7 @@ NS_ASSUME_NONNULL_BEGIN
 
     hash = hash ^ self.fullName.hash;
 
-    if (self.imageData) {
+    hash = hash ^ self.imageHash;
-        NSUInteger thumbnailHash = 0;
-        NSData *thumbnailHashData =
-            [Cryptography computeSHA256Digest:self.imageData truncatedToBytes:sizeof(thumbnailHash)];
-        [thumbnailHashData getBytes:&thumbnailHash length:sizeof(thumbnailHash)];
-        hash = hash ^ thumbnailHash;
-    }
 
     for (PhoneNumber *phoneNumber in self.parsedPhoneNumbers) {
         hash = hash ^ phoneNumber.toE164.hash;
@@ -326,9 +319,14 @@ NS_ASSUME_NONNULL_BEGIN
     return contacts.firstObject;
 }
 
-- (CNContact *)buildCNContactMergedWithNewContact:(CNContact *)newCNContact
++ (CNContact *)mergeCNContact:(CNContact *)oldCNContact newCNContact:(CNContact *)newCNContact
 {
-    CNMutableContact *_Nullable mergedCNContact = [self.cnContact mutableCopy];
+    OWSAssert(oldCNContact);
+    OWSAssert(newCNContact);
+
+    Contact *oldContact = [[Contact alloc] initWithSystemContact:oldCNContact];
+
+    CNMutableContact *_Nullable mergedCNContact = [oldCNContact mutableCopy];
     if (!mergedCNContact) {
         OWSFail(@"%@ in %s mergedCNContact was unexpectedly nil", self.logTag, __PRETTY_FUNCTION__);
         return [CNContact new];
@@ -351,8 +349,8 @@ NS_ASSUME_NONNULL_BEGIN
     }
 
     // Phone Numbers
-    NSSet<PhoneNumber *> *existingParsedPhoneNumberSet = [NSSet setWithArray:self.parsedPhoneNumbers];
+    NSSet<PhoneNumber *> *existingParsedPhoneNumberSet = [NSSet setWithArray:oldContact.parsedPhoneNumbers];
-    NSSet<NSString *> *existingUnparsedPhoneNumberSet = [NSSet setWithArray:self.userTextPhoneNumbers];
+    NSSet<NSString *> *existingUnparsedPhoneNumberSet = [NSSet setWithArray:oldContact.userTextPhoneNumbers];
 
     NSMutableArray<CNLabeledValue<CNPhoneNumber *> *> *mergedPhoneNumbers = [mergedCNContact.phoneNumbers mutableCopy];
     for (CNLabeledValue<CNPhoneNumber *> *labeledPhoneNumber in newCNContact.phoneNumbers) {
@@ -371,7 +369,7 @@ NS_ASSUME_NONNULL_BEGIN
     mergedCNContact.phoneNumbers = mergedPhoneNumbers;
     
     // Emails
-    NSSet<NSString *> *existingEmailSet = [NSSet setWithArray:self.emails];
+    NSSet<NSString *> *existingEmailSet = [NSSet setWithArray:oldContact.emails];
     NSMutableArray<CNLabeledValue<NSString *> *> *mergedEmailAddresses = [mergedCNContact.emailAddresses mutableCopy];
     for (CNLabeledValue<NSString *> *labeledEmail in newCNContact.emailAddresses) {
         NSString *normalizedValue = labeledEmail.value.ows_stripped;

SignalServiceKit/src/Devices/OWSContactsOutputStream.h

@@ -1,19 +1,22 @@
 //
-//  Copyright (c) 2017 Open Whisper Systems. All rights reserved.
+//  Copyright (c) 2018 Open Whisper Systems. All rights reserved.
 //
 
 #import "OWSChunkedOutputStream.h"
 
 NS_ASSUME_NONNULL_BEGIN
 
-@class SignalAccount;
 @class OWSRecipientIdentity;
+@class SignalAccount;
+
+@protocol ContactsManagerProtocol;
 
 @interface OWSContactsOutputStream : OWSChunkedOutputStream
 
 - (void)writeSignalAccount:(SignalAccount *)signalAccount
          recipientIdentity:(nullable OWSRecipientIdentity *)recipientIdentity
-            profileKeyData:(nullable NSData *)profileKeyData;
+            profileKeyData:(nullable NSData *)profileKeyData
+           contactsManager:(id<ContactsManagerProtocol>)contactsManager;
 
 @end
 

SignalServiceKit/src/Devices/OWSContactsOutputStream.m

@@ -4,6 +4,7 @@
 
 #import "OWSContactsOutputStream.h"
 #import "Contact.h"
+#import "ContactsManagerProtocol.h"
 #import "Cryptography.h"
 #import "MIMETypeUtil.h"
 #import "NSData+keyVersionByte.h"
@@ -22,9 +23,11 @@ NS_ASSUME_NONNULL_BEGIN
 - (void)writeSignalAccount:(SignalAccount *)signalAccount
          recipientIdentity:(nullable OWSRecipientIdentity *)recipientIdentity
             profileKeyData:(nullable NSData *)profileKeyData
+           contactsManager:(id<ContactsManagerProtocol>)contactsManager
 {
     OWSAssert(signalAccount);
     OWSAssert(signalAccount.contact);
+    OWSAssert(contactsManager);
 
     OWSSignalServiceProtosContactDetailsBuilder *contactBuilder = [OWSSignalServiceProtosContactDetailsBuilder new];
     [contactBuilder setName:signalAccount.contact.fullName];
@@ -38,16 +41,19 @@ NS_ASSUME_NONNULL_BEGIN
         contactBuilder.verifiedBuilder = verifiedBuilder;
     }
 
-    NSData *avatarPng;
+    UIImage *_Nullable rawAvatar = [contactsManager avatarImageForCNContactId:signalAccount.contact.cnContactId];
-    if (signalAccount.contact.image) {
+    NSData *_Nullable avatarPng;
+    if (rawAvatar) {
+        avatarPng = UIImagePNGRepresentation(rawAvatar);
+        if (avatarPng) {
             OWSSignalServiceProtosContactDetailsAvatarBuilder *avatarBuilder =
                 [OWSSignalServiceProtosContactDetailsAvatarBuilder new];
 
             [avatarBuilder setContentType:OWSMimeTypeImagePng];
-        avatarPng = UIImagePNGRepresentation(signalAccount.contact.image);
             [avatarBuilder setLength:(uint32_t)avatarPng.length];
             [contactBuilder setAvatarBuilder:avatarBuilder];
         }
+    }
 
     if (profileKeyData) {
         OWSAssert(profileKeyData.length == kAES256_KeyByteLength);
@@ -79,7 +85,7 @@ NS_ASSUME_NONNULL_BEGIN
     [self.delegateStream writeRawVarint32:contactDataLength];
     [self.delegateStream writeRawData:contactData];
 
-    if (signalAccount.contact.image) {
+    if (avatarPng) {
         [self.delegateStream writeRawData:avatarPng];
     }
 }

SignalServiceKit/src/Messages/DeviceSyncing/OWSSyncContactsMessage.m

@@ -13,6 +13,7 @@
 #import "SignalAccount.h"
 #import "TSAttachment.h"
 #import "TSAttachmentStream.h"
+#import "TextSecureKitEnv.h"
 
 NS_ASSUME_NONNULL_BEGIN
 
@@ -71,6 +72,8 @@ NS_ASSUME_NONNULL_BEGIN
 
 - (NSData *)buildPlainTextAttachmentData
 {
+    id<ContactsManagerProtocol> contactsManager = TextSecureKitEnv.sharedEnv.contactsManager;
+
     // TODO use temp file stream to avoid loading everything into memory at once
     // First though, we need to re-engineer our attachment process to accept streams (encrypting with stream,
     // and uploading with streams).
@@ -85,7 +88,8 @@ NS_ASSUME_NONNULL_BEGIN
 
         [contactsOutputStream writeSignalAccount:signalAccount
                                recipientIdentity:recipientIdentity
-                                  profileKeyData:profileKeyData];
+                                  profileKeyData:profileKeyData
+                                 contactsManager:contactsManager];
     }
 
     [contactsOutputStream flush];

SignalServiceKit/src/Protocols/ContactsManagerProtocol.h

@@ -4,6 +4,7 @@
 
 NS_ASSUME_NONNULL_BEGIN
 
+@class CNContact;
 @class Contact;
 @class PhoneNumber;
 @class SignalAccount;
@@ -20,6 +21,12 @@ NS_ASSUME_NONNULL_BEGIN
 - (NSComparisonResult)compareSignalAccount:(SignalAccount *)left
                          withSignalAccount:(SignalAccount *)right NS_SWIFT_NAME(compare(signalAccount:with:));
 
+#pragma mark - CNContacts
+
+- (nullable CNContact *)cnContactWithId:(nullable NSString *)contactId;
+- (nullable NSData *)avatarDataForCNContactId:(nullable NSString *)contactId;
+- (nullable UIImage *)avatarImageForCNContactId:(nullable NSString *)contactId;
+
 @end
 
 NS_ASSUME_NONNULL_END