diff --git a/OpenNest.Engine.Tests/Jobs/NestPipelineCommitTests.cs b/OpenNest.Engine.Tests/Jobs/NestPipelineCommitTests.cs index 2cbbd5f..275f4ee 100644 --- a/OpenNest.Engine.Tests/Jobs/NestPipelineCommitTests.cs +++ b/OpenNest.Engine.Tests/Jobs/NestPipelineCommitTests.cs @@ -75,6 +75,37 @@ public class NestPipelineCommitTests Assert.Equal(2, Assert.Single(applied).Parts.Count); } + [Fact] + public void DrawingChangedAfterValidationIsRefusedBeforeCommitEvenWithConsent() + { + 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, string.Join("; ", result.Violations)); + drawing.Program = TestDrawingFactory.Rectangle(100, 100); + Assert.Throws(() => + NestPipelineCommit.ApplyToEmptyPlates(result, manager, allowInvalid: true)); + Assert.Same(empty, Assert.Single(nest.Plates)); + Assert.Empty(empty.Parts); + Assert.Equal(0, drawing.Quantity.Nested); + } + + private sealed class PreMutationStub : INestingEngine + { + public NestJobResult Solve(NestJob job, IProgress? progress = null, + CancellationToken token = default) + { + var builder = new NestJobResultBuilder(job); + builder.AddSheet(job.Plates[0], new[] { (job.Parts[0].Id, 1.0, 1.0, 0.0) }); + return builder.Build(NestJobStopReason.Completed); + } + } + [Fact] public void CancelledCommitDoesNotCreateAnyPlate() { diff --git a/OpenNest.Engine.Tests/Jobs/NestPipelineTests.cs b/OpenNest.Engine.Tests/Jobs/NestPipelineTests.cs index fab6faf..a4bc077 100644 --- a/OpenNest.Engine.Tests/Jobs/NestPipelineTests.cs +++ b/OpenNest.Engine.Tests/Jobs/NestPipelineTests.cs @@ -258,6 +258,20 @@ public class NestPipelineTests ); } + [Fact] + public void DrawingChangedDuringSolveCannotBindGeometryOtherThanTheValidatedSnapshot() + { + var item = Item("bracket", 1); + var result = NestPipeline.Run(new StubEngine(job => + { + item.Drawing.Program = TestDrawingFactory.Rectangle(100, 100); + return OnePlate(job, new NestJobPlacement(job.Parts[0].Id, 0, 1, 1, 0)); + }), "Mutator", Request("Mutator", item)); + Assert.False(result.CanKeep); + Assert.Empty(result.Plates); + Assert.Contains(result.Violations, v => v.Contains("changed") && v.Contains("bracket")); + } + [Fact] public void CancellationPropagatesWithoutAResult() { diff --git a/OpenNest.Engine/Jobs/NestPipeline.cs b/OpenNest.Engine/Jobs/NestPipeline.cs index 45507ff..8d9d0e5 100644 --- a/OpenNest.Engine/Jobs/NestPipeline.cs +++ b/OpenNest.Engine/Jobs/NestPipeline.cs @@ -37,7 +37,8 @@ public sealed class NestPipelineResult IReadOnlyList violations, bool canKeep, TimeSpan solveTime, - TimeSpan validationTime + TimeSpan validationTime, + IReadOnlyDictionary drawingsByPartId ) { EngineName = engineName; @@ -48,6 +49,7 @@ public sealed class NestPipelineResult CanKeep = canKeep; SolveTime = solveTime; ValidationTime = validationTime; + DrawingsByPartId = drawingsByPartId; } public string EngineName { get; } @@ -63,6 +65,7 @@ public sealed class NestPipelineResult public NestJobStopReason StopReason => Raw.StopReason; public TimeSpan SolveTime { get; } public TimeSpan ValidationTime { get; } + internal IReadOnlyDictionary DrawingsByPartId { get; } } /// @@ -191,6 +194,10 @@ public static class NestPipeline canKeep = false; } } + var freshness = NestPipelineDrawingFreshness.Changes(job, drawingsByPartId); + violations.AddRange(freshness); + if (freshness.Count > 0) + canKeep = false; var validationTime = clock.Elapsed; var plates = canKeep @@ -202,6 +209,19 @@ public static class NestPipeline .ToList() : new List(); + // Binding clones the caller's current Program; a concurrent edit during binding + // must not return parts whose bytes differ from the already validated snapshot. + if (canKeep) + { + var changes = NestPipelineDrawingFreshness.Changes(job, drawingsByPartId); + if (changes.Count > 0) + { + violations.AddRange(changes); + plates.Clear(); + canKeep = false; + } + } + token.ThrowIfCancellationRequested(); if (prepass != null && canKeep) { @@ -222,7 +242,8 @@ public static class NestPipeline violations, canKeep, solveTime, - validationTime + validationTime, + drawingsByPartId ); } } diff --git a/OpenNest.Engine/Jobs/NestPipelineCommit.cs b/OpenNest.Engine/Jobs/NestPipelineCommit.cs index cd59bb3..dea346a 100644 --- a/OpenNest.Engine/Jobs/NestPipelineCommit.cs +++ b/OpenNest.Engine/Jobs/NestPipelineCommit.cs @@ -20,6 +20,8 @@ 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."); 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 new file mode 100644 index 0000000..de8dfad --- /dev/null +++ b/OpenNest.Engine/Jobs/NestPipelineDrawingFreshness.cs @@ -0,0 +1,51 @@ +using System; +using System.Collections.Generic; +using System.Linq; + +namespace OpenNest.Engine.Jobs; + +/// Refuse a proposal when a caller drawing no longer matches its checked snapshot. +internal static class NestPipelineDrawingFreshness +{ + internal static IReadOnlyList Changes(NestJob job, + IReadOnlyDictionary drawings) + { + var changed = new List(); + foreach (var requirement in job.Parts) + { + if (!drawings.TryGetValue(requirement.Id, out var drawing) + || drawing?.Program == null) + { + changed.Add($"Drawing for '{requirement.Id}' changed after the nesting snapshot"); + continue; + } + try + { + var current = PartGeometrySnapshot.FromProgram(drawing.Program); + if (Same(requirement.Geometry, current)) + continue; + } + catch (Exception ex) when (ex is ArgumentException or NotSupportedException + or InvalidOperationException) + { + // A now-unsupported program is not the geometry that was checked. + } + changed.Add($"Drawing '{drawing.Name ?? requirement.Id}' changed after the nesting snapshot"); + } + 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 => + pair.First.Type == pair.Second.Type + && Bits(pair.First.X) == Bits(pair.Second.X) + && Bits(pair.First.Y) == Bits(pair.Second.Y) + && Bits(pair.First.CenterX) == Bits(pair.Second.CenterX) + && Bits(pair.First.CenterY) == Bits(pair.Second.CenterY) + && pair.First.Rotation == pair.Second.Rotation + && pair.First.Layer == pair.Second.Layer + && pair.First.Suppressed == pair.Second.Suppressed); + + private static long Bits(double value) => BitConverter.DoubleToInt64Bits(value); +}