diff --git a/code-reviews/OpcUaServer/findings.md b/code-reviews/OpcUaServer/findings.md index 857d4a35..d6924b49 100644 --- a/code-reviews/OpcUaServer/findings.md +++ b/code-reviews/OpcUaServer/findings.md @@ -7,7 +7,7 @@ | Review date | 2026-06-19 | | Commit reviewed | `7286d320` | | Status | Reviewed | -| Open findings | 1 | +| Open findings | 0 | ## Checklist coverage @@ -40,7 +40,7 @@ a category produced nothing rather than leaving it blank. | Severity | Medium | | Category | Correctness & logic bugs | | Location | `AddressSpacePlan.cs:56` (`AddressSpacePlan.IsEmpty`), `AddressSpacePlan.cs:80` (`AddressSpacePlanner.Compute`) | -| Status | Open | +| Status | Resolved | **Description:** `AddressSpaceComposition` carries the UNS topology (`UnsAreas` + `UnsLines`), and `AddressSpaceApplier.MaterialiseHierarchy` uses each area's/line's `DisplayName` for the OPC UA @@ -63,7 +63,53 @@ in place + `ClearChangeMasks`). Deferred: a complete fix spans the Runtime modul (`OpcUaPublishActor` must honour the new plan flag and call a hierarchy-refresh / rebuild path), which is outside this module's edit boundary. -**Resolution:** _(Open — deferred: needs a coordinated change in the Runtime module's `OpcUaPublishActor` to act on a UNS-changed plan; an in-`AddressSpacePlan` change alone is inert because `Apply`/`MaterialiseHierarchy` do not refresh existing folder names.)_ +**Resolution:** Resolved — 2026-06-20 (SHA pending): a UNS Area / Line **rename-only** deploy now produces a +non-empty plan that refreshes the existing folder's `DisplayName` IN PLACE (no rebuild, subscriptions +preserved). No EF migration, no entity-schema change, and no wire/proto/Commons-contract break — the +`AddressSpacePlan` is a pure in-process runtime diff (never serialized), and `DeploymentArtifact` already +round-trips each area/line `Name` into its `UnsAreaProjection`/`UnsLineProjection.DisplayName`, so the +artifact mirror needed NO change to carry the new name (byte-parity preserved). Changes span OpcUaServer + +Commons + Runtime: + +- **`AddressSpacePlan.cs`** (OpcUaServer) — added an init-only `RenamedFolders` diff set (new nested + `FolderRename(FolderNodeId, NewDisplayName)` record) mirroring the EquipmentTag/VirtualTag init-only + pattern, and folded it into `IsEmpty`. `AddressSpacePlanner.Compute` now diffs `prev.UnsAreas/UnsLines` + vs `next.UnsAreas/UnsLines` by stable id (`UnsAreaId`/`UnsLineId` — the exact NodeId scheme + `MaterialiseHierarchy` uses), emitting a rename only when a surviving folder's `DisplayName` differs + (ordinal). Added/removed areas are NOT renames (handled by the hierarchy/rebuild path). Output is + deterministic (areas first, then lines; each id-sorted). +- **`ISurgicalAddressSpaceSink.cs`** (Commons) — added an in-place + `bool UpdateFolderDisplayName(folderNodeId, displayName)` to the existing optional surgical-capability + interface (already forwarded by `DeferredAddressSpaceSink`); returns false when the folder is missing so + the caller rebuilds. +- **`OtOpcUaNodeManager.UpdateFolderDisplayName`** (OpcUaServer) — the surgical counterpart of + `EnsureFolder` (which early-returns on an existing folder and never touched its name): under `Lock` it + mutates the live `FolderState.DisplayName` and calls `ClearChangeMasks` (the SDK gotcha) so subscribers + see the new name immediately; NodeId/BrowseName unchanged. `SdkAddressSpaceSink` + `DeferredAddressSpaceSink` + forward it (the deferred forward is load-bearing: actors inject the wrapper, so without it the optimization + would be inert on every driver-role host — same trap as the F10b surgical-tag forward). +- **`AddressSpaceApplier.Apply`** (OpcUaServer) — when no structural rebuild fires, a rename-only (or + rename + surgical-tag) plan applies each rename via `UpdateFolderDisplayName`; a sink lacking the surgical + capability or a missing folder falls back to a full rebuild (safe default). When a structural rebuild DOES + fire (any add/remove/structural change), `MaterialiseHierarchy` re-creates every folder with the new names, + so renames are covered for free and no separate surgical call is made. `RenamedFolders.Count` is added to + the outcome's `ChangedNodes`. +- **`OpcUaPublishActor.HandleRebuild`** (Runtime) — no edit needed beyond the now-non-empty plan: the + existing `_applier.Apply(plan)` drives the in-place refresh, and the subsequent idempotent + `MaterialiseHierarchy(composition)` leaves the just-renamed folder alone (EnsureFolder early-returns on the + existing id). The IsEmpty short-circuit now correctly proceeds for a rename-only deploy. + +**Tests** (all green): `AddressSpacePlannerTests` — area-rename / line-rename / both-renames-ordered yield a +non-empty plan with the rename present, and no-change / added-area yield NO rename (no false positives). +`AddressSpaceApplierTests` — rename-only updates the folder in place with no rebuild; mixed-with-structural +rebuilds; non-surgical sink + surgical-returns-false both fall back to rebuild. `AddressSpaceApplierHierarchyTests` +— a real-SDK end-to-end apply asserts the existing area+line `FolderState.DisplayName` is swapped in place +with no node-count change. `DeferredAddressSpaceSinkTests` (OpcUaServer + Commons) — the deferred wrapper +forwards the new capability and returns the inner's result / false when not surgical. +`OpcUaPublishActorRebuildTests.Rebuild_with_area_rename_only_updates_folder_in_place_without_rebuild` — drives +the actor end-to-end: a second deploy changing ONLY the area Name reaches the apply path and issues a surgical +`UpdateFolderDisplayName("area-1", "Plant South")` without a second full rebuild. Suites: +`OpcUaServer.Tests` 297/297, `Runtime.Tests` 278/278, `Commons.Tests` deferred-sink 11/11. ### OpcUaServer-002 diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/DeferredAddressSpaceSink.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/DeferredAddressSpaceSink.cs index 9ba6f436..d3b15f01 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/DeferredAddressSpaceSink.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/DeferredAddressSpaceSink.cs @@ -87,4 +87,17 @@ public sealed class DeferredAddressSpaceSink : IOpcUaAddressSpaceSink, ISurgical public bool UpdateTagAttributes(string variableNodeId, bool writable, string? historianTagname, string dataType, bool isArray, uint? arrayLength) => _inner is ISurgicalAddressSpaceSink surgical && surgical.UpdateTagAttributes(variableNodeId, writable, historianTagname, dataType, isArray, arrayLength); + + /// Forwards an in-place folder-display-name update (OpcUaServer-001 — UNS Area / Line + /// rename) to the inner sink when it supports the surgical capability. Returns false otherwise — + /// before the real SdkAddressSpaceSink is swapped in (inner is still the null sink), or any + /// inner sink that isn't surgical — so the caller (AddressSpaceApplier) falls back to a full rebuild. + /// Without this forward the rename-refresh optimization is inert on every driver-role host, because + /// actors inject THIS wrapper, not the inner sink. + /// The folder node id whose display name to update in place. + /// The new display name to apply. + /// True when the inner sink applied the update; false when it lacks the capability or the folder is missing. + public bool UpdateFolderDisplayName(string folderNodeId, string displayName) + => _inner is ISurgicalAddressSpaceSink surgical + && surgical.UpdateFolderDisplayName(folderNodeId, displayName); } diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/ISurgicalAddressSpaceSink.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/ISurgicalAddressSpaceSink.cs index 1de37aed..7dc685f6 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/ISurgicalAddressSpaceSink.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/ISurgicalAddressSpaceSink.cs @@ -23,4 +23,15 @@ public interface ISurgicalAddressSpaceSink /// The declared length of the 1-D array when is true; ignored for scalars. /// True when the in-place update was applied; false when the node is missing (caller rebuilds). bool UpdateTagAttributes(string variableNodeId, bool writable, string? historianTagname, string dataType, bool isArray, uint? arrayLength); + + /// Update an existing folder node's DisplayName IN PLACE, notifying subscribers + /// (ClearChangeMasks) without a rebuild — used by AddressSpaceApplier to apply a UNS Area / Line + /// rename without tearing down + repopulating the whole address space (which would drop every + /// client's MonitoredItems). The folder's NodeId is unchanged (a rename only touches the friendly + /// browse/display name, not the logical id). Returns false if the folder does not exist (caller + /// should rebuild instead). + /// The node id of the folder whose display name to update in place. + /// The new display name to apply. + /// True when the in-place update was applied; false when the folder is missing (caller rebuilds). + bool UpdateFolderDisplayName(string folderNodeId, string displayName); } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceApplier.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceApplier.cs index 2161b3db..f8a22fd8 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceApplier.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceApplier.cs @@ -75,7 +75,9 @@ public sealed class AddressSpaceApplier var changedCount = plan.ChangedEquipment.Count + plan.ChangedDrivers.Count + plan.ChangedAlarms.Count + plan.ChangedEquipmentTags.Count + - plan.ChangedEquipmentVirtualTags.Count; + plan.ChangedEquipmentVirtualTags.Count + + // OpcUaServer-001: a UNS Area/Line rename is an in-place change to an existing folder node. + plan.RenamedFolders.Count; var addedCount = plan.AddedEquipment.Count + plan.AddedDrivers.Count + plan.AddedAlarms.Count + plan.AddedEquipmentTags.Count + @@ -113,6 +115,11 @@ public sealed class AddressSpaceApplier plan.ChangedEquipmentVirtualTags.Any(d => !VtagDeltaIsNodeIrrelevant(d)); var surgicalTagDeltas = plan.ChangedEquipmentTags.Where(TagDeltaIsSurgicalEligible).ToList(); + // OpcUaServer-001 — UNS Area / Line renames are surgically applicable (in-place DisplayName swap) + // when no structural rebuild fires. When a rebuild DOES fire (any add/remove/structural change), + // MaterialiseHierarchy re-creates every folder with the new display names, so the renames are + // covered for free and need no separate surgical pass. + var renamedFolders = plan.RenamedFolders; var rebuilt = false; if (structuralRebuild) @@ -120,12 +127,24 @@ public sealed class AddressSpaceApplier SafeRebuild(); rebuilt = true; } - else if (surgicalTagDeltas.Count > 0) + else if (surgicalTagDeltas.Count > 0 || renamedFolders.Count > 0) { if (_sink is ISurgicalAddressSpaceSink surgical) { var allApplied = true; - foreach (var d in surgicalTagDeltas) + // Folder renames first — an in-place DisplayName swap on the existing Area/Line folder. + foreach (var rename in renamedFolders) + { + bool ok; + try { ok = surgical.UpdateFolderDisplayName(rename.FolderNodeId, rename.NewDisplayName); } + catch (Exception ex) + { + _logger.LogError(ex, "AddressSpaceApplier: surgical UpdateFolderDisplayName threw for {Node}", rename.FolderNodeId); + ok = false; + } + if (!ok) { allApplied = false; break; } + } + foreach (var d in allApplied ? surgicalTagDeltas : Enumerable.Empty()) { // Compute the node id + writable + historian + shape EXACTLY as MaterialiseEquipmentTags // would so the in-place update matches what a rebuild would have produced. Array tags are @@ -155,8 +174,8 @@ public sealed class AddressSpaceApplier } _logger.LogInformation( - "AddressSpaceApplier: applied plan (added={Added}, removed={Removed}, changed={Changed}, surgicalTags={Surgical}, rebuild={Rebuild})", - addedCount, removedCount, changedCount, rebuilt ? 0 : surgicalTagDeltas.Count, rebuilt); + "AddressSpaceApplier: applied plan (added={Added}, removed={Removed}, changed={Changed}, surgicalTags={Surgical}, renamedFolders={Renamed}, rebuild={Rebuild})", + addedCount, removedCount, changedCount, rebuilt ? 0 : surgicalTagDeltas.Count, rebuilt ? 0 : renamedFolders.Count, rebuilt); return new AddressSpaceApplyOutcome(removedCount, addedCount, changedCount, rebuilt); } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpacePlan.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpacePlan.cs index e5ecdade..815b1590 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpacePlan.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpacePlan.cs @@ -52,19 +52,45 @@ public sealed record AddressSpacePlan( /// public IReadOnlyList ChangedEquipmentVirtualTags { get; init; } = Array.Empty(); + /// + /// OpcUaServer-001 — UNS Area / Line folder renames: a folder whose stable id is unchanged but + /// whose DisplayName differs between the previous + next composition. A deploy whose ONLY + /// change is an Area or Line rename produces NO Equipment / Driver / Alarm / Tag / VirtualTag + /// delta, so without this set the plan would be and + /// OpcUaPublishActor.HandleRebuild would short-circuit BEFORE the apply path runs, leaving + /// the folder's stale OPC UA DisplayName until some unrelated structural change forced a + /// rebuild. applies each rename IN PLACE via + /// (folder NodeId = + /// the area's UnsAreaId / line's UnsLineId, the exact ids MaterialiseHierarchy + /// uses), preserving every client's subscriptions; a sink lacking the surgical capability or a + /// missing folder falls back to a full rebuild. Added as an init-only member (defaulting empty) so + /// every existing AddressSpacePlan construction site compiles unchanged — consistent with + /// the EquipmentTag / EquipmentVirtualTag diff sets above. + /// + public IReadOnlyList RenamedFolders { get; init; } = Array.Empty(); + /// Gets a value indicating whether the composition plan contains no changes. public bool IsEmpty => AddedEquipment.Count == 0 && RemovedEquipment.Count == 0 && ChangedEquipment.Count == 0 && AddedDrivers.Count == 0 && RemovedDrivers.Count == 0 && ChangedDrivers.Count == 0 && AddedAlarms.Count == 0 && RemovedAlarms.Count == 0 && ChangedAlarms.Count == 0 && AddedEquipmentTags.Count == 0 && RemovedEquipmentTags.Count == 0 && ChangedEquipmentTags.Count == 0 && - AddedEquipmentVirtualTags.Count == 0 && RemovedEquipmentVirtualTags.Count == 0 && ChangedEquipmentVirtualTags.Count == 0; + AddedEquipmentVirtualTags.Count == 0 && RemovedEquipmentVirtualTags.Count == 0 && ChangedEquipmentVirtualTags.Count == 0 && + RenamedFolders.Count == 0; public sealed record EquipmentDelta(EquipmentNode Previous, EquipmentNode Current); public sealed record DriverDelta(DriverInstancePlan Previous, DriverInstancePlan Current); public sealed record AlarmDelta(ScriptedAlarmPlan Previous, ScriptedAlarmPlan Current); public sealed record EquipmentTagDelta(EquipmentTagPlan Previous, EquipmentTagPlan Current); public sealed record EquipmentVirtualTagDelta(EquipmentVirtualTagPlan Previous, EquipmentVirtualTagPlan Current); + + /// OpcUaServer-001 — one renamed UNS Area / Line folder: the stable folder + /// (the area's UnsAreaId or line's UnsLineId, the exact + /// NodeId MaterialiseHierarchy placed the folder at) and the + /// to apply in place. + /// The folder's stable NodeId (area/line id) — unchanged by a rename. + /// The new display name to apply. + public sealed record FolderRename(string FolderNodeId, string NewDisplayName); } public static class AddressSpacePlanner @@ -112,6 +138,15 @@ public static class AddressSpacePlanner t => t.VirtualTagId, (a, b) => new AddressSpacePlan.EquipmentVirtualTagDelta(a, b)); + // OpcUaServer-001 — UNS Area / Line renames: a folder whose stable id is unchanged but whose + // DisplayName differs. Diffed by stable id (UnsAreaId / UnsLineId) so an Area/Line whose ONLY + // change is its friendly name is no longer a silent no-op at the IsEmpty gate. The folder NodeId + // IS the area/line id (the exact scheme MaterialiseHierarchy uses), so the rename carries it + // directly. Areas first, then lines; each list is independently sorted by id for determinism. + var renamedFolders = DiffRenames(previous.UnsAreas, next.UnsAreas, a => a.UnsAreaId, a => a.DisplayName) + .Concat(DiffRenames(previous.UnsLines, next.UnsLines, l => l.UnsLineId, l => l.DisplayName)) + .ToList(); + return new AddressSpacePlan( addedEq, removedEq, changedEq, addedDrv, removedDrv, changedDrv, @@ -123,9 +158,37 @@ public static class AddressSpacePlanner AddedEquipmentVirtualTags = addedVTags, RemovedEquipmentVirtualTags = removedVTags, ChangedEquipmentVirtualTags = changedVTags, + RenamedFolders = renamedFolders, }; } + /// + /// OpcUaServer-001 — emit a for every folder present in + /// BOTH snapshots (matched by stable ) whose + /// differs (ordinal). Added/removed folders are NOT renames — they're handled by the equipment / + /// hierarchy rebuild path — so this pass only flags an in-place display-name change on a surviving + /// folder. Sorted by id for deterministic ordering. + /// + private static List DiffRenames( + IReadOnlyList previous, + IReadOnlyList next, + Func identity, + Func displayName) where T : class + { + var prevById = previous.ToDictionary(identity, StringComparer.Ordinal); + var renames = new List(); + foreach (var n in next) + { + if (prevById.TryGetValue(identity(n), out var p) + && !string.Equals(displayName(p), displayName(n), StringComparison.Ordinal)) + { + renames.Add(new AddressSpacePlan.FolderRename(identity(n), displayName(n))); + } + } + renames.Sort((a, b) => string.CompareOrdinal(a.FolderNodeId, b.FolderNodeId)); + return renames; + } + private static (IReadOnlyList Added, IReadOnlyList Removed, IReadOnlyList Changed) DiffById( IReadOnlyList previous, diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs index e6f7c8b4..24f845c1 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs @@ -1307,6 +1307,34 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 } } + /// + /// Update an EXISTING folder node's in place and notify + /// subscribers, WITHOUT a rebuild — the in-place counterpart of for a + /// UNS Area / Line rename (OpcUaServer-001). early-returns for an + /// already-present folder id and never touches an existing folder's display name, so a + /// rename-only deploy would otherwise be invisible until a full + /// cleared _folders. The NodeId + BrowseName are unchanged (a rename only touches the + /// friendly display name, not the logical id, so browse-path resolution + ACLs are unaffected). + /// ClearChangeMasks is called so subscribed clients see the new DisplayName immediately, + /// mirroring the surgical path. Returns false when the folder id + /// is unknown (caller falls back to a full rebuild). + /// + /// The node identifier of the folder to update in place. + /// The new display name to apply. + /// True when the in-place update was applied; false when the folder id is unknown. + public bool UpdateFolderDisplayName(string folderNodeId, string displayName) + { + ArgumentException.ThrowIfNullOrEmpty(folderNodeId); + ArgumentException.ThrowIfNullOrEmpty(displayName); + lock (Lock) + { + if (!_folders.TryGetValue(folderNodeId, out var folder)) return false; + folder.DisplayName = displayName; + folder.ClearChangeMasks(SystemContext, includeChildren: false); + return true; + } + } + /// /// Ensure a Variable node exists at parented under /// (or root when null). Initial value=null, quality=Bad, diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/SdkAddressSpaceSink.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/SdkAddressSpaceSink.cs index 85f7238a..0bf157bf 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/SdkAddressSpaceSink.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/SdkAddressSpaceSink.cs @@ -77,6 +77,15 @@ public sealed class SdkAddressSpaceSink : IOpcUaAddressSpaceSink, ISurgicalAddre public bool UpdateTagAttributes(string variableNodeId, bool writable, string? historianTagname, string dataType, bool isArray, uint? arrayLength) => _nodeManager.UpdateTagAttributes(variableNodeId, writable, historianTagname, dataType, isArray, arrayLength); + /// OpcUaServer-001: surgically update an existing folder node's display name in place (no + /// rebuild) for a UNS Area / Line rename. Returns false when the folder does not exist (caller falls + /// back to a full rebuild). + /// The folder node identifier whose display name to update in place. + /// The new display name to apply. + /// True when the in-place update was applied; false when the folder is missing. + public bool UpdateFolderDisplayName(string folderNodeId, string displayName) + => _nodeManager.UpdateFolderDisplayName(folderNodeId, displayName); + /// Rebuilds the entire OPC UA address space. public void RebuildAddressSpace() => _nodeManager.RebuildAddressSpace(); } diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/OpcUa/DeferredAddressSpaceSinkTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/OpcUa/DeferredAddressSpaceSinkTests.cs index 885fb595..1780263d 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/OpcUa/DeferredAddressSpaceSinkTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/OpcUa/DeferredAddressSpaceSinkTests.cs @@ -90,6 +90,30 @@ public class DeferredAddressSpaceSinkTests surgical.UpdateCalled.ShouldBeTrue(); } + [Fact] + public void UpdateFolderDisplayName_returns_false_for_non_surgical_inner() + { + // SpySink does NOT implement ISurgicalAddressSpaceSink. + var sink = new DeferredAddressSpaceSink(); + sink.SetSink(new SpySink()); + + sink.UpdateFolderDisplayName("area-1", "Plant South").ShouldBeFalse(); + } + + [Fact] + public void UpdateFolderDisplayName_returns_true_for_surgical_inner() + { + // OpcUaServer-001: the deferred wrapper must forward the folder-rename capability to a surgical inner. + var surgical = new SpySurgicalSink(); + var sink = new DeferredAddressSpaceSink(); + sink.SetSink(surgical); + + var result = sink.UpdateFolderDisplayName("area-1", "Plant South"); + + result.ShouldBeTrue(); + surgical.FolderRenameCalled.ShouldBeTrue(); + } + // ---------- SetSink(null) reverts to null sink ---------- [Fact] @@ -137,6 +161,7 @@ public class DeferredAddressSpaceSinkTests private sealed class SpySurgicalSink : IOpcUaAddressSpaceSink, ISurgicalAddressSpaceSink { public bool UpdateCalled { get; private set; } + public bool FolderRenameCalled { get; private set; } public void WriteValue(string nodeId, object? value, OpcUaQuality quality, DateTime sourceTimestampUtc) { } public void WriteAlarmCondition(string alarmNodeId, AlarmConditionSnapshot state, DateTime sourceTimestampUtc) { } @@ -150,5 +175,11 @@ public class DeferredAddressSpaceSinkTests UpdateCalled = true; return true; } + + public bool UpdateFolderDisplayName(string folderNodeId, string displayName) + { + FolderRenameCalled = true; + return true; + } } } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierHierarchyTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierHierarchyTests.cs index c533e184..17ba1248 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierHierarchyTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierHierarchyTests.cs @@ -210,6 +210,70 @@ public sealed class AddressSpaceApplierHierarchyTests : IDisposable sdkServer.NodeManager!.VariableCount.ShouldBe(1); // the Speed signal under the equipment folder } + /// OpcUaServer-001 — a UNS Area / Line rename-only deploy refreshes the EXISTING folder's + /// DisplayName IN PLACE against a real SDK node manager (no rebuild, no node count change). Proves the + /// full chain: planner emits a RenamedFolders delta → applier drives the surgical + /// SdkAddressSpaceSink.UpdateFolderDisplayName → OtOpcUaNodeManager mutates the live FolderState + + /// ClearChangeMasks. Before the fix, EnsureFolder early-returned on the existing folder and the + /// DisplayName stayed stale until a full RebuildAddressSpace. + [Fact] + public async Task Area_and_line_rename_updates_existing_folder_display_name_in_place_against_real_SDK() + { + await using var host = new OpcUaApplicationHost( + new OpcUaApplicationHostOptions + { + ApplicationName = "OtOpcUa.FolderRename", + ApplicationUri = $"urn:OtOpcUa.FolderRename:{Guid.NewGuid():N}", + OpcUaPort = AllocateFreePort(), + PublicHostname = "localhost", + PkiStoreRoot = _pkiRoot, + }, + NullLogger.Instance); + + var sdkServer = new OtOpcUaSdkServer(); + await host.StartAsync(sdkServer, Ct); + sdkServer.NodeManager.ShouldNotBeNull(); + var nm = sdkServer.NodeManager!; + + var sink = new SdkAddressSpaceSink(nm); + var applier = new AddressSpaceApplier(sink, NullLogger.Instance); + + // Materialise the initial hierarchy (area + line + equipment) with the OLD display names. + var initial = new AddressSpaceComposition( + UnsAreas: new[] { new UnsAreaProjection("area-1", "Plant North") }, + UnsLines: new[] { new UnsLineProjection("line-1", "area-1", "Cell A") }, + EquipmentNodes: new[] { new EquipmentNode("eq-1", "Pump-1", "line-1") }, + DriverInstancePlans: Array.Empty(), + ScriptedAlarmPlans: Array.Empty()); + applier.MaterialiseHierarchy(initial); + + nm.FolderCount.ShouldBe(3); + nm.TryGetFolder("area-1")!.DisplayName.Text.ShouldBe("Plant North"); + nm.TryGetFolder("line-1")!.DisplayName.Text.ShouldBe("Cell A"); + + // Now a rename-only deploy: same ids, new display names on BOTH the area and the line. + var renamed = new AddressSpaceComposition( + UnsAreas: new[] { new UnsAreaProjection("area-1", "Plant South") }, + UnsLines: new[] { new UnsLineProjection("line-1", "area-1", "Cell B") }, + EquipmentNodes: new[] { new EquipmentNode("eq-1", "Pump-1", "line-1") }, + DriverInstancePlans: Array.Empty(), + ScriptedAlarmPlans: Array.Empty()); + + var plan = AddressSpacePlanner.Compute(initial, renamed); + plan.IsEmpty.ShouldBeFalse(); // rename is no longer a silent no-op + plan.RenamedFolders.Count.ShouldBe(2); + + var outcome = applier.Apply(plan); + + // In-place: no rebuild, no node-count change — only the DisplayNames were swapped. + outcome.RebuildCalled.ShouldBeFalse(); + nm.FolderCount.ShouldBe(3); + nm.TryGetFolder("area-1")!.DisplayName.Text.ShouldBe("Plant South"); + nm.TryGetFolder("line-1")!.DisplayName.Text.ShouldBe("Cell B"); + // The unrelated equipment folder is untouched. + nm.TryGetFolder("eq-1")!.DisplayName.Text.ShouldBe("Pump-1"); + } + private static int AllocateFreePort() { using var listener = new System.Net.Sockets.TcpListener(System.Net.IPAddress.Loopback, 0); 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 5650d42e..f15c2795 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierTests.cs @@ -1473,6 +1473,144 @@ public sealed class AddressSpaceApplierTests sink.SurgicalCalls.ShouldHaveSingleItem(); // the surgical update was attempted first } + // ----- OpcUaServer-001: in-place UNS Area / Line folder rename (no rebuild) ----- + + /// OpcUaServer-001 — a deploy whose ONLY change is a UNS Area rename must SKIP the rebuild and + /// apply the new DisplayName IN PLACE via ISurgicalAddressSpaceSink.UpdateFolderDisplayName, + /// preserving every client's subscriptions. Exactly one surgical folder call lands with the area's + /// NodeId (== UnsAreaId) and the NEW display name; the rename still counts as a change. + [Fact] + public void Area_rename_only_skips_rebuild_and_updates_folder_in_place() + { + var sink = new RecordingSink(); + var applier = new AddressSpaceApplier(sink, NullLogger.Instance); + + var previous = CompositionWithAreas(new UnsAreaProjection("area-1", "Plant North")); + var next = CompositionWithAreas(new UnsAreaProjection("area-1", "Plant South")); + + var plan = AddressSpacePlanner.Compute(previous, next); + plan.RenamedFolders.Count.ShouldBe(1); + + var outcome = applier.Apply(plan); + + outcome.RebuildCalled.ShouldBeFalse(); + sink.RebuildCalls.ShouldBe(0); // NO RebuildAddressSpace — subscriptions preserved + var call = sink.FolderRenameCalls.ShouldHaveSingleItem(); + call.FolderNodeId.ShouldBe("area-1"); // folder NodeId == UnsAreaId (MaterialiseHierarchy scheme) + call.DisplayName.ShouldBe("Plant South"); // the NEW display name + outcome.ChangedNodes.ShouldBe(1); + } + + /// OpcUaServer-001 — a Line rename only updates the line folder in place (NodeId == UnsLineId) + /// and skips the rebuild. + [Fact] + public void Line_rename_only_skips_rebuild_and_updates_folder_in_place() + { + var sink = new RecordingSink(); + var applier = new AddressSpaceApplier(sink, NullLogger.Instance); + + var previous = CompositionWithLines(new UnsLineProjection("line-1", "area-1", "Cell A")); + var next = CompositionWithLines(new UnsLineProjection("line-1", "area-1", "Cell B")); + + var plan = AddressSpacePlanner.Compute(previous, next); + plan.RenamedFolders.Count.ShouldBe(1); + + var outcome = applier.Apply(plan); + + outcome.RebuildCalled.ShouldBeFalse(); + sink.RebuildCalls.ShouldBe(0); + var call = sink.FolderRenameCalls.ShouldHaveSingleItem(); + call.FolderNodeId.ShouldBe("line-1"); + call.DisplayName.ShouldBe("Cell B"); + } + + /// OpcUaServer-001 — a folder rename MIXED with a structural change (here an added equipment) + /// must rebuild: the rebuild + MaterialiseHierarchy re-create every folder with the new names, so no + /// separate surgical folder call is made. The rename is covered by the rebuild for free. + [Fact] + public void Folder_rename_mixed_with_added_equipment_rebuilds() + { + var sink = new RecordingSink(); + var applier = new AddressSpaceApplier(sink, NullLogger.Instance); + + var previous = new AddressSpaceComposition( + UnsAreas: new[] { new UnsAreaProjection("area-1", "North") }, + UnsLines: Array.Empty(), + EquipmentNodes: Array.Empty(), + DriverInstancePlans: Array.Empty(), + ScriptedAlarmPlans: Array.Empty()); + // Area renamed AND a brand-new equipment node — the structural add forces a rebuild. + var next = new AddressSpaceComposition( + UnsAreas: new[] { new UnsAreaProjection("area-1", "South") }, + UnsLines: Array.Empty(), + EquipmentNodes: new[] { new EquipmentNode("eq-new", "New", "line-1") }, + DriverInstancePlans: Array.Empty(), + ScriptedAlarmPlans: Array.Empty()); + + var plan = AddressSpacePlanner.Compute(previous, next); + plan.RenamedFolders.Count.ShouldBe(1); + plan.AddedEquipment.Count.ShouldBe(1); + + var outcome = applier.Apply(plan); + + outcome.RebuildCalled.ShouldBeTrue(); + sink.RebuildCalls.ShouldBe(1); + sink.FolderRenameCalls.ShouldBeEmpty(); // no surgical folder call — rebuild covers it + } + + /// OpcUaServer-001 fallback — a rename-only plan on a sink that does NOT implement + /// cannot apply the in-place update, so it drives a full + /// rebuild (safe default). + [Fact] + public void Folder_rename_on_non_surgical_sink_rebuilds() + { + var sink = new PlainRecordingSink(); + var applier = new AddressSpaceApplier(sink, NullLogger.Instance); + + var previous = CompositionWithAreas(new UnsAreaProjection("area-1", "North")); + var next = CompositionWithAreas(new UnsAreaProjection("area-1", "South")); + + var plan = AddressSpacePlanner.Compute(previous, next); + plan.RenamedFolders.Count.ShouldBe(1); + + var outcome = applier.Apply(plan); + + outcome.RebuildCalled.ShouldBeTrue(); + sink.RebuildCalls.ShouldBe(1); + } + + /// OpcUaServer-001 fallback — when the surgical sink reports the folder MISSING + /// (UpdateFolderDisplayName returns false), the applier falls back to a full rebuild. The + /// surgical call is still attempted (recorded once) before the fallback fires. + [Fact] + public void Folder_rename_surgical_returning_false_falls_back_to_rebuild() + { + var sink = new RecordingSink { FolderRenameReturns = false }; + var applier = new AddressSpaceApplier(sink, NullLogger.Instance); + + var previous = CompositionWithAreas(new UnsAreaProjection("area-1", "North")); + var next = CompositionWithAreas(new UnsAreaProjection("area-1", "South")); + + var plan = AddressSpacePlanner.Compute(previous, next); + plan.RenamedFolders.Count.ShouldBe(1); + + var outcome = applier.Apply(plan); + + outcome.RebuildCalled.ShouldBeTrue(); + sink.RebuildCalls.ShouldBe(1); // fell back to a full rebuild + sink.FolderRenameCalls.ShouldHaveSingleItem(); // the surgical update was attempted first + } + + private static AddressSpaceComposition CompositionWithAreas(params UnsAreaProjection[] areas) => + new( + areas, Array.Empty(), Array.Empty(), + Array.Empty(), Array.Empty()); + + private static AddressSpaceComposition CompositionWithLines(params UnsLineProjection[] lines) => + new( + Array.Empty(), lines, Array.Empty(), + Array.Empty(), Array.Empty()); + private static AddressSpaceComposition CompositionWithTags(params EquipmentTagPlan[] tags) => new( Array.Empty(), Array.Empty(), Array.Empty()) @@ -1535,6 +1673,23 @@ public sealed class AddressSpaceApplierTests return SurgicalReturns; } + /// Gets the queue of surgical in-place folder display-name update calls (OpcUaServer-001). + public ConcurrentQueue<(string FolderNodeId, string DisplayName)> FolderRenameQueue { get; } = new(); + /// Gets the list of recorded surgical folder display-name update calls. + public List<(string FolderNodeId, string DisplayName)> FolderRenameCalls => FolderRenameQueue.ToList(); + /// When false, reports the folder missing (returns + /// false), driving the applier's rebuild fallback. Defaults to true. + public bool FolderRenameReturns { get; init; } = true; + + /// Records a surgical in-place folder display-name update; returns . + /// The folder node ID to update in place. + /// The new display name to apply. + public bool UpdateFolderDisplayName(string folderNodeId, string displayName) + { + FolderRenameQueue.Enqueue((folderNodeId, displayName)); + return FolderRenameReturns; + } + /// Gets the queue of alarm condition write calls. public ConcurrentQueue<(string NodeId, AlarmConditionSnapshot State)> AlarmQueue { get; } = new(); /// Gets the queue of folder creation calls. diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpacePlannerTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpacePlannerTests.cs index 645056cd..7f01b9b3 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpacePlannerTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpacePlannerTests.cs @@ -353,6 +353,131 @@ public sealed class AddressSpacePlannerTests plan.RemovedEquipment.Select(e => e.EquipmentId).ShouldBe(new[] { "a", "z" }); } + /// OpcUaServer-001 — a deploy whose ONLY change is a UNS Area rename (the area's + /// DisplayName differs; same id, no equipment/driver/alarm/tag/vtag delta) must yield a + /// NON-empty plan with the rename in , so + /// OpcUaPublishActor.HandleRebuild no longer short-circuits at the IsEmpty gate before the + /// folder's DisplayName is refreshed. + [Fact] + public void Area_rename_only_yields_non_empty_plan_with_renamed_folder() + { + var prev = new AddressSpaceComposition( + UnsAreas: new[] { new UnsAreaProjection("area-1", "Plant North") }, + UnsLines: Array.Empty(), + EquipmentNodes: Array.Empty(), + DriverInstancePlans: Array.Empty(), + ScriptedAlarmPlans: Array.Empty()); + var next = new AddressSpaceComposition( + UnsAreas: new[] { new UnsAreaProjection("area-1", "Plant South") }, + UnsLines: Array.Empty(), + EquipmentNodes: Array.Empty(), + DriverInstancePlans: Array.Empty(), + ScriptedAlarmPlans: Array.Empty()); + + var plan = AddressSpacePlanner.Compute(prev, next); + + plan.IsEmpty.ShouldBeFalse(); + var rename = plan.RenamedFolders.ShouldHaveSingleItem(); + rename.FolderNodeId.ShouldBe("area-1"); // folder NodeId == the area id (MaterialiseHierarchy scheme) + rename.NewDisplayName.ShouldBe("Plant South"); + } + + /// OpcUaServer-001 — a deploy whose ONLY change is a UNS Line rename yields a NON-empty plan + /// with the rename in (folder NodeId == the line id). + [Fact] + public void Line_rename_only_yields_non_empty_plan_with_renamed_folder() + { + var prev = new AddressSpaceComposition( + UnsAreas: Array.Empty(), + UnsLines: new[] { new UnsLineProjection("line-1", "area-1", "Cell A") }, + EquipmentNodes: Array.Empty(), + DriverInstancePlans: Array.Empty(), + ScriptedAlarmPlans: Array.Empty()); + var next = new AddressSpaceComposition( + UnsAreas: Array.Empty(), + UnsLines: new[] { new UnsLineProjection("line-1", "area-1", "Cell B") }, + EquipmentNodes: Array.Empty(), + DriverInstancePlans: Array.Empty(), + ScriptedAlarmPlans: Array.Empty()); + + var plan = AddressSpacePlanner.Compute(prev, next); + + plan.IsEmpty.ShouldBeFalse(); + var rename = plan.RenamedFolders.ShouldHaveSingleItem(); + rename.FolderNodeId.ShouldBe("line-1"); + rename.NewDisplayName.ShouldBe("Cell B"); + } + + /// OpcUaServer-001 — no false positives: two compositions with identical Area + Line topology + /// (same ids, same DisplayNames) produce NO rename and an empty plan. Pins that the rename diff fires + /// ONLY on an actual DisplayName change, not on every redeploy. + [Fact] + public void Identical_area_line_topology_yields_empty_plan_no_renames() + { + var areas = new[] { new UnsAreaProjection("area-1", "Plant North") }; + var lines = new[] { new UnsLineProjection("line-1", "area-1", "Cell A") }; + var prev = new AddressSpaceComposition(areas, lines, Array.Empty(), + Array.Empty(), Array.Empty()); + // Fresh projection instances, identical contents — proves value (not reference) comparison. + var next = new AddressSpaceComposition( + new[] { new UnsAreaProjection("area-1", "Plant North") }, + new[] { new UnsLineProjection("line-1", "area-1", "Cell A") }, + Array.Empty(), Array.Empty(), Array.Empty()); + + var plan = AddressSpacePlanner.Compute(prev, next); + + plan.IsEmpty.ShouldBeTrue(); + plan.RenamedFolders.ShouldBeEmpty(); + } + + /// OpcUaServer-001 — a brand-new Area (added, not renamed) is NOT a rename: an area present + /// only in next is materialised by the hierarchy pass, not refreshed in place. The rename diff + /// must only flag folders present in BOTH snapshots whose DisplayName changed. + [Fact] + public void Added_area_is_not_a_rename() + { + var prev = new AddressSpaceComposition( + Array.Empty(), Array.Empty(), Array.Empty()); + var next = new AddressSpaceComposition( + UnsAreas: new[] { new UnsAreaProjection("area-1", "Plant North") }, + UnsLines: Array.Empty(), + EquipmentNodes: Array.Empty(), + DriverInstancePlans: Array.Empty(), + ScriptedAlarmPlans: Array.Empty()); + + var plan = AddressSpacePlanner.Compute(prev, next); + + plan.RenamedFolders.ShouldBeEmpty(); + } + + /// OpcUaServer-001 — when BOTH an Area and a Line are renamed in one deploy, both renames + /// appear in , ordered deterministically by folder id + /// (areas + lines concatenated, each sorted by id). + [Fact] + public void Area_and_line_renames_both_captured_and_ordered_by_id() + { + var prev = new AddressSpaceComposition( + UnsAreas: new[] { new UnsAreaProjection("area-1", "North") }, + UnsLines: new[] { new UnsLineProjection("line-1", "area-1", "Cell A") }, + EquipmentNodes: Array.Empty(), + DriverInstancePlans: Array.Empty(), + ScriptedAlarmPlans: Array.Empty()); + var next = new AddressSpaceComposition( + UnsAreas: new[] { new UnsAreaProjection("area-1", "South") }, + UnsLines: new[] { new UnsLineProjection("line-1", "area-1", "Cell B") }, + EquipmentNodes: Array.Empty(), + DriverInstancePlans: Array.Empty(), + ScriptedAlarmPlans: Array.Empty()); + + var plan = AddressSpacePlanner.Compute(prev, next); + + plan.RenamedFolders.Count.ShouldBe(2); + // "area-1" sorts before "line-1" (areas first, then lines, each id-sorted) — deterministic order. + plan.RenamedFolders.Select(r => r.FolderNodeId).ShouldBe(new[] { "area-1", "line-1" }); + plan.RenamedFolders.Single(r => r.FolderNodeId == "area-1").NewDisplayName.ShouldBe("South"); + plan.RenamedFolders.Single(r => r.FolderNodeId == "line-1").NewDisplayName.ShouldBe("Cell B"); + } + /// Verifies that mixed changes across all three classes are captured in one pass. [Fact] public void Mixed_changes_across_all_three_classes_are_captured_in_one_pass() diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/DeferredAddressSpaceSinkTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/DeferredAddressSpaceSinkTests.cs index f7727d5d..9d6f28f4 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/DeferredAddressSpaceSinkTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/DeferredAddressSpaceSinkTests.cs @@ -139,6 +139,40 @@ public sealed class DeferredAddressSpaceSinkTests .ShouldBeFalse(); } + /// OpcUaServer-001 regression: the deferred wrapper MUST forward the folder-rename surgical + /// capability to a surgical inner sink — otherwise AddressSpaceApplier (which injects THIS + /// wrapper on every driver-role host) never sees the capability and the in-place UNS Area/Line rename + /// refresh is inert in production (silently falls back to a full rebuild). + [Fact] + public void UpdateFolderDisplayName_forwards_to_a_surgical_inner_sink() + { + var deferred = new DeferredAddressSpaceSink(); + var inner = new SurgicalRecordingSink { Result = true }; + deferred.SetSink(inner); + + ((ISurgicalAddressSpaceSink)deferred).UpdateFolderDisplayName("area-1", "Plant South") + .ShouldBeTrue(); + + var call = inner.FolderRenameCalls.ShouldHaveSingleItem(); + call.FolderNodeId.ShouldBe("area-1"); + call.DisplayName.ShouldBe("Plant South"); + } + + /// The folder-rename forward returns the inner's own result (false ⇒ folder missing) so the + /// caller falls back to a full rebuild; and returns false when the inner is not surgical at all. + [Fact] + public void UpdateFolderDisplayName_returns_inner_result_and_false_when_not_surgical() + { + var deferred = new DeferredAddressSpaceSink(); + // Inner reports the folder missing ⇒ forward returns false. + deferred.SetSink(new SurgicalRecordingSink { Result = false }); + ((ISurgicalAddressSpaceSink)deferred).UpdateFolderDisplayName("area-1", "X").ShouldBeFalse(); + + // Non-surgical inner (the null sink before swap-in) ⇒ false. + deferred.SetSink(null); + ((ISurgicalAddressSpaceSink)deferred).UpdateFolderDisplayName("area-1", "X").ShouldBeFalse(); + } + /// Builds a minimal for the forwarding tests (the /// inner sink only records the node id, so the exact state values don't matter here). private static AlarmConditionSnapshot Snapshot(bool active = false) => @@ -182,10 +216,12 @@ public sealed class DeferredAddressSpaceSinkTests private sealed class SurgicalRecordingSink : IOpcUaAddressSpaceSink, ISurgicalAddressSpaceSink { - /// Gets or sets the value returns. + /// Gets or sets the value + return. public bool Result { get; set; } = true; /// Gets the recorded surgical calls (incl. the FB-7 DataType/array-shape args). public List<(string NodeId, bool Writable, string? Historian, string DataType, bool IsArray, uint? ArrayLength)> SurgicalCalls { get; } = new(); + /// Gets the recorded surgical folder-rename calls (OpcUaServer-001). + public List<(string FolderNodeId, string DisplayName)> FolderRenameCalls { get; } = new(); /// public bool UpdateTagAttributes(string variableNodeId, bool writable, string? historianTagname, string dataType, bool isArray, uint? arrayLength) @@ -194,6 +230,13 @@ public sealed class DeferredAddressSpaceSinkTests return Result; } + /// + public bool UpdateFolderDisplayName(string folderNodeId, string displayName) + { + FolderRenameCalls.Add((folderNodeId, displayName)); + return Result; + } + /// public void WriteValue(string nodeId, object? value, OpcUaQuality quality, DateTime sourceTimestampUtc) { } /// diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorRebuildTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorRebuildTests.cs index 0a414439..b9afe52b 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorRebuildTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorRebuildTests.cs @@ -249,12 +249,80 @@ public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase ctx.SaveChanges(); } - private sealed class RecordingSink : IOpcUaAddressSpaceSink + /// OpcUaServer-001 — a deploy whose ONLY change is a UNS Area rename (the area Name differs; + /// no equipment/driver/alarm/tag delta) must NOT short-circuit at the actor's IsEmpty gate: the second + /// RebuildAddressSpace drives the in-place folder display-name refresh (a surgical + /// UpdateFolderDisplayName call) without a full rebuild. Proves the planner→applier→sink chain + /// reaches the apply path through the actor for a rename-only redeploy. + [Fact] + public void Rebuild_with_area_rename_only_updates_folder_in_place_without_rebuild() + { + var db = NewInMemoryDbFactory(); + var sink = new RecordingSink(); + var applier = new AddressSpaceApplier(sink, NullLogger.Instance); + var dep1 = SeedAreaDeployment(db, areaName: "Plant North"); + + var actor = Sys.ActorOf(OpcUaPublishActor.PropsForTests( + sink: sink, dbFactory: db, applier: applier)); + + // First deploy: the area folder is materialised with the OLD name (one rebuild). + actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId(), new DeploymentId(dep1))); + AwaitAssert(() => sink.RebuildCalls.ShouldBe(1), duration: TimeSpan.FromSeconds(2)); + + // Second deploy: ONLY the area Name changed — a rename. The actor must reach the apply path and + // drive a surgical in-place folder rename (NOT a rebuild, NOT a no-op). + var dep2 = SeedAreaDeployment(db, areaName: "Plant South"); + actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId(), new DeploymentId(dep2))); + + AwaitAssert(() => + { + sink.FolderRenameCalls.ShouldContain(("area-1", "Plant South")); + }, duration: TimeSpan.FromSeconds(2)); + sink.RebuildCalls.ShouldBe(1); // still 1 — the rename did NOT force a second full rebuild + } + + /// Seal a deployment whose area carries , with a line + one + /// equipment under it. The equipment makes the FIRST deploy a structural rebuild (so the area folder is + /// materialised); a later redeploy that changes ONLY the area Name is then a pure rename. Returns the + /// new DeploymentId so the test can target THAT artifact (apply-time, not-yet-sealed semantics). + private static Guid SeedAreaDeployment(IDbContextFactory dbFactory, string areaName) + { + var artifact = JsonSerializer.SerializeToUtf8Bytes(new + { + UnsAreas = new[] { new { UnsAreaId = "area-1", ClusterId = "c1", Name = areaName } }, + UnsLines = new[] { new { UnsLineId = "line-1", UnsAreaId = "area-1", Name = "Cell A" } }, + Equipment = new[] { new { EquipmentId = "eq-1", DriverInstanceId = (string?)null, UnsLineId = "line-1", Name = "Pump-1", MachineCode = "EQ-1" } }, + DriverInstances = Array.Empty(), + Namespaces = Array.Empty(), + Tags = Array.Empty(), + ScriptedAlarms = Array.Empty(), + }); + + var id = Guid.NewGuid(); + using var ctx = dbFactory.CreateDbContext(); + ctx.Deployments.Add(new Deployment + { + DeploymentId = id, + RevisionHash = new string('c', 64), + Status = DeploymentStatus.Sealed, + CreatedBy = "test", + SealedAtUtc = DateTime.UtcNow, + ArtifactBlob = artifact, + }); + ctx.SaveChanges(); + return id; + } + + private sealed class RecordingSink : IOpcUaAddressSpaceSink, ISurgicalAddressSpaceSink { /// Gets the list of recorded sink calls. public ConcurrentQueue Calls { get; } = new(); /// Gets or sets the count of rebuild address space calls. public int RebuildCalls; + /// Gets the queue of surgical in-place folder display-name update calls (OpcUaServer-001). + public ConcurrentQueue<(string FolderNodeId, string DisplayName)> FolderRenameQueue { get; } = new(); + /// Gets the list of recorded surgical folder display-name update calls. + public List<(string FolderNodeId, string DisplayName)> FolderRenameCalls => FolderRenameQueue.ToList(); /// Records a value write call. /// The OPC UA node ID. /// The value to write. @@ -293,5 +361,19 @@ public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase => Calls.Enqueue($"EV:{variableNodeId}"); /// Records a rebuild address space call. public void RebuildAddressSpace() => Interlocked.Increment(ref RebuildCalls); + /// Records a surgical in-place tag-attribute update (always succeeds in this recording sink). + public bool UpdateTagAttributes(string variableNodeId, bool writable, string? historianTagname, string dataType, bool isArray, uint? arrayLength) + { + Calls.Enqueue($"UT:{variableNodeId}"); + return true; + } + /// Records a surgical in-place folder display-name update (always succeeds in this recording sink). + /// The folder node ID to update in place. + /// The new display name to apply. + public bool UpdateFolderDisplayName(string folderNodeId, string displayName) + { + FolderRenameQueue.Enqueue((folderNodeId, displayName)); + return true; + } } }