- Address favourites saving on quit, which appears to sometimes die during preference serialisation, causing Issue #1332. Reworking the order of commands ensure this is performed more safely.

 - Move upgrade routine to only be called on version upgrades, tweak order of calls

3 files changed, 66 insertions, 67 deletions

diff --git a/Source/SPAppController.m b/Source/SPAppController.m
index 87a97f19..ed8fd400 100644
--- a/Source/SPAppController.m
+++ b/Source/SPAppController.m
@@ -93,9 +93,6 @@ YY_BUFFER_STATE yy_scan_string (const char *);
 {
 	// Register application defaults
 	[[NSUserDefaults standardUserDefaults] registerDefaults:[NSDictionary dictionaryWithContentsOfFile:[[NSBundle mainBundle] pathForResource:@"PreferenceDefaults" ofType:@"plist"]]];
-
-	// Migrate old connection favorites to the app's support directory (only uncomment when ready)
-	SPMigrateConnectionFavoritesData();
 }
 
 /**
diff --git a/Source/SPFavoritesController.m b/Source/SPFavoritesController.m
index 0a069b3a..da8be7c6 100644
--- a/Source/SPFavoritesController.m
+++ b/Source/SPFavoritesController.m
@@ -356,7 +356,21 @@ static SPFavoritesController *sharedFavoritesController = nil;
 	
 	NSError *error = nil;
 	NSString *errorString = nil;
-	
+
+	// Before starting the file actions, attempt to create a dictionary
+	// from the current favourites tree and convert it to a dictionary representation
+	// to create the plist data.  This is done before file changes as it can sometimes
+	// be terminated during shutdown.
+	NSDictionary *dictionary = [NSDictionary dictionaryWithObject:data forKey:SPFavoritesRootKey];
+	NSData *plistData = [NSPropertyListSerialization dataFromPropertyList:dictionary
+																   format:NSPropertyListXMLFormat_v1_0
+														 errorDescription:&errorString];
+	if (errorString) {
+		NSLog(@"Error converting favorites data to plist format: %@", errorString);
+		[errorString release];
+	}
+
+
 	NSFileManager *fileManager = [NSFileManager defaultManager];
 	
 	NSString *dataPath = [fileManager applicationSupportDirectoryForSubDirectory:SPDataSupportFolder error:&error];
@@ -389,33 +403,19 @@ static SPFavoritesController *sharedFavoritesController = nil;
 			return;
 		}
 	}
-	
-	NSDictionary *dictionary = [NSDictionary dictionaryWithObject:data forKey:SPFavoritesRootKey];
-	
-	// Convert the current favorites tree to a dictionary representation to create the plist data
-	NSData *plistData = [NSPropertyListSerialization dataFromPropertyList:dictionary
-																   format:NSPropertyListXMLFormat_v1_0
-														 errorDescription:&errorString];
-	
-	if (plistData) {
-		[plistData writeToFile:favoritesFile options:NSAtomicWrite error:&error];
+
+	// Write the converted data to the favourites file
+	[plistData writeToFile:favoritesFile options:NSAtomicWrite error:&error];
+
+	if (error) {
+		NSLog(@"Error writing favorites data. Restoring backup if available: %@", [error localizedDescription]);
 		
-		if (error) {
-			NSLog(@"Error writing favorites data. Restoring backup if available: %@", [error localizedDescription]);
-			
-			// Restore the original data file
-			[fileManager moveItemAtPath:favoritesBackupFile toPath:favoritesFile error:NULL];
-		}
-		else {
-			// Remove the original backup
-			[fileManager removeItemAtPath:favoritesBackupFile error:NULL];
-		}
+		// Restore the original data file
+		[fileManager moveItemAtPath:favoritesBackupFile toPath:favoritesFile error:NULL];
 	}
-	else if (errorString) {
-		NSLog(@"Error converting favorites data to plist format: %@", errorString);
-		
-		[errorString release];
-		
+	else {
+
+		// Remove the original backup
 		[fileManager removeItemAtPath:favoritesBackupFile error:NULL];
 	}
 	
diff --git a/Source/SPPreferencesUpgrade.m b/Source/SPPreferencesUpgrade.m
index ac25b6b3..a7123ce6 100644
--- a/Source/SPPreferencesUpgrade.m
+++ b/Source/SPPreferencesUpgrade.m
@@ -298,7 +298,12 @@ void SPApplyRevisionChanges(void)
 		
 		[prefs setObject:newMappedValue forKey:@"DefaultEncodingTag"];
 	}
-	
+
+	// For versions prior to 3695 (<1.0), migrate the favourites across if appropriate
+	if (recordedVersionNumber < 3695) {
+		SPMigrateConnectionFavoritesData();
+	}
+
 	// Update the prefs revision
 	[prefs setObject:[NSNumber numberWithInteger:currentVersionNumber] forKey:SPLastUsedVersion];
 }
@@ -312,9 +317,20 @@ void SPMigrateConnectionFavoritesData(void)
 	NSError *error = nil;
 	NSFileManager *fileManager = [NSFileManager defaultManager];
 	NSUserDefaults *prefs = [NSUserDefaults standardUserDefaults];
-	
+
+	NSString *dataPath = [fileManager applicationSupportDirectoryForSubDirectory:SPDataSupportFolder error:&error];
+	if (error) {
+		NSLog(@"Error loading favorites: %@", [error localizedDescription]);
+		return;
+	}
+
+	NSString *favoritesFile = [dataPath stringByAppendingPathComponent:SPFavoritesDataFile];
+
+	// If the favourites file already exists, don't proceed
+	if ([fileManager fileExistsAtPath:favoritesFile]) return;
+
 	NSMutableArray *favorites = [[NSMutableArray alloc] initWithArray:[prefs objectForKey:SPOldFavoritesKey]];
-	
+
 	// Change the last used favorite and default favorite's indexes to be ID based
 	if (![prefs objectForKey:SPLastFavoriteID] && [favorites count]) {
 		
@@ -332,47 +348,33 @@ void SPMigrateConnectionFavoritesData(void)
 		// TOOD: Favorites migration - only uncomment when we want to remove backwards compatibility
 		//[prefs removeObjectForKey:@"LastFavoriteIndex"];
 	}
+
+	NSDictionary *newFavorites = [NSDictionary dictionaryWithObject:[NSDictionary dictionaryWithObjectsAndKeys:NSLocalizedString(@"Favorites", @"favorites label"), SPFavoritesGroupNameKey, favorites, SPFavoriteChildrenKey, nil] forKey:SPFavoritesRootKey];
 	
-	NSString *dataPath = [fileManager applicationSupportDirectoryForSubDirectory:SPDataSupportFolder error:&error];
-	
-	if (error) {
-		NSLog(@"Error loading favorites: %@", [error localizedDescription]);
-		[favorites release];
-		return;
-	}
-	
-	NSString *favoritesFile = [dataPath stringByAppendingPathComponent:SPFavoritesDataFile];
+	error = nil;
+	NSString *errorString = nil;
 	
-	// Only proceed if the new favorites plist doesn't already exist
-	if (![fileManager fileExistsAtPath:favoritesFile]) {	
-		
-		NSDictionary *newFavorites = [NSDictionary dictionaryWithObject:[NSDictionary dictionaryWithObjectsAndKeys:NSLocalizedString(@"Favorites", @"favorites label"), SPFavoritesGroupNameKey, favorites, SPFavoriteChildrenKey, nil] forKey:SPFavoritesRootKey];
-		
-		error = nil;
-		NSString *errorString = nil;
+	NSData *plistData = [NSPropertyListSerialization dataFromPropertyList:newFavorites
+																   format:NSPropertyListXMLFormat_v1_0
+														 errorDescription:&errorString];
+	if (plistData) {
+		[plistData writeToFile:favoritesFile options:NSAtomicWrite error:&error];
 		
-		NSData *plistData = [NSPropertyListSerialization dataFromPropertyList:newFavorites
-																	   format:NSPropertyListXMLFormat_v1_0
-															 errorDescription:&errorString];
-		if (plistData) {
-			[plistData writeToFile:favoritesFile options:NSAtomicWrite error:&error];
-			
-			if (error) {
-				NSLog(@"Error migrating favorites data: %@", [error localizedDescription]);
-			}
-			else {
-				// TOOD: Favorites migration - only uncomment when we want to remove backwards compatibility
-				//[prefs removeObjectForKey:SPOldFavoritesKey];
-			}
+		if (error) {
+			NSLog(@"Error migrating favorites data: %@", [error localizedDescription]);
 		}
-		else if (errorString) {
-			NSLog(@"Error converting migrating favorites data to plist format: %@", errorString);
-			
-			[favorites release];
-			[errorString release];
-			return;
+		else {
+			// TOOD: Favorites migration - only uncomment when we want to remove backwards compatibility
+			//[prefs removeObjectForKey:SPOldFavoritesKey];
 		}
 	}
+	else if (errorString) {
+		NSLog(@"Error converting migrating favorites data to plist format: %@", errorString);
+		
+		[favorites release];
+		[errorString release];
+		return;
+	}
 		
 	[favorites release];
 }