diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceApplier.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceApplier.cs index d91a2047..06948558 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceApplier.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceApplier.cs @@ -828,35 +828,36 @@ public sealed class AddressSpaceApplier /// parented under its equipment folder (the logical node /// already created), THEN an Organizes reference /// UNS→Raw to its backing raw node. The UNS node inherits DataType / writable / array shape / historian - /// tagname from its backing RAW tag (looked up by ), so a - /// historized raw tag's UNS reference registers the SAME tagname (both NodeIds → one tagname → HistoryRead - /// works against either). The UNS node binds NO driver — the driver publish path fans values to it (WP3 - /// runtime binding). Every sink call passes / the Organizes edge - /// passes both realms EXPLICITLY. Idempotent. + /// tagname from its backing RAW tag, so a historized raw tag's UNS reference registers the SAME tagname + /// (both NodeIds → one tagname → HistoryRead works against either). The UNS node binds NO driver — the + /// driver publish path fans values to it (WP3 runtime binding). Every sink call passes + /// / the Organizes edge passes both realms EXPLICITLY. Idempotent. + /// The inherited values are read from the RECORD, not + /// re-looked-up from by RawPath. The record is the + /// single truth: the composer copies them off the backing tag's TagConfigIntent at compose time + /// (both compose seams, byte-parity), and having them ON the record is what makes a raw-side historian / + /// shape edit produce the coincident UnsReferenceDelta that drives this node's repair + /// (remove-previous in Apply, recreate here). A back-lookup would re-introduce the split the + /// 2026-08-15 review found: a record that says one thing and a materialised node built from another. /// - /// The composition carrying the UNS reference variables + Raw tags (for inherited shape). + /// The composition carrying the UNS reference variables. /// The count of swallowed sink failures in this pass. public int MaterialiseUnsReferences(AddressSpaceComposition composition) { ArgumentNullException.ThrowIfNull(composition); if (composition.UnsReferenceVariables.Count == 0) return 0; - // Backing raw tag shape/historian by RawPath — a UNS reference inherits these so its node matches the - // raw node it mirrors (a historized raw tag registers the SAME tagname on the UNS node for HistoryRead). - var rawByPath = new Dictionary(StringComparer.Ordinal); - foreach (var t in composition.RawTags) rawByPath[t.NodeId] = t; - var failed = 0; foreach (var v in composition.UnsReferenceVariables) { - var backing = rawByPath.GetValueOrDefault(v.BackingRawPath); - var isArray = backing?.IsArray ?? false; - uint? arrayLength = backing?.ArrayLength; + var isArray = v.BackingIsArray; + uint? arrayLength = v.BackingArrayLength; // A UNS reference to a historized raw tag registers the SAME effective historian tagname (override - // else the backing RawPath) so HistoryRead resolves against the UNS NodeId too. The mux ref stays - // single (keyed by RawPath) — FeedHistorizedRefs emits raw refs only. - string? historianTagname = backing is { IsHistorized: true } - ? (string.IsNullOrWhiteSpace(backing.HistorianTagname) ? backing.NodeId : backing.HistorianTagname) + // else the backing RawPath — BackingRawPath IS the backing raw tag's NodeId) so HistoryRead resolves + // against the UNS NodeId too. The mux ref stays single (keyed by RawPath) — FeedHistorizedRefs emits + // raw refs only. + string? historianTagname = v.BackingIsHistorized + ? (string.IsNullOrWhiteSpace(v.BackingHistorianTagname) ? v.BackingRawPath : v.BackingHistorianTagname) : null; var writable = v.Writable && !isArray; if (!SafeEnsureVariable(v.NodeId, v.EquipmentId, v.EffectiveName, v.DataType, writable, AddressSpaceRealm.Uns, historianTagname, isArray, arrayLength)) diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceChangeClassifier.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceChangeClassifier.cs index c554b629..24ebcb4f 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceChangeClassifier.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceChangeClassifier.cs @@ -74,6 +74,19 @@ public static class AddressSpaceChangeClassifier // • ChangedUnsReferenceVariables ⇒ surgical remove-previous + recreate-current, applier-owned (a UNS // reference variable is a mirror node binding no driver, so it carries no notifier wiring of its // own — the alarm fan-out it participates in is re-derived from the raw tag's referencing set). + // No rebuild is forced for a UNS-reference delta in EITHER direction, and that is load-bearing for + // the repair path below. + // UNS-REFERENCE STALENESS, and why nothing here gates it (2026-08-15 review). Making non-alarm raw-tag + // deltas surgical meant a raw-side IsHistorized / HistorianTagname / IsArray / ArrayLength edit updated + // the raw node in place while every referencing UNS MIRROR node kept the old historian tagname + shape + // (EnsureVariable early-returns on an existing node) — wrong history series through the UNS NodeId, and + // a scalar→array flip left the mirror scalar AND writable. The fix is in the RECORD, not the classifier: + // UnsReferenceVariable now carries the backing tag's Backing{IsHistorized,HistorianTagname,IsArray, + // ArrayLength}, so the same edit makes the reference record unequal and the planner emits a COINCIDENT + // UnsReferenceDelta alongside the RawTagDelta. Both are non-rebuild kinds, so the plan stays + // AttributeOnly and the applier repairs the mirror surgically (remove-previous + materialise-recreate). + // Adding a new inherited field to UnsReferenceVariable therefore needs no change here — but OMITTING one + // silently reopens the staleness. // Anything a future surgical raw-tag path lets through that DOES touch alarm notifiers must still // re-wire/unwire them (WireAlarmNotifiers is a reconcile — pass the tag's COMPLETE current referencing // set — plus an explicit unwire on removal), else a de-referenced equipment keeps receiving the events. diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceComposer.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceComposer.cs index c770e863..90f9e812 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceComposer.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceComposer.cs @@ -137,6 +137,17 @@ public sealed record RawContainerNode( /// referencing equipment. is always . /// A null means the historian tagname defaults to the RawPath (resolved /// later, not here). +/// ADDING A FIELD IS A FOUR-PLACE CONTRACT — the hand-written equality below does NOT close +/// over new members. Because forces a custom +/// Equals/GetHashCode, the compiler's positional equality is suppressed, so a field added +/// to the ctor and nowhere else silently never diffs (a redeploy that changes only it produces an EMPTY +/// plan). Update, in order: (1) the primary constructor, (2) , +/// (3) . Then reconsider two downstream consumers: (4a) +/// AddressSpaceChangeClassifier.RawTagDeltaIsSurgicalEligible's with-equality whitelist — +/// a new field is default-closed (⇒ Rebuild) until it is listed, which is the safe direction but may be +/// the wrong one; and (4b) 's Backing* mirror fields — if the +/// new field is INHERITED by a UNS reference node at materialise time, it must be mirrored there too or +/// referencing UNS nodes go stale on a surgical raw edit (the exact 2026-08-15 review finding). /// public sealed record RawTagPlan( string TagId, @@ -200,6 +211,27 @@ public sealed record RawTagPlan( /// edit keeps the same id while the NodeId/effective-name changes; a backing-tag rename keeps the same id /// + NodeId while re-points). is always /// . +/// / / +/// / mirror the backing raw tag's +/// historian intent + array shape, and they are on THIS record for one reason: to make a raw-side +/// edit produce a COINCIDENT UnsReferenceDelta. The 2026-08-15 surgical-deploys review +/// found that once non-alarm raw-tag deltas became surgical, editing a backing tag's +/// IsHistorized / HistorianTagname / IsArray / ArrayLength left every +/// referencing UNS node STALE: the applier updates the raw node in place, no UNS-reference delta +/// fired (none of those fields were on this record), and EnsureVariable early-returns on an +/// existing node — so HistoryRead via the UNS NodeId served a different series than via the raw +/// NodeId, a de-historized tag kept serving history through UNS, and a scalar→array flip left the +/// UNS mirror scalar AND writable (bypassing the array read-only write gate). Carrying the four +/// fields here makes record equality flag the reference as Changed in the same plan, and the +/// existing machinery repairs it: the applier removes the previous UNS node and +/// recreates it from these values. +/// / were already inherited onto the record and are +/// covered by the same mechanism. +/// Equality is the COMPILER-GENERATED positional-record equality: every member is a +/// value/string type (no IReadOnlyList member, unlike ), so the four +/// new fields are compared by value with no hand-written Equals/GetHashCode needed. If +/// a collection-typed member is ever added here, custom equality becomes mandatory — see the +/// three-place contract comment on . /// public sealed record UnsReferenceVariable( string UnsTagReferenceId, @@ -209,6 +241,10 @@ public sealed record UnsReferenceVariable( string BackingRawPath, string DataType, bool Writable, + bool BackingIsHistorized = false, + string? BackingHistorianTagname = null, + bool BackingIsArray = false, + uint? BackingArrayLength = null, AddressSpaceRealm Realm = AddressSpaceRealm.Uns); public sealed record UnsAreaProjection(string UnsAreaId, string DisplayName); @@ -799,6 +835,12 @@ public static class AddressSpaceComposer continue; // invalid effective-name segment — dropped (deploy gate rejects invalid names) } + // The backing tag's historian intent + array shape ride the record so a raw-side edit to any of + // them produces a coincident UnsReferenceDelta that repairs this mirror node (see the record's + // doc-comment; 2026-08-15 surgical-deploys review). Parsed from the SAME TagConfigIntent the raw + // tag plan uses, so the two sides can never disagree. + var intent = TagConfigIntent.Parse(tag.TagConfig); + vars.Add(new UnsReferenceVariable( UnsTagReferenceId: r.UnsTagReferenceId, EquipmentId: r.EquipmentId, @@ -806,7 +848,11 @@ public static class AddressSpaceComposer EffectiveName: effectiveName, BackingRawPath: rawPath, DataType: tag.DataType, - Writable: tag.AccessLevel == TagAccessLevel.ReadWrite)); + Writable: tag.AccessLevel == TagAccessLevel.ReadWrite, + BackingIsHistorized: intent.IsHistorized, + BackingHistorianTagname: intent.HistorianTagname, + BackingIsArray: intent.IsArray, + BackingArrayLength: intent.ArrayLength)); } vars.Sort((x, y) => string.CompareOrdinal(x.NodeId, y.NodeId)); diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierRawUnsTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierRawUnsTests.cs index 2f4715a3..150e864c 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierRawUnsTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierRawUnsTests.cs @@ -253,7 +253,8 @@ public sealed class AddressSpaceApplierRawUnsTests UnsReferenceVariables = new[] { new UnsReferenceVariable("r1", "EQ-1", "filling/line1/station1/speed", "speed", - "Plant/A/dev/speed", "Float", Writable: true), + "Plant/A/dev/speed", "Float", Writable: true, + BackingIsHistorized: true, BackingHistorianTagname: "WW.Speed"), }, }; @@ -276,11 +277,79 @@ public sealed class AddressSpaceApplierRawUnsTests r.ReferenceType.ShouldBe("Organizes"); } + /// + /// 2026-08-15 review — the inherited historian tagname + array shape come from the + /// RECORD, never from a RawTags back-lookup by RawPath. The + /// fixture makes the two disagree DELIBERATELY (the raw tag says de-historized + scalar; the reference + /// record says historized "WW.Record" + array-of-4) so the assertion can only pass on the record. That + /// matters because the record is what the planner diffed to produce the repair delta: materialising + /// from anything else would recreate the node with values nobody diffed. + /// + [Fact] + public void MaterialiseUnsReferences_inherits_shape_and_historian_from_the_record_not_the_raw_tag() + { + var sink = new RealmRecordingSink(); + var composition = new AddressSpaceComposition( + Array.Empty(), Array.Empty(), Array.Empty()) + { + RawTags = new[] + { + new RawTagPlan("t1", "Plant/A/dev/speed", "Plant/A/dev", "drv", "speed", "Float", + Writable: true, Alarm: null, ReferencingEquipmentPaths: new[] { "filling/line1/station1" }, + IsHistorized: false, HistorianTagname: "WW.RawTag", IsArray: false, ArrayLength: null), + }, + UnsReferenceVariables = new[] + { + new UnsReferenceVariable("r1", "EQ-1", "filling/line1/station1/speed", "speed", + "Plant/A/dev/speed", "Float", Writable: true, + BackingIsHistorized: true, BackingHistorianTagname: "WW.Record", + BackingIsArray: true, BackingArrayLength: 4), + }, + }; + + NewApplier(sink).MaterialiseUnsReferences(composition); + + var v = sink.Variables.ShouldHaveSingleItem(); + v.HistorianTagname.ShouldBe("WW.Record"); // the record wins; the raw tag says not historized at all + v.IsArray.ShouldBeTrue(); + v.ArrayLength.ShouldBe(4u); + v.Writable.ShouldBeFalse(); // Writable && !IsArray ⇒ an array mirror is read-only + } + + /// The historian-resolution semantics carried over verbatim from the old back-lookup: a BLANK + /// BackingHistorianTagname on a historized reference falls back to the BackingRawPath (which + /// IS the backing raw tag's NodeId, the raw side's own default), and a non-historized reference registers + /// no tagname at all even when an override string is present. + [Theory] + [InlineData(true, null, "Plant/A/dev/speed")] + [InlineData(true, " ", "Plant/A/dev/speed")] + [InlineData(true, "WW.Override", "WW.Override")] + [InlineData(false, "WW.Override", null)] + public void MaterialiseUnsReferences_resolves_the_historian_tagname_like_the_raw_side( + bool historized, string? tagname, string? expected) + { + var sink = new RealmRecordingSink(); + var composition = new AddressSpaceComposition( + Array.Empty(), Array.Empty(), Array.Empty()) + { + UnsReferenceVariables = new[] + { + new UnsReferenceVariable("r1", "EQ-1", "filling/line1/station1/speed", "speed", + "Plant/A/dev/speed", "Float", Writable: true, + BackingIsHistorized: historized, BackingHistorianTagname: tagname), + }, + }; + + NewApplier(sink).MaterialiseUnsReferences(composition); + + sink.Variables.ShouldHaveSingleItem().HistorianTagname.ShouldBe(expected); + } + /// A capturing sink that records (nodeId, realm) for every materialise + the Organizes edges. private sealed class RealmRecordingSink : IOpcUaAddressSpaceSink { public List<(string NodeId, string? Parent, AddressSpaceRealm Realm)> Folders { get; } = new(); - public List<(string NodeId, string? Parent, bool Writable, string? HistorianTagname, AddressSpaceRealm Realm)> Variables { get; } = new(); + public List<(string NodeId, string? Parent, bool Writable, string? HistorianTagname, AddressSpaceRealm Realm, bool IsArray, uint? ArrayLength)> Variables { get; } = new(); public List<(string NodeId, string? Parent, bool IsNative, AddressSpaceRealm Realm)> Conditions { get; } = new(); public List<(string Source, AddressSpaceRealm SourceRealm, string Target, AddressSpaceRealm TargetRealm, string ReferenceType)> References { get; } = new(); public List<(string AlarmNodeId, AddressSpaceRealm AlarmRealm, IReadOnlyList FolderNodeIds, AddressSpaceRealm FolderRealm)> NotifierWirings { get; } = new(); @@ -289,7 +358,7 @@ public sealed class AddressSpaceApplierRawUnsTests => Folders.Add((folderNodeId, parentNodeId, realm)); public void EnsureVariable(string variableNodeId, string? parentFolderNodeId, string displayName, string dataType, bool writable, AddressSpaceRealm realm, string? historianTagname = null, bool isArray = false, uint? arrayLength = null) - => Variables.Add((variableNodeId, parentFolderNodeId, writable, historianTagname, realm)); + => Variables.Add((variableNodeId, parentFolderNodeId, writable, historianTagname, realm, isArray, arrayLength)); public void MaterialiseAlarmCondition(string alarmNodeId, string equipmentNodeId, string displayName, string alarmType, int severity, AddressSpaceRealm realm, bool isNative = false) => Conditions.Add((alarmNodeId, equipmentNodeId, isNative, realm)); diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierTests.cs index b9535fa5..ad370d03 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierTests.cs @@ -1553,6 +1553,81 @@ public sealed class AddressSpaceApplierTests outcome.ChangedNodes.ShouldBe(1); } + /// + /// 2026-08-15 review — the combined plan a backing-tag HISTORIAN edit produces: a surgical raw-tag + /// delta AND the coincident UNS-reference delta (the reference record carries the backing historian + /// intent, so it goes unequal too). Both are non-rebuild kinds, so ONE apply must update the raw node + /// in place AND tear the stale UNS mirror down for the materialise pass to recreate — with NO rebuild. + /// Without the coincident delta the UNS mirror kept the OLD historian tagname and HistoryRead through + /// the UNS NodeId served a different series than through the raw NodeId. + /// + [Fact] + public void Backing_historian_edit_updates_raw_in_place_and_removes_the_stale_uns_mirror_without_rebuilding() + { + var sink = new RecordingSink(); + var applier = new AddressSpaceApplier(sink, NullLogger.Instance); + + var rawBefore = RawTag("Plant/Modbus/dev1/speed") with { IsHistorized = true, HistorianTagname = "WW.Old" }; + var rawAfter = rawBefore with { HistorianTagname = "WW.New" }; + var unsBefore = UnsRef("filling/line-1/station-1/speed", "speed") with + { + BackingIsHistorized = true, BackingHistorianTagname = "WW.Old", + }; + var unsAfter = unsBefore with { BackingHistorianTagname = "WW.New" }; + + var previous = CompositionWith(rawBefore, unsBefore); + var current = CompositionWith(rawAfter, unsAfter); + var plan = AddressSpacePlanner.Compute(previous, current); + + // The plan shape the fix produces: one delta in EACH realm, nothing structural. + plan.ChangedRawTags.Count.ShouldBe(1); + plan.ChangedUnsReferenceVariables.Count.ShouldBe(1); + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.AttributeOnly); + + var outcome = applier.Apply(plan); + + outcome.RebuildCalled.ShouldBeFalse(); + sink.RebuildCalls.ShouldBe(0); + + // Raw node: updated in place with the NEW tagname, in the Raw realm. + var call = sink.SurgicalCalls.ShouldHaveSingleItem(); + call.NodeId.ShouldBe("Plant/Modbus/dev1/speed"); + call.Historian.ShouldBe("WW.New"); + call.Realm.ShouldBe(AddressSpaceRealm.Raw); + + // UNS mirror: terminal Bad + in-place remove of the PREVIOUS node (the recreate rides the materialise + // pass after Apply returns, which reads the CURRENT record's Backing* values). + sink.ValueWrites.ShouldContain((unsBefore.NodeId, OpcUaStatus.Bad, AddressSpaceRealm.Uns)); + sink.RemoveCalls.ShouldHaveSingleItem().ShouldBe(("var", unsBefore.NodeId, AddressSpaceRealm.Uns)); + } + + /// The same combined-plan repair for a scalar→ARRAY flip on the backing tag: without it the UNS + /// mirror stays scalar AND writable, bypassing the array read-only write gate through the UNS NodeId. + [Fact] + public void Backing_shape_flip_removes_the_stale_uns_mirror_without_rebuilding() + { + var sink = new RecordingSink(); + var applier = new AddressSpaceApplier(sink, NullLogger.Instance); + + var rawBefore = RawTag("Plant/Modbus/dev1/block"); + var rawAfter = rawBefore with { IsArray = true, ArrayLength = 8 }; + var unsBefore = UnsRef("filling/line-1/station-1/block", "block"); + var unsAfter = unsBefore with { BackingIsArray = true, BackingArrayLength = 8 }; + + var plan = AddressSpacePlanner.Compute( + CompositionWith(rawBefore, unsBefore), CompositionWith(rawAfter, unsAfter)); + + plan.ChangedRawTags.Count.ShouldBe(1); + plan.ChangedUnsReferenceVariables.Count.ShouldBe(1); + + var outcome = applier.Apply(plan); + + outcome.RebuildCalled.ShouldBeFalse(); + sink.RebuildCalls.ShouldBe(0); + sink.SurgicalCalls.ShouldHaveSingleItem().IsArray.ShouldBeTrue(); + sink.RemoveCalls.ShouldHaveSingleItem().ShouldBe(("var", unsBefore.NodeId, AddressSpaceRealm.Uns)); + } + /// The one-way rebuild ratchet holds for the new raw-tag path: a surgical update that reports the /// node missing (false) falls back to a full rebuild. [Fact] @@ -1625,6 +1700,16 @@ public sealed class AddressSpaceApplierTests RawTags = tags, }; + // Both realms in one composition — the shape a backing-tag edit produces (a raw delta AND its coincident + // UNS-reference delta) needs the raw tag and the reference to it side by side. + private static AddressSpaceComposition CompositionWith(RawTagPlan tag, UnsReferenceVariable reference) => + new( + Array.Empty(), Array.Empty(), Array.Empty()) + { + RawTags = new[] { tag }, + UnsReferenceVariables = new[] { reference }, + }; + private static AddressSpaceComposition CompositionWithUnsRefs(params UnsReferenceVariable[] refs) => new( Array.Empty(), Array.Empty(), Array.Empty()) diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceChangeClassifierDualNamespaceTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceChangeClassifierDualNamespaceTests.cs index 779c572b..0f388b92 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceChangeClassifierDualNamespaceTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceChangeClassifierDualNamespaceTests.cs @@ -104,9 +104,13 @@ public sealed class AddressSpaceChangeClassifierDualNamespaceTests [InlineData("historianTagname")] [InlineData("dataType")] [InlineData("array")] + [InlineData("arrayLength")] public void Attribute_raw_tag_delta_is_not_a_rebuild(string field) { - var prev = RawTag("Plant/Modbus1/PLC-A/Speed"); + var baseline = RawTag("Plant/Modbus1/PLC-A/Speed"); + // ArrayLength moving ALONE (a resize of an already-array tag) — the whitelist must carry ArrayLength + // in its own right, not only as a passenger of an IsArray flip. Its "previous" is already an array. + var prev = field == "arrayLength" ? baseline with { IsArray = true, ArrayLength = 8 } : baseline; var cur = field switch { "writable" => prev with { Writable = true }, @@ -114,6 +118,7 @@ public sealed class AddressSpaceChangeClassifierDualNamespaceTests "historianTagname" => prev with { IsHistorized = true, HistorianTagname = "Plant.PLC_A.Speed" }, "dataType" => prev with { DataType = "Double" }, "array" => prev with { IsArray = true, ArrayLength = 8 }, + "arrayLength" => prev with { ArrayLength = 16 }, _ => throw new ArgumentOutOfRangeException(nameof(field)), }; @@ -141,6 +146,42 @@ public sealed class AddressSpaceChangeClassifierDualNamespaceTests AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.Rebuild); } + /// WP4 binding guard, CONTENT edition: an alarm present on BOTH sides whose severity (or type) + /// moved still rebuilds. The null-check pair alone does not decide it — the with-equality whitelist has to + /// leave Alarm out, so the two values compare unequal. + [Fact] + public void Alarm_content_only_raw_tag_delta_still_rebuilds() + { + var prev = RawTag("Plant/Modbus1/PLC-A/Speed") with { Alarm = new EquipmentTagAlarmInfo("OffNormalAlarm", 500) }; + var cur = prev with { Alarm = new EquipmentTagAlarmInfo("OffNormalAlarm", 900) }; + + var plan = Empty() with { ChangedRawTags = new[] { new AddressSpacePlan.RawTagDelta(prev, cur) } }; + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.Rebuild); + } + + /// The QUANTIFIER: the rebuild gate is .Any(d => !eligible(d)), so ONE ineligible raw-tag + /// delta must rebuild even when every other delta in the same plan is surgical. Pins against a regression + /// to .All(eligible) on the wrong side, or to inspecting only First() — either of which would + /// apply an identity/alarm change as an in-place attribute update and leave the node mis-wired. + [Fact] + public void One_ineligible_raw_tag_delta_among_eligible_ones_still_rebuilds() + { + var eligiblePrev = RawTag("Plant/Modbus1/PLC-A/Speed"); + var ineligiblePrev = RawTag("Plant/Modbus1/PLC-A/Temp") with { TagId = "t-2" }; + + var plan = Empty() with + { + ChangedRawTags = new[] + { + // Surgical: a plain historize toggle. Listed FIRST so a First()-only check would pass it. + new AddressSpacePlan.RawTagDelta(eligiblePrev, eligiblePrev with { IsHistorized = true }), + // Ineligible: DriverInstanceId re-routes the node to a different driver point. + new AddressSpacePlan.RawTagDelta(ineligiblePrev, ineligiblePrev with { DriverInstanceId = "drv-2" }), + }, + }; + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.Rebuild); + } + /// Default-closed: an identity field outside the whitelist (DriverInstanceId re-routes the node to /// another driver point; Name/TagId/ParentNodeId re-shape identity) still forces a full rebuild. [Theory] diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceComposerDualNamespaceTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceComposerDualNamespaceTests.cs index c22cdf79..f1e51249 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceComposerDualNamespaceTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceComposerDualNamespaceTests.cs @@ -178,6 +178,70 @@ public sealed class AddressSpaceComposerDualNamespaceTests statusRef.Writable.ShouldBeFalse(); // inherited from the Read raw tag } + /// + /// 2026-08-15 surgical-deploys review — a UNS reference also carries its backing tag's HISTORIAN + /// intent + ARRAY shape (Backing{IsHistorized,HistorianTagname,IsArray,ArrayLength}). They are + /// on the record so a raw-side edit to any of them makes the reference record unequal, producing the + /// coincident UnsReferenceDelta that repairs the mirror node; without them the UNS node kept the + /// old historian tagname / scalar shape after a surgical raw edit. + /// + [Fact] + public void Uns_reference_variables_carry_backing_historian_intent_and_array_shape() + { + var c = ComposeFullFixture(); + + // Backing tag historized WITH an override → both fields ride the reference verbatim. + var speedRef = c.UnsReferenceVariables.Single(v => v.UnsTagReferenceId == "ref-speed"); + speedRef.BackingIsHistorized.ShouldBeTrue(); + speedRef.BackingHistorianTagname.ShouldBe("speed_hist"); + speedRef.BackingIsArray.ShouldBeFalse(); + speedRef.BackingArrayLength.ShouldBeNull(); + + // Backing tag neither historized nor array → the defaults. + var statusRef = c.UnsReferenceVariables.Single(v => v.UnsTagReferenceId == "ref-status"); + statusRef.BackingIsHistorized.ShouldBeFalse(); + statusRef.BackingHistorianTagname.ShouldBeNull(); + statusRef.BackingIsArray.ShouldBeFalse(); + statusRef.BackingArrayLength.ShouldBeNull(); + } + + /// The array + historize-without-override branches of the same inheritance: an ARRAY backing tag + /// projects its shape onto the reference (so the UNS mirror is created as an array, and therefore + /// read-only), and a historized tag with a BLANK override leaves BackingHistorianTagname null — the + /// applier resolves that to the BackingRawPath, exactly as the raw side defaults to its NodeId. + [Fact] + public void Uns_reference_variables_carry_array_shape_and_blank_historian_override() + { + var driver = new DriverInstance + { + DriverInstanceId = "drv-modbus", ClusterId = "c1", RawFolderId = null, + Name = "Modbus1", DriverType = "Modbus", DriverConfig = "{}", + }; + var device = new Device { DeviceId = "dev-1", DriverInstanceId = "drv-modbus", Name = "PLC-A", DeviceConfig = "{}" }; + var levels = new Tag + { + TagId = "t-levels", DeviceId = "dev-1", Name = "Levels", DataType = "Int32", + AccessLevel = TagAccessLevel.ReadWrite, + TagConfig = "{\"isArray\":true,\"arrayLength\":8,\"isHistorized\":true}", + }; + var area = new UnsArea { UnsAreaId = "area-1", ClusterId = "c1", Name = "filling" }; + var line = new UnsLine { UnsLineId = "line-1", UnsAreaId = "area-1", Name = "line-1" }; + var equip = new Equipment { EquipmentId = "eq-1", UnsLineId = "line-1", Name = "station-1", MachineCode = "STATION_001" }; + var reference = new UnsTagReference { UnsTagReferenceId = "ref-levels", EquipmentId = "eq-1", TagId = "t-levels" }; + + var c = AddressSpaceComposer.Compose( + new[] { area }, new[] { line }, new[] { equip }, + new[] { driver }, Array.Empty(), + unsTagReferences: new[] { reference }, tags: new[] { levels }, + rawFolders: Array.Empty(), devices: new[] { device }); + + var levelsRef = c.UnsReferenceVariables.ShouldHaveSingleItem(); + levelsRef.BackingIsArray.ShouldBeTrue(); + levelsRef.BackingArrayLength.ShouldBe(8u); + levelsRef.BackingIsHistorized.ShouldBeTrue(); + levelsRef.BackingHistorianTagname.ShouldBeNull(); // blank override ⇒ resolved to BackingRawPath later + } + /// A composition with no raw/UNS inputs leaves all three new subtree sets empty (the legacy /// convenience overloads + earlier callers keep working). [Fact] diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpacePlannerDualNamespaceTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpacePlannerDualNamespaceTests.cs index b63807f9..dc7156cc 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpacePlannerDualNamespaceTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpacePlannerDualNamespaceTests.cs @@ -20,7 +20,7 @@ public sealed class AddressSpacePlannerDualNamespaceTests // Compose a fixture with one raw tag (given name) under Plant/Modbus1/PLC-A and one UNS reference to it // (with an optional display-name override so the effective name can be held stable across a raw rename). - private static AddressSpaceComposition Compose(string tagName, string? displayOverride) + private static AddressSpaceComposition Compose(string tagName, string? displayOverride, string tagConfig = "{}") { var folder = new RawFolder { RawFolderId = "raw-plant", ClusterId = "c1", Name = RawFolderName }; var driver = new DriverInstance @@ -32,7 +32,7 @@ public sealed class AddressSpacePlannerDualNamespaceTests var tag = new Tag { TagId = "t-speed", DeviceId = "dev-1", Name = tagName, - DataType = "Float", AccessLevel = TagAccessLevel.ReadWrite, TagConfig = "{}", + DataType = "Float", AccessLevel = TagAccessLevel.ReadWrite, TagConfig = tagConfig, }; var area = new UnsArea { UnsAreaId = "area-1", ClusterId = "c1", Name = "filling" }; var line = new UnsLine { UnsLineId = "line-1", UnsAreaId = "area-1", Name = "line-1" }; @@ -219,6 +219,55 @@ public sealed class AddressSpacePlannerDualNamespaceTests _ = prev; } + /// + /// THE PIN (2026-08-15 surgical-deploys review): a RAW-side edit to the backing tag's historian + /// intent or array shape must produce a ChangedRawTags delta AND a coincident + /// ChangedUnsReferenceVariables delta on every reference to it. The coincident delta is the ONLY + /// repair trigger for the UNS mirror node: the raw node is updated surgically in place and + /// EnsureVariable early-returns on the existing UNS node, so without this delta the mirror keeps + /// the OLD historian tagname (HistoryRead via the UNS NodeId serves a different series than via the raw + /// NodeId) and the OLD shape (a scalar→array flip leaves it scalar AND writable). Nothing about the + /// reference ROW changed here — only the backing tag's TagConfig — so the delta exists purely because + /// the four Backing* fields are on . + /// + [Theory] + [InlineData("{\"isHistorized\":true,\"historianTagname\":\"WW.Old\"}", "{\"isHistorized\":true,\"historianTagname\":\"WW.New\"}")] + [InlineData("{\"isHistorized\":true}", "{}")] + [InlineData("{}", "{\"isArray\":true,\"arrayLength\":8}")] + [InlineData("{\"isArray\":true,\"arrayLength\":8}", "{\"isArray\":true,\"arrayLength\":16}")] + public void Backing_tag_historian_or_shape_edit_produces_a_coincident_uns_reference_delta(string before, string after) + { + var prev = Compose("Speed", displayOverride: null, tagConfig: before); + var next = Compose("Speed", displayOverride: null, tagConfig: after); + + var plan = AddressSpacePlanner.Compute(prev, next); + + // Raw side: an attribute edit on a stable RawPath ⇒ a Changed delta (not remove+add). + var rawDelta = plan.ChangedRawTags.ShouldHaveSingleItem(); + rawDelta.Previous.NodeId.ShouldBe(rawDelta.Current.NodeId); + + // UNS side: the SAME reference row, same NodeId + BackingRawPath — flagged Changed ONLY because the + // inherited backing historian/shape moved. This is what drives the applier's remove+recreate repair. + var unsDelta = plan.ChangedUnsReferenceVariables.ShouldHaveSingleItem(); + unsDelta.Previous.UnsTagReferenceId.ShouldBe("ref-1"); + unsDelta.Previous.NodeId.ShouldBe(unsDelta.Current.NodeId); + unsDelta.Previous.BackingRawPath.ShouldBe(unsDelta.Current.BackingRawPath); + unsDelta.Previous.ShouldNotBe(unsDelta.Current); + + // And the inherited values on the CURRENT side match the backing tag's new intent, so the recreate + // materialises the mirror from the same truth the raw node was updated with. + unsDelta.Current.BackingIsHistorized.ShouldBe(rawDelta.Current.IsHistorized); + unsDelta.Current.BackingHistorianTagname.ShouldBe(rawDelta.Current.HistorianTagname); + unsDelta.Current.BackingIsArray.ShouldBe(rawDelta.Current.IsArray); + unsDelta.Current.BackingArrayLength.ShouldBe(rawDelta.Current.ArrayLength); + + // Neither realm sees a structural add/remove — this stays a surgical, subscription-preserving deploy. + plan.AddedRawTags.ShouldBeEmpty(); + plan.RemovedRawTags.ShouldBeEmpty(); + plan.AddedUnsReferenceVariables.ShouldBeEmpty(); + plan.RemovedUnsReferenceVariables.ShouldBeEmpty(); + } + /// Identical compositions diff to an empty plan across both realms (fresh list instances must /// not spuriously flag RawTags/UnsReferenceVariables as changed). [Fact] diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DeploymentArtifactRawUnsParityTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DeploymentArtifactRawUnsParityTests.cs index 21acd7ff..c80884ed 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DeploymentArtifactRawUnsParityTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DeploymentArtifactRawUnsParityTests.cs @@ -61,11 +61,14 @@ public sealed class DeploymentArtifactRawUnsParityTests var line = new UnsLine { UnsLineId = "l1", UnsAreaId = "a1", Name = "line1" }; var equip = new Equipment { EquipmentId = "EQ-1", UnsLineId = "l1", Name = "station1", MachineCode = "M1" }; var reference = new UnsTagReference { UnsTagReferenceId = "ref-1", EquipmentId = "EQ-1", TagId = "tag-speed", DisplayNameOverride = "MotorSpeed" }; + // A second reference onto the ARRAY tag: the UNS reference record carries the backing tag's array + // shape + historian intent (2026-08-15 review), so both branches must survive the artifact round-trip. + var arrayReference = new UnsTagReference { UnsTagReferenceId = "ref-2", EquipmentId = "EQ-1", TagId = "tag-profile", DisplayNameOverride = null }; var composed = AddressSpaceComposer.Compose( new[] { area }, new[] { line }, new[] { equip }, new[] { driver }, Array.Empty(), - unsTagReferences: new[] { reference }, tags: new[] { speed, run, profile, tempHi }, + unsTagReferences: new[] { reference, arrayReference }, tags: new[] { speed, run, profile, tempHi }, rawFolders: new[] { folder }, devices: new[] { device }, tagGroups: new[] { group }); // Artifact JSON — the same shape ConfigComposer serializes (enums numeric: ReadWrite = 1, Read = 0). @@ -85,7 +88,11 @@ public sealed class DeploymentArtifactRawUnsParityTests UnsAreas = new[] { new { UnsAreaId = "a1", Name = "filling", ClusterId = "c1" } }, UnsLines = new[] { new { UnsLineId = "l1", UnsAreaId = "a1", Name = "line1" } }, Equipment = new[] { new { EquipmentId = "EQ-1", UnsLineId = "l1", Name = "station1", MachineCode = "M1" } }, - UnsTagReferences = new[] { new { UnsTagReferenceId = "ref-1", EquipmentId = "EQ-1", TagId = "tag-speed", DisplayNameOverride = "MotorSpeed" } }, + UnsTagReferences = new[] + { + new { UnsTagReferenceId = "ref-1", EquipmentId = "EQ-1", TagId = "tag-speed", DisplayNameOverride = (string?)"MotorSpeed" }, + new { UnsTagReferenceId = "ref-2", EquipmentId = "EQ-1", TagId = "tag-profile", DisplayNameOverride = (string?)null }, + }, }); var decoded = DeploymentArtifact.ParseComposition(blob); @@ -103,9 +110,20 @@ public sealed class DeploymentArtifactRawUnsParityTests composedAlarm.Alarm.ShouldNotBeNull(); composedAlarm.Alarm!.AlarmType.ShouldBe("OffNormalAlarm"); composedAlarm.Writable.ShouldBeTrue(); - composed.UnsReferenceVariables.Count.ShouldBe(1); - composed.UnsReferenceVariables[0].NodeId.ShouldBe("filling/line1/station1/MotorSpeed"); - composed.UnsReferenceVariables[0].BackingRawPath.ShouldBe("Cell1/Modbus/Dev1/Fast/Speed"); + composed.UnsReferenceVariables.Count.ShouldBe(2); + var composedSpeedRef = composed.UnsReferenceVariables.Single(v => v.UnsTagReferenceId == "ref-1"); + composedSpeedRef.NodeId.ShouldBe("filling/line1/station1/MotorSpeed"); + composedSpeedRef.BackingRawPath.ShouldBe("Cell1/Modbus/Dev1/Fast/Speed"); + + // Same "both right, not both blind" sanity for the backing historian intent + array shape the UNS + // reference inherits — a composer that dropped them entirely would parity-match a decode that did too. + composedSpeedRef.BackingIsHistorized.ShouldBeTrue(); + composedSpeedRef.BackingHistorianTagname.ShouldBe("WW.Speed"); + composedSpeedRef.BackingIsArray.ShouldBeFalse(); + var composedProfileRef = composed.UnsReferenceVariables.Single(v => v.UnsTagReferenceId == "ref-2"); + composedProfileRef.BackingIsArray.ShouldBeTrue(); + composedProfileRef.BackingArrayLength.ShouldBe(16u); + composedProfileRef.BackingIsHistorized.ShouldBeFalse(); // Byte-parity: the three dual-namespace node sets are record-equal between the two seams. decoded.RawContainers.ShouldBe(composed.RawContainers);