From 42bf2676070f841da2c2ceb39c415b56027301ed Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Wed, 7 Nov 2018 16:42:40 -0600 Subject: [PATCH] fixup plumbing for incoming messages/synced transcripts --- .../ConversationViewController.m | 25 ++++---- .../attachments/SignalAttachment.swift | 2 +- .../src/Devices/OWSRecordTranscriptJob.m | 21 ++++--- .../Attachments/OWSAttachmentsProcessor.h | 14 +---- .../Attachments/OWSAttachmentsProcessor.m | 61 +++---------------- .../Attachments/TSAttachmentPointer.h | 8 ++- .../Attachments/TSAttachmentPointer.m | 24 ++++++-- .../src/Messages/Interactions/OWSContact.m | 2 +- .../Messages/Interactions/TSQuotedMessage.m | 2 +- .../src/Messages/OWSMessageManager.m | 60 ++++++------------ 10 files changed, 83 insertions(+), 136 deletions(-) diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m index 1652d4b34..6ec594bff 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m @@ -1647,16 +1647,18 @@ typedef enum : NSUInteger { attachmentPointer:(TSAttachmentPointer *)attachmentPointer { OWSAttachmentsProcessor *processor = - [[OWSAttachmentsProcessor alloc] initWithAttachmentPointer:attachmentPointer - networkManager:self.networkManager]; - [processor fetchAttachmentsForMessage:message - primaryStorage:self.primaryStorage - success:^(NSArray *attachmentStreams) { - OWSLogInfo(@"Successfully redownloaded attachment in thread: %@", message.thread); - } - failure:^(NSError *error) { - OWSLogWarn(@"Failed to redownload message with error: %@", error); - }]; + [[OWSAttachmentsProcessor alloc] initWithAttachmentPointers:@[ attachmentPointer ]]; + + [self.editingDatabaseConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *_Nonnull transaction) { + [processor fetchAttachmentsForMessage:message + transaction:transaction + success:^(NSArray *attachmentStreams) { + OWSLogInfo(@"Successfully redownloaded attachment in thread: %@", message.thread); + } + failure:^(NSError *error) { + OWSLogWarn(@"Failed to redownload message with error: %@", error); + }]; + }]; } - (void)handleUnsentMessageTap:(TSOutgoingMessage *)message @@ -2191,8 +2193,7 @@ typedef enum : NSUInteger { } OWSAttachmentsProcessor *processor = - [[OWSAttachmentsProcessor alloc] initWithAttachmentPointer:attachmentPointer - networkManager:self.networkManager]; + [[OWSAttachmentsProcessor alloc] initWithAttachmentPointers:@[ attachmentPointer ]]; [self.editingDatabaseConnection asyncReadWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { [processor fetchAttachmentsForMessage:nil diff --git a/SignalMessaging/attachments/SignalAttachment.swift b/SignalMessaging/attachments/SignalAttachment.swift index e8176968d..d4ad2c049 100644 --- a/SignalMessaging/attachments/SignalAttachment.swift +++ b/SignalMessaging/attachments/SignalAttachment.swift @@ -241,7 +241,7 @@ public class SignalAttachment: NSObject { @objc public func buildOutgoingAttachmentInfo(message: TSMessage) -> OutgoingAttachmentInfo { - OWSAssertDebug(message.uniqueId) + assert(message.uniqueId != nil) return OutgoingAttachmentInfo(dataSource: dataSource, contentType: mimeType, sourceFilename: filenameOrDefault, diff --git a/SignalServiceKit/src/Devices/OWSRecordTranscriptJob.m b/SignalServiceKit/src/Devices/OWSRecordTranscriptJob.m index 2875b9a47..a4a04cd59 100644 --- a/SignalServiceKit/src/Devices/OWSRecordTranscriptJob.m +++ b/SignalServiceKit/src/Devices/OWSRecordTranscriptJob.m @@ -78,18 +78,12 @@ NS_ASSUME_NONNULL_BEGIN return; } - OWSAttachmentsProcessor *attachmentsProcessor = - [[OWSAttachmentsProcessor alloc] initWithAttachmentProtos:transcript.attachmentPointerProtos - networkManager:self.networkManager - transaction:transaction]; - - // TODO group updates. Currently desktop doesn't support group updates, so not a problem yet. TSOutgoingMessage *outgoingMessage = [[TSOutgoingMessage alloc] initOutgoingMessageWithTimestamp:transcript.timestamp inThread:transcript.thread messageBody:transcript.body - attachmentIds:[attachmentsProcessor.attachmentIds mutableCopy] + attachmentIds:[NSMutableArray new] expiresInSeconds:transcript.expirationDuration expireStartedAt:transcript.expirationStartedAt isVoiceMessage:NO @@ -97,6 +91,14 @@ NS_ASSUME_NONNULL_BEGIN quotedMessage:transcript.quotedMessage contactShare:transcript.contact]; + NSArray *attachmentPointers = + [TSAttachmentPointer attachmentPointersFromProtos:transcript.attachmentPointerProtos + albumMessage:outgoingMessage]; + for (TSAttachmentPointer *pointer in attachmentPointers) { + [pointer saveWithTransaction:transaction]; + [outgoingMessage.attachmentIds addObject:pointer.uniqueId]; + } + TSQuotedMessage *_Nullable quotedMessage = transcript.quotedMessage; if (quotedMessage && quotedMessage.thumbnailAttachmentPointerId) { // We weren't able to derive a local thumbnail, so we'll fetch the referenced attachment. @@ -106,8 +108,7 @@ NS_ASSUME_NONNULL_BEGIN if ([attachmentPointer isKindOfClass:[TSAttachmentPointer class]]) { OWSAttachmentsProcessor *attachmentProcessor = - [[OWSAttachmentsProcessor alloc] initWithAttachmentPointer:attachmentPointer - networkManager:self.networkManager]; + [[OWSAttachmentsProcessor alloc] initWithAttachmentPointers:@[ attachmentPointer ]]; OWSLogDebug(@"downloading thumbnail for transcript: %lu", (unsigned long)transcript.timestamp); [attachmentProcessor fetchAttachmentsForMessage:outgoingMessage @@ -160,6 +161,8 @@ NS_ASSUME_NONNULL_BEGIN [self.readReceiptManager applyEarlyReadReceiptsForOutgoingMessageFromLinkedDevice:outgoingMessage transaction:transaction]; + OWSAttachmentsProcessor *attachmentsProcessor = + [[OWSAttachmentsProcessor alloc] initWithAttachmentPointers:attachmentPointers]; [attachmentsProcessor fetchAttachmentsForMessage:outgoingMessage transaction:transaction diff --git a/SignalServiceKit/src/Messages/Attachments/OWSAttachmentsProcessor.h b/SignalServiceKit/src/Messages/Attachments/OWSAttachmentsProcessor.h index effbbec23..44fadda8d 100644 --- a/SignalServiceKit/src/Messages/Attachments/OWSAttachmentsProcessor.h +++ b/SignalServiceKit/src/Messages/Attachments/OWSAttachmentsProcessor.h @@ -23,26 +23,16 @@ extern NSString *const kAttachmentDownloadAttachmentIDKey; */ @interface OWSAttachmentsProcessor : NSObject -@property (nonatomic, readonly) NSArray *attachmentIds; @property (nonatomic, readonly) NSArray *attachmentPointers; -@property (nonatomic, readonly) BOOL hasSupportedAttachments; - (instancetype)init NS_UNAVAILABLE; -- (instancetype)initWithAttachmentProtos:(NSArray *)attachmentProtos - networkManager:(TSNetworkManager *)networkManager - transaction:(YapDatabaseReadWriteTransaction *)transaction NS_DESIGNATED_INITIALIZER; - /* * Retry fetching failed attachment download */ -- (instancetype)initWithAttachmentPointer:(TSAttachmentPointer *)attachmentPointer - networkManager:(TSNetworkManager *)networkManager NS_DESIGNATED_INITIALIZER; +- (instancetype)initWithAttachmentPointers:(NSArray *)attachmentPointers + NS_DESIGNATED_INITIALIZER; -- (void)fetchAttachmentsForMessage:(nullable TSMessage *)message - primaryStorage:(OWSPrimaryStorage *)primaryStorage - success:(void (^)(NSArray *attachmentStreams))successHandler - failure:(void (^)(NSError *error))failureHandler; - (void)fetchAttachmentsForMessage:(nullable TSMessage *)message transaction:(YapDatabaseReadWriteTransaction *)transaction success:(void (^)(NSArray *attachmentStreams))successHandler diff --git a/SignalServiceKit/src/Messages/Attachments/OWSAttachmentsProcessor.m b/SignalServiceKit/src/Messages/Attachments/OWSAttachmentsProcessor.m index 941388d0a..15aca313f 100644 --- a/SignalServiceKit/src/Messages/Attachments/OWSAttachmentsProcessor.m +++ b/SignalServiceKit/src/Messages/Attachments/OWSAttachmentsProcessor.m @@ -12,6 +12,7 @@ #import "OWSFileSystem.h" #import "OWSPrimaryStorage.h" #import "OWSRequestFactory.h" +#import "SSKEnvironment.h" #import "TSAttachmentPointer.h" #import "TSAttachmentStream.h" #import "TSGroupModel.h" @@ -43,68 +44,27 @@ static const CGFloat kAttachmentDownloadProgressTheta = 0.001f; @implementation OWSAttachmentsProcessor -- (instancetype)initWithAttachmentPointer:(TSAttachmentPointer *)attachmentPointer - networkManager:(TSNetworkManager *)networkManager +- (instancetype)initWithAttachmentPointers:(NSArray *)attachmentPointers { self = [super init]; if (!self) { return self; } - _networkManager = networkManager; - - _attachmentPointers = @[ attachmentPointer ]; - _attachmentIds = @[ attachmentPointer.uniqueId ]; + _attachmentPointers = attachmentPointers; return self; } -- (instancetype)initWithAttachmentProtos:(NSArray *)attachmentProtos - networkManager:(TSNetworkManager *)networkManager - transaction:(YapDatabaseReadWriteTransaction *)transaction -{ - self = [super init]; - if (!self) { - return self; - } - - _networkManager = networkManager; - - NSMutableArray *attachmentIds = [NSMutableArray new]; - NSMutableArray *attachmentPointers = [NSMutableArray new]; - - for (SSKProtoAttachmentPointer *attachmentProto in attachmentProtos) { - TSAttachmentPointer *_Nullable pointer = [TSAttachmentPointer attachmentPointerFromProto:attachmentProto]; - if (!pointer) { - OWSFailDebug(@"Invalid attachment."); - continue; - } - - [attachmentIds addObject:pointer.uniqueId]; - [pointer saveWithTransaction:transaction]; - [attachmentPointers addObject:pointer]; - } +#pragma mark - Dependencies - _attachmentIds = [attachmentIds copy]; - _attachmentPointers = [attachmentPointers copy]; - - return self; -} - -// PERF: Remove this and use a pre-existing dbConnection -- (void)fetchAttachmentsForMessage:(nullable TSMessage *)message - primaryStorage:(OWSPrimaryStorage *)primaryStorage - success:(void (^)(NSArray *attachmentStreams))successHandler - failure:(void (^)(NSError *error))failureHandler +- (TSNetworkManager *)networkManager { - [[primaryStorage newDatabaseConnection] readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { - [self fetchAttachmentsForMessage:message - transaction:transaction - success:successHandler - failure:failureHandler]; - }]; + return SSKEnvironment.shared.networkManager; } +#pragma mark + - (void)fetchAttachmentsForMessage:(nullable TSMessage *)message transaction:(YapDatabaseReadWriteTransaction *)transaction success:(void (^)(NSArray *attachmentStreams))successHandler @@ -516,11 +476,6 @@ static const CGFloat kAttachmentDownloadProgressTheta = 0.001f; } } -- (BOOL)hasSupportedAttachments -{ - return self.attachmentPointers.count > 0; -} - @end NS_ASSUME_NONNULL_END diff --git a/SignalServiceKit/src/Messages/Attachments/TSAttachmentPointer.h b/SignalServiceKit/src/Messages/Attachments/TSAttachmentPointer.h index 01fedcdf3..12a8382cf 100644 --- a/SignalServiceKit/src/Messages/Attachments/TSAttachmentPointer.h +++ b/SignalServiceKit/src/Messages/Attachments/TSAttachmentPointer.h @@ -7,6 +7,7 @@ NS_ASSUME_NONNULL_BEGIN @class SSKProtoAttachmentPointer; +@class TSMessage; typedef NS_ENUM(NSUInteger, TSAttachmentPointerState) { TSAttachmentPointerStateEnqueued = 0, @@ -31,7 +32,12 @@ typedef NS_ENUM(NSUInteger, TSAttachmentPointerState) { albumMessageId:(nullable NSString *)albumMessageId attachmentType:(TSAttachmentType)attachmentType NS_DESIGNATED_INITIALIZER; -+ (nullable TSAttachmentPointer *)attachmentPointerFromProto:(SSKProtoAttachmentPointer *)attachmentProto; ++ (nullable TSAttachmentPointer *)attachmentPointerFromProto:(SSKProtoAttachmentPointer *)attachmentProto + albumMessage:(nullable TSMessage *)message; + ++ (NSArray *)attachmentPointersFromProtos: + (NSArray *)attachmentProtos + albumMessage:(TSMessage *)message; @property (atomic) TSAttachmentPointerState state; @property (nullable, atomic) NSString *mostRecentFailureLocalizedText; diff --git a/SignalServiceKit/src/Messages/Attachments/TSAttachmentPointer.m b/SignalServiceKit/src/Messages/Attachments/TSAttachmentPointer.m index 7af0bde9c..363e31f26 100644 --- a/SignalServiceKit/src/Messages/Attachments/TSAttachmentPointer.m +++ b/SignalServiceKit/src/Messages/Attachments/TSAttachmentPointer.m @@ -55,9 +55,8 @@ NS_ASSUME_NONNULL_BEGIN return self; } - + (nullable TSAttachmentPointer *)attachmentPointerFromProto:(SSKProtoAttachmentPointer *)attachmentProto - message:(TSMessage *)message + albumMessage:(nullable TSMessage *)albumMessage { if (attachmentProto.id < 1) { OWSFailDebug(@"Invalid attachment id."); @@ -86,10 +85,10 @@ NS_ASSUME_NONNULL_BEGIN if (attachmentProto.hasCaption) { caption = attachmentProto.caption; } + NSString *_Nullable albumMessageId; - if ([MIMETypeUtil isVisualMedia:attachmentProto.contentType]) { - OWSAssertDebug(message.uniqueId); - albumMessageId = message.uniqueId; + if (albumMessage != nil) { + albumMessageId = albumMessage.uniqueId; } TSAttachmentPointer *pointer = [[TSAttachmentPointer alloc] initWithServerId:attachmentProto.id @@ -104,6 +103,21 @@ NS_ASSUME_NONNULL_BEGIN return pointer; } ++ (NSArray *)attachmentPointersFromProtos: + (NSArray *)attachmentProtos + albumMessage:(TSMessage *)albumMessage +{ + NSMutableArray *attachmentPointers = [NSMutableArray new]; + for (SSKProtoAttachmentPointer *attachmentProto in attachmentProtos) { + TSAttachmentPointer *_Nullable attachmentPointer = + [self attachmentPointerFromProto:attachmentProto albumMessage:albumMessage]; + if (attachmentPointer) { + [attachmentPointers addObject:attachmentPointer]; + } + } + return [attachmentPointers copy]; +} + - (BOOL)isDecimalNumberText:(NSString *)text { return [text componentsSeparatedByCharactersInSet:[NSCharacterSet decimalDigitCharacterSet]].count == 1; diff --git a/SignalServiceKit/src/Messages/Interactions/OWSContact.m b/SignalServiceKit/src/Messages/Interactions/OWSContact.m index 8a6ac2930..8893a00df 100644 --- a/SignalServiceKit/src/Messages/Interactions/OWSContact.m +++ b/SignalServiceKit/src/Messages/Interactions/OWSContact.m @@ -1000,7 +1000,7 @@ NSString *NSStringForContactAddressType(OWSContactAddressType value) SSKProtoAttachmentPointer *avatarAttachment = avatarInfo.avatar; TSAttachmentPointer *_Nullable attachmentPointer = - [TSAttachmentPointer attachmentPointerFromProto:avatarAttachment]; + [TSAttachmentPointer attachmentPointerFromProto:avatarAttachment albumMessage:nil]; if (attachmentPointer) { [attachmentPointer saveWithTransaction:transaction]; contact.avatarAttachmentId = attachmentPointer.uniqueId; diff --git a/SignalServiceKit/src/Messages/Interactions/TSQuotedMessage.m b/SignalServiceKit/src/Messages/Interactions/TSQuotedMessage.m index 5bb9bd265..e1c60161d 100644 --- a/SignalServiceKit/src/Messages/Interactions/TSQuotedMessage.m +++ b/SignalServiceKit/src/Messages/Interactions/TSQuotedMessage.m @@ -188,7 +188,7 @@ NS_ASSUME_NONNULL_BEGIN SSKProtoAttachmentPointer *thumbnailAttachmentProto = quotedAttachment.thumbnail; TSAttachmentPointer *_Nullable thumbnailPointer = - [TSAttachmentPointer attachmentPointerFromProto:thumbnailAttachmentProto]; + [TSAttachmentPointer attachmentPointerFromProto:thumbnailAttachmentProto albumMessage:nil]; if (thumbnailPointer) { [thumbnailPointer saveWithTransaction:transaction]; diff --git a/SignalServiceKit/src/Messages/OWSMessageManager.m b/SignalServiceKit/src/Messages/OWSMessageManager.m index 3cb39fa8e..b10f35b9f 100644 --- a/SignalServiceKit/src/Messages/OWSMessageManager.m +++ b/SignalServiceKit/src/Messages/OWSMessageManager.m @@ -729,15 +729,11 @@ NS_ASSUME_NONNULL_BEGIN return; } + TSAttachmentPointer *avatarPointer = + [TSAttachmentPointer attachmentPointerFromProto:dataMessage.group.avatar albumMessage:nil]; OWSAttachmentsProcessor *attachmentsProcessor = - [[OWSAttachmentsProcessor alloc] initWithAttachmentProtos:@[ dataMessage.group.avatar ] - networkManager:self.networkManager - transaction:transaction]; + [[OWSAttachmentsProcessor alloc] initWithAttachmentPointers:@[ avatarPointer ]]; - if (!attachmentsProcessor.hasSupportedAttachments) { - OWSLogWarn(@"received unsupported group avatar envelope"); - return; - } [attachmentsProcessor fetchAttachmentsForMessage:nil transaction:transaction success:^(NSArray *attachmentStreams) { @@ -775,26 +771,26 @@ NS_ASSUME_NONNULL_BEGIN return; } - OWSAttachmentsProcessor *attachmentsProcessor = - [[OWSAttachmentsProcessor alloc] initWithAttachmentProtos:dataMessage.attachments - networkManager:self.networkManager - transaction:transaction]; - if (!attachmentsProcessor.hasSupportedAttachments) { - OWSLogWarn(@"received unsupported media envelope"); - return; - } - TSIncomingMessage *_Nullable createdMessage = [self handleReceivedEnvelope:envelope withDataMessage:dataMessage - attachmentIds:attachmentsProcessor.attachmentIds transaction:transaction]; - if (!createdMessage) { return; } + NSArray *attachmentPointers = + [TSAttachmentPointer attachmentPointersFromProtos:dataMessage.attachments albumMessage:createdMessage]; + for (TSAttachmentPointer *pointer in attachmentPointers) { + [pointer saveWithTransaction:transaction]; + [createdMessage.attachmentIds addObject:pointer.uniqueId]; + } + [createdMessage saveWithTransaction:transaction]; + OWSLogDebug(@"incoming attachment message: %@", createdMessage.debugDescription); + OWSAttachmentsProcessor *attachmentsProcessor = + [[OWSAttachmentsProcessor alloc] initWithAttachmentPointers:attachmentPointers]; + [attachmentsProcessor fetchAttachmentsForMessage:createdMessage transaction:transaction success:^(NSArray *attachmentStreams) { @@ -1054,7 +1050,7 @@ NS_ASSUME_NONNULL_BEGIN return; } - [self handleReceivedEnvelope:envelope withDataMessage:dataMessage attachmentIds:@[] transaction:transaction]; + [self handleReceivedEnvelope:envelope withDataMessage:dataMessage transaction:transaction]; } - (void)handleGroupInfoRequest:(SSKProtoEnvelope *)envelope @@ -1137,7 +1133,6 @@ NS_ASSUME_NONNULL_BEGIN - (TSIncomingMessage *_Nullable)handleReceivedEnvelope:(SSKProtoEnvelope *)envelope withDataMessage:(SSKProtoDataMessage *)dataMessage - attachmentIds:(NSArray *)attachmentIds transaction:(YapDatabaseReadWriteTransaction *)transaction { if (!envelope) { @@ -1247,14 +1242,6 @@ NS_ASSUME_NONNULL_BEGIN return nil; } - if (body.length == 0 && attachmentIds.count < 1 && !contact) { - OWSLogWarn(@"ignoring empty incoming message from: %@ for group: %@ with timestamp: %lu", - envelopeAddress(envelope), - groupId, - (unsigned long)timestamp); - return nil; - } - TSQuotedMessage *_Nullable quotedMessage = [TSQuotedMessage quotedMessageForDataMessage:dataMessage thread:oldGroupThread transaction:transaction]; @@ -1270,7 +1257,7 @@ NS_ASSUME_NONNULL_BEGIN authorId:envelope.source sourceDeviceId:envelope.sourceDevice messageBody:body - attachmentIds:attachmentIds + attachmentIds:@[] expiresInSeconds:dataMessage.expireTimer quotedMessage:quotedMessage contactShare:contact @@ -1289,13 +1276,6 @@ NS_ASSUME_NONNULL_BEGIN } } } else { - if (body.length == 0 && attachmentIds.count < 1 && !contact) { - OWSLogWarn(@"ignoring empty incoming message from: %@ with timestamp: %lu", - envelopeAddress(envelope), - (unsigned long)timestamp); - return nil; - } - OWSLogDebug( @"incoming message from: %@ with timestamp: %lu", envelopeAddress(envelope), (unsigned long)timestamp); TSContactThread *thread = @@ -1311,7 +1291,7 @@ NS_ASSUME_NONNULL_BEGIN authorId:[thread contactIdentifier] sourceDeviceId:envelope.sourceDevice messageBody:body - attachmentIds:attachmentIds + attachmentIds:@[] expiresInSeconds:dataMessage.expireTimer quotedMessage:quotedMessage contactShare:contact @@ -1365,8 +1345,7 @@ NS_ASSUME_NONNULL_BEGIN if ([attachmentPointer isKindOfClass:[TSAttachmentPointer class]]) { OWSAttachmentsProcessor *attachmentProcessor = - [[OWSAttachmentsProcessor alloc] initWithAttachmentPointer:attachmentPointer - networkManager:self.networkManager]; + [[OWSAttachmentsProcessor alloc] initWithAttachmentPointers:@[ attachmentPointer ]]; OWSLogDebug(@"downloading thumbnail for message: %lu", (unsigned long)incomingMessage.timestamp); [attachmentProcessor fetchAttachmentsForMessage:incomingMessage @@ -1396,8 +1375,7 @@ NS_ASSUME_NONNULL_BEGIN OWSFailDebug(@"avatar attachmentPointer was unexpectedly nil"); } else { OWSAttachmentsProcessor *attachmentProcessor = - [[OWSAttachmentsProcessor alloc] initWithAttachmentPointer:attachmentPointer - networkManager:self.networkManager]; + [[OWSAttachmentsProcessor alloc] initWithAttachmentPointers:@[ attachmentPointer ]]; OWSLogDebug(@"downloading contact avatar for message: %lu", (unsigned long)incomingMessage.timestamp); [attachmentProcessor fetchAttachmentsForMessage:incomingMessage