Fix a bug where migrating directories stomped on migrated commands. When the...

Fix a bug where migrating directories stomped on migrated commands. When the backing store type changes, just change the persistent store type instead of destroying and re-creating the world, losing the user's data in the process.

iTerm2.xcodeproj/project.pbxproj

@@ -2549,7 +2549,7 @@
 		A6CFDAD5185D53C2005DC94B /* AsyncHostLookupController.h */ = {isa = PBXFileReference; fileEncoding = 4; indentWidth = 4; lastKnownFileType = sourcecode.c.h; path = AsyncHostLookupController.h; sourceTree = "<group>"; tabWidth = 4; };
 		A6CFDAD6185D53C2005DC94B /* AsyncHostLookupController.m */ = {isa = PBXFileReference; fileEncoding = 4; indentWidth = 4; lastKnownFileType = sourcecode.c.objc; path = AsyncHostLookupController.m; sourceTree = "<group>"; tabWidth = 4; };
 		A6D22A421BC8BE6B004084E0 /* Model.xcdatamodel */ = {isa = PBXFileReference; lastKnownFileType = wrapper.xcdatamodel; path = Model.xcdatamodel; sourceTree = "<group>"; };
-		A6D22B431BC9D368004084E0 /* iTermShellHistoryTest.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = iTermShellHistoryTest.m; sourceTree = "<group>"; };
+		A6D22B431BC9D368004084E0 /* iTermShellHistoryTest.m */ = {isa = PBXFileReference; fileEncoding = 4; indentWidth = 4; lastKnownFileType = sourcecode.c.objc; path = iTermShellHistoryTest.m; sourceTree = "<group>"; };
 		A6DF401A1897607E00F05947 /* NSTextField+iTerm.h */ = {isa = PBXFileReference; fileEncoding = 4; indentWidth = 4; lastKnownFileType = sourcecode.c.h; path = "NSTextField+iTerm.h"; sourceTree = "<group>"; tabWidth = 4; };
 		A6DF401B1897607E00F05947 /* NSTextField+iTerm.m */ = {isa = PBXFileReference; fileEncoding = 4; indentWidth = 4; lastKnownFileType = sourcecode.c.objc; path = "NSTextField+iTerm.m"; sourceTree = "<group>"; tabWidth = 4; };
 		A6DF7B711A5BA33900D96409 /* iTermParser.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = iTermParser.h; sourceTree = "<group>"; };

iTerm2XCTests/iTermShellHistoryTest.m

@@ -622,6 +622,69 @@ static NSTimeInterval kDefaultTime = 10000000;
 #pragma mark - Generic
+// There was a bug where migrating directories would stop on migrated commands. This tests migrating
+// both to ensure this isn't an issue.
+- (void)testCommandHistoryAndDirectoryMigration {
+  _now = kDefaultTime;
+
+  NSDictionary *commandDictionary =
+        @{
+            @"user1@host1":
+               @[
+                   @{
+                       @"command": @"command1",
+                       @"uses": @10,
+                       @"last used": @(_now),
+                       @"use times":
+                           @[
+                               @[ @(_now),
+                                  @"/path1",
+                                  @"mark-guid-1",
+                                  @"command1",
+                                ],
+                            ]
+                    }
+                ],
+         };
+    NSString *commandPlistPath = [kFakeCommandHistoryPlistPath stringByAppendingString:_guid];
+    [NSKeyedArchiver archiveRootObject:commandDictionary toFile:commandPlistPath];
+    XCTAssert([[NSFileManager defaultManager] fileExistsAtPath:commandPlistPath isDirectory:nil]);
+
+    NSDictionary *directoryDictionary =
+        @{
+            @"user1@host1":
+               @[
+                   @{
+                       @"path": @"/abc/def/ghi/good",
+                       @"use count": @5,
+                       @"last use": @(_now),
+                       @"starred": @NO,
+                    },
+                ]
+         };
+    NSString *directoriesPlistPath = [kFakeDirectoriesPlistPath stringByAppendingString:_guid];
+    [NSKeyedArchiver archiveRootObject:directoryDictionary toFile:directoriesPlistPath];
+    XCTAssert([[NSFileManager defaultManager] fileExistsAtPath:directoriesPlistPath isDirectory:nil]);
+
+    iTermShellHistoryController *historyController =
+        [[[iTermShellHistoryControllerForTesting alloc] initWithGuid:_guid] autorelease];
+
+    XCTAssert(![[NSFileManager defaultManager] fileExistsAtPath:commandPlistPath isDirectory:nil]);
+    XCTAssert(![[NSFileManager defaultManager] fileExistsAtPath:directoriesPlistPath isDirectory:nil]);
+
+    VT100RemoteHost *remoteHost = [[[VT100RemoteHost alloc] init] autorelease];
+    remoteHost.username = @"user1";
+    remoteHost.hostname = @"host1";
+    NSArray<iTermCommandHistoryEntryMO *> *actualEntries =
+        [historyController commandHistoryEntriesWithPrefix:@""
+                                                    onHost:remoteHost];
+    XCTAssertEqual(1, actualEntries.count);
+
+    NSArray<iTermRecentDirectoryMO *> *actualDirectories =
+        [historyController directoriesSortedByScoreOnHost:remoteHost];
+    XCTAssertEqual(1, actualDirectories.count);
+}
+
 - (void)testCorruptDatabase {
     iTermShellHistoryControllerForTesting *historyController =
         [[[iTermShellHistoryControllerForTesting alloc] initWithGuid:_guid] autorelease];

sources/iTermShellHistoryController.m

@@ -143,7 +143,12 @@ static const NSTimeInterval kMaxTimeToRememberDirectories = 60 * 60 * 24 * 90;
 }
 - (NSString *)pathToDatabase {
-    return [self pathForFileNamed:self.databaseFilenamePrefix];
+    NSString *path = [self pathForFileNamed:self.databaseFilenamePrefix];
+    if (!self.shouldSaveToDisk) {
+        // For migration to work the in-memory path must be different than the on-disk path.
+        path = [path stringByAppendingString:@".ram"];
+    }
+    return path;
 }
 #pragma mark - Core Data
@@ -295,16 +300,17 @@ static const NSTimeInterval kMaxTimeToRememberDirectories = 60 * 60 * 24 * 90;
 // Returns YES if a migration was attempted.
 - (BOOL)migrateFromPlistToCoreData {
     BOOL attempted = NO;
-    if ([self migrateCommandHistoryFromPlistToCoreData]) {
+    NSMutableDictionary *records = [NSMutableDictionary dictionary];
+    if ([self migrateCommandHistoryFromPlistToCoreData:records]) {
         attempted = YES;
     }
-    if ([self migrateDirectoriesFromPlistToCoreData]) {
+    if ([self migrateDirectoriesFromPlistToCoreData:records]) {
         attempted = YES;
     }
     return attempted;
 }
-- (BOOL)migrateDirectoriesFromPlistToCoreData {
+- (BOOL)migrateDirectoriesFromPlistToCoreData:(NSMutableDictionary *)records {
     NSString *path = [self pathToDeprecatedDirectoriesPlist];
     if (![[NSFileManager defaultManager] fileExistsAtPath:path isDirectory:nil]) {
         return NO;
@@ -315,9 +321,10 @@ static const NSTimeInterval kMaxTimeToRememberDirectories = 60 * 60 * 24 * 90;
         if (parts.count != 2) {
             continue;
         }
-        iTermHostRecordMO *hostRecord = _records[host];
+        iTermHostRecordMO *hostRecord = records[host];
         if (!hostRecord) {
             hostRecord = [iTermHostRecordMO hostRecordInContext:_managedObjectContext];
+            records[host] = hostRecord;
         }
         hostRecord.username = parts[0];
         hostRecord.hostname = parts[1];
@@ -338,7 +345,7 @@ static const NSTimeInterval kMaxTimeToRememberDirectories = 60 * 60 * 24 * 90;
     return YES;
 }
-- (BOOL)migrateCommandHistoryFromPlistToCoreData {
+- (BOOL)migrateCommandHistoryFromPlistToCoreData:(NSMutableDictionary *)records {
     NSString *path = [self pathToDeprecatedCommandHistoryPlist];
     if (![[NSFileManager defaultManager] fileExistsAtPath:path isDirectory:nil]) {
         return NO;
@@ -349,9 +356,10 @@ static const NSTimeInterval kMaxTimeToRememberDirectories = 60 * 60 * 24 * 90;
         if (parts.count != 2) {
             continue;
         }
-        iTermHostRecordMO *hostRecord = _records[host];
+        iTermHostRecordMO *hostRecord = records[host];
         if (!hostRecord) {
             hostRecord = [iTermHostRecordMO hostRecordInContext:_managedObjectContext];
+            records[host] = hostRecord;
         }
         hostRecord.username = parts[0];
         hostRecord.hostname = parts[1];
@@ -402,26 +410,32 @@ static const NSTimeInterval kMaxTimeToRememberDirectories = 60 * 60 * 24 * 90;
 }
 - (void)backingStoreTypeDidChange {
-    if (!self.shouldSaveToDisk) {
-        [self eraseCommandHistory:YES directories:YES];
+    NSPersistentStore *store =
+        _managedObjectContext.persistentStoreCoordinator.persistentStores.firstObject;
+    NSString *storeType = self.shouldSaveToDisk ? NSSQLiteStoreType : NSInMemoryStoreType;
+    if ([store.type isEqualToString:storeType]) {
+        // No change
+        return;
     }
-    [_managedObjectContext release];
-    _managedObjectContext = nil;
-    [self initializeCoreDataWithRetry:YES vacuum:NO];
-
-    // Reload everything.
-    [_records removeAllObjects];
-    [_expandedCache removeAllObjects];
-    [_tree release];
-    _tree = [[iTermDirectoryTree alloc] init];
-    [self loadObjectGraph];
-    if (!_initializing) {
-        [[NSNotificationCenter defaultCenter] postNotificationName:kDirectoriesDidChangeNotificationName
-                                                            object:nil];
-        [[NSNotificationCenter defaultCenter] postNotificationName:kCommandHistoryDidChangeNotificationName
-                                                            object:nil];
+    // Change the store to the new type
+    NSError *error = nil;
+    [_managedObjectContext.persistentStoreCoordinator migratePersistentStore:store
+                                                                       toURL:[NSURL fileURLWithPath:[self pathToDatabase]]
+                                                                     options:@{}
+                                                                    withType:storeType
+                                                                       error:&error];
+    if (error) {
+        NSLog(@"Failed to migrate to on-disk storage: %@", error);
+        // Do it the hard way.
+        [_managedObjectContext release];
+        _managedObjectContext = nil;
+        [self initializeCoreDataWithRetry:YES vacuum:NO];
+    }
+    // Erase files containing user data.
+    if (!self.shouldSaveToDisk) {
+        [self deleteDatabase];
     }
 }
@@ -691,8 +705,8 @@ static const NSTimeInterval kMaxTimeToRememberDirectories = 60 * 60 * 24 * 90;
     for (iTermCommandHistoryCommandUseMO *commandUse in results) {
         iTermCommandHistoryEntryMO *entry = commandUse.entry;
         [entry removeUsesObject:commandUse];
-        
-        if (entry.uses.count == 0) {
+
+        if (entry && entry.uses.count == 0) {
             [_managedObjectContext deleteObject:entry];
         }
     }