Merge branch 'charlesmchen/orphanCleanup'

Podfile.lock

@@ -136,7 +136,7 @@ CHECKOUT OPTIONS:
     :commit: 521686c112bbae7a762f85d52b1e41eeb1760772
     :git: https://github.com/WhisperSystems/JSQMessagesViewController.git
   SignalServiceKit:
-    :commit: 9115a1f973c0ad3c381c70382f1fbbf6f3cecdd6
+    :commit: 640ec13b2e4bd21a9d9361ffcd57860359b1fbe5
     :git: https://github.com/WhisperSystems/SignalServiceKit.git
   SocketRocket:
     :commit: 877ac7438be3ad0b45ef5ca3969574e4b97112bf

Signal/src/Models/OWSMessagesBubblesSizeCalculator.m

@@ -100,7 +100,7 @@ NS_ASSUME_NONNULL_BEGIN
         TSInteraction *interaction = ((OWSCall *)messageData).interaction;
         return [self sizeForSystemMessage:interaction cacheKey:cacheKey layout:layout];
     } else {
-        OWSFail(@"Can't size unknown message data type: %@", [messageData class]);
+        // Ignore unknown message types; the tests use mocks.
     }
 
     // BEGIN HACK iOS10EmojiBug see: https://github.com/WhisperSystems/Signal-iOS/issues/1368

Signal/src/ViewControllers/DebugUI/DebugUIDiskUsage.m

@@ -5,6 +5,7 @@
 #import "DebugUIDiskUsage.h"
 #import "OWSTableViewController.h"
 #import "Signal-Swift.h"
+#import <SignalServiceKit/OWSOrphanedDataCleaner.h>
 #import <SignalServiceKit/TSDatabaseView.h>
 #import <SignalServiceKit/TSInteraction.h>
 #import <SignalServiceKit/TSStorageManager.h>
@@ -38,11 +39,11 @@ NS_ASSUME_NONNULL_BEGIN
                                        items:@[
                                            [OWSTableItem itemWithTitle:@"Audit & Log"
                                                            actionBlock:^{
-                                                               [DebugUIDiskUsage auditWithoutCleanup];
+                                                               [OWSOrphanedDataCleaner auditAsync];
                                                            }],
                                            [OWSTableItem itemWithTitle:@"Audit & Clean Up"
                                                            actionBlock:^{
-                                                               [DebugUIDiskUsage auditWithCleanup];
+                                                               [OWSOrphanedDataCleaner auditAndCleanupAsync:nil];
                                                            }],
                                            [OWSTableItem itemWithTitle:@"Save All Attachments"
                                                            actionBlock:^{
@@ -55,173 +56,6 @@ NS_ASSUME_NONNULL_BEGIN
                                        ]];
 }
 
-+ (void)auditWithoutCleanup
-{
-    [self auditAndCleanup:NO];
-}
-
-+ (void)auditWithCleanup
-{
-    [self auditAndCleanup:YES];
-}
-
-+ (void)auditAndCleanup:(BOOL)shouldCleanup
-{
-    NSString *attachmentsFolder = [TSAttachmentStream attachmentsFolder];
-    DDLogError(@"attachmentsFolder: %@", attachmentsFolder);
-
-    __block int fileCount = 0;
-    __block long long totalFileSize = 0;
-    NSMutableSet *diskFilePaths = [NSMutableSet new];
-    __unsafe_unretained __block void (^visitAttachmentFilesRecursable)(NSString *);
-    void (^visitAttachmentFiles)(NSString *);
-    visitAttachmentFiles = ^(NSString *dirPath) {
-        NSError *error;
-        NSArray<NSString *> *fileNames =
-            [[NSFileManager defaultManager] contentsOfDirectoryAtPath:dirPath error:&error];
-        if (error) {
-            OWSFail(@"contentsOfDirectoryAtPath error: %@", error);
-            return;
-        }
-        for (NSString *fileName in fileNames) {
-            NSString *filePath = [dirPath stringByAppendingPathComponent:fileName];
-            BOOL isDirectory;
-            [[NSFileManager defaultManager] fileExistsAtPath:filePath isDirectory:&isDirectory];
-            if (isDirectory) {
-                visitAttachmentFilesRecursable(filePath);
-            } else {
-                NSNumber *fileSize =
-                    [[NSFileManager defaultManager] attributesOfItemAtPath:filePath error:&error][NSFileSize];
-                if (error) {
-                    OWSFail(@"attributesOfItemAtPath: %@ error: %@", filePath, error);
-                    continue;
-                }
-                totalFileSize += fileSize.longLongValue;
-                fileCount++;
-                [diskFilePaths addObject:filePath];
-            }
-        }
-    };
-    visitAttachmentFilesRecursable = visitAttachmentFiles;
-    visitAttachmentFiles(attachmentsFolder);
-
-    __block int attachmentStreamCount = 0;
-    NSMutableSet<NSString *> *attachmentFilePaths = [NSMutableSet new];
-    NSMutableSet<NSString *> *attachmentIds = [NSMutableSet new];
-    TSStorageManager *storageManager = [TSStorageManager sharedManager];
-    [storageManager.newDatabaseConnection readWithBlock:^(YapDatabaseReadTransaction *_Nonnull transaction) {
-        [transaction enumerateKeysAndObjectsInCollection:TSAttachmentStream.collection
-                                              usingBlock:^(NSString *key, TSAttachment *attachment, BOOL *stop) {
-                                                  [attachmentIds addObject:attachment.uniqueId];
-                                                  if (![attachment isKindOfClass:[TSAttachmentStream class]]) {
-                                                      return;
-                                                  }
-                                                  TSAttachmentStream *attachmentStream
-                                                      = (TSAttachmentStream *)attachment;
-                                                  attachmentStreamCount++;
-                                                  NSString *_Nullable filePath = [attachmentStream filePath];
-                                                  OWSAssert(filePath);
-                                                  [attachmentFilePaths addObject:filePath];
-                                              }];
-    }];
-
-    DDLogError(@"fileCount: %d", fileCount);
-    DDLogError(@"totalFileSize: %lld", totalFileSize);
-    DDLogError(@"attachmentStreams: %d", attachmentStreamCount);
-    DDLogError(@"attachmentStreams with file paths: %zd", attachmentFilePaths.count);
-
-    NSMutableSet<NSString *> *orphanDiskFilePaths = [diskFilePaths mutableCopy];
-    [orphanDiskFilePaths minusSet:attachmentFilePaths];
-    NSMutableSet<NSString *> *missingAttachmentFilePaths = [attachmentFilePaths mutableCopy];
-    [missingAttachmentFilePaths minusSet:diskFilePaths];
-
-    DDLogError(@"orphan disk file paths: %zd", orphanDiskFilePaths.count);
-    DDLogError(@"missing attachment file paths: %zd", missingAttachmentFilePaths.count);
-
-    [self printPaths:orphanDiskFilePaths.allObjects label:@"orphan disk file paths"];
-    [self printPaths:missingAttachmentFilePaths.allObjects label:@"missing attachment file paths"];
-
-    NSMutableSet *threadIds = [NSMutableSet new];
-    [storageManager.newDatabaseConnection readWithBlock:^(YapDatabaseReadTransaction *_Nonnull transaction) {
-        [transaction enumerateKeysInCollection:TSThread.collection
-                                    usingBlock:^(NSString *_Nonnull key, BOOL *_Nonnull stop) {
-                                        [threadIds addObject:key];
-                                    }];
-    }];
-
-    NSMutableSet<TSInteraction *> *orphanInteractions = [NSMutableSet new];
-    NSMutableSet<NSString *> *messageAttachmentIds = [NSMutableSet new];
-    [storageManager.newDatabaseConnection readWithBlock:^(YapDatabaseReadTransaction *_Nonnull transaction) {
-        [transaction enumerateKeysAndObjectsInCollection:TSMessage.collection
-                                              usingBlock:^(NSString *key, TSInteraction *interaction, BOOL *stop) {
-                                                  if (![threadIds containsObject:interaction.uniqueThreadId]) {
-                                                      [orphanInteractions addObject:interaction];
-                                                  }
-
-                                                  if (![interaction isKindOfClass:[TSMessage class]]) {
-                                                      return;
-                                                  }
-                                                  TSMessage *message = (TSMessage *)interaction;
-                                                  if (message.attachmentIds.count > 0) {
-                                                      [messageAttachmentIds addObjectsFromArray:message.attachmentIds];
-                                                  }
-                                              }];
-    }];
-
-    DDLogError(@"attachmentIds: %zd", attachmentIds.count);
-    DDLogError(@"messageAttachmentIds: %zd", messageAttachmentIds.count);
-
-    NSMutableSet<NSString *> *orphanAttachmentIds = [attachmentIds mutableCopy];
-    [orphanAttachmentIds minusSet:messageAttachmentIds];
-    NSMutableSet<NSString *> *missingAttachmentIds = [messageAttachmentIds mutableCopy];
-    [missingAttachmentIds minusSet:attachmentIds];
-
-    DDLogError(@"orphan attachmentIds: %zd", orphanAttachmentIds.count);
-    DDLogError(@"missing attachmentIds: %zd", missingAttachmentIds.count);
-
-    DDLogError(@"orphan interactions: %zd", orphanInteractions.count);
-
-    if (shouldCleanup) {
-        [storageManager.newDatabaseConnection
-            readWriteWithBlock:^(YapDatabaseReadWriteTransaction *_Nonnull transaction) {
-                for (TSInteraction *interaction in orphanInteractions) {
-                    [interaction removeWithTransaction:transaction];
-                }
-                for (NSString *attachmentId in orphanAttachmentIds) {
-                    TSAttachment *attachment = [TSAttachment fetchObjectWithUniqueID:attachmentId];
-                    OWSAssert(attachment);
-                    if (![attachment isKindOfClass:[TSAttachmentStream class]]) {
-                        continue;
-                    }
-                    TSAttachmentStream *attachmentStream = (TSAttachmentStream *)attachment;
-                    // Don't delete attachments which were created in the last N minutes.
-                    const NSTimeInterval kMinimumOrphanAttachmentAge = 2 * 60.f;
-                    if (fabs([attachmentStream.creationTimestamp timeIntervalSinceNow]) < kMinimumOrphanAttachmentAge) {
-                        DDLogInfo(@"Skipping orphan attachment due to age: %f",
-                            fabs([attachmentStream.creationTimestamp timeIntervalSinceNow]));
-                        continue;
-                    }
-                    [attachmentStream removeWithTransaction:transaction];
-                }
-            }];
-
-        for (NSString *filePath in orphanDiskFilePaths) {
-            NSError *error;
-            [[NSFileManager defaultManager] removeItemAtPath:filePath error:&error];
-            if (error) {
-                OWSFail(@"Could not remove orphan file at: %@", filePath);
-            }
-        }
-    }
-}
-
-+ (void)printPaths:(NSArray<NSString *> *)paths label:(NSString *)label
-{
-    for (NSString *path in [paths sortedArrayUsingSelector:@selector(compare:)]) {
-        DDLogError(@"%@: %@", label, path);
-    }
-}
-
 + (void)saveAllAttachments
 {
     TSStorageManager *storageManager = [TSStorageManager sharedManager];

Signal/src/environment/VersionMigrations.m

@@ -80,16 +80,16 @@
         [self clearBloomFilterCache];
     }
 
-    if ([self isVersion:previousVersion atLeast:@"2.0.0" andLessThan:@"2.4.1"] && [TSAccountManager isRegistered]) {
+#ifdef DEBUG
-        // Cleaning orphaned data can take a while, so let's run it in the background.
+    // A bug in orphan cleanup could be disastrous so let's only
-        // This means this migration is not resiliant to failures - we'll only run it once
+    // run it in DEBUG builds for a few releases.
-        // regardless of its success.
+    //
-        dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
+    // TODO: Release to production once we have analytics.
-            DDLogInfo(@"OWSMigration: beginning removing orphaned data.");
+    // TODO: Orphan cleanup is somewhat expensive - not least in doing a bunch
-            [[OWSOrphanedDataCleaner new] removeOrphanedData];
+    //       of disk access.  We might want to only run it "once per version"
-            DDLogInfo(@"OWSMigration: completed removing orphaned data.");
+    //       or something like that in production.
-        });
+    [OWSOrphanedDataCleaner auditAndCleanupAsync:nil];
-    }
+#endif
 
     [[[OWSDatabaseMigrationRunner alloc] initWithStorageManager:[TSStorageManager sharedManager]] runAllOutstanding];
 }

Signal/test/Models/AccountManagerTest.swift

@@ -42,7 +42,7 @@ class TokenObtainingTSAccountManager: VerifyingTSAccountManager {
 
 class AccountManagerTest: XCTestCase {
 
-    let tsAccountManager = FailingTSAccountManager(networkManager: TSNetworkManager.sharedManager() as! TSNetworkManager, storageManager: TSStorageManager.shared())
+    let tsAccountManager = FailingTSAccountManager(networkManager: TSNetworkManager.shared(), storageManager: TSStorageManager.shared())
 
     func testRegisterWhenEmptyCode() {
         let accountManager = AccountManager(textSecureAccountManager: tsAccountManager)
@@ -86,7 +86,7 @@ class AccountManagerTest: XCTestCase {
     }
 
     func testSuccessfulRegistration() {
-        let tsAccountManager = TokenObtainingTSAccountManager(networkManager: TSNetworkManager.sharedManager() as! TSNetworkManager, storageManager: TSStorageManager.shared())
+        let tsAccountManager = TokenObtainingTSAccountManager(networkManager: TSNetworkManager.shared(), storageManager: TSStorageManager.shared())
 
         let accountManager = AccountManager(textSecureAccountManager: tsAccountManager)
 

Signal/test/ViewControllers/Signals/TSMessageAdapters/TSMessageAdapterTest.m

@@ -103,7 +103,7 @@
 {
     TSAttachmentStream *videoAttachment =
     [[TSAttachmentStream alloc] initWithContentType:@"video/mp4" sourceFilename:nil];
-
+    [videoAttachment save];
     self.messageAdapter.mediaItem = [[TSVideoAttachmentAdapter alloc] initWithAttachment:videoAttachment incoming:NO];
 
     XCTAssertTrue([self.messageAdapter canPerformEditingAction:@selector(delete:)]);
@@ -118,6 +118,7 @@
 {
     TSAttachmentStream *audioAttachment =
         [[TSAttachmentStream alloc] initWithContentType:@"audio/mp3" sourceFilename:nil];
+    [audioAttachment save];
     self.messageAdapter.mediaItem = [[TSVideoAttachmentAdapter alloc] initWithAttachment:audioAttachment incoming:NO];
 
     XCTAssertTrue([self.messageAdapter canPerformEditingAction:@selector(delete:)]);
@@ -253,6 +254,7 @@
     TSAttachmentStream *videoAttachment =
         [[TSAttachmentStream alloc] initWithContentType:@"video/mp4" sourceFilename:nil];
     [videoAttachment writeData:self.fakeVideoData error:&error];
+    [videoAttachment save];
     self.messageAdapter.mediaItem = [[TSVideoAttachmentAdapter alloc] initWithAttachment:videoAttachment incoming:YES];
 
     [self.messageAdapter performEditingAction:@selector(copy:)];
@@ -270,6 +272,7 @@
     TSAttachmentStream *audioAttachment =
         [[TSAttachmentStream alloc] initWithContentType:@"audio/mp3" sourceFilename:nil];
     [audioAttachment writeData:self.fakeAudioData error:&error];
+    [audioAttachment save];
     self.messageAdapter.mediaItem = [[TSVideoAttachmentAdapter alloc] initWithAttachment:audioAttachment incoming:NO];
 
     [self.messageAdapter performEditingAction:@selector(copy:)];
@@ -285,6 +288,7 @@
     TSAttachmentStream *audioAttachment =
         [[TSAttachmentStream alloc] initWithContentType:@"audio/x-m4a" sourceFilename:nil];
     [audioAttachment writeData:self.fakeAudioData error:&error];
+    [audioAttachment save];
     self.messageAdapter.mediaItem = [[TSVideoAttachmentAdapter alloc] initWithAttachment:audioAttachment incoming:NO];
 
     [self.messageAdapter performEditingAction:@selector(copy:)];
@@ -300,6 +304,7 @@
     TSAttachmentStream *audioAttachment =
         [[TSAttachmentStream alloc] initWithContentType:@"audio/wav" sourceFilename:nil];
     [audioAttachment writeData:self.fakeAudioData error:&error];
+    [audioAttachment save];
     self.messageAdapter.mediaItem = [[TSVideoAttachmentAdapter alloc] initWithAttachment:audioAttachment incoming:NO];
 
     [self.messageAdapter performEditingAction:@selector(copy:)];

Signal/test/contact/ContactsPickerTest.swift

@@ -3,6 +3,7 @@
 //
 
 import XCTest
+import Contacts
 @testable import Signal
 import Contacts