respond to code review

// FREEBIE

Signal/src/ViewControllers/ConversationView/Cells/OWSContactShareView.m

@@ -110,7 +110,6 @@ NS_ASSUME_NONNULL_BEGIN
     [contentView autoPinEdgeToSuperviewEdge:ALEdgeTop withInset:self.vMargin];
     [contentView autoPinEdgeToSuperviewEdge:ALEdgeBottom withInset:self.vMargin];
 
-    // TODO: Use the contact's avatar if present and downloaded.
     AvatarImageView *avatarView = [AvatarImageView new];
     avatarView.image =
         [self.contactShare getAvatarImageWithDiameter:self.iconSize contactsManager:self.contactsManager];

Signal/src/ViewControllers/ConversationView/ConversationViewController.m

@@ -2969,12 +2969,11 @@ typedef enum : NSUInteger {
                                                                    messageSender:self.messageSender
                                                                       completion:nil];
             [self messageWasSent:message];
-        }];
-
 
-    if (didAddToProfileWhitelist) {
-        [self ensureDynamicInteractions];
-    }
+            if (didAddToProfileWhitelist) {
+                [self ensureDynamicInteractions];
+            }
+        }];
 }
 
 - (NSURL *)videoTempFolder

Signal/src/ViewControllers/DebugUI/DebugUIMessages.m

@@ -3584,9 +3584,9 @@ typedef OWSContact * (^OWSContactBlock)(YapDatabaseReadWriteTransaction *transac
 
                   TSContactThread *contactThread = [TSContactThread getOrCreateThreadWithContactId:phoneNumber.toE164];
                   [self sendFakeMessages:messageCount thread:contactThread];
-                  DDLogError(@"Create fake thread: %@, interactions: %tu",
+                  DDLogError(@"Create fake thread: %@, interactions: %lu",
                       phoneNumber.toE164,
-                      contactThread.numberOfInteractions);
+                      (unsigned long)contactThread.numberOfInteractions);
               }];
 }
 
@@ -3612,7 +3612,10 @@ typedef OWSContact * (^OWSContactBlock)(YapDatabaseReadWriteTransaction *transac
                         [self sendFakeMessages:batchSize thread:thread isTextOnly:isTextOnly transaction:transaction];
                     }];
                 remainder -= batchSize;
-                DDLogInfo(@"%@ sendFakeMessages %td / %tu", self.logTag, counter - remainder, counter);
+                DDLogInfo(@"%@ sendFakeMessages %lu / %lu",
+                    self.logTag,
+                    (unsigned long)(counter - remainder),
+                    (unsigned long)counter);
             }
         });
     }
@@ -3624,7 +3627,7 @@ typedef OWSContact * (^OWSContactBlock)(YapDatabaseReadWriteTransaction *transac
               isTextOnly:(BOOL)isTextOnly
              transaction:(YapDatabaseReadWriteTransaction *)transaction
 {
-    DDLogInfo(@"%@ sendFakeMessages: %tu", self.logTag, counter);
+    DDLogInfo(@"%@ sendFakeMessages: %lu", self.logTag, (unsigned long)counter);
 
     for (NSUInteger i = 0; i < counter; i++) {
         NSString *randomText = [self randomText];
@@ -3776,7 +3779,7 @@ typedef OWSContact * (^OWSContactBlock)(YapDatabaseReadWriteTransaction *transac
 {
     OWSAssert(thread);
 
-    DDLogInfo(@"%@ injectIncomingMessageInThread: %tu", self.logTag, counter);
+    DDLogInfo(@"%@ injectIncomingMessageInThread: %lu", self.logTag, (unsigned long)counter);
 
     NSString *randomText = [self randomText];
     NSString *text = [[[@(counter) description] stringByAppendingString:@" "] stringByAppendingString:randomText];
@@ -4168,7 +4171,8 @@ typedef OWSContact * (^OWSContactBlock)(YapDatabaseReadWriteTransaction *transac
                                 inThread:thread
                                 authorId:thread.recipientIdentifiers.firstObject
                           sourceDeviceId:0
-                             messageBody:[NSString stringWithFormat:@"Should disappear 60s after %tu", now]
+                             messageBody:[NSString
+                                             stringWithFormat:@"Should disappear 60s after %lu", (unsigned long)now]
                            attachmentIds:[NSMutableArray new]
                         expiresInSeconds:60
                            quotedMessage:nil

SignalMessaging/attachments/ApproveContactShareViewController.swift

@@ -19,7 +19,7 @@ protocol ContactShareField: class {
 
     func setIsIncluded(_ isIncluded: Bool)
 
-    func applyToContact(contact: OWSContact)
+    func applyToContact(contact: ContactShareViewModel)
 }
 
 // MARK: -

SignalMessaging/utils/OWSAvatarBuilder.m

@@ -89,14 +89,14 @@ NS_ASSUME_NONNULL_BEGIN
 
 - (nullable UIImage *)buildSavedImage
 {
-    OWSRaiseException(
-        NSInternalInconsistencyException, @"You must override %@ in a subclass", NSStringFromSelector(_cmd));
+    OWS_ABSTRACT_METHOD();
+    return nil;
 }
 
 - (UIImage *)buildDefaultImage
 {
-    OWSRaiseException(
-        NSInternalInconsistencyException, @"You must override %@ in a subclass", NSStringFromSelector(_cmd));
+    OWS_ABSTRACT_METHOD();
+    return [UIImage new];
 }
 
 @end

SignalServiceKit/src/Contacts/TSThread.m

@@ -112,11 +112,6 @@ NS_ASSUME_NONNULL_BEGIN
     return @[];
 }
 
-- (nullable UIImage *)image
-{
-    return nil;
-}
-
 - (BOOL)hasSafetyNumbers
 {
     return NO;
@@ -256,7 +251,9 @@ NS_ASSUME_NONNULL_BEGIN
              // who's test devices are constantly reinstalled. We could add a purpose-built DB view,
              // but I think in the real world this is rare to be a hotspot.
              if (missedCount > 50) {
-                 DDLogWarn(@"%@ found last interaction for inbox after skipping %tu items", self.logTag, missedCount);
+                 DDLogWarn(@"%@ found last interaction for inbox after skipping %lu items",
+                     self.logTag,
+                     (unsigned long)missedCount);
              }
              *stop = YES;
          }

SignalServiceKit/src/Devices/OWSRecordTranscriptJob.m

@@ -110,7 +110,8 @@ NS_ASSUME_NONNULL_BEGIN
                 [[OWSAttachmentsProcessor alloc] initWithAttachmentPointer:attachmentPointer
                                                             networkManager:self.networkManager];
 
-            DDLogDebug(@"%@ downloading thumbnail for transcript: %tu", self.logTag, transcript.timestamp);
+            DDLogDebug(
+                @"%@ downloading thumbnail for transcript: %lu", self.logTag, (unsigned long)transcript.timestamp);
             [attachmentProcessor fetchAttachmentsForMessage:outgoingMessage
                 transaction:transaction
                 success:^(TSAttachmentStream *_Nonnull attachmentStream) {
@@ -121,9 +122,9 @@ NS_ASSUME_NONNULL_BEGIN
                         }];
                 }
                 failure:^(NSError *_Nonnull error) {
-                    DDLogWarn(@"%@ failed to fetch thumbnail for transcript: %tu with error: %@",
+                    DDLogWarn(@"%@ failed to fetch thumbnail for transcript: %lu with error: %@",
                         self.logTag,
-                        transcript.timestamp,
+                        (unsigned long)transcript.timestamp,
                         error);
                 }];
         }

SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m

@@ -710,8 +710,9 @@ NS_ASSUME_NONNULL_BEGIN
 {
     OWSAssert(attachmentId.length > 0);
 
-    // TODO we should hoist this transaction to make sure we're getting a consistent view in the message sending process
-    // A brief glance shows it touches quite a bit of code, but should be straight forward.
+    // TODO we should past in a transaction, rather than sneakily generate one in `fetch...` to make sure we're
+    // getting a consistent view in the message sending process. A brief glance shows it touches quite a bit of code,
+    // but should be straight forward.
     TSAttachment *attachment = [TSAttachmentStream fetchObjectWithUniqueID:attachmentId];
     if (![attachment isKindOfClass:[TSAttachmentStream class]]) {
         DDLogError(@"Unexpected type for attachment builder: %@", attachment);

SignalServiceKit/src/Messages/Interactions/TSInteraction.m

@@ -129,8 +129,10 @@ NS_ASSUME_NONNULL_BEGIN
 
 - (NSString *)description
 {
-    return [NSString
-        stringWithFormat:@"%@ in thread: %@ timestamp: %tu", [super description], self.uniqueThreadId, self.timestamp];
+    return [NSString stringWithFormat:@"%@ in thread: %@ timestamp: %lu",
+                     [super description],
+                     self.uniqueThreadId,
+                     (unsigned long)self.timestamp];
 }
 
 - (void)saveWithTransaction:(YapDatabaseReadWriteTransaction *)transaction {

SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.m

@@ -738,9 +738,9 @@ NSString *NSStringForOutgoingMessageRecipientState(OWSOutgoingMessageRecipientSt
         OWSFail(@"%@ message body length too long.", self.logTag);
         NSString *truncatedBody = [self.body copy];
         while ([truncatedBody lengthOfBytesUsingEncoding:NSUTF8StringEncoding] > kOversizeTextMessageSizeThreshold) {
-            DDLogError(@"%@ truncating body which is too long: %tu",
+            DDLogError(@"%@ truncating body which is too long: %lu",
                 self.logTag,
-                [truncatedBody lengthOfBytesUsingEncoding:NSUTF8StringEncoding]);
+                (unsigned long)[truncatedBody lengthOfBytesUsingEncoding:NSUTF8StringEncoding]);
             truncatedBody = [truncatedBody substringToIndex:truncatedBody.length / 2];
         }
         [builder setBody:truncatedBody];
@@ -800,7 +800,7 @@ NSString *NSStringForOutgoingMessageRecipientState(OWSOutgoingMessageRecipientSt
         if (contactProto) {
             [builder addContact:contactProto];
         } else {
-            OWSFail(@"%@ in %s <# thing #> was unexpectedly nil", self.logTag, __PRETTY_FUNCTION__);
+            OWSFail(@"%@ in %s contactProto was unexpectedly nil", self.logTag, __PRETTY_FUNCTION__);
         }
     }
 

SignalServiceKit/src/Messages/Interactions/TSQuotedMessage.m

@@ -153,19 +153,19 @@ NS_ASSUME_NONNULL_BEGIN
                                                   transaction:transaction];
 
         if (thumbnailStream) {
-            DDLogDebug(@"%@ Generated local thumbnail for quoted quoted message: %@:%tu",
+            DDLogDebug(@"%@ Generated local thumbnail for quoted quoted message: %@:%lu",
                 self.logTag,
                 thread.uniqueId,
-                timestamp);
+                (unsigned long)timestamp);
 
             [thumbnailStream saveWithTransaction:transaction];
 
             attachmentInfo.thumbnailAttachmentStreamId = thumbnailStream.uniqueId;
         } else if (quotedAttachment.hasThumbnail) {
-            DDLogDebug(@"%@ Saving reference for fetching remote thumbnail for quoted message: %@:%tu",
+            DDLogDebug(@"%@ Saving reference for fetching remote thumbnail for quoted message: %@:%lu",
                 self.logTag,
                 thread.uniqueId,
-                timestamp);
+                (unsigned long)timestamp);
 
             OWSSignalServiceProtosAttachmentPointer *thumbnailAttachmentProto = quotedAttachment.thumbnail;
             TSAttachmentPointer *thumbnailPointer =
@@ -174,7 +174,8 @@ NS_ASSUME_NONNULL_BEGIN
 
             attachmentInfo.thumbnailAttachmentPointerId = thumbnailPointer.uniqueId;
         } else {
-            DDLogDebug(@"%@ No thumbnail for quoted message: %@:%tu", self.logTag, thread.uniqueId, timestamp);
+            DDLogDebug(
+                @"%@ No thumbnail for quoted message: %@:%lu", self.logTag, thread.uniqueId, (unsigned long)timestamp);
         }
 
         [attachmentInfos addObject:attachmentInfo];

SignalServiceKit/src/Messages/OWSBatchMessageProcessor.m

@@ -356,10 +356,10 @@ NSString *const OWSMessageContentJobFinderExtensionGroup = @"OWSMessageContentJo
 
     backgroundTask = nil;
 
-    DDLogVerbose(@"%@ completed %zu/%zu jobs. %zd jobs left.",
+    DDLogVerbose(@"%@ completed %lu/%lu jobs. %zd jobs left.",
         self.logTag,
-        processedJobs.count,
-        batchJobs.count,
+        (unsigned long)processedJobs.count,
+        (unsigned long)batchJobs.count,
         [OWSMessageContentJob numberOfKeysInCollection]);
 
     // Wait a bit in hopes of increasing the batch size.

SignalServiceKit/src/Messages/OWSDisappearingMessagesJob.m

@@ -135,7 +135,7 @@ void AssertIsOnDisappearingMessagesQueue()
                                                                transaction:transaction];
     }];
 
-    DDLogDebug(@"%@ Removed %tu expired messages", self.logTag, expirationCount);
+    DDLogDebug(@"%@ Removed %lu expired messages", self.logTag, (unsigned long)expirationCount);
 
     backgroundTask = nil;
     return expirationCount;
@@ -398,7 +398,7 @@ void AssertIsOnDisappearingMessagesQueue()
 {
     [self.disappearingMessagesFinder enumerateMessagesWhichFailedToStartExpiringWithBlock:^(
         TSMessage *_Nonnull message) {
-        DDLogWarn(@"%@ starting old timer for message timestamp: %tu", self.logTag, message.timestamp);
+        DDLogWarn(@"%@ starting old timer for message timestamp: %lu", self.logTag, (unsigned long)message.timestamp);
         [self setExpirationForMessage:message expirationStartedAt:message.timestampForSorting transaction:transaction];
     }
                                                                               transaction:transaction];

SignalServiceKit/src/Messages/OWSMessageManager.m

@@ -1109,7 +1109,8 @@ NS_ASSUME_NONNULL_BEGIN
                 [[OWSAttachmentsProcessor alloc] initWithAttachmentPointer:attachmentPointer
                                                             networkManager:self.networkManager];
 
-            DDLogDebug(@"%@ downloading thumbnail for message: %tu", self.logTag, incomingMessage.timestamp);
+            DDLogDebug(
+                @"%@ downloading thumbnail for message: %lu", self.logTag, (unsigned long)incomingMessage.timestamp);
             [attachmentProcessor fetchAttachmentsForMessage:incomingMessage
                 transaction:transaction
                 success:^(TSAttachmentStream *_Nonnull attachmentStream) {
@@ -1120,9 +1121,9 @@ NS_ASSUME_NONNULL_BEGIN
                         }];
                 }
                 failure:^(NSError *_Nonnull error) {
-                    DDLogWarn(@"%@ failed to fetch thumbnail for message: %tu with error: %@",
+                    DDLogWarn(@"%@ failed to fetch thumbnail for message: %lu with error: %@",
                         self.logTag,
-                        incomingMessage.timestamp,
+                        (unsigned long)incomingMessage.timestamp,
                         error);
                 }];
         }
@@ -1140,7 +1141,9 @@ NS_ASSUME_NONNULL_BEGIN
                 [[OWSAttachmentsProcessor alloc] initWithAttachmentPointer:attachmentPointer
                                                             networkManager:self.networkManager];
 
-            DDLogDebug(@"%@ downloading contact avatar for message: %tu", self.logTag, incomingMessage.timestamp);
+            DDLogDebug(@"%@ downloading contact avatar for message: %lu",
+                self.logTag,
+                (unsigned long)incomingMessage.timestamp);
             [attachmentProcessor fetchAttachmentsForMessage:incomingMessage
                 transaction:transaction
                 success:^(TSAttachmentStream *_Nonnull attachmentStream) {
@@ -1150,9 +1153,9 @@ NS_ASSUME_NONNULL_BEGIN
                         }];
                 }
                 failure:^(NSError *_Nonnull error) {
-                    DDLogWarn(@"%@ failed to fetch contact avatar for message: %tu with error: %@",
+                    DDLogWarn(@"%@ failed to fetch contact avatar for message: %lu with error: %@",
                         self.logTag,
-                        incomingMessage.timestamp,
+                        (unsigned long)incomingMessage.timestamp,
                         error);
                 }];
         }

SignalServiceKit/src/Messages/OWSReadReceiptManager.m

@@ -559,9 +559,9 @@ NSString *const OWSReadReceiptManagerAreReadReceiptsEnabled = @"areReadReceiptsE
     }
     
     if (wasLocal) {
-        DDLogError(@"Marking %zu messages as read locally.", newlyReadList.count);
+        DDLogError(@"Marking %lu messages as read locally.", (unsigned long)newlyReadList.count);
     } else {
-        DDLogError(@"Marking %zu messages as read by linked device.", newlyReadList.count);
+        DDLogError(@"Marking %lu messages as read by linked device.", (unsigned long)newlyReadList.count);
     }
     for (id<OWSReadTracking> readItem in newlyReadList) {
         [readItem markAsReadAtTimestamp:readTimestamp sendReadReceipt:wasLocal transaction:transaction];

SignalServiceKit/src/Storage/OWSOrphanedDataCleaner.m

@@ -83,18 +83,20 @@ NS_ASSUME_NONNULL_BEGIN
                                               }];
     }];
 
-    CleanupLogDebug(@"%@ fileCount: %tu", self.logTag, fileCount);
+    CleanupLogDebug(@"%@ fileCount: %lu", self.logTag, (unsigned long)fileCount);
     CleanupLogDebug(@"%@ totalFileSize: %lld", self.logTag, totalFileSize);
     CleanupLogDebug(@"%@ attachmentStreams: %d", self.logTag, attachmentStreamCount);
-    CleanupLogDebug(@"%@ attachmentStreams with file paths: %tu", self.logTag, attachmentFilePaths.count);
+    CleanupLogDebug(
+        @"%@ attachmentStreams with file paths: %lu", self.logTag, (unsigned long)attachmentFilePaths.count);
 
     NSMutableSet<NSString *> *orphanDiskFilePaths = [diskFilePaths mutableCopy];
     [orphanDiskFilePaths minusSet:attachmentFilePaths];
     NSMutableSet<NSString *> *missingAttachmentFilePaths = [attachmentFilePaths mutableCopy];
     [missingAttachmentFilePaths minusSet:diskFilePaths];
 
-    CleanupLogDebug(@"%@ orphan disk file paths: %tu", self.logTag, orphanDiskFilePaths.count);
-    CleanupLogDebug(@"%@ missing attachment file paths: %tu", self.logTag, missingAttachmentFilePaths.count);
+    CleanupLogDebug(@"%@ orphan disk file paths: %lu", self.logTag, (unsigned long)orphanDiskFilePaths.count);
+    CleanupLogDebug(
+        @"%@ missing attachment file paths: %lu", self.logTag, (unsigned long)missingAttachmentFilePaths.count);
 
     [self printPaths:orphanDiskFilePaths.allObjects label:@"orphan disk file paths"];
     [self printPaths:missingAttachmentFilePaths.allObjects label:@"missing attachment file paths"];
@@ -140,10 +142,13 @@ NS_ASSUME_NONNULL_BEGIN
                                               }];
     }];
 
-    CleanupLogDebug(@"%@ attachmentIds: %tu", self.logTag, attachmentIds.count);
-    CleanupLogDebug(@"%@ messageAttachmentIds: %tu", self.logTag, messageAttachmentIds.count);
-    CleanupLogDebug(@"%@ quotedReplyThumbnailAttachmentIds: %tu", self.logTag, quotedReplyThumbnailAttachmentIds.count);
-    CleanupLogDebug(@"%@ contactShareAvatarAttachmentIds: %tu", self.logTag, contactShareAvatarAttachmentIds.count);
+    CleanupLogDebug(@"%@ attachmentIds: %lu", self.logTag, (unsigned long)attachmentIds.count);
+    CleanupLogDebug(@"%@ messageAttachmentIds: %lu", self.logTag, (unsigned long)messageAttachmentIds.count);
+    CleanupLogDebug(@"%@ quotedReplyThumbnailAttachmentIds: %lu",
+        self.logTag,
+        (unsigned long)quotedReplyThumbnailAttachmentIds.count);
+    CleanupLogDebug(
+        @"%@ contactShareAvatarAttachmentIds: %lu", self.logTag, (unsigned long)contactShareAvatarAttachmentIds.count);
 
     NSMutableSet<NSString *> *orphanAttachmentIds = [attachmentIds mutableCopy];
     [orphanAttachmentIds minusSet:messageAttachmentIds];
@@ -152,9 +157,9 @@ NS_ASSUME_NONNULL_BEGIN
     NSMutableSet<NSString *> *missingAttachmentIds = [messageAttachmentIds mutableCopy];
     [missingAttachmentIds minusSet:attachmentIds];
 
-    CleanupLogDebug(@"%@ orphan attachmentIds: %tu", self.logTag, orphanAttachmentIds.count);
-    CleanupLogDebug(@"%@ missing attachmentIds: %tu", self.logTag, missingAttachmentIds.count);
-    CleanupLogDebug(@"%@ orphan interactions: %tu", self.logTag, orphanInteractionIds.count);
+    CleanupLogDebug(@"%@ orphan attachmentIds: %lu", self.logTag, (unsigned long)orphanAttachmentIds.count);
+    CleanupLogDebug(@"%@ missing attachmentIds: %lu", self.logTag, (unsigned long)missingAttachmentIds.count);
+    CleanupLogDebug(@"%@ orphan interactions: %lu", self.logTag, (unsigned long)orphanInteractionIds.count);
 
     // We need to avoid cleaning up new attachments and files that are still in the process of
     // being created/written, so we don't clean up anything recent.