From 76b49677c46e1296b4c9ccb93d17f0838cf94292 Mon Sep 17 00:00:00 2001 From: AJ Isaacs Date: Mon, 12 Oct 2026 19:00:50 -0400 Subject: [PATCH] fix(data): keep clamp setting fallbacks physical and report nulls Read clamps.json as a raw document so an explicit null is reported as an ignored value (Partial, which the desktop warns about) instead of being taken as an absent field. Missing or invalid sizes now fall back to the physical 1.5 x 0.625 inch clamp expressed in the file's units, so a millimeter file no longer gets a 1.5 x 0.625 mm clamp. Values that would overflow when converted between inches and millimeters are ignored and reported instead of throwing while creating a nest. Review findings 2, 3 and 6, Project Memory 4e5f566d. --- OpenNest.Data/ClampSettings.cs | 68 ++++++++++------------- OpenNest.Tests/Data/ClampSettingsTests.cs | 58 +++++++++++++++++++ 2 files changed, 88 insertions(+), 38 deletions(-) diff --git a/OpenNest.Data/ClampSettings.cs b/OpenNest.Data/ClampSettings.cs index 3f231521..4c6d04f1 100644 --- a/OpenNest.Data/ClampSettings.cs +++ b/OpenNest.Data/ClampSettings.cs @@ -144,10 +144,20 @@ public sealed class ClampSettings return settings; } - ClampSettingsDto? dto; + // Read as a raw document so a missing field (fallback, Ok) is distinguishable from a + // present but unusable one, including an explicit null (fallback, Partial). + Dictionary values; try { - dto = JsonSerializer.Deserialize(File.ReadAllText(path), JsonOptions); + using var document = JsonDocument.Parse(File.ReadAllText(path)); + if (document.RootElement.ValueKind != JsonValueKind.Object) + { + status = ClampSettingsStatus.Invalid; + return settings; + } + values = new Dictionary(StringComparer.OrdinalIgnoreCase); + foreach (var property in document.RootElement.EnumerateObject()) + values[property.Name] = property.Value.Clone(); } catch (Exception ex) when (IsUnreadableFile(ex)) { @@ -155,15 +165,9 @@ public sealed class ClampSettings return settings; } - if (dto is null) - { - status = ClampSettingsStatus.Invalid; - return settings; - } - var ignored = false; - if (dto.Enabled is { } enabled) + if (values.TryGetValue("enabled", out var enabled)) { if (enabled.ValueKind is JsonValueKind.True or JsonValueKind.False) settings.Enabled = enabled.GetBoolean(); @@ -171,7 +175,7 @@ public sealed class ClampSettings ignored = true; } - if (dto.Units is { } units) + if (values.TryGetValue("units", out var units)) { if (TryParseUnits(units, out var parsed)) settings.Units = parsed; @@ -179,35 +183,34 @@ public sealed class ClampSettings ignored = true; } - if (dto.Width is { } width) + // Fallback sizes are the physical default clamp, expressed in the file's units. + settings.Width = ConvertDefault(DefaultWidth, settings.Units); + settings.Depth = ConvertDefault(DefaultDepth, settings.Units); + + if (values.TryGetValue("width", out var width)) { - if (TryPositive(width, out var value)) + if (TryLength(width, out var value) && value > 0) settings.Width = value; else ignored = true; } - if (dto.Depth is { } depth) + if (values.TryGetValue("depth", out var depth)) { - if (TryPositive(depth, out var value)) + if (TryLength(depth, out var value) && value > 0) settings.Depth = value; else ignored = true; } - if (dto.Centers is { } centers) + if (values.TryGetValue("centers", out var centers)) { if (centers.ValueKind == JsonValueKind.Array) { var valid = new List(); foreach (var center in centers.EnumerateArray()) { - if ( - center.ValueKind == JsonValueKind.Number - && center.TryGetDouble(out var x) - && double.IsFinite(x) - && x >= 0 - ) + if (TryLength(center, out var x) && x >= 0) valid.Add(x); else ignored = true; @@ -267,13 +270,17 @@ public sealed class ClampSettings private static double ConvertDefault(double inches, Units units) => inches * Scale(Units.Inches, units); - private static bool TryPositive(JsonElement element, out double value) + /// + /// A finite JSON number that stays finite in either unit, so applying it to an inch or a + /// millimeter nest can never overflow. + /// + private static bool TryLength(JsonElement element, out double value) { value = 0; return element.ValueKind == JsonValueKind.Number && element.TryGetDouble(out value) && double.IsFinite(value) - && value > 0; + && double.IsFinite(value * MillimetersPerInch); } /// @@ -306,19 +313,4 @@ public sealed class ClampSettings or UnauthorizedAccessException or NotSupportedException or System.Security.SecurityException; - - /// - /// Wire format. Every value is read as raw JSON so a wrongly typed field falls back on - /// its own instead of failing the whole file; unknown fields (including a future higher - /// version) are ignored. - /// - private sealed record ClampSettingsDto - { - public JsonElement? Version { get; init; } - public JsonElement? Enabled { get; init; } - public JsonElement? Units { get; init; } - public JsonElement? Width { get; init; } - public JsonElement? Depth { get; init; } - public JsonElement? Centers { get; init; } - } } diff --git a/OpenNest.Tests/Data/ClampSettingsTests.cs b/OpenNest.Tests/Data/ClampSettingsTests.cs index c90bf306..94f7234b 100644 --- a/OpenNest.Tests/Data/ClampSettingsTests.cs +++ b/OpenNest.Tests/Data/ClampSettingsTests.cs @@ -201,6 +201,64 @@ public class ClampSettingsTests : IDisposable Assert.Equal(ClampSettingsStatus.Partial, LoadStatus(_path)); } + [Theory] + [InlineData("enabled")] + [InlineData("units")] + [InlineData("width")] + [InlineData("depth")] + [InlineData("centers")] + public void ExplicitNullIsAnIgnoredValueAndIsReported(string field) + { + File.WriteAllText(_path, $$"""{ "enabled": true, "centers": [6], "{{field}}": null }"""); + + var loaded = ClampSettings.Load(_path, out var status); + + Assert.Equal(ClampSettingsStatus.Partial, status); + Assert.Equal(1.5, loaded.Width); + Assert.Equal(0.625, loaded.Depth); + } + + [Fact] + public void MetricFileWithoutSizesGetsThePhysicalDefaultClamp() + { + File.WriteAllText(_path, """{ "enabled": true, "units": "millimeters", "centers": [152.4] }"""); + + var loaded = ClampSettings.Load(_path, out var status); + + Assert.Equal(ClampSettingsStatus.Ok, status); + Assert.Equal(1.5 * 25.4, loaded.Width, 9); + Assert.Equal(0.625 * 25.4, loaded.Depth, 9); + Assert.Equal(1.5, loaded.RailFor(Units.Inches)!.Width, 9); + } + + [Fact] + public void MetricFileWithAnInvalidSizeFallsBackToThePhysicalDefault() + { + File.WriteAllText(_path, """{ "enabled": true, "units": "millimeters", "width": -1, "depth": 15, "centers": [152.4] }"""); + + var loaded = ClampSettings.Load(_path, out var status); + + Assert.Equal(ClampSettingsStatus.Partial, status); + Assert.Equal(38.1, loaded.Width, 9); + Assert.Equal(15, loaded.Depth); + } + + [Theory] + [InlineData("""{ "enabled": true, "width": 1e308, "centers": [6] }""")] + [InlineData("""{ "enabled": true, "depth": 1e308, "centers": [6] }""")] + [InlineData("""{ "enabled": true, "centers": [6, 1e308] }""")] + public void ValuesThatCannotConvertBetweenUnitsAreIgnored(string json) + { + File.WriteAllText(_path, json); + + var loaded = ClampSettings.Load(_path, out var status); + + Assert.Equal(ClampSettingsStatus.Partial, status); + var nest = new Nest { Units = Units.Millimeters }; + loaded.ApplyToNewNest(nest); // must not throw while creating a nest + Assert.Equal(new[] { 6 * 25.4 }, nest.PlateDefaults.ClampRail!.Centers); + } + [Fact] public void AnInvalidCenterIsDroppedButTheRestOfTheRailStillLoads() {