diff --git a/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/CollisionDetector.cs b/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/CollisionDetector.cs index ed8001e7..e4a145e1 100644 --- a/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/CollisionDetector.cs +++ b/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/CollisionDetector.cs @@ -123,24 +123,37 @@ public static class CollisionDetector List members, HashSet visited) { + // `visited` is the current RECURSION PATH, not a global visited-set: an id is + // added on entry and REMOVED on exit (finally), so a template legitimately + // composed into more than one slot is re-descended for each slot. The early + // return fires only on a genuine cycle — an id already on the ACTIVE path. + // (A global visited-set would blind the detector to every collision reachable + // only through the second-and-later slot of a repeatedly-composed template.) if (!visited.Add(template.Id)) return; - // Add direct members of this composed template with the prefix. Inherited - // placeholder rows are KEPT here: the composed-module walk does not climb the - // composed template's parent chain, so the placeholders are the only - // representation of a derived module's inherited members. - CollectDirectMembers(template, prefix, $"module '{prefix}'", members, skipInherited: false); - - // Recurse into nested compositions - foreach (var composition in template.Compositions) + try { - if (lookup.TryGetValue(composition.ComposedTemplateId, out var nested)) + // Add direct members of this composed template with the prefix. Inherited + // placeholder rows are KEPT here: the composed-module walk does not climb the + // composed template's parent chain, so the placeholders are the only + // representation of a derived module's inherited members. + CollectDirectMembers(template, prefix, $"module '{prefix}'", members, skipInherited: false); + + // Recurse into nested compositions + foreach (var composition in template.Compositions) { - var nestedPrefix = $"{prefix}.{composition.InstanceName}"; - CollectComposedMembers(nested, nestedPrefix, lookup, members, visited); + if (lookup.TryGetValue(composition.ComposedTemplateId, out var nested)) + { + var nestedPrefix = $"{prefix}.{composition.InstanceName}"; + CollectComposedMembers(nested, nestedPrefix, lookup, members, visited); + } } } + finally + { + visited.Remove(template.Id); + } } private static void CollectInheritedMembers( diff --git a/tests/ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests/CollisionDetectorTests.cs b/tests/ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests/CollisionDetectorTests.cs index ccfc5727..5f556e05 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests/CollisionDetectorTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests/CollisionDetectorTests.cs @@ -188,4 +188,64 @@ public class CollisionDetectorTests Assert.NotEmpty(collisions); Assert.Contains(collisions, c => c.Contains("dup.Shared")); } + + // ======================================================================== + // Task 10: repeated composition blindness. A template composed into TWO + // different slots was added to the global visited-set on the first slot, so + // the SECOND slot's subtree was never collected — the detector went blind to + // any collision reachable only through the second slot. The visited-set must + // be a recursion PATH set (removed on the way back out), not a global set, so + // siblings/other slots can re-descend the same template. + // ======================================================================== + + [Fact] + public void DetectCollisions_TemplateComposedTwiceWithinSubtree_SeesCollisionUnderSecondSlot() + { + // Z carries the leaf member "Val". + var z = new Template("Z") { Id = 4 }; + z.Attributes.Add(new TemplateAttribute("Val") { Id = 40, TemplateId = 4, DataType = DataType.Float }); + + // Y composes Z as "z". + var y = new Template("Y") { Id = 3 }; + y.Compositions.Add(new TemplateComposition("z") { Id = 1, TemplateId = 3, ComposedTemplateId = 4 }); + + // W composes Y into TWO slots (y1, y2). Because both slots are visited within + // the SAME CollectComposedMembers walk, they share one visited set — the + // second slot's subtree was previously never collected. + var w = new Template("W") { Id = 2 }; + w.Compositions.Add(new TemplateComposition("y1") { Id = 2, TemplateId = 2, ComposedTemplateId = 3 }); + w.Compositions.Add(new TemplateComposition("y2") { Id = 3, TemplateId = 2, ComposedTemplateId = 3 }); + + // X composes W as "w" AND declares a direct attribute whose path collides with + // a member reachable only via the SECOND slot: "w.y2.z.Val". + var x = new Template("X") { Id = 1 }; + x.Attributes.Add(new TemplateAttribute("w.y2.z.Val") { Id = 10, TemplateId = 1, DataType = DataType.Float }); + x.Compositions.Add(new TemplateComposition("w") { Id = 4, TemplateId = 1, ComposedTemplateId = 2 }); + + var all = new List