diff --git a/OpenNest.Core/CNC/CuttingPlanning/PlateCuttingState.cs b/OpenNest.Core/CNC/CuttingPlanning/PlateCuttingState.cs index a9694af..1902fa6 100644 --- a/OpenNest.Core/CNC/CuttingPlanning/PlateCuttingState.cs +++ b/OpenNest.Core/CNC/CuttingPlanning/PlateCuttingState.cs @@ -52,7 +52,9 @@ public sealed class PlateCuttingState /// /// Captures on the caller thread. Unsupported or malformed programs throw - /// or . + /// or . Settings whose + /// authored state cannot be captured exactly are recorded as refused: the plate stays + /// plannable but every later commit against it reports Stale. /// public static PlateCuttingState Capture(Plate plate, CancellationToken token = default) { @@ -83,6 +85,8 @@ public sealed class PlateCuttingState void Fingerprint(CuttingParameters parameters) { + // A refused capture is stored as-is; Difference treats it as never current, so + // every Apply for such settings is Stale and no foreign code ever runs at Apply. if (parameters != null && !settings.ContainsKey(parameters)) settings.Add(parameters, StateFingerprint.Of(parameters)); } @@ -94,6 +98,9 @@ public sealed class PlateCuttingState /// Null when current; otherwise the first observed difference. public string Difference(CancellationToken token = default) { + const string Current = "current"; + const string Stale = "stale"; + var checkedSettings = new Dictionary(ReferenceEqualityComparer.Instance); var plate = Plate; if (!ReferenceEquals(plate.Parts, partList) || !ReferenceEquals(plate.CutOffs, cutOffList)) return "The plate's part or cutoff list was replaced."; @@ -151,18 +158,31 @@ public sealed class PlateCuttingState return null; // References were compared already; this catches in-place edits of the same object. + // Each distinct settings object is fingerprinted at most once per check. A refused + // capture (Invalid) is never current: the state cannot be proven unchanged, and the + // live object is deliberately not re-read. A fingerprint that fails on re-read is + // likewise stale, never an escaping exception (cancellation stays distinct). bool SameSettings(CuttingParameters parameters) { if (parameters == null) return true; - try + if (checkedSettings.TryGetValue(parameters, out var decision)) + return decision == Current; + var current = false; + if (settings.TryGetValue(parameters, out var captured) + && captured != StateFingerprint.Invalid) { - return settings.TryGetValue(parameters, out var captured) && captured == StateFingerprint.Of(parameters); - } - catch (NotSupportedException) - { - return false; // Grown past the exact fingerprint budget since capture. + try + { + current = captured == StateFingerprint.Of(parameters); + } + catch (Exception exception) when (exception is not OperationCanceledException) + { + current = false; + } } + checkedSettings[parameters] = current ? Current : Stale; + return current; } } diff --git a/OpenNest.Core/CNC/CuttingPlanning/StateFingerprint.cs b/OpenNest.Core/CNC/CuttingPlanning/StateFingerprint.cs index 7d0c8cd..71dcc01 100644 --- a/OpenNest.Core/CNC/CuttingPlanning/StateFingerprint.cs +++ b/OpenNest.Core/CNC/CuttingPlanning/StateFingerprint.cs @@ -4,104 +4,171 @@ using System.Collections.Generic; using System.Globalization; using System.Linq; using System.Reflection; -using System.Text; +using OpenNest.CNC.CuttingStrategy; namespace OpenNest.CNC.CuttingPlanning; /// -/// Exact text record of an object's public state (properties and fields, recursively; scalars by -/// bits, runtime types included) for detecting in-place edits of settings after capture. It is a -/// freshness fingerprint, not a serializer. Value types contribute fields only, so computed struct -/// properties cannot recurse without bound. +/// Exact text record of a settings object's authored state for detecting in-place edits of +/// settings after capture. It is a freshness fingerprint, not a serializer. +/// No code of a foreign (non-OpenNest) type is ever executed: capture reads no foreign +/// property getter (it could mutate live state or throw) and enumerates no foreign collection +/// (its enumerator could throw after capture). Concretely it renders invariant scalars +/// (doubles by bit pattern, so results are culture-free); arrays and the exact BCL containers +/// List/Dictionary/HashSet/KeyValuePair, whose contents are rendered recursively and, where +/// order is insertion order rather than authored semantics, sorted; concrete public types of +/// an OpenNest assembly through their public readable properties and fields; and any other +/// concrete type through its declared instance fields only, which include auto-property +/// backing fields, so edits to them are seen. Behavioral enumerables and delegates are +/// refused, as are depth and node-budget overflow and member-read failures; a reference +/// already on the current path renders as a stable cycle marker, which loses nothing because +/// the first visit rendered everything reachable from it. A refusal yields +/// , which never compares equal, so refused state is unequal (Stale) +/// instead of silently equal. /// internal static class StateFingerprint { - private const int MaxDepth = 16; + internal const string Invalid = ""; + private const int MaxDepth = 24; private const int NodeBudget = 100000; + private static readonly Assembly Bcl = typeof(object).Assembly; + private static readonly Assembly[] OpenNestAssemblies = + [typeof(CuttingParameters).Assembly, typeof(OpenNest.Geometry.Vector).Assembly]; + /// + /// Fingerprint of , or when the exact + /// authored state cannot be captured without executing foreign code or guessing. + /// internal static string Of(object value) { - var text = new StringBuilder(); - var path = new HashSet(ReferenceEqualityComparer.Instance); var nodes = 0; - Append(value, 0); - return text.ToString(); + var exact = true; + var path = new HashSet(ReferenceEqualityComparer.Instance); + var result = Render(value, 0); + return exact ? result : Invalid; - void Append(object item, int depth) + string Render(object item, int depth) { - if (++nodes > NodeBudget) - throw new NotSupportedException("Settings are too large to fingerprint exactly."); - if (item == null) + if (!exact) + return Invalid; + if (++nodes > NodeBudget || depth > MaxDepth) { - text.Append("null;"); - return; + exact = false; // Truncation is lossy: never claim it as equal-able state. + return Invalid; } + if (item == null) + return "null"; var type = item.GetType(); - text.Append(type.FullName).Append('='); switch (item) { case double number: - text.Append(BitConverter.DoubleToInt64Bits(number)).Append(';'); - return; + // Bit pattern: NaN, infinities and signed zero stay exact. Explicit + // invariant formatting: ambient culture can substitute other digits. + return $"{type.FullName}#{BitConverter.DoubleToInt64Bits(number).ToString(CultureInfo.InvariantCulture)}"; case float number: - text.Append(BitConverter.SingleToInt32Bits(number)).Append(';'); - return; + return $"{type.FullName}#{BitConverter.SingleToInt32Bits(number).ToString(CultureInfo.InvariantCulture)}"; case string characters: - text.Append(characters.Length).Append(':').Append(characters).Append(';'); - return; + return $"{type.FullName}#{characters.Length.ToString(CultureInfo.InvariantCulture)}:{characters}"; } - if (type.IsPrimitive || type.IsEnum || item is decimal) + if (type.IsPrimitive || type.IsEnum || item is decimal or Guid || item is DateTime + || item is DateTimeOffset || item is TimeSpan) + return $"{type.FullName}#{Convert.ToString(item, CultureInfo.InvariantCulture)}"; + if (typeof(Delegate).IsAssignableFrom(type)) { - text.Append(Convert.ToString(item, CultureInfo.InvariantCulture)).Append(';'); - return; + exact = false; // A delegate is behavior, not authored state. + return Invalid; } - if (depth >= MaxDepth || (!type.IsValueType && !path.Add(item))) + + var definition = type.IsGenericType ? type.GetGenericTypeDefinition() : null; + var exactContainer = type.Assembly == Bcl && (type.IsArray + || definition == typeof(List<>) || definition == typeof(Dictionary<,>) + || definition == typeof(HashSet<>) || definition == typeof(KeyValuePair<,>)); + if (!exactContainer && typeof(IEnumerable).IsAssignableFrom(type)) { - text.Append(";"); - return; + // A behavioral collection: enumerating it would run foreign code now, or the + // enumerator could throw after capture. Refuse instead of reading it. + exact = false; + return Invalid; } + + var tracked = !type.IsValueType && path.Add(item); + if (!type.IsValueType && !tracked) + return $"cycle@{type.FullName}"; // Already rendered on this path. try { - text.Append('{'); - if (item is IEnumerable sequence) - { - foreach (var element in sequence) - Append(element, depth + 1); - } - else - { - foreach (var field in type.GetFields(BindingFlags.Public | BindingFlags.Instance) - .OrderBy(f => f.Name, StringComparer.Ordinal)) - { - text.Append(field.Name).Append(':'); - Append(field.GetValue(item), depth + 1); - } - if (!type.IsValueType) - foreach (var property in type.GetProperties(BindingFlags.Public | BindingFlags.Instance) - .Where(p => p.CanRead && p.GetIndexParameters().Length == 0) - .OrderBy(p => p.Name, StringComparer.Ordinal)) - { - text.Append(property.Name).Append(':'); - object read; - try - { - read = property.GetValue(item); - } - catch (TargetInvocationException exception) - { - text.Append("throws ").Append(exception.InnerException?.GetType().FullName).Append(';'); - continue; - } - Append(read, depth + 1); - } - } - text.Append('}'); + if (definition == typeof(KeyValuePair<,>)) + return $"{type.FullName}({Render(Property("Key"), depth + 1)}" + + $"=>{Render(Property("Value"), depth + 1)})"; + var entries = exactContainer + ? ((IEnumerable)item).Cast().Select(element => Render(element, depth + 1)).ToList() + : StateMembers(item, depth + 1); + if (!exact) + return Invalid; + if (definition == typeof(Dictionary<,>) || definition == typeof(HashSet<>)) + // Container order is insertion order, not authored semantics. + entries.Sort(StringComparer.Ordinal); + return exactContainer + ? $"{type.FullName}[{string.Join("|", entries)}]" + : $"{type.FullName}({string.Join(";", entries)})"; } finally { - if (!type.IsValueType) + if (tracked) path.Remove(item); } + + List StateMembers(object target, int childDepth) + { + var parts = new List(); + if (OpenNestAssemblies.Contains(type.Assembly)) + // Own code: public readable properties and fields are the authored state. + // A property and a field sharing a name must not collapse; order by both. + foreach (var member in type + .GetMembers(BindingFlags.Public | BindingFlags.Instance | BindingFlags.FlattenHierarchy) + .Where(m => m is FieldInfo || m is PropertyInfo property && property.CanRead + && property.GetIndexParameters().Length == 0) + .OrderBy(m => m.Name, StringComparer.Ordinal) + .ThenBy(m => m.MemberType)) + { + var value = member switch + { + PropertyInfo property => Read(() => property.GetValue(target)), + _ => Read(() => ((FieldInfo)member).GetValue(target)), + }; + if (!exact) + return parts; + parts.Add($"{member.Name}={Render(value, childDepth)}"); + } + else + // Foreign type: declared instance fields only, including auto-property + // backing fields. Reading a field executes no foreign code. + for (var walk = type; walk != null && walk != typeof(Delegate) && exact; walk = walk.BaseType) + foreach (var field in walk.GetFields(BindingFlags.Public | BindingFlags.NonPublic + | BindingFlags.Instance | BindingFlags.DeclaredOnly) + .OrderBy(f => f.Name, StringComparer.Ordinal)) + { + var value = Read(() => field.GetValue(target)); + if (!exact) + return parts; + parts.Add($"{walk.Name}.{field.Name}={Render(value, childDepth)}"); + } + return parts; + } + + object Property(string name) => type.GetProperty(name)!.GetValue(item); + + object Read(Func read) + { + try + { + return read(); + } + catch (Exception exception) when (exception is not OperationCanceledException) + { + exact = false; // A member that cannot be read makes the state non-exact. + return null; + } + } } } } diff --git a/OpenNest.Tests/CuttingPlanning/SettingsFreshnessTests.cs b/OpenNest.Tests/CuttingPlanning/SettingsFreshnessTests.cs new file mode 100644 index 0000000..3c29136 --- /dev/null +++ b/OpenNest.Tests/CuttingPlanning/SettingsFreshnessTests.cs @@ -0,0 +1,254 @@ +using System.Globalization; +using System.Reflection; +using OpenNest.CNC; +using OpenNest.CNC.CuttingPlanning; +using OpenNest.CNC.CuttingStrategy; +using OpenNest.Engine.CuttingPlanning; +using OpenNest.Geometry; + +namespace OpenNest.Tests.CuttingPlanning; + +/// +/// Delta-review regressions for settings freshness: capture must see accepted custom settings +/// state exactly, must execute no foreign getter or enumerator, must never throw out of a +/// commit, and must be culture-independent. Unsupported shapes refuse conservatively (Stale), +/// never silently compare equal. +/// +public class SettingsFreshnessTests +{ + [Fact] + public void Capture_DictionarySettingsEditInPlace_IsStale() + { + var settings = new DictionarySettings(); + settings.Values["leadLength"] = 0.3; + var (plate, _) = SinglePart(settings); + var result = CuttingPlanService.Plan(CuttingPlanRequest.ForPlate(plate)); + Assert.Equal(CuttingPlanStatus.Ready, result.Status); + settings.Values["leadLength"] = 9; + Assert.Equal(CuttingCommitStatus.Stale, CuttingPlanService.Apply([result]).Status); + } + + [Fact] + public void Capture_PropertyBackedStructSettingsEdit_IsStale() + { + var settings = new StructSettings { Extra = new ScalarSettings { Length = 0.3 } }; + var (plate, _) = SinglePart(settings); + var result = CuttingPlanService.Plan(CuttingPlanRequest.ForPlate(plate)); + Assert.Equal(CuttingPlanStatus.Ready, result.Status); + settings.Extra = new ScalarSettings { Length = 9 }; + Assert.Equal(CuttingCommitStatus.Stale, CuttingPlanService.Apply([result]).Status); + } + + [Fact] + public void Capture_DeepSettingsChain_IsFullyCompared() + { + var root = new Chain(); + var leaf = root; + for (var i = 0; i < 8; i++) + { + leaf.Next = new Chain(); + leaf = leaf.Next; + } + var settings = new DeepSettings { Extra = root }; + var (plate, _) = SinglePart(settings); + var result = CuttingPlanService.Plan(CuttingPlanRequest.ForPlate(plate)); + Assert.Equal(CuttingPlanStatus.Ready, result.Status); + Assert.Equal(CuttingCommitStatus.Applied, CuttingPlanService.Apply([result]).Status); + + var (plate2, _) = SinglePart(settings); + var second = CuttingPlanService.Plan(CuttingPlanRequest.ForPlate(plate2)); + Assert.Equal(CuttingPlanStatus.Ready, second.Status); + leaf.Length = 9; + Assert.Equal(CuttingCommitStatus.Stale, CuttingPlanService.Apply([second]).Status); + } + + [Fact] + public void Capture_UnsupportedSettingsDepth_RefusesConservatively() + { + var root = new Chain(); + var leaf = root; + for (var i = 0; i < 40; i++) + { + leaf.Next = new Chain(); + leaf = leaf.Next; + } + var settings = new DeepSettings { Extra = root }; + var (plate, _) = SinglePart(settings); + var result = CuttingPlanService.Plan(CuttingPlanRequest.ForPlate(plate)); + Assert.Equal(CuttingPlanStatus.Ready, result.Status); + // The deep leaf cannot be captured; an edit there must still never apply. + leaf.Length = 9; + Assert.Equal(CuttingCommitStatus.Stale, CuttingPlanService.Apply([result]).Status); + var (untouched, _) = SinglePart(new DeepSettings { Extra = new Chain() }); + var other = CuttingPlanService.Plan(CuttingPlanRequest.ForPlate(untouched)); + Assert.Equal(CuttingPlanStatus.Ready, other.Status); + } + + [Fact] + public void Capture_ReadsNoForeignGetterState() + { + var settings = new MutatingSettings(); + var (plate, _) = SinglePart(settings); + var result = CuttingPlanService.Plan(CuttingPlanRequest.ForPlate(plate)); + Assert.Equal(CuttingPlanStatus.Ready, result.Status); + Assert.Equal(0, settings.Kerf); // The nominally read-only capture never ran ++Kerf. + Assert.Equal(CuttingCommitStatus.Applied, CuttingPlanService.Apply([result]).Status); + } + + [Fact] + public void Apply_FailingSettingsEnumerable_IsStaleWithoutException() + { + var settings = new EnumerableSettings(); + var (plate, _) = SinglePart(settings); + var result = CuttingPlanService.Plan(CuttingPlanRequest.ForPlate(plate)); + Assert.Equal(CuttingPlanStatus.Ready, result.Status); + settings.EnableFailure(); + var commit = CuttingPlanService.Apply([result]); + Assert.Equal(CuttingCommitStatus.Stale, commit.Status); + } + + [Fact] + public void Commit_BuiltInSettingsAreCultureIndependent() + { + var before = CultureInfo.CurrentCulture; + try + { + CultureInfo.CurrentCulture = CultureInfo.InvariantCulture; + var settings = new CuttingParameters + { + ExternalLeadIn = new LineLeadIn { Length = 0.3, ApproachAngle = -90 } + }; + var (plate, _) = SinglePart(settings); + var result = CuttingPlanService.Plan(CuttingPlanRequest.ForPlate(plate)); + Assert.Equal(CuttingPlanStatus.Ready, result.Status); + CultureInfo.CurrentCulture = CultureInfo.GetCultureInfo("ar-SA"); + Assert.Equal(CuttingCommitStatus.Applied, CuttingPlanService.Apply([result]).Status); + } + finally + { + CultureInfo.CurrentCulture = before; + } + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void Commit_ConfirmedSettingsEdit_StalesOnlyWhenAlsoLiveState(bool aliasesLive) + { + var method = typeof(CuttingPlanCommitTests).GetMethod("RegeneratedPlate", + BindingFlags.Static | BindingFlags.NonPublic)!; + var (_, plate, _, live) = ((Nest, Plate, Part, CuttingParameters))method + .Invoke(null, [Vector.Zero])!; + var confirmed = aliasesLive ? live : OwnedCuttingParameters.Copy(live); + var result = CuttingPlanService.Plan(CuttingPlanRequest.ForPlate(plate, confirmedParameters: confirmed)); + Assert.Equal(CuttingPlanStatus.Ready, result.Status); + ((LineLeadIn)confirmed.ExternalLeadIn).Length = 9; + Assert.Equal(aliasesLive ? CuttingCommitStatus.Stale : CuttingCommitStatus.Applied, + CuttingPlanService.Apply([result]).Status); + } + + [Fact] + public void Fingerprint_BuiltInSettingsAreDeterministicAndDetectSignedZero() + { + LeadIn[] leads = [new NoLeadIn(), new LineLeadIn(), new ArcLeadIn(), new LineArcLeadIn(), + new LineLineLeadIn(), new CleanHoleLeadIn()]; + LeadOut[] outs = [new NoLeadOut(), new LineLeadOut(), new ArcLeadOut()]; + Tab[] tabs = [new NormalTab(), new BreakerTab(), new MachineTab()]; + foreach (var lead in leads) + foreach (var leadOut in outs) + foreach (var tab in tabs) + { + var settings = new CuttingParameters + { + ExternalLeadIn = lead, + InternalLeadIn = lead, + ArcCircleLeadIn = lead, + ExternalLeadOut = leadOut, + InternalLeadOut = leadOut, + ArcCircleLeadOut = leadOut, + TabConfig = tab, + TabsEnabled = true + }; + tab.TabLeadIn = lead; + tab.TabLeadOut = leadOut; + var one = StateFingerprint.Of(settings); + Assert.NotEqual(StateFingerprint.Invalid, one); + Assert.Equal(one, StateFingerprint.Of(settings)); + settings.Kerf = BitConverter.Int64BitsToDouble(unchecked((long)0x8000000000000000)); + Assert.NotEqual(one, StateFingerprint.Of(settings)); + } + } + + [Fact] + public void Fingerprint_CycleThroughLeadInIsDeterministic() + { + var tab = new NormalTab(); + var settings = new CuttingParameters { TabConfig = tab }; + tab.TabLeadIn = new LineLeadIn { Length = 1 }; + tab.TabLeadOut = new LineLeadOut { Length = 2 }; + var one = StateFingerprint.Of(settings); + Assert.NotEqual(StateFingerprint.Invalid, one); + Assert.Equal(one, StateFingerprint.Of(settings)); + ((LineLeadIn)tab.TabLeadIn).Length = 3; + Assert.NotEqual(one, StateFingerprint.Of(settings)); + } + + private static (Plate Plate, Part Part) SinglePart(CuttingParameters settings) + { + var rectangle = typeof(CuttingDependencyTests).GetMethod("Rectangle", + BindingFlags.Static | BindingFlags.NonPublic)!; + var part = (Part)rectangle.Invoke(null, [10.0, 10.0, 2.0, 2.0])!; + part.CuttingParameters = settings; + var plate = new Nest().CreatePlate(); + plate.Parts.Add(part); + return (plate, part); + } + + private sealed class DictionarySettings : CuttingParameters + { + public Dictionary Values { get; } = []; + } + + private struct ScalarSettings + { + public double Length { get; set; } + } + + private sealed class StructSettings : CuttingParameters + { + public ScalarSettings Extra { get; set; } + } + + private sealed class Chain + { + public Chain? Next { get; set; } + public double Length { get; set; } + } + + private sealed class DeepSettings : CuttingParameters + { + public Chain? Extra { get; set; } + } + + private sealed class MutatingSettings : CuttingParameters + { + public double Bump => ++Kerf; + } + + private sealed class EnumerableSettings : CuttingParameters + { + private bool fail; + + public void EnableFailure() => fail = true; + + public IEnumerable Values + { + get + { + yield return 1; + if (fail) + throw new IOException("changed enumerable"); + } + } + } +} diff --git a/docs/cutting-planner.md b/docs/cutting-planner.md index 6b1d3c3..d2bf3c8 100644 --- a/docs/cutting-planner.md +++ b/docs/cutting-planner.md @@ -164,7 +164,14 @@ difference on any plate returns `Stale` and changes nothing, so a proposal that changes a plate can be applied once; an unchanged (no-op) proposal stays current. A malformed live program is also `Stale`, not an exception. A part repeated on two plates of one scope is `InvalidInput`. Caller-confirmed planning settings are -input, not plate state: editing them after capture does not stale the result. +input, not plate state: editing a separate confirmed-settings object after +capture does not stale the result (confirmed settings that are also a part's or +the plate's live settings are live state, and editing them does). Settings state +whose exact capture would require executing foreign code — custom property +getters, behavioral enumerables, structures beyond the capture limits — is +recorded as refused at capture: the plate stays plannable, but every commit +against it reports `Stale`, and capture or commit never runs or enumerates +foreign code. The whole scope is validated and its bounds staged first; cancellation is checked immediately before the install. Order changes through `ObservableList.Reorder`