From b7dc7dee13e3c0cb9199fde6421c16b310e5287b Mon Sep 17 00:00:00 2001 From: AJ Isaacs Date: Sun, 4 Oct 2026 12:01:12 -0400 Subject: [PATCH] fix(defaults): accept only defined unit names, with a caller fallback Enum.TryParse accepted numeric strings ("7" loaded an undefined unit, "1" loaded millimeters) and comma-joined names, and a JSON number for units failed the whole file. Units now load only from a defined unit name; anything else falls back for that field alone, while the other valid fields still load. Add Load(path, fallbackUnits, out status) so callers can keep their existing unit preference when the file is missing, unusable, or lacks a usable unit (previously a readable file without units silently became inches). An undefined fallback is ignored in favour of the built-in units. The numeric/comma/JSON-number rows failed against the previous commit; dropping the fallback assignment or its IsDefined guard fails the new fallback tests. --- OpenNest.Data/NestDefaults.cs | 54 ++++++++++++++++--- OpenNest.Tests/Data/NestDefaultsTests.cs | 68 ++++++++++++++++++++++++ 2 files changed, 115 insertions(+), 7 deletions(-) diff --git a/OpenNest.Data/NestDefaults.cs b/OpenNest.Data/NestDefaults.cs index 66ae2ca..7fd24e0 100644 --- a/OpenNest.Data/NestDefaults.cs +++ b/OpenNest.Data/NestDefaults.cs @@ -73,9 +73,26 @@ public sealed class NestDefaults /// present but unreadable/invalid, so callers can warn about a corrupt /// file while still returning usable values. /// - public static NestDefaults Load(string path, out NestDefaultsStatus status) + public static NestDefaults Load(string path, out NestDefaultsStatus status) => + Load(path, Fallback.Units, out status); + + /// + /// Loads defaults like , + /// but a missing file, an unusable file, or a missing/undefined unit + /// field yields (the caller's existing + /// unit preference) instead of the built-in units. Other valid fields + /// still load. An undefined is ignored. + /// + public static NestDefaults Load( + string path, + Units fallbackUnits, + out NestDefaultsStatus status + ) { var defaults = Fallback; + if (Enum.IsDefined(fallbackUnits)) + defaults.Units = fallbackUnits; + if (string.IsNullOrWhiteSpace(path) || !File.Exists(path)) { status = NestDefaultsStatus.Missing; @@ -104,10 +121,7 @@ public sealed class NestDefaults status = NestDefaultsStatus.Ok; - if ( - dto.Units is not null - && Enum.TryParse(dto.Units, ignoreCase: true, out var units) - ) + if (TryParseUnits(dto.Units, out var units)) defaults.Units = units; if ( @@ -192,7 +206,7 @@ public sealed class NestDefaults var dto = new NestDefaultsDto { Version = CurrentVersion, - Units = Units.ToString().ToLowerInvariant(), + Units = JsonSerializer.SerializeToElement(Units.ToString().ToLowerInvariant()), Size = new SizeDto { Width = Size.Width, Length = Size.Length }, Quadrant = Quadrant, PartSpacing = PartSpacing, @@ -225,6 +239,30 @@ public sealed class NestDefaults } } + /// + /// Accepts only a defined unit name (case-insensitive). Enum.TryParse + /// would also accept numeric strings ("7") and comma-joined names, + /// producing undefined or unintended values. + /// + private static bool TryParseUnits(JsonElement? element, out Units units) + { + units = default; + if (element is not { ValueKind: JsonValueKind.String } value) + return false; + + var text = value.GetString(); + foreach (var candidate in Enum.GetValues()) + { + if (string.Equals(candidate.ToString(), text, StringComparison.OrdinalIgnoreCase)) + { + units = candidate; + return true; + } + } + + return false; + } + private static bool IsUnreadableFile(Exception ex) => ex is JsonException @@ -252,7 +290,9 @@ public sealed class NestDefaults private sealed record NestDefaultsDto { public int? Version { get; init; } = CurrentVersion; - public string? Units { get; init; } + // Read as raw JSON so a wrongly typed unit falls back on its own + // instead of failing the whole file. + public JsonElement? Units { get; init; } public SizeDto? Size { get; init; } public int? Quadrant { get; init; } public double? PartSpacing { get; init; } diff --git a/OpenNest.Tests/Data/NestDefaultsTests.cs b/OpenNest.Tests/Data/NestDefaultsTests.cs index f6b1004..6ff5cdf 100644 --- a/OpenNest.Tests/Data/NestDefaultsTests.cs +++ b/OpenNest.Tests/Data/NestDefaultsTests.cs @@ -147,6 +147,74 @@ public class NestDefaultsTests : IDisposable Assert.Equal(1, loaded.PartSpacing); } + [Theory] + [InlineData("\"7\"")] + [InlineData("\"1\"")] + [InlineData("\"-1\"")] + [InlineData("\"inches, millimeters\"")] + [InlineData("1")] + public void Load_UndefinedOrNumericUnits_FallBackPerField(string unitsJson) + { + File.WriteAllText(_path, $$"""{ "units": {{unitsJson}}, "partSpacing": 4 }"""); + + var loaded = NestDefaults.Load(_path, out var status); + + // Only a defined unit name is accepted; the other fields still load. + Assert.Equal(NestDefaultsStatus.Ok, status); + Assert.Equal(Units.Inches, loaded.Units); + Assert.Equal(4, loaded.PartSpacing); + } + + [Theory] + [InlineData("""{ "partSpacing": 4 }""")] + [InlineData("""{ "units": "7", "partSpacing": 4 }""")] + [InlineData("""{ "units": "furlongs", "partSpacing": 4 }""")] + [InlineData("""{ "units": null, "partSpacing": 4 }""")] + public void Load_MissingOrInvalidUnits_UseCallerFallbackUnits(string json) + { + File.WriteAllText(_path, json); + + var loaded = NestDefaults.Load(_path, Units.Millimeters, out var status); + + // The caller's existing unit preference survives a readable file + // that lacks a usable unit, and the valid fields still load. + Assert.Equal(NestDefaultsStatus.Ok, status); + Assert.Equal(Units.Millimeters, loaded.Units); + Assert.Equal(4, loaded.PartSpacing); + } + + [Fact] + public void Load_SavedUnits_OverrideCallerFallbackUnits() + { + new NestDefaults { Units = Units.Inches }.Save(_path); + + var loaded = NestDefaults.Load(_path, Units.Millimeters, out var status); + + Assert.Equal(NestDefaultsStatus.Ok, status); + Assert.Equal(Units.Inches, loaded.Units); + } + + [Fact] + public void Load_MissingOrCorruptFile_UsesCallerFallbackUnits() + { + var missing = NestDefaults.Load(_path, Units.Millimeters, out var missingStatus); + File.WriteAllText(_path, "{ this is not json"); + var corrupt = NestDefaults.Load(_path, Units.Millimeters, out var corruptStatus); + + Assert.Equal(NestDefaultsStatus.Missing, missingStatus); + Assert.Equal(Units.Millimeters, missing.Units); + Assert.Equal(NestDefaultsStatus.Invalid, corruptStatus); + Assert.Equal(Units.Millimeters, corrupt.Units); + } + + [Fact] + public void Load_UndefinedCallerFallbackUnits_UseBuiltInUnits() + { + var loaded = NestDefaults.Load(_path, (Units)7, out _); + + Assert.Equal(Units.Inches, loaded.Units); + } + [Fact] public void Load_UnknownFieldsAndFutureVersion_Ignored() {