diff --git a/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/Flattening/FlatteningService.cs b/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/Flattening/FlatteningService.cs index 2be6a578..5a685113 100644 --- a/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/Flattening/FlatteningService.cs +++ b/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/Flattening/FlatteningService.cs @@ -262,7 +262,7 @@ public class FlatteningService IReadOnlyDictionary> compositionMap, IReadOnlyDictionary> composedTemplateChains, Dictionary attributes, - HashSet visited) + HashSet path) { if (!composedTemplateChains.TryGetValue(composition.ComposedTemplateId, out var composedChain)) return; @@ -283,17 +283,27 @@ public class FlatteningService } // Descend into nested compositions of every template in the chain. + // Guard on the recursion PATH, not globally: composition is by-slot, so a + // template legitimately appears twice under different prefixes; only a + // template already on the CURRENT path is a cycle. foreach (var composedTemplate in composedChain) { - if (!visited.Add(composedTemplate.Id)) - continue; - if (!compositionMap.TryGetValue(composedTemplate.Id, out var nestedCompositions)) + if (!path.Add(composedTemplate.Id)) continue; + try + { + if (!compositionMap.TryGetValue(composedTemplate.Id, out var nestedCompositions)) + continue; - foreach (var nested in nestedCompositions) - ResolveComposedAttributesRecursive( - nested, $"{prefix}.{nested.InstanceName}", - compositionMap, composedTemplateChains, attributes, visited); + foreach (var nested in nestedCompositions) + ResolveComposedAttributesRecursive( + nested, $"{prefix}.{nested.InstanceName}", + compositionMap, composedTemplateChains, attributes, path); + } + finally + { + path.Remove(composedTemplate.Id); + } } } @@ -625,7 +635,7 @@ public class FlatteningService IReadOnlyDictionary> composedTemplateChains, Dictionary alarms, Dictionary alarmScriptIds, - HashSet visited) + HashSet path) { if (!composedTemplateChains.TryGetValue(composition.ComposedTemplateId, out var composedChain)) return; @@ -646,17 +656,27 @@ public class FlatteningService } // Descend into nested compositions of every template in the chain. + // Guard on the recursion PATH, not globally: composition is by-slot, so a + // template legitimately appears twice under different prefixes; only a + // template already on the CURRENT path is a cycle. foreach (var composedTemplate in composedChain) { - if (!visited.Add(composedTemplate.Id)) - continue; - if (!compositionMap.TryGetValue(composedTemplate.Id, out var nestedCompositions)) + if (!path.Add(composedTemplate.Id)) continue; + try + { + if (!compositionMap.TryGetValue(composedTemplate.Id, out var nestedCompositions)) + continue; - foreach (var nested in nestedCompositions) - ResolveComposedAlarmsRecursive( - nested, $"{prefix}.{nested.InstanceName}", - compositionMap, composedTemplateChains, alarms, alarmScriptIds, visited); + foreach (var nested in nestedCompositions) + ResolveComposedAlarmsRecursive( + nested, $"{prefix}.{nested.InstanceName}", + compositionMap, composedTemplateChains, alarms, alarmScriptIds, path); + } + finally + { + path.Remove(composedTemplate.Id); + } } } @@ -739,7 +759,7 @@ public class FlatteningService IReadOnlyDictionary> compositionMap, IReadOnlyDictionary> composedTemplateChains, Dictionary sources, - HashSet visited) + HashSet path) { if (!composedTemplateChains.TryGetValue(composition.ComposedTemplateId, out var composedChain)) return; @@ -758,17 +778,27 @@ public class FlatteningService } } + // Guard on the recursion PATH, not globally: composition is by-slot, so a + // template legitimately appears twice under different prefixes; only a + // template already on the CURRENT path is a cycle. foreach (var composedTemplate in composedChain) { - if (!visited.Add(composedTemplate.Id)) - continue; - if (!compositionMap.TryGetValue(composedTemplate.Id, out var nestedCompositions)) + if (!path.Add(composedTemplate.Id)) continue; + try + { + if (!compositionMap.TryGetValue(composedTemplate.Id, out var nestedCompositions)) + continue; - foreach (var nested in nestedCompositions) - ResolveComposedNativeAlarmSourcesRecursive( - nested, $"{prefix}.{nested.InstanceName}", - compositionMap, composedTemplateChains, sources, visited); + foreach (var nested in nestedCompositions) + ResolveComposedNativeAlarmSourcesRecursive( + nested, $"{prefix}.{nested.InstanceName}", + compositionMap, composedTemplateChains, sources, path); + } + finally + { + path.Remove(composedTemplate.Id); + } } } @@ -889,7 +919,7 @@ public class FlatteningService IReadOnlyDictionary> composedTemplateChains, Dictionary scripts, Dictionary scriptCanonicalById, - HashSet visited) + HashSet path) { if (!composedTemplateChains.TryGetValue(composition.ComposedTemplateId, out var composedChain)) return; @@ -910,17 +940,27 @@ public class FlatteningService } // Descend into nested compositions of every template in the chain. + // Guard on the recursion PATH, not globally: composition is by-slot, so a + // template legitimately appears twice under different prefixes; only a + // template already on the CURRENT path is a cycle. foreach (var composedTemplate in composedChain) { - if (!visited.Add(composedTemplate.Id)) - continue; - if (!compositionMap.TryGetValue(composedTemplate.Id, out var nestedCompositions)) + if (!path.Add(composedTemplate.Id)) continue; + try + { + if (!compositionMap.TryGetValue(composedTemplate.Id, out var nestedCompositions)) + continue; - foreach (var nested in nestedCompositions) - ResolveComposedScriptsRecursive( - nested, $"{prefix}.{nested.InstanceName}", parentPath: prefix, - compositionMap, composedTemplateChains, scripts, scriptCanonicalById, visited); + foreach (var nested in nestedCompositions) + ResolveComposedScriptsRecursive( + nested, $"{prefix}.{nested.InstanceName}", parentPath: prefix, + compositionMap, composedTemplateChains, scripts, scriptCanonicalById, path); + } + finally + { + path.Remove(composedTemplate.Id); + } } } diff --git a/tests/ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests/Flattening/FlatteningServiceTests.cs b/tests/ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests/Flattening/FlatteningServiceTests.cs index 7f06d7cc..9eb2f31c 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests/Flattening/FlatteningServiceTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests/Flattening/FlatteningServiceTests.cs @@ -851,4 +851,103 @@ public class FlatteningServiceTests Assert.Equal("List", attr.DataType); // type unchanged Assert.Equal("String", attr.ElementDataType); // element type preserved } + + // ── Repeated composition: same template under multiple slots ────────── + + [Fact] + public void Flatten_SameTemplateComposedTwice_ResolvesNestedMembersUnderBothSlots() + { + // Root(1) composes X(2) as "x"; X composes Y(3) as "y1" AND "y2"; + // Y composes Z(4) as "z"; Z carries one member of each kind. + // Because y1 and y2 share one recursion subtree, a globally-keyed cycle + // guard would descend into Z under y1 but skip it under y2, dropping the + // whole x.y2.z.* subtree. The path-keyed guard resolves both. + var z = CreateTemplate(4, "Z"); + z.Attributes.Add(new TemplateAttribute("Val") { DataType = DataType.Double, Value = "1" }); + z.Alarms.Add(new TemplateAlarm("Alm") + { + TriggerType = AlarmTriggerType.RangeViolation, + TriggerConfiguration = "{\"attributeName\":\"Val\",\"high\":100}", + PriorityLevel = 1 + }); + z.Scripts.Add(new TemplateScript("Run", "// run") { Id = 40 }); + z.NativeAlarmSources.Add(new TemplateNativeAlarmSource("Src") + { ConnectionName = "Opc", SourceReference = "ns=2;s=Z" }); + + var y = CreateTemplate(3, "Y"); + var x = CreateTemplate(2, "X"); + var root = CreateTemplate(1, "Root"); + + var compositions = new Dictionary> + { + [1] = new List { new("x") { ComposedTemplateId = 2 } }, + [2] = new List + { + new("y1") { ComposedTemplateId = 3 }, + new("y2") { ComposedTemplateId = 3 } + }, + [3] = new List { new("z") { ComposedTemplateId = 4 } } + }; + + var composedChains = new Dictionary> + { + [2] = [x], + [3] = [y], + [4] = [z] + }; + + var result = _sut.Flatten( + CreateInstance(), + [root], + compositions, + composedChains, + new Dictionary()); + + Assert.True(result.IsSuccess); + + Assert.Contains(result.Value.Attributes, a => a.CanonicalName == "x.y1.z.Val"); + Assert.Contains(result.Value.Attributes, a => a.CanonicalName == "x.y2.z.Val"); + + Assert.Contains(result.Value.Alarms, a => a.CanonicalName == "x.y1.z.Alm"); + Assert.Contains(result.Value.Alarms, a => a.CanonicalName == "x.y2.z.Alm"); + + Assert.Contains(result.Value.Scripts, s => s.CanonicalName == "x.y1.z.Run"); + Assert.Contains(result.Value.Scripts, s => s.CanonicalName == "x.y2.z.Run"); + + Assert.Contains(result.Value.NativeAlarmSources, s => s.CanonicalName == "x.y1.z.Src"); + Assert.Contains(result.Value.NativeAlarmSources, s => s.CanonicalName == "x.y2.z.Src"); + } + + [Fact] + public void Flatten_CompositionCycle_StillTerminates() + { + // X(1) composes Y(2) as "y"; Y composes X(1) as "x" — a composition + // cycle. The path-keyed guard must still break the cycle (a template + // already on the CURRENT path) so flattening terminates. + var x = CreateTemplate(1, "X"); + x.Attributes.Add(new TemplateAttribute("Root") { DataType = DataType.String, Value = "r" }); + var y = CreateTemplate(2, "Y"); + + var compositions = new Dictionary> + { + [1] = new List { new("y") { ComposedTemplateId = 2 } }, + [2] = new List { new("x") { ComposedTemplateId = 1 } } + }; + + var composedChains = new Dictionary> + { + [1] = [x], + [2] = [y] + }; + + var result = _sut.Flatten( + CreateInstance(), + [x], + compositions, + composedChains, + new Dictionary()); + + Assert.True(result.IsSuccess); + Assert.Contains(result.Value.Attributes, a => a.CanonicalName == "Root"); + } }