mirror of
https://github.com/ajisaacs/OpenNest.git
synced 2026-10-22 10:41:37 -04:00
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.
This commit is contained in:
1 parent
aea51b5e1d
commit
76b49677c4
2 files changed
+88
-38
No files matched your search
@@ -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<string, JsonElement> values;
|
||||
try
|
||||
{
|
||||
dto = JsonSerializer.Deserialize<ClampSettingsDto>(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<string, JsonElement>(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<double>();
|
||||
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)
|
||||
/// <summary>
|
||||
/// A finite JSON number that stays finite in either unit, so applying it to an inch or a
|
||||
/// millimeter nest can never overflow.
|
||||
/// </summary>
|
||||
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);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
@@ -306,19 +313,4 @@ public sealed class ClampSettings
|
||||
or UnauthorizedAccessException
|
||||
or NotSupportedException
|
||||
or System.Security.SecurityException;
|
||||
|
||||
/// <summary>
|
||||
/// 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
|
||||
/// <c>version</c>) are ignored.
|
||||
/// </summary>
|
||||
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; }
|
||||
}
|
||||
}
|
||||
@@ -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()
|
||||
{
|
||||
|
||||
Reference in new issue
Block a user