diff --git a/archreview/03-server-runtime.md b/archreview/03-server-runtime.md index 66b212fe..dc2e934f 100644 --- a/archreview/03-server-runtime.md +++ b/archreview/03-server-runtime.md @@ -25,7 +25,7 @@ Every finding from the 2026-07-08 review, verified against the actual code (not | S9 | No server-cert renewal/expiry monitoring | **STILL OPEN.** `OpcUaApplicationHost.cs:305-317` unchanged. | | S10 | SDK start failure swallowed; node runs with Null sinks, invisible to /health | **STILL OPEN.** `OtOpcUaServerHostedService.cs:110-129` unchanged. | | S11 | `AuditWriterActor` unbounded buffer, drops batches on outage | **STILL OPEN** (and still moot — see U3). | -| P1 | Any structural deploy change → full rebuild severs all monitored items | **STILL OPEN.** `AddressSpaceApplier.cs:134-154` / `OtOpcUaNodeManager.cs:1690-1736` unchanged. The U2 reflection guard (now merged) was the declared prerequisite gate for the surgical pure-adds work; the work itself has not started. | +| P1 | Any structural deploy change → full rebuild severs all monitored items | **REMEDIATED (offline; live gates deferred)** on `r2/07-surgical-pure-adds` (R2-07 Phases 1–3, `bb226f45`…`e2b47638`). New pure `AddressSpaceChangeClassifier` routes each deploy delta to its minimal mutation in `AddressSpaceApplier.Apply`: **PureAdd/AttributeOnly** skip `RebuildAddressSpace` (the idempotent Materialise passes add exactly the new nodes; `AnnounceAddedNodes` fires a Part 3 NodeAdded per parent), **PureRemove/AddRemoveMix** tear down only the affected subtree in place via 3 new `ISurgicalAddressSpaceSink` remove members (guard-first, negative-control observed) + `OtOpcUaNodeManager` scoped teardown with `NodeDeleted` model-change, and **Rebuild** stays the default-closed safety valve (any node-affecting change, plus every surgical false/throw ⇒ full-rebuild ratchet). So an add-one-tag / remove-one-tag / mixed deploy no longer severs unrelated subscriptions. OpcUaServer.Tests 336/336, Runtime.Tests 398/398, Commons forwarding guard green. **Deferred:** the over-the-wire `SubscriptionSurvivalTests` (add/remove/mixed) + the docker-dev live-`/run` gates (`kind=PureAdd/PureRemove/AddRemoveMix, rebuild=False`) — authored + compile-green, run in the serial heavy pass. | | P2 | Per-value global-lock write, no batching | **STILL OPEN.** `OtOpcUaNodeManager.cs:266-281` unchanged. Same guard-unblocked, not-started status as P1. | | P3 | HistoryRead-Events unpaged, `0` → `int.MaxValue` | **STILL OPEN.** `OtOpcUaNodeManager.cs:1914-1954` unchanged. | | P4 | Deploy re-runs four Materialise passes (low) | **STILL OPEN.** `OpcUaPublishActor.cs:338-354` unchanged. | diff --git a/archreview/plans/R2-07-surgical-pure-adds-plan.md b/archreview/plans/R2-07-surgical-pure-adds-plan.md index ff9b466f..090533f3 100644 --- a/archreview/plans/R2-07-surgical-pure-adds-plan.md +++ b/archreview/plans/R2-07-surgical-pure-adds-plan.md @@ -369,3 +369,32 @@ Tasks are bite-sized TDD increments. **Classification:** `high-risk` = touches t - TDD: mixed-deploy integration case (subscribe A; deploy −B/+C; A alive, B gone, C browsable). Live: one deploy carrying +1/−1 on docker-dev (both centrals), log `kind=AddRemoveMix, rebuild=False`, survivor subscription uninterrupted. Update STATUS.md + the P1 finding row. - Commit: `test+docs: mixed-deploy survival, live evidence, STATUS/P1 close-out (R2-07 T14)` - **── PHASE 3 / PLAN COMPLETE ──** + +--- + +## Execution deviations (R2-07) + +- **Applier phase-pin tests were flipped, not "untouched-green".** The plan framed T2/T11/T13 as + behavior-preserving ("every pre-existing applier test untouched-green"). In reality several pre-existing + tests directly encoded the OLD full-rebuild-on-any-topology-change behavior — `Added_equipment_*`, + `Removed_equipment_*`, `Surgical_eligible_tag_delta_mixed_with_added_equipment_rebuilds`, + `Folder_rename_mixed_with_added_equipment_rebuilds`, `Removed_equipment_tags_and_virtual_tags_only_rebuild_*` + — and were flipped to assert the new PureAdd/PureRemove/AddRemoveMix no-rebuild behavior (each doc-commented + "Supersedes the pre-R2-07 … pin"). The genuinely-behavior-preserving tests (surgical attribute path, folder + renames, changed-non-surgical ⇒ rebuild) stayed green untouched. +- **Incidental `RebuildCalled` assertions repointed.** Tests whose *subject* was not the rebuild decision but + which happened to assert `RebuildCalled==true` on a now-PureAdd plan were adjusted: + `AddressSpaceApplierFailureSurfaceTests` (rebuild-failed surfacing) + `OpcUaPublishActorApplyFailureTests` + (apply.failed meter) were repointed to a **rebuild-forcing ChangedEquipment plan** so they still exercise + the SafeRebuild path; `AddressSpaceApplierProvisioningTests` `RebuildCalled` assertions became + `RebuildCalled==false` + `AddedNodes>0` (provisioning still fires on a pure-add). +- **T1 predicate references:** the two eligibility predicates moved to `AddressSpaceChangeClassifier` as + `internal static`; the applier references them directly (`AddressSpaceChangeClassifier.TagDeltaIsSurgicalEligible`) + rather than keeping thin delegating wrappers. +- **Integration + live gates deferred-live (memory constraint).** Per the executor's memory budget + (`*.IntegrationTests` leak ~16 GB/run, other plans running concurrently), the over-the-wire + `SubscriptionSurvivalTests` (add/remove/mixed) were **authored + `dotnet build`-compile-verified but NOT + run**, and T6/T12/T14 docker-dev live-`/run` gates were marked `deferred-live`. These are the F10b + inertness nets and MUST be run in the serial heavy pass before merge/trust. +- **Worktree entry:** `EnterWorktree` refused to switch from the repo root; all work was done against the + worktree via absolute paths + `git -C`, never touching the main checkout. diff --git a/archreview/plans/R2-07-surgical-pure-adds-plan.md.tasks.json b/archreview/plans/R2-07-surgical-pure-adds-plan.md.tasks.json index 27f9622c..2d9cd149 100644 --- a/archreview/plans/R2-07-surgical-pure-adds-plan.md.tasks.json +++ b/archreview/plans/R2-07-surgical-pure-adds-plan.md.tasks.json @@ -5,92 +5,127 @@ { "id": "T1", "subject": "Phase 1: AddressSpaceChangeClassifier + table-driven tests (move TagDeltaIsSurgicalEligible/VtagDeltaIsNodeIrrelevant)", - "status": "pending", + "status": "completed", "blockedBy": [] }, { "id": "T2", - "subject": "Phase 1: route AddressSpaceApplier.Apply through the classifier — PureAdd skips rebuild (high-risk)", - "status": "pending", - "blockedBy": ["T1"] + "subject": "Phase 1: route AddressSpaceApplier.Apply through the classifier \u2014 PureAdd skips rebuild (high-risk)", + "status": "completed", + "blockedBy": [ + "T1" + ] }, { "id": "T3", "subject": "Phase 1: ComputeAddAnnouncements + AnnounceAddedNodes on the applier (dedup parent NodeAdded announces)", - "status": "pending", - "blockedBy": ["T1"] + "status": "completed", + "blockedBy": [ + "T1" + ] }, { "id": "T4a", "subject": "Phase 1: idempotent MaterialiseAlarmCondition (skip-if-present-and-same-kind) (high-risk)", - "status": "pending", - "blockedBy": ["T1"] + "status": "completed", + "blockedBy": [ + "T1" + ] }, { "id": "T4b", "subject": "Phase 1: OpcUaPublishActor calls AnnounceAddedNodes after the Materialise passes on non-rebuild applies (high-risk)", - "status": "pending", - "blockedBy": ["T2", "T3", "T4a"] + "status": "completed", + "blockedBy": [ + "T2", + "T3", + "T4a" + ] }, { "id": "T5", - "subject": "Phase 1: over-the-wire SubscriptionSurvivalTests — subscribe, pure-add, item survives + new node browsable", - "status": "pending", - "blockedBy": ["T4b"] + "subject": "Phase 1: over-the-wire SubscriptionSurvivalTests \u2014 subscribe, pure-add, item survives + new node browsable", + "status": "deferred-live", + "blockedBy": [ + "T4b" + ], + "note": "authored + compiles (dotnet build green); NOT run \u2014 *.IntegrationTests suite leaks ~16GB/run per the memory constraint. MUST run in the serial heavy integration pass (single-test, boots one in-process server + client)." }, { "id": "T6", - "subject": "Phase 1: MANDATORY live-/run gate on docker-dev (rebuild BOTH centrals; Client.CLI subscribe survives +1-tag deploy; kind=PureAdd rebuild=False) — Phase 1 shippable boundary", - "status": "pending", - "blockedBy": ["T5"] + "subject": "Phase 1: MANDATORY live-/run gate on docker-dev (rebuild BOTH centrals; Client.CLI subscribe survives +1-tag deploy; kind=PureAdd rebuild=False) \u2014 Phase 1 shippable boundary", + "status": "deferred-live", + "blockedBy": [ + "T5" + ], + "note": "docker-dev live /run surgical-path; serial heavy pass \u2014 F10b inertness risk, MUST run (subscribe survives +1-tag deploy; central log kind=PureAdd, rebuild=False; new node browsable/Good)." }, { "id": "T7", - "subject": "Phase 2: add RemoveVariableNode/RemoveAlarmConditionNode/RemoveEquipmentSubtree to ISurgicalAddressSpaceSink + DeferredAddressSpaceSink forwarding — guard-first with observed negative control (high-risk)", - "status": "pending", - "blockedBy": ["T6"] + "subject": "Phase 2: add RemoveVariableNode/RemoveAlarmConditionNode/RemoveEquipmentSubtree to ISurgicalAddressSpaceSink + DeferredAddressSpaceSink forwarding \u2014 guard-first with observed negative control (high-risk)", + "status": "completed", + "blockedBy": [ + "T6" + ] }, { "id": "T8", "subject": "Phase 2: OtOpcUaNodeManager.RemoveVariableNode + BuildNodesRemovedModelChange (NodeDeleted) (high-risk)", - "status": "pending", - "blockedBy": ["T7"] + "status": "completed", + "blockedBy": [ + "T7" + ] }, { "id": "T9", "subject": "Phase 2: OtOpcUaNodeManager.RemoveAlarmConditionNode (+ native-flag cleanup) (high-risk)", - "status": "pending", - "blockedBy": ["T7"] + "status": "completed", + "blockedBy": [ + "T7" + ] }, { "id": "T10", "subject": "Phase 2: OtOpcUaNodeManager.RemoveEquipmentSubtree (descendants + notifier demotion) + SdkAddressSpaceSink forwards (high-risk)", - "status": "pending", - "blockedBy": ["T8", "T9"] + "status": "completed", + "blockedBy": [ + "T8", + "T9" + ] }, { "id": "T11", - "subject": "Phase 2: applier PureRemove arm — terminal Bad/RemovedConditionState writes, equipment subsumption, false-to-rebuild ratchet (high-risk)", - "status": "pending", - "blockedBy": ["T10"] + "subject": "Phase 2: applier PureRemove arm \u2014 terminal Bad/RemovedConditionState writes, equipment subsumption, false-to-rebuild ratchet (high-risk)", + "status": "completed", + "blockedBy": [ + "T10" + ] }, { "id": "T12", - "subject": "Phase 2: remove-side subscription-survival integration test + live-/run remove gate (kind=PureRemove rebuild=False) — Phase 2 shippable boundary", - "status": "pending", - "blockedBy": ["T11"] + "subject": "Phase 2: remove-side subscription-survival integration test + live-/run remove gate (kind=PureRemove rebuild=False) \u2014 Phase 2 shippable boundary", + "status": "deferred-live", + "blockedBy": [ + "T11" + ], + "note": "remove-side SubscriptionSurvivalTests authored + compiles (dotnet build green); NOT run \u2014 *.IntegrationTests memory constraint. Live docker-dev pure-remove gate (survivor subscription uninterrupted; central log kind=PureRemove, rebuild=False; removed node reads BadNodeIdUnknown) MUST run in the serial heavy pass \u2014 F10b inertness risk." }, { "id": "T13", - "subject": "Phase 3: AddRemoveMix routing — removes-then-adds surgically, same-NodeId remove-then-recreate ordering test (high-risk)", - "status": "pending", - "blockedBy": ["T12"] + "subject": "Phase 3: AddRemoveMix routing \u2014 removes-then-adds surgically, same-NodeId remove-then-recreate ordering test (high-risk)", + "status": "completed", + "blockedBy": [ + "T12" + ] }, { "id": "T14", - "subject": "Phase 3: mixed-deploy integration test + live-/run mixed gate + STATUS.md/P1 close-out — plan complete", - "status": "pending", - "blockedBy": ["T13"] + "subject": "Phase 3: mixed-deploy integration test + live-/run mixed gate + STATUS.md/P1 close-out \u2014 plan complete", + "status": "deferred-live", + "blockedBy": [ + "T13" + ], + "note": "mixed-deploy SubscriptionSurvivalTests authored + compiles (dotnet build green); docs close-out DONE (STATUS.md + 03/P1 marked REMEDIATED + plan deviations). Live docker-dev mixed gate (one deploy +1/-1; kind=AddRemoveMix, rebuild=False; survivor uninterrupted) MUST run in the serial heavy pass \u2014 F10b inertness risk." } ] } diff --git a/archreview/plans/STATUS.md b/archreview/plans/STATUS.md index 45e9f4cf..89a6b68b 100644 --- a/archreview/plans/STATUS.md +++ b/archreview/plans/STATUS.md @@ -179,6 +179,8 @@ what the unit leg cannot; (c) migrate the 3 xunit v2 holdouts once Akka ships an | **R2-08** | 03/S2 (High, action #8), 03/S3 (High) | `r2/08-ldap-historyread-async` (off `1a698cbb`) | `ce6d9e21`…`` (14 task commits) | **Code-complete, offline-verified.** **S2 (LDAP async/timeout):** the SDK raises the impersonation callback synchronously under a `SessionManager`-wide `m_eventLock` (verified vs. 1.5.378 — a hung bind serializes ALL activations), so the fix is entirely at the `LdapOpcUaUserAuthenticator` boundary: five new `Security:Ldap` keys (`ConnectionTimeoutMs`=10000 projected into the shared lib, `AuthTimeoutMs`=15000 hard whole-flow `WaitAsync` bound → fail-closed "timed out", `MaxConcurrentAuthentications`=8 `SemaphoreSlim` with release-on-core-completion so orphaned binds still count, `OutageFailureThreshold`=3/`OutageCooldownSeconds`=15 directory-outage circuit → fast-deny "unavailable", half-open probe) + `LdapAuthResult.IsSystemFailure` seam (mapped from `ServiceAccountBindFailed`) distinguishing outage from credential-deny + `LdapOptionsValidator` rules + `HandleImpersonation` contract doc + `docs/security.md`. Stall-repro RED→GREEN; offline outage-sim through the real `HandleImpersonation` against an unroutable host. **S3 (HistoryRead fan-out):** all four HistoryRead arms async-ified (`ServeNodeAsync`/`ServeRawPagedAsync`/`ServeEventsAsync`, OCE→`BadTimeout`) then bounded per-node fan-out (`RunBounded` semaphore + single arm-boundary bridge) + per-request deadline CTS (`HistoryReadDeadline`=60s) + process-wide lazy batch limiter (`MaxConcurrentHistoryReadBatches`=16 → flood degrades to `BadTooManyOperations`); 16×4=64 in-flight cap; three new `ServerHistorian` keys + node-manager props + hosted-service wiring + `docs/Historian.md`/CLAUDE.md tables. Sequential-serving RED→GREEN. Host.IntegrationTests (LDAP resilience) 13/13, Security.Tests 11/11, Runtime.Tests options 11/11, OpcUaServer.Tests HistoryRead 30/30. **Deferred (live):** S2 docker-dev unreachable-LDAP + GLAuth outage/recovery cycle; S3 `Category=LiveIntegration` gateway soak. | | **R2-11** | 01/C-1, 01/P-1 + 05/CONV-2, UNDER-1, UNDER-6 | `r2/11-tagconfig-consolidation` (off `46fedda3`) | first `2e14fe1f` … (24 task commits) | **Code-complete, offline-verified.** **Part 1 (C-1/P-1):** single `TagConfigIntent.Parse` + `DeviceConfigIntent` in Commons replace the four byte-parity TagConfig parse copies (composer/artifact/walker/DraftValidator) — composer now parses once per tag (P-1); a permanent `Runtime.Tests/TagConfigCorpusParityTests` (30-blob corpus) guards compose≡decode; `grep "MUST parse identically" src/` = 0. **Part 2 (CONV-2):** shared strict-capable `TagConfigJson` readers in Core.Abstractions replace the six copy-pasted `ReadEnum`s; all six driver parsers gain `Inspect()` warnings + honour the `writable` key (FOCAS **forced read-only**, 05/UNDER-1); FOCAS equipment tags run the capability-matrix pre-flight (05/UNDER-6); Modbus probe parses the factory DTO shape (`timeoutMs` no longer dropped). New `EquipmentTagConfigInspector` in ControlPlane + `AdminOperationsActor` deploy-gate wiring with `Deployment:TagConfigValidationMode` (**Warn default | Error opt-in**), proven through the actor (F10b consuming test). AdminUI: `writable` checkbox in the six typed editors. **Phase-C follow-up (deferred, recorded here):** flip the runtime parsers to strict (typo'd enum ⇒ `BadNodeIdUnknown` instead of wrong-width `Good`) once fleets have run `Error` mode clean — behavior change deliberately not shipped in this plan. **Deferred (T22/T24):** docker-dev `/run` of one typed editor's Writable checkbox + whole-solution serial sweep. | +| **R2-07 (Phases 1+2+3)** | 03/P1 (action #7, surgical pure-adds/removes/mixed) | `r2/07-surgical-pure-adds` (off `1a698cbb`) | `bb226f45`…`e2b47638` (T1–T14) | **Phases 1+2 code-complete, offline-verified.** New pure `AddressSpaceChangeClassifier` (Empty/AttributeOnly/PureAdd/PureRemove/AddRemoveMix/Rebuild; first-match-wins, default-closed Rebuild; the F10b tag/vtag surgical-eligibility predicates moved here as `internal static`). `AddressSpaceApplier.Apply` routes by kind. **Phase 1 (PureAdd/AttributeOnly):** skip `RebuildAddressSpace` (the idempotent Materialise passes create exactly the added nodes; existing subscriptions survive); adds announced via `ComputeAddAnnouncements`+`AnnounceAddedNodes` (dedup affected-parent → existing guard-covered `RaiseNodesAddedModelChange`), fired by `OpcUaPublishActor` after the 4 passes gated on `!outcome.RebuildCalled`; `MaterialiseAlarmCondition` made idempotent (skip-if-present-and-same-kind keeps the condition NodeState alive). **NO new sink members in Phase 1.** **Phase 2 (PureRemove):** 3 new `ISurgicalAddressSpaceSink` members (`RemoveVariableNode`/`RemoveAlarmConditionNode`/`RemoveEquipmentSubtree`) — guard-first with the **observed negative control** (`DeferredSinkForwardingReflectionTests` fails member-named until `DeferredAddressSpaceSink` forwards) + `SdkAddressSpaceSink` forwards + `OtOpcUaNodeManager` in-place teardown (scoped map cleanup, notifier demotion via `RemoveRootNotifier`, `BuildNodesRemovedModelChange` verb=NodeDeleted reported outside `Lock`). Applier `PureRemove` arm: terminal Bad / `RemovedConditionState` writes (closing the removed-alarm-tag today-gap) → subsumption partition (removed-equipment children folded into the subtree remove) → in-place removes → any false/throw ⇒ full-rebuild ratchet; non-surgical sink ⇒ rebuild. **Phase 3 (AddRemoveMix):** removes-then-adds within one apply — the remove pass runs in `Apply` (no rebuild), then the publish actor's Materialise passes create the adds + `AnnounceAddedNodes` announces them; an id reused across the remove+add sets is remove-then-recreated as a fresh NodeState (correct — a different tag). **Guard stays green (Commons.Tests 17/17), OpcUaServer.Tests 336/336, Runtime.Tests 398/398.** **03/P1 marked REMEDIATED in `archreview/03-server-runtime.md`.** **Deferred-live (T5, T6, T12, T14):** `SubscriptionSurvivalTests` add + remove + mixed cases (authored + compile-green; NOT run — `*.IntegrationTests` memory constraint) and the docker-dev live-`/run` add/remove/mixed gates (`kind=PureAdd/PureRemove/AddRemoveMix, rebuild=False`; survivor subscription uninterrupted; removed node → `BadNodeIdUnknown`) — MUST run in the serial heavy pass (F10b inertness risk). | + **Report anchor drifts to fold into the next report refresh** (from the R2-02 verification pass): (1) `01-core-composition.md` U-6 anchor `DriverResilienceOptions.cs:23-25` → the two bulkhead properties sat at `:23` and `:29` (now deleted); (2) `01-core-composition.md` U-6 anchor 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 701f8a55..f70d8be2 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/DeferredAddressSpaceSink.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/DeferredAddressSpaceSink.cs @@ -68,4 +68,21 @@ public sealed class DeferredAddressSpaceSink : IOpcUaAddressSpaceSink, ISurgical public bool UpdateFolderDisplayName(string folderNodeId, string displayName) => _inner is ISurgicalAddressSpaceSink surgical && surgical.UpdateFolderDisplayName(folderNodeId, displayName); + + // R2-07 T7 — surgical remove forwards. Same capability-sniffing pattern as UpdateTagAttributes / + // UpdateFolderDisplayName: forward when the inner sink supports the surgical capability; return false + // otherwise (before the real SdkAddressSpaceSink is swapped in, or any non-surgical inner) so the caller + // (AddressSpaceApplier) falls back to a full rebuild. Without these forwards the surgical remove path is + // inert on every driver-role host — the F10b / PR#423 trap the reflection guard exists to catch. + /// + public bool RemoveVariableNode(string variableNodeId) + => _inner is ISurgicalAddressSpaceSink surgical && surgical.RemoveVariableNode(variableNodeId); + + /// + public bool RemoveAlarmConditionNode(string alarmNodeId) + => _inner is ISurgicalAddressSpaceSink surgical && surgical.RemoveAlarmConditionNode(alarmNodeId); + + /// + public bool RemoveEquipmentSubtree(string equipmentNodeId) + => _inner is ISurgicalAddressSpaceSink surgical && surgical.RemoveEquipmentSubtree(equipmentNodeId); } 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 7dc685f6..f4033030 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/ISurgicalAddressSpaceSink.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/ISurgicalAddressSpaceSink.cs @@ -34,4 +34,34 @@ public interface ISurgicalAddressSpaceSink /// 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); + + /// Remove ONE existing value-variable node IN PLACE (detach from its parent, drop it from the + /// predefined-node set, drop any historized-tagname registration), then raise a Part 3 + /// GeneralModelChangeEvent (verb=NodeDeleted) outside the lock — so subscribers of OTHER nodes are + /// untouched and their MonitoredItems survive. The caller has already written a final Bad quality to the + /// node so in-flight MonitoredItems on the removed node observe the removal; after the node is gone, + /// re-subscription attempts get BadNodeIdUnknown from the SDK. Returns false when the node id is unknown + /// (the node-manager maps drifted from the plan — caller falls back to a full rebuild). + /// The folder-scoped node id of the variable to remove in place. + /// True when the node was removed; false when the id is unknown (caller rebuilds). + bool RemoveVariableNode(string variableNodeId); + + /// Remove ONE existing Part 9 alarm-condition node IN PLACE (detach, drop from the + /// predefined-node set, clear the native-alarm flag), then raise NodeDeleted outside the lock. The + /// caller has already written the terminal RemovedConditionState (Retain=false) so a removed condition + /// stops replaying on ConditionRefresh. Returns false when the condition id is unknown (caller + /// rebuilds). + /// The alarm condition node id (== ScriptedAlarmId, or a native alarm tag's folder-scoped id). + /// True when the condition was removed; false when the id is unknown (caller rebuilds). + bool RemoveAlarmConditionNode(string alarmNodeId); + + /// Remove an equipment folder AND its entire descendant subtree (sub-folders, value variables, + /// condition nodes) IN PLACE: per-descendant map/registration cleanup (variables, historized tagnames, + /// conditions, native flags), notifier demotion (sever the Server↔folder root-notifier ref + + /// notifier-folder + event-notifier-source registrations for the equipment id), then one NodeDeleted for + /// the equipment folder outside the lock. Returns false when the folder id is unknown (caller + /// rebuilds). + /// The equipment folder node id whose entire subtree to remove. + /// True when the subtree was removed; false when the id is unknown (caller rebuilds). + bool RemoveEquipmentSubtree(string equipmentNodeId); } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceApplier.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceApplier.cs index 5d63a6c0..a4f72c49 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceApplier.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceApplier.cs @@ -7,22 +7,31 @@ namespace ZB.MOM.WW.OtOpcUa.OpcUaServer; /// /// Side-effecting orchestrator over . Drives an /// to materialise the diff between two -/// snapshots: +/// snapshots, routing each delta class to its MINIMAL mutation +/// (R2-07 03/P1) via instead of rebuilding the whole +/// address space for any topology change (which would kill every client's server-wide subscriptions): /// /// -/// RemovedEquipment / RemovedAlarms — write Bad-quality on every removed -/// node id then call RebuildAddressSpace at the end so the sink can -/// actually tear down the OPC UA folders + variables. -/// AddedEquipment / AddedAlarms — same Rebuild trigger (real SDK NodeManager -/// will repopulate from the persisted artifact). For now we record the work. -/// ChangedEquipment / ChangedAlarms — record what changed; the SDK adapter -/// that lands in F10b will decide between in-place property writes and -/// tear-down + rebuild. +/// PureAdd — no rebuild. OpcUaPublishActor's idempotent Materialise passes +/// create exactly the added folders/variables/conditions (no-op'ing existing nodes), then +/// AnnounceAddedNodes raises a Part 3 NodeAdded per affected parent so +/// model-aware clients re-browse. Existing MonitoredItems are untouched. +/// AttributeOnly — no rebuild. Surgical-eligible changed tags + UNS folder renames +/// are applied IN PLACE via (F10b). Any false +/// or throw from a surgical call falls back to a full rebuild (safe default). +/// PureRemove — scoped in-place teardown of only the removed subtree (writes a +/// terminal Bad / RemovedConditionState first so in-flight MonitoredItems observe the +/// removal). Maps to a full rebuild until R2-07 Phase 2 ships. +/// AddRemoveMix — removes-then-adds within one apply. Maps to a full rebuild until +/// R2-07 Phase 3 ships. +/// Rebuild — the default-closed safety valve for any node-affecting change +/// (ChangedEquipment / ChangedAlarms / non-surgical ChangedEquipmentTags / node-relevant +/// ChangedEquipmentVirtualTags), so the worst outcome of any misclassification is today's +/// full-rebuild behaviour. /// /// -/// This is the side-effecting layer deferred to F14. It stays pure-of-SDK so -/// production binds a real SDK sink, dev/Mac binds , -/// and tests can capture every call. +/// Stays pure-of-SDK so production binds a real SDK sink, dev/Mac binds +/// , and tests can capture every call. /// public sealed class AddressSpaceApplier { @@ -111,38 +120,36 @@ public sealed class AddressSpaceApplier plan.AddedEquipmentTags.Count + plan.AddedEquipmentVirtualTags.Count; - // Any add / remove / in-place CHANGE of Equipment, ScriptedAlarm, Equipment tag, or Equipment - // VirtualTag topology requires a real address-space rebuild — the materialise passes re-derive - // every node from the composition, so a changed-only deploy (e.g. a renamed equipment, a - // re-severitied alarm, a flipped tag dataType) must still rebuild or the running server keeps - // the stale node. - // ChangedDrivers is deliberately EXCLUDED: a driver-instance config change doesn't touch the - // address-space topology — it routes through DriverHostActor's spawn-plan in Runtime, which - // re-spawns the affected driver actor without re-materialising any nodes. - // F10b (vtag skip): a CHANGED VirtualTag whose ONLY differences are Expression/DependencyRefs/ - // Historize is node-IRRELEVANT (see VtagDeltaIsNodeIrrelevant) — its materialised node is - // byte-identical and the vtag engine adopts those edits via VirtualTagHostActor's INDEPENDENT - // respawn (DriverHostActor → ApplyVirtualTags), so it skips the rebuild and PRESERVES every - // client's server-wide subscriptions. Any structural / node-affecting vtag change (Name/ - // FolderPath/DataType) — or any non-vtag change anywhere — still forces a full rebuild. - // F10b (surgical tag write): a CHANGED equipment tag whose ONLY differences are Writable / - // IsHistorized / HistorianTagname / DataType / IsArray / ArrayLength (a plain value variable — no - // alarm condition node) can be updated IN PLACE on the existing node via - // ISurgicalAddressSpaceSink.UpdateTagAttributes (see TagDeltaIsSurgicalEligible), avoiding the full - // rebuild and preserving subscriptions. The shape fields (DataType/IsArray/ArrayLength) are now - // surgical because the sink swaps DataType + ValueRank + ArrayDimensions in place and raises a - // GeneralModelChangeEvent. Any other tag difference (FullName/Name/DriverInstanceId/identity/alarm) — - // or a sink that lacks the surgical capability, or a node that turns out missing — falls back to a - // full rebuild (safe default). - var structuralRebuild = - plan.AddedEquipment.Count > 0 || plan.RemovedEquipment.Count > 0 || plan.ChangedEquipment.Count > 0 || - plan.AddedAlarms.Count > 0 || plan.RemovedAlarms.Count > 0 || plan.ChangedAlarms.Count > 0 || - plan.AddedEquipmentTags.Count > 0 || plan.RemovedEquipmentTags.Count > 0 || - plan.ChangedEquipmentTags.Any(d => !TagDeltaIsSurgicalEligible(d)) || - plan.AddedEquipmentVirtualTags.Count > 0 || plan.RemovedEquipmentVirtualTags.Count > 0 || - plan.ChangedEquipmentVirtualTags.Any(d => !VtagDeltaIsNodeIrrelevant(d)); + // R2-07 03/P1 — classify the plan and route each delta class to its MINIMAL mutation instead of + // rebuilding the world for any topology change. AddressSpaceChangeClassifier is a pure policy over + // the planner's diff (the planner stays a pure diff). The routing here: + // • PureAdd / AttributeOnly ⇒ NO rebuild. The idempotent Materialise passes in + // OpcUaPublishActor.HandleRebuild create exactly the added nodes (no-op'ing existing ones), so + // every client subscription server-wide survives; coincident surgical tag deltas + folder + // renames are applied IN PLACE via ISurgicalAddressSpaceSink below (any false/throw there falls + // back to a full rebuild — the F10b contract). + // • PureRemove ⇒ NO full rebuild. The removed nodes are torn down IN PLACE, scoped to the affected + // subtree, via the ISurgicalAddressSpaceSink remove members below (each preceded by a terminal + // Bad / RemovedConditionState write so in-flight MonitoredItems observe the removal); subscribers + // of OTHER nodes are untouched. Any false/throw from a remove ⇒ full-rebuild fallback (ratchet). + // • AddRemoveMix ⇒ NO full rebuild. Removes-then-adds within one apply: the removed nodes are torn + // down IN PLACE here (the PureRemove pass), then OpcUaPublishActor's idempotent Materialise passes + // create the added nodes and AnnounceAddedNodes announces them — natural remove-then-recreate + // order even when an id is reused across the remove + add sets (the recreated node is a fresh + // NodeState, correct — it is a different tag). Any remove false/throw ⇒ full-rebuild fallback. + // • Rebuild ⇒ the default-closed safety valve for any node-affecting change (ChangedEquipment / + // ChangedAlarms / non-surgical ChangedEquipmentTags / node-relevant ChangedEquipmentVirtualTags) + // — the classifier's rule 2, which also catches any future plan field that makes a changed + // record unequal, so the worst outcome of a misclassification is today's full rebuild. + // ChangedDrivers is node-inert (routes through DriverHostActor's spawn plan) — the classifier leaves + // a driver-only plan AttributeOnly, so it never rebuilds here. + var kind = AddressSpaceChangeClassifier.Classify(plan); + var mustRebuild = kind is AddressSpaceChangeKind.Rebuild; + // PureRemove + AddRemoveMix both run the in-place remove pass below (AddRemoveMix's adds are handled + // afterward by the publish actor's Materialise passes + announce). + var hasRemovePass = kind is AddressSpaceChangeKind.PureRemove or AddressSpaceChangeKind.AddRemoveMix; - var surgicalTagDeltas = plan.ChangedEquipmentTags.Where(TagDeltaIsSurgicalEligible).ToList(); + var surgicalTagDeltas = plan.ChangedEquipmentTags.Where(AddressSpaceChangeClassifier.TagDeltaIsSurgicalEligible).ToList(); // 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 @@ -151,12 +158,12 @@ public sealed class AddressSpaceApplier var rebuilt = false; var rebuildFailed = false; - if (structuralRebuild) + if (mustRebuild) { rebuildFailed = !SafeRebuild(); rebuilt = true; } - else if (surgicalTagDeltas.Count > 0 || renamedFolders.Count > 0) + else if (surgicalTagDeltas.Count > 0 || renamedFolders.Count > 0 || hasRemovePass) { if (_sink is ISurgicalAddressSpaceSink surgical) { @@ -192,6 +199,15 @@ public sealed class AddressSpaceApplier } if (!ok) { allApplied = false; break; } } + // R2-07 Phase 2/3 — scoped remove teardown for PureRemove AND AddRemoveMix (runs after any + // coincident surgical attribute updates / renames succeeded). Terminal Bad / + // RemovedConditionState writes then in-place removes; any false/throw flips allApplied → the + // rebuild ratchet below. For AddRemoveMix the adds are materialised afterward by the publish + // actor's passes (remove-then-recreate order). + if (allApplied && hasRemovePass) + { + allApplied = ApplyPureRemove(surgical, plan, ts, ref failedNodes); + } if (!allApplied) { rebuildFailed = !SafeRebuild(); rebuilt = true; } } else @@ -203,8 +219,8 @@ public sealed class AddressSpaceApplier } _logger.LogInformation( - "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); + "AddressSpaceApplier: applied plan (kind={Kind}, added={Added}, removed={Removed}, changed={Changed}, surgicalTags={Surgical}, renamedFolders={Renamed}, rebuild={Rebuild})", + kind, addedCount, removedCount, changedCount, rebuilt ? 0 : surgicalTagDeltas.Count, rebuilt ? 0 : renamedFolders.Count, rebuilt); // After the address-space work has completed, auto-provision the historian for the added // historized tags. This is fully detached (fire-and-forget) and wrapped so it can NEVER block @@ -219,6 +235,165 @@ public sealed class AddressSpaceApplier return new AddressSpaceApplyOutcome(removedCount, addedCount, changedCount, rebuilt, rebuildFailed, failedNodes); } + /// + /// R2-07 Phase 2 — apply a PureRemove plan by tearing down ONLY the affected nodes IN PLACE (no full + /// rebuild). Removed equipment "own" their child tags/vtags/alarms, so any removed child whose + /// EquipmentId is itself in is SUBSUMED by the + /// subtree removal and skipped individually. For surviving-equipment removals: a removed value + /// variable gets a terminal Bad WriteValue then ; + /// a removed alarm-bearing tag (the pre-R2-07 today-gap: it got no terminal condition write) and a + /// removed scripted alarm get a terminal write then + /// ; each removed equipment gets one + /// . The terminal write is what an + /// in-flight MonitoredItem observes (a final Bad); after the node is gone, re-subscription gets + /// BadNodeIdUnknown. Returns false on the FIRST remove that reports the node unknown / throws + /// (the caller's one-way rebuild ratchet takes over), leaving no further surgical work attempted. + /// + /// The surgical sink to route removes through. + /// The PureRemove plan. + /// The timestamp for the terminal writes. + /// Accumulator for swallowed terminal-write failures (archreview 01/S-1). + /// True when every removal succeeded; false on the first failure (caller rebuilds). + private bool ApplyPureRemove(ISurgicalAddressSpaceSink surgical, AddressSpacePlan plan, DateTime ts, ref int failedNodes) + { + var removedEquipmentIds = new HashSet( + plan.RemovedEquipment.Select(e => e.EquipmentId), StringComparer.Ordinal); + bool NotSubsumed(string equipmentId) => !removedEquipmentIds.Contains(equipmentId); + + // Removed equipment tags — value variables OR alarm-bearing conditions — for SURVIVING equipment. + foreach (var tag in plan.RemovedEquipmentTags.Where(t => NotSubsumed(t.EquipmentId))) + { + var nodeId = EquipmentNodeIds.Variable(tag.EquipmentId, tag.FolderPath, tag.Name); + if (tag.Alarm is not null) + { + // Alarm-bearing tag → a Part 9 condition node. Terminal RemovedConditionState (today-gap + // closed), then remove the condition in place. + if (!SafeWriteAlarmCondition(nodeId, RemovedConditionState, ts)) failedNodes++; + if (!SafeRemoveAlarmCondition(surgical, nodeId)) return false; + } + else + { + // Plain value variable → terminal Bad so an in-flight MonitoredItem sees the removal. + SafeWriteValue(nodeId, null, OpcUaQuality.Bad, ts); + if (!SafeRemoveVariable(surgical, nodeId)) return false; + } + } + + // Removed VirtualTags (always plain value variables) for surviving equipment. + foreach (var v in plan.RemovedEquipmentVirtualTags.Where(t => NotSubsumed(t.EquipmentId))) + { + var nodeId = EquipmentNodeIds.Variable(v.EquipmentId, v.FolderPath, v.Name); + SafeWriteValue(nodeId, null, OpcUaQuality.Bad, ts); + if (!SafeRemoveVariable(surgical, nodeId)) return false; + } + + // Removed scripted alarms for surviving equipment — the terminal RemovedConditionState was already + // written by the top-of-Apply removal block; here we tear the condition node down in place. + foreach (var alarm in plan.RemovedAlarms.Where(a => NotSubsumed(a.EquipmentId))) + { + if (!SafeRemoveAlarmCondition(surgical, alarm.ScriptedAlarmId)) return false; + } + + // Removed equipment — one subtree teardown each (subsumes their child tags/vtags/alarms). The + // top-of-Apply block already wrote the equipment id's terminal condition state. + foreach (var eq in plan.RemovedEquipment) + { + if (!SafeRemoveEquipmentSubtree(surgical, eq.EquipmentId)) return false; + } + + return true; + } + + /// Remove a value-variable node in place, treating a throw as a false (⇒ rebuild fallback). + /// true when the node was removed; false when unknown or the sink threw. + private bool SafeRemoveVariable(ISurgicalAddressSpaceSink surgical, string nodeId) + { + try { return surgical.RemoveVariableNode(nodeId); } + catch (Exception ex) { _logger.LogError(ex, "AddressSpaceApplier: surgical RemoveVariableNode threw for {Node}", nodeId); return false; } + } + + /// Remove an alarm-condition node in place, treating a throw as a false (⇒ rebuild fallback). + /// true when the node was removed; false when unknown or the sink threw. + private bool SafeRemoveAlarmCondition(ISurgicalAddressSpaceSink surgical, string nodeId) + { + try { return surgical.RemoveAlarmConditionNode(nodeId); } + catch (Exception ex) { _logger.LogError(ex, "AddressSpaceApplier: surgical RemoveAlarmConditionNode threw for {Node}", nodeId); return false; } + } + + /// Remove an equipment subtree in place, treating a throw as a false (⇒ rebuild fallback). + /// true when the subtree was removed; false when unknown or the sink threw. + private bool SafeRemoveEquipmentSubtree(ISurgicalAddressSpaceSink surgical, string nodeId) + { + try { return surgical.RemoveEquipmentSubtree(nodeId); } + catch (Exception ex) { _logger.LogError(ex, "AddressSpaceApplier: surgical RemoveEquipmentSubtree threw for {Node}", nodeId); return false; } + } + + /// Write a value, swallowing (and Warning-logging) any sink fault — used for the terminal Bad + /// on a removed variable so an in-flight MonitoredItem observes the removal. + /// true when the write landed; false when the sink threw. + private bool SafeWriteValue(string nodeId, object? value, OpcUaQuality quality, DateTime ts) + { + try { _sink.WriteValue(nodeId, value, quality, ts); return true; } + catch (Exception ex) { _logger.LogWarning(ex, "AddressSpaceApplier: WriteValue threw for {Node}", nodeId); return false; } + } + + /// + /// Compute the deduplicated, deterministically-ordered set of affected PARENT node ids to announce + /// (Part 3 NodeAdded) after a pure-add apply's Materialise passes have created the new nodes. + /// For each added tag/vtag the parent is its materialise folder (the equipment folder, or the + /// FolderPath sub-folder — the exact ids the Materialise passes place children at); for each + /// added scripted alarm the parent is its equipment folder (where the condition node parents); for + /// each added equipment the parent is its UNS line folder when set, else the equipment's own new + /// folder id (a valid Part 3 announcement of the folder itself — the root has no announceable id). + /// Pure — no sink interaction — so it is unit-testable in isolation and safe to call before the + /// nodes exist. Ordinal-sorted so the announce sequence is stable. + /// + /// The plan whose added nodes' parents to announce. + /// The distinct affected parent node ids, ordinal-sorted. + public IReadOnlyList ComputeAddAnnouncements(AddressSpacePlan plan) + { + ArgumentNullException.ThrowIfNull(plan); + + var ids = new HashSet(StringComparer.Ordinal); + foreach (var t in plan.AddedEquipmentTags) ids.Add(MaterialiseParent(t.EquipmentId, t.FolderPath)); + foreach (var v in plan.AddedEquipmentVirtualTags) ids.Add(MaterialiseParent(v.EquipmentId, v.FolderPath)); + foreach (var a in plan.AddedAlarms) ids.Add(a.EquipmentId); + foreach (var eq in plan.AddedEquipment) + ids.Add(string.IsNullOrWhiteSpace(eq.UnsLineId) ? eq.EquipmentId : eq.UnsLineId); + + var list = ids.ToList(); + list.Sort(StringComparer.Ordinal); + return list; + } + + /// The materialise-parent node id a tag/vtag's variable (or native-alarm condition) hangs + /// under: the equipment folder when is null/empty, else the FolderPath + /// sub-folder. Mirrors the parent derivation the Materialise passes use. + /// The owning equipment folder id. + /// The tag/vtag FolderPath, or null/empty for "directly under the equipment". + /// The materialise-parent node id. + private static string MaterialiseParent(string equipmentId, string? folderPath) => + string.IsNullOrWhiteSpace(folderPath) ? equipmentId : EquipmentNodeIds.SubFolder(equipmentId, folderPath); + + /// + /// Announce a Part 3 GeneralModelChangeEvent(NodeAdded) per distinct affected parent from + /// , so model-aware clients re-browse the parents the new nodes + /// were added under. Called by OpcUaPublishActor AFTER the Materialise passes (the nodes exist + /// by then) and ONLY on a non-rebuild apply — after a full rebuild the announcement is moot + /// (subscriptions are dead anyway). Uses the existing, guard-covered + /// — no new sink member. Each call is + /// Safe-wrapped: a faulting announcement can never break a deploy (the nodes already stand). + /// + /// The plan whose added nodes to announce. + public void AnnounceAddedNodes(AddressSpacePlan plan) + { + foreach (var id in ComputeAddAnnouncements(plan)) + { + try { _sink.RaiseNodesAddedModelChange(id); } + catch (Exception ex) { _logger.LogWarning(ex, "AddressSpaceApplier: RaiseNodesAddedModelChange threw for {Node}", id); } + } + } + /// /// Auto-provision the historian for the added historized equipment tags. Runs on the OPC UA /// publish actor's pinned thread, so the synchronous portion is kept to building the request @@ -663,46 +838,10 @@ public sealed class AddressSpaceApplier catch (Exception ex) { _logger.LogWarning(ex, "AddressSpaceApplier: EnsureVariable threw for {Node}", nodeId); return false; } } - // A VirtualTag's materialised OPC UA node (MaterialiseEquipmentVirtualTags) is derived ONLY from - // {EquipmentId, FolderPath, Name, DataType}. Expression/DependencyRefs/Historize are engine/write-side - // only and are adopted by VirtualTagHostActor's INDEPENDENT respawn (DriverHostActor → ApplyVirtualTags), - // so a delta changing ONLY those three leaves a byte-identical node and needs no address-space rebuild. - // Whitelist-of-may-differ via `with` + the record's custom Equals: any OTHER field difference (current - // or future) makes the override unequal → falls back to a full rebuild (safe default). - private static bool VtagDeltaIsNodeIrrelevant(AddressSpacePlan.EquipmentVirtualTagDelta d) => - (d.Previous with - { - Expression = d.Current.Expression, - DependencyRefs = d.Current.DependencyRefs, - Historize = d.Current.Historize, - }).Equals(d.Current); - - // F10b: a CHANGED equipment tag whose ONLY differences are Writable / IsHistorized / - // HistorianTagname / DataType / IsArray / ArrayLength (a plain value variable — no alarm condition node) - // can be updated IN PLACE on the existing node via ISurgicalAddressSpaceSink.UpdateTagAttributes, - // avoiding a full rebuild (preserving subscriptions). The presentation-shape fields (DataType / IsArray / - // ArrayLength) join the whitelist now that the surgical sink swaps DataType + ValueRank + ArrayDimensions - // in place (and raises a GeneralModelChangeEvent). FullName / DriverInstanceId / Name / identity / alarm - // differences still fall through to a rebuild — FullName/DriverInstanceId re-route the node to a different - // driver point, Name re-derives the NodeId, and an alarm flip turns the node into a Part 9 condition. The - // override-unequal default also covers any future field. - // REACH (live-verified): the shape path only fires for drivers whose TagConfig carries a stable - // top-level "FullName" (Galaxy = tag_name.AttributeName; OpcUaClient = the node id) — there a DataType/array - // edit leaves FullName untouched ⇒ surgical. For structured-TagConfig protocol drivers (Modbus/S7/AbCip/…) - // TagConfigIntent.Parse falls back to the RAW TagConfig blob as FullName, so a DataType/ - // array edit also mutates that blob ⇒ FullName differs ⇒ this returns false ⇒ full rebuild. That is the - // correct safe default (a protocol driver's subscription needs re-spawning for a new shape anyway). - private static bool TagDeltaIsSurgicalEligible(AddressSpacePlan.EquipmentTagDelta d) => - d.Previous.Alarm is null && d.Current.Alarm is null && - (d.Previous with - { - Writable = d.Current.Writable, - IsHistorized = d.Current.IsHistorized, - HistorianTagname = d.Current.HistorianTagname, - DataType = d.Current.DataType, - IsArray = d.Current.IsArray, - ArrayLength = d.Current.ArrayLength, - }).Equals(d.Current); + // The tag/vtag surgical-eligibility predicates moved to AddressSpaceChangeClassifier (R2-07 T1) so the + // whole delta classification is one pure, table-testable unit; the applier references them there (and the + // classifier's Classify names the routing). The `with`-expression whitelist technique is preserved there + // byte-for-byte. /// The "no-event" condition state written to a removed equipment / alarm node before the /// rebuild tears it down: inactive, acked, confirmed, enabled, unshelved, severity 0, empty message. diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceChangeClassifier.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceChangeClassifier.cs new file mode 100644 index 00000000..695ef436 --- /dev/null +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceChangeClassifier.cs @@ -0,0 +1,126 @@ +namespace ZB.MOM.WW.OtOpcUa.OpcUaServer; + +/// How must mutate the address space for a given +/// . A pure policy classification OVER the planner's diff (the planner stays +/// a pure diff; the kind is the applier's routing policy over that diff — see R2-07 03/P1). +public enum AddressSpaceChangeKind +{ + /// — nothing to do. + Empty, + + /// Only surgical-eligible changed tags, node-irrelevant vtag changes, folder renames, and/or + /// node-inert driver deltas — the existing F10b in-place path. No adds, no removes. + AttributeOnly, + + /// Only additions (any mix of Equipment/Alarms/Tags/VirtualTags) on top of at-most-AttributeOnly + /// changes — no removals, no node-affecting changes. Phase 1: the idempotent Materialise passes create + /// exactly the added nodes with no teardown, so every client subscription survives. + PureAdd, + + /// Only removals on top of at-most-AttributeOnly changes — no additions, no node-affecting + /// changes. Phase 2: scoped in-place teardown of only the removed subtree. + PureRemove, + + /// Additions AND removals, still no node-affecting changes. Phase 3 applies removes-then-adds + /// surgically; until Phase 3 it maps to a full rebuild. + AddRemoveMix, + + /// Anything with a node-affecting change (ChangedEquipment, ChangedAlarms, non-surgical + /// ChangedEquipmentTags, node-relevant ChangedEquipmentVirtualTags) — the unclassifiable-safe default. + /// Full rebuild. + Rebuild, +} + +/// +/// Pure static classifier over an — names the delta class the applier +/// routes each plan through, so a pure-add deploy no longer tears down and recreates every node on the +/// server (03/P1). First-match-wins per the plan's classification table; the default-closed +/// catches every node-affecting change (including any future +/// plan field that makes a changed record unequal), so the worst outcome of any misclassification is +/// today's full-rebuild behaviour. +/// +public static class AddressSpaceChangeClassifier +{ + /// Classify into the delta class the applier routes it through. + /// The plan to classify. + /// The for the plan. + public static AddressSpaceChangeKind Classify(AddressSpacePlan plan) + { + ArgumentNullException.ThrowIfNull(plan); + + // 1 — nothing to do. + if (plan.IsEmpty) return AddressSpaceChangeKind.Empty; + + // 2 — any node-affecting CHANGE forces a full rebuild (default-closed). ChangedEquipment / + // ChangedAlarms are always node-affecting; a changed tag is node-affecting UNLESS it is + // surgical-eligible, and a changed vtag is node-affecting UNLESS it is node-irrelevant. Evaluated + // BEFORE the add/remove split so a non-surgical change alongside pure adds still rebuilds. + if (plan.ChangedEquipment.Count > 0 || + plan.ChangedAlarms.Count > 0 || + plan.ChangedEquipmentTags.Any(d => !TagDeltaIsSurgicalEligible(d)) || + plan.ChangedEquipmentVirtualTags.Any(d => !VtagDeltaIsNodeIrrelevant(d))) + { + return AddressSpaceChangeKind.Rebuild; + } + + // Driver deltas (Added/Removed/ChangedDrivers) are node-INERT — they never touch the address-space + // topology (they route to DriverHostActor's spawn plan in Runtime), so they are excluded from the + // add/remove split below and leave a driver-only plan classified AttributeOnly. + var hasAdds = + plan.AddedEquipment.Count + plan.AddedAlarms.Count + + plan.AddedEquipmentTags.Count + plan.AddedEquipmentVirtualTags.Count > 0; + var hasRemoves = + plan.RemovedEquipment.Count + plan.RemovedAlarms.Count + + plan.RemovedEquipmentTags.Count + plan.RemovedEquipmentVirtualTags.Count > 0; + + // 3 — additions AND removals. + if (hasAdds && hasRemoves) return AddressSpaceChangeKind.AddRemoveMix; + // 4 — additions only. + if (hasAdds) return AddressSpaceChangeKind.PureAdd; + // 5 — removals only. + if (hasRemoves) return AddressSpaceChangeKind.PureRemove; + // 6 — only surgical tag deltas / node-irrelevant vtag deltas / folder renames / driver deltas. + return AddressSpaceChangeKind.AttributeOnly; + } + + // A VirtualTag's materialised OPC UA node (MaterialiseEquipmentVirtualTags) is derived ONLY from + // {EquipmentId, FolderPath, Name, DataType}. Expression/DependencyRefs/Historize are engine/write-side + // only and are adopted by VirtualTagHostActor's INDEPENDENT respawn (DriverHostActor → ApplyVirtualTags), + // so a delta changing ONLY those three leaves a byte-identical node and needs no address-space rebuild. + // Whitelist-of-may-differ via `with` + the record's custom Equals: any OTHER field difference (current + // or future) makes the override unequal → falls back to a full rebuild (safe default). + internal static bool VtagDeltaIsNodeIrrelevant(AddressSpacePlan.EquipmentVirtualTagDelta d) => + (d.Previous with + { + Expression = d.Current.Expression, + DependencyRefs = d.Current.DependencyRefs, + Historize = d.Current.Historize, + }).Equals(d.Current); + + // F10b: a CHANGED equipment tag whose ONLY differences are Writable / IsHistorized / + // HistorianTagname / DataType / IsArray / ArrayLength (a plain value variable — no alarm condition node) + // can be updated IN PLACE on the existing node via ISurgicalAddressSpaceSink.UpdateTagAttributes, + // avoiding a full rebuild (preserving subscriptions). The presentation-shape fields (DataType / IsArray / + // ArrayLength) join the whitelist now that the surgical sink swaps DataType + ValueRank + ArrayDimensions + // in place (and raises a GeneralModelChangeEvent). FullName / DriverInstanceId / Name / identity / alarm + // differences still fall through to a rebuild — FullName/DriverInstanceId re-route the node to a different + // driver point, Name re-derives the NodeId, and an alarm flip turns the node into a Part 9 condition. The + // override-unequal default also covers any future field. + // REACH (live-verified): the shape path only fires for drivers whose TagConfig carries a stable + // top-level "FullName" (Galaxy = tag_name.AttributeName; OpcUaClient = the node id) — there a DataType/array + // edit leaves FullName untouched ⇒ surgical. For structured-TagConfig protocol drivers (Modbus/S7/AbCip/…) + // TagConfigIntent.Parse falls back to the RAW TagConfig blob as FullName, so a DataType/ + // array edit also mutates that blob ⇒ FullName differs ⇒ this returns false ⇒ full rebuild. That is the + // correct safe default (a protocol driver's subscription needs re-spawning for a new shape anyway). + internal static bool TagDeltaIsSurgicalEligible(AddressSpacePlan.EquipmentTagDelta d) => + d.Previous.Alarm is null && d.Current.Alarm is null && + (d.Previous with + { + Writable = d.Current.Writable, + IsHistorized = d.Current.IsHistorized, + HistorianTagname = d.Current.HistorianTagname, + DataType = d.Current.DataType, + IsArray = d.Current.IsArray, + ArrayLength = d.Current.ArrayLength, + }).Equals(d.Current); +} diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs index 964c2c76..335840d8 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs @@ -619,6 +619,17 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 lock (Lock) { + // R2-07 T4a: idempotent skip-if-present-and-same-kind. On a PURE-ADD apply, + // MaterialiseScriptedAlarms + the native-alarm materialise re-run over the FULL composition, so + // an existing enabled condition is re-materialised with the SAME id + kind. Skip it — KEEP the + // existing AlarmConditionState instance alive so every MonitoredItem on that condition node + // survives the deploy (the drop-and-recreate below would otherwise kill them). A genuine + // re-severity/type change arrives as a ChangedAlarms delta ⇒ full rebuild (the map is cleared + // first), so it never reaches this path with a stale-but-present entry; the drop-and-recreate + // stays ONLY for the kind-swap (native↔scripted) case, which flips the native flag. + if (_alarmConditions.ContainsKey(alarmNodeId) && _nativeAlarmNodeIds.Contains(alarmNodeId) == isNative) + return; + // Idempotent: drop any prior node for this id so a re-materialise (e.g. changed // type/severity on redeploy) reflects cleanly instead of leaking the old node. if (_alarmConditions.TryRemove(alarmNodeId, out var existing)) @@ -1681,6 +1692,59 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 return e; } + /// Build (but do not report) the Part 3 GeneralModelChangeEvent announcing that the node + /// at was DELETED. Mirrors + /// exactly — the only differences are Verb = NodeDeleted and that the affected node has already + /// been dropped from the maps, so AffectedType resolves to (a valid Part 3 + /// "type not applicable"; clients re-browse the parent regardless). internal so a node-manager test + /// can assert the populated Changes structure at the nearest deterministic seam. + /// The node id of the deleted node. + /// A populated, unreported . + internal GeneralModelChangeEventState BuildNodesRemovedModelChange(string affectedNodeId) + { + var affected = new NodeId(affectedNodeId, NamespaceIndex); + var e = new GeneralModelChangeEventState(null); + e.Initialize( + SystemContext, + source: null, + severity: EventSeverity.Medium, + message: new LocalizedText($"Node deleted: {affected}")); + // Part 3 §8.7.4: emitted by the Server object — set SourceNode/SourceName to Server explicitly + // (mirrors BuildNodesAddedModelChange). + e.SetChildValue(SystemContext, BrowseNames.SourceNode, ObjectIds.Server, false); + e.SetChildValue(SystemContext, BrowseNames.SourceName, "Server", false); + var change = new ModelChangeStructureDataType + { + Affected = affected, + // The node is already gone from the maps, so its TypeDefinition is not applicable (Null). + AffectedType = ResolveAffectedTypeDefinition(affectedNodeId), + Verb = (byte)ModelChangeStructureVerbMask.NodeDeleted, + }; + e.SetChildValue(SystemContext, BrowseNames.Changes, new[] { change }, false); + return e; + } + + /// Report a pre-built OUTSIDE Lock — + /// Server.ReportEvent re-enters the server's own subscription/event path, so holding Lock + /// across it risks a lock-order inversion (mirrors / + /// ReportNodeShapeChangedEvent). Tolerant: swallow-and-log when eventing is disabled / there are no + /// monitored items / the server is shutting down — the node mutation already stands. + /// The pre-built event to report. + /// The affected node id (for the diagnostic only). + private void ReportModelChangeOutsideLock(GeneralModelChangeEventState e, string affectedNodeId) + { + try + { + Server.ReportEvent(SystemContext, e); + } + catch (Exception ex) + { +#pragma warning disable CS0618 // Utils.LogError is [Obsolete] in favour of an ITelemetryContext this manager doesn't carry. + Utils.LogError(ex, "OtOpcUaNodeManager: failed to report GeneralModelChangeEvent for {0}", affectedNodeId); +#pragma warning restore CS0618 + } + } + /// Resolve the TypeDefinition of a materialised node id from the live folder/variable maps for a /// model-change event's AffectedType; when the id is not registered. /// The folder-scoped node id whose TypeDefinition is wanted. @@ -1763,6 +1827,122 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 } } + /// + public bool RemoveVariableNode(string variableNodeId) + { + ArgumentException.ThrowIfNullOrEmpty(variableNodeId); + EnsureAddressSpaceCreated(); + + GeneralModelChangeEventState e; + lock (Lock) + { + // Unknown id ⇒ the node-manager maps drifted from what the planner believes; return false so the + // caller (AddressSpaceApplier) falls back to a full rebuild (resync). Mirrors the per-node + // teardown inside RebuildAddressSpace, scoped to this one id. + if (!_variables.TryRemove(variableNodeId, out var variable)) return false; + variable.Parent?.RemoveChild(variable); + PredefinedNodes?.Remove(variable.NodeId); + // Drop the historized-tagname registration alongside the variable it maps (Phase C parity). + _historizedTagnames.TryRemove(variableNodeId, out _); + e = BuildNodesRemovedModelChange(variableNodeId); + } + ReportModelChangeOutsideLock(e, variableNodeId); + return true; + } + + /// + public bool RemoveAlarmConditionNode(string alarmNodeId) + { + ArgumentException.ThrowIfNullOrEmpty(alarmNodeId); + EnsureAddressSpaceCreated(); + + GeneralModelChangeEventState e; + lock (Lock) + { + // Unknown id ⇒ map drift ⇒ false (caller rebuilds). Mirrors the condition teardown inside + // RebuildAddressSpace, scoped to this one condition. The equipment folder's event-notifier + // promotion is intentionally NOT demoted here — other conditions may still hang under it, and a + // lingering notifier folder is harmless (RemoveEquipmentSubtree demotes it when the whole + // equipment goes). + if (!_alarmConditions.TryRemove(alarmNodeId, out var condition)) return false; + condition.Parent?.RemoveChild(condition); + PredefinedNodes?.Remove(condition.NodeId); + _nativeAlarmNodeIds.Remove(alarmNodeId); + e = BuildNodesRemovedModelChange(alarmNodeId); + } + ReportModelChangeOutsideLock(e, alarmNodeId); + return true; + } + + /// + public bool RemoveEquipmentSubtree(string equipmentNodeId) + { + ArgumentException.ThrowIfNullOrEmpty(equipmentNodeId); + EnsureAddressSpaceCreated(); + + GeneralModelChangeEventState e; + lock (Lock) + { + // Unknown equipment id ⇒ map drift ⇒ false (caller rebuilds). + if (!_folders.ContainsKey(equipmentNodeId)) return false; + + // Folder-scoped NodeIds mean every descendant's id is the equipment id itself or begins with + // "/" (sub-folders, variables, condition nodes). Snapshot each map's in-scope keys + // first (ToList) so we don't mutate while enumerating. + var prefix = equipmentNodeId + "/"; + bool InScope(string id) => id == equipmentNodeId || id.StartsWith(prefix, StringComparison.Ordinal); + + // Value variables (+ their historized-tagname registrations). + foreach (var id in _variables.Keys.Where(InScope).ToList()) + { + if (_variables.TryRemove(id, out var v)) + { + v.Parent?.RemoveChild(v); + PredefinedNodes?.Remove(v.NodeId); + } + _historizedTagnames.TryRemove(id, out _); + } + + // Part 9 condition nodes (+ their native flags). + foreach (var id in _alarmConditions.Keys.Where(InScope).ToList()) + { + if (_alarmConditions.TryRemove(id, out var c)) + { + c.Parent?.RemoveChild(c); + PredefinedNodes?.Remove(c.NodeId); + } + _nativeAlarmNodeIds.Remove(id); + } + + // Notifier demotion BEFORE dropping the folders: sever the Server↔folder HasNotifier ref for + // every promoted folder in the subtree (an equipment folder, or a sub-folder that hosted an + // alarm), else the removal leaks an orphaned root-notifier reference on the Server object. + // _notifierFolders is keyed by the folder's NodeId; match on its string identifier. + foreach (var kvp in _notifierFolders.Where(kv => InScope(kv.Key.Identifier?.ToString() ?? string.Empty)).ToList()) + { + RemoveRootNotifier(kvp.Value); + _notifierFolders.Remove(kvp.Key); + } + // Drop the event-history source registrations for the subtree (keyed by the folder id string). + foreach (var src in _eventNotifierSources.Keys.Where(InScope).ToList()) + _eventNotifierSources.TryRemove(src, out _); + + // Sub-folders + the equipment folder itself (children already detached above). + foreach (var id in _folders.Keys.Where(InScope).ToList()) + { + if (_folders.TryRemove(id, out var f)) + { + f.Parent?.RemoveChild(f); + PredefinedNodes?.Remove(f.NodeId); + } + } + + e = BuildNodesRemovedModelChange(equipmentNodeId); + } + ReportModelChangeOutsideLock(e, equipmentNodeId); + return true; + } + private FolderState ResolveParentFolder(string? parentNodeId) { EnsureAddressSpaceCreated(); diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/SdkAddressSpaceSink.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/SdkAddressSpaceSink.cs index 9611c9fc..16350b02 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/SdkAddressSpaceSink.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/SdkAddressSpaceSink.cs @@ -48,6 +48,18 @@ public sealed class SdkAddressSpaceSink : IOpcUaAddressSpaceSink, ISurgicalAddre public bool UpdateFolderDisplayName(string folderNodeId, string displayName) => _nodeManager.UpdateFolderDisplayName(folderNodeId, displayName); + /// + public bool RemoveVariableNode(string variableNodeId) + => _nodeManager.RemoveVariableNode(variableNodeId); + + /// + public bool RemoveAlarmConditionNode(string alarmNodeId) + => _nodeManager.RemoveAlarmConditionNode(alarmNodeId); + + /// + public bool RemoveEquipmentSubtree(string equipmentNodeId) + => _nodeManager.RemoveEquipmentSubtree(equipmentNodeId); + /// public void RebuildAddressSpace() => _nodeManager.RebuildAddressSpace(); diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/OpcUa/OpcUaPublishActor.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/OpcUa/OpcUaPublishActor.cs index b6648b2d..f8c856c0 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/OpcUa/OpcUaPublishActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/OpcUa/OpcUaPublishActor.cs @@ -357,6 +357,16 @@ public sealed class OpcUaPublishActor : ReceiveActor, IWithTimers // values once the first dependency update arrives; until then variables show BadWaitingForInitialData. failedNodes += _applier.MaterialiseEquipmentVirtualTags(composition); + // R2-07 T4b — on a NON-rebuild apply (PureAdd / AttributeOnly), the Materialise passes above + // just created exactly the added nodes WITHOUT tearing anything down, so announce them with a + // Part 3 GeneralModelChangeEvent(NodeAdded) per affected parent — model-aware clients then + // re-browse and pick up the new nodes while every existing MonitoredItem stays alive. Ordering + // is correct by construction: Apply returned before the passes ran, and this announce runs after + // them, so the nodes exist when clients re-browse. After a full rebuild the announcement is moot + // (subscriptions are dead anyway) — the guard skips it. AnnounceAddedNodes is Safe-wrapped + // internally, so a faulting announce can never break the deploy. + if (!outcome.RebuildCalled) _applier.AnnounceAddedNodes(plan); + OtOpcUaTelemetry.OpcUaSinkWrite.Add(1, new KeyValuePair("kind", "rebuild")); if (outcome.RebuildFailed || failedNodes > 0) { 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 fe0b9ea5..6b2e2ae8 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 @@ -114,6 +114,44 @@ public class DeferredAddressSpaceSinkTests surgical.FolderRenameCalled.ShouldBeTrue(); } + // ---------- R2-07 Phase 2: surgical remove forwarding ---------- + + [Fact] + public void Remove_members_return_false_for_non_surgical_inner() + { + var sink = new DeferredAddressSpaceSink(); + sink.SetSink(new SpySink()); + + sink.RemoveVariableNode("v-1").ShouldBeFalse(); + sink.RemoveAlarmConditionNode("a-1").ShouldBeFalse(); + sink.RemoveEquipmentSubtree("eq-1").ShouldBeFalse(); + } + + [Fact] + public void Remove_members_forward_to_surgical_inner() + { + var surgical = new SpySurgicalSink(); + var sink = new DeferredAddressSpaceSink(); + sink.SetSink(surgical); + + sink.RemoveVariableNode("v-1").ShouldBeTrue(); + sink.RemoveAlarmConditionNode("a-1").ShouldBeTrue(); + sink.RemoveEquipmentSubtree("eq-1").ShouldBeTrue(); + + surgical.RemoveVariableCalled.ShouldBeTrue(); + surgical.RemoveAlarmCalled.ShouldBeTrue(); + surgical.RemoveSubtreeCalled.ShouldBeTrue(); + } + + [Fact] + public void Before_SetSink_remove_members_return_false() + { + var sink = new DeferredAddressSpaceSink(); + sink.RemoveVariableNode("v-1").ShouldBeFalse(); + sink.RemoveAlarmConditionNode("a-1").ShouldBeFalse(); + sink.RemoveEquipmentSubtree("eq-1").ShouldBeFalse(); + } + // ---------- SetSink(null) reverts to null sink ---------- [Fact] @@ -183,5 +221,13 @@ public class DeferredAddressSpaceSinkTests FolderRenameCalled = true; return true; } + + public bool RemoveVariableCalled { get; private set; } + public bool RemoveAlarmCalled { get; private set; } + public bool RemoveSubtreeCalled { get; private set; } + + public bool RemoveVariableNode(string variableNodeId) { RemoveVariableCalled = true; return true; } + public bool RemoveAlarmConditionNode(string alarmNodeId) { RemoveAlarmCalled = true; return true; } + public bool RemoveEquipmentSubtree(string equipmentNodeId) { RemoveSubtreeCalled = true; return true; } } } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/SubscriptionSurvivalTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/SubscriptionSurvivalTests.cs new file mode 100644 index 00000000..432c77cb --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/SubscriptionSurvivalTests.cs @@ -0,0 +1,380 @@ +using System.Net; +using System.Net.Sockets; +using Microsoft.Extensions.Logging.Abstractions; +using Opc.Ua; +using Opc.Ua.Client; +using Shouldly; +using Xunit; + +namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests; + +/// +/// R2-07 03/P1 — the HEADLINE proof: an over-the-wire subscription on an existing tag SURVIVES a +/// pure-add deploy (one new tag), instead of being killed by the full-rebuild teardown that pre-R2-07 +/// every topology change forced. Boots the real , materialises tag A, +/// opens a real client subscription + monitored item on A, then drives a PURE-ADD apply adding tag B +/// (classifier ⇒ PureAdd ⇒ no rebuild ⇒ the idempotent Materialise passes create B and leave A's +/// NodeState untouched), and asserts: +/// +/// A's monitored item still delivers a subsequent value write (the item is alive — no +/// BadNodeIdUnknown, no dead subscription); +/// B is browsable + readable (the add landed). +/// +/// Written against the master (pre-T2) behaviour this FAILS (the rebuild recreates A as a new NodeState +/// and the monitored item goes dead); it passes once the classifier routing (T2) + the publish-actor +/// announce (T4b) land. +/// Run note: this is a heavy in-process server+client integration test — run it in the +/// serial integration pass (it is NOT part of the lightweight unit sweep). The +/// namespace URI is ; the client resolves the +/// folder-scoped variable NodeIds against it. +/// +public sealed class SubscriptionSurvivalTests +{ + private const string ServerUri = "urn:OtOpcUa.SubscriptionSurvival"; + + /// A subscription on tag A survives a pure-add deploy adding tag B; B becomes readable. + [Fact] + public async Task Subscription_on_existing_tag_survives_pure_add_of_another_tag() + { + var pkiRoot = Path.Combine(Path.GetTempPath(), $"otopcua-subsurvive-{Guid.NewGuid():N}"); + var port = AllocateFreePort(); + var ct = TestContext.Current.CancellationToken; + + try + { + var options = new OpcUaApplicationHostOptions + { + ApplicationName = ServerUri, + ApplicationUri = ServerUri, + OpcUaPort = port, + PublicHostname = "127.0.0.1", + PkiStoreRoot = pkiRoot, + EnabledSecurityProfiles = new List { OpcUaSecurityProfile.None }, + AutoAcceptUntrustedClientCertificates = true, + }; + var server = new OtOpcUaSdkServer(); + await using var host = new OpcUaApplicationHost(options, NullLogger.Instance); + await host.StartAsync(server, ct); + + var nm = server.NodeManager!; + var sink = new SdkAddressSpaceSink(nm); + var applier = new AddressSpaceApplier(sink, NullLogger.Instance); + + // --- Materialise tag A (equipment eq-1, tag "A") and seed it a Good value. --- + var withA = CompositionWith( + new EquipmentTagPlan("tag-a", "eq-1", "drv", FolderPath: "", Name: "A", DataType: "Int32", FullName: "1", Writable: false, Alarm: null)); + applier.MaterialiseHierarchy(withA); + applier.MaterialiseEquipmentTags(withA); + var nodeIdAString = Commons.OpcUa.EquipmentNodeIds.Variable("eq-1", "", "A"); + sink.WriteValue(nodeIdAString, 41, Commons.OpcUa.OpcUaQuality.Good, DateTime.UtcNow); + + // --- Open a client session + subscription + monitored item on A. --- + using var session = await OpenSessionAsync($"opc.tcp://127.0.0.1:{port}/OtOpcUa", ct); + var ns = (ushort)session.NamespaceUris.GetIndex(OtOpcUaNodeManager.DefaultNamespaceUri); + ns.ShouldBeGreaterThan((ushort)0); + var nodeIdA = new NodeId(nodeIdAString, ns); + + var received = new List(); + var gate = new object(); + var subscription = new Subscription(session.DefaultSubscription) { PublishingInterval = 100 }; + session.AddSubscription(subscription); + await subscription.CreateAsync(ct); + + var monitored = new MonitoredItem(subscription.DefaultItem) + { + StartNodeId = nodeIdA, + AttributeId = Attributes.Value, + SamplingInterval = 50, + QueueSize = 10, + }; + monitored.Notification += (item, _) => + { + foreach (var v in item.DequeueValues()) + lock (gate) received.Add(v); + }; + subscription.AddItem(monitored); + await subscription.ApplyChangesAsync(ct); + + // Baseline: the initial value (41) is delivered. + await WaitUntilAsync(() => { lock (gate) return received.Count >= 1; }, TimeSpan.FromSeconds(5)); + + // --- Drive a PURE-ADD apply adding tag B (previous={A}, next={A,B}). --- + var withAB = CompositionWith( + new EquipmentTagPlan("tag-a", "eq-1", "drv", FolderPath: "", Name: "A", DataType: "Int32", FullName: "1", Writable: false, Alarm: null), + new EquipmentTagPlan("tag-b", "eq-1", "drv", FolderPath: "", Name: "B", DataType: "Int32", FullName: "2", Writable: false, Alarm: null)); + var plan = AddressSpacePlanner.Compute(withA, withAB); + plan.AddedEquipmentTags.Count.ShouldBe(1); + + var outcome = applier.Apply(plan); + outcome.RebuildCalled.ShouldBeFalse(); // PureAdd — no teardown + + // Materialise passes (as the publish actor runs them) then announce — over the FULL composition. + applier.MaterialiseHierarchy(withAB); + applier.MaterialiseEquipmentTags(withAB); + applier.AnnounceAddedNodes(plan); + + // --- A's monitored item must still be alive: a new write to A is delivered. --- + lock (gate) received.Clear(); + sink.WriteValue(nodeIdAString, 42, Commons.OpcUa.OpcUaQuality.Good, DateTime.UtcNow); + + await WaitUntilAsync(() => { lock (gate) return received.Any(v => Equals(v.Value, 42)); }, TimeSpan.FromSeconds(5)); + lock (gate) + { + received.ShouldContain(v => Equals(v.Value, 42)); + received.ShouldNotContain(v => v.StatusCode == StatusCodes.BadNodeIdUnknown); + } + + // --- B is browsable + readable (the add landed). --- + var nodeIdB = new NodeId(Commons.OpcUa.EquipmentNodeIds.Variable("eq-1", "", "B"), ns); + sink.WriteValue(nodeIdB.Identifier.ToString()!, 7, Commons.OpcUa.OpcUaQuality.Good, DateTime.UtcNow); + var bValue = await session.ReadValueAsync(nodeIdB, ct); + bValue.StatusCode.ShouldNotBe(StatusCodes.BadNodeIdUnknown); + bValue.Value.ShouldBe(7); + } + finally + { + if (Directory.Exists(pkiRoot)) Directory.Delete(pkiRoot, recursive: true); + } + } + + /// R2-07 Phase 2 — a subscription on tag A survives a PURE-REMOVE deploy that removes tag B; B's + /// monitored item observes a final Bad and the node then disappears (a re-read returns BadNodeIdUnknown), + /// while A stays alive. + [Fact] + public async Task Subscription_on_surviving_tag_survives_pure_remove_of_another_tag() + { + var pkiRoot = Path.Combine(Path.GetTempPath(), $"otopcua-subsurvive-rm-{Guid.NewGuid():N}"); + var port = AllocateFreePort(); + var ct = TestContext.Current.CancellationToken; + + try + { + var options = new OpcUaApplicationHostOptions + { + ApplicationName = ServerUri, + ApplicationUri = ServerUri + ".Remove", + OpcUaPort = port, + PublicHostname = "127.0.0.1", + PkiStoreRoot = pkiRoot, + EnabledSecurityProfiles = new List { OpcUaSecurityProfile.None }, + AutoAcceptUntrustedClientCertificates = true, + }; + var server = new OtOpcUaSdkServer(); + await using var host = new OpcUaApplicationHost(options, NullLogger.Instance); + await host.StartAsync(server, ct); + + var nm = server.NodeManager!; + var sink = new SdkAddressSpaceSink(nm); + var applier = new AddressSpaceApplier(sink, NullLogger.Instance); + + // Materialise A + B and seed both Good. + var withAB = CompositionWith( + new EquipmentTagPlan("tag-a", "eq-1", "drv", FolderPath: "", Name: "A", DataType: "Int32", FullName: "1", Writable: false, Alarm: null), + new EquipmentTagPlan("tag-b", "eq-1", "drv", FolderPath: "", Name: "B", DataType: "Int32", FullName: "2", Writable: false, Alarm: null)); + applier.MaterialiseHierarchy(withAB); + applier.MaterialiseEquipmentTags(withAB); + var nodeIdAString = Commons.OpcUa.EquipmentNodeIds.Variable("eq-1", "", "A"); + var nodeIdBString = Commons.OpcUa.EquipmentNodeIds.Variable("eq-1", "", "B"); + sink.WriteValue(nodeIdAString, 10, Commons.OpcUa.OpcUaQuality.Good, DateTime.UtcNow); + sink.WriteValue(nodeIdBString, 20, Commons.OpcUa.OpcUaQuality.Good, DateTime.UtcNow); + + using var session = await OpenSessionAsync($"opc.tcp://127.0.0.1:{port}/OtOpcUa", ct); + var ns = (ushort)session.NamespaceUris.GetIndex(OtOpcUaNodeManager.DefaultNamespaceUri); + var nodeIdA = new NodeId(nodeIdAString, ns); + var nodeIdB = new NodeId(nodeIdBString, ns); + + var receivedA = new List(); + var receivedB = new List(); + var gate = new object(); + var subscription = new Subscription(session.DefaultSubscription) { PublishingInterval = 100 }; + session.AddSubscription(subscription); + await subscription.CreateAsync(ct); + var miA = new MonitoredItem(subscription.DefaultItem) { StartNodeId = nodeIdA, AttributeId = Attributes.Value, SamplingInterval = 50, QueueSize = 10 }; + var miB = new MonitoredItem(subscription.DefaultItem) { StartNodeId = nodeIdB, AttributeId = Attributes.Value, SamplingInterval = 50, QueueSize = 10 }; + miA.Notification += (item, _) => { foreach (var v in item.DequeueValues()) lock (gate) receivedA.Add(v); }; + miB.Notification += (item, _) => { foreach (var v in item.DequeueValues()) lock (gate) receivedB.Add(v); }; + subscription.AddItem(miA); + subscription.AddItem(miB); + await subscription.ApplyChangesAsync(ct); + await WaitUntilAsync(() => { lock (gate) return receivedA.Count >= 1 && receivedB.Count >= 1; }, TimeSpan.FromSeconds(5)); + + // Drive a PURE-REMOVE apply removing B (previous={A,B}, next={A}). + var withA = CompositionWith( + new EquipmentTagPlan("tag-a", "eq-1", "drv", FolderPath: "", Name: "A", DataType: "Int32", FullName: "1", Writable: false, Alarm: null)); + var plan = AddressSpacePlanner.Compute(withAB, withA); + plan.RemovedEquipmentTags.Count.ShouldBe(1); + + lock (gate) { receivedA.Clear(); receivedB.Clear(); } + var outcome = applier.Apply(plan); + outcome.RebuildCalled.ShouldBeFalse(); // PureRemove — no full rebuild + + // B's monitored item observes a final Bad; the node is then gone from the address space. + await WaitUntilAsync(() => { lock (gate) return receivedB.Any(v => StatusCode.IsBad(v.StatusCode)); }, TimeSpan.FromSeconds(5)); + + // A stays alive: a fresh write to A is delivered. + sink.WriteValue(nodeIdAString, 11, Commons.OpcUa.OpcUaQuality.Good, DateTime.UtcNow); + await WaitUntilAsync(() => { lock (gate) return receivedA.Any(v => Equals(v.Value, 11)); }, TimeSpan.FromSeconds(5)); + + // B is gone: a re-read returns BadNodeIdUnknown. + var bRead = await session.ReadValueAsync(nodeIdB, ct); + bRead.StatusCode.ShouldBe((StatusCode)StatusCodes.BadNodeIdUnknown); + } + finally + { + if (Directory.Exists(pkiRoot)) Directory.Delete(pkiRoot, recursive: true); + } + } + + /// R2-07 Phase 3 — a subscription on tag A survives a MIXED deploy (−B, +C in one apply): A + /// stays alive, B is gone (BadNodeIdUnknown), and C is browsable + readable. + [Fact] + public async Task Subscription_survives_mixed_add_remove_deploy() + { + var pkiRoot = Path.Combine(Path.GetTempPath(), $"otopcua-subsurvive-mix-{Guid.NewGuid():N}"); + var port = AllocateFreePort(); + var ct = TestContext.Current.CancellationToken; + + try + { + var options = new OpcUaApplicationHostOptions + { + ApplicationName = ServerUri, + ApplicationUri = ServerUri + ".Mixed", + OpcUaPort = port, + PublicHostname = "127.0.0.1", + PkiStoreRoot = pkiRoot, + EnabledSecurityProfiles = new List { OpcUaSecurityProfile.None }, + AutoAcceptUntrustedClientCertificates = true, + }; + var server = new OtOpcUaSdkServer(); + await using var host = new OpcUaApplicationHost(options, NullLogger.Instance); + await host.StartAsync(server, ct); + + var nm = server.NodeManager!; + var sink = new SdkAddressSpaceSink(nm); + var applier = new AddressSpaceApplier(sink, NullLogger.Instance); + + var withAB = CompositionWith( + new EquipmentTagPlan("tag-a", "eq-1", "drv", FolderPath: "", Name: "A", DataType: "Int32", FullName: "1", Writable: false, Alarm: null), + new EquipmentTagPlan("tag-b", "eq-1", "drv", FolderPath: "", Name: "B", DataType: "Int32", FullName: "2", Writable: false, Alarm: null)); + applier.MaterialiseHierarchy(withAB); + applier.MaterialiseEquipmentTags(withAB); + var nodeIdAString = Commons.OpcUa.EquipmentNodeIds.Variable("eq-1", "", "A"); + sink.WriteValue(nodeIdAString, 10, Commons.OpcUa.OpcUaQuality.Good, DateTime.UtcNow); + + using var session = await OpenSessionAsync($"opc.tcp://127.0.0.1:{port}/OtOpcUa", ct); + var ns = (ushort)session.NamespaceUris.GetIndex(OtOpcUaNodeManager.DefaultNamespaceUri); + var nodeIdA = new NodeId(nodeIdAString, ns); + + var receivedA = new List(); + var gate = new object(); + var subscription = new Subscription(session.DefaultSubscription) { PublishingInterval = 100 }; + session.AddSubscription(subscription); + await subscription.CreateAsync(ct); + var miA = new MonitoredItem(subscription.DefaultItem) { StartNodeId = nodeIdA, AttributeId = Attributes.Value, SamplingInterval = 50, QueueSize = 10 }; + miA.Notification += (item, _) => { foreach (var v in item.DequeueValues()) lock (gate) receivedA.Add(v); }; + subscription.AddItem(miA); + await subscription.ApplyChangesAsync(ct); + await WaitUntilAsync(() => { lock (gate) return receivedA.Count >= 1; }, TimeSpan.FromSeconds(5)); + + // MIXED apply: remove B, add C (previous={A,B}, next={A,C}). + var withAC = CompositionWith( + new EquipmentTagPlan("tag-a", "eq-1", "drv", FolderPath: "", Name: "A", DataType: "Int32", FullName: "1", Writable: false, Alarm: null), + new EquipmentTagPlan("tag-c", "eq-1", "drv", FolderPath: "", Name: "C", DataType: "Int32", FullName: "3", Writable: false, Alarm: null)); + var plan = AddressSpacePlanner.Compute(withAB, withAC); + plan.RemovedEquipmentTags.Count.ShouldBe(1); + plan.AddedEquipmentTags.Count.ShouldBe(1); + + lock (gate) receivedA.Clear(); + var outcome = applier.Apply(plan); + outcome.RebuildCalled.ShouldBeFalse(); // AddRemoveMix — removes in place, no rebuild + // Publish-actor sequence: removes ran inside Apply; now materialise the adds + announce. + applier.MaterialiseEquipmentTags(withAC); + applier.AnnounceAddedNodes(plan); + + // A stays alive. + sink.WriteValue(nodeIdAString, 11, Commons.OpcUa.OpcUaQuality.Good, DateTime.UtcNow); + await WaitUntilAsync(() => { lock (gate) return receivedA.Any(v => Equals(v.Value, 11)); }, TimeSpan.FromSeconds(5)); + + // B is gone. + var nodeIdB = new NodeId(Commons.OpcUa.EquipmentNodeIds.Variable("eq-1", "", "B"), ns); + (await session.ReadValueAsync(nodeIdB, ct)).StatusCode.ShouldBe((StatusCode)StatusCodes.BadNodeIdUnknown); + + // C is browsable + readable. + var nodeIdC = new NodeId(Commons.OpcUa.EquipmentNodeIds.Variable("eq-1", "", "C"), ns); + sink.WriteValue(nodeIdC.Identifier.ToString()!, 9, Commons.OpcUa.OpcUaQuality.Good, DateTime.UtcNow); + var cRead = await session.ReadValueAsync(nodeIdC, ct); + cRead.StatusCode.ShouldNotBe((StatusCode)StatusCodes.BadNodeIdUnknown); + cRead.Value.ShouldBe(9); + } + finally + { + if (Directory.Exists(pkiRoot)) Directory.Delete(pkiRoot, recursive: true); + } + } + + private static AddressSpaceComposition CompositionWith(params EquipmentTagPlan[] tags) => + new( + UnsAreas: Array.Empty(), + UnsLines: Array.Empty(), + EquipmentNodes: new[] { new EquipmentNode("eq-1", "Equipment 1", "") }, + DriverInstancePlans: Array.Empty(), + ScriptedAlarmPlans: Array.Empty()) + { + EquipmentTags = tags, + }; + + private static async Task OpenSessionAsync(string endpointUrl, CancellationToken ct) + { + var appConfig = new ApplicationConfiguration + { + ApplicationName = "OtOpcUa.SubscriptionSurvivalClient", + ApplicationUri = $"urn:OtOpcUa.SubscriptionSurvivalClient.{Guid.NewGuid():N}", + ApplicationType = ApplicationType.Client, + SecurityConfiguration = new SecurityConfiguration + { + ApplicationCertificate = new CertificateIdentifier(), + AutoAcceptUntrustedCertificates = true, + }, + ClientConfiguration = new ClientConfiguration { DefaultSessionTimeout = 60_000 }, + }; + await appConfig.ValidateAsync(ApplicationType.Client, ct); + appConfig.CertificateValidator.CertificateValidation += (_, e) => e.Accept = true; + + var endpoint = await CoreClientUtils.SelectEndpointAsync( + appConfig, endpointUrl, false, DefaultTelemetry.Create(_ => { }), ct); + var endpointConfiguration = EndpointConfiguration.Create(appConfig); + var configuredEndpoint = new ConfiguredEndpoint(null, endpoint, endpointConfiguration); + + return await new DefaultSessionFactory(DefaultTelemetry.Create(_ => { })).CreateAsync( + appConfig, + configuredEndpoint, + updateBeforeConnect: false, + checkDomain: false, + sessionName: "SubscriptionSurvivalTests", + sessionTimeout: 60_000, + identity: new UserIdentity(new AnonymousIdentityToken()), + preferredLocales: null, + ct: ct); + } + + private static async Task WaitUntilAsync(Func condition, TimeSpan timeout) + { + var deadline = DateTime.UtcNow + timeout; + while (DateTime.UtcNow < deadline) + { + if (condition()) return; + await Task.Delay(50); + } + condition().ShouldBeTrue("condition not met within timeout"); + } + + private static int AllocateFreePort() + { + var listener = new TcpListener(IPAddress.Loopback, 0); + listener.Start(); + var port = ((IPEndPoint)listener.LocalEndpoint).Port; + listener.Stop(); + return port; + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierFailureSurfaceTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierFailureSurfaceTests.cs index c4a1b75d..9582c8d5 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierFailureSurfaceTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierFailureSurfaceTests.cs @@ -23,7 +23,7 @@ public sealed class AddressSpaceApplierFailureSurfaceTests var sink = new ConfigurableThrowingSink { ThrowOnRebuild = true }; var applier = new AddressSpaceApplier(sink, NullLogger.Instance); - var outcome = applier.Apply(AddedEquipmentPlan("new")); + var outcome = applier.Apply(RebuildingPlan("eq-1")); outcome.RebuildCalled.ShouldBeTrue(); outcome.RebuildFailed.ShouldBeTrue(); @@ -36,7 +36,7 @@ public sealed class AddressSpaceApplierFailureSurfaceTests var sink = new ConfigurableThrowingSink(); var applier = new AddressSpaceApplier(sink, NullLogger.Instance); - var outcome = applier.Apply(AddedEquipmentPlan("new")); + var outcome = applier.Apply(RebuildingPlan("eq-1")); outcome.RebuildCalled.ShouldBeTrue(); outcome.RebuildFailed.ShouldBeFalse(); @@ -121,10 +121,18 @@ public sealed class AddressSpaceApplierFailureSurfaceTests EquipmentScriptedAlarms = alarms, }; - private static AddressSpacePlan AddedEquipmentPlan(string id) => new( - AddedEquipment: new[] { new EquipmentNode(id, id, "line-1") }, + // A rebuild-forcing plan: a node-affecting ChangedEquipment delta classifies as Rebuild (R2-07), so + // these tests exercise the SafeRebuild path (RebuildCalled/RebuildFailed) regardless of the pure-add + // routing. A pure-add plan would no longer rebuild, so the failure-surface tests must use a change. + private static AddressSpacePlan RebuildingPlan(string id) => new( + AddedEquipment: Array.Empty(), RemovedEquipment: Array.Empty(), - ChangedEquipment: Array.Empty(), + ChangedEquipment: new[] + { + new AddressSpacePlan.EquipmentDelta( + new EquipmentNode(id, id, "line-1"), + new EquipmentNode(id, id + "-renamed", "line-1")), + }, AddedDrivers: Array.Empty(), RemovedDrivers: Array.Empty(), ChangedDrivers: Array.Empty(), diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierProvisioningTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierProvisioningTests.cs index 0e753aac..f8edaa56 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierProvisioningTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierProvisioningTests.cs @@ -64,7 +64,8 @@ public sealed class AddressSpaceApplierProvisioningTests var outcome = applier.Apply(plan); - outcome.RebuildCalled.ShouldBeTrue(); + outcome.RebuildCalled.ShouldBeFalse(); // PureAdd — no rebuild; provisioning still fires + outcome.AddedNodes.ShouldBeGreaterThan(0); // Fire-and-forget: await the capturing double's signal so the assertion is deterministic. await prov.Called.WaitAsync(TimeSpan.FromSeconds(5), TestContext.Current.CancellationToken); @@ -108,7 +109,7 @@ public sealed class AddressSpaceApplierProvisioningTests var outcome = applier.Apply(PlanWithAddedTags( HistorizedTag(displayName: "Temp", historianName: "Pump1.Temp", dataType: "Float32"))); - outcome.RebuildCalled.ShouldBeTrue(); // address-space work still completed + outcome.RebuildCalled.ShouldBeFalse(); // PureAdd — no rebuild; Apply still completed + provisioned } /// The default ctor (no provisioner) binds the no-op @@ -121,7 +122,8 @@ public sealed class AddressSpaceApplierProvisioningTests var outcome = applier.Apply(PlanWithAddedTags( HistorizedTag(displayName: "Temp", historianName: "Pump1.Temp", dataType: "Float32"))); - outcome.RebuildCalled.ShouldBeTrue(); + outcome.RebuildCalled.ShouldBeFalse(); // PureAdd — no rebuild; default no-op provisioning still safe + outcome.AddedNodes.ShouldBeGreaterThan(0); } /// An added historized tag whose DataType string is not a is @@ -319,7 +321,7 @@ public sealed class AddressSpaceApplierProvisioningTests var outcome = applier.Apply(PlanWithAddedTags( HistorizedTag(displayName: "Temp", historianName: "Pump1.Temp", dataType: "Float32"))); - outcome.RebuildCalled.ShouldBeTrue(); // address-space work still completed + outcome.RebuildCalled.ShouldBeFalse(); // PureAdd — no rebuild; Apply still completed + provisioned } private static EquipmentTagPlan HistorizedTag(string displayName, string? historianName, string dataType, string fullName = "ref") 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 96433384..97dfefc5 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierTests.cs @@ -25,9 +25,12 @@ public sealed class AddressSpaceApplierTests sink.AlarmWrites.ShouldBeEmpty(); } - /// Verifies that removed equipment writes inactive alarm state and triggers rebuild. + /// R2-07 T11 — removed equipment is a PureRemove: the applier writes each id's terminal + /// "no-event" condition state (top-of-Apply block) then tears down each equipment SUBTREE in place via + /// RemoveEquipmentSubtree — NO full rebuild (other clients' subscriptions survive). (Supersedes the + /// pre-R2-07 "removed equipment ⇒ rebuild" pin.) [Fact] - public void Removed_equipment_writes_inactive_alarm_state_per_id_and_triggers_rebuild() + public void Removed_equipment_writes_terminal_state_and_removes_subtree_without_rebuild() { var sink = new RecordingSink(); var applier = new AddressSpaceApplier(sink, NullLogger.Instance); @@ -36,16 +39,20 @@ public sealed class AddressSpaceApplierTests var outcome = applier.Apply(plan); outcome.RemovedNodes.ShouldBe(2); - outcome.RebuildCalled.ShouldBeTrue(); + outcome.RebuildCalled.ShouldBeFalse(); + sink.RebuildCalls.ShouldBe(0); + // Terminal "no-event" condition state written per id (inactive + acked + confirmed). sink.AlarmWrites.Select(a => a.NodeId).OrderBy(x => x).ShouldBe(new[] { "eq-1", "eq-2" }); - // Removed nodes are reset to the "no-event" state: inactive + acked + confirmed + enabled. sink.AlarmWrites.All(a => !a.State.Active && a.State.Acknowledged && a.State.Confirmed).ShouldBeTrue(); - sink.RebuildCalls.ShouldBe(1); + // Each equipment torn down as a subtree. + sink.RemoveCalls.OrderBy(x => x.NodeId).ShouldBe(new[] { ("equipment", "eq-1"), ("equipment", "eq-2") }); } - /// Verifies that added equipment triggers rebuild without writing alarm state. + /// R2-07 T2 — added equipment is a PureAdd: the applier SKIPS the rebuild (the idempotent + /// Materialise passes create the new folder; existing client subscriptions survive) and writes no alarm + /// state. (Supersedes the pre-R2-07 "added equipment ⇒ rebuild" pin.) [Fact] - public void Added_equipment_triggers_rebuild_without_alarm_writes() + public void Added_equipment_is_pure_add_and_skips_rebuild() { var sink = new RecordingSink(); var applier = new AddressSpaceApplier(sink, NullLogger.Instance); @@ -63,10 +70,10 @@ public sealed class AddressSpaceApplierTests var outcome = applier.Apply(plan); - outcome.RebuildCalled.ShouldBeTrue(); + outcome.RebuildCalled.ShouldBeFalse(); // PureAdd — no teardown, subscriptions preserved outcome.AddedNodes.ShouldBe(1); sink.AlarmWrites.ShouldBeEmpty(); - sink.RebuildCalls.ShouldBe(1); + sink.RebuildCalls.ShouldBe(0); } /// Verifies that driver-only changes do not trigger address space rebuild. @@ -675,11 +682,11 @@ public sealed class AddressSpaceApplierTests sink.ModelChangeCalls.ShouldBeEmpty(); } - /// Verifies that added equipment tags in an otherwise-empty plan trigger an - /// address-space rebuild (the planner now diffs equipment tags, so a tags-only deploy is no - /// longer a silent no-op). + /// R2-07 T2 — added equipment tags in an otherwise-empty plan are a PureAdd: the applier + /// SKIPS the rebuild (the idempotent Materialise passes create the new variables; existing subscriptions + /// survive). (Supersedes the pre-R2-07 "added tags ⇒ rebuild" pin.) [Fact] - public void Added_equipment_tags_trigger_rebuild() + public void Added_equipment_tags_are_pure_add_and_skip_rebuild() { var sink = new RecordingSink(); var applier = new AddressSpaceApplier(sink, NullLogger.Instance); @@ -694,16 +701,16 @@ public sealed class AddressSpaceApplierTests var outcome = applier.Apply(plan); - outcome.RebuildCalled.ShouldBeTrue(); + outcome.RebuildCalled.ShouldBeFalse(); outcome.AddedNodes.ShouldBe(1); - sink.RebuildCalls.ShouldBe(1); + sink.RebuildCalls.ShouldBe(0); } - /// Verifies that added Equipment VirtualTags in an otherwise-empty plan trigger an - /// address-space rebuild (parity with the equipment-tag path — the planner now diffs VirtualTags, - /// so a VirtualTag-only deploy is no longer a silent no-op). + /// R2-07 T2 — added Equipment VirtualTags in an otherwise-empty plan are a PureAdd: the applier + /// SKIPS the rebuild (parity with the equipment-tag path). (Supersedes the pre-R2-07 "added vtags ⇒ + /// rebuild" pin.) [Fact] - public void Added_equipment_virtual_tags_trigger_rebuild() + public void Added_equipment_virtual_tags_are_pure_add_and_skip_rebuild() { var sink = new RecordingSink(); var applier = new AddressSpaceApplier(sink, NullLogger.Instance); @@ -719,8 +726,289 @@ public sealed class AddressSpaceApplierTests var outcome = applier.Apply(plan); - outcome.RebuildCalled.ShouldBeTrue(); + outcome.RebuildCalled.ShouldBeFalse(); outcome.AddedNodes.ShouldBe(1); + sink.RebuildCalls.ShouldBe(0); + } + + /// R2-07 T2 — a PureAdd plan carrying a coincident surgical tag delta (only Writable flips on + /// an existing tag) SKIPS the rebuild AND applies the in-place tag update via the surgical sink. The + /// classifier routes adds+surgical-change to PureAdd (rule 2 clears because the change is + /// surgical-eligible); the applier still runs the F10b surgical pass. + [Fact] + public void Pure_add_with_coincident_surgical_tag_delta_skips_rebuild_and_updates_in_place() + { + var sink = new RecordingSink(); + var applier = new AddressSpaceApplier(sink, NullLogger.Instance); + + var plan = EmptyPlan with + { + AddedEquipmentTags = new[] + { + new EquipmentTagPlan("tag-new", "eq-1", "drv", FolderPath: "", Name: "New", DataType: "Float", FullName: "40009", Writable: false, Alarm: null), + }, + ChangedEquipmentTags = new[] + { + new AddressSpacePlan.EquipmentTagDelta( + new EquipmentTagPlan("tag-1", "eq-1", "drv", FolderPath: "", Name: "Speed", DataType: "Float", FullName: "40001", Writable: false, Alarm: null), + new EquipmentTagPlan("tag-1", "eq-1", "drv", FolderPath: "", Name: "Speed", DataType: "Float", FullName: "40001", Writable: true, Alarm: null)), + }, + }; + + var outcome = applier.Apply(plan); + + outcome.RebuildCalled.ShouldBeFalse(); + sink.RebuildCalls.ShouldBe(0); + var call = sink.SurgicalCalls.ShouldHaveSingleItem(); + call.NodeId.ShouldBe(EquipmentNodeIds.Variable("eq-1", "", "Speed")); + call.Writable.ShouldBeTrue(); + } + + /// R2-07 T2 — during a PureAdd apply, if a coincident surgical tag update reports the node + /// missing (returns false), the applier falls back to a full rebuild (the F10b safety valve is + /// preserved through the classifier routing). + [Fact] + public void Pure_add_with_surgical_returns_false_falls_back_to_rebuild() + { + var sink = new RecordingSink { SurgicalReturns = false }; + var applier = new AddressSpaceApplier(sink, NullLogger.Instance); + + var plan = EmptyPlan with + { + AddedEquipmentTags = new[] + { + new EquipmentTagPlan("tag-new", "eq-1", "drv", FolderPath: "", Name: "New", DataType: "Float", FullName: "40009", Writable: false, Alarm: null), + }, + ChangedEquipmentTags = new[] + { + new AddressSpacePlan.EquipmentTagDelta( + new EquipmentTagPlan("tag-1", "eq-1", "drv", FolderPath: "", Name: "Speed", DataType: "Float", FullName: "40001", Writable: false, Alarm: null), + new EquipmentTagPlan("tag-1", "eq-1", "drv", FolderPath: "", Name: "Speed", DataType: "Float", FullName: "40001", Writable: true, Alarm: null)), + }, + }; + + var outcome = applier.Apply(plan); + + outcome.RebuildCalled.ShouldBeTrue(); // fell back to a full rebuild + sink.RebuildCalls.ShouldBe(1); + sink.SurgicalCalls.ShouldHaveSingleItem(); // the surgical update was attempted first + } + + /// R2-07 T11 — a remove-one-tag plan is a PureRemove: the applier writes a terminal Bad to the + /// removed variable then removes it in place via RemoveVariableNode — NO rebuild. (Supersedes the T2 + /// phase pin.) + [Fact] + public void Removed_equipment_tag_writes_terminal_bad_and_removes_variable_without_rebuild() + { + var sink = new RecordingSink(); + var applier = new AddressSpaceApplier(sink, NullLogger.Instance); + + var plan = EmptyPlan with + { + RemovedEquipmentTags = new[] + { + new EquipmentTagPlan("tag-1", "eq-1", "drv", FolderPath: "", Name: "Speed", DataType: "Float", FullName: "40001", Writable: false, Alarm: null), + }, + }; + + var outcome = applier.Apply(plan); + + outcome.RebuildCalled.ShouldBeFalse(); + sink.RebuildCalls.ShouldBe(0); + var nodeId = EquipmentNodeIds.Variable("eq-1", "", "Speed"); + // Terminal Bad written to the removed variable BEFORE the removal. + sink.ValueWrites.ShouldContain((nodeId, OpcUaQuality.Bad)); + sink.RemoveCalls.ShouldHaveSingleItem().ShouldBe(("var", nodeId)); + } + + /// R2-07 T11 — a removed alarm-bearing tag writes the terminal RemovedConditionState (closing + /// the pre-R2-07 today-gap where a removed alarm tag got no condition write) then removes the CONDITION + /// node in place (not a value variable). + [Fact] + public void Removed_alarm_bearing_tag_writes_removed_condition_state_and_removes_condition() + { + var sink = new RecordingSink(); + var applier = new AddressSpaceApplier(sink, NullLogger.Instance); + + var plan = EmptyPlan with + { + RemovedEquipmentTags = new[] + { + new EquipmentTagPlan("tag-alm", "eq-1", "drv", FolderPath: "", Name: "OverTemp", DataType: "Boolean", FullName: "00001", Writable: false, + Alarm: new EquipmentTagAlarmInfo("OffNormalAlarm", 700)), + }, + }; + + var outcome = applier.Apply(plan); + + outcome.RebuildCalled.ShouldBeFalse(); + var nodeId = EquipmentNodeIds.Variable("eq-1", "", "OverTemp"); + // Terminal RemovedConditionState (inactive/acked/confirmed) written to the condition node. + var write = sink.AlarmWrites.ShouldHaveSingleItem(); + write.NodeId.ShouldBe(nodeId); + (!write.State.Active && write.State.Acknowledged && write.State.Confirmed).ShouldBeTrue(); + // Removed as a condition node, not a value variable. + sink.RemoveCalls.ShouldHaveSingleItem().ShouldBe(("alarm", nodeId)); + sink.ValueWrites.ShouldNotContain(w => w.NodeId == nodeId); + } + + /// R2-07 T11 — a removed child (tag/vtag/alarm) whose equipment is ALSO removed is SUBSUMED by + /// the equipment-subtree removal: only RemoveEquipmentSubtree fires, no individual child remove. + [Fact] + public void Removed_child_under_removed_equipment_is_subsumed_by_subtree_removal() + { + var sink = new RecordingSink(); + var applier = new AddressSpaceApplier(sink, NullLogger.Instance); + + var plan = EmptyPlan with + { + RemovedEquipment = new[] { new EquipmentNode("eq-1", "One", "line-1") }, + RemovedEquipmentTags = new[] + { + new EquipmentTagPlan("tag-1", "eq-1", "drv", FolderPath: "", Name: "Speed", DataType: "Float", FullName: "40001", Writable: false, Alarm: null), + }, + RemovedEquipmentVirtualTags = new[] + { + new EquipmentVirtualTagPlan("vt-1", "eq-1", FolderPath: "", Name: "Eff", DataType: "Float", Expression: "a", DependencyRefs: new[] { "a" }), + }, + }; + + var outcome = applier.Apply(plan); + + outcome.RebuildCalled.ShouldBeFalse(); + // ONLY the subtree removal fires — no individual var removal for the subsumed child tag/vtag. + sink.RemoveCalls.ShouldHaveSingleItem().ShouldBe(("equipment", "eq-1")); + } + + /// R2-07 T11 — if a surgical remove reports the node unknown (RemoveReturns=false), the applier + /// falls back to a full rebuild (the one-way ratchet) and attempts no further surgical removes. + [Fact] + public void Pure_remove_with_remove_returns_false_falls_back_to_rebuild() + { + var sink = new RecordingSink { RemoveReturns = false }; + var applier = new AddressSpaceApplier(sink, NullLogger.Instance); + + var plan = EmptyPlan with + { + RemovedEquipmentTags = new[] + { + new EquipmentTagPlan("tag-1", "eq-1", "drv", FolderPath: "", Name: "A", DataType: "Float", FullName: "1", Writable: false, Alarm: null), + new EquipmentTagPlan("tag-2", "eq-1", "drv", FolderPath: "", Name: "B", DataType: "Float", FullName: "2", Writable: false, Alarm: null), + }, + }; + + var outcome = applier.Apply(plan); + + outcome.RebuildCalled.ShouldBeTrue(); + sink.RebuildCalls.ShouldBe(1); + // Ratchet: stopped at the FIRST failed remove — did not attempt the second. + sink.RemoveCalls.Count.ShouldBe(1); + } + + /// R2-07 T11 — a PureRemove on a sink lacking the surgical capability falls back to a full + /// rebuild (safe default), same as the F10b attribute path. + [Fact] + public void Pure_remove_on_non_surgical_sink_rebuilds() + { + var sink = new PlainRecordingSink(); + var applier = new AddressSpaceApplier(sink, NullLogger.Instance); + + var plan = EmptyPlan with + { + RemovedEquipmentTags = new[] + { + new EquipmentTagPlan("tag-1", "eq-1", "drv", FolderPath: "", Name: "A", DataType: "Float", FullName: "1", Writable: false, Alarm: null), + }, + }; + + var outcome = applier.Apply(plan); + + outcome.RebuildCalled.ShouldBeTrue(); + sink.RebuildCalls.ShouldBe(1); + } + + /// R2-07 T13 — an add+remove plan is an AddRemoveMix: the applier removes the old node IN PLACE + /// (RemoveVariableNode) and does NOT rebuild; the add is materialised afterward by the publish actor's + /// passes. (Supersedes the T2 phase pin.) + [Fact] + public void Add_and_remove_is_mixed_removes_in_place_without_rebuild() + { + var sink = new RecordingSink(); + var applier = new AddressSpaceApplier(sink, NullLogger.Instance); + + var plan = EmptyPlan with + { + AddedEquipmentTags = new[] + { + new EquipmentTagPlan("tag-new", "eq-1", "drv", FolderPath: "", Name: "New", DataType: "Float", FullName: "40009", Writable: false, Alarm: null), + }, + RemovedEquipmentTags = new[] + { + new EquipmentTagPlan("tag-old", "eq-1", "drv", FolderPath: "", Name: "Old", DataType: "Float", FullName: "40001", Writable: false, Alarm: null), + }, + }; + + var outcome = applier.Apply(plan); + + outcome.RebuildCalled.ShouldBeFalse(); + sink.RebuildCalls.ShouldBe(0); + // The removed node is torn down in place; the add is left to the publish actor's materialise passes. + sink.RemoveCalls.ShouldHaveSingleItem().ShouldBe(("var", EquipmentNodeIds.Variable("eq-1", "", "Old"))); + outcome.AddedNodes.ShouldBe(1); + outcome.RemovedNodes.ShouldBe(1); + } + + /// R2-07 T13 — an id REUSED across the remove + add sets in one deploy (a tag removed at the + /// same folder-scoped NodeId another tag is added at): the applier removes the old node in place (the + /// recreate is the publish actor's job, a fresh NodeState). No rebuild. + [Fact] + public void Add_and_remove_reusing_same_node_id_removes_the_old_node_without_rebuild() + { + var sink = new RecordingSink(); + var applier = new AddressSpaceApplier(sink, NullLogger.Instance); + + var nodeId = EquipmentNodeIds.Variable("eq-1", "", "Slot"); + var plan = EmptyPlan with + { + // Different TagId, but both resolve to the SAME folder-scoped NodeId (eq-1/Slot). + RemovedEquipmentTags = new[] + { + new EquipmentTagPlan("tag-old", "eq-1", "drv", FolderPath: "", Name: "Slot", DataType: "Float", FullName: "40001", Writable: false, Alarm: null), + }, + AddedEquipmentTags = new[] + { + new EquipmentTagPlan("tag-new", "eq-1", "drv", FolderPath: "", Name: "Slot", DataType: "Int32", FullName: "40002", Writable: false, Alarm: null), + }, + }; + + var outcome = applier.Apply(plan); + + outcome.RebuildCalled.ShouldBeFalse(); + sink.RemoveCalls.ShouldHaveSingleItem().ShouldBe(("var", nodeId)); + } + + /// R2-07 T13 — a remove failure inside an AddRemoveMix still trips the rebuild ratchet. + [Fact] + public void Add_and_remove_with_remove_returns_false_falls_back_to_rebuild() + { + var sink = new RecordingSink { RemoveReturns = false }; + var applier = new AddressSpaceApplier(sink, NullLogger.Instance); + + var plan = EmptyPlan with + { + AddedEquipmentTags = new[] + { + new EquipmentTagPlan("tag-new", "eq-1", "drv", FolderPath: "", Name: "New", DataType: "Float", FullName: "40009", Writable: false, Alarm: null), + }, + RemovedEquipmentTags = new[] + { + new EquipmentTagPlan("tag-old", "eq-1", "drv", FolderPath: "", Name: "Old", DataType: "Float", FullName: "40001", Writable: false, Alarm: null), + }, + }; + + var outcome = applier.Apply(plan); + + outcome.RebuildCalled.ShouldBeTrue(); sink.RebuildCalls.ShouldBe(1); } @@ -1163,9 +1451,14 @@ public sealed class AddressSpaceApplierTests var outcome = applier.Apply(plan); - outcome.RebuildCalled.ShouldBeTrue(); - sink.RebuildCalls.ShouldBe(1); - outcome.RemovedNodes.ShouldBe(2); // both removals counted (was 0 before the fix) + // R2-07 T11 — a removed-tag + removed-vtag plan is a PureRemove: both variables are torn down in + // place (RemoveVariableNode ×2), NO full rebuild; both removals are still counted. + outcome.RebuildCalled.ShouldBeFalse(); + sink.RebuildCalls.ShouldBe(0); + outcome.RemovedNodes.ShouldBe(2); // both removals counted + sink.RemoveCalls.Count.ShouldBe(2); + sink.RemoveCalls.ShouldContain(("var", EquipmentNodeIds.Variable("eq-1", "", "Speed"))); + sink.RemoveCalls.ShouldContain(("var", EquipmentNodeIds.Variable("eq-1", "", "Efficiency"))); } // ----- F10b: surgical in-place tag-attribute writes (Writable / IsHistorized / HistorianTagname) ----- @@ -1496,11 +1789,12 @@ public sealed class AddressSpaceApplierTests outcome.ChangedNodes.ShouldBe(2); } - /// F10b — a surgical-eligible tag delta MIXED with another change (here an added equipment) - /// must still rebuild: the rebuild is forced by the OTHER change. The surgical path is taken ONLY when - /// the tag deltas are the sole change. No surgical call is made (the rebuild materialises everything). + /// R2-07 T2 — a surgical-eligible tag delta MIXED with an added equipment is now a PureAdd: + /// the added equipment is created by the Materialise passes and the surgical tag change is applied IN + /// PLACE, so NO rebuild fires and exactly one surgical call lands. (Supersedes the pre-R2-07 + /// "surgical-mixed-with-add ⇒ rebuild" pin — an add no longer forces a rebuild.) [Fact] - public void Surgical_eligible_tag_delta_mixed_with_added_equipment_rebuilds() + public void Surgical_eligible_tag_delta_mixed_with_added_equipment_is_pure_add_and_skips_rebuild() { var sink = new RecordingSink(); var applier = new AddressSpaceApplier(sink, NullLogger.Instance); @@ -1529,9 +1823,9 @@ public sealed class AddressSpaceApplierTests var outcome = applier.Apply(plan); - outcome.RebuildCalled.ShouldBeTrue(); - sink.RebuildCalls.ShouldBe(1); - sink.SurgicalCalls.ShouldBeEmpty(); + outcome.RebuildCalled.ShouldBeFalse(); + sink.RebuildCalls.ShouldBe(0); + sink.SurgicalCalls.ShouldHaveSingleItem().Writable.ShouldBeTrue(); } /// F10b fallback — a sink that does NOT implement cannot @@ -1632,11 +1926,12 @@ public sealed class AddressSpaceApplierTests 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. + /// R2-07 T2 — a folder rename MIXED with an added equipment is now a PureAdd: the added + /// equipment is created by the Materialise passes and the rename is applied IN PLACE via the surgical + /// sink, so NO rebuild fires and exactly one folder-rename call lands. (Supersedes the pre-R2-07 + /// "rename-mixed-with-add ⇒ rebuild" pin.) [Fact] - public void Folder_rename_mixed_with_added_equipment_rebuilds() + public void Folder_rename_mixed_with_added_equipment_is_pure_add_and_skips_rebuild() { var sink = new RecordingSink(); var applier = new AddressSpaceApplier(sink, NullLogger.Instance); @@ -1647,7 +1942,8 @@ public sealed class AddressSpaceApplierTests EquipmentNodes: Array.Empty(), DriverInstancePlans: Array.Empty(), ScriptedAlarmPlans: Array.Empty()); - // Area renamed AND a brand-new equipment node — the structural add forces a rebuild. + // Area renamed AND a brand-new equipment node — an add no longer forces a rebuild; the rename + // rides the in-place surgical path alongside the PureAdd. var next = new AddressSpaceComposition( UnsAreas: new[] { new UnsAreaProjection("area-1", "South") }, UnsLines: Array.Empty(), @@ -1661,9 +1957,9 @@ public sealed class AddressSpaceApplierTests var outcome = applier.Apply(plan); - outcome.RebuildCalled.ShouldBeTrue(); - sink.RebuildCalls.ShouldBe(1); - sink.FolderRenameCalls.ShouldBeEmpty(); // no surgical folder call — rebuild covers it + outcome.RebuildCalled.ShouldBeFalse(); + sink.RebuildCalls.ShouldBe(0); + sink.FolderRenameCalls.ShouldHaveSingleItem().ShouldBe(("area-1", "South")); } /// OpcUaServer-001 fallback — a rename-only plan on a sink that does NOT implement @@ -1709,6 +2005,105 @@ public sealed class AddressSpaceApplierTests sink.FolderRenameCalls.ShouldHaveSingleItem(); // the surgical update was attempted first } + // ----- R2-07 T3: pure-add NodeAdded announcements ----- + + /// Two tags added under the SAME equipment (no FolderPath) dedup to ONE announcement for the + /// equipment id. + [Fact] + public void ComputeAddAnnouncements_two_tags_under_same_equipment_dedup_to_one() + { + var applier = new AddressSpaceApplier(new RecordingSink(), NullLogger.Instance); + + var plan = EmptyPlan with + { + AddedEquipmentTags = new[] + { + new EquipmentTagPlan("t1", "eq-1", "drv", FolderPath: "", Name: "A", DataType: "Float", FullName: "1", Writable: false, Alarm: null), + new EquipmentTagPlan("t2", "eq-1", "drv", FolderPath: "", Name: "B", DataType: "Float", FullName: "2", Writable: false, Alarm: null), + }, + }; + + applier.ComputeAddAnnouncements(plan).ShouldBe(new[] { "eq-1" }); + } + + /// A tag with a FolderPath announces its SUB-FOLDER (the materialise parent), not the equipment. + [Fact] + public void ComputeAddAnnouncements_tag_with_folder_path_announces_subfolder() + { + var applier = new AddressSpaceApplier(new RecordingSink(), NullLogger.Instance); + + var plan = EmptyPlan with + { + AddedEquipmentTags = new[] + { + new EquipmentTagPlan("t1", "eq-1", "drv", FolderPath: "Diag", Name: "A", DataType: "Float", FullName: "1", Writable: false, Alarm: null), + }, + }; + + applier.ComputeAddAnnouncements(plan).ShouldBe(new[] { EquipmentNodeIds.SubFolder("eq-1", "Diag") }); + } + + /// An added scripted alarm announces its equipment folder (where its condition node parents). + [Fact] + public void ComputeAddAnnouncements_added_alarm_announces_equipment() + { + var applier = new AddressSpaceApplier(new RecordingSink(), NullLogger.Instance); + + var plan = EmptyPlan with + { + AddedAlarms = new[] { new ScriptedAlarmPlan("alm-1", "eq-9", "scr", "msg") }, + }; + + applier.ComputeAddAnnouncements(plan).ShouldBe(new[] { "eq-9" }); + } + + /// An added equipment WITH a UnsLineId announces its parent line; WITHOUT one announces its own + /// new folder id (a valid Part 3 NodeAdded of the folder itself; the root has no announceable id). + [Fact] + public void ComputeAddAnnouncements_added_equipment_announces_line_or_self() + { + var applier = new AddressSpaceApplier(new RecordingSink(), NullLogger.Instance); + + var withLine = EmptyPlan with { AddedEquipment = new[] { new EquipmentNode("eq-1", "One", "line-7") } }; + applier.ComputeAddAnnouncements(withLine).ShouldBe(new[] { "line-7" }); + + var noLine = EmptyPlan with { AddedEquipment = new[] { new EquipmentNode("eq-2", "Two", "") } }; + applier.ComputeAddAnnouncements(noLine).ShouldBe(new[] { "eq-2" }); + } + + /// A plan with no additions yields no announcements. + [Fact] + public void ComputeAddAnnouncements_empty_add_plan_yields_none() + { + var applier = new AddressSpaceApplier(new RecordingSink(), NullLogger.Instance); + applier.ComputeAddAnnouncements(EmptyPlan).ShouldBeEmpty(); + } + + /// AnnounceAddedNodes raises exactly one RaiseNodesAddedModelChange per DISTINCT affected + /// parent (dedup + subfolder-parent + added-equipment cases), Safe-wrapped. + [Fact] + public void AnnounceAddedNodes_raises_one_model_change_per_distinct_parent() + { + var sink = new RecordingSink(); + var applier = new AddressSpaceApplier(sink, NullLogger.Instance); + + var plan = EmptyPlan with + { + AddedEquipment = new[] { new EquipmentNode("eq-2", "Two", "line-7") }, + AddedEquipmentTags = new[] + { + new EquipmentTagPlan("t1", "eq-1", "drv", FolderPath: "", Name: "A", DataType: "Float", FullName: "1", Writable: false, Alarm: null), + new EquipmentTagPlan("t2", "eq-1", "drv", FolderPath: "", Name: "B", DataType: "Float", FullName: "2", Writable: false, Alarm: null), + new EquipmentTagPlan("t3", "eq-1", "drv", FolderPath: "Diag", Name: "C", DataType: "Float", FullName: "3", Writable: false, Alarm: null), + }, + }; + + applier.AnnounceAddedNodes(plan); + + sink.ModelChangeCalls.OrderBy(x => x).ShouldBe( + new[] { "eq-1", EquipmentNodeIds.SubFolder("eq-1", "Diag"), "line-7" }.OrderBy(x => x)); + } + private static AddressSpaceComposition CompositionWithAreas(params UnsAreaProjection[] areas) => new( areas, Array.Empty(), Array.Empty(), @@ -1798,6 +2193,35 @@ public sealed class AddressSpaceApplierTests return FolderRenameReturns; } + /// Gets the queue of surgical remove calls (kind + node id) in call order (R2-07 Phase 2). + public ConcurrentQueue<(string Kind, string NodeId)> RemoveQueue { get; } = new(); + /// Gets the list of recorded surgical remove calls. + public List<(string Kind, string NodeId)> RemoveCalls => RemoveQueue.ToList(); + /// When false, the Remove* members report the node missing (return false), driving the + /// applier's rebuild fallback. Defaults to true (node present, removal succeeds). + public bool RemoveReturns { get; init; } = true; + + /// Records a surgical variable-node removal; returns . + public bool RemoveVariableNode(string variableNodeId) + { + RemoveQueue.Enqueue(("var", variableNodeId)); + return RemoveReturns; + } + + /// Records a surgical alarm-condition-node removal; returns . + public bool RemoveAlarmConditionNode(string alarmNodeId) + { + RemoveQueue.Enqueue(("alarm", alarmNodeId)); + return RemoveReturns; + } + + /// Records a surgical equipment-subtree removal; returns . + public bool RemoveEquipmentSubtree(string equipmentNodeId) + { + RemoveQueue.Enqueue(("equipment", equipmentNodeId)); + return RemoveReturns; + } + /// Gets the queue of alarm condition write calls. public ConcurrentQueue<(string NodeId, AlarmConditionSnapshot State)> AlarmQueue { get; } = new(); /// Gets the queue of folder creation calls. @@ -1828,12 +2252,18 @@ public sealed class AddressSpaceApplierTests /// Gets the list of recorded alarm-condition materialise calls. public List<(string AlarmNodeId, string EquipmentNodeId, string DisplayName, string AlarmType, int Severity, bool IsNative)> AlarmConditionCalls => AlarmConditionQueue.ToList(); - /// Records a value write (no-op in this recording sink). + /// Gets the queue of value writes (NodeId, quality) — used to assert the PureRemove terminal Bad. + public ConcurrentQueue<(string NodeId, OpcUaQuality Quality)> ValueWriteQueue { get; } = new(); + /// Gets the list of recorded value writes. + public List<(string NodeId, OpcUaQuality Quality)> ValueWrites => ValueWriteQueue.ToList(); + + /// Records a value write (NodeId + quality). /// The node ID. /// The value to write. /// The OPC UA quality. /// The source timestamp in UTC. - public void WriteValue(string nodeId, object? value, OpcUaQuality quality, DateTime sourceTimestampUtc) { } + public void WriteValue(string nodeId, object? value, OpcUaQuality quality, DateTime sourceTimestampUtc) + => ValueWriteQueue.Enqueue((nodeId, quality)); /// Records an alarm condition write call. /// The alarm node ID. /// The full condition state snapshot. diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceChangeClassifierTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceChangeClassifierTests.cs new file mode 100644 index 00000000..24507eb6 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceChangeClassifierTests.cs @@ -0,0 +1,285 @@ +using Shouldly; +using Xunit; + +namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests; + +/// +/// R2-07 T1 — table-driven tests over . The +/// classifier is a pure function over an : it names the delta class the +/// applier routes each plan through (Empty / AttributeOnly / PureAdd / PureRemove / AddRemoveMix / +/// Rebuild). First-match-wins per the plan's classification table, with the default-closed +/// Rebuild safety valve catching every node-affecting change. +/// +public sealed class AddressSpaceChangeClassifierTests +{ + [Fact] + public void Empty_plan_classifies_as_Empty() + { + AddressSpaceChangeClassifier.Classify(Plan()).ShouldBe(AddressSpaceChangeKind.Empty); + } + + // ----- single add class ⇒ PureAdd ----- + + [Fact] + public void Added_equipment_only_is_PureAdd() + { + var plan = Plan(addedEquipment: new[] { Eq("eq-1") }); + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.PureAdd); + } + + [Fact] + public void Added_alarm_only_is_PureAdd() + { + var plan = Plan(addedAlarms: new[] { Alarm("alm-1") }); + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.PureAdd); + } + + [Fact] + public void Added_equipment_tag_only_is_PureAdd() + { + var plan = Plan(addedTags: new[] { Tag("tag-1", "eq-1") }); + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.PureAdd); + } + + [Fact] + public void Added_virtual_tag_only_is_PureAdd() + { + var plan = Plan(addedVtags: new[] { Vtag("vt-1", "eq-1") }); + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.PureAdd); + } + + // ----- single remove class ⇒ PureRemove ----- + + [Fact] + public void Removed_equipment_only_is_PureRemove() + { + var plan = Plan(removedEquipment: new[] { Eq("eq-1") }); + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.PureRemove); + } + + [Fact] + public void Removed_alarm_only_is_PureRemove() + { + var plan = Plan(removedAlarms: new[] { Alarm("alm-1") }); + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.PureRemove); + } + + [Fact] + public void Removed_equipment_tag_only_is_PureRemove() + { + var plan = Plan(removedTags: new[] { Tag("tag-1", "eq-1") }); + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.PureRemove); + } + + [Fact] + public void Removed_virtual_tag_only_is_PureRemove() + { + var plan = Plan(removedVtags: new[] { Vtag("vt-1", "eq-1") }); + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.PureRemove); + } + + // ----- add AND remove ⇒ AddRemoveMix ----- + + [Fact] + public void Add_and_remove_is_AddRemoveMix() + { + var plan = Plan(addedTags: new[] { Tag("tag-1", "eq-1") }, removedTags: new[] { Tag("tag-2", "eq-1") }); + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.AddRemoveMix); + } + + [Fact] + public void Add_equipment_and_remove_vtag_is_AddRemoveMix() + { + var plan = Plan(addedEquipment: new[] { Eq("eq-2") }, removedVtags: new[] { Vtag("vt-1", "eq-1") }); + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.AddRemoveMix); + } + + // ----- node-affecting change ⇒ Rebuild ----- + + [Fact] + public void Changed_equipment_is_Rebuild() + { + var plan = Plan(changedEquipment: new[] + { + new AddressSpacePlan.EquipmentDelta(Eq("eq-1"), Eq("eq-1") with { DisplayName = "New" }), + }); + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.Rebuild); + } + + [Fact] + public void Changed_alarm_is_Rebuild() + { + var plan = Plan(changedAlarms: new[] + { + new AddressSpacePlan.AlarmDelta(Alarm("alm-1"), Alarm("alm-1") with { MessageTemplate = "changed" }), + }); + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.Rebuild); + } + + [Fact] + public void Non_surgical_changed_tag_is_Rebuild() + { + // FullName re-routes the node to a different driver point ⇒ NOT surgical-eligible ⇒ Rebuild. + var plan = Plan(changedTags: new[] + { + new AddressSpacePlan.EquipmentTagDelta( + Tag("tag-1", "eq-1", fullName: "40001"), + Tag("tag-1", "eq-1", fullName: "40002")), + }); + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.Rebuild); + } + + [Fact] + public void Node_relevant_changed_vtag_is_Rebuild() + { + // Name change re-derives the NodeId ⇒ node-relevant ⇒ Rebuild. + var plan = Plan(changedVtags: new[] + { + new AddressSpacePlan.EquipmentVirtualTagDelta( + Vtag("vt-1", "eq-1", name: "A"), + Vtag("vt-1", "eq-1", name: "B")), + }); + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.Rebuild); + } + + // ----- surgical-only / rename-only / driver-only ⇒ AttributeOnly ----- + + [Fact] + public void Surgical_eligible_changed_tag_only_is_AttributeOnly() + { + // Only Writable flips ⇒ surgical-eligible ⇒ AttributeOnly (no adds, no removes). + var plan = Plan(changedTags: new[] + { + new AddressSpacePlan.EquipmentTagDelta( + Tag("tag-1", "eq-1", writable: false), + Tag("tag-1", "eq-1", writable: true)), + }); + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.AttributeOnly); + } + + [Fact] + public void Node_irrelevant_changed_vtag_only_is_AttributeOnly() + { + // Only Expression differs ⇒ node-irrelevant ⇒ AttributeOnly. + var plan = Plan(changedVtags: new[] + { + new AddressSpacePlan.EquipmentVirtualTagDelta( + Vtag("vt-1", "eq-1", expression: "a"), + Vtag("vt-1", "eq-1", expression: "a * 2")), + }); + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.AttributeOnly); + } + + [Fact] + public void Folder_renames_only_is_AttributeOnly() + { + var plan = Plan(renamedFolders: new[] { new AddressSpacePlan.FolderRename("area-1", "New Area") }); + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.AttributeOnly); + } + + [Fact] + public void Driver_only_deltas_are_AttributeOnly_node_inert() + { + // Driver adds/removes/changes never touch the address space — excluded from hasAdds/hasRemoves. + var plan = Plan( + addedDrivers: new[] { new DriverInstancePlan("d-1", "Modbus", "{}") }, + removedDrivers: new[] { new DriverInstancePlan("d-2", "Modbus", "{}") }, + changedDrivers: new[] + { + new AddressSpacePlan.DriverDelta( + new DriverInstancePlan("d-3", "Modbus", "{\"v\":1}"), + new DriverInstancePlan("d-3", "Modbus", "{\"v\":2}")), + }); + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.AttributeOnly); + } + + // ----- mixed-with-surgical: adds still win over coincident surgical changes ----- + + [Fact] + public void Adds_with_coincident_surgical_change_is_PureAdd() + { + var plan = Plan( + addedTags: new[] { Tag("tag-new", "eq-1") }, + changedTags: new[] + { + new AddressSpacePlan.EquipmentTagDelta( + Tag("tag-1", "eq-1", writable: false), + Tag("tag-1", "eq-1", writable: true)), + }); + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.PureAdd); + } + + [Fact] + public void Adds_with_coincident_non_surgical_change_is_Rebuild() + { + // Rule 2 (node-affecting change) is evaluated BEFORE the add/remove split, so a non-surgical + // change forces Rebuild even alongside pure adds. + var plan = Plan( + addedTags: new[] { Tag("tag-new", "eq-1") }, + changedTags: new[] + { + new AddressSpacePlan.EquipmentTagDelta( + Tag("tag-1", "eq-1", fullName: "40001"), + Tag("tag-1", "eq-1", fullName: "40002")), + }); + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.Rebuild); + } + + [Fact] + public void Adds_with_coincident_folder_rename_is_PureAdd() + { + var plan = Plan( + addedTags: new[] { Tag("tag-new", "eq-1") }, + renamedFolders: new[] { new AddressSpacePlan.FolderRename("area-1", "New Area") }); + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.PureAdd); + } + + // ----- construction helpers ----- + + private static EquipmentNode Eq(string id) => new(id, id, "line-1"); + + private static ScriptedAlarmPlan Alarm(string id) => new(id, "eq-1", "scr-1", "msg"); + + private static EquipmentTagPlan Tag(string id, string equipmentId, string fullName = "40001", bool writable = false) => + new(id, equipmentId, "drv", FolderPath: "", Name: "Speed", DataType: "Float", FullName: fullName, Writable: writable, Alarm: null); + + private static EquipmentVirtualTagPlan Vtag(string id, string equipmentId, string name = "Efficiency", string expression = "ctx.GetTag(\"a\")") => + new(id, equipmentId, FolderPath: "", Name: name, DataType: "Float64", Expression: expression, DependencyRefs: new[] { "a" }); + + private static AddressSpacePlan Plan( + IReadOnlyList? addedEquipment = null, + IReadOnlyList? removedEquipment = null, + IReadOnlyList? changedEquipment = null, + IReadOnlyList? addedDrivers = null, + IReadOnlyList? removedDrivers = null, + IReadOnlyList? changedDrivers = null, + IReadOnlyList? addedAlarms = null, + IReadOnlyList? removedAlarms = null, + IReadOnlyList? changedAlarms = null, + IReadOnlyList? addedTags = null, + IReadOnlyList? removedTags = null, + IReadOnlyList? changedTags = null, + IReadOnlyList? addedVtags = null, + IReadOnlyList? removedVtags = null, + IReadOnlyList? changedVtags = null, + IReadOnlyList? renamedFolders = null) => + new( + addedEquipment ?? Array.Empty(), + removedEquipment ?? Array.Empty(), + changedEquipment ?? Array.Empty(), + addedDrivers ?? Array.Empty(), + removedDrivers ?? Array.Empty(), + changedDrivers ?? Array.Empty(), + addedAlarms ?? Array.Empty(), + removedAlarms ?? Array.Empty(), + changedAlarms ?? Array.Empty()) + { + AddedEquipmentTags = addedTags ?? Array.Empty(), + RemovedEquipmentTags = removedTags ?? Array.Empty(), + ChangedEquipmentTags = changedTags ?? Array.Empty(), + AddedEquipmentVirtualTags = addedVtags ?? Array.Empty(), + RemovedEquipmentVirtualTags = removedVtags ?? Array.Empty(), + ChangedEquipmentVirtualTags = changedVtags ?? Array.Empty(), + RenamedFolders = renamedFolders ?? Array.Empty(), + }; +} 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 2fedad28..d10a9f58 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/DeferredAddressSpaceSinkTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/DeferredAddressSpaceSinkTests.cs @@ -174,6 +174,40 @@ public sealed class DeferredAddressSpaceSinkTests ((ISurgicalAddressSpaceSink)deferred).UpdateFolderDisplayName("area-1", "X").ShouldBeFalse(); } + /// R2-07 Phase 2 — the three surgical remove members forward to a surgical inner with + /// arg-fidelity (kind + node id), returning the inner's own result; false when the inner is not surgical + /// so the caller falls back to a full rebuild. + [Fact] + public void Remove_members_forward_to_a_surgical_inner_sink_with_arg_fidelity() + { + var deferred = new DeferredAddressSpaceSink(); + var inner = new SurgicalRecordingSink { Result = true }; + deferred.SetSink(inner); + + ((ISurgicalAddressSpaceSink)deferred).RemoveVariableNode("eq-1/A").ShouldBeTrue(); + ((ISurgicalAddressSpaceSink)deferred).RemoveAlarmConditionNode("alm-1").ShouldBeTrue(); + ((ISurgicalAddressSpaceSink)deferred).RemoveEquipmentSubtree("eq-1").ShouldBeTrue(); + + inner.RemoveCalls.ShouldBe(new[] { ("var", "eq-1/A"), ("alarm", "alm-1"), ("equipment", "eq-1") }); + } + + /// The remove forwards return the inner's own result (false ⇒ id unknown ⇒ caller rebuilds), + /// and return false when the inner is not surgical at all. + [Fact] + public void Remove_members_return_inner_result_and_false_when_not_surgical() + { + var deferred = new DeferredAddressSpaceSink(); + deferred.SetSink(new SurgicalRecordingSink { Result = false }); + ((ISurgicalAddressSpaceSink)deferred).RemoveVariableNode("v-1").ShouldBeFalse(); + ((ISurgicalAddressSpaceSink)deferred).RemoveAlarmConditionNode("a-1").ShouldBeFalse(); + ((ISurgicalAddressSpaceSink)deferred).RemoveEquipmentSubtree("eq-1").ShouldBeFalse(); + + deferred.SetSink(null); + ((ISurgicalAddressSpaceSink)deferred).RemoveVariableNode("v-1").ShouldBeFalse(); + ((ISurgicalAddressSpaceSink)deferred).RemoveAlarmConditionNode("a-1").ShouldBeFalse(); + ((ISurgicalAddressSpaceSink)deferred).RemoveEquipmentSubtree("eq-1").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) => @@ -240,6 +274,16 @@ public sealed class DeferredAddressSpaceSinkTests return Result; } + /// Gets the recorded surgical remove calls (kind + node id), in call order (R2-07 Phase 2). + public List<(string Kind, string NodeId)> RemoveCalls { get; } = new(); + + /// + public bool RemoveVariableNode(string variableNodeId) { RemoveCalls.Add(("var", variableNodeId)); return Result; } + /// + public bool RemoveAlarmConditionNode(string alarmNodeId) { RemoveCalls.Add(("alarm", alarmNodeId)); return Result; } + /// + public bool RemoveEquipmentSubtree(string equipmentNodeId) { RemoveCalls.Add(("equipment", equipmentNodeId)); return Result; } + /// public void WriteValue(string nodeId, object? value, OpcUaQuality quality, DateTime sourceTimestampUtc) { } /// diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerAlarmIdempotentMaterialiseTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerAlarmIdempotentMaterialiseTests.cs new file mode 100644 index 00000000..c34a3151 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerAlarmIdempotentMaterialiseTests.cs @@ -0,0 +1,132 @@ +using Shouldly; +using Xunit; + +namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests; + +/// +/// R2-07 T4a — must be idempotent for a +/// same-id + same-kind re-apply: it KEEPS the existing AlarmConditionState instance instead of +/// dropping-and-recreating it. This is what preserves a client's MonitoredItems on an alarm-condition +/// node across a PURE-ADD deploy (the MaterialiseScriptedAlarms pass re-runs over the whole composition +/// every apply, re-touching every existing enabled condition). A genuine kind-swap (native↔scripted) +/// still recreates so the flip takes effect. +/// +public sealed class NodeManagerAlarmIdempotentMaterialiseTests : IDisposable +{ + private static CancellationToken Ct => TestContext.Current.CancellationToken; + + private readonly string _pkiRoot = Path.Combine( + Path.GetTempPath(), + $"otopcua-alarm-idempotent-{Guid.NewGuid():N}"); + + /// Re-materialising the SAME alarm id + kind returns the SAME instance (reference-equal) — the + /// condition node is not torn down, so subscriptions on it survive. + [Trait("Category", "Unit")] + [Fact] + public async Task Re_materialise_same_id_and_kind_keeps_the_same_instance() + { + var (host, server) = await BootAsync(); + var nm = server.NodeManager!; + nm.EnsureFolder("eq-1", parentNodeId: null, displayName: "Equipment 1"); + + nm.MaterialiseAlarmCondition("alm-1", "eq-1", "HighTemp", "OffNormalAlarm", 700, isNative: false); + var first = nm.TryGetAlarmCondition("alm-1"); + first.ShouldNotBeNull(); + + // Same id + same kind ⇒ skip-if-present: the existing instance is kept. + nm.MaterialiseAlarmCondition("alm-1", "eq-1", "HighTemp", "OffNormalAlarm", 700, isNative: false); + var second = nm.TryGetAlarmCondition("alm-1"); + + second.ShouldBeSameAs(first); + + await host.DisposeAsync(); + } + + /// A kind-swap (scripted → native) on the same id still RECREATES the node so the native flag + /// flips — the drop-and-recreate is preserved for the kind-swap case. + [Trait("Category", "Unit")] + [Fact] + public async Task Kind_swap_recreates_and_flips_native_flag() + { + var (host, server) = await BootAsync(); + var nm = server.NodeManager!; + nm.EnsureFolder("eq-1", parentNodeId: null, displayName: "Equipment 1"); + + nm.MaterialiseAlarmCondition("alm-1", "eq-1", "HighTemp", "OffNormalAlarm", 700, isNative: false); + var scripted = nm.TryGetAlarmCondition("alm-1"); + nm.IsNativeAlarmNode("alm-1").ShouldBeFalse(); + + // Same id but the OTHER kind ⇒ recreate (a different instance) and the native flag is now set. + nm.MaterialiseAlarmCondition("alm-1", "eq-1", "HighTemp", "OffNormalAlarm", 700, isNative: true); + var native = nm.TryGetAlarmCondition("alm-1"); + + native.ShouldNotBeSameAs(scripted); + nm.IsNativeAlarmNode("alm-1").ShouldBeTrue(); + + await host.DisposeAsync(); + } + + /// After a full cleared the maps, the next + /// materialise creates a FRESH instance (the skip-if-present guard sees no entry) — so a re-severity + /// arriving via the rebuild path is reflected. + [Trait("Category", "Unit")] + [Fact] + public async Task Materialise_after_rebuild_creates_fresh_instance() + { + var (host, server) = await BootAsync(); + var nm = server.NodeManager!; + nm.EnsureFolder("eq-1", parentNodeId: null, displayName: "Equipment 1"); + + nm.MaterialiseAlarmCondition("alm-1", "eq-1", "HighTemp", "OffNormalAlarm", 700, isNative: false); + var before = nm.TryGetAlarmCondition("alm-1"); + + nm.RebuildAddressSpace(); + nm.TryGetAlarmCondition("alm-1").ShouldBeNull(); + + nm.EnsureFolder("eq-1", parentNodeId: null, displayName: "Equipment 1"); + nm.MaterialiseAlarmCondition("alm-1", "eq-1", "HighTemp", "OffNormalAlarm", 700, isNative: false); + var after = nm.TryGetAlarmCondition("alm-1"); + + after.ShouldNotBeNull(); + after.ShouldNotBeSameAs(before); + + await host.DisposeAsync(); + } + + private async Task<(OpcUaApplicationHost Host, OtOpcUaSdkServer Server)> BootAsync() + { + var host = new OpcUaApplicationHost( + new OpcUaApplicationHostOptions + { + ApplicationName = "OtOpcUa.AlarmIdempotentTest", + ApplicationUri = $"urn:OtOpcUa.AlarmIdempotentTest:{Guid.NewGuid():N}", + OpcUaPort = AllocateFreePort(), + PublicHostname = "localhost", + PkiStoreRoot = _pkiRoot, + }, + Microsoft.Extensions.Logging.Abstractions.NullLogger.Instance); + + var server = new OtOpcUaSdkServer(); + await host.StartAsync(server, Ct); + return (host, server); + } + + private static int AllocateFreePort() + { + using var listener = new System.Net.Sockets.TcpListener(System.Net.IPAddress.Loopback, 0); + listener.Start(); + var port = ((System.Net.IPEndPoint)listener.LocalEndpoint).Port; + listener.Stop(); + return port; + } + + /// Cleans up the PKI root directory. + public void Dispose() + { + if (Directory.Exists(_pkiRoot)) + { + try { Directory.Delete(_pkiRoot, recursive: true); } + catch { /* best-effort cleanup */ } + } + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerSurgicalRemoveTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerSurgicalRemoveTests.cs new file mode 100644 index 00000000..fd225779 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerSurgicalRemoveTests.cs @@ -0,0 +1,220 @@ +using Opc.Ua; +using Shouldly; +using Xunit; + +namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests; + +/// +/// R2-07 Phase 2 (T8/T9/T10) — surgical IN-PLACE removal on the node manager: a single value variable +/// (), a single Part 9 alarm condition +/// (), and an equipment folder + its whole +/// descendant subtree with notifier demotion (). +/// Each removal detaches only the scoped node(s), cleans the matching maps, and raises a Part 3 +/// NodeDeleted model-change; an unknown id returns false (the caller falls back to a full rebuild). +/// +public sealed class NodeManagerSurgicalRemoveTests : IDisposable +{ + private static CancellationToken Ct => TestContext.Current.CancellationToken; + + private readonly string _pkiRoot = Path.Combine( + Path.GetTempPath(), + $"otopcua-surgical-remove-{Guid.NewGuid():N}"); + + // ---------- T8: RemoveVariableNode ---------- + + /// Ensure a variable then remove it: it disappears from the maps (TryGetVariable null, + /// VariableCount decremented), its historized-tagname registration is dropped, and the call returns + /// true. + [Trait("Category", "Unit")] + [Fact] + public async Task RemoveVariableNode_drops_variable_and_historian_registration() + { + var (host, server) = await BootAsync(); + var nm = server.NodeManager!; + nm.EnsureFolder("eq-1", parentNodeId: null, displayName: "Equipment 1"); + nm.EnsureVariable("eq-1/A", "eq-1", "A", "Float", writable: false, historianTagname: "Hist.A"); + nm.EnsureVariable("eq-1/B", "eq-1", "B", "Float", writable: false); + var countBefore = nm.VariableCount; + nm.TryGetHistorizedTagname("eq-1/A", out _).ShouldBeTrue(); + + nm.RemoveVariableNode("eq-1/A").ShouldBeTrue(); + + nm.TryGetVariable("eq-1/A").ShouldBeNull(); + nm.VariableCount.ShouldBe(countBefore - 1); + nm.TryGetHistorizedTagname("eq-1/A", out _).ShouldBeFalse(); + // The sibling variable is untouched. + nm.TryGetVariable("eq-1/B").ShouldNotBeNull(); + + await host.DisposeAsync(); + } + + /// Removing an unknown variable id returns false (map drift ⇒ caller rebuilds). + [Trait("Category", "Unit")] + [Fact] + public async Task RemoveVariableNode_unknown_id_returns_false() + { + var (host, server) = await BootAsync(); + var nm = server.NodeManager!; + + nm.RemoveVariableNode("eq-1/nope").ShouldBeFalse(); + + await host.DisposeAsync(); + } + + /// The built removed-node event announces the deleted node with verb NodeDeleted. + [Trait("Category", "Unit")] + [Fact] + public async Task Built_removed_event_announces_the_deleted_node_with_NodeDeleted_verb() + { + var (host, server) = await BootAsync(); + var nm = server.NodeManager!; + nm.EnsureFolder("eq-1", parentNodeId: null, displayName: "Equipment 1"); + nm.EnsureVariable("eq-1/A", "eq-1", "A", "Float", writable: false); + + var e = nm.BuildNodesRemovedModelChange("eq-1/A"); + + e.ShouldNotBeNull(); + e.Changes.ShouldNotBeNull(); + var changes = e.Changes.Value; + changes.Length.ShouldBe(1); + changes[0].Verb.ShouldBe((byte)ModelChangeStructureVerbMask.NodeDeleted); + + await host.DisposeAsync(); + } + + // ---------- T9: RemoveAlarmConditionNode ---------- + + /// Materialise a native alarm condition then remove it: it disappears from the condition map and + /// the native flag is cleared; unknown id returns false. + [Trait("Category", "Unit")] + [Fact] + public async Task RemoveAlarmConditionNode_drops_condition_and_native_flag() + { + var (host, server) = await BootAsync(); + var nm = server.NodeManager!; + nm.EnsureFolder("eq-1", parentNodeId: null, displayName: "Equipment 1"); + nm.MaterialiseAlarmCondition("eq-1/OverTemp", "eq-1", "OverTemp", "OffNormalAlarm", 700, isNative: true); + nm.TryGetAlarmCondition("eq-1/OverTemp").ShouldNotBeNull(); + nm.IsNativeAlarmNode("eq-1/OverTemp").ShouldBeTrue(); + + nm.RemoveAlarmConditionNode("eq-1/OverTemp").ShouldBeTrue(); + + nm.TryGetAlarmCondition("eq-1/OverTemp").ShouldBeNull(); + nm.IsNativeAlarmNode("eq-1/OverTemp").ShouldBeFalse(); + + nm.RemoveAlarmConditionNode("eq-1/OverTemp").ShouldBeFalse(); // already gone ⇒ false + + await host.DisposeAsync(); + } + + /// A scripted condition removes cleanly too (no native flag was set). + [Trait("Category", "Unit")] + [Fact] + public async Task RemoveAlarmConditionNode_removes_scripted_condition() + { + var (host, server) = await BootAsync(); + var nm = server.NodeManager!; + nm.EnsureFolder("eq-1", parentNodeId: null, displayName: "Equipment 1"); + nm.MaterialiseAlarmCondition("alm-1", "eq-1", "HighTemp", "OffNormalAlarm", 500, isNative: false); + + nm.RemoveAlarmConditionNode("alm-1").ShouldBeTrue(); + nm.TryGetAlarmCondition("alm-1").ShouldBeNull(); + + await host.DisposeAsync(); + } + + // ---------- T10: RemoveEquipmentSubtree ---------- + + /// Remove an equipment folder carrying a sub-folder, variables (one historized), and a condition: + /// every descendant disappears from every map, the notifier registration is demoted, and a SIBLING + /// equipment is fully intact. + [Trait("Category", "Unit")] + [Fact] + public async Task RemoveEquipmentSubtree_removes_all_descendants_and_leaves_siblings_intact() + { + var (host, server) = await BootAsync(); + var nm = server.NodeManager!; + + // Target equipment eq-1 with a sub-folder, two variables (one historized), and a native condition. + nm.EnsureFolder("eq-1", parentNodeId: null, displayName: "Equipment 1"); + nm.EnsureFolder("eq-1/Diag", parentNodeId: "eq-1", displayName: "Diag"); + nm.EnsureVariable("eq-1/A", "eq-1", "A", "Float", writable: false, historianTagname: "Hist.A"); + nm.EnsureVariable("eq-1/Diag/T", "eq-1/Diag", "T", "Float", writable: false); + nm.MaterialiseAlarmCondition("eq-1/OverTemp", "eq-1", "OverTemp", "OffNormalAlarm", 700, isNative: true); + + // Sibling equipment eq-2 that must survive untouched. + nm.EnsureFolder("eq-2", parentNodeId: null, displayName: "Equipment 2"); + nm.EnsureVariable("eq-2/S", "eq-2", "S", "Float", writable: false); + + nm.RemoveEquipmentSubtree("eq-1").ShouldBeTrue(); + + // Every eq-1 descendant is gone from every map. + nm.TryGetFolder("eq-1").ShouldBeNull(); + nm.TryGetFolder("eq-1/Diag").ShouldBeNull(); + nm.TryGetVariable("eq-1/A").ShouldBeNull(); + nm.TryGetVariable("eq-1/Diag/T").ShouldBeNull(); + nm.TryGetAlarmCondition("eq-1/OverTemp").ShouldBeNull(); + nm.TryGetHistorizedTagname("eq-1/A", out _).ShouldBeFalse(); + nm.IsNativeAlarmNode("eq-1/OverTemp").ShouldBeFalse(); + + // Sibling eq-2 is fully intact. + nm.TryGetFolder("eq-2").ShouldNotBeNull(); + nm.TryGetVariable("eq-2/S").ShouldNotBeNull(); + + // Re-materialising an alarm under eq-2 still works (the notifier machinery was not corrupted by the + // eq-1 demotion) — proves no orphaned root-notifier ref broke the event path. + Should.NotThrow(() => nm.MaterialiseAlarmCondition("eq-2/Alm", "eq-2", "Alm", "OffNormalAlarm", 300, isNative: false)); + + await host.DisposeAsync(); + } + + /// Removing an unknown equipment id returns false (map drift ⇒ caller rebuilds). + [Trait("Category", "Unit")] + [Fact] + public async Task RemoveEquipmentSubtree_unknown_id_returns_false() + { + var (host, server) = await BootAsync(); + var nm = server.NodeManager!; + + nm.RemoveEquipmentSubtree("eq-nope").ShouldBeFalse(); + + await host.DisposeAsync(); + } + + private async Task<(OpcUaApplicationHost Host, OtOpcUaSdkServer Server)> BootAsync() + { + var host = new OpcUaApplicationHost( + new OpcUaApplicationHostOptions + { + ApplicationName = "OtOpcUa.SurgicalRemoveTest", + ApplicationUri = $"urn:OtOpcUa.SurgicalRemoveTest:{Guid.NewGuid():N}", + OpcUaPort = AllocateFreePort(), + PublicHostname = "localhost", + PkiStoreRoot = _pkiRoot, + }, + Microsoft.Extensions.Logging.Abstractions.NullLogger.Instance); + + var server = new OtOpcUaSdkServer(); + await host.StartAsync(server, Ct); + return (host, server); + } + + private static int AllocateFreePort() + { + using var listener = new System.Net.Sockets.TcpListener(System.Net.IPAddress.Loopback, 0); + listener.Start(); + var port = ((System.Net.IPEndPoint)listener.LocalEndpoint).Port; + listener.Stop(); + return port; + } + + /// Cleans up the PKI root directory. + public void Dispose() + { + if (Directory.Exists(_pkiRoot)) + { + try { Directory.Delete(_pkiRoot, recursive: true); } + catch { /* best-effort cleanup */ } + } + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorApplyFailureTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorApplyFailureTests.cs index 2d401aaf..3bb225fa 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorApplyFailureTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorApplyFailureTests.cs @@ -25,7 +25,10 @@ namespace ZB.MOM.WW.OtOpcUa.Runtime.Tests.OpcUa; /// public sealed class OpcUaPublishActorApplyFailureTests : RuntimeActorTestBase { - /// A rebuild whose sink throws increments otopcua.opcua.apply.failed (kind=rebuild). + /// A rebuild whose sink throws increments otopcua.opcua.apply.failed (kind=rebuild). R2-07 — a + /// pure-add no longer rebuilds, so the degraded-rebuild path is exercised via a rebuild-forcing change: + /// a first PureAdd deploy establishes the applied composition, then a rename (ChangedEquipment ⇒ Rebuild) + /// drives the throwing SafeRebuild. [Fact] public void Rebuild_when_sink_throws_increments_apply_failed_meter() { @@ -33,10 +36,17 @@ public sealed class OpcUaPublishActorApplyFailureTests : RuntimeActorTestBase var db = NewInMemoryDbFactory(); var sink = new ThrowOnRebuildSink(); var applier = new AddressSpaceApplier(sink, NullLogger.Instance); - SeedEquipmentDeployment(db, "eq-1"); + var dep1 = SeedEquipmentDeployment(db, ("eq-1", "eq-1")); var actor = Sys.ActorOf(OpcUaPublishActor.PropsForTests(sink: sink, dbFactory: db, applier: applier)); - actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId())); + // First deploy: PureAdd — no rebuild, so the throwing sink is not hit yet. + actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId(), new DeploymentId(dep1))); + Thread.Sleep(200); + recorder.Total.ShouldBe(0); + + // Second deploy: eq-1 RENAMED ⇒ ChangedEquipment ⇒ Rebuild ⇒ the sink's RebuildAddressSpace throws. + var dep2 = SeedEquipmentDeployment(db, ("eq-1", "eq-1-renamed")); + actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId(), new DeploymentId(dep2))); AwaitAssert(() => { @@ -53,7 +63,7 @@ public sealed class OpcUaPublishActorApplyFailureTests : RuntimeActorTestBase var db = NewInMemoryDbFactory(); var sink = new NoopSink(); var applier = new AddressSpaceApplier(sink, NullLogger.Instance); - SeedEquipmentDeployment(db, "eq-1"); + SeedEquipmentDeployment(db, ("eq-1", "eq-1")); var actor = Sys.ActorOf(OpcUaPublishActor.PropsForTests(sink: sink, dbFactory: db, applier: applier)); actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId())); @@ -75,25 +85,26 @@ public sealed class OpcUaPublishActorApplyFailureTests : RuntimeActorTestBase if (last is not null) throw last; } - private static void SeedEquipmentDeployment(IDbContextFactory dbFactory, params string[] equipmentIds) + private static Guid SeedEquipmentDeployment(IDbContextFactory dbFactory, params (string Id, string Name)[] equipment) { var artifact = JsonSerializer.SerializeToUtf8Bytes(new { - Equipment = equipmentIds.Select(id => new + Equipment = equipment.Select(e => new { - EquipmentId = id, - MachineCode = id.ToUpperInvariant(), + EquipmentId = e.Id, + MachineCode = e.Id.ToUpperInvariant(), UnsLineId = "line-1", - Name = id, + Name = e.Name, }).ToArray(), DriverInstances = Array.Empty(), ScriptedAlarms = Array.Empty(), }); + var id = Guid.NewGuid(); using var ctx = dbFactory.CreateDbContext(); ctx.Deployments.Add(new Deployment { - DeploymentId = Guid.NewGuid(), + DeploymentId = id, RevisionHash = new string('a', 64), Status = DeploymentStatus.Sealed, CreatedBy = "test", @@ -101,6 +112,7 @@ public sealed class OpcUaPublishActorApplyFailureTests : RuntimeActorTestBase ArtifactBlob = artifact, }); ctx.SaveChanges(); + return id; } /// A sink whose RebuildAddressSpace throws (drives the applier's SafeRebuild catch → RebuildFailed). 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 ed596971..cc3b2fa1 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 @@ -18,7 +18,10 @@ namespace ZB.MOM.WW.OtOpcUa.Runtime.Tests.OpcUa; public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase { - /// Tests that RebuildAddressSpace with dbFactory loads artifact, composes, and applies. + /// R2-07 — RebuildAddressSpace with dbFactory loads the artifact, composes, and applies. Two + /// brand-new equipment are a PureAdd: the Materialise passes create the equipment folders WITHOUT a full + /// rebuild (existing subscriptions survive), and the added parents are announced via NodeAdded. (Was: + /// pre-R2-07 this asserted one full rebuild.) [Fact] public void RebuildAddressSpace_with_dbFactory_loads_artifact_composes_and_applies() { @@ -37,10 +40,13 @@ public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase AwaitAssert(() => { - // Add path: Equipment + Driver + Alarm — but only Equipment/Alarm topology triggers - // RebuildAddressSpace. With 2 new equipment we expect one Rebuild call. - sink.RebuildCalls.ShouldBe(1); + // PureAdd: the equipment folders are materialised, no full rebuild, and the added parent + // (the shared line-1) is announced exactly once. + sink.Calls.ShouldContain("EF:eq-1"); + sink.Calls.ShouldContain("EF:eq-2"); + sink.Calls.ShouldContain("NA:line-1"); }, duration: TimeSpan.FromSeconds(2)); + sink.RebuildCalls.ShouldBe(0); } /// Tests that rebuild with no artifact is idempotent no-op. @@ -63,7 +69,10 @@ public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase sink.RebuildCalls.ShouldBe(0); } - /// Tests that second rebuild with same artifact is empty plan no-op. + /// Tests that a second rebuild with the same artifact is an empty-plan no-op. The first deploy + /// (one new equipment) is a PureAdd — no full rebuild, but it DOES materialise the folder; the second + /// deploy of the identical composition diffs to an empty plan and the actor short-circuits, adding no + /// further sink calls. [Fact] public void Second_rebuild_with_same_artifact_is_empty_plan_no_op() { @@ -76,12 +85,15 @@ public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase sink: sink, dbFactory: db, applier: applier)); actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId())); - AwaitAssert(() => sink.RebuildCalls.ShouldBe(1), duration: TimeSpan.FromSeconds(2)); + AwaitAssert(() => sink.Calls.ShouldContain("EF:eq-1"), duration: TimeSpan.FromSeconds(2)); + sink.RebuildCalls.ShouldBe(0); // PureAdd — no full rebuild + var callsAfterFirst = sink.Calls.Count; actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId())); Thread.Sleep(200); - // Same composition ⇒ plan IsEmpty ⇒ applier not called again. - sink.RebuildCalls.ShouldBe(1); + // Same composition ⇒ plan IsEmpty ⇒ applier + materialise passes not run again. + sink.Calls.Count.ShouldBe(callsAfterFirst); + sink.RebuildCalls.ShouldBe(0); } /// Tests that rebuild without dbFactory falls back to raw sink rebuild. @@ -125,9 +137,10 @@ public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase siteActor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId())); - AwaitAssert(() => sinkA.RebuildCalls.ShouldBe(1), duration: TimeSpan.FromSeconds(2)); + // PureAdd (equipment + tag) ⇒ no full rebuild; the materialise passes still run the cluster slice. // t-sa (EquipmentId "eq-sa", FolderPath "F", Name "S1") → folder-scoped variable "eq-sa/F/S1". - sinkA.Calls.ShouldContain("EV:eq-sa/F/S1"); + AwaitAssert(() => sinkA.Calls.ShouldContain("EV:eq-sa/F/S1"), duration: TimeSpan.FromSeconds(2)); + sinkA.RebuildCalls.ShouldBe(0); // t-main (MAIN cluster) must NOT leak onto the SITE-A node. sinkA.Calls.ShouldNotContain("EV:eq-main/F/M1"); @@ -145,8 +158,8 @@ public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase mainActor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId())); - AwaitAssert(() => sinkM.RebuildCalls.ShouldBe(1), duration: TimeSpan.FromSeconds(2)); - sinkM.Calls.ShouldContain("EV:eq-main/F/M1"); + AwaitAssert(() => sinkM.Calls.ShouldContain("EV:eq-main/F/M1"), duration: TimeSpan.FromSeconds(2)); + sinkM.RebuildCalls.ShouldBe(0); sinkM.Calls.ShouldNotContain("EV:eq-sa/F/S1"); } @@ -212,6 +225,100 @@ public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase ctx.SaveChanges(); } + /// R2-07 T4b — a PureAdd deploy (here a single new equipment) does NOT rebuild, and the + /// NodeAdded announcement is raised AFTER the Materialise passes have created the node (so the node + /// exists when a re-browsing client arrives). Proven via the ordered Calls queue: the "NA:" + /// announcement index is strictly after the "EF:" materialise index. + [Fact] + public void Pure_add_announces_node_after_materialising_and_never_rebuilds() + { + var db = NewInMemoryDbFactory(); + var sink = new RecordingSink(); + var applier = new AddressSpaceApplier(sink, NullLogger.Instance); + SeedDeployment(db, equipmentIds: new[] { "eq-1" }, driverIds: Array.Empty()); + + var actor = Sys.ActorOf(OpcUaPublishActor.PropsForTests(sink: sink, dbFactory: db, applier: applier)); + actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId())); + + AwaitAssert(() => sink.Calls.ShouldContain("NA:line-1"), duration: TimeSpan.FromSeconds(2)); + sink.RebuildCalls.ShouldBe(0); + + var calls = sink.Calls.ToList(); + var materialiseIdx = calls.IndexOf("EF:eq-1"); + var announceIdx = calls.IndexOf("NA:line-1"); + materialiseIdx.ShouldBeGreaterThanOrEqualTo(0); + announceIdx.ShouldBeGreaterThan(materialiseIdx); // announced AFTER the node was materialised + } + + /// R2-07 T4b — when a deploy forces a full rebuild (here a renamed equipment ⇒ ChangedEquipment + /// ⇒ Rebuild) even though it ALSO adds an equipment, the post-materialise announce hook is SKIPPED (the + /// rebuild made subscriptions moot). Proven by capturing the NodeAdded count after the first pure-add + /// deploy and asserting the rebuild-forcing second deploy adds none. + [Fact] + public void Rebuild_kind_deploy_skips_the_node_added_announce() + { + var db = NewInMemoryDbFactory(); + var sink = new RecordingSink(); + var applier = new AddressSpaceApplier(sink, NullLogger.Instance); + + var dep1 = SeedNamedEquipmentDeployment(db, ("eq-1", "Pump-1")); + var actor = Sys.ActorOf(OpcUaPublishActor.PropsForTests(sink: sink, dbFactory: db, applier: applier)); + + actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId(), new DeploymentId(dep1))); + AwaitAssert(() => sink.Calls.Count(c => c.StartsWith("NA:")).ShouldBe(1), duration: TimeSpan.FromSeconds(2)); + sink.RebuildCalls.ShouldBe(0); + var naAfterFirst = sink.Calls.Count(c => c.StartsWith("NA:")); + + // Second deploy: eq-1 RENAMED (ChangedEquipment ⇒ Rebuild) AND a new eq-2 (an add). The rebuild + // fires, so the announce hook is skipped despite eq-2 being added. + var dep2 = SeedNamedEquipmentDeployment(db, ("eq-1", "Pump-1-RENAMED"), ("eq-2", "Pump-2")); + actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId(), new DeploymentId(dep2))); + + AwaitAssert(() => sink.RebuildCalls.ShouldBe(1), duration: TimeSpan.FromSeconds(2)); + // No new NodeAdded announcement was raised for the rebuild-kind deploy. + sink.Calls.Count(c => c.StartsWith("NA:")).ShouldBe(naAfterFirst); + } + + /// Seal a deployment carrying the given (equipmentId, name) rows under a shared line, returning + /// the new DeploymentId so a test can target THAT artifact. Renaming an equipment across two such + /// deployments produces a ChangedEquipment delta (Rebuild), while adding a row is a PureAdd. + private static Guid SeedNamedEquipmentDeployment( + IDbContextFactory dbFactory, + params (string Id, string Name)[] equipment) + { + var artifact = JsonSerializer.SerializeToUtf8Bytes(new + { + UnsAreas = new[] { new { UnsAreaId = "area-1", ClusterId = "c1", Name = "Area" } }, + UnsLines = new[] { new { UnsLineId = "line-1", UnsAreaId = "area-1", Name = "Line" } }, + Equipment = equipment.Select(e => new + { + EquipmentId = e.Id, + DriverInstanceId = (string?)null, + UnsLineId = "line-1", + Name = e.Name, + MachineCode = e.Id.ToUpperInvariant(), + }).ToArray(), + 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('d', 64), + Status = DeploymentStatus.Sealed, + CreatedBy = "test", + SealedAtUtc = DateTime.UtcNow, + ArtifactBlob = artifact, + }); + ctx.SaveChanges(); + return id; + } + private static void SeedDeployment( IDbContextFactory dbFactory, string[] equipmentIds, @@ -265,9 +372,11 @@ public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase 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). + // First deploy: the area folder is materialised with the OLD name. R2-07 — this is now a PureAdd + // (area + line + equipment all added), so NO full rebuild; the folder is materialised directly. actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId(), new DeploymentId(dep1))); - AwaitAssert(() => sink.RebuildCalls.ShouldBe(1), duration: TimeSpan.FromSeconds(2)); + AwaitAssert(() => sink.Calls.ShouldContain("EF:area-1"), duration: TimeSpan.FromSeconds(2)); + sink.RebuildCalls.ShouldBe(0); // 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). @@ -278,7 +387,7 @@ public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase { 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 + sink.RebuildCalls.ShouldBe(0); // the rename did NOT force a full rebuild } /// Seal a deployment whose area carries , with a line + one @@ -378,5 +487,23 @@ public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase FolderRenameQueue.Enqueue((folderNodeId, displayName)); return true; } + /// Records a surgical variable-node removal (always succeeds in this recording sink). + public bool RemoveVariableNode(string variableNodeId) + { + Calls.Enqueue($"RV:{variableNodeId}"); + return true; + } + /// Records a surgical alarm-condition-node removal (always succeeds in this recording sink). + public bool RemoveAlarmConditionNode(string alarmNodeId) + { + Calls.Enqueue($"RA:{alarmNodeId}"); + return true; + } + /// Records a surgical equipment-subtree removal (always succeeds in this recording sink). + public bool RemoveEquipmentSubtree(string equipmentNodeId) + { + Calls.Enqueue($"RE:{equipmentNodeId}"); + return true; + } } }