Merge pull request #1528 from jlewin/1.6.0

Refactor ProfileManager to prefer isolated instances

ApplicationView/MainApplicationWidget.cs

@@ -388,24 +388,6 @@ namespace MatterHackers.MatterControl
 			}
 		}
 
-		private static string MakeValidFileName(string name)
-		{
-			if (string.IsNullOrEmpty(name))
-			{
-				return name;
-			}
-
-			string invalidChars = Regex.Escape(new string(Path.GetInvalidFileNameChars()));
-			string invalidRegStr = string.Format(@"([{0}]*\.+$)|([{0}]+)", invalidChars);
-
-			return Regex.Replace(name, invalidRegStr, "_");
-		}
-
-		public string GetSessionUsernameForFileSystem()
-		{
-			return MakeValidFileName(AuthenticationData.Instance.ActiveSessionUsername);
-		}
-
 		bool pendingReloadRequest = false;
 		public void ReloadAll(object sender, EventArgs e)
 		{
@@ -476,8 +458,10 @@ namespace MatterHackers.MatterControl
 
 						// set the colors
 						LoadUITheme();
-						// This will initialize the theme for the first printer to load
+
-						ProfileManager.Reload();
+						// We previously made a call to Reload, which fired the method twice due to it being in the static constructor. Accessing
+						// any property will run the static constructor and perform the Reload behavior without the overhead of duplicate calls
+						bool na = ProfileManager.Instance.IsGuestProfile;
 
 						if (UserSettings.Instance.DisplayMode == ApplicationDisplayType.Touchscreen)
 						{
@@ -558,10 +542,10 @@ namespace MatterHackers.MatterControl
 			// Ensure SQLite printers are imported
 			profileManager.EnsurePrintersImported();
 
-			var guestDB = ProfileManager.LoadGuestDB();
+			var guest = ProfileManager.LoadGuestProfiles();
 
 			// If profiles.json was created, run the import wizard to pull in any SQLite printers
-			if (guestDB?.Profiles != null && guestDB.Profiles.Any() && !profileManager.IsGuestProfile && !profileManager.PrintersImported)
+			if (guest?.Profiles != null && guest.Profiles.Any() && !profileManager.IsGuestProfile && !profileManager.PrintersImported)
 			{
 				var wizardPage = new CopyGuestProfilesToUser(() =>
 				{

DataStorage/Classic/ClassicSqlitePrinterProfiles.cs

@@ -105,7 +105,7 @@ namespace MatterHackers.MatterControl.DataStorage.ClassicDB
 
 			if (string.IsNullOrEmpty(ProfileManager.Instance.LastProfileID))
 			{
-				ProfileManager.Instance.SetLastProfile(printer.Id.ToString());
+				ProfileManager.Instance.LastProfileID = printer.Id.ToString();
 			}
 
 			printerSettings.UserLayer[SettingsKey.active_theme_name] = UserSettings.Instance.get(UserSettingsKey.ActiveThemeName);

SetupWizard/CopyGuestProfilesToUser.cs

@@ -68,8 +68,8 @@ namespace MatterHackers.MatterControl
 
 			var byCheckbox = new Dictionary<CheckBox, PrinterInfo>();
 
-			var guestProfileManager = ProfileManager.LoadGuestDB();
+			var guest = ProfileManager.LoadGuestProfiles();
-			if (guestProfileManager?.Profiles.Count > 0)
+			if (guest?.Profiles.Count > 0)
 			{
 				container.AddChild(new TextWidget("Printers to Copy:".Localize())
 				{
@@ -77,7 +77,7 @@ namespace MatterHackers.MatterControl
 					Margin = new BorderDouble(0, 3, 0, 15),
 				});
 
-				foreach (var printerInfo in guestProfileManager.Profiles)
+				foreach (var printerInfo in guest.Profiles)
 				{
 					var checkBox = new CheckBox(printerInfo.Name)
 					{
@@ -104,20 +104,23 @@ namespace MatterHackers.MatterControl
 						// import the printer
 						var printerInfo = byCheckbox[checkBox];
 
-						string existingPath = Path.Combine(ProfileManager.GuestDBDirectory, printerInfo.ID + ProfileManager.ProfileExtension); ;
+						string existingPath = guest.ProfilePath(printerInfo);
-
-						ProfileManager.Instance.Profiles.Add(printerInfo);
-						guestProfileManager.Profiles.Remove(printerInfo);
 
 						// PrinterSettings files must actually be copied to the users profile directory
 						if (File.Exists(existingPath))
 						{
 							File.Copy(existingPath, printerInfo.ProfilePath);
+
+							// Only add if copy succeeds
+							ProfileManager.Instance.Profiles.Add(printerInfo);
+
+							// TODO: Do we copy or migrate. This looks a lot like migrate which is not the current expected behavior
+							// guestProfileManager.Profiles.Remove(printerInfo);
 						}
 					}
 				}
 
-				guestProfileManager.Save();
+				guest.Save();
 
 				// close the window
 				UiThread.RunOnIdle(() =>

SlicerConfiguration/Settings/ActiveSliceSettings.cs

@@ -141,12 +141,12 @@ namespace MatterHackers.MatterControl.SlicerConfiguration
 		static ActiveSliceSettings()
 		{
 			// Load last profile or fall back to empty
-			Instance = ProfileManager.Instance?.LoadLastProfileWithoutRecovery() ?? ProfileManager.LoadEmptyProfile();
+			Instance = ProfileManager.Instance?.LoadWithoutRecovery(ProfileManager.Instance.LastProfileID) ?? ProfileManager.LoadEmptyProfile();
 		}
 
 		internal static async Task SwitchToProfile(string printerID)
 		{
-			ProfileManager.Instance.SetLastProfile(printerID);
+			ProfileManager.Instance.LastProfileID = printerID;
 			Instance = (await ProfileManager.LoadProfileAsync(printerID)) ?? ProfileManager.LoadEmptyProfile();
 		}
 

SlicerConfiguration/Settings/ProfileManager.cs

@@ -47,104 +47,95 @@ namespace MatterHackers.MatterControl.SlicerConfiguration
 	{
 		public static RootedObjectEventHandler ProfilesListChanged = new RootedObjectEventHandler();
 
-		public static ProfileManager Instance { get; set; }
+		public static ProfileManager Instance { get; private set; }
+
+		private static EventHandler unregisterEvents;
 
 		public const string ProfileExtension = ".printer";
 		public const string ConfigFileExtension = ".slice";
+		public const string ProfileDocExtension = ".profiles";
 
-		private static object writeLock = new object();
+		private object writeLock = new object();
-		private static EventHandler unregisterEvents;
-		private static readonly string userDataPath = ApplicationDataStorage.ApplicationUserDataPath;
-
-		/// <summary>
-		/// The user specific path to the Profiles directory
-		/// </summary>
-		private static string ProfilesPath
-		{
-			get
-			{
-				// Determine username
-				string username = ApplicationController.Instance.GetSessionUsernameForFileSystem();
-				if (string.IsNullOrEmpty(username))
-				{
-					username = "guest";
-				}
-				else
-				{
-					username = ApplicationController.EnvironmentName + username;
-				}
-
-				string path = Path.Combine(userDataPath, "Profiles", username);
-
-				// Ensure directory exists
-				Directory.CreateDirectory(path);
-
-				return path;
-			}
-		}
-
-		private const string userDBExtension = ".profiles";
-		private const string guestDBFileName = "guest" + userDBExtension;
-
-		internal static string GuestDBDirectory => Path.Combine(userDataPath, "Profiles", "guest");
-		private static string GuestDBPath => Path.Combine(GuestDBDirectory, guestDBFileName);
-
-		internal static string ProfilesDBPath
-		{
-			get
-			{
-				string username = ApplicationController.Instance.GetSessionUsernameForFileSystem();
-				if (string.IsNullOrEmpty(username))
-				{ 
-					username = GuestDBPath;
-				}
-				else
-				{
-					username = Path.Combine(ProfilesPath, $"{username}{userDBExtension}");
-				}
-
-				return username;
-			}
-		}
 
 		static ProfileManager()
 		{
 			SliceSettingsWidget.SettingChanged.RegisterEvent(SettingsChanged, ref unregisterEvents);
-
-			// Ensure the profiles directory exists
-			Directory.CreateDirectory(ProfilesPath);
-
 			Reload();
 		}
 
-		public ProfileManager()
+		public ProfileManager(string userName)
 		{
+			this.UserName = userName;
+		}
+
+		public string UserName { get; set; }
+
+		/// <summary>
+		/// The user specific path to the Profiles directory
+		/// </summary>
+		[JsonIgnore]
+		private string UserProfilesDirectory => GetProfilesDirectoryForUser(this.UserName);
+
+		/// <summary>
+		/// The user specific path to the Profiles document
+		/// </summary>
+		[JsonIgnore]
+		public string ProfilesDocPath => GetProfilesDocPathForUser(this.UserName);
+
+		private static string GetProfilesDocPathForUser(string userName)
+		{
+			return Path.Combine(GetProfilesDirectoryForUser(userName), $"{userName}{ProfileDocExtension}");
+		}
+
+		private static string GetProfilesDirectoryForUser(string userName)
+		{
+			string userProfilesDirectory = Path.Combine(ApplicationDataStorage.ApplicationUserDataPath, "Profiles", userName);
+
+			// Ensure directory exists
+			Directory.CreateDirectory(userProfilesDirectory);
+
+			return userProfilesDirectory;
 		}
 
 		[JsonIgnore]
-		public bool IsGuestProfile => Path.GetFileName(ProfilesDBPath) == guestDBFileName;
+		public bool IsGuestProfile { get; private set; } = false;
 
 		public static void Reload()
 		{
+			string userName = AuthenticationData.Instance.FileSystemSafeUserName;
+			if (string.IsNullOrEmpty(userName))
+			{
+				userName = "guest";
+			}
+
+			if (Instance?.UserName == userName)
+			{
+				return;
+			}
+
 			if (Instance?.Profiles != null)
 			{
 				// Release event registration
 				Instance.Profiles.CollectionChanged -= Profiles_CollectionChanged;
 			}
 
-			// Load the profiles document
+			string profilesDocPath = GetProfilesDocPathForUser(userName);
-			if (File.Exists(ProfilesDBPath))
+
+			// Reassign the active instance based on the logged in user
+			if (File.Exists(profilesDocPath))
 			{
-				string json = File.ReadAllText(ProfilesDBPath);
+				string json = File.ReadAllText(profilesDocPath);
 				Instance = JsonConvert.DeserializeObject<ProfileManager>(json);
+				Instance.UserName = userName;
 			}
 			else
 			{
-				Instance = new ProfileManager();
+				Instance = new ProfileManager(userName);
 			}
 
 			if (ActiveSliceSettings.Instance?.ID != Instance.LastProfileID)
 			{
+				// async so we can safely wait for LoadProfileAsync to complete
 				Task.Run(async () =>
 				{
 					// Load or download on a background thread
@@ -152,6 +143,7 @@ namespace MatterHackers.MatterControl.SlicerConfiguration
 
 					if (MatterControlApplication.IsLoading)
 					{
+						// TODO: Not true - we're on a background thread in an async lambda... what is the intent of this?
 						// Assign on the UI thread
 						ActiveSliceSettings.Instance = lastProfile ?? LoadEmptyProfile();
 					}
@@ -170,15 +162,9 @@ namespace MatterHackers.MatterControl.SlicerConfiguration
 			Instance.Profiles.CollectionChanged += Profiles_CollectionChanged;
 		}
 
-		internal static ProfileManager LoadGuestDB()
+		internal static ProfileManager LoadGuestProfiles()
 		{
-			if (File.Exists(GuestDBPath))
+			return new ProfileManager("guest") { IsGuestProfile = true };
-			{
-				string json = File.ReadAllText(GuestDBPath);
-				return JsonConvert.DeserializeObject<ProfileManager>(json);
-			}
-
-			return null;
 		}
 
 		internal static void SettingsChanged(object sender, EventArgs e)
@@ -227,35 +213,27 @@ namespace MatterHackers.MatterControl.SlicerConfiguration
 		{
 			get
 			{
-				string activeUserName = ApplicationController.Instance.GetSessionUsernameForFileSystem();
+				return UserSettings.Instance.get($"ActiveProfileID-{UserName}");
-				return UserSettings.Instance.get($"ActiveProfileID-{activeUserName}");
+			}
+			set
+			{
+				UserSettings.Instance.set($"ActiveProfileID-{UserName}", value);
 			}
 		}
 
 		public bool PrintersImported { get; set; } = false;
 
-		public PrinterSettings LoadLastProfileWithoutRecovery()
-		{
-			return LoadWithoutRecovery(this.LastProfileID);
-		}
-
-		public void SetLastProfile(string printerID)
-		{
-			string activeUserName = ApplicationController.Instance.GetSessionUsernameForFileSystem();
-			UserSettings.Instance.set($"ActiveProfileID-{activeUserName}", printerID);
-		}
-
-		public string ProfilePath(PrinterInfo printer)
-		{
-			return Path.Combine(ProfileManager.ProfilesPath, printer.ID + ProfileExtension);
-		}
-
 		public string ProfilePath(string printerID)
 		{
 			return ProfilePath(this[printerID]);
 		}
 
-		public static PrinterSettings LoadWithoutRecovery(string profileID)
+		public string ProfilePath(PrinterInfo printer)
+		{
+			return Path.Combine(UserProfilesDirectory, printer.ID + ProfileExtension);
+		}
+
+		public PrinterSettings LoadWithoutRecovery(string profileID)
 		{
 			var printerInfo = Instance[profileID];
 
@@ -291,14 +269,14 @@ namespace MatterHackers.MatterControl.SlicerConfiguration
 			}
 
 			// Only load profiles by ID that are defined in the profiles document
-			var printerInfo = ProfileManager.Instance[profileID];
+			var printerInfo = Instance[profileID];
 			if (printerInfo == null)
 			{
 				return null;
 			}
 
 			// Attempt to load from disk, pull from the web or fall back using recovery logic
-			PrinterSettings printerSettings = LoadWithoutRecovery(profileID);
+			PrinterSettings printerSettings = Instance.LoadWithoutRecovery(profileID);
 			if (printerSettings != null)
 			{
 				return printerSettings;
@@ -553,7 +531,7 @@ namespace MatterHackers.MatterControl.SlicerConfiguration
 			if (IsGuestProfile && !PrintersImported)
 			{
 				// Import Sqlite printer profiles into local json files
-				DataStorage.ClassicDB.ClassicSqlitePrinterProfiles.ImportPrinters(Instance, ProfilesPath);
+				DataStorage.ClassicDB.ClassicSqlitePrinterProfiles.ImportPrinters(Instance, UserProfilesDirectory);
 				PrintersImported = true;
 				Save();
 			}
@@ -574,7 +552,7 @@ namespace MatterHackers.MatterControl.SlicerConfiguration
 		{
 			lock(writeLock)
 			{
-				File.WriteAllText(ProfilesDBPath, JsonConvert.SerializeObject(this, Formatting.Indented));
+				File.WriteAllText(ProfilesDocPath, JsonConvert.SerializeObject(this, Formatting.Indented));
 			}
 		}
 	}

Utilities/AuthenticationData.cs

@@ -7,6 +7,8 @@ using System.IO;
 using System.Runtime.Serialization;
 using System.Runtime.Serialization.Formatters.Binary;
 using MatterHackers.Localizations;
+using System.Text.RegularExpressions;
+using Newtonsoft.Json;
 
 namespace MatterHackers.MatterControl
 {
@@ -156,5 +158,21 @@ namespace MatterHackers.MatterControl
 				ApplicationSettings.Instance.set($"{ApplicationController.EnvironmentName}LastSessionUsername", value);
 			}
 		}
+
+		[JsonIgnore]
+		public string FileSystemSafeUserName => MakeValidFileName(this.ActiveSessionUsername);
+
+		private static string MakeValidFileName(string name)
+		{
+			if (string.IsNullOrEmpty(name))
+			{
+				return name;
+			}
+
+			string invalidChars = Regex.Escape(new string(Path.GetInvalidFileNameChars()));
+			string invalidRegStr = string.Format(@"([{0}]*\.+$)|([{0}]+)", invalidChars);
+
+			return Regex.Replace(name, invalidRegStr, "_");
+		}
 	}
 }