From 1b9d8a36d8e3d4be961732baacdd377c9e0c318e Mon Sep 17 00:00:00 2001 From: AJ Isaacs Date: Fri, 9 Oct 2026 20:28:05 -0400 Subject: [PATCH] fix(pipeline): certify bound programs and cancellation before commit --- .../Jobs/NestPipelineCommitTests.cs | 36 ++++++++++++++ .../Jobs/NestPipelineTests.cs | 24 ++++++++++ OpenNest.Engine/Jobs/NestPipeline.cs | 3 +- OpenNest.Engine/Jobs/NestPipelineCommit.cs | 7 ++- .../Jobs/NestPipelineDrawingFreshness.cs | 48 +++++++++++++++++++ 5 files changed, 115 insertions(+), 3 deletions(-) diff --git a/OpenNest.Engine.Tests/Jobs/NestPipelineCommitTests.cs b/OpenNest.Engine.Tests/Jobs/NestPipelineCommitTests.cs index 275f4ee..071ef46 100644 --- a/OpenNest.Engine.Tests/Jobs/NestPipelineCommitTests.cs +++ b/OpenNest.Engine.Tests/Jobs/NestPipelineCommitTests.cs @@ -106,6 +106,42 @@ public class NestPipelineCommitTests } } + private sealed class CancelDuringSnapshot(double x, double y, CancellationTokenSource source) + : OpenNest.CNC.LinearMove(x, y) + { + public override OpenNest.CNC.CodeType Type + { + get + { + source.Cancel(); + return base.Type; + } + } + } + + [Fact] + public void CancellationDuringFreshnessScanDoesNotMutateNest() + { + var drawing = new Drawing("part", TestDrawingFactory.Rectangle()); + var nest = new Nest(); + nest.Drawings.Add(drawing); + var empty = nest.CreatePlate(); + using var manager = new PlateManager(nest); + var result = NestPipeline.Run(new PreMutationStub(), "stub", + new NestPipelineRequest("stub", new[] { new NestItem { Drawing = drawing, Quantity = 1 } }, + new[] { new NestPlateStock("sheet", new Size(48, 96), 1, 0.25) })); + Assert.True(result.IsValid); + using var cts = new CancellationTokenSource(); + var proposed = Assert.Single(Assert.Single(result.Plates).Parts); + proposed.Program.Codes[1] = new CancelDuringSnapshot(10, 0, cts); + Assert.Throws(() => + NestPipelineCommit.ApplyToEmptyPlates(result, manager, token: cts.Token)); + Assert.True(cts.IsCancellationRequested); + Assert.Same(empty, Assert.Single(nest.Plates)); + Assert.Empty(empty.Parts); + Assert.Equal(0, drawing.Quantity.Nested); + } + [Fact] public void CancelledCommitDoesNotCreateAnyPlate() { diff --git a/OpenNest.Engine.Tests/Jobs/NestPipelineTests.cs b/OpenNest.Engine.Tests/Jobs/NestPipelineTests.cs index a4bc077..b3ff92f 100644 --- a/OpenNest.Engine.Tests/Jobs/NestPipelineTests.cs +++ b/OpenNest.Engine.Tests/Jobs/NestPipelineTests.cs @@ -272,6 +272,30 @@ public class NestPipelineTests Assert.Contains(result.Violations, v => v.Contains("changed") && v.Contains("bracket")); } + private sealed class TransientCloneMove(double x, double y) : LinearMove(x, y) + { + public override ICode Clone() + { + var before = EndPoint; + EndPoint = new Vector(100, 100); + try { return base.Clone(); } + finally { EndPoint = before; } + } + } + + [Fact] + public void BindingTransientDrawingEditCannotReturnOrCommitUncheckedGeometry() + { + var item = Item("bracket", 1); + item.Drawing.Program.Codes[^1] = new TransientCloneMove(0, 0); + var result = NestPipeline.Run(new StubEngine(job => + OnePlate(job, new NestJobPlacement(job.Parts[0].Id, 0, 1, 1, 0))), + "Transient", Request("Transient", item)); + Assert.False(result.CanKeep); + Assert.Empty(result.Plates); + Assert.Contains(result.Violations, v => v.Contains("Bound part") && v.Contains("bracket")); + } + [Fact] public void CancellationPropagatesWithoutAResult() { diff --git a/OpenNest.Engine/Jobs/NestPipeline.cs b/OpenNest.Engine/Jobs/NestPipeline.cs index 8d9d0e5..7e33e5e 100644 --- a/OpenNest.Engine/Jobs/NestPipeline.cs +++ b/OpenNest.Engine/Jobs/NestPipeline.cs @@ -213,7 +213,8 @@ public static class NestPipeline // must not return parts whose bytes differ from the already validated snapshot. if (canKeep) { - var changes = NestPipelineDrawingFreshness.Changes(job, drawingsByPartId); + var changes = NestPipelineDrawingFreshness.Changes(job, drawingsByPartId).ToList(); + changes.AddRange(NestPipelineDrawingFreshness.BoundChanges(job, raw, plates, drawingsByPartId)); if (changes.Count > 0) { violations.AddRange(changes); diff --git a/OpenNest.Engine/Jobs/NestPipelineCommit.cs b/OpenNest.Engine/Jobs/NestPipelineCommit.cs index dea346a..cf46ac7 100644 --- a/OpenNest.Engine/Jobs/NestPipelineCommit.cs +++ b/OpenNest.Engine/Jobs/NestPipelineCommit.cs @@ -20,8 +20,11 @@ public static class NestPipelineCommit token.ThrowIfCancellationRequested(); if (!result.CanKeep || (!result.IsValid && !allowInvalid)) throw new InvalidOperationException("The nesting result cannot be committed without a keepable layout and explicit consent to its violations."); - if (NestPipelineDrawingFreshness.Changes(result.Job, result.DrawingsByPartId).Count > 0) - throw new InvalidOperationException("A drawing changed after the nesting proposal was validated; run Auto Nest again."); + if (NestPipelineDrawingFreshness.Changes(result.Job, result.DrawingsByPartId).Count > 0 + || NestPipelineDrawingFreshness.BoundChanges(result.Job, result.Raw, + result.Plates, result.DrawingsByPartId).Count > 0) + throw new InvalidOperationException("A drawing or proposed part changed after the nesting proposal was validated; run Auto Nest again."); + token.ThrowIfCancellationRequested(); var applied = new List(); // Commit is synchronous on the caller's owning thread. Cancellation is checked diff --git a/OpenNest.Engine/Jobs/NestPipelineDrawingFreshness.cs b/OpenNest.Engine/Jobs/NestPipelineDrawingFreshness.cs index de8dfad..9f81005 100644 --- a/OpenNest.Engine/Jobs/NestPipelineDrawingFreshness.cs +++ b/OpenNest.Engine/Jobs/NestPipelineDrawingFreshness.cs @@ -1,6 +1,8 @@ using System; using System.Collections.Generic; using System.Linq; +using OpenNest.Engine.Jobs.Adapters; +using OpenNest.Geometry; namespace OpenNest.Engine.Jobs; @@ -35,6 +37,52 @@ internal static class NestPipelineDrawingFreshness return changed; } + /// Check the actual cloned programs, not only the drawing before/after + /// binding. A transient edit during Clone may leave the drawing unchanged. + internal static IReadOnlyList BoundChanges(NestJob job, NestJobResult raw, + IReadOnlyList plates, IReadOnlyDictionary drawings) + { + var changed = new List(); + var requirements = job.Parts.ToDictionary(p => p.Id, StringComparer.Ordinal); + if (plates.Count != raw.Plates.Count) + return new[] { "Bound plate count changed after validation" }; + for (var sheetIndex = 0; sheetIndex < plates.Count; sheetIndex++) + { + var poses = raw.Plates[sheetIndex].Placements; + var bound = plates[sheetIndex].Parts; + if (bound.Count != poses.Count) + return new[] { "Bound part count changed after validation" }; + for (var index = 0; index < poses.Count; index++) + { + var pose = poses[index]; + var actual = bound[index]; + if (pose.PartId == null || !requirements.TryGetValue(pose.PartId, out var requirement) + || !drawings.TryGetValue(pose.PartId, out var drawing) + || !ReferenceEquals(actual.BaseDrawing, drawing)) + return new[] { "A bound part no longer matches its requirement" }; + try + { + var expected = new Part(DrawingJobMapper.CreateDrawing(requirement)); + expected.Rotate(pose.Rotation); + expected.Location = new Vector(pose.X, pose.Y); + if (Same(PartGeometrySnapshot.FromProgram(expected.Program), + PartGeometrySnapshot.FromProgram(actual.Program)) + && Bits(expected.Location.X) == Bits(actual.Location.X) + && Bits(expected.Location.Y) == Bits(actual.Location.Y) + && Bits(expected.Rotation) == Bits(actual.Rotation)) + continue; + } + catch (Exception ex) when (ex is ArgumentException or NotSupportedException + or InvalidOperationException) + { + // A changed/unsupported program cannot represent the checked pose. + } + changed.Add($"Bound part for '{drawing.Name ?? requirement.Id}' changed after validation"); + } + } + return changed; + } + private static bool Same(PartGeometrySnapshot a, PartGeometrySnapshot b) => a.Mode == b.Mode && a.Motions.Count == b.Motions.Count && a.Motions.Zip(b.Motions).All(pair =>