From 42cc88b9d4c2c8be53b82952add9e689667b9d9e Mon Sep 17 00:00:00 2001 From: John Lewin Date: Fri, 21 Oct 2016 12:59:10 -0700 Subject: [PATCH] Refactor ProfileManager to prefer isolated instances - Replace DB nomenclatures with ProfilesDoc/Profiles terms - Convert SetLastProfile() to LastProfileID property - Remove LoadLastProfileWithoutRecovery function - Use LoadWithoutRecovery(LastProfileID) - One caller, far more clear with only one LoadWithout function - Move safe FileSystem UserName to AuthenticationData, like UserName - Skip importing guest profiles that are missing PrinterSettings files --- ApplicationView/MainApplicationWidget.cs | 28 +-- .../Classic/ClassicSqlitePrinterProfiles.cs | 2 +- SetupWizard/CopyGuestProfilesToUser.cs | 19 +- .../Settings/ActiveSliceSettings.cs | 4 +- .../Settings/ProfileManager.cs | 170 ++++++++---------- Utilities/AuthenticationData.cs | 18 ++ 6 files changed, 112 insertions(+), 129 deletions(-) diff --git a/ApplicationView/MainApplicationWidget.cs b/ApplicationView/MainApplicationWidget.cs index f1e3bbbec..88f5d859f 100644 --- a/ApplicationView/MainApplicationWidget.cs +++ b/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(() => { diff --git a/DataStorage/Classic/ClassicSqlitePrinterProfiles.cs b/DataStorage/Classic/ClassicSqlitePrinterProfiles.cs index c3f23e47d..49bc1e3d5 100644 --- a/DataStorage/Classic/ClassicSqlitePrinterProfiles.cs +++ b/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); diff --git a/SetupWizard/CopyGuestProfilesToUser.cs b/SetupWizard/CopyGuestProfilesToUser.cs index 25156f506..1cc42b8c0 100644 --- a/SetupWizard/CopyGuestProfilesToUser.cs +++ b/SetupWizard/CopyGuestProfilesToUser.cs @@ -68,8 +68,8 @@ namespace MatterHackers.MatterControl var byCheckbox = new Dictionary(); - var guestProfileManager = ProfileManager.LoadGuestDB(); - if (guestProfileManager?.Profiles.Count > 0) + var guest = ProfileManager.LoadGuestProfiles(); + 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); ; - - ProfileManager.Instance.Profiles.Add(printerInfo); - guestProfileManager.Profiles.Remove(printerInfo); + string existingPath = guest.ProfilePath(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(() => diff --git a/SlicerConfiguration/Settings/ActiveSliceSettings.cs b/SlicerConfiguration/Settings/ActiveSliceSettings.cs index 99b3c7b48..58e3ebde0 100644 --- a/SlicerConfiguration/Settings/ActiveSliceSettings.cs +++ b/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(); } diff --git a/SlicerConfiguration/Settings/ProfileManager.cs b/SlicerConfiguration/Settings/ProfileManager.cs index 136893394..d21732da3 100644 --- a/SlicerConfiguration/Settings/ProfileManager.cs +++ b/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 static EventHandler unregisterEvents; - private static readonly string userDataPath = ApplicationDataStorage.ApplicationUserDataPath; - - /// - /// The user specific path to the Profiles directory - /// - 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; - } - } + private object writeLock = new object(); 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; } + + /// + /// The user specific path to the Profiles directory + /// + [JsonIgnore] + private string UserProfilesDirectory => GetProfilesDirectoryForUser(this.UserName); + + /// + /// The user specific path to the Profiles document + /// + [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 - if (File.Exists(ProfilesDBPath)) + string profilesDocPath = GetProfilesDocPathForUser(userName); + + // 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(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)) - { - string json = File.ReadAllText(GuestDBPath); - return JsonConvert.DeserializeObject(json); - } - - return null; + return new ProfileManager("guest") { IsGuestProfile = true }; } 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-{activeUserName}"); + return UserSettings.Instance.get($"ActiveProfileID-{UserName}"); + } + 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)); } } } diff --git a/Utilities/AuthenticationData.cs b/Utilities/AuthenticationData.cs index 40aca44a4..859784a1f 100644 --- a/Utilities/AuthenticationData.cs +++ b/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, "_"); + } } } \ No newline at end of file