Merge branch 'charlesmchen/attachmentFilenames'

src/Messages/Attachments/OWSAttachmentsProcessor.m

@@ -92,7 +92,7 @@ static const CGFloat kAttachmentDownloadProgressTheta = 0.001f;
                                                                               digest:digest
                                                                          contentType:attachmentProto.contentType
                                                                                relay:relay
-                                                                            filename:attachmentProto.fileName
+                                                                      sourceFilename:attachmentProto.fileName
                                                                       attachmentType:attachmentType];
 
         [attachmentIds addObject:pointer.uniqueId];

src/Messages/Attachments/TSAttachment.h

@@ -31,20 +31,20 @@ typedef NS_ENUM(NSUInteger, TSAttachmentType) {
 @property (atomic, readwrite) BOOL isDownloaded;
 @property (nonatomic) TSAttachmentType attachmentType;
 
-// Represents the "nominal" filename sent or received in the protos,
+// Represents the "source" filename sent or received in the protos,
 // not the filename on disk.
-@property (nonatomic, readonly, nullable) NSString *filename;
+@property (nonatomic, readonly, nullable) NSString *sourceFilename;
 
 // This constructor is used for new instances of TSAttachmentPointer,
 // i.e. undownloaded incoming attachments.
 - (instancetype)initWithServerId:(UInt64)serverId
                    encryptionKey:(NSData *)encryptionKey
                      contentType:(NSString *)contentType
-                        filename:(nullable NSString *)filename;
+                  sourceFilename:(nullable NSString *)sourceFilename;
 
 // This constructor is used for new instances of TSAttachmentStream
 // that represent new, un-uploaded outgoing attachments.
-- (instancetype)initWithContentType:(NSString *)contentType filename:(nullable NSString *)filename;
+- (instancetype)initWithContentType:(NSString *)contentType sourceFilename:(nullable NSString *)sourceFilename;
 
 // This constructor is used for new instances of TSAttachmentStream
 // that represent downloaded incoming attachments.

src/Messages/Attachments/TSAttachment.m

@@ -22,7 +22,7 @@ NSUInteger const TSAttachmentSchemaVersion = 3;
 - (instancetype)initWithServerId:(UInt64)serverId
                    encryptionKey:(NSData *)encryptionKey
                      contentType:(NSString *)contentType
-                        filename:(nullable NSString *)filename
+                  sourceFilename:(nullable NSString *)sourceFilename
 {
     self = [super init];
     if (!self) {
@@ -33,14 +33,14 @@ NSUInteger const TSAttachmentSchemaVersion = 3;
     _encryptionKey = encryptionKey;
     _contentType = contentType;
     _attachmentSchemaVersion = TSAttachmentSchemaVersion;
-    _filename = filename;
+    _sourceFilename = sourceFilename;
 
     return self;
 }
 
 // This constructor is used for new instances of TSAttachmentStream
 // that represent new, un-uploaded outgoing attachments.
-- (instancetype)initWithContentType:(NSString *)contentType filename:(nullable NSString *)filename
+- (instancetype)initWithContentType:(NSString *)contentType sourceFilename:(nullable NSString *)sourceFilename
 {
     self = [super init];
     if (!self) {
@@ -49,7 +49,7 @@ NSUInteger const TSAttachmentSchemaVersion = 3;
 
     _contentType = contentType;
     _attachmentSchemaVersion = TSAttachmentSchemaVersion;
-    _filename = filename;
+    _sourceFilename = sourceFilename;
 
     return self;
 }
@@ -67,7 +67,7 @@ NSUInteger const TSAttachmentSchemaVersion = 3;
     _serverId = pointer.serverId;
     _encryptionKey = pointer.encryptionKey;
     _contentType = pointer.contentType;
-    _filename = pointer.filename;
+    _sourceFilename = pointer.sourceFilename;
     _attachmentSchemaVersion = TSAttachmentSchemaVersion;
 
     return self;
@@ -85,6 +85,12 @@ NSUInteger const TSAttachmentSchemaVersion = 3;
         _attachmentSchemaVersion = TSAttachmentSchemaVersion;
     }
 
+    if (!_sourceFilename) {
+        // renamed _filename to _sourceFilename
+        _sourceFilename = [coder decodeObjectForKey:@"filename"];
+        OWSAssert(!_sourceFilename || [_sourceFilename isKindOfClass:[NSString class]]);
+    }
+
     return self;
 }
 
@@ -107,9 +113,9 @@ NSUInteger const TSAttachmentSchemaVersion = 3;
         return [NSString stringWithFormat:@"📽 %@", attachmentString];
     } else if ([MIMETypeUtil isAudio:self.contentType]) {
 
-        // a missing filename is the legacy way to determin if an audio attachment is a voice note vs. other arbitrary
+        // a missing filename is the legacy way to determine if an audio attachment is
-        // audio attachments.
+        // a voice note vs. other arbitrary audio attachments.
-        if (self.isVoiceMessage || !self.filename || self.filename.length == 0) {
+        if (self.isVoiceMessage || !self.sourceFilename || self.sourceFilename.length == 0) {
             attachmentString = NSLocalizedString(@"ATTACHMENT_TYPE_VOICE_MESSAGE",
                 @"Short text label for a voice message attachment, used for thread preview and on lockscreen");
             return [NSString stringWithFormat:@"🎤 %@", attachmentString];

src/Messages/Attachments/TSAttachmentPointer.h

@@ -24,7 +24,7 @@ typedef NS_ENUM(NSUInteger, TSAttachmentPointerState) {
                           digest:(nullable NSData *)digest
                      contentType:(NSString *)contentType
                            relay:(NSString *)relay
-                        filename:(nullable NSString *)filename
+                  sourceFilename:(nullable NSString *)sourceFilename
                   attachmentType:(TSAttachmentType)attachmentType NS_DESIGNATED_INITIALIZER;
 
 @property (nonatomic, readonly) NSString *relay;

src/Messages/Attachments/TSAttachmentPointer.m

@@ -30,10 +30,10 @@ NS_ASSUME_NONNULL_BEGIN
                           digest:(nullable NSData *)digest
                      contentType:(NSString *)contentType
                            relay:(NSString *)relay
-                        filename:(nullable NSString *)filename
+                  sourceFilename:(nullable NSString *)sourceFilename
                   attachmentType:(TSAttachmentType)attachmentType
 {
-    self = [super initWithServerId:serverId encryptionKey:key contentType:contentType filename:filename];
+    self = [super initWithServerId:serverId encryptionKey:key contentType:contentType sourceFilename:sourceFilename];
     if (!self) {
         return self;
     }

src/Messages/Attachments/TSAttachmentStream.h

@@ -10,12 +10,15 @@
 NS_ASSUME_NONNULL_BEGIN
 
 @class TSAttachmentPointer;
+@class YapDatabaseReadWriteTransaction;
 
 @interface TSAttachmentStream : TSAttachment
 
 - (instancetype)init NS_UNAVAILABLE;
-- (instancetype)initWithContentType:(NSString *)contentType filename:(NSString *)filename NS_DESIGNATED_INITIALIZER;
+- (instancetype)initWithContentType:(NSString *)contentType
+                     sourceFilename:(nullable NSString *)sourceFilename NS_DESIGNATED_INITIALIZER;
 - (instancetype)initWithPointer:(TSAttachmentPointer *)pointer NS_DESIGNATED_INITIALIZER;
+- (nullable instancetype)initWithCoder:(NSCoder *)coder NS_DESIGNATED_INITIALIZER;
 
 // Though now required, `digest` may be null for pre-existing records or from
 // messages received from other clients
@@ -32,8 +35,10 @@ NS_ASSUME_NONNULL_BEGIN
 - (BOOL)isImage;
 - (BOOL)isVideo;
 - (BOOL)isAudio;
-- (nullable NSString *)filePath;
 - (nullable NSURL *)mediaURL;
+
+- (nullable NSString *)filePath;
+
 - (nullable NSData *)readDataFromFileWithError:(NSError **)error;
 - (BOOL)writeData:(NSData *)data error:(NSError **)error;
 

src/Messages/Attachments/TSAttachmentStream.m

@@ -6,15 +6,26 @@
 #import "MIMETypeUtil.h"
 #import "TSAttachmentPointer.h"
 #import <AVFoundation/AVFoundation.h>
+#import <YapDatabase/YapDatabase.h>
 #import <YapDatabase/YapDatabaseTransaction.h>
 
 NS_ASSUME_NONNULL_BEGIN
 
+@interface TSAttachmentStream ()
+
+// We only want to generate the file path for this attachment once, so that
+// changes in the file path generation logic don't break existing attachments.
+@property (nullable, nonatomic) NSString *localRelativeFilePath;
+
+@end
+
+#pragma mark -
+
 @implementation TSAttachmentStream
 
-- (instancetype)initWithContentType:(NSString *)contentType filename:(NSString *)filename
+- (instancetype)initWithContentType:(NSString *)contentType sourceFilename:(nullable NSString *)sourceFilename
 {
-    self = [super initWithContentType:contentType filename:filename];
+    self = [super initWithContentType:contentType sourceFilename:sourceFilename];
     if (!self) {
         return self;
     }
@@ -25,6 +36,9 @@ NS_ASSUME_NONNULL_BEGIN
     // attachments which haven't been uploaded yet.
     _isUploaded = NO;
 
+    // This instance hasn't been persisted yet.
+    [self ensureFilePathAndPersist:NO];
+
     return self;
 }
 
@@ -44,6 +58,23 @@ NS_ASSUME_NONNULL_BEGIN
     _isUploaded = YES;
     self.attachmentType = pointer.attachmentType;
 
+    // This instance hasn't been persisted yet.
+    [self ensureFilePathAndPersist:NO];
+
+    return self;
+}
+
+- (nullable instancetype)initWithCoder:(NSCoder *)coder
+{
+    self = [super initWithCoder:coder];
+    if (!self) {
+        return self;
+    }
+
+    // This instance has been persisted, we need to
+    // update it in the database.
+    [self ensureFilePathAndPersist:YES];
+
     return self;
 }
 
@@ -60,25 +91,75 @@ NS_ASSUME_NONNULL_BEGIN
     }
 }
 
-#pragma mark - TSYapDatabaseModel overrides
+- (void)ensureFilePathAndPersist:(BOOL)shouldPersist
-
-- (void)removeWithTransaction:(YapDatabaseReadWriteTransaction *)transaction
 {
-    [super removeWithTransaction:transaction];
+    if (self.localRelativeFilePath) {
-    [self removeFile];
+        return;
+    }
+
+    NSString *attachmentsFolder = [[self class] attachmentsFolder];
+    NSString *filePath = [MIMETypeUtil filePathForAttachment:self.uniqueId
+                                                  ofMIMEType:self.contentType
+                                              sourceFilename:self.sourceFilename
+                                                    inFolder:attachmentsFolder];
+    if (!filePath) {
+        DDLogError(@"%@ Could not generate path for attachment.", self.tag);
+        OWSAssert(0);
+        return;
+    }
+    if (![filePath hasPrefix:attachmentsFolder]) {
+        DDLogError(@"%@ Attachment paths should all be in the attachments folder.", self.tag);
+        OWSAssert(0);
+        return;
+    }
+    NSString *localRelativeFilePath = [filePath substringFromIndex:attachmentsFolder.length];
+    if (localRelativeFilePath.length < 1) {
+        DDLogError(@"%@ Empty local relative attachment paths.", self.tag);
+        OWSAssert(0);
+        return;
+    }
+
+    self.localRelativeFilePath = localRelativeFilePath;
+    OWSAssert(self.filePath);
+
+    if (shouldPersist) {
+        // It's not ideal to do this asynchronously, but we can create a new transaction
+        // within initWithCoder: which will be called from within a transaction.
+        dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
+            [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) {
+                OWSAssert(transaction);
+
+                [self saveWithTransaction:transaction];
+            }];
+        });
+    }
 }
 
 #pragma mark - File Management
 
 - (nullable NSData *)readDataFromFileWithError:(NSError **)error
 {
-    return [NSData dataWithContentsOfFile:self.filePath options:0 error:error];
+    *error = nil;
+    NSString *_Nullable filePath = self.filePath;
+    if (!filePath) {
+        DDLogError(@"%@ Missing path for attachment.", self.tag);
+        OWSAssert(0);
+        return nil;
+    }
+    return [NSData dataWithContentsOfFile:filePath options:0 error:error];
 }
 
 - (BOOL)writeData:(NSData *)data error:(NSError **)error
 {
-    DDLogInfo(@"%@ Created file at %@", self.tag, self.filePath);
+    *error = nil;
-    return [data writeToFile:self.filePath options:0 error:error];
+    NSString *_Nullable filePath = self.filePath;
+    if (!filePath) {
+        DDLogError(@"%@ Missing path for attachment.", self.tag);
+        OWSAssert(0);
+        return NO;
+    }
+    DDLogInfo(@"%@ Writing attachment to file: %@", self.tag, filePath);
+    return [data writeToFile:filePath options:0 error:error];
 }
 
 + (NSString *)attachmentsFolder
@@ -114,28 +195,47 @@ NS_ASSUME_NONNULL_BEGIN
 
 - (nullable NSString *)filePath
 {
-    return [MIMETypeUtil filePathForAttachment:self.uniqueId
+    if (!self.localRelativeFilePath) {
-                                    ofMIMEType:self.contentType
+        OWSAssert(0);
-                                      filename:self.filename
+        return nil;
-                                      inFolder:[[self class] attachmentsFolder]];
+    }
+
+    return [[[self class] attachmentsFolder] stringByAppendingPathComponent:self.localRelativeFilePath];
 }
 
 - (nullable NSURL *)mediaURL
 {
-    NSString *filePath = self.filePath;
+    NSString *_Nullable filePath = self.filePath;
-    return filePath ? [NSURL fileURLWithPath:filePath] : nil;
+    if (!filePath) {
+        DDLogError(@"%@ Missing path for attachment.", self.tag);
+        OWSAssert(0);
+        return nil;
+    }
+    return [NSURL fileURLWithPath:filePath];
 }
 
-- (void)removeFile
+- (void)removeFileWithTransaction:(YapDatabaseReadWriteTransaction *)transaction
 {
+    NSString *_Nullable filePath = self.filePath;
+    if (!filePath) {
+        DDLogError(@"%@ Missing path for attachment.", self.tag);
+        OWSAssert(0);
+        return;
+    }
     NSError *error;
-    [[NSFileManager defaultManager] removeItemAtPath:[self filePath] error:&error];
+    [[NSFileManager defaultManager] removeItemAtPath:filePath error:&error];
 
     if (error) {
         DDLogError(@"%@ remove file errored with: %@", self.tag, error);
     }
 }
 
+- (void)removeWithTransaction:(YapDatabaseReadWriteTransaction *)transaction
+{
+    [super removeWithTransaction:transaction];
+    [self removeFileWithTransaction:transaction];
+}
+
 - (BOOL)isAnimated {
     return [MIMETypeUtil isAnimated:self.contentType];
 }
@@ -157,14 +257,21 @@ NS_ASSUME_NONNULL_BEGIN
     if ([self isVideo] || [self isAudio]) {
         return [self videoThumbnail];
     } else {
-        // [self isAnimated] || [self isImage]
+        NSURL *_Nullable mediaUrl = [self mediaURL];
-        return [UIImage imageWithData:[NSData dataWithContentsOfURL:[self mediaURL]]];
+        if (!mediaUrl) {
+            return nil;
+        }
+        return [UIImage imageWithData:[NSData dataWithContentsOfURL:mediaUrl]];
     }
 }
 
 - (nullable UIImage *)videoThumbnail
 {
-    AVURLAsset *asset = [[AVURLAsset alloc] initWithURL:[NSURL fileURLWithPath:self.filePath] options:nil];
+    NSURL *_Nullable mediaUrl = [self mediaURL];
+    if (!mediaUrl) {
+        return nil;
+    }
+    AVURLAsset *asset = [[AVURLAsset alloc] initWithURL:mediaUrl options:nil];
     AVAssetImageGenerator *generate         = [[AVAssetImageGenerator alloc] initWithAsset:asset];
     generate.appliesPreferredTrackTransform = YES;
     NSError *err                            = NULL;

src/Messages/Interactions/TSOutgoingMessage.h

@@ -89,7 +89,7 @@ typedef NS_ENUM(NSInteger, TSGroupMetaMessage) {
 @property (atomic, readonly) BOOL hasSyncedTranscript;
 @property (atomic, readonly) NSString *customMessage;
 @property (atomic, readonly) NSString *mostRecentFailureText;
-// A map of attachment id-to-filename.
+// A map of attachment id-to-"source" filename.
 @property (nonatomic, readonly) NSMutableDictionary<NSString *, NSString *> *attachmentFilenameMap;
 
 @property (atomic, readonly) TSGroupMetaMessage groupMetaMessage;

src/Messages/Interactions/TSOutgoingMessage.m

@@ -435,8 +435,8 @@ NSString *const kTSOutgoingMessageSentRecipientAll = @"kTSOutgoingMessageSentRec
     if (!attachmentWasGroupAvatar) {
         NSMutableArray *attachments = [NSMutableArray new];
         for (NSString *attachmentId in self.attachmentIds) {
-            NSString *filename = self.attachmentFilenameMap[attachmentId];
+            NSString *sourceFilename = self.attachmentFilenameMap[attachmentId];
-            [attachments addObject:[self buildAttachmentProtoForAttachmentId:attachmentId filename:filename]];
+            [attachments addObject:[self buildAttachmentProtoForAttachmentId:attachmentId filename:sourceFilename]];
         }
         [builder setAttachmentsArray:attachments];
     }

src/Messages/OWSMessageSender.h

@@ -71,7 +71,7 @@ NS_SWIFT_NAME(MessageSender)
  */
 - (void)sendAttachmentData:(NSData *)attachmentData
                contentType:(NSString *)contentType
-                  filename:(nullable NSString *)filename
+            sourceFilename:(nullable NSString *)sourceFilename
                  inMessage:(TSOutgoingMessage *)outgoingMessage
                    success:(void (^)())successHandler
                    failure:(void (^)(NSError *error))failureHandler;

src/Messages/OWSMessageSender.m

@@ -493,7 +493,7 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException";
 
     [self sendAttachmentData:attachmentData
                  contentType:contentType
-                    filename:nil
+              sourceFilename:nil
                    inMessage:message
                      success:successWithDeleteHandler
                      failure:failureWithDeleteHandler];
@@ -501,7 +501,7 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException";
 
 - (void)sendAttachmentData:(NSData *)data
                contentType:(NSString *)contentType
-                  filename:(nullable NSString *)filename
+            sourceFilename:(nullable NSString *)sourceFilename
                  inMessage:(TSOutgoingMessage *)message
                    success:(void (^)())successHandler
                    failure:(void (^)(NSError *error))failureHandler
@@ -515,7 +515,7 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException";
 
     dispatch_async([OWSDispatch attachmentsQueue], ^{
         TSAttachmentStream *attachmentStream =
-            [[TSAttachmentStream alloc] initWithContentType:contentType filename:filename];
+            [[TSAttachmentStream alloc] initWithContentType:contentType sourceFilename:sourceFilename];
         if (message.isVoiceMessage) {
             attachmentStream.attachmentType = TSAttachmentTypeVoiceMessage;
         }
@@ -529,8 +529,8 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException";
 
         [attachmentStream save];
         [message.attachmentIds addObject:attachmentStream.uniqueId];
-        if (filename) {
+        if (sourceFilename) {
-            message.attachmentFilenameMap[attachmentStream.uniqueId] = filename;
+            message.attachmentFilenameMap[attachmentStream.uniqueId] = sourceFilename;
         }
         [message save];
 

src/Messages/TSMessagesManager.m

@@ -758,7 +758,7 @@ NS_ASSUME_NONNULL_BEGIN
     if (gThread.groupModel.groupImage) {
         [self.messageSender sendAttachmentData:UIImagePNGRepresentation(gThread.groupModel.groupImage)
             contentType:OWSMimeTypeImagePng
-            filename:nil
+            sourceFilename:nil
             inMessage:message
             success:^{
                 DDLogDebug(@"%@ Successfully sent group update with avatar", self.tag);

src/Util/MIMETypeUtil.h

@@ -34,7 +34,7 @@ extern NSString *const OWSMimeTypeUnknownForTests;
 // filename is optional and should not be trusted.
 + (nullable NSString *)filePathForAttachment:(NSString *)uniqueId
                                   ofMIMEType:(NSString *)contentType
-                                    filename:(nullable NSString *)filename
+                              sourceFilename:(nullable NSString *)sourceFilename
                                     inFolder:(NSString *)folder;
 
 + (NSSet<NSString *> *)supportedVideoUTITypes;

src/Util/MIMETypeUtil.m

@@ -267,14 +267,14 @@ NSString *const OWSMimeTypeUnknownForTests = @"unknown/mimetype";
 
 + (nullable NSString *)filePathForAttachment:(NSString *)uniqueId
                                   ofMIMEType:(NSString *)contentType
-                                    filename:(nullable NSString *)filename
+                              sourceFilename:(nullable NSString *)sourceFilename
                                     inFolder:(NSString *)folder
 {
     NSString *kDefaultFileExtension = @"bin";
 
-    if (filename.length > 0) {
+    if (sourceFilename.length > 0) {
         NSString *normalizedFilename =
-            [filename stringByTrimmingCharactersInSet:[NSCharacterSet whitespaceCharacterSet]];
+            [sourceFilename stringByTrimmingCharactersInSet:[NSCharacterSet whitespaceCharacterSet]];
         // Ensure that the filename is a valid filesystem name,
         // replacing invalid characters with an underscore.
         for (NSCharacterSet *invalidCharacterSet in @[