From fd2e9c2f56763fc657467c07a2458817e62b5136 Mon Sep 17 00:00:00 2001 From: AJ Isaacs Date: Tue, 27 Oct 2026 21:44:07 -0400 Subject: [PATCH] fix(settings): keep user settings across reinstalls and versions WinForms user settings lived in user.config under %LOCALAPPDATA%\OpenNest, the Velopack install folder that setup clears on a reinstall, and the default provider starts empty at each assembly version. Store them in %APPDATA%\OpenNest\user-settings.json instead, carrying over any values the old user.config still holds on first run. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 2 +- .../AppDataSettingsProviderTests.cs | 160 ++++++++++++++++++ .../Properties/AppDataSettingsProvider.cs | 150 ++++++++++++++++ OpenNest/Properties/Settings.cs | 10 ++ docs/auto-update.md | 7 + 5 files changed, 328 insertions(+), 1 deletion(-) create mode 100644 OpenNest.WinForms.Tests/AppDataSettingsProviderTests.cs create mode 100644 OpenNest/Properties/AppDataSettingsProvider.cs create mode 100644 OpenNest/Properties/Settings.cs diff --git a/AGENTS.md b/AGENTS.md index 5c517b1a..78c03eaf 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -40,7 +40,7 @@ Releases: follow [the release procedure](docs/releasing.md) and `scripts/Publish - `OpenNest.Engine`: whole-job API in `Jobs/`, interactive proposals via `PlateFillService`, fill strategies, best-fit pairs, packing, sequencing and rapid planning. `INestingEngine.Solve(NestJob)` returns stock IDs/poses; boundary adapters map drawings and materialize results. `NestJobRunner` validates its candidates before committing demand/stock accounting. Do not assume arbitrary plug-in output or interactive paths received that validation. Job identity is reference-based, not drawing-name-based. - Built-in whole-job engines live in `OpenNest.Engine/NestingEngines//`, named for the jobs they suit; see [nesting engines](docs/nesting-engines.md). A change must beat that engine's current benchmark result with every layout valid. External plug-ins implement `INestingEngine` with a public parameterless constructor and load from `Engines/` beside the host; keep their projects out of this solution. - `OpenNest.IO`: ACadSharp import/export and ZIP-based `.nest` persistence; depends on Core only (an IO.Tests guard enforces it). Best fits persist only when callers pass `BestFitCacheStore.Shared` as `NestReader`/`NestWriter.BestFitStore`. All DXF-to-Drawing conversion goes through `CadImporter`: `Import` + `BuildDrawing` for editable/reporting flows, `ImportDrawing` for headless callers. Preserve source offsets, entity IDs, suppressed entities and bends. Bend repair is opt-in, requires explicit source units and may not alter cut geometry or unrelated marks. -- `OpenNest`: WinForms UI (`Forms/`, `Controls/PlateView`, `Actions/`). `OpenNest.Data` holds cross-platform persistence; new-nest defaults live in `%APPDATA%\OpenNest\defaults.json` (in Database mode, the server's shared copy once it holds one). Posts live in `Posts/OpenNest.Posts./` and deploy to the desktop output's `Posts/` directory. Desktop nest edits go through `BeginChange`/`RecordChange`; see [undo and redo](docs/undo-redo.md). +- `OpenNest`: WinForms UI (`Forms/`, `Controls/PlateView`, `Actions/`). `OpenNest.Data` holds cross-platform persistence; new-nest defaults live in `%APPDATA%\OpenNest\defaults.json` (in Database mode, the server's shared copy once it holds one). WinForms `Properties.Settings` persist to `%APPDATA%\OpenNest\user-settings.json` through `AppDataSettingsProvider`, never inside the Velopack install folder. Posts live in `Posts/OpenNest.Posts./` and deploy to the desktop output's `Posts/` directory. Desktop nest edits go through `BeginChange`/`RecordChange`; see [undo and redo](docs/undo-redo.md). - `OpenNest.Desktop` (net8.0): toolkit-neutral desktop logic shared by the WinForms app and `OpenNest.Avalonia`, the cross-platform desktop in progress. It must not reference WinForms/WindowsDesktop, `System.Drawing.Common` or Avalonia (`OpenNest.Desktop.Tests` guards this, including transitive packages); put toolkit-specific rendering, dialogs and settings behind adapters in the front ends. - `OpenNest.Console`, `OpenNest.Mcp`, `OpenNest.Api`: front ends; `OpenNest.Benchmark`: whole-job engine comparisons; `OpenNest.Gpu`: GPU evaluators; `OpenNest.Training`: ML data collection. Benchmark timing comparisons require `--parallel 1`; validate layouts and fulfillment, not just elapsed time. diff --git a/OpenNest.WinForms.Tests/AppDataSettingsProviderTests.cs b/OpenNest.WinForms.Tests/AppDataSettingsProviderTests.cs new file mode 100644 index 00000000..5a144be3 --- /dev/null +++ b/OpenNest.WinForms.Tests/AppDataSettingsProviderTests.cs @@ -0,0 +1,160 @@ +using System.Collections.Specialized; +using System.Configuration; +using System.Drawing; +using System.Reflection; +using OpenNest.Properties; + +namespace OpenNest.WinForms.Tests; + +public sealed class AppDataSettingsProviderTests : IDisposable +{ + private readonly string _directory = Path.Combine(Path.GetTempPath(), "OpenNestSettings-" + Guid.NewGuid().ToString("N")); + + private string SettingsPath => Path.Combine(_directory, "user-settings.json"); + + public void Dispose() + { + if (Directory.Exists(_directory)) + Directory.Delete(_directory, recursive: true); + } + + [Fact] + public void SettingsUseTheAppDataProvider() + { + var attribute = typeof(Settings).GetCustomAttribute(); + + Assert.NotNull(attribute); + Assert.Equal(typeof(AppDataSettingsProvider).AssemblyQualifiedName, attribute.ProviderTypeName); + } + + [Fact] + public void SavedValuesReadBackInANewProvider() + { + var properties = Properties(); + var values = new AppDataSettingsProvider(SettingsPath, null).GetPropertyValues(new SettingsContext(), properties); + values["IncludeDevBuilds"].PropertyValue = true; + values["WindowLocation"].PropertyValue = new Point(40, 60); + new AppDataSettingsProvider(SettingsPath, null).SetPropertyValues(new SettingsContext(), values); + + var reloaded = new AppDataSettingsProvider(SettingsPath, null).GetPropertyValues(new SettingsContext(), Properties()); + + Assert.Equal(true, reloaded["IncludeDevBuilds"].PropertyValue); + Assert.Equal(new Point(40, 60), reloaded["WindowLocation"].PropertyValue); + Assert.Equal("", reloaded["FeedUrl"].PropertyValue); + } + + [Fact] + public void UntouchedDefaultsAreNotWritten() + { + var provider = new AppDataSettingsProvider(SettingsPath, null); + var values = provider.GetPropertyValues(new SettingsContext(), Properties()); + _ = values["IncludeDevBuilds"].PropertyValue; + values["FeedUrl"].PropertyValue = "http://feed"; + provider.SetPropertyValues(new SettingsContext(), values); + + var json = File.ReadAllText(SettingsPath); + + Assert.Contains("FeedUrl", json); + Assert.DoesNotContain("IncludeDevBuilds", json); + Assert.DoesNotContain("WindowLocation", json); + } + + [Fact] + public void EntriesThisBuildDoesNotKnowAreKept() + { + Directory.CreateDirectory(_directory); + File.WriteAllText(SettingsPath, """{ "FromNewerBuild": "x" }"""); + var provider = new AppDataSettingsProvider(SettingsPath, null); + var values = provider.GetPropertyValues(new SettingsContext(), Properties()); + values["FeedUrl"].PropertyValue = "http://feed"; + + provider.SetPropertyValues(new SettingsContext(), values); + + Assert.Contains("FromNewerBuild", File.ReadAllText(SettingsPath)); + } + + [Fact] + public void LegacyValuesAreCarriedOverOnceWhenTheFileIsMissing() + { + var legacyCalls = 0; + SettingsProvider Legacy() + { + legacyCalls++; + return new FixedProvider(new() { ["IncludeDevBuilds"] = "True" }); + } + + var values = new AppDataSettingsProvider(SettingsPath, Legacy).GetPropertyValues(new SettingsContext(), Properties()); + var reloaded = new AppDataSettingsProvider(SettingsPath, Legacy).GetPropertyValues(new SettingsContext(), Properties()); + + Assert.Equal(true, values["IncludeDevBuilds"].PropertyValue); + Assert.Equal(true, reloaded["IncludeDevBuilds"].PropertyValue); + Assert.Equal(1, legacyCalls); + } + + [Fact] + public void TheDefaultUserConfigProviderCanBeReadForCarryOver() + { + var context = new SettingsContext { ["GroupName"] = typeof(AppDataSettingsProviderTests).FullName, ["SettingsKey"] = "" }; + + var values = new AppDataSettingsProvider(SettingsPath, () => new LocalFileSettingsProvider()) + .GetPropertyValues(context, Properties()); + + Assert.Equal(false, values["IncludeDevBuilds"].PropertyValue); + Assert.False(File.Exists(SettingsPath)); + } + + [Fact] + public void ACorruptFileReadsAsDefaults() + { + Directory.CreateDirectory(_directory); + File.WriteAllText(SettingsPath, "not json"); + + var values = new AppDataSettingsProvider(SettingsPath, null).GetPropertyValues(new SettingsContext(), Properties()); + + Assert.Equal(false, values["IncludeDevBuilds"].PropertyValue); + } + + private static SettingsPropertyCollection Properties() + { + return new SettingsPropertyCollection + { + UserSetting("IncludeDevBuilds", typeof(bool), "False"), + UserSetting("FeedUrl", typeof(string), ""), + UserSetting("WindowLocation", typeof(Point), "0, 0"), + }; + } + + private static SettingsProperty UserSetting(string name, Type type, string defaultValue) + { + var attributes = new SettingsAttributeDictionary { [typeof(UserScopedSettingAttribute)] = new UserScopedSettingAttribute() }; + return new SettingsProperty(name, type, null, false, defaultValue, SettingsSerializeAs.String, attributes, false, false); + } + + /// Stands in for user.config: returns the given serialized values. + private sealed class FixedProvider : SettingsProvider + { + private readonly Dictionary _values; + + public FixedProvider(Dictionary values) => _values = values; + + public override string ApplicationName { get; set; } = "Test"; + + public override void Initialize(string name, NameValueCollection config) => base.Initialize("Fixed", config); + + public override SettingsPropertyValueCollection GetPropertyValues(SettingsContext context, SettingsPropertyCollection collection) + { + var values = new SettingsPropertyValueCollection(); + foreach (SettingsProperty property in collection) + { + var value = new SettingsPropertyValue(property); + if (_values.TryGetValue(property.Name, out var serialized)) + value.SerializedValue = serialized; + values.Add(value); + } + + return values; + } + + public override void SetPropertyValues(SettingsContext context, SettingsPropertyValueCollection collection) { } + } +} diff --git a/OpenNest/Properties/AppDataSettingsProvider.cs b/OpenNest/Properties/AppDataSettingsProvider.cs new file mode 100644 index 00000000..a8cedd90 --- /dev/null +++ b/OpenNest/Properties/AppDataSettingsProvider.cs @@ -0,0 +1,150 @@ +using System; +using System.Collections.Generic; +using System.Collections.Specialized; +using System.Configuration; +using System.IO; +using System.Text.Json; + +namespace OpenNest.Properties +{ + /// + /// Stores the desktop app's user-scoped in + /// %APPDATA%\OpenNest\user-settings.json instead of .NET's default user.config. + /// + /// + /// The default provider writes to %LOCALAPPDATA%\OpenNest\<exe>_Url_<hash>\<version>\, + /// which is inside the Velopack install folder (setup wipes it on reinstall) and + /// starts empty at every assembly version. This file survives both. When it does + /// not exist yet, values the default provider still holds for this copy are + /// carried over once. + /// + public sealed class AppDataSettingsProvider : SettingsProvider + { + private static readonly JsonSerializerOptions JsonOptions = new() { WriteIndented = true }; + + private readonly string _path; + private readonly Func _legacyProvider; + + public AppDataSettingsProvider() + : this(DefaultPath, () => new LocalFileSettingsProvider()) { } + + /// The settings file. + /// Supplies values to carry over when does not exist; null for none. + public AppDataSettingsProvider(string path, Func legacyProvider) + { + _path = path; + _legacyProvider = legacyProvider; + } + + /// %APPDATA%\OpenNest\user-settings.json. + public static string DefaultPath => Path.Combine( + Environment.GetFolderPath(Environment.SpecialFolder.ApplicationData), + "OpenNest", "user-settings.json"); + + public override string ApplicationName { get; set; } = "OpenNest"; + + public override void Initialize(string name, NameValueCollection config) + => base.Initialize(string.IsNullOrEmpty(name) ? nameof(AppDataSettingsProvider) : name, config); + + public override SettingsPropertyValueCollection GetPropertyValues(SettingsContext context, SettingsPropertyCollection collection) + { + var stored = File.Exists(_path) ? Load() : Migrate(context, collection); + var values = new SettingsPropertyValueCollection(); + + foreach (SettingsProperty property in collection) + { + var value = new SettingsPropertyValue(property); + if (stored.TryGetValue(property.Name, out var serialized)) + { + value.SerializedValue = serialized; + value.IsDirty = false; + } + + values.Add(value); + } + + return values; + } + + public override void SetPropertyValues(SettingsContext context, SettingsPropertyValueCollection collection) + { + // Keep entries this build does not know about, so older and newer copies share the file. + var stored = Load(); + + foreach (SettingsPropertyValue value in collection) + { + // Untouched defaults stay out of the file so a later default change still applies. + if (!value.IsDirty && (value.UsingDefaultValue || value.SerializedValue is null)) + continue; + + if (value.SerializedValue is string serialized) + stored[value.Property.Name] = serialized; + else + stored.Remove(value.Property.Name); + + value.IsDirty = false; + } + + Write(stored); + } + + private Dictionary Load() + { + try + { + return File.Exists(_path) + ? JsonSerializer.Deserialize>(File.ReadAllText(_path)) ?? new() + : new(); + } + catch (Exception ex) when (ex is JsonException or IOException or UnauthorizedAccessException) + { + return new(); + } + } + + private Dictionary Migrate(SettingsContext context, SettingsPropertyCollection collection) + { + var stored = new Dictionary(); + if (_legacyProvider is null) + return stored; + + try + { + var legacy = _legacyProvider(); + legacy.Initialize(null, null); + // user.config reports a missing setting as its default text; skip those. + foreach (SettingsPropertyValue value in legacy.GetPropertyValues(context, collection)) + { + if (value.SerializedValue is string serialized + && serialized != value.Property.DefaultValue as string) + stored[value.Property.Name] = serialized; + } + } + catch (Exception ex) when (ex is ConfigurationException or IOException or UnauthorizedAccessException) + { + // An unreadable user.config only means there is nothing to carry over. + return new(); + } + + if (stored.Count > 0) + Write(stored); + + return stored; + } + + private void Write(Dictionary stored) + { + try + { + Directory.CreateDirectory(Path.GetDirectoryName(Path.GetFullPath(_path))); + var temp = _path + ".tmp"; + File.WriteAllText(temp, JsonSerializer.Serialize(stored, JsonOptions)); + File.Move(temp, _path, overwrite: true); + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) + { + // Settings are a convenience: keep running with the in-memory values. + } + } + } +} diff --git a/OpenNest/Properties/Settings.cs b/OpenNest/Properties/Settings.cs new file mode 100644 index 00000000..f59da263 --- /dev/null +++ b/OpenNest/Properties/Settings.cs @@ -0,0 +1,10 @@ +using System.Configuration; + +namespace OpenNest.Properties +{ + // Keeps user settings outside the Velopack install folder; see AppDataSettingsProvider. + [SettingsProvider(typeof(AppDataSettingsProvider))] + internal sealed partial class Settings + { + } +} diff --git a/docs/auto-update.md b/docs/auto-update.md index 04762db3..a87b128a 100644 --- a/docs/auto-update.md +++ b/docs/auto-update.md @@ -37,6 +37,13 @@ user) to the feed's base URL (`http://host:port`) and restart. A self-hosted feed URL is operator infrastructure and never belongs in source control; clearing the field returns to GitHub Releases. +These and the app's other `OpenNest.Properties.Settings` values live in +`%APPDATA%\OpenNest\user-settings.json` (`AppDataSettingsProvider`), shared by every +copy run by that Windows user. .NET's default `user.config` sat inside the Velopack +install folder, which setup clears on a reinstall, and started empty at each new +assembly version; the first run without the JSON file carries over any values that +copy's `user.config` still holds. + Velopack only applies updates to a Velopack installation (`UpdateManager.IsInstalled`): a copy installed with `OpenNest-win-Setup.exe` or extracted from `OpenNest-win-Portable.zip`. The plain `OpenNest.vX.Y.Z.win-x64.zip` and ordinary