settings manager moved from generic to explicit settings

Author: ibetitsmike

App/App.xaml.cs

@@ -92,7 +92,8 @@ public App()
         // FileSyncListMainPage is created by FileSyncListWindow.
         services.AddTransient<FileSyncListWindow>();
 
-        services.AddSingleton<ISettingsManager>(_ => new SettingsManager("CoderDesktop"));
+        services.AddSingleton<ISettingsManager, SettingsManager>();
+        services.AddSingleton<IStartupManager, StartupManager>();
         // SettingsWindow views and view models
         services.AddTransient<SettingsViewModel>();
         // SettingsMainPage is created by SettingsWindow.
@@ -159,10 +160,6 @@ protected override void OnLaunched(LaunchActivatedEventArgs args)
 
         // Start connecting to the manager in the background.
         var rpcController = _services.GetRequiredService<IRpcController>();
-        if (rpcController.GetState().RpcLifecycle == RpcLifecycle.Disconnected)
-            // Passing in a CT with no cancellation is desired here, because
-            // the named pipe open will block until the pipe comes up.
-            _logger.LogDebug("reconnecting with VPN service");
         _ = rpcController.Reconnect(CancellationToken.None).ContinueWith(t =>
         {
             if (t.Exception != null)
@@ -172,22 +169,18 @@ protected override void OnLaunched(LaunchActivatedEventArgs args)
                 Debug.WriteLine(t.Exception);
                 Debugger.Break();
 #endif
-            } else
+                return;
+            }
+            if (_settingsManager.ConnectOnLaunch)
             {
-                if (rpcController.GetState().VpnLifecycle == VpnLifecycle.Stopped)
+                _logger.LogInformation("RPC lifecycle is disconnected, but ConnectOnLaunch is enabled; attempting to connect");
+                _ = rpcController.StartVpn(CancellationToken.None).ContinueWith(connectTask =>
                 {
-                    if (_settingsManager.Read(SettingsManager.ConnectOnLaunchKey, false))
+                    if (connectTask.Exception != null)
                     {
-                        _logger.LogInformation("RPC lifecycle is disconnected, but ConnectOnLaunch is enabled; attempting to connect");
-                        _ = rpcController.StartVpn(CancellationToken.None).ContinueWith(connectTask =>
-                        {
-                            if (connectTask.Exception != null)
-                            {
-                                _logger.LogError(connectTask.Exception, "failed to connect on launch");
-                            }
-                        });
+                        _logger.LogError(connectTask.Exception, "failed to connect on launch");
                     }
-                }
+                });
             }
         });
 

App/Services/SettingsManager.cs

@@ -4,110 +4,150 @@
 using System.Text.Json;
 
 namespace Coder.Desktop.App.Services;
+
 /// <summary>
-/// Generic persistence contract for simple key/value settings.
+/// Settings contract exposing properties for app settings.
 /// </summary>
 public interface ISettingsManager
 {
     /// <summary>
-    /// Saves <paramref name="value"/> under <paramref name="name"/> and returns the value.
+    /// Returns the value of the StartOnLogin setting. Returns <c>false</c> if the key is not found.
     /// </summary>
-    T Save<T>(string name, T value);
+    bool StartOnLogin { get; set; }
 
     /// <summary>
-    /// Reads the setting or returns <paramref name="defaultValue"/> when the key is missing.
+    /// Returns the value of the ConnectOnLaunch setting. Returns <c>false</c> if the key is not found.
     /// </summary>
-    T Read<T>(string name, T defaultValue);
+    bool ConnectOnLaunch { get; set; }
 }
+
 /// <summary>
-/// JSON‑file implementation that works in unpackaged Win32/WinUI 3 apps.
+/// Implemention of <see cref="ISettingsManager"/> that persists settings to a JSON file
+/// located in the user's local application data folder.
 /// </summary>
 public sealed class SettingsManager : ISettingsManager
 {
     private readonly string _settingsFilePath;
     private readonly string _fileName = "app-settings.json";
+    private readonly string _appName = "CoderDesktop";
     private readonly object _lock = new();
     private Dictionary<string, JsonElement> _cache;
 
-    public static readonly string ConnectOnLaunchKey = "ConnectOnLaunch";
-    public static readonly string StartOnLoginKey = "StartOnLogin";
+    public const string ConnectOnLaunchKey = "ConnectOnLaunch";
+    public const string StartOnLoginKey = "StartOnLogin";
 
-    /// <param name="appName">
-    /// Sub‑folder under %LOCALAPPDATA% (e.g. "CoderDesktop").
-    /// If <c>null</c> the folder name defaults to the executable name.
+    public bool StartOnLogin
+    {
+        get
+        {
+            return Read(StartOnLoginKey, false);
+        }
+        set
+        {
+            Save(StartOnLoginKey, value);
+        }
+    }
+
+    public bool ConnectOnLaunch
+    {
+        get
+        {
+            return Read(ConnectOnLaunchKey, false);
+        }
+        set
+        {
+            Save(ConnectOnLaunchKey, value);
+        }
+    }
+
+    /// <param name="settingsFilePath">
     /// For unit‑tests you can pass an absolute path that already exists.
+    /// Otherwise the settings file will be created in the user's local application data folder.
     /// </param>
-    public SettingsManager(string? appName = null)
+    public SettingsManager(string? settingsFilePath = null)
     {
-        // Allow unit‑tests to inject a fully‑qualified path.
-        if (appName is not null && Path.IsPathRooted(appName))
+        if (settingsFilePath is null)
         {
-            _settingsFilePath = Path.Combine(appName, _fileName);
-            Directory.CreateDirectory(Path.GetDirectoryName(_settingsFilePath)!);
+            settingsFilePath = Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData);
         }
-        else
+        else if (!Path.IsPathRooted(settingsFilePath))
         {
-            string folder = Path.Combine(
-                Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData),
-                appName ?? AppDomain.CurrentDomain.FriendlyName.ToLowerInvariant());
-            Directory.CreateDirectory(folder);
-            _settingsFilePath = Path.Combine(folder, _fileName);
+            throw new ArgumentException("settingsFilePath must be an absolute path if provided", nameof(settingsFilePath));
+        }
+
+        string folder = Path.Combine(
+                settingsFilePath,
+                _appName);
+
+        Directory.CreateDirectory(folder);
+        _settingsFilePath = Path.Combine(folder, _fileName);
+
+        if(!File.Exists(_settingsFilePath))
+        {
+            // Create the settings file if it doesn't exist
+            string emptyJson = JsonSerializer.Serialize(new { });
+            File.WriteAllText(_settingsFilePath, emptyJson);
         }
 
         _cache = Load();
     }
 
-    public T Save<T>(string name, T value)
+    private void Save(string name, bool value)
     {
         lock (_lock)
         {
-            _cache[name] = JsonSerializer.SerializeToElement(value);
-            Persist();
-            return value;
+            try
+            {
+                // Ensure cache is loaded before saving
+                using var fs = new FileStream(_settingsFilePath,
+                    FileMode.OpenOrCreate,
+                    FileAccess.ReadWrite,
+                    FileShare.None);
+                
+                var currentCache = JsonSerializer.Deserialize<Dictionary<string, JsonElement>>(fs) ?? new();
+                _cache = currentCache;
+                _cache[name] = JsonSerializer.SerializeToElement(value);
+                fs.Position = 0; // Reset stream position to the beginning before writing to override the file
+                var options = new JsonSerializerOptions { WriteIndented = true};
+                JsonSerializer.Serialize(fs, _cache, options);
+            }
+            catch
+            {
+                throw new InvalidOperationException($"Failed to persist settings to {_settingsFilePath}. The file may be corrupted, malformed or locked.");
+            }
         }
     }
 
-    public T Read<T>(string name, T defaultValue)
+    private bool Read(string name, bool defaultValue)
     {
         lock (_lock)
         {
             if (_cache.TryGetValue(name, out var element))
             {
                 try
                 {
-                    return element.Deserialize<T>() ?? defaultValue;
+                    return element.Deserialize<bool?>() ?? defaultValue;
                 }
                 catch
                 {
-                    // Malformed value – fall back.
+                    // malformed value – return default value
                     return defaultValue;
                 }
             }
-            return defaultValue; // key not found – return caller‑supplied default (false etc.)
+            return defaultValue; // key not found – return default value
         }
     }
 
     private Dictionary<string, JsonElement> Load()
     {
-        if (!File.Exists(_settingsFilePath))
-            return new();
-
         try
         {
             using var fs = File.OpenRead(_settingsFilePath);
             return JsonSerializer.Deserialize<Dictionary<string, JsonElement>>(fs) ?? new();
         }
-        catch
+        catch (Exception ex)
         {
-            // Corrupted file – start fresh.
-            return new();
+            throw new InvalidOperationException($"Failed to load settings from {_settingsFilePath}. The file may be corrupted or malformed. Exception: {ex.Message}");
         }
     }
-
-    private void Persist()
-    {
-        using var fs = File.Create(_settingsFilePath);
-        var options = new JsonSerializerOptions { WriteIndented = true };
-        JsonSerializer.Serialize(fs, _cache, options);
-    }
 }

App/Services/StartupManager.cs

@@ -4,7 +4,30 @@
 using System.Security;
 
 namespace Coder.Desktop.App.Services;
-public static class StartupManager
+
+public interface IStartupManager
+{
+    /// <summary>
+    /// Adds the current executable to the per‑user Run key. Returns <c>true</c> if successful.
+    /// Fails (returns <c>false</c>) when blocked by policy or lack of permissions.
+    /// </summary>
+    bool Enable();
+    /// <summary>
+    /// Removes the value from the Run key (no-op if missing).
+    /// </summary>
+    void Disable();
+    /// <summary>
+    /// Checks whether the value exists in the Run key.
+    /// </summary>
+    bool IsEnabled();
+    /// <summary>
+    /// Detects whether group policy disables per‑user startup programs.
+    /// Mirrors <see cref="Windows.ApplicationModel.StartupTaskState.DisabledByPolicy"/>.
+    /// </summary>
+    bool IsDisabledByPolicy();
+}
+
+public class StartupManager : IStartupManager
 {
     private const string RunKey = @"Software\\Microsoft\\Windows\\CurrentVersion\\Run";
     private const string PoliciesExplorerUser = @"Software\\Microsoft\\Windows\\CurrentVersion\\Policies\\Explorer";
@@ -14,11 +37,7 @@ public static class StartupManager
 
     private const string _defaultValueName = "CoderDesktopApp";
 
-    /// <summary>
-    /// Adds the current executable to the per‑user Run key. Returns <c>true</c> if successful.
-    /// Fails (returns <c>false</c>) when blocked by policy or lack of permissions.
-    /// </summary>
-    public static bool Enable()
+    public bool Enable()
     {
         if (IsDisabledByPolicy())
             return false;
@@ -35,25 +54,19 @@ public static bool Enable()
         catch (SecurityException) { return false; }
     }
 
-    /// <summary>Removes the value from the Run key (no-op if missing).</summary>
-    public static void Disable()
+    public void Disable()
     {
         using var key = Registry.CurrentUser.OpenSubKey(RunKey, writable: true);
         key?.DeleteValue(_defaultValueName, throwOnMissingValue: false);
     }
 
-    /// <summary>Checks whether the value exists in the Run key.</summary>
-    public static bool IsEnabled()
+    public bool IsEnabled()
     {
         using var key = Registry.CurrentUser.OpenSubKey(RunKey);
         return key?.GetValue(_defaultValueName) != null;
     }
 
-    /// <summary>
-    /// Detects whether group policy disables per‑user startup programs.
-    /// Mirrors <see cref="Windows.ApplicationModel.StartupTaskState.DisabledByPolicy"/>.
-    /// </summary>
-    public static bool IsDisabledByPolicy()
+    public bool IsDisabledByPolicy()
     {
         // User policy – HKCU
         using (var keyUser = Registry.CurrentUser.OpenSubKey(PoliciesExplorerUser))
@@ -65,8 +78,6 @@ public static bool IsDisabledByPolicy()
         {
             if ((int?)keyMachine?.GetValue(DisableLocalMachineRun) == 1) return true;
         }
-
-        // Some non‑desktop SKUs report DisabledByPolicy implicitly
         return false;
     }
 }

App/ViewModels/SettingsViewModel.cs

@@ -12,34 +12,36 @@ public partial class SettingsViewModel : ObservableObject
     private readonly ILogger<SettingsViewModel> _logger;
 
     [ObservableProperty]
-    public partial bool ConnectOnLaunch { get; set; } = false;
+    public partial bool ConnectOnLaunch { get; set; }
 
     [ObservableProperty]
-    public partial bool StartOnLoginDisabled { get; set; } = false;
+    public partial bool StartOnLoginDisabled { get; set; }
 
     [ObservableProperty]
-    public partial bool StartOnLogin { get; set; } = false;
+    public partial bool StartOnLogin { get; set; }
 
     private ISettingsManager _settingsManager;
+    private IStartupManager _startupManager;
 
-    public SettingsViewModel(ILogger<SettingsViewModel> logger, ISettingsManager settingsManager)
+    public SettingsViewModel(ILogger<SettingsViewModel> logger, ISettingsManager settingsManager, IStartupManager startupManager)
     {
         _settingsManager = settingsManager;
+        _startupManager = startupManager;
         _logger = logger;
-        ConnectOnLaunch = _settingsManager.Read(SettingsManager.ConnectOnLaunchKey, false);
-        StartOnLogin = _settingsManager.Read(SettingsManager.StartOnLoginKey, false);
+        ConnectOnLaunch = _settingsManager.ConnectOnLaunch;
+        StartOnLogin = _settingsManager.StartOnLogin;
 
         // Various policies can disable the "Start on login" option.
         // We disable the option in the UI if the policy is set.
-        StartOnLoginDisabled = StartupManager.IsDisabledByPolicy();
+        StartOnLoginDisabled = _startupManager.IsDisabledByPolicy();
 
         this.PropertyChanged += (_, args) =>
         {
             if (args.PropertyName == nameof(ConnectOnLaunch))
             {
                 try
                 {
-                    _settingsManager.Save(SettingsManager.ConnectOnLaunchKey, ConnectOnLaunch);
+                    _settingsManager.ConnectOnLaunch = ConnectOnLaunch;
                 }
                 catch (Exception ex)
                 {
@@ -50,14 +52,14 @@ public SettingsViewModel(ILogger<SettingsViewModel> logger, ISettingsManager set
             {
                 try
                 {
-                    _settingsManager.Save(SettingsManager.StartOnLoginKey, StartOnLogin);
+                    _settingsManager.StartOnLogin = StartOnLogin;
                     if (StartOnLogin)
                     {
-                        StartupManager.Enable();
+                        _startupManager.Enable();
                     }
                     else
                     {
-                        StartupManager.Disable();
+                        _startupManager.Disable();
                     }
                 }
                 catch (Exception ex)
@@ -68,9 +70,9 @@ public SettingsViewModel(ILogger<SettingsViewModel> logger, ISettingsManager set
         };
 
         // Ensure the StartOnLogin property matches the current startup state.
-        if (StartOnLogin != StartupManager.IsEnabled())
+        if (StartOnLogin != _startupManager.IsEnabled())
         {
-            StartOnLogin = StartupManager.IsEnabled();
+            StartOnLogin = _startupManager.IsEnabled();
         }
     }
 }

Tests.App/Services/SettingsManagerTest.cs

@@ -1,57 +1,53 @@
 using Coder.Desktop.App.Services;
 
-namespace Coder.Desktop.Tests.App.Services
+namespace Coder.Desktop.Tests.App.Services;
+[TestFixture]
+public sealed class SettingsManagerTests
 {
-    [TestFixture]
-    public sealed class SettingsManagerTests
+    private string _tempDir = string.Empty;
+    private SettingsManager _sut = null!;
+
+    [SetUp]
+    public void SetUp()
     {
-        private string _tempDir = string.Empty;
-        private SettingsManager _sut = null!;
-
-        [SetUp]
-        public void SetUp()
-        {
-            _tempDir = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString());
-            Directory.CreateDirectory(_tempDir);
-            _sut = new SettingsManager(_tempDir); // inject isolated path
-        }
-
-        [TearDown]
-        public void TearDown()
-        {
-            try { Directory.Delete(_tempDir, true); } catch { /* ignore */ }
-        }
-
-        [Test]
-        public void Save_ReturnsValue_AndPersists()
-        {
-            int expected = 42;
-            int actual = _sut.Save("Answer", expected);
-
-            Assert.That(actual, Is.EqualTo(expected));
-            Assert.That(_sut.Read("Answer", -1), Is.EqualTo(expected));
-        }
-
-        [Test]
-        public void Read_MissingKey_ReturnsDefault()
-        {
-            bool result = _sut.Read("DoesNotExist", defaultValue: false);
-            Assert.That(result, Is.False);
-        }
-
-        [Test]
-        public void Read_AfterReload_ReturnsPreviouslySavedValue()
-        {
-            const string key = "Greeting";
-            const string value = "Hello";
-
-            _sut.Save(key, value);
-
-            // Create new instance to force file reload.
-            var newManager = new SettingsManager(_tempDir);
-            string persisted = newManager.Read(key, string.Empty);
-
-            Assert.That(persisted, Is.EqualTo(value));
-        }
+        _tempDir = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString());
+        Directory.CreateDirectory(_tempDir);
+        _sut = new SettingsManager(_tempDir); // inject isolated path
+    }
+
+    [TearDown]
+    public void TearDown()
+    {
+        try { Directory.Delete(_tempDir, true); } catch { /* ignore */ }
+    }
+
+    [Test]
+    public void Save_Persists()
+    {
+        bool expected = true;
+        _sut.StartOnLogin = expected;
+
+        Assert.That(_sut.StartOnLogin, Is.EqualTo(expected));
+    }
+
+    [Test]
+    public void Read_MissingKey_ReturnsDefault()
+    {
+        bool result = _sut.ConnectOnLaunch; // default is false
+        Assert.That(result, Is.False);
+    }
+
+    [Test]
+    public void Read_AfterReload_ReturnsPreviouslySavedValue()
+    {
+        const bool value = true;
+
+        _sut.ConnectOnLaunch = value;
+
+        // Create new instance to force file reload.
+        var newManager = new SettingsManager(_tempDir);
+        bool persisted = newManager.ConnectOnLaunch;
+
+        Assert.That(persisted, Is.EqualTo(value));
     }
 }