Skip to content

added new settings dialog + settings manager #113

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 18 commits into from
Jun 10, 2025
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions App/App.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
<ItemGroup>
<PackageReference Include="CommunityToolkit.Mvvm" Version="8.4.0" />
<PackageReference Include="CommunityToolkit.WinUI.Controls.Primitives" Version="8.2.250402" />
<PackageReference Include="CommunityToolkit.WinUI.Controls.SettingsControls" Version="8.2.250402" />
<PackageReference Include="CommunityToolkit.WinUI.Extensions" Version="8.2.250402" />
<PackageReference Include="DependencyPropertyGenerator" Version="1.5.0">
<PrivateAssets>all</PrivateAssets>
Expand Down
136 changes: 83 additions & 53 deletions App/App.xaml.cs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ public partial class App : Application
private readonly ILogger<App> _logger;
private readonly IUriHandler _uriHandler;

private readonly ISettingsManager<CoderConnectSettings> _settingsManager;

private readonly IHostApplicationLifetime _appLifetime;

public App()
{
var builder = Host.CreateApplicationBuilder();
Expand Down Expand Up @@ -90,6 +94,13 @@ public App()
// FileSyncListMainPage is created by FileSyncListWindow.
services.AddTransient<FileSyncListWindow>();

services.AddSingleton<ISettingsManager<CoderConnectSettings>, SettingsManager<CoderConnectSettings>>();
services.AddSingleton<IStartupManager, StartupManager>();
// SettingsWindow views and view models
services.AddTransient<SettingsViewModel>();
// SettingsMainPage is created by SettingsWindow.
services.AddTransient<SettingsWindow>();

// DirectoryPickerWindow views and view models are created by FileSyncListViewModel.

// TrayWindow views and view models
Expand All @@ -107,8 +118,10 @@ public App()
services.AddTransient<TrayWindow>();

_services = services.BuildServiceProvider();
_logger = (ILogger<App>)_services.GetService(typeof(ILogger<App>))!;
_uriHandler = (IUriHandler)_services.GetService(typeof(IUriHandler))!;
_logger = _services.GetRequiredService<ILogger<App>>();
_uriHandler = _services.GetRequiredService<IUriHandler>();
_settingsManager = _services.GetRequiredService<ISettingsManager<CoderConnectSettings>>();
_appLifetime = _services.GetRequiredService<IHostApplicationLifetime>();

InitializeComponent();
}
Expand All @@ -129,58 +142,8 @@ public async Task ExitApplication()
protected override void OnLaunched(LaunchActivatedEventArgs args)
{
_logger.LogInformation("new instance launched");
// 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)
{
_logger.LogError(t.Exception, "failed to connect to VPN service");
#if DEBUG
Debug.WriteLine(t.Exception);
Debugger.Break();
#endif
}
});

// Load the credentials in the background.
var credentialManagerCts = new CancellationTokenSource(TimeSpan.FromSeconds(15));
var credentialManager = _services.GetRequiredService<ICredentialManager>();
_ = credentialManager.LoadCredentials(credentialManagerCts.Token).ContinueWith(t =>
{
if (t.Exception != null)
{
_logger.LogError(t.Exception, "failed to load credentials");
#if DEBUG
Debug.WriteLine(t.Exception);
Debugger.Break();
#endif
}

credentialManagerCts.Dispose();
}, CancellationToken.None);

// Initialize file sync.
// We're adding a 5s delay here to avoid race conditions when loading the mutagen binary.

_ = Task.Delay(5000).ContinueWith((_) =>
{
var syncSessionCts = new CancellationTokenSource(TimeSpan.FromSeconds(10));
var syncSessionController = _services.GetRequiredService<ISyncSessionController>();
syncSessionController.RefreshState(syncSessionCts.Token).ContinueWith(
t =>
{
if (t.IsCanceled || t.Exception != null)
{
_logger.LogError(t.Exception, "failed to refresh sync state (canceled = {canceled})", t.IsCanceled);
}
syncSessionCts.Dispose();
}, CancellationToken.None);
});
_ = InitializeServicesAsync(_appLifetime.ApplicationStopping);

// Prevent the TrayWindow from closing, just hide it.
var trayWindow = _services.GetRequiredService<TrayWindow>();
Expand All @@ -192,6 +155,73 @@ protected override void OnLaunched(LaunchActivatedEventArgs args)
};
}

/// <summary>
/// Loads stored VPN credentials, reconnects the RPC controller,
/// and (optionally) starts the VPN tunnel on application launch.
/// </summary>
private async Task InitializeServicesAsync(CancellationToken cancellationToken = default)
{
var credentialManager = _services.GetRequiredService<ICredentialManager>();
var rpcController = _services.GetRequiredService<IRpcController>();

using var credsCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
credsCts.CancelAfter(TimeSpan.FromSeconds(15));

var loadCredsTask = credentialManager.LoadCredentials(credsCts.Token);
var reconnectTask = rpcController.Reconnect(cancellationToken);
var settingsTask = _settingsManager.Read(cancellationToken);

var dependenciesLoaded = true;

try
{
await Task.WhenAll(loadCredsTask, reconnectTask, settingsTask);
}
catch (Exception)
{
if (loadCredsTask.IsFaulted)
_logger.LogError(loadCredsTask.Exception!.GetBaseException(),
"Failed to load credentials");

if (reconnectTask.IsFaulted)
_logger.LogError(reconnectTask.Exception!.GetBaseException(),
"Failed to connect to VPN service");

if (settingsTask.IsFaulted)
_logger.LogError(settingsTask.Exception!.GetBaseException(),
"Failed to fetch Coder Connect settings");

// Don't attempt to connect if we failed to load credentials or reconnect.
// This will prevent the app from trying to connect to the VPN service.
dependenciesLoaded = false;
}

var attemptCoderConnection = settingsTask.Result?.ConnectOnLaunch ?? false;
if (dependenciesLoaded && attemptCoderConnection)
{
try
{
await rpcController.StartVpn(cancellationToken);
}
catch (Exception ex)
{
_logger.LogError(ex, "Failed to connect on launch");
}
}

// Initialize file sync.
var syncSessionCts = new CancellationTokenSource(TimeSpan.FromSeconds(10));
var syncSessionController = _services.GetRequiredService<ISyncSessionController>();
try
{
await syncSessionController.RefreshState(syncSessionCts.Token);
}
catch (Exception ex)
{
_logger.LogError($"Failed to refresh sync session state {ex.Message}", ex);
}
}

public void OnActivated(object? sender, AppActivationArguments args)
{
switch (args.Kind)
Expand Down
201 changes: 201 additions & 0 deletions App/Services/SettingsManager.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,201 @@
using Google.Protobuf.WellKnownTypes;
using System;
using System.Collections.Generic;
using System.IO;
using System.Text.Json;
using System.Text.Json.Serialization;
using System.Threading;
using System.Threading.Tasks;
using System.Xml.Linq;

namespace Coder.Desktop.App.Services;

/// <summary>
/// Settings contract exposing properties for app settings.
/// </summary>
public interface ISettingsManager<T> where T : ISettings, new()
{
/// <summary>
/// Reads the settings from the file system.
/// Always returns the latest settings, even if they were modified by another instance of the app.
/// Returned object is always a fresh instance, so it can be modified without affecting the stored settings.
/// </summary>
/// <param name="ct"></param>
/// <returns></returns>
public Task<T> Read(CancellationToken ct = default);
/// <summary>
/// Writes the settings to the file system.
/// </summary>
/// <param name="settings">Object containing the settings.</param>
/// <param name="ct"></param>
/// <returns></returns>
public Task Write(T settings, CancellationToken ct = default);
/// <summary>
/// Returns null if the settings are not cached or not available.
/// </summary>
/// <returns></returns>
public T? GetFromCache();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than have this, Read should probably return from cache by default. I know this goes against a point I made in a previous review but if we're going with the Read/Write API then it should just be all in

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, Read should probably not fail if it failed to read/parse the file. IDK what good behavior is, but I think for now just returning a default config is OK

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is definitely going against this comment :D
#113 (comment)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm like 50/50 on whether we should crash or just use the default config... IDK what the best option is. For now with a single option maybe it's fine to just keep going, but in the future if we add config options that are more sensitive to being clobbered it might be bad to just ignore a parse failure...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it throws an exception now and I think that's OK considering - if you fail to operate the file there's a high likelihood that the feature won't work ata ll.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that's fair. We can revisit this if it's causing issues

}

/// <summary>
/// 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<T> : ISettingsManager<T> where T : ISettings, new()
{
private readonly string _settingsFilePath;
private readonly string _appName = "CoderDesktop";
private string _fileName;
private readonly object _lock = new();

private T? _cachedSettings;

private readonly SemaphoreSlim _gate = new(1, 1);
private static readonly TimeSpan LockTimeout = TimeSpan.FromSeconds(3);

/// <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? settingsFilePath = null)
{
if (settingsFilePath is null)
{
settingsFilePath = Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData);
}
else if (!Path.IsPathRooted(settingsFilePath))
{
throw new ArgumentException("settingsFilePath must be an absolute path if provided", nameof(settingsFilePath));
}

var folder = Path.Combine(
settingsFilePath,
_appName);

Directory.CreateDirectory(folder);

_fileName = T.SettingsFileName;
_settingsFilePath = Path.Combine(folder, _fileName);
}

public async Task<T> Read(CancellationToken ct = default)
{
// try to get the lock with short timeout
if (!await _gate.WaitAsync(LockTimeout, ct).ConfigureAwait(false))
throw new InvalidOperationException(
$"Could not acquire the settings lock within {LockTimeout.TotalSeconds} s.");

try
{
if (!File.Exists(_settingsFilePath))
return new();

var json = await File.ReadAllTextAsync(_settingsFilePath, ct)
.ConfigureAwait(false);

// deserialize; fall back to default(T) if empty or malformed
var result = JsonSerializer.Deserialize<T>(json)!;
_cachedSettings = result;
return result;
}
catch (OperationCanceledException)
{
throw; // propagate caller-requested cancellation
}
catch (Exception ex)
{
throw new InvalidOperationException(
$"Failed to read settings from {_settingsFilePath}. " +
"The file may be corrupted, malformed or locked.", ex);
}
finally
{
_gate.Release();
}
}

public async Task Write(T settings, CancellationToken ct = default)
{
// try to get the lock with short timeout
if (!await _gate.WaitAsync(LockTimeout, ct).ConfigureAwait(false))
throw new InvalidOperationException(
$"Could not acquire the settings lock within {LockTimeout.TotalSeconds} s.");

try
{
// overwrite the settings file with the new settings
var json = JsonSerializer.Serialize(
settings, new JsonSerializerOptions() { WriteIndented = true });
_cachedSettings = settings; // cache the settings
await File.WriteAllTextAsync(_settingsFilePath, json, ct)
.ConfigureAwait(false);
}
catch (OperationCanceledException)
{
throw; // let callers observe cancellation
}
catch (Exception ex)
{
throw new InvalidOperationException(
$"Failed to persist settings to {_settingsFilePath}. " +
"The file may be corrupted, malformed or locked.", ex);
}
finally
{
_gate.Release();
}
}

public T? GetFromCache()
{
return _cachedSettings;
}
}

public interface ISettings
{
/// <summary>
/// Gets the version of the settings schema.
/// </summary>
int Version { get; }

/// <summary>
/// FileName where the settings are stored.
/// </summary>
static abstract string SettingsFileName { get; }
}

/// <summary>
/// CoderConnect settings class that holds the settings for the CoderConnect feature.
/// </summary>
public class CoderConnectSettings : ISettings
{
/// <summary>
/// CoderConnect settings version. Increment this when the settings schema changes.
/// In future iterations we will be able to handle migrations when the user has
/// an older version.
/// </summary>
public int Version { get; set; }
public bool ConnectOnLaunch { get; set; }
public static string SettingsFileName { get; } = "coder-connect-settings.json";

private const int VERSION = 1; // Default version for backward compatibility
public CoderConnectSettings()
{
Version = VERSION;
ConnectOnLaunch = false;
}

public CoderConnectSettings(int? version, bool connectOnLogin)
{
Version = version ?? VERSION;
ConnectOnLaunch = connectOnLogin;
}

public CoderConnectSettings Clone()
{
return new CoderConnectSettings(Version, ConnectOnLaunch);
}


}
Loading
Loading