diff --git a/CLAUDE.md b/CLAUDE.md index 75b1380f..8a4eb123 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -130,6 +130,53 @@ the single-namespace `EquipmentTags` materialization path are gone. See `docs/plans/2026-07-15-raw-uns-two-subtree-v3-design.md` + `docs/plans/2026-07-15-v3-batch4-address-space-plan.md`, and `docs/Uns.md`. +**How a deploy reaches that address space (surgical-deploys, 2026-08-15).** The old +blanket rule — ANY non-empty `ChangedRawTags` / `ChangedUnsReferenceVariables` routes to +`Rebuild` — is gone, because it made a routine edit a fleet-wide outage: +`RawTagPlan.ReferencingEquipmentPaths` is part of the record's equality, so adding or +dropping **one** `UnsTagReference` flipped its backing raw tag to Changed and tore down +every node on the server, killing every client subscription. `AddressSpaceChangeClassifier` +is still default-closed; what changed is which deltas it whitelists. + +- **Still a full rebuild:** an **alarm-bearing** raw-tag delta (`Alarm != null` on either + side — the WP4 guard: only a rebuild clears and re-wires native-alarm notifiers), an + **identity** change on a raw tag (`TagId`/`DriverInstanceId`/`Name`/`ParentNodeId`/`Realm`), + a raw **container** (Folder/Driver/Device/TagGroup) rename or removal, `ChangedEquipment` / + `ChangedAlarms`, a non-surgical equipment-tag delta, and anything else the + `with`-equality whitelist does not name (including any FUTURE field). +- **Now surgical:** a **non-alarm** raw-tag delta over the in-place-updatable attributes + (`Writable` / `IsHistorized` / `HistorianTagname` / `DataType` / `IsArray` / `ArrayLength`) + and/or `ReferencingEquipmentPaths` — applied via `ISurgicalAddressSpaceSink.UpdateTagAttributes` + in the **Raw** realm — and **every** `UnsReferenceDelta` (add / remove / re-point / + backing-edit). +- ⚠️ **UNS mirrors repair through the RECORD, not the classifier.** A raw-side historian or + shape edit would otherwise leave every referencing UNS mirror on the old tagname/shape + (`EnsureVariable` early-returns on an existing node). `UnsReferenceVariable` therefore + carries the backing tag's `Backing{IsHistorized,HistorianTagname,IsArray,ArrayLength}`, so the + same edit makes the reference record unequal and the planner emits a **coincident** + `UnsReferenceDelta`. Adding a newly-inherited field needs no classifier change — **omitting + one silently reopens the staleness.** Repair is **in place** (`UpdateTagAttributes`, Uns + realm) when the mirror's identity is unchanged, so its MonitoredItems survive; only an + identity change (re-point, effective-name edit) is remove+recreate — and only for that + one node. +- **A non-rebuild apply materialises the DELTA, not the composition.** `MaterialiseHierarchy` + still runs full (parents must exist); the other passes take the plan's added/changed subsets. + Adding one tag to a 50k-node server was ~50k sink calls on the pinned publish thread. +- ⚠️ **A degraded non-rebuild apply does NOT advance `_lastApplied`.** The full passes used to + re-ensure every node on every apply, accidentally self-healing any swallowed sink fault; the + delta path removed that. Holding the composition back is the replacement — the next deploy + re-diffs the same composition and re-ensures what failed, at the cost of re-emitting that + apply's removes (a re-remove returns false and ratchets to a rebuild, which is the correct, + bounded resync). A **rebuild** still advances even with failures. +- **Any surgical `false`/throw still ratchets to `SafeRebuild()`** — that one-way fallback is + what makes every whitelist above safe. +- ⚠️ **Known gap, no longer neutral:** `AnnounceAddedNodes` is Uns-only, so surgically-added + **raw** containers/tags are materialised but never announced — previously invisible (the + rebuild forced a re-browse anyway), now genuinely undiscoverable until a client re-browses. + Added UNS reference variables ARE announced. + +See `docs/plans/2026-08-15-surgical-deploys.md`, `docs/Uns.md` and `docs/Raw.md`. + ### Key Concept: Tag Name and FullName Galaxy objects have a **tag_name** — a globally unique system name used for @@ -467,6 +514,16 @@ fail-closed). Things worth knowing before touching it: hash as the lowercase-hex SHA-256 of the exact blob, so the node verifies a fetch against the `RevisionHash` the dispatch already carries — **no hash rides in the proto, and `DispatchDeployment` stays payload-free.** A future change to how the revision hash is derived silently breaks every fetch. + **The blob is gzip-compressed since 2026-08-15, and the hash covers the COMPRESSED bytes** — + `ConfigComposer.Pack` gzips (`CompressionLevel.Fastest`) immediately after `SerializeToUtf8Bytes` and + hashes the result, so the invariant holds unchanged and nothing between the composer and + `DeploymentArtifact.Open` (SQL `ArtifactBlob`, the LocalDb cache chunks, the gRPC fetch stream) ever + looks inside. `Open` sniffs the gzip magic `1F 8B` — valid JSON can never start with `0x1F` — so + pre-upgrade uncompressed rows keep parsing; inflation runs under a 512 MiB guard and every + decompression failure surfaces as `JsonException`, the same family the malformed-JSON paths already + handle. Two operational consequences in `docs/Configuration.md` §`ConfigSource`: the first deploy after + upgrade **re-hashes every revision** (a one-time full re-dispatch, not a no-op), and **nodes must be + upgraded BEFORE central starts emitting compressed blobs.** - **A null fetch is "no answer," never an empty config (#485).** All endpoints down / `NotFound` / SHA mismatch / zero-length ⇒ the fetcher returns null ⇒ the apply FAILS, the revision does not advance, and the node keeps serving last-known-good. It never tears down to an empty address space. At boot a diff --git a/docs/Configuration.md b/docs/Configuration.md index 86fa1aeb..b83a8c30 100644 --- a/docs/Configuration.md +++ b/docs/Configuration.md @@ -217,6 +217,39 @@ See the [ClusterClient→gRPC migration design](plans/2026-08-06-clusterclient-t **`RevisionHash == SHA-256(artifact bytes)` is load-bearing.** `ConfigComposer` computes the deployment's revision hash as the lowercase-hex SHA-256 of the exact artifact bytes, so the node verifies a fetch against the `RevisionHash` the dispatch already carries — no hash rides in the proto. If a future change makes the revision hash anything else, every fetch fails verification. +#### Artifact compression (2026-08-15) + +The artifact blob is **gzip-compressed, inside the hashed bytes**. `ConfigComposer.Pack` gzips the +serialised snapshot (`CompressionLevel.Fastest`, ~10–20x on this highly repetitive JSON) immediately +after `SerializeToUtf8Bytes` and **then** takes the SHA-256, so the compressed bytes *are* the artifact +everywhere — `Deployment.ArtifactBlob`, the LocalDb cache chunks, the gRPC fetch stream, and the +`RevisionHash` a `FetchAndCache` node verifies against. The invariant above is preserved exactly, which +is why the fetcher, the cache and the serve path are untouched: nothing between the composer and +`DeploymentArtifact.Open` looks inside the blob. + +Decompression lives in exactly one place, `DeploymentArtifact.Open`, which **sniffs the gzip magic +`1F 8B`** at offset 0 — valid JSON can never start with `0x1F`, so no framing of our own is needed and +**pre-upgrade uncompressed rows keep parsing unchanged**. Inflation runs under a 512 MiB guard so a +corrupt or hostile blob cannot OOM the node, and every decompression failure (corrupt, truncated, +over-limit) is reported as `JsonException` — the same exception family the malformed-JSON path already +owns, so no caller's decision changes. + +Two operational facts: + +- **The first deploy after the upgrade re-hashes every revision.** The same configuration now serialises + to different (compressed) bytes, so it produces a different `RevisionHash`. Expect a **one-time full + re-dispatch to every node**, not the usual no-op — this is normal, and it happens once. +- **Rollout ordering matters: upgrade the NODES first.** A node must be running artifact-sniffing code + *before* central starts emitting compressed blobs. A pre-upgrade node handed a gzip artifact cannot + parse it and **fails the apply**, which means it keeps serving last-known-good (the #485 contract) — + degraded but not destructive, and it recovers as soon as it is upgraded. Rolling back central is + likewise safe in the other direction, since sniffing code still reads uncompressed blobs. + +(gzip output is not guaranteed byte-stable across .NET releases. Harmless here: the hash is computed +from the bytes emitted at compose time and never re-derived from the JSON, so an implementation change +costs at most one spurious "changed" revision after a runtime upgrade. Snapshot determinism is +unaffected — compression wraps the final bytes and reorders nothing.) + On the docker-dev rig the central pair carries `ConfigServe` (port `4055`, dev key) and stays `Direct`; the four site nodes carry `ConfigSource` (endpoints + matching key) defaulting to `Direct`. Flip only the site nodes with `OTOPCUA_CONFIG_MODE=FetchAndCache` at `docker compose up` — central keeps SQL and models the target topology. **`FetchAndCache` is mandatory on a driver-only node — per-cluster mesh Phase 4.** `Program.cs` now diff --git a/docs/Raw.md b/docs/Raw.md index 03f7836b..144ba49f 100644 --- a/docs/Raw.md +++ b/docs/Raw.md @@ -70,6 +70,39 @@ serialize the endpoint keys, so `DeviceConfig` is the single source of truth. tag-groups" toggle mirrors the browse folder nesting. The browser dials a real ephemeral driver against the **merged** Driver+Device config. +## What a deploy tears down (surgical deploys, 2026-08-15) + +Editing a raw **tag's** attributes is no longer a fleet-wide event. A **non-alarm** +raw-tag delta whose only differences are the in-place-updatable attributes — +`Writable`, `IsHistorized`, `HistorianTagname`, `DataType`, `IsArray`, `ArrayLength` — +and/or its set of referencing equipment is applied **in place** on the existing Raw-realm +node (`ISurgicalAddressSpaceSink.UpdateTagAttributes`), so every client subscription on +that tag and on the rest of the server survives. The same is true of **every** UNS +tag-reference change: adding, removing or re-pointing a reference used to flip the +backing raw tag to Changed (the referencing set is part of its record equality) and +rebuild the whole address space for a one-variable edit. + +**Still a full rebuild**, deliberately: an **alarm-bearing** raw tag (a delta with +`Alarm != null` on either side — only a rebuild clears and re-wires the native-alarm +notifiers that fan the condition to its referencing equipment folders); an **identity** +change on a tag (`TagId`, `DriverInstanceId`, `Name`, `ParentNodeId`, realm); and any +change to a raw **container** — a Folder / Driver / Device / TagGroup **rename or +removal** — because the surgical sink has no folder-remove. The classifier is +default-closed: any field it does not explicitly whitelist, including fields added +later, still routes to a rebuild, and any surgical call returning `false` or throwing +ratchets back to one. + +**Mirror repair.** A raw-side historian or shape edit also reaches every UNS reference +mirroring the tag: the reference record carries the backing tag's historian/shape, so +the edit emits a coincident UNS-reference delta. Identity-preserving repairs update the +mirror **in place** (its MonitoredItems survive); a re-point or effective-name change +removes and recreates **that one mirror node**. See [`Uns.md`](Uns.md) and +[`plans/2026-08-15-surgical-deploys.md`](plans/2026-08-15-surgical-deploys.md). + +⚠️ A surgically added raw container/tag is materialised but **not** announced — +`AnnounceAddedNodes` is UNS-only, so model-aware clients see new raw nodes on their next +browse. This mattered less when every add forced a rebuild (and therefore a re-browse). + ## The `Calculation` driver Signal-level calculated tags are ordinary raw tags bound to a `Calculation` driver diff --git a/docs/Uns.md b/docs/Uns.md index 43689d43..7ccc5238 100644 --- a/docs/Uns.md +++ b/docs/Uns.md @@ -442,6 +442,40 @@ Edits here change the configuration only. As the page header notes, **changes apply on the next deployment** — run a **Deploy** (Deployments page) to push them into the running address space. +### What a deploy actually tears down (surgical deploys, 2026-08-15) + +Adding, removing or re-pointing a UNS **tag reference** no longer rebuilds the +address space. It used to: `RawTagPlan.ReferencingEquipmentPaths` participates in +the backing raw tag's record equality, so touching one reference flipped that tag to +Changed and the classifier's blanket `ChangedRawTags ⇒ Rebuild` rule tore down and +recreated **every** node on the server — killing every client's subscriptions for a +one-variable edit. Now **every** `UnsReferenceDelta` is applied surgically, and so is +a **non-alarm** raw-tag delta over the in-place-updatable attributes +(`Writable` / `IsHistorized` / `HistorianTagname` / `DataType` / `IsArray` / +`ArrayLength`). A **full rebuild** is still forced by an **alarm-bearing** raw-tag +delta (only a rebuild re-wires native-alarm notifiers), an **identity** change on a +raw tag (`TagId` / `DriverInstanceId` / `Name` / `ParentNodeId` / `Realm`), a raw +**container** rename or removal, and any changed equipment / scripted alarm. The +classifier stays default-closed — anything its whitelist does not name still rebuilds, +and any surgical failure ratchets back to a rebuild. + +**Mirror repair.** A UNS reference variable mirrors its backing raw tag, so a raw-side +historian or shape edit must reach it too — `UnsReferenceVariable` carries the backing +tag's `IsHistorized` / `HistorianTagname` / `IsArray` / `ArrayLength`, which makes the +reference record unequal on such an edit and emits a **coincident** `UnsReferenceDelta` +alongside the raw one. **Identity-preserving** repairs (the mirror keeps the same +NodeId, equipment and backing RawPath) are applied **in place**, so the mirror's +subscriptions survive an operator historizing or reshaping the tag behind it. Only an +**identity change** — a re-point to a different raw tag, or an effective-name/display-name +edit that moves the NodeId — is a remove + recreate, and it affects **that one node**, +not the tree. + +⚠️ Two consequences worth knowing. A surgically **added** UNS reference variable *is* +announced (Part 3 `GeneralModelChangeEvent`), but an added **raw** container/tag is +not — clients pick those up on their next browse. And a **degraded** non-rebuild apply +(some node failed to materialise) deliberately does not record its composition as +applied, so the next deploy re-diffs and repairs it. + ## See also - [Configuration.md](Configuration.md) — the underlying config entities. diff --git a/docs/plans/2026-08-15-surgical-deploys.md b/docs/plans/2026-08-15-surgical-deploys.md index 24b6ea63..6e3d6899 100644 --- a/docs/plans/2026-08-15-surgical-deploys.md +++ b/docs/plans/2026-08-15-surgical-deploys.md @@ -316,3 +316,65 @@ Steps: full `dotnet build ZB.MOM.WW.OtOpcUa.slnx`; run every non-DB test project - **No `AnnounceAddedNodes` extension to raw/UNS adds** — pre-existing gap, noted in comments. - **No cross-actor shared `ParsedDeploymentArtifact`** — one parse per actor is the target; sharing a disposable across mailboxes isn't worth the ownership complexity. - **No artifact schema changes** — the JSON inside the (now compressed) blob is byte-identical. + +--- + +## Follow-ups (recorded, not implemented) + +Found during implementation and review of this plan. None is a regression introduced by the branch +except where stated; each is recorded here rather than folded in, so the branch stays reviewable. + +**(a) Raw-realm `RaiseNodesAddedModelChange` — surgically-added raw nodes are never announced.** +`AnnounceAddedNodes` / `ComputeAddAnnouncements` only walk the equipment (Uns) add sets, and the +announce hardcodes the Uns realm. The UNS half is now closed — an added `UnsReferenceVariable` is +announced against its equipment folder. The **raw** half is not: an added raw container or raw tag is +materialised and then silent. This was neutral before the branch (a plan carrying raw adds went through +a full rebuild, so every client had to re-browse anyway); with surgical adds keeping subscriptions +alive, an unannounced raw add is genuinely invisible until a client happens to browse. The fix is a +realm-aware `RaiseNodesAddedModelChange` call plus raw-side add sets in `ComputeAddAnnouncements`. This +is a real follow-up, not an accepted limitation. + +**(b) `RemoveEquipmentSubtree` shares the dangling-inverse-edge exposure that `RemoveVariableNode` now +fixes.** I3 on this branch made `RemoveVariableNode` sever the inverse half of every outgoing +`Organizes` edge (the variable-node counterpart of `UnwireAlarmNotifiers`) — before, rebuilds swept +those up, and surgical applies made rebuilds rare enough that they accumulated on the backing raw node +for the life of the process. `RemoveEquipmentSubtree` removes a whole equipment folder and has the same +shape of exposure for the edges its descendants own; it was not touched here because it is only reached +by a `PureRemove`/`AddRemoveMix` plan and needs its own test scaffolding. Same class of leak, same fix. + +**(c) End-to-end test: a degraded remove-bearing apply followed by a second deploy.** The C1 resync +semantic — a degraded non-rebuild apply does not advance `_lastApplied`, so the next deploy re-diffs the +same composition, re-emits that apply's removes, and the re-remove's `false` ratchets into a full +rebuild — is reasoned in the code comment and each half is independently pinned (the non-advance, and +the ratchet-on-false). What is **not** covered is the composed path: drive a degraded apply that carried +removes, then a second deploy, and observe the ratcheted rebuild. Worth building because the semantic is +deliberately self-correcting-by-escalation, and an escalation nobody has watched fire is an assumption. + +**(d) Delta materialise of scripted alarms uses thin-plan ids against rich rows.** The alarm subset +handed to the materialise pass is identified by the plan's ids, while the pass reads the full +composition's rich alarm rows. An alarm whose **predicate script** arrives in a later revision than the +alarm row itself is therefore invisible to a delta apply: the alarm id is not in any diff set, so the +subset never names it, and the pass never re-reads it. This is **pre-existing planner blindness** (the +planner has always diffed on the thin shape), but the delta path widens it — the full passes used to +re-read every alarm on every apply and would eventually pick the script up. Until fixed, such an alarm +converges only on the next rebuild. + +**(e) Deploy dispatch carries no monotonic sequence.** Under the gRPC command transport, dispatches are +fire-and-forget with no retry and no ordering guarantee across nodes, so two dispatches issued close +together can be overtaken and "the final revision" is whichever arrived last, not whichever was sealed +last. This is pre-existing and independent of this branch — but the stash-instead-of-forward-spin change +(Task 7 Part B) makes it the **only** remaining nondeterminism in the deferral path, so it is now worth +naming. A monotonic dispatch sequence that a node refuses to go backwards on would close it. + +**(f) LocalDb store ordering under concurrent `StoreAsync`.** With the cache write moved off the mailbox +(`PipeTo`), two applies in quick succession can have their stores in flight simultaneously; the artifact +chunks and the pointer are written separately, so the pointer can briefly name the older artifact. It +self-heals on the next deploy (the newer artifact is stored and the pointer is rewritten), and boot-from- +cache is a fallback rather than a system of record, so the exposure is a stale — never a corrupt — cache +in a narrow window. A store-level sequence guard, or serialising stores per cluster, would remove it. + +**(g) `RestoreApplied` could thread its in-hand blob.** Task 1 removed the redundant central-SQL reads +from the Direct apply path, but the boot/restore path still reads the artifact and then hands the +publish actor a bytes-less `RebuildAddressSpace`, which re-reads it. That is **one** extra SQL read per +node restart — not per apply — which is why it was left out of scope. Threading `appliedBlob` through +`RestoreApplied` the same way `ApplyAndAck` now does would finish the job. diff --git a/docs/plans/2026-08-15-surgical-deploys.md.tasks.json b/docs/plans/2026-08-15-surgical-deploys.md.tasks.json index 347c712e..74f55a05 100644 --- a/docs/plans/2026-08-15-surgical-deploys.md.tasks.json +++ b/docs/plans/2026-08-15-surgical-deploys.md.tasks.json @@ -1,15 +1,15 @@ { "planPath": "docs/plans/2026-08-15-surgical-deploys.md", "tasks": [ - {"id": 1, "subject": "Task 1: Direct mode threads the already-read blob", "status": "pending"}, - {"id": 2, "subject": "Task 2: DeploymentArtifact.Open() — parse once", "status": "pending"}, - {"id": 3, "subject": "Task 3: Actors consume Open()", "status": "pending", "blockedBy": [1, 2]}, - {"id": 4, "subject": "Task 4: Classifier — surgical eligibility for raw-tag and UNS-reference deltas", "status": "pending"}, - {"id": 5, "subject": "Task 5: Applier — apply the new surgical deltas", "status": "pending", "blockedBy": [4]}, - {"id": 6, "subject": "Task 6: Delta materialisation", "status": "pending", "blockedBy": [5]}, - {"id": 7, "subject": "Task 7: Blocking I/O off the host mailbox", "status": "pending", "blockedBy": [3]}, - {"id": 8, "subject": "Task 8: Compressed artifact blob", "status": "pending", "blockedBy": [2]}, - {"id": 9, "subject": "Task 9: Full verification + docs", "status": "pending", "blockedBy": [3, 6, 7, 8]} + {"id": 1, "subject": "Task 1: Direct mode threads the already-read blob", "status": "completed"}, + {"id": 2, "subject": "Task 2: DeploymentArtifact.Open() — parse once", "status": "completed"}, + {"id": 3, "subject": "Task 3: Actors consume Open()", "status": "completed", "blockedBy": [1, 2]}, + {"id": 4, "subject": "Task 4: Classifier — surgical eligibility for raw-tag and UNS-reference deltas", "status": "completed"}, + {"id": 5, "subject": "Task 5: Applier — apply the new surgical deltas", "status": "completed", "blockedBy": [4]}, + {"id": 6, "subject": "Task 6: Delta materialisation", "status": "completed", "blockedBy": [5]}, + {"id": 7, "subject": "Task 7: Blocking I/O off the host mailbox", "status": "completed", "blockedBy": [3]}, + {"id": 8, "subject": "Task 8: Compressed artifact blob", "status": "completed", "blockedBy": [2]}, + {"id": 9, "subject": "Task 9: Full verification + docs", "status": "completed", "blockedBy": [3, 6, 7, 8]} ], - "lastUpdated": "2026-08-15T00:00:00Z" + "lastUpdated": "2026-08-15T22:30:00Z" } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/AdminOperations/ConfigComposer.cs b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/AdminOperations/ConfigComposer.cs index 4c1f22fc..396e13bc 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/AdminOperations/ConfigComposer.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/AdminOperations/ConfigComposer.cs @@ -1,3 +1,4 @@ +using System.IO.Compression; using System.Security.Cryptography; using System.Text.Json; using Microsoft.EntityFrameworkCore; @@ -8,9 +9,10 @@ namespace ZB.MOM.WW.OtOpcUa.ControlPlane.AdminOperations; /// /// Pure snapshot composer: reads the current live-edit state from -/// and serialises it into a deterministic byte[] artifact + SHA-256 hex revision hash. Determinism -/// comes from sorting every collection by its natural key before serialising, so two snapshots over -/// the same DB state always produce the same hash regardless of EF row ordering. +/// and serialises it into a deterministic, gzip-compressed byte[] artifact + SHA-256 hex revision hash. +/// Determinism comes from sorting every collection by its natural key before serialising, so two snapshots +/// over the same DB state always produce the same hash regardless of EF row ordering; compression wraps the +/// final bytes and reorders nothing. The hash is taken over the COMPRESSED bytes — see . /// public static class ConfigComposer { @@ -61,9 +63,49 @@ public static class ConfigComposer ScriptedAlarms = await db.ScriptedAlarms.AsNoTracking().OrderBy(x => x.ScriptedAlarmId).ToListAsync(ct), }; - var blob = JsonSerializer.SerializeToUtf8Bytes(snapshot, JsonOptions); - var hash = Convert.ToHexStringLower(SHA256.HashData(blob)); - return new ConfigArtifact(blob, hash); + return Pack(JsonSerializer.SerializeToUtf8Bytes(snapshot, JsonOptions)); + } + + /// + /// Compress the serialised snapshot and hash the COMPRESSED bytes — the emitted artifact and its + /// revision hash, in one step, so the two can never be derived from different byte sequences. + /// + /// + /// + /// Compression happens before hashing on purpose. The invariant the whole deploy path + /// rests on is RevisionHash == SHA-256(stored/wire bytes): a FetchAndCache node + /// verifies the bytes it fetched against the hash its dispatch already carried, and the LocalDb + /// cache stores those same bytes. Hashing the uncompressed JSON instead would break every one of + /// those checks. Nothing between here and DeploymentArtifact.Open looks inside the blob, + /// which is what makes the change invisible to the cache and the gRPC serve path. + /// + /// + /// gzip with the standard magic 1F 8B and no framing of our own: valid JSON can never + /// start with 0x1F, so the read side sniffs the first two bytes and pre-upgrade + /// uncompressed rows keep parsing unchanged. because the + /// artifact is highly repetitive JSON — the marginal ratio from a slower level is small next to + /// the ~10–20x the first level already delivers. + /// + /// + /// gzip output is not guaranteed byte-stable across .NET releases. Harmless here: the hash is + /// computed from the bytes emitted at compose time and never re-derived from the JSON, so an + /// implementation change costs at most one spurious "changed" revision after a runtime upgrade. + /// Determinism of the snapshot itself is unaffected — compression wraps the final bytes and + /// reorders nothing. + /// + /// + /// The deterministic UTF-8 JSON snapshot. + /// The compressed artifact blob and the SHA-256 hex digest OF THAT BLOB. + internal static ConfigArtifact Pack(byte[] json) + { + using var output = new MemoryStream(json.Length / 8); + using (var gzip = new GZipStream(output, CompressionLevel.Fastest, leaveOpen: true)) + { + gzip.Write(json, 0, json.Length); + } + + var blob = output.ToArray(); + return new ConfigArtifact(blob, Convert.ToHexStringLower(SHA256.HashData(blob))); } /// Returns the SHA-256 hex digest of the supplied artifact bytes (lowercase, no prefix). diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceApplier.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceApplier.cs index 2438c3c6..eee64688 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceApplier.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceApplier.cs @@ -17,8 +17,10 @@ namespace ZB.MOM.WW.OtOpcUa.OpcUaServer; /// 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 +/// AttributeOnly — no rebuild. Surgical-eligible changed equipment tags, changed RAW +/// tags (in the Raw realm) and UNS folder renames are applied IN PLACE via +/// (F10b); a changed UNS reference variable is a +/// remove-previous here + recreate-current in the materialise pass. 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 @@ -160,6 +162,12 @@ public sealed class AddressSpaceApplier // MaterialiseHierarchy re-creates every folder with the new display names, so the renames are // covered for free and need no separate surgical pass. var renamedFolders = plan.RenamedFolders; + // v3 surgical raw-tag + UNS-reference deltas (2026-08-15 surgical-deploys, Task 5). When kind is not + // Rebuild the classifier has ALREADY guaranteed every ChangedRawTags entry is surgical-eligible; the + // filter is defense-in-depth so a future classifier change can only ever under-apply here (the + // non-eligible remainder is left to the rebuild the classifier would have demanded). + var surgicalRawTagDeltas = plan.ChangedRawTags.Where(AddressSpaceChangeClassifier.RawTagDeltaIsSurgicalEligible).ToList(); + var unsRefDeltas = plan.ChangedUnsReferenceVariables; var rebuilt = false; var rebuildFailed = false; @@ -168,7 +176,8 @@ public sealed class AddressSpaceApplier rebuildFailed = !SafeRebuild(); rebuilt = true; } - else if (surgicalTagDeltas.Count > 0 || renamedFolders.Count > 0 || hasRemovePass) + else if (surgicalTagDeltas.Count > 0 || renamedFolders.Count > 0 || hasRemovePass || + surgicalRawTagDeltas.Count > 0 || unsRefDeltas.Count > 0) { if (_sink is ISurgicalAddressSpaceSink surgical) { @@ -204,6 +213,93 @@ public sealed class AddressSpaceApplier } if (!ok) { allApplied = false; break; } } + // v3 RAW tags — the same in-place attribute update, in the RAW realm. Adding or dropping ONE + // UnsTagReference flips its backing raw tag to Changed purely through ReferencingEquipmentPaths, + // and before this path existed that tore down every node on the server for an edit that creates + // a single UNS variable. + foreach (var d in allApplied ? surgicalRawTagDeltas : Enumerable.Empty()) + { + // ReferencingEquipmentPaths is node-INERT for a plain value tag (it only drives native-alarm + // notifier wiring, and an alarm-bearing delta never reaches here — the classifier routes it + // to Rebuild). So a delta whose ONLY difference is the referencing set needs NOTHING said to + // the sink: compare the two sides with the referencing set copied across, and skip on equal. + if ((d.Previous with { ReferencingEquipmentPaths = d.Current.ReferencingEquipmentPaths }).Equals(d.Current)) + { + continue; + } + + // Compute writable + historian EXACTLY as MaterialiseRawSubtree does so the in-place update + // matches what a rebuild would have produced: arrays are forced read-only (the driver write + // path can't handle them) and a blank historian override falls back to the RawPath (== NodeId). + var writable = d.Current.Writable && !d.Current.IsArray; + var historian = d.Current.IsHistorized + ? (string.IsNullOrWhiteSpace(d.Current.HistorianTagname) ? d.Current.NodeId : d.Current.HistorianTagname) + : null; + bool ok; + try { ok = surgical.UpdateTagAttributes(d.Current.NodeId, writable, historian, d.Current.DataType, d.Current.IsArray, d.Current.ArrayLength, AddressSpaceRealm.Raw); } + catch (Exception ex) + { + _logger.LogError(ex, "AddressSpaceApplier: surgical UpdateTagAttributes threw for raw {Node}", d.Current.NodeId); + ok = false; + } + if (!ok) { allApplied = false; break; } + } + // v3 UNS reference variables — TWO shapes, split on whether the delta moves the node's + // IDENTITY (2026-08-15 review I2). + // + // (a) IDENTITY UNCHANGED (same NodeId, same EquipmentId, same BackingRawPath): only the + // inherited Backing* / DataType / Writable attributes moved — the 875ec9fe repair shape, + // produced whenever a raw tag's historian or array shape is edited under a reference that + // was not itself touched. Nothing about the node's identity or its Organizes edge changes, + // so an in-place UpdateTagAttributes in the Uns realm says everything that needs saying + // AND the mirror's client MonitoredItems survive. Remove+recreate here would kill the + // subscriptions of every client watching a UNS signal because someone historized the raw + // tag behind it. + // + // (b) IDENTITY CHANGED — a re-point (BackingRawPath moved: the node's Organizes edge must move + // with it, and the surgical sink surface has no RemoveReference) or an effective-name edit + // (the effective name is part of the UNS NodeId, and an OPC UA NodeId is immutable). Remove + // the PREVIOUS node here (terminal Bad first, so an in-flight MonitoredItem observes the + // removal) and let the CURRENT node be created by the materialise pass after Apply returns. + // The cost is that ONE node's monitored items, against today's cost of a full rebuild + // killing every subscription server-wide. + // + // BOTH shapes leave the delta's Current in the publish actor's materialise subset, and that is + // deliberate: for (b) the subset IS the recreate (dropping it would leave the reference removed + // and never brought back), and for (a) it is a cheap idempotent no-op — EnsureVariable + // early-returns on the already-present node and AddReference is idempotent on the unchanged + // edge. Making the subset shape-aware would buy nothing and add a way for the two sides to + // disagree about which references were repaired. + foreach (var d in allApplied ? unsRefDeltas : Enumerable.Empty()) + { + if (IsIdentityPreservingUnsRefDelta(d)) + { + // Resolve writable + historian EXACTLY as MaterialiseUnsReferences does when it creates + // the node: arrays are forced read-only, and a blank override falls back to the backing + // RawPath (which IS the backing raw tag's NodeId). + var writable = d.Current.Writable && !d.Current.BackingIsArray; + var historian = d.Current.BackingIsHistorized + ? (string.IsNullOrWhiteSpace(d.Current.BackingHistorianTagname) ? d.Current.BackingRawPath : d.Current.BackingHistorianTagname) + : null; + bool updated; + try + { + updated = surgical.UpdateTagAttributes( + d.Current.NodeId, writable, historian, d.Current.DataType, + d.Current.BackingIsArray, d.Current.BackingArrayLength, AddressSpaceRealm.Uns); + } + catch (Exception ex) + { + _logger.LogError(ex, "AddressSpaceApplier: surgical UpdateTagAttributes threw for UNS reference {Node}", d.Current.NodeId); + updated = false; + } + if (!updated) { allApplied = false; break; } + continue; + } + + if (!SafeWriteValue(d.Previous.NodeId, null, OpcUaStatus.Bad, ts, AddressSpaceRealm.Uns)) failedNodes++; + if (!SafeRemoveVariable(surgical, d.Previous.NodeId, AddressSpaceRealm.Uns)) { 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 @@ -224,8 +320,10 @@ public sealed class AddressSpaceApplier } _logger.LogInformation( - "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); + "AddressSpaceApplier: applied plan (kind={Kind}, added={Added}, removed={Removed}, changed={Changed}, surgicalTags={Surgical}, surgicalRawTags={SurgicalRaw}, unsRefDeltas={UnsRefs}, renamedFolders={Renamed}, rebuild={Rebuild})", + kind, addedCount, removedCount, changedCount, rebuilt ? 0 : surgicalTagDeltas.Count, + rebuilt ? 0 : surgicalRawTagDeltas.Count, rebuilt ? 0 : unsRefDeltas.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 @@ -307,8 +405,15 @@ public sealed class AddressSpaceApplier } // v3 Batch 4 — removed UNS reference variables (Uns realm, plain value nodes): terminal Bad then - // in-place remove. A backing-raw rename is a re-point (Changed, → rebuild), NOT a removal, so a UNS - // ref only reaches here on a genuine dereference (the equipment dropped the reference). + // in-place remove. + // + // WHAT REACHES THIS LOOP (the "→ rebuild" this comment used to claim is long dead — re-points are + // surgical now): only genuine DEREFERENCES, i.e. the equipment dropped the reference row, so the node + // is gone for good. A CHANGED reference never lands in RemovedUnsReferenceVariables at all — the + // planner reports it as a UnsReferenceDelta, which Apply routes above either to an in-place + // UpdateTagAttributes (identity unchanged) or to its own remove-the-previous + recreate-the-current + // pair (identity changed). That second shape does remove a node, but it removes it from Apply's delta + // loop, not from here. foreach (var v in plan.RemovedUnsReferenceVariables) { SafeWriteValue(v.NodeId, null, OpcUaStatus.Bad, ts, AddressSpaceRealm.Uns); @@ -340,6 +445,22 @@ public sealed class AddressSpaceApplier return true; } + /// + /// 2026-08-15 review I2 — true when a can be applied + /// as an IN-PLACE attribute update rather than a remove+recreate: the node's identity is untouched. + /// Three fields make up that identity — the NodeId (an OPC UA NodeId is immutable, so a moved + /// effective name is a different node), the EquipmentId (the node's PARENT, which + /// UpdateTagAttributes cannot re-hang), and the BackingRawPath (the target of the node's + /// Organizes edge, which the surgical sink surface has no way to move — there is no + /// RemoveReference). Everything else on the record is a plain attribute the in-place update carries. + /// + /// The UNS reference delta to classify. + /// true when the delta is attribute-only and may be applied in place. + private static bool IsIdentityPreservingUnsRefDelta(AddressSpacePlan.UnsReferenceDelta d) => + string.Equals(d.Previous.NodeId, d.Current.NodeId, StringComparison.Ordinal) && + string.Equals(d.Previous.EquipmentId, d.Current.EquipmentId, StringComparison.Ordinal) && + string.Equals(d.Previous.BackingRawPath, d.Current.BackingRawPath, StringComparison.Ordinal); + /// 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, AddressSpaceRealm realm) @@ -399,6 +520,13 @@ public sealed class AddressSpaceApplier 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); + // v3 Batch 4 / review I5 — an added UNS reference variable is materialised directly under its + // equipment folder (MaterialiseUnsReferences passes EquipmentId as the parent), and the equipment + // folder is a UNS-realm node — the realm AnnounceAddedNodes hardcodes. So it announces correctly with + // no realm plumbing. (Added RAW containers/tags are still NOT announced: their parents are Raw-realm + // nodes, which needs a realm-aware RaiseNodesAddedModelChange call — see the gap note in + // OpcUaPublishActor.HandleRebuild.) + foreach (var v in plan.AddedUnsReferenceVariables) ids.Add(v.EquipmentId); foreach (var eq in plan.AddedEquipment) ids.Add(string.IsNullOrWhiteSpace(eq.UnsLineId) ? eq.EquipmentId : eq.UnsLineId); @@ -636,6 +764,14 @@ public sealed class AddressSpaceApplier /// clients browsing the server see proper folder structure instead of flat tag ids. /// Idempotent: each EnsureFolder call returns the existing folder if already /// present, so re-applies are cheap. + /// This pass has NO subset overload, deliberately — it always runs over the FULL + /// composition, unlike the five node-materialising passes below (which take an optional + /// only* delta subset so a non-rebuild apply touches the plan's added nodes instead of the + /// whole address space). Two reasons: it is O(areas + lines + equipment), which is a rounding error + /// next to the tag/reference counts the subsetting exists to bound; and every added node materialised + /// by the other passes needs its ANCESTOR folders to already exist, so narrowing this one to the + /// plan's delta would mean re-deriving the ancestor closure of every add — strictly more work than + /// re-ensuring the folders. /// /// The composition result containing the hierarchy to materialise. /// The count of swallowed sink failures in this pass (0 on a clean apply) — the publish actor @@ -676,18 +812,33 @@ public sealed class AddressSpaceApplier /// (override else the RawPath). Array tags are forced read-only (the driver write path can't handle /// arrays). Every sink call passes EXPLICITLY — the driver binds /// values to these raw NodeIds, and every referencing UNS node fans out from them. Idempotent. + /// Subset overload (2026-08-15 surgical-deploys). / + /// narrow WHAT is materialised to a plan's delta while + /// stays the FULL composition for context lookups — specifically the + /// native-alarm notifier map (), which must see every + /// equipment on the node, not just the ones this deploy touched. A null subset means "the whole + /// composition" (the rebuild path), so every pre-existing call site is unchanged. Ordering holds in + /// both modes: the composer sorts RawContainers by NodeId and the planner sorts its added set by + /// the same key, so a parent's RawPath still precedes each child's. /// - /// The composition carrying the Raw subtree. + /// The FULL composition — the context for lookups even when a subset is passed. + /// Materialise only these containers; null ⇒ the whole composition's. + /// Materialise only these tags; null ⇒ the whole composition's. /// The count of swallowed sink failures in this pass. - public int MaterialiseRawSubtree(AddressSpaceComposition composition) + public int MaterialiseRawSubtree( + AddressSpaceComposition composition, + IReadOnlyList? onlyContainers = null, + IReadOnlyList? onlyTags = null) { ArgumentNullException.ThrowIfNull(composition); - if (composition.RawContainers.Count == 0 && composition.RawTags.Count == 0) return 0; + var containers = onlyContainers ?? composition.RawContainers; + var tags = onlyTags ?? composition.RawTags; + if (containers.Count == 0 && tags.Count == 0) return 0; var failed = 0; // Containers first — the composer sorts them by NodeId (ordinal) so a parent's RawPath precedes each // child's; EnsureFolder is idempotent, so a re-apply is cheap. - foreach (var c in composition.RawContainers) + foreach (var c in containers) { if (!SafeEnsureFolder(c.NodeId, c.ParentNodeId, c.DisplayName, AddressSpaceRealm.Raw)) failed++; } @@ -698,7 +849,7 @@ public sealed class AddressSpaceApplier // when at least one raw tag carries an alarm (the common no-alarm deploy pays nothing). IReadOnlyDictionary? equipIdByFolderPath = null; - foreach (var t in composition.RawTags) + foreach (var t in tags) { if (t.Alarm is not null) { @@ -754,8 +905,8 @@ public sealed class AddressSpaceApplier } _logger.LogInformation( - "AddressSpaceApplier: raw subtree materialised (containers={Containers}, tags={Tags}, failed={Failed})", - composition.RawContainers.Count, composition.RawTags.Count, failed); + "AddressSpaceApplier: raw subtree materialised (containers={Containers}, tags={Tags}, subset={Subset}, failed={Failed})", + containers.Count, tags.Count, onlyContainers is not null || onlyTags is not null, failed); return failed; } @@ -765,35 +916,45 @@ public sealed class AddressSpaceApplier /// parented under its equipment folder (the logical node /// already created), THEN an Organizes reference /// UNS→Raw to its backing raw node. The UNS node inherits DataType / writable / array shape / historian - /// tagname from its backing RAW tag (looked up by ), so a - /// historized raw tag's UNS reference registers the SAME tagname (both NodeIds → one tagname → HistoryRead - /// works against either). The UNS node binds NO driver — the driver publish path fans values to it (WP3 - /// runtime binding). Every sink call passes / the Organizes edge - /// passes both realms EXPLICITLY. Idempotent. + /// tagname from its backing RAW tag, so a historized raw tag's UNS reference registers the SAME tagname + /// (both NodeIds → one tagname → HistoryRead works against either). The UNS node binds NO driver — the + /// driver publish path fans values to it (WP3 runtime binding). Every sink call passes + /// / the Organizes edge passes both realms EXPLICITLY. Idempotent. + /// The inherited values are read from the RECORD, not + /// re-looked-up from by RawPath. The record is the + /// single truth: the composer copies them off the backing tag's TagConfigIntent at compose time + /// (both compose seams, byte-parity), and having them ON the record is what makes a raw-side historian / + /// shape edit produce the coincident UnsReferenceDelta that drives this node's repair + /// (remove-previous in Apply, recreate here). A back-lookup would re-introduce the split the + /// 2026-08-15 review found: a record that says one thing and a materialised node built from another. + /// Subset overload (2026-08-15 surgical-deploys). narrows WHAT is + /// materialised to a plan's delta (its added references PLUS the current side of every changed + /// one — Apply removed the previous node, so omitting the currents would leave a re-pointed or + /// renamed reference removed and never recreated). stays the FULL + /// composition for context; a null subset means "the whole composition" (the rebuild path). /// - /// The composition carrying the UNS reference variables + Raw tags (for inherited shape). + /// The FULL composition — the context even when a subset is passed. + /// Materialise only these references; null ⇒ the whole composition's. /// The count of swallowed sink failures in this pass. - public int MaterialiseUnsReferences(AddressSpaceComposition composition) + public int MaterialiseUnsReferences( + AddressSpaceComposition composition, + IReadOnlyList? only = null) { ArgumentNullException.ThrowIfNull(composition); - if (composition.UnsReferenceVariables.Count == 0) return 0; - - // Backing raw tag shape/historian by RawPath — a UNS reference inherits these so its node matches the - // raw node it mirrors (a historized raw tag registers the SAME tagname on the UNS node for HistoryRead). - var rawByPath = new Dictionary(StringComparer.Ordinal); - foreach (var t in composition.RawTags) rawByPath[t.NodeId] = t; + var references = only ?? composition.UnsReferenceVariables; + if (references.Count == 0) return 0; var failed = 0; - foreach (var v in composition.UnsReferenceVariables) + foreach (var v in references) { - var backing = rawByPath.GetValueOrDefault(v.BackingRawPath); - var isArray = backing?.IsArray ?? false; - uint? arrayLength = backing?.ArrayLength; + var isArray = v.BackingIsArray; + uint? arrayLength = v.BackingArrayLength; // A UNS reference to a historized raw tag registers the SAME effective historian tagname (override - // else the backing RawPath) so HistoryRead resolves against the UNS NodeId too. The mux ref stays - // single (keyed by RawPath) — FeedHistorizedRefs emits raw refs only. - string? historianTagname = backing is { IsHistorized: true } - ? (string.IsNullOrWhiteSpace(backing.HistorianTagname) ? backing.NodeId : backing.HistorianTagname) + // else the backing RawPath — BackingRawPath IS the backing raw tag's NodeId) so HistoryRead resolves + // against the UNS NodeId too. The mux ref stays single (keyed by RawPath) — FeedHistorizedRefs emits + // raw refs only. + string? historianTagname = v.BackingIsHistorized + ? (string.IsNullOrWhiteSpace(v.BackingHistorianTagname) ? v.BackingRawPath : v.BackingHistorianTagname) : null; var writable = v.Writable && !isArray; if (!SafeEnsureVariable(v.NodeId, v.EquipmentId, v.EffectiveName, v.DataType, writable, AddressSpaceRealm.Uns, historianTagname, isArray, arrayLength)) @@ -808,8 +969,8 @@ public sealed class AddressSpaceApplier } _logger.LogInformation( - "AddressSpaceApplier: UNS reference variables materialised (refs={Refs}, failed={Failed})", - composition.UnsReferenceVariables.Count, failed); + "AddressSpaceApplier: UNS reference variables materialised (refs={Refs}, subset={Subset}, failed={Failed})", + references.Count, only is not null, failed); return failed; } @@ -826,14 +987,21 @@ public sealed class AddressSpaceApplier /// variables (and any per-tag sub-folder); owns the /// equipment folders and this pass never re-creates them. The sink's EnsureVariable /// takes a plain string dataType (not a DriverAttributeInfo). + /// Subset overload (2026-08-15 surgical-deploys). narrows WHAT is + /// materialised to a plan's added tags; stays the FULL composition for + /// context. A null subset means "the whole composition" (the rebuild path). /// - /// The composition result containing the equipment tags to materialise. + /// The FULL composition — the context even when a subset is passed. + /// Materialise only these tags; null ⇒ the whole composition's. /// The count of swallowed sink failures in this pass (0 on a clean apply) — the publish actor /// sums it into the degraded-apply surface (archreview 01/S-1). - public int MaterialiseEquipmentTags(AddressSpaceComposition composition) + public int MaterialiseEquipmentTags( + AddressSpaceComposition composition, + IReadOnlyList? only = null) { ArgumentNullException.ThrowIfNull(composition); - if (composition.EquipmentTags.Count == 0) return 0; + var equipmentTags = only ?? composition.EquipmentTags; + if (equipmentTags.Count == 0) return 0; var failed = 0; // Sub-folders first — a tag's FolderPath becomes one folder UNDER its equipment folder @@ -841,7 +1009,7 @@ public sealed class AddressSpaceApplier // equipment folder, which MaterialiseHierarchy already created (never re-create // the equipment folder here). var foldersCreated = new HashSet(StringComparer.Ordinal); - foreach (var tag in composition.EquipmentTags) + foreach (var tag in equipmentTags) { if (string.IsNullOrWhiteSpace(tag.FolderPath)) continue; var folderNodeId = UnsSubFolder(tag.EquipmentId, tag.FolderPath); @@ -855,7 +1023,7 @@ public sealed class AddressSpaceApplier // signal. The driver-side FullName lives on EquipmentTagPlan for the later values milestone to // route by. Parent is the FolderPath sub-folder when set, else the equipment folder directly. // Per-variable idempotency relies on the sink's own EnsureVariable. - foreach (var tag in composition.EquipmentTags) + foreach (var tag in equipmentTags) { var parent = string.IsNullOrWhiteSpace(tag.FolderPath) ? tag.EquipmentId @@ -884,9 +1052,10 @@ public sealed class AddressSpaceApplier } _logger.LogInformation( - "AddressSpaceApplier: equipment tags materialised (tags={Tags}, equipment={Equipment}, failed={Failed})", - composition.EquipmentTags.Count, - composition.EquipmentTags.Select(t => t.EquipmentId).Distinct(StringComparer.Ordinal).Count(), + "AddressSpaceApplier: equipment tags materialised (tags={Tags}, equipment={Equipment}, subset={Subset}, failed={Failed})", + equipmentTags.Count, + equipmentTags.Select(t => t.EquipmentId).Distinct(StringComparer.Ordinal).Count(), + only is not null, failed); return failed; } @@ -903,21 +1072,28 @@ public sealed class AddressSpaceApplier /// different equipments never collide in the sink (which keys on NodeId). Variables start /// BadWaitingForInitialData; VirtualTagActor fills live values in a later milestone. /// Idempotent (per-variable idempotency relies on the sink's own EnsureVariable). + /// Subset overload (2026-08-15 surgical-deploys). narrows WHAT is + /// materialised to a plan's added VirtualTags; stays the FULL + /// composition for context. A null subset means "the whole composition" (the rebuild path). /// - /// The composition result containing the equipment VirtualTags to materialise. + /// The FULL composition — the context even when a subset is passed. + /// Materialise only these VirtualTags; null ⇒ the whole composition's. /// The count of swallowed sink failures in this pass (0 on a clean apply) — the publish actor /// sums it into the degraded-apply surface (archreview 01/S-1). - public int MaterialiseEquipmentVirtualTags(AddressSpaceComposition composition) + public int MaterialiseEquipmentVirtualTags( + AddressSpaceComposition composition, + IReadOnlyList? only = null) { ArgumentNullException.ThrowIfNull(composition); - if (composition.EquipmentVirtualTags.Count == 0) return 0; + var virtualTags = only ?? composition.EquipmentVirtualTags; + if (virtualTags.Count == 0) return 0; var failed = 0; // Sub-folders first — a VirtualTag's FolderPath becomes one folder UNDER its equipment folder // (deduped per distinct equipment+path). VirtualTags with no FolderPath hang directly under the // equipment folder, which MaterialiseHierarchy already created (never re-create it here). var foldersCreated = new HashSet(StringComparer.Ordinal); - foreach (var v in composition.EquipmentVirtualTags) + foreach (var v in virtualTags) { if (string.IsNullOrWhiteSpace(v.FolderPath)) continue; var folderNodeId = UnsSubFolder(v.EquipmentId, v.FolderPath); @@ -931,7 +1107,7 @@ public sealed class AddressSpaceApplier // forwards historized results to IHistoryWriter. It is intentionally NOT materialised as an SDK // Historizing/HistoryRead variable here (no server-side OPC UA HistoryRead for vtags), so these // stay plain read-only computed-output nodes. - foreach (var v in composition.EquipmentVirtualTags) + foreach (var v in virtualTags) { var parent = string.IsNullOrWhiteSpace(v.FolderPath) ? v.EquipmentId @@ -942,9 +1118,10 @@ public sealed class AddressSpaceApplier } _logger.LogInformation( - "AddressSpaceApplier: equipment virtualtags materialised (vtags={Vtags}, equipment={Equipment}, failed={Failed})", - composition.EquipmentVirtualTags.Count, - composition.EquipmentVirtualTags.Select(v => v.EquipmentId).Distinct(StringComparer.Ordinal).Count(), + "AddressSpaceApplier: equipment virtualtags materialised (vtags={Vtags}, equipment={Equipment}, subset={Subset}, failed={Failed})", + virtualTags.Count, + virtualTags.Select(v => v.EquipmentId).Distinct(StringComparer.Ordinal).Count(), + only is not null, failed); return failed; } @@ -957,18 +1134,25 @@ public sealed class AddressSpaceApplier /// Disabled alarms are skipped — they expose no node. Must run AFTER /// so the equipment folders exist. Idempotent (the sink's /// MaterialiseAlarmCondition re-creates cleanly on re-apply). + /// Subset overload (2026-08-15 surgical-deploys). narrows WHAT is + /// materialised to a plan's added alarms; stays the FULL composition for + /// context. A null subset means "the whole composition" (the rebuild path). /// - /// The composition result containing the scripted alarms to materialise. + /// The FULL composition — the context even when a subset is passed. + /// Materialise only these alarms; null ⇒ the whole composition's. /// The count of swallowed sink failures in this pass (0 on a clean apply) — the publish actor /// sums it into the degraded-apply surface (archreview 01/S-1). - public int MaterialiseScriptedAlarms(AddressSpaceComposition composition) + public int MaterialiseScriptedAlarms( + AddressSpaceComposition composition, + IReadOnlyList? only = null) { ArgumentNullException.ThrowIfNull(composition); - if (composition.EquipmentScriptedAlarms.Count == 0) return 0; + var alarms = only ?? composition.EquipmentScriptedAlarms; + if (alarms.Count == 0) return 0; var materialised = 0; var failed = 0; - foreach (var alarm in composition.EquipmentScriptedAlarms) + foreach (var alarm in alarms) { if (!alarm.Enabled) continue; if (!SafeMaterialiseAlarmCondition(alarm.ScriptedAlarmId, alarm.EquipmentId, alarm.Name, alarm.AlarmType, alarm.Severity, isNative: false, realm: AddressSpaceRealm.Uns)) failed++; @@ -976,10 +1160,11 @@ public sealed class AddressSpaceApplier } _logger.LogInformation( - "AddressSpaceApplier: scripted alarms materialised (alarms={Alarms}, equipment={Equipment}, failed={Failed})", + "AddressSpaceApplier: scripted alarms materialised (alarms={Alarms}, equipment={Equipment}, subset={Subset}, failed={Failed})", materialised, - composition.EquipmentScriptedAlarms.Where(a => a.Enabled) + alarms.Where(a => a.Enabled) .Select(a => a.EquipmentId).Distinct(StringComparer.Ordinal).Count(), + only is not null, failed); return failed; } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceChangeClassifier.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceChangeClassifier.cs index 52082009..24ebcb4f 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceChangeClassifier.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceChangeClassifier.cs @@ -8,8 +8,9 @@ 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. + /// Only surgical-eligible changed equipment/raw tags, surgical UNS-reference deltas, + /// 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 @@ -25,9 +26,9 @@ public enum AddressSpaceChangeKind /// 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. + /// Anything with a node-affecting change (ChangedEquipment, ChangedAlarms, ChangedRawContainers, + /// non-surgical ChangedEquipmentTags, alarm-bearing/identity-changing ChangedRawTags, node-relevant + /// ChangedEquipmentVirtualTags) — the unclassifiable-safe default. Full rebuild. Rebuild, } @@ -52,25 +53,47 @@ public static class AddressSpaceChangeClassifier 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. The v3 - // Batch-4 Raw/UNS Changed sets (container re-shape, raw-tag attribute/alarm edit, UNS re-point / - // display-name-override) route to Rebuild as the safe default — WP3's applier owns any surgical - // in-place refinement for them. Evaluated BEFORE the add/remove split so a non-surgical change - // alongside pure adds still rebuilds. + // ChangedAlarms / ChangedRawContainers are always node-affecting; a changed equipment tag is + // node-affecting UNLESS it is surgical-eligible, a changed raw tag UNLESS it is + // RawTagDeltaIsSurgicalEligible, and a changed vtag UNLESS it is node-irrelevant. Evaluated BEFORE the + // add/remove split so a non-surgical change alongside pure adds still rebuilds. // - // WP4 BINDING GUARD (Wave C review M3): ChangedRawTags → Rebuild is what keeps native-alarm notifier - // wiring correct today. A native alarm's set of referencing equipment lives in - // RawTagPlan.ReferencingEquipmentPaths (part of its record equality), so ADDING/DROPPING a reference - // makes the raw tag a ChangedRawTag → full rebuild → OtOpcUaNodeManager clears + re-wires the notifiers - // cleanly. ANY future surgical (non-rebuild) ChangedRawTags path MUST also re-wire/unwire the alarm - // notifiers (WireAlarmNotifiers is a reconcile — pass the tag's COMPLETE current referencing set — plus - // an explicit unwire on removal), else a de-referenced equipment keeps receiving the alarm's events. + // WP4 BINDING GUARD (Wave C review M3), as narrowed by the 2026-08-15 surgical-deploys work. The + // guard's subject is NATIVE-ALARM NOTIFIER WIRING: a native alarm's set of referencing equipment lives + // in RawTagPlan.ReferencingEquipmentPaths (part of its record equality), and only a rebuild clears + + // re-wires those notifiers, so an ALARM-BEARING raw tag (Alarm != null on EITHER side) still routes to + // Rebuild. What changed is the blast radius: ReferencingEquipmentPaths is node-INERT for a plain VALUE + // tag, yet adding/dropping ONE UnsTagReference flips its backing raw tag to Changed — so the blanket + // "ChangedRawTags ⇒ Rebuild" tore down every node on the server (killing every client subscription) for + // an edit that creates a single UNS variable. The new contract: + // • alarm-bearing raw-tag delta ⇒ Rebuild (notifier wiring is owned by the rebuild — unchanged); + // • non-alarm raw-tag delta over the in-place-updatable attributes and/or ReferencingEquipmentPaths + // ⇒ surgical (the applier's ISurgicalAddressSpaceSink.UpdateTagAttributes path); + // • any other raw-tag field difference ⇒ Rebuild, via RawTagDeltaIsSurgicalEligible's + // with-equality default-closed whitelist (which also closes over any FUTURE field); + // • ChangedUnsReferenceVariables ⇒ surgical remove-previous + recreate-current, applier-owned (a UNS + // reference variable is a mirror node binding no driver, so it carries no notifier wiring of its + // own — the alarm fan-out it participates in is re-derived from the raw tag's referencing set). + // No rebuild is forced for a UNS-reference delta in EITHER direction, and that is load-bearing for + // the repair path below. + // UNS-REFERENCE STALENESS, and why nothing here gates it (2026-08-15 review). Making non-alarm raw-tag + // deltas surgical meant a raw-side IsHistorized / HistorianTagname / IsArray / ArrayLength edit updated + // the raw node in place while every referencing UNS MIRROR node kept the old historian tagname + shape + // (EnsureVariable early-returns on an existing node) — wrong history series through the UNS NodeId, and + // a scalar→array flip left the mirror scalar AND writable. The fix is in the RECORD, not the classifier: + // UnsReferenceVariable now carries the backing tag's Backing{IsHistorized,HistorianTagname,IsArray, + // ArrayLength}, so the same edit makes the reference record unequal and the planner emits a COINCIDENT + // UnsReferenceDelta alongside the RawTagDelta. Both are non-rebuild kinds, so the plan stays + // AttributeOnly and the applier repairs the mirror surgically (remove-previous + materialise-recreate). + // Adding a new inherited field to UnsReferenceVariable therefore needs no change here — but OMITTING one + // silently reopens the staleness. + // Anything a future surgical raw-tag path lets through that DOES touch alarm notifiers must still + // re-wire/unwire them (WireAlarmNotifiers is a reconcile — pass the tag's COMPLETE current referencing + // set — plus an explicit unwire on removal), else a de-referenced equipment keeps receiving the events. if (plan.ChangedEquipment.Count > 0 || plan.ChangedAlarms.Count > 0 || plan.ChangedRawContainers.Count > 0 || - plan.ChangedRawTags.Count > 0 || - plan.ChangedUnsReferenceVariables.Count > 0 || + plan.ChangedRawTags.Any(d => !RawTagDeltaIsSurgicalEligible(d)) || plan.ChangedEquipmentTags.Any(d => !TagDeltaIsSurgicalEligible(d)) || plan.ChangedEquipmentVirtualTags.Any(d => !VtagDeltaIsNodeIrrelevant(d))) { @@ -141,4 +164,23 @@ public static class AddressSpaceChangeClassifier IsArray = d.Current.IsArray, ArrayLength = d.Current.ArrayLength, }).Equals(d.Current); + + // v3 surgical raw-tag eligibility: a NON-alarm raw tag whose only differences are the in-place-updatable + // attributes (Writable / IsHistorized / HistorianTagname / DataType / IsArray / ArrayLength) and/or its + // ReferencingEquipmentPaths. The referencing set is node-inert for a VALUE tag — it only drives native-alarm + // notifier wiring (the WP4 guard), which is why Alarm != null on EITHER side disqualifies. Identity fields + // (NodeId is the diff key; TagId / DriverInstanceId / Name / ParentNodeId / Realm) still force a rebuild via + // the with-equality default-closed pattern. + internal static bool RawTagDeltaIsSurgicalEligible(AddressSpacePlan.RawTagDelta 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, + ReferencingEquipmentPaths = d.Current.ReferencingEquipmentPaths, + }).Equals(d.Current); } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceComposer.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceComposer.cs index c770e863..90f9e812 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceComposer.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceComposer.cs @@ -137,6 +137,17 @@ public sealed record RawContainerNode( /// referencing equipment. is always . /// A null means the historian tagname defaults to the RawPath (resolved /// later, not here). +/// ADDING A FIELD IS A FOUR-PLACE CONTRACT — the hand-written equality below does NOT close +/// over new members. Because forces a custom +/// Equals/GetHashCode, the compiler's positional equality is suppressed, so a field added +/// to the ctor and nowhere else silently never diffs (a redeploy that changes only it produces an EMPTY +/// plan). Update, in order: (1) the primary constructor, (2) , +/// (3) . Then reconsider two downstream consumers: (4a) +/// AddressSpaceChangeClassifier.RawTagDeltaIsSurgicalEligible's with-equality whitelist — +/// a new field is default-closed (⇒ Rebuild) until it is listed, which is the safe direction but may be +/// the wrong one; and (4b) 's Backing* mirror fields — if the +/// new field is INHERITED by a UNS reference node at materialise time, it must be mirrored there too or +/// referencing UNS nodes go stale on a surgical raw edit (the exact 2026-08-15 review finding). /// public sealed record RawTagPlan( string TagId, @@ -200,6 +211,27 @@ public sealed record RawTagPlan( /// edit keeps the same id while the NodeId/effective-name changes; a backing-tag rename keeps the same id /// + NodeId while re-points). is always /// . +/// / / +/// / mirror the backing raw tag's +/// historian intent + array shape, and they are on THIS record for one reason: to make a raw-side +/// edit produce a COINCIDENT UnsReferenceDelta. The 2026-08-15 surgical-deploys review +/// found that once non-alarm raw-tag deltas became surgical, editing a backing tag's +/// IsHistorized / HistorianTagname / IsArray / ArrayLength left every +/// referencing UNS node STALE: the applier updates the raw node in place, no UNS-reference delta +/// fired (none of those fields were on this record), and EnsureVariable early-returns on an +/// existing node — so HistoryRead via the UNS NodeId served a different series than via the raw +/// NodeId, a de-historized tag kept serving history through UNS, and a scalar→array flip left the +/// UNS mirror scalar AND writable (bypassing the array read-only write gate). Carrying the four +/// fields here makes record equality flag the reference as Changed in the same plan, and the +/// existing machinery repairs it: the applier removes the previous UNS node and +/// recreates it from these values. +/// / were already inherited onto the record and are +/// covered by the same mechanism. +/// Equality is the COMPILER-GENERATED positional-record equality: every member is a +/// value/string type (no IReadOnlyList member, unlike ), so the four +/// new fields are compared by value with no hand-written Equals/GetHashCode needed. If +/// a collection-typed member is ever added here, custom equality becomes mandatory — see the +/// three-place contract comment on . /// public sealed record UnsReferenceVariable( string UnsTagReferenceId, @@ -209,6 +241,10 @@ public sealed record UnsReferenceVariable( string BackingRawPath, string DataType, bool Writable, + bool BackingIsHistorized = false, + string? BackingHistorianTagname = null, + bool BackingIsArray = false, + uint? BackingArrayLength = null, AddressSpaceRealm Realm = AddressSpaceRealm.Uns); public sealed record UnsAreaProjection(string UnsAreaId, string DisplayName); @@ -799,6 +835,12 @@ public static class AddressSpaceComposer continue; // invalid effective-name segment — dropped (deploy gate rejects invalid names) } + // The backing tag's historian intent + array shape ride the record so a raw-side edit to any of + // them produces a coincident UnsReferenceDelta that repairs this mirror node (see the record's + // doc-comment; 2026-08-15 surgical-deploys review). Parsed from the SAME TagConfigIntent the raw + // tag plan uses, so the two sides can never disagree. + var intent = TagConfigIntent.Parse(tag.TagConfig); + vars.Add(new UnsReferenceVariable( UnsTagReferenceId: r.UnsTagReferenceId, EquipmentId: r.EquipmentId, @@ -806,7 +848,11 @@ public static class AddressSpaceComposer EffectiveName: effectiveName, BackingRawPath: rawPath, DataType: tag.DataType, - Writable: tag.AccessLevel == TagAccessLevel.ReadWrite)); + Writable: tag.AccessLevel == TagAccessLevel.ReadWrite, + BackingIsHistorized: intent.IsHistorized, + BackingHistorianTagname: intent.HistorianTagname, + BackingIsArray: intent.IsArray, + BackingArrayLength: intent.ArrayLength)); } vars.Sort((x, y) => string.CompareOrdinal(x.NodeId, y.NodeId)); diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs index 45248cde..8c2883d0 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs @@ -1249,11 +1249,14 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 /// Wave C review M3 — this is a RECONCILE, not additive-only: it wires the supplied set AND unwires any /// previously-tracked folder whose id is no longer in /// (bidirectional), so a de-referenced equipment stops receiving the alarm. Today the applier always - /// passes the COMPLETE referencing-equipment set for the condition, and a de-reference arrives as a - /// ChangedRawTags full rebuild (which clears the wiring), so the unwire leg is a no-op on the - /// current paths — but it makes this method correct for a FUTURE surgical ChangedRawTags path - /// (see AddressSpaceChangeClassifier's ChangedRawTags → Rebuild branch, which carries the - /// matching binding guard). + /// passes the COMPLETE referencing-equipment set for the condition, and a de-reference on an + /// ALARM-BEARING raw tag still arrives as a full rebuild (which clears the wiring), so the unwire leg is + /// a no-op on the current paths — but it makes this method correct for a FUTURE surgical path that + /// reaches an alarm-bearing tag. The 2026-08-15 surgical-deploys work narrowed the classifier's blanket + /// ChangedRawTags → Rebuild branch to alarm-bearing / identity-changing raw-tag deltas + /// only (see AddressSpaceChangeClassifier.RawTagDeltaIsSurgicalEligible, which carries the + /// matching binding guard): a NON-alarm raw tag carries no notifier wiring, so its + /// ReferencingEquipmentPaths edit is node-inert and routes surgically. /// public void WireAlarmNotifiers(string alarmNodeId, AddressSpaceRealm alarmRealm, IReadOnlyList notifierFolderNodeIds, AddressSpaceRealm notifierFolderRealm) @@ -2615,6 +2618,18 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 // caller (AddressSpaceApplier) falls back to a full rebuild (resync). Mirrors the per-node // teardown inside RebuildAddressSpace, scoped to this one id. if (!_variables.TryRemove(key, out var variable)) return false; + // 2026-08-15 review I3: sever the INVERSE half of every cross-tree Organizes edge this node holds + // BEFORE detaching it — the same hazard (and the same fix shape) as UnwireAlarmNotifiers on a + // condition removal. AddReference wires the edge bidirectionally: forward on the UNS reference + // variable, inverse (OrganizedBy) on the backing RAW node. The raw node SURVIVES this scoped + // remove, so without this it keeps a dangling inverse reference to a node that no longer exists — + // browsable, unresolvable, and permanent. + // + // Why it matters more now than it used to: a full RebuildAddressSpace clears every node and every + // reference with it, so these edges were swept up on the next structural deploy. Surgical applies + // made rebuilds rare, so the dangling edges now ACCUMULATE on the raw node for the life of the + // process — one per re-point or effective-name edit of any reference that ever pointed at it. + SeverOutgoingReferences(variable); variable.Parent?.RemoveChild(variable); PredefinedNodes?.Remove(variable.NodeId); // Drop the historized-tagname registration alongside the variable it maps (Phase C parity). @@ -2625,6 +2640,33 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 return true; } + /// + /// 2026-08-15 review I3 — remove the INVERSE half, held on the far node, of every FORWARD reference + /// carries to another node this manager owns. The exact reverse of the + /// bidirectional wiring in , and the variable-node counterpart of + /// . MUST be called under Lock. + /// Only FORWARD references are walked. A node's link to its own parent manifests as an INVERSE + /// reference (the parent owns the forward half through AddChild), and the parent's side of that + /// is severed by Parent.RemoveChild — walking inverse references too would try to unwire the + /// parent link a second time. A forward reference whose target is not one of this manager's nodes is + /// skipped: there is nothing here to mutate, and the target's own owner is responsible for it. + /// + /// The node being removed, whose outgoing edges' inverse halves must be severed. + private void SeverOutgoingReferences(NodeState node) + { + var references = new List(); + node.GetReferences(SystemContext, references); + foreach (var r in references) + { + if (r.IsInverse || r.TargetId is null || r.TargetId.IsAbsolute) continue; + var targetNodeId = (NodeId)r.TargetId; + if (NodeId.IsNull(targetNodeId)) continue; + if (ResolveNodeState(MapKey(targetNodeId)) is not { } target) continue; + if (!target.RemoveReference(r.ReferenceTypeId, isInverse: true, node.NodeId)) continue; + target.ClearChangeMasks(SystemContext, includeChildren: false); + } + } + /// public bool RemoveAlarmConditionNode(string alarmNodeId, AddressSpaceRealm realm) { diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DeploymentArtifact.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DeploymentArtifact.cs index 6e2d199c..9800e657 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DeploymentArtifact.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DeploymentArtifact.cs @@ -1,3 +1,5 @@ +using System.Buffers; +using System.IO.Compression; using System.Text.Json; using System.Text.Json.Nodes; using ZB.MOM.WW.OtOpcUa.Commons.Types; @@ -73,6 +75,14 @@ public sealed record DataPlaneAuthorityProjection( /// The node's ClusterId when is ScopeTo; otherwise null. public readonly record struct ClusterScope(ClusterFilterMode Mode, string? ClusterId); +/// +/// Static, blob-in / result-out facade over . Every entry point +/// here parses the blob, answers ONE question, and throws the parsed document away — so an apply that +/// asks several questions of the same bytes pays for several full builds. +/// Prefer when more than one answer is needed from the same artifact; these wrappers +/// exist so the one-shot call sites (and every existing test) keep their exact shape and their lenient +/// empty/malformed contract. +/// public static class DeploymentArtifact { private static readonly JsonSerializerOptions JsonOptions = new() @@ -80,6 +90,113 @@ public static class DeploymentArtifact PropertyNameCaseInsensitive = true, }; + /// + /// Parse ONCE into a reusable , so + /// every consumer of a single apply reads the same instead of rebuilding + /// it per question. Dispose it when the apply completes. + /// + /// + /// + /// This is the ONLY place an artifact blob is decompressed. ConfigComposer gzips the + /// serialised snapshot BEFORE hashing it, so the compressed bytes are the artifact everywhere: + /// Deployment.ArtifactBlob, the LocalDb cache chunks, the gRPC fetch stream, and the + /// RevisionHash the fetcher verifies against are all over these exact bytes. Nothing between + /// the composer and here looks inside the blob; every reader comes through this method, so a + /// gzipped artifact and a pre-upgrade uncompressed one are indistinguishable to every consumer. + /// + /// + /// The deployment artifact bytes, gzip-compressed or raw JSON (sniffed by the gzip + /// magic 1F 8B, which valid JSON can never start with). Raw-JSON bytes are not copied — the returned + /// artifact reads directly from this array, which must not be mutated for its lifetime (artifact blobs never + /// are: they are deployment-immutable bytes read from SQL, the LocalDb cache, or a config fetch). A gzipped + /// blob is decompressed into a fresh array the artifact owns. + /// The parsed artifact. The caller owns it and must dispose it. + /// The blob is not valid JSON, is not decompressible, or inflates past + /// . Unlike the static wrappers — which own the lenient "malformed ⇒ + /// empty result" contract — opening is explicit about unusable bytes. + internal static ParsedDeploymentArtifact Open(byte[] blob) => + new(JsonDocument.Parse(Inflate(blob, MaxDecompressedBytes))); + + /// + /// Hard ceiling on the decompressed size of an artifact blob (512 MiB). A corrupt or hostile blob + /// must not be able to OOM the node: gzip over repetitive input reaches ~1000:1, so a few-hundred-KB + /// row could otherwise inflate to hundreds of megabytes. No honest artifact comes close — the real + /// ones are single-digit MB before compression. + /// + /// + /// A deliberately: it used to be a mutable static that the size-guard test + /// lowered and restored. xUnit runs test CLASSES in parallel, so that window was visible to every + /// sibling class parsing a >4 KB artifact concurrently — a spurious "refusing to inflate" failure + /// with no relation to the test that caused it. The limit is a parameter + /// instead, so the guard is provable at any ceiling without any process-wide mutation. + /// + internal const int MaxDecompressedBytes = 512 * 1024 * 1024; + + /// + /// Return the artifact's JSON bytes, decompressing when the blob carries the gzip magic + /// 1F 8B at offset 0. Raw-JSON blobs (everything written before compression shipped) are + /// returned as-is — a JSON document can never begin with 0x1F, so the sniff is unambiguous + /// and no framing of our own is needed. + /// + /// + /// + /// gzip output is NOT guaranteed byte-stable across .NET releases. That is deliberately harmless + /// here: the revision hash is computed FROM the emitted bytes at compose time and is never + /// re-derived from the JSON, so the worst an implementation change can cost is one spurious + /// "changed" revision on the first compose after a runtime upgrade. + /// + /// + /// Every failure — corrupt stream, truncated stream, over-limit inflation — is reported as + /// so it lands in the exact catch the malformed-JSON path already + /// owns: the static wrappers degrade to empty results, and the apply paths fail the apply and + /// keep last-known-good (#485 "no bytes is no answer"). Unusable bytes are unusable bytes; the + /// reason they are unusable does not change any caller's decision. + /// + /// + /// The stored/wire artifact bytes. + /// Ceiling on the inflated size. Production always passes + /// via ; the parameter exists so the guard can be + /// proved at a small ceiling without mutating process-wide state that parallel test classes share. + /// The JSON bytes to parse. + /// The gzip stream is corrupt or inflates past . + internal static byte[] Inflate(byte[] blob, int maxBytes) + { + if (blob.Length < 2 || blob[0] != 0x1F || blob[1] != 0x8B) return blob; + + var limit = maxBytes; + try + { + using var input = new MemoryStream(blob, writable: false); + using var gzip = new GZipStream(input, CompressionMode.Decompress); + using var output = new MemoryStream((int)Math.Min(blob.Length * 4L, 1 << 20)); + + var buffer = ArrayPool.Shared.Rent(64 * 1024); + try + { + int read; + while ((read = gzip.Read(buffer, 0, buffer.Length)) > 0) + { + if (output.Length + read > limit) + { + throw new JsonException( + $"Deployment artifact decompresses to more than the {limit}-byte limit; refusing to inflate it."); + } + output.Write(buffer, 0, read); + } + } + finally + { + ArrayPool.Shared.Return(buffer); + } + + return output.ToArray(); + } + catch (Exception ex) when (ex is InvalidDataException or EndOfStreamException or IOException) + { + throw new JsonException("Deployment artifact is not a readable gzip stream.", ex); + } + } + /// /// Parse a deployment artifact blob into the list of driver-instance specs to spawn. /// Empty / malformed blobs return an empty list — callers log + treat as "no drivers". @@ -89,32 +206,10 @@ public static class DeploymentArtifact public static IReadOnlyList ParseDriverInstances(ReadOnlySpan blob) { if (blob.IsEmpty) return Array.Empty(); - try { - using var doc = JsonDocument.Parse(blob.ToArray()); - var root = doc.RootElement; - if (!root.TryGetProperty("DriverInstances", out var arr) - || arr.ValueKind != JsonValueKind.Array) - { - return Array.Empty(); - } - - // v3: each spec's DriverConfig is the MERGED config the driver factory binds from — - // DriverConfig (protocol/channel) + per-Device DeviceConfig (endpoint) + the driver's authored - // raw tags (RawTags array, keyed by RawPath) — folded once here via DriverDeviceConfigMerger so - // both spawn (TryCreate) and redeploy (ApplyDelta) hand the driver the same shape. - var rawTagsByDriver = BuildRawTagsByDriver(root); - var deviceRowsByDriver = BuildDeviceRowsByDriver(root); - - var result = new List(arr.GetArrayLength()); - foreach (var el in arr.EnumerateArray()) - { - if (el.ValueKind != JsonValueKind.Object) continue; - var spec = TryReadSpec(el, rawTagsByDriver, deviceRowsByDriver); - if (spec is not null) result.Add(spec); - } - return result; + using var parsed = Open(blob.ToArray()); + return parsed.ParseDriverInstances(); } catch (JsonException) { @@ -122,6 +217,209 @@ public static class DeploymentArtifact } } + /// + /// Resolve how a node should scope a deployment artifact to its own ClusterId. Single-cluster + /// (or legacy) artifacts resolve to so every existing + /// deployment applies unchanged. In a multi-cluster artifact the node's ClusterNode row + /// (matched by ) selects its + /// ClusterId; a missing row resolves to . Empty / + /// malformed blobs resolve to (lenient, matching the + /// other parsers). + /// + /// The deployment artifact blob to inspect. + /// The node's identity (e.g. a "host:port" string) to match against Nodes. + /// The resolved decision for this node. + public static ClusterScope ResolveClusterScope(ReadOnlySpan blob, string nodeId) + { + if (blob.IsEmpty) return new ClusterScope(ClusterFilterMode.None, null); + try + { + using var parsed = Open(blob.ToArray()); + return parsed.ResolveClusterScope(nodeId); + } + catch (JsonException) + { + return new ClusterScope(ClusterFilterMode.None, null); + } + } + + /// + /// Parse a deployment artifact blob into the driver-instance specs a specific node should + /// spawn, scoped to its own ClusterId via . Single-cluster / + /// legacy artifacts return every spec; a multi-cluster artifact returns only the matching + /// cluster's specs (or none when the node's row is absent). + /// + /// The deployment artifact blob to parse. + /// The node's identity (e.g. a "host:port" string) used to resolve cluster scope. + /// The driver-instance specs this node should spawn. + public static IReadOnlyList ParseDriverInstances(ReadOnlySpan blob, string nodeId) + { + if (blob.IsEmpty) return Array.Empty(); + try + { + using var parsed = Open(blob.ToArray()); + return parsed.ParseDriverInstances(nodeId); + } + catch (JsonException) + { + return Array.Empty(); + } + } + + /// + /// Parse the data-plane authorization inputs a node needs to authenticate OPC UA sessions: + /// the ApiKeys the artifact carries and the NodeAcls granting their principals + /// permissions (#520, stage 4). See + /// for the scoping + leniency contract. + /// + /// The deployment artifact blob to parse. + /// The node's identity, used to resolve cluster scope for the grants. + /// The node's cluster (null when single-cluster), its keys, and its cluster's grants. + public static DataPlaneAuthorityProjection ParseDataPlaneAuthority(ReadOnlySpan blob, string nodeId) + { + if (blob.IsEmpty) return DataPlaneAuthorityProjection.Empty; + try + { + using var parsed = Open(blob.ToArray()); + return parsed.ParseDataPlaneAuthority(nodeId); + } + catch (JsonException) + { + return DataPlaneAuthorityProjection.Empty; + } + } + + /// + /// Parse the artifact into the projected used by + /// AddressSpacePlanner + AddressSpaceApplier. Returns an empty composition for empty/ + /// malformed blobs so callers can treat parse failure as a no-op deploy. + /// + /// The deployment artifact blob to parse. + /// The projected , or an empty composition for empty/malformed blobs. + public static AddressSpaceComposition ParseComposition(ReadOnlySpan blob) + { + if (blob.IsEmpty) return ParsedDeploymentArtifact.Empty(); + try + { + using var parsed = Open(blob.ToArray()); + return parsed.ParseComposition(); + } + catch (JsonException) + { + return ParsedDeploymentArtifact.Empty(); + } + } + + /// Cluster-scoped overload: the address-space composition a node should materialise given + /// its NodeId. See . + /// The deployment artifact blob. + /// This node's identity in "host:port" form. + /// Optional diagnostic callback for cross-cluster orphan bindings; null disables the check. + /// The filtered composition per the node's scoping decision. + public static AddressSpaceComposition ParseComposition( + ReadOnlySpan blob, string nodeId, Action? onInconsistency = null) + { + if (blob.IsEmpty) return ParsedDeploymentArtifact.Empty(); + try + { + using var parsed = Open(blob.ToArray()); + return parsed.ParseComposition(nodeId, onInconsistency); + } + catch (JsonException) + { + return ParsedDeploymentArtifact.Empty(); + } + } +} + +/// +/// A deployment artifact parsed ONCE — one serving every consumer of a single +/// apply (driver specs, cluster scope, data-plane authority, address-space composition), instead of the +/// full copy-and-reparse each static entry point on pays per call. +/// +/// +/// +/// Open it with at the top of an apply and dispose it when the +/// apply completes. Everything it returns is fully materialised (records, lists, strings) and holds +/// no reference back into the document, so results stay valid after disposal. +/// +/// +/// Not thread-safe: the per-node memo is an unsynchronised +/// dictionary. An apply resolves its consumers sequentially, which is the only intended use. +/// +/// +/// Internal on purpose. Every consumer is inside this assembly (the apply paths in +/// DriverHostActor and OpcUaPublishActor, plus the tests via InternalsVisibleTo), +/// and the lenient static wrappers on are the cross-assembly door. +/// Keeping the disposable, non-thread-safe, document-owning type unexported is what makes the +/// "results survive disposal" contract enforceable rather than merely documented. +/// +/// +internal sealed class ParsedDeploymentArtifact : IDisposable +{ + private readonly JsonDocument _document; + + // Every consumer of one apply resolves the SAME node id, so this memo turns N identical scope scans + // into one. Keyed ordinally: the scan itself matches node ids case-insensitively, so a differently + // cased key simply misses the memo and re-scans to the same answer — never a wrong one. + private readonly Dictionary _scopeByNodeId = new(StringComparer.Ordinal); + + private bool _disposed; + + /// Takes ownership of an already-parsed artifact document. Use + /// ; this exists so the document is parsed in exactly one place. + /// The parsed artifact document, disposed with this instance. + internal ParsedDeploymentArtifact(JsonDocument document) => _document = document; + + /// + /// The artifact's root JSON object. Valid only until — do not retain it, + /// and do not hand it outside this type. Unlike everything the parse methods return (fully + /// materialised records, lists and strings that hold no reference back into the document), a + /// is a live view over the disposed-with-us : + /// reading one after disposal throws, or worse, reads a recycled pooled buffer. Internal so that + /// escape hatch cannot be taken from outside the assembly. + /// + internal JsonElement Root => _document.RootElement; + + /// Releases the underlying . Idempotent. + public void Dispose() + { + if (_disposed) return; + _disposed = true; + _document.Dispose(); + } + + /// + /// The list of driver-instance specs to spawn, from the artifact's DriverInstances array. + /// An artifact with no such array yields an empty list. + /// + /// The parsed driver-instance specs. + public IReadOnlyList ParseDriverInstances() + { + var root = Root; + if (!root.TryGetProperty("DriverInstances", out var arr) + || arr.ValueKind != JsonValueKind.Array) + { + return Array.Empty(); + } + + // v3: each spec's DriverConfig is the MERGED config the driver factory binds from — + // DriverConfig (protocol/channel) + per-Device DeviceConfig (endpoint) + the driver's authored + // raw tags (RawTags array, keyed by RawPath) — folded once here via DriverDeviceConfigMerger so + // both spawn (TryCreate) and redeploy (ApplyDelta) hand the driver the same shape. + var rawTagsByDriver = BuildRawTagsByDriver(root); + var deviceRowsByDriver = BuildDeviceRowsByDriver(root); + + var result = new List(arr.GetArrayLength()); + foreach (var el in arr.EnumerateArray()) + { + if (el.ValueKind != JsonValueKind.Object) continue; + var spec = TryReadSpec(el, rawTagsByDriver, deviceRowsByDriver); + if (spec is not null) result.Add(spec); + } + return result; + } + /// Flatten the artifact's raw topology (RawFolders / DriverInstances / Devices / TagGroups / /// Tags) into each driver's authored list, keyed by DriverInstanceId. Each /// tag's RawPath is computed via the shared (the single identity authority), @@ -499,71 +797,67 @@ public static class DeploymentArtifact el.TryGetProperty(property, out var p) && p.ValueKind == JsonValueKind.String ? p.GetString() : null; /// - /// Resolve how a node should scope a deployment artifact to its own ClusterId. Single-cluster + /// Resolve how a node should scope this artifact to its own ClusterId. Single-cluster /// (or legacy) artifacts resolve to so every existing /// deployment applies unchanged. In a multi-cluster artifact the node's ClusterNode row /// (matched by ) selects its - /// ClusterId; a missing row resolves to . Empty / - /// malformed blobs resolve to (lenient, matching the - /// other parsers). + /// ClusterId; a missing row resolves to . + /// Memoized per node id — every consumer of one apply asks about the same node. /// - /// The deployment artifact blob to inspect. /// The node's identity (e.g. a "host:port" string) to match against Nodes. /// The resolved decision for this node. - public static ClusterScope ResolveClusterScope(ReadOnlySpan blob, string nodeId) + public ClusterScope ResolveClusterScope(string nodeId) { - if (blob.IsEmpty) return new ClusterScope(ClusterFilterMode.None, null); - try - { - using var doc = JsonDocument.Parse(blob.ToArray()); - var root = doc.RootElement; - var clusterCount = root.TryGetProperty("Clusters", out var cl) && cl.ValueKind == JsonValueKind.Array - ? cl.GetArrayLength() : 0; - if (clusterCount <= 1) return new ClusterScope(ClusterFilterMode.None, null); + if (_scopeByNodeId.TryGetValue(nodeId, out var cached)) return cached; + var resolved = ResolveClusterScopeCore(nodeId); + _scopeByNodeId[nodeId] = resolved; + return resolved; + } - string? myCluster = null; - if (root.TryGetProperty("Nodes", out var nodes) && nodes.ValueKind == JsonValueKind.Array) - { - foreach (var el in nodes.EnumerateArray()) - { - if (el.ValueKind != JsonValueKind.Object) continue; - var nid = el.TryGetProperty("NodeId", out var nEl) ? nEl.GetString() : null; - if (!string.Equals(nid, nodeId, StringComparison.OrdinalIgnoreCase)) continue; - myCluster = el.TryGetProperty("ClusterId", out var cEl) ? cEl.GetString() : null; - break; - } - } - return string.IsNullOrWhiteSpace(myCluster) - ? new ClusterScope(ClusterFilterMode.Suppress, null) - : new ClusterScope(ClusterFilterMode.ScopeTo, myCluster); - } - catch (JsonException) + private ClusterScope ResolveClusterScopeCore(string nodeId) + { + var root = Root; + var clusterCount = root.TryGetProperty("Clusters", out var cl) && cl.ValueKind == JsonValueKind.Array + ? cl.GetArrayLength() : 0; + if (clusterCount <= 1) return new ClusterScope(ClusterFilterMode.None, null); + + string? myCluster = null; + if (root.TryGetProperty("Nodes", out var nodes) && nodes.ValueKind == JsonValueKind.Array) { - return new ClusterScope(ClusterFilterMode.None, null); + foreach (var el in nodes.EnumerateArray()) + { + if (el.ValueKind != JsonValueKind.Object) continue; + var nid = el.TryGetProperty("NodeId", out var nEl) ? nEl.GetString() : null; + if (!string.Equals(nid, nodeId, StringComparison.OrdinalIgnoreCase)) continue; + myCluster = el.TryGetProperty("ClusterId", out var cEl) ? cEl.GetString() : null; + break; + } } + return string.IsNullOrWhiteSpace(myCluster) + ? new ClusterScope(ClusterFilterMode.Suppress, null) + : new ClusterScope(ClusterFilterMode.ScopeTo, myCluster); } /// - /// Parse a deployment artifact blob into the driver-instance specs a specific node should - /// spawn, scoped to its own ClusterId via . Single-cluster / - /// legacy artifacts return every spec; a multi-cluster artifact returns only the matching - /// cluster's specs (or none when the node's row is absent). + /// The driver-instance specs a specific node should spawn, scoped to its own ClusterId via + /// . Single-cluster / legacy artifacts return every spec; + /// a multi-cluster artifact returns only the matching cluster's specs (or none when the node's row + /// is absent). /// - /// The deployment artifact blob to parse. /// The node's identity (e.g. a "host:port" string) used to resolve cluster scope. /// The driver-instance specs this node should spawn. - public static IReadOnlyList ParseDriverInstances(ReadOnlySpan blob, string nodeId) + public IReadOnlyList ParseDriverInstances(string nodeId) { - var scope = ResolveClusterScope(blob, nodeId); + var scope = ResolveClusterScope(nodeId); if (scope.Mode == ClusterFilterMode.Suppress) return Array.Empty(); - var all = ParseDriverInstances(blob); + var all = ParseDriverInstances(); return scope.Mode == ClusterFilterMode.ScopeTo ? all.Where(s => string.Equals(s.ClusterId, scope.ClusterId, StringComparison.OrdinalIgnoreCase)).ToArray() : all; } /// - /// Parse the data-plane authorization inputs a node needs to authenticate OPC UA sessions: + /// The data-plane authorization inputs a node needs to authenticate OPC UA sessions: /// the ApiKeys the artifact carries and the NodeAcls granting their principals /// permissions (#520, stage 4). /// @@ -573,60 +867,48 @@ public static class DeploymentArtifact /// it with no ConfigDb, so without this it denies every UserName token. /// /// - /// Grants are scoped to the node's own cluster (); keys are - /// not, because ApiKey carries no ClusterId — a key is a fleet-wide credential and it + /// Grants are scoped to the node's own cluster (); keys + /// are not, because ApiKey carries no ClusterId — a key is a fleet-wide credential and it /// is the ACL rows that bind it to a cluster's nodes. A /// scope — the node's row is absent from a multi-cluster artifact — yields NO grants, matching /// the driver-spawn path's reading that an unlisted node was not deployed to. /// /// - /// Lenient like every other parser here: an empty or malformed blob yields empty lists. The - /// caller must NOT treat that as "no keys are valid" and install it — see issue #485; the - /// apply paths only reach this with bytes that already passed their own emptiness guard. + /// Lenient like every other parser here: an artifact carrying neither section yields empty + /// lists. The caller must NOT treat that as "no keys are valid" and install it — see issue + /// #485; the apply paths only reach this with bytes that already passed their own guard. /// /// - /// The deployment artifact blob to parse. /// The node's identity, used to resolve cluster scope for the grants. /// The node's cluster (null when single-cluster), its keys, and its cluster's grants. - public static DataPlaneAuthorityProjection ParseDataPlaneAuthority(ReadOnlySpan blob, string nodeId) + public DataPlaneAuthorityProjection ParseDataPlaneAuthority(string nodeId) { - if (blob.IsEmpty) return DataPlaneAuthorityProjection.Empty; + var scope = ResolveClusterScope(nodeId); + var root = Root; - var scope = ResolveClusterScope(blob, nodeId); - - try + var keys = new List(); + foreach (var el in EnumerateArray(root, "ApiKeys")) { - using var doc = JsonDocument.Parse(blob.ToArray()); - var root = doc.RootElement; - - var keys = new List(); - foreach (var el in EnumerateArray(root, "ApiKeys")) - { - var key = TryReadApiKey(el); - if (key is not null) keys.Add(key); - } - - var grants = new List(); - foreach (var el in EnumerateArray(root, "NodeAcls")) - { - var acl = TryReadNodeAcl(el); - if (acl is null) continue; - if (scope.Mode == ClusterFilterMode.Suppress) continue; - if (scope.Mode == ClusterFilterMode.ScopeTo - && !string.Equals(acl.ClusterId, scope.ClusterId, StringComparison.OrdinalIgnoreCase)) - { - continue; - } - grants.Add(acl); - } - - return new DataPlaneAuthorityProjection( - scope.ClusterId, keys, grants, BuildAclScopePaths(root, grants)); + var key = TryReadApiKey(el); + if (key is not null) keys.Add(key); } - catch (JsonException) + + var grants = new List(); + foreach (var el in EnumerateArray(root, "NodeAcls")) { - return DataPlaneAuthorityProjection.Empty; + var acl = TryReadNodeAcl(el); + if (acl is null) continue; + if (scope.Mode == ClusterFilterMode.Suppress) continue; + if (scope.Mode == ClusterFilterMode.ScopeTo + && !string.Equals(acl.ClusterId, scope.ClusterId, StringComparison.OrdinalIgnoreCase)) + { + continue; + } + grants.Add(acl); } + + return new DataPlaneAuthorityProjection( + scope.ClusterId, keys, grants, BuildAclScopePaths(root, grants)); } /// @@ -880,25 +1162,19 @@ public static class DeploymentArtifact /// /// Parse the artifact into the projected used by - /// AddressSpacePlanner + AddressSpaceApplier. Returns an empty composition for empty/ - /// malformed blobs so callers can treat parse failure as a no-op deploy. + /// AddressSpacePlanner + AddressSpaceApplier. An artifact carrying none of the + /// projected arrays yields an empty composition, which callers treat as a no-op deploy. /// /// The artifact JSON is produced by ConfigComposer.SnapshotAndFlattenAsync in the /// ControlPlane — its Pascal-case property names match the EF entities. We only need a /// subset of fields per entity class to drive the address-space rebuild on driver-role /// nodes. /// - /// The deployment artifact blob to parse. - /// The projected , or an empty composition for empty/malformed blobs. - public static AddressSpaceComposition ParseComposition(ReadOnlySpan blob) + /// The projected . + public AddressSpaceComposition ParseComposition() { - if (blob.IsEmpty) return Empty(); - - try + var root = Root; { - using var doc = JsonDocument.Parse(blob.ToArray()); - var root = doc.RootElement; - var areas = ReadArray(root, "UnsAreas", ReadAreaProjection); var lines = ReadArray(root, "UnsLines", ReadLineProjection); var equipment = ReadArray(root, "Equipment", ReadEquipmentNode); @@ -932,14 +1208,10 @@ public static class DeploymentArtifact UnsReferenceVariables = unsReferenceVariables, }; } - catch (JsonException) - { - return Empty(); - } } /// Cluster-scoped overload: the address-space composition a node should materialise given - /// its NodeId. Filters every projection to the node's own ClusterId (see ). + /// its NodeId. Filters every projection to the node's own ClusterId (see ). /// Equipment attribution is dual-path: driver-bound equipment (non-null DriverInstanceId) is kept when /// its driver is in-cluster; driver-less equipment (null DriverInstanceId) is kept when its UNS line's /// area is in-cluster. @@ -948,19 +1220,17 @@ public static class DeploymentArtifact /// that violates the same-cluster invariant and would orphan the equipment folder. This is /// detection only (observability); the equipment is still returned, since the upstream draft validator /// is the authority that should prevent the binding in the first place. - /// The deployment artifact blob. /// This node's identity in "host:port" form. /// Optional diagnostic callback for cross-cluster orphan bindings; null disables the check. /// The filtered composition per the node's scoping decision. - public static AddressSpaceComposition ParseComposition( - ReadOnlySpan blob, string nodeId, Action? onInconsistency = null) + public AddressSpaceComposition ParseComposition(string nodeId, Action? onInconsistency = null) { - var scope = ResolveClusterScope(blob, nodeId); - if (scope.Mode == ClusterFilterMode.None) return ParseComposition(blob); + var scope = ResolveClusterScope(nodeId); + if (scope.Mode == ClusterFilterMode.None) return ParseComposition(); if (scope.Mode == ClusterFilterMode.Suppress) return Empty(); - var full = ParseComposition(blob); - var sets = BuildClusterSets(blob, scope.ClusterId!); + var full = ParseComposition(); + var sets = BuildClusterSets(Root, scope.ClusterId!); var keptLines = full.UnsLines.Where(l => sets.AreaIds.Contains(l.UnsAreaId)).ToArray(); var keptEquipment = full.EquipmentNodes.Where(e => sets.EquipmentIds.Contains(e.EquipmentId)).ToArray(); @@ -1012,18 +1282,16 @@ public static class DeploymentArtifact /// that directly carry the ClusterId, plus in-cluster EquipmentIds — driver-bound equipment attributed /// by its driver's cluster, and driver-less equipment (null DriverInstanceId) attributed by its UNS /// line's area cluster. - /// The deployment artifact blob. + /// The artifact root element — the SAME document every other consumer reads, rather + /// than a second parse of the same bytes (this used to re-parse the blob on every scoped compose). /// The node's ClusterId to scope to. - /// The resolved in-cluster id sets (empty on parse failure => empty composition). - private static ClusterSets BuildClusterSets(ReadOnlySpan blob, string clusterId) + /// The resolved in-cluster id sets. + private static ClusterSets BuildClusterSets(JsonElement root, string clusterId) { var driverIds = new HashSet(StringComparer.Ordinal); var areaIds = new HashSet(StringComparer.Ordinal); var equipmentIds = new HashSet(StringComparer.Ordinal); - try { - using var doc = JsonDocument.Parse(blob.ToArray()); - var root = doc.RootElement; CollectIdsWhereCluster(root, "DriverInstances", "DriverInstanceId", clusterId, driverIds); CollectIdsWhereCluster(root, "UnsAreas", "UnsAreaId", clusterId, areaIds); // Map each UnsLine to its owning UnsArea so driver-less equipment can be attributed via @@ -1055,7 +1323,6 @@ public static class DeploymentArtifact } } } - catch (JsonException) { /* empty sets => nothing matches => empty composition */ } return new ClusterSets(driverIds, areaIds, equipmentIds); } @@ -1080,7 +1347,9 @@ public static class DeploymentArtifact } } - private static AddressSpaceComposition Empty() => new( + /// The composition an absent / empty / unparseable artifact projects to: nothing at all. + /// An empty composition. + internal static AddressSpaceComposition Empty() => new( Array.Empty(), Array.Empty(), Array.Empty(), diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs index e56b1c31..cab1515b 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs @@ -54,7 +54,7 @@ namespace ZB.MOM.WW.OtOpcUa.Runtime.Drivers; /// AttributeValuePublished survives only as the message /// fabricates straight to the dependency mux; the host itself no longer receives it. /// -public sealed class DriverHostActor : ReceiveActor, IWithTimers +public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedStash { public const string DeploymentsTopic = "deployments"; public const string DeploymentAcksTopic = "deployment-acks"; @@ -64,6 +64,24 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers /// Publishing interval handed to each driver's SubscribeBulk pass after an apply. private static readonly TimeSpan SubscriptionPublishingInterval = TimeSpan.FromSeconds(1); + /// Timer key for the -state wedge watchdog. Started by + /// , cancelled by . + private const string ApplyWedgeTimerKey = "apply-wedged"; + + /// + /// How long the host may sit in before it says so. + /// + /// + /// Five minutes ≈ 2× the worst configured artifact-fetch deadline, so an ordinary slow fetch + /// never trips it and a genuinely wedged one always does. It is not a timeout: nothing is + /// cancelled or failed, because the apply may still land and the coordinator's own apply + /// deadline is the authority on when a deployment is dead. The watchdog exists purely so a host + /// stuck in is VISIBLE — the pre-stash forward-spin used to make that + /// unmissable (a flood of log lines), and taking the spin away accidentally took the only alarm + /// with it. + /// + public static readonly TimeSpan DefaultApplyWedgeTimeout = TimeSpan.FromMinutes(5); + /// /// Cache key used when an artifact carries no cluster scoping (single-cluster or unscoped /// deployments). A literal rather than an empty string so the row is visibly deliberate when @@ -148,6 +166,51 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers private readonly IActorRef? _scriptedAlarmHostOverride; private readonly ILoggingAdapter _log = Context.GetLogger(); + /// + /// Holds dispatches that arrive for a DIFFERENT deployment while one is being applied, until + /// the apply transitions back to Steady. Assigned by the ActorCell after construction — + /// it is inside the constructor, so nothing there may touch it. + /// + /// + /// The predecessor was Self.Forward(msg), which put the message back at the tail of this + /// actor's OWN mailbox: with nothing else queued, the host spun dequeue → log → enqueue for the + /// whole apply (a FetchAndCache fetch window is seconds of network I/O), burning a core + /// and emitting a log line per iteration — measured at ~570k lines in 1.5 s. The stash holds the + /// message OUTSIDE the mailbox, so the host simply idles until there is something real to do. + /// + public IStash Stash { get; set; } = null!; + + /// + /// The set of deployment ids currently sitting in , so the + /// state stashes each distinct deployment AT MOST ONCE. Cleared in + /// , which is the same moment empties + /// the stash itself — a dispatch that is re-stashed by the NEXT apply re-adds itself naturally. + /// + /// + /// + /// It bounds the stash by DISTINCT deployments. A coordinator that re-dispatches on a + /// timer (or several coordinators racing) can emit the same deployment many times inside one + /// apply window; without this, every copy is stashed and every copy is re-delivered on the + /// way back to — an unbounded queue of messages that will all + /// short-circuit as duplicates anyway. + /// + /// + /// It also moots an Akka stash-equality question — which was raised in review as a + /// restart hazard and, MEASURED, is not one on the pinned version. The string + /// "Can't stash the same message {0} more than once" (the + /// IllegalActorStateException that AbstractStash.Stash() is documented to + /// throw when an envelope equals the last one stashed) IS present verbatim in the shipped + /// Akka.dll 1.5.62. But a throwaway probe actor on this exact package — stashing one + /// message instance twice in a row, then two value-equal instances, both with and without an + /// explicit sender — accepted all four stashes and never threw. So the guard is not on this + /// path in 1.5.62, and the dedup set is NOT fixing a live crash. What it does is make the + /// question moot: since is a sealed record (value + /// equality), a future Akka that does enforce the check would restart this host in the + /// middle of a deployment, and nothing here would have to change to stay safe. + /// + /// + private readonly HashSet _deferredDispatches = new(); + /// The single VirtualTag-host child that spawns/reconciles Equipment-namespace /// VirtualTagActors and bridges their results onto the OPC UA publish actor. Spawned in /// when an OPC UA publish actor is wired; receives @@ -306,6 +369,10 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers /// private readonly Func? _sharesHistorizationScopeWithLocal; + /// This host's -wedge watchdog delay (see + /// ). + private readonly TimeSpan _applyWedgeTimeout; + /// /// Host of this node's LocalDb replication partner — the only node that holds a copy of this /// node's alarm queue, and therefore the only node it may ever stand down in favour of. @@ -417,6 +484,8 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers /// Per-cluster mesh Phase 5: optional node-local live-telemetry hub each /// Primary-gated native-alarm transition is also emitted into, and which is threaded into the spawned /// VirtualTag + ScriptedAlarm hosts. Defaults to null (admin-only nodes / tests skip the emit). + /// How long the host may sit in before the + /// wedge watchdog logs a Warning. Defaults to . /// The Akka.NET used to spawn this actor. public static Props Props( IDbContextFactory? dbFactory, @@ -444,7 +513,8 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers ITelemetryLocalHub? telemetryHub = null, IDriverPrimaryRoleSource? primaryRoleSource = null, IHistorizationRoleView? historizationRoleView = null, - Func? sharesHistorizationScopeWithLocal = null) => + Func? sharesHistorizationScopeWithLocal = null, + TimeSpan? applyWedgeTimeout = null) => // WARNING: this forwarding list is POSITIONAL, and Props.Create compiles it into an // expression tree. Six IActorRef? parameters and several interface-typed ones mean a // mis-ordered argument is usually type-compatible and therefore compiles clean, then binds @@ -456,7 +526,7 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers loggerFactory, scriptRootLogger, scriptedAlarmHostOverride, invokerFactory, driverMemberCountProvider, deploymentArtifactCache, redundancyRoleView, replicationPeerHost, fetchAndCacheMode, artifactFetcher, alarmStateStore, telemetryHub, primaryRoleSource, historizationRoleView, - sharesHistorizationScopeWithLocal)); + sharesHistorizationScopeWithLocal, applyWedgeTimeout)); /// Initializes a new DriverHostActor with the specified dependencies. /// Database context factory for configuration database access. @@ -512,6 +582,9 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers /// snapshot serves this node's address space and therefore records the same historized refs. Null /// (the default) means the caller could not establish it, and this node never stands down — see /// . + /// How long the host may sit in before the + /// wedge watchdog logs a Warning. Defaults to ; a test shortens + /// it so the watchdog is observable in a test's lifetime. public DriverHostActor( IDbContextFactory? dbFactory, CommonsNodeId localNode, @@ -538,8 +611,10 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers ITelemetryLocalHub? telemetryHub = null, IDriverPrimaryRoleSource? primaryRoleSource = null, IHistorizationRoleView? historizationRoleView = null, - Func? sharesHistorizationScopeWithLocal = null) + Func? sharesHistorizationScopeWithLocal = null, + TimeSpan? applyWedgeTimeout = null) { + _applyWedgeTimeout = applyWedgeTimeout ?? DefaultApplyWedgeTimeout; _historizationRoleView = historizationRoleView; _sharesHistorizationScopeWithLocal = sharesHistorizationScopeWithLocal; _telemetryHub = telemetryHub; @@ -567,6 +642,8 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers _scriptedAlarmHostOverride = scriptedAlarmHostOverride; // Default behavior is Steady — PreStart may flip to Stale or replay an orphan apply. + // The ONE Become(Steady) that is not BecomeSteady(): Stash is assigned by the ActorCell after + // the constructor returns, so it is null here — and nothing can have been stashed yet anyway. Become(Steady); } @@ -703,7 +780,7 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers if (_dbFactory is null) { _log.Info("DriverHost {Node}: no ConfigDb on the Direct bootstrap path; entering Steady (no revision)", _localNode); - Become(Steady); + BecomeSteady(); return; } @@ -722,7 +799,7 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers if (latest is null) { _log.Info("DriverHost {Node}: no prior deployments; entering Steady (no revision)", _localNode); - Become(Steady); + BecomeSteady(); return; } @@ -738,7 +815,7 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers case NodeDeploymentStatus.Applied: _currentRevision = revision; _log.Info("DriverHost {Node}: recovered Applied state at rev {Rev}", _localNode, revision); - Become(Steady); + BecomeSteady(); // The revision is recovered but the in-memory driver children + OPC UA address // space were lost on restart. Re-spawn + re-materialise + re-subscribe from the // applied deployment so a restarted/rebuilt node restores its served state instead @@ -749,13 +826,13 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers _log.Warning("DriverHost {Node}: found orphan Applying row for deployment {Id}; replaying", _localNode, latest.DeploymentId); if (revision is not null) ApplyAndAck(new DeploymentId(latest.DeploymentId), revision.Value, CorrelationId.NewId()); - else Become(Steady); + else BecomeSteady(); break; case NodeDeploymentStatus.Failed: default: _log.Info("DriverHost {Node}: prior deployment {Id} failed; entering Steady at last known rev", _localNode, latest.DeploymentId); - Become(Steady); + BecomeSteady(); break; } } @@ -796,7 +873,7 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers "DriverHost {Node}: FetchAndCache boot — the LocalDb cache is empty; entering Steady " + "(no revision). The first dispatch will fetch this node's configuration from central.", _localNode); - Become(Steady); + BecomeSteady(); return; } @@ -804,7 +881,7 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers var revision = RevisionHash.Parse(cached.RevisionHash); _currentRevision = revision; - Become(Steady); + BecomeSteady(); // Restore drivers + address space + subscriptions from the cached bytes — no SQL read, no // re-ack (the deployment is already Applied). Same in-hand-bytes apply as the Direct-mode // cache boot, but this is the steady-state config source, not an outage fallback. @@ -821,7 +898,7 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers "DriverHost {Node}: FetchAndCache boot — failed to read the LocalDb pointer; entering " + "Steady (no revision). The next dispatch will fetch.", _localNode); - Become(Steady); + BecomeSteady(); } } @@ -876,7 +953,7 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers // Stale would have started does not run here, so recovery rides on the next dispatch — // matching how a normally-booted node behaves. _currentRevision = revision; - Become(Steady); + BecomeSteady(); ApplyCachedArtifact(deploymentId, cached.Artifact); @@ -923,9 +1000,47 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers PushDesiredSubscriptionsFromArtifact(deploymentId, blob); } + /// + /// Enters and releases anything the state stashed. + /// + /// + /// + /// A deferred dispatch must re-enter the mailbox exactly once and in order — + /// prepends the stash to the front of the mailbox in stash + /// order, which is both. Forgetting it on any one path silently strands that dispatch + /// forever, and the coordinator only learns about it at its apply deadline. + /// + /// + /// Every Become(Steady) goes through here — including the bootstrap and Stale-recovery + /// paths, where the stash is provably empty (only ever stashes, and + /// those paths run before or outside it). Unstashing an empty stash is a no-op, and routing + /// all of them through one helper is what makes "did we cover every transition?" answerable + /// by grep. The constructor is the one exception: is assigned by the + /// ActorCell only after construction returns. + /// + /// + /// It is also the single place the state's two side-tables are + /// released: (which must be cleared in lockstep with the + /// unstash, or the re-delivered dispatch would be treated as an already-deferred duplicate + /// and dropped) and the watchdog. + /// + /// + private void BecomeSteady() + { + // Cancelled BEFORE the unstash so a re-stash by the very next apply starts a fresh window. + Timers.Cancel(ApplyWedgeTimerKey); + _deferredDispatches.Clear(); + Become(Steady); + Stash.UnstashAll(); + } + private void Steady() { Receive(HandleDispatchFromSteady); + Receive(HandleArtifactCached); + Receive(HandleArtifactCacheFailed); + // A wedge tick already in the mailbox when the apply finished — dropped at Debug, see the handler. + Receive(HandleApplyWedged); Receive(HandleGetDiagnostics); Receive(ForwardBatchToMux); Receive(ForwardNativeAlarm); @@ -953,13 +1068,33 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers _localNode, msg.DeploymentId); return; } - _log.Info("DriverHost {Node}: dispatch for {Id} received while still applying {Cur}; deferring", + + // At most ONE stashed copy per distinct deployment (see _deferredDispatches): the redundant + // copies would all short-circuit as duplicates on the way out anyway, so a re-dispatching + // coordinator would otherwise grow the stash with retries rather than with work. + if (!_deferredDispatches.Add(msg.DeploymentId)) + { + _log.Debug("DriverHost {Node}: dispatch for {Id} is already deferred behind {Cur}; dropping the duplicate", + _localNode, msg.DeploymentId, _applyingDeploymentId); + return; + } + + // Stashed, NOT re-forwarded to Self: a forward re-enqueues on this actor's own mailbox and + // is dequeued again immediately, spinning for the whole apply. Every transition back to + // Steady unstashes (see BecomeSteady), so this is logged once per deferred dispatch rather + // than once per spin iteration. + _log.Debug("DriverHost {Node}: dispatch for {Id} received while still applying {Cur}; deferring", _localNode, msg.DeploymentId, _applyingDeploymentId); - Self.Forward(msg); // re-deliver after we transition back + Stash.Stash(); }); // The FetchAndCache fetch pipes its result back here (BeginFetchAndCacheApply ran in Steady, // then Become(Applying)); completing the apply transitions back to Steady. Receive(HandleFetchedForApply); + // The cache write is piped back too, and it outlives the apply that started it by design — so + // its completion can land in EITHER state and both must handle it or it dead-letters. + Receive(HandleArtifactCached); + Receive(HandleArtifactCacheFailed); + Receive(HandleApplyWedged); Receive(HandleGetDiagnostics); Receive(ForwardBatchToMux); Receive(ForwardNativeAlarm); @@ -1521,6 +1656,13 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers { _log.Warning("DriverHost {Node}: ignoring DispatchDeployment while Stale (DB unreachable)", _localNode); }); + // A cache store started before this node fell Stale still pipes back here (see + // CacheAppliedArtifact: Bootstrap's orphan-replay ApplyAndAck and RestoreApplied both start + // stores, and either can be followed by the ConfigDb failure that lands us in Stale). Without + // these the pipe-back dead-letters and a failed cache write goes unreported in exactly the + // situation — central unreachable — where the local cache is the thing keeping the node alive. + Receive(HandleArtifactCached); + Receive(HandleArtifactCacheFailed); Receive(HandleGetDiagnostics); Receive(_ => TryRecoverFromStale()); Receive(HandleRestartDriver); @@ -1597,10 +1739,19 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers /// inside a receive. The apply itself completes synchronously on the actor thread in /// , mirroring . /// + /// + /// This is the ONLY path that arms the watchdog, deliberately. + /// enters and leaves inside a single mailbox + /// turn, so it cannot be "stuck in Applying" in any sense a watchdog could report: if it hung, + /// it would be hanging the actor thread, and a watchdog delivered as a mailbox message could not + /// be dequeued to say so. The fetch is the only unbounded wait during which the host is still + /// alive and dequeuing — which is exactly what makes a silent wedge possible there. + /// private void BeginFetchAndCacheApply(DeploymentId deploymentId, RevisionHash revision, CorrelationId correlation) { _applyingDeploymentId = deploymentId; Become(Applying); + Timers.StartSingleTimer(ApplyWedgeTimerKey, new ApplyWedged(deploymentId), _applyWedgeTimeout); // Persist Applying row (idempotent on PK) — same crash-boundary marker as ApplyAndAck. UpsertNodeDeploymentState(deploymentId, NodeDeploymentStatus.Applying, failureReason: null); @@ -1614,7 +1765,7 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers SendAck(deploymentId, ApplyAckOutcome.Failed, reason, correlation); _log.Error("DriverHost {Node}: cannot apply {Id} — {Reason}", _localNode, deploymentId, reason); _applyingDeploymentId = null; - Become(Steady); + BecomeSteady(); return; } @@ -1651,13 +1802,17 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers span?.SetTag("otopcua.correlation_id", msg.Correlation.ToString()); var sw = Stopwatch.StartNew(); + // Owned by this handler, disposed in the finally: ONE JsonDocument for the whole apply. Declared + // out here (rather than a `using` inside the try) so the catch/finally still see it. + ParsedDeploymentArtifact? parsed = null; try { // #485: null bytes = "no answer" (every endpoint unreachable / NotFound / SHA mismatch). // Fail the apply and stay on the current revision so a re-dispatch actually retries; keep // serving the last-known-good address space, drivers and subscriptions throughout. - var appliedBlob = ReconcileDriversFromBlob(msg.DeploymentId, msg.Bytes); - if (appliedBlob is null) + var (appliedBlob, parsedArtifact) = ReconcileDriversFromParsed(msg.DeploymentId, msg.Bytes); + parsed = parsedArtifact; + if (appliedBlob is null || parsed is null) { const string reason = "the deployment artifact could not be fetched from central (all endpoints unreachable, NotFound, or SHA-256 mismatch); nothing was applied"; @@ -1676,11 +1831,14 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers SendAck(msg.DeploymentId, ApplyAckOutcome.Applied, failureReason: null, msg.Correlation); // Rebuild the address space + push subscriptions FROM the fetched bytes so OpcUaPublishActor // never reads central SQL (the whole point of FetchAndCache). + // The message carries the raw BYTES, not `parsed`: the publish actor is a different actor with + // its own mailbox, and ParsedDeploymentArtifact is a disposable, non-thread-safe object owned + // by this handler. It opens its own (single) parse on the other side of the mailbox. _opcUaPublishActor?.Tell(new ZB.MOM.WW.OtOpcUa.Runtime.OpcUa.OpcUaPublishActor.RebuildAddressSpace( msg.Correlation, msg.DeploymentId, appliedBlob)); - PushDesiredSubscriptionsFromArtifact(msg.DeploymentId, appliedBlob); + PushDesiredSubscriptionsFromParsed(msg.DeploymentId, parsed); // Cache the fetched bytes as this node's last-known-good (idempotent store; skips empty). - CacheAppliedArtifact(msg.DeploymentId, msg.Revision, appliedBlob); + CacheAppliedArtifact(msg.DeploymentId, msg.Revision, appliedBlob, parsed); _isRunningFromCache = false; OtOpcUaTelemetry.DeploymentApplied.Add(1, new KeyValuePair("outcome", "ack")); _log.Info("DriverHost {Node}: applied fetched deployment {Id} (rev {Rev}, children={Count})", @@ -1696,12 +1854,46 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers } finally { + parsed?.Dispose(); OtOpcUaTelemetry.DeploymentApplyDurationSec.Record(sw.Elapsed.TotalSeconds); _applyingDeploymentId = null; - Become(Steady); + BecomeSteady(); } } + /// + /// Self-timer telling this host it has been in for + /// without finishing. + /// + private sealed record ApplyWedged(DeploymentId DeploymentId); + + /// + /// Reports a host that is still in long after it should have finished. + /// + /// + /// Purely a signal — nothing is cancelled and no ACK is sent, because the apply may yet land and + /// the coordinator's apply deadline already owns "this deployment is dead". Registered in + /// AND : cancels the timer, + /// but a fire already in the mailbox when the apply completes still has to land somewhere, and + /// dead-lettering it would be a second, more confusing signal. Such a late fire finds + /// null (or naming a different deployment) and drops at Debug. + /// + private void HandleApplyWedged(ApplyWedged msg) + { + if (_applyingDeploymentId is null || msg.DeploymentId != _applyingDeploymentId.Value) + { + _log.Debug("DriverHost {Node}: late apply-wedge tick for {Id} (in-flight={Cur}); dropping", + _localNode, msg.DeploymentId, _applyingDeploymentId); + return; + } + + _log.Warning( + "DriverHost {Node}: deployment {Id} appears WEDGED — still applying after {Timeout} " + + "(the artifact fetch has neither answered nor failed); {Deferred} further dispatch(es) are " + + "deferred behind it. The node keeps serving revision {Rev}.", + _localNode, msg.DeploymentId, _applyWedgeTimeout, _deferredDispatches.Count, _currentRevision); + } + /// Pipe-back carrier for a FetchAndCache fetch result (null bytes = no answer). private sealed record FetchedForApply( DeploymentId DeploymentId, RevisionHash Revision, CorrelationId Correlation, byte[]? Bytes); @@ -1720,9 +1912,13 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers // Persist Applying row (idempotent on PK). UpsertNodeDeploymentState(deploymentId, NodeDeploymentStatus.Applying, failureReason: null); + // Owned by this handler, disposed in the finally: ONE JsonDocument for the whole apply. Declared + // out here (rather than a `using` inside the try) so the catch/finally still see it. + ParsedDeploymentArtifact? parsed = null; try { - var appliedBlob = ReconcileDrivers(deploymentId); + var (appliedBlob, parsedArtifact) = ReconcileDriversForApply(deploymentId); + parsed = parsedArtifact; // Issue #486 — a null blob means the artifact could not be read (unreachable ConfigDb, or the // empty bytes a missing row yields), so NOTHING was applied. Reporting Applied here used to @@ -1732,7 +1928,7 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers // what makes the retry land instead of being waved through. The node keeps serving its // last-known-good address space, drivers and subscriptions throughout (issue #485) — this is // about telling the truth, not about tearing anything down. - if (appliedBlob is null) + if (appliedBlob is null || parsed is null) { const string reason = "the deployment artifact could not be read (ConfigDb unreachable, or the row is missing/empty); nothing was applied"; @@ -1752,11 +1948,25 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers // Trigger the OPC UA address-space rebuild so the local SDK reflects the new // composition. The publish actor handles the load-compose-diff-apply pipeline; we // just forward the same correlation id so the audit trail joins up. - _opcUaPublishActor?.Tell(new ZB.MOM.WW.OtOpcUa.Runtime.OpcUa.OpcUaPublishActor.RebuildAddressSpace(correlation, deploymentId)); + // + // The already-read bytes ride along. ReconcileDrivers just read this exact blob out of central + // SQL and handed it back (it is non-null here — the null case returned above), so a bytes-less + // RebuildAddressSpace would make the publish actor re-read the same immutable row, and the + // SubscribeBulk pass would read it a THIRD time: three round-trips per apply for one blob that + // cannot have changed between them. Threading it is also the shape the FetchAndCache apply has + // always had (HandleFetchedForApply), for the harder reason that it has no ConfigDb to re-read. + // Consequence: OpcUaPublishActor.LoadArtifact / LoadLatestArtifact are no longer on any apply + // path — they remain only for a legacy/dev caller that Tells RebuildAddressSpace without bytes. + // + // What rides along is the BYTES, deliberately — not this apply's `parsed`. The publish actor is + // a different actor with its own mailbox, and ParsedDeploymentArtifact is a disposable, + // non-thread-safe object owned by this handler (disposed in the finally below, likely before the + // other actor even dequeues). It opens its own single parse on the far side. + _opcUaPublishActor?.Tell(new ZB.MOM.WW.OtOpcUa.Runtime.OpcUa.OpcUaPublishActor.RebuildAddressSpace(correlation, deploymentId, appliedBlob)); // SubscribeBulk pass: hand each driver its desired tag references so live values flow into // the just-rebuilt address space instead of staying BadWaitingForInitialData. - PushDesiredSubscriptions(deploymentId); - CacheAppliedArtifact(deploymentId, revision, appliedBlob); + PushDesiredSubscriptionsFromParsed(deploymentId, parsed); + CacheAppliedArtifact(deploymentId, revision, appliedBlob, parsed); // Reaching here means the artifact came from central, so the node is no longer serving a // cache-sourced configuration. Note this only clears on a real apply: a dispatch of the // revision already booted from cache short-circuits in HandleDispatchFromSteady without @@ -1776,9 +1986,10 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers } finally { + parsed?.Dispose(); OtOpcUaTelemetry.DeploymentApplyDurationSec.Record(sw.Elapsed.TotalSeconds); _applyingDeploymentId = null; - Become(Steady); + BecomeSteady(); } } @@ -1794,18 +2005,36 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers /// loaded at all. /// /// - /// Returning the blob rather than swallowing it is load-bearing for the artifact cache. This - /// method catches its own DB failures, logs a warning and returns WITHOUT rethrowing — so - /// proceeds to its success path, ACKs Applied and logs success - /// having spawned zero drivers. A cache write that trusted "the apply succeeded" would then - /// persist an empty artifact as this node's last-known-good configuration, and the node would - /// later boot from it into an empty address space. The caller distinguishes the two cases by - /// this return value. + /// + /// The bootstrap-restore entry point (). The apply paths use + /// , which keeps its parse for the rest of the apply. + /// + /// + /// Returning the blob rather than swallowing it is load-bearing for the artifact cache. This + /// method catches its own DB failures, logs a warning and returns WITHOUT rethrowing — so the + /// caller proceeds to its success path having spawned zero drivers. A cache write that trusted + /// "the apply succeeded" would then persist an empty artifact as this node's last-known-good + /// configuration, and the node would later boot from it into an empty address space. The caller + /// distinguishes the two cases by this return value. + /// /// private byte[]? ReconcileDrivers(DeploymentId deploymentId) + { + var blob = LoadArtifactBlob(deploymentId); + return blob is null ? null : ReconcileDriversFromBlob(deploymentId, blob); + } + + /// + /// The ConfigDb read half of , shared with the Direct apply path + /// () so both read the artifact exactly once and log the + /// same way when they cannot. + /// + /// The artifact bytes (possibly empty, when the row is missing), or + /// when there is no ConfigDb to read or the read failed. + private byte[]? LoadArtifactBlob(DeploymentId deploymentId) { // Phase 4 belt-and-suspenders: this ConfigDb read is Direct-only (the FetchAndCache apply uses - // ReconcileDriversFromBlob with bytes in hand). A null factory can't read, so return null — the + // ReconcileDriversFromParsed with bytes in hand). A null factory can't read, so return null — the // caller treats that as "artifact could not be read" (#486) and keeps last-known-good. if (_dbFactory is null) { @@ -1813,11 +2042,10 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers return null; } - byte[] blob; try { using var db = _dbFactory.CreateDbContext(); - blob = db.Deployments.AsNoTracking() + return db.Deployments.AsNoTracking() .Where(d => d.DeploymentId == deploymentId.Value) .Select(d => d.ArtifactBlob) .FirstOrDefault() ?? Array.Empty(); @@ -1828,35 +2056,104 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers _localNode, deploymentId); return null; } - - return ReconcileDriversFromBlob(deploymentId, blob); } /// - /// The blob-processing half of , split out so the - /// FetchAndCache apply path (which already holds the fetched bytes) and the - /// boot-from-cache path can reconcile without a ConfigDb read. + /// An apply's artifact: the bytes that were reconciled plus the ONE + /// every later consumer of that apply reads. Both are + /// when nothing was applied (#485/#486). + /// + /// The reconciled artifact bytes, or null when there was no usable artifact. + /// The parsed artifact, owned by the apply handler — dispose before it returns. + private readonly record struct ReconciledArtifact(byte[]? Blob, ParsedDeploymentArtifact? Parsed); + + /// + /// The Direct apply path's reconcile: read the artifact from the ConfigDb, parse it ONCE, and + /// reconcile the driver children from that parse. The caller owns (and disposes) the returned + /// and threads it through the rest of the apply, so a + /// Direct apply builds exactly one JsonDocument in this actor. + /// + /// The artifact bytes are not valid JSON. Unlike the + /// lenient blob wrappers (malformed ⇒ no specs ⇒ every running child planned for StopChild) this + /// surfaces, and 's catch turns it into a Failed ACK with the children left + /// running — the honest reading of bytes that cannot be understood. + private ReconciledArtifact ReconcileDriversForApply(DeploymentId deploymentId) + { + var blob = LoadArtifactBlob(deploymentId); + return blob is null ? default : ReconcileDriversFromParsed(deploymentId, blob); + } + + /// + /// The parse-once reconcile shared by both apply paths ( via + /// , and with the + /// fetched bytes already in hand). + /// + /// The reconciled blob + its parsed artifact, or a default (both null) when the blob was + /// empty (#485). + private ReconciledArtifact ReconcileDriversFromParsed(DeploymentId deploymentId, byte[]? blob) + { + blob ??= Array.Empty(); + if (blob.Length == 0) + { + WarnEmptyArtifactSkippingReconcile(deploymentId); + return default; + } + + // Opened here, NOT inside the try below: DeploymentArtifact.Open throws on malformed bytes and + // that throw must reach the apply handler's catch (Failed ACK), not be mistaken for a reconcile + // failure. Ownership transfers to the caller on success; a throw from the reconcile itself + // disposes it here, since the caller never saw it. + var parsed = DeploymentArtifact.Open(blob); + try + { + ApplyDriverSpecs(parsed.ParseDriverInstances(_localNode.Value)); + } + catch + { + parsed.Dispose(); + throw; + } + + return new ReconciledArtifact(blob, parsed); + } + + /// + /// The blob-processing half of , split out so the boot-from-cache + /// and bootstrap-restore paths can reconcile from bytes with the lenient + /// "malformed ⇒ no specs" parse those paths have always had. The apply paths use + /// instead, so they parse once and keep the parse. /// /// The reconciled blob, or when it was empty (#485). private byte[]? ReconcileDriversFromBlob(DeploymentId deploymentId, byte[]? blob) { blob ??= Array.Empty(); - - // Issue #485 — no bytes is no answer, not "a configuration with no drivers". Parsing zero bytes - // yields zero specs, and DriverSpawnPlanner then plans EVERY running child for StopChild: a missing - // deployment row (or a half-written one) silently takes this node's entire field I/O down while the - // apply still ACKs Applied. Nothing legitimate produces a zero-length blob — deleting the last - // driver still deploys a JSON document with empty arrays — so treat it exactly like the load - // failure above and leave the running children alone. if (blob.Length == 0) { - _log.Warning( - "DriverHost {Node}: artifact for {Id} read back empty; skipping reconcile and keeping the {Count} running driver(s)", - _localNode, deploymentId, _children.Count); + WarnEmptyArtifactSkippingReconcile(deploymentId); return null; } - var specs = DeploymentArtifact.ParseDriverInstances(blob, _localNode.Value); + ApplyDriverSpecs(DeploymentArtifact.ParseDriverInstances(blob, _localNode.Value)); + return blob; + } + + /// + /// Issue #485 — no bytes is no answer, not "a configuration with no drivers". Parsing zero bytes + /// yields zero specs, and DriverSpawnPlanner then plans EVERY running child for StopChild: a + /// missing deployment row (or a half-written one) silently takes this node's entire field I/O down + /// while the apply still ACKs Applied. Nothing legitimate produces a zero-length blob — deleting the + /// last driver still deploys a JSON document with empty arrays — so treat it exactly like a load + /// failure and leave the running children alone. + /// + private void WarnEmptyArtifactSkippingReconcile(DeploymentId deploymentId) => + _log.Warning( + "DriverHost {Node}: artifact for {Id} read back empty; skipping reconcile and keeping the {Count} running driver(s)", + _localNode, deploymentId, _children.Count); + + /// Spawn / delta / stop the driver children to match . + /// The driver-instance specs this node should be running. + private void ApplyDriverSpecs(IReadOnlyList specs) + { var snapshots = _children.ToDictionary( kv => kv.Key, kv => new DriverChildSnapshot(kv.Value.DriverType, kv.Value.LastConfigJson, kv.Value.ResilienceConfig), @@ -1874,8 +2171,6 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers // sequenced AFTER ReinitializeAsync rebuilds DependencyRefs — a synchronous re-register HERE would // re-read the STALE ref set (ApplyDelta runs asynchronously on the child), so it is deliberately NOT // done inline. - - return blob; } /// @@ -1897,13 +2192,50 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers /// space during the next outage — strictly worse than having no cache at all. /// /// - /// Runs synchronously on the actor thread. That matches every other DB call on this path - /// (all of which already block), and the alternative — piping the result back as a - /// self-message — would need a handler registered in Steady, Applying AND Stale or it - /// dead-letters after the Become(Steady) in the enclosing finally. + /// The store runs OFF the actor thread. It is hundreds of chunk inserts into SQLite; + /// blocking on it (as this did) parked the host for the duration, so it could not dequeue a + /// driver value burst, a diagnostics request or the next dispatch. The task is piped back as + /// / so completion is observed + /// rather than fire-and-forgotten — the earlier objection to this shape was that the + /// pipe-back needs a handler in every state it can land in, which is why + /// , AND all register one. + /// + /// + /// Stale is included on purpose. This used to claim Stale could not be reached after a + /// store had started, because it is only ever entered from . That is + /// not a property of the Stale entry — starts stores of its own, via + /// the orphan-replay and via , and both + /// run inside the very try block whose catch Becomes Stale. Today no statement between those + /// calls and the catch happens to throw, so the old claim held by accident of ordering rather + /// than by design; a restart can orphan an in-flight pipe-back the same way. Registering the + /// handlers costs two lines and removes the need to re-derive that argument. + /// + /// + /// The apply does not depend on the outcome — it never did, and the async shape keeps + /// it that way. By the time this runs the deployment is recorded Applied and an Applied ACK + /// has gone to the coordinator; a failure logs a Warning and changes nothing else. + /// + /// + /// Idempotence is unchanged. The skip-if-already-cached check lives inside + /// LocalDbDeploymentArtifactCache.StoreAsync, so it moves off-thread with the rest of + /// the store rather than being lost. + /// + /// + /// No boot-path race. The readers — and + /// — run in , before this actor has + /// processed a single message and therefore before any apply could have started a store. + /// The only concurrent writer a read can meet is the pair peer's replication, which the + /// cache already handles. /// /// - private void CacheAppliedArtifact(DeploymentId deploymentId, RevisionHash revision, byte[]? blob) + /// The deployment whose artifact is being cached. + /// The revision hash to store alongside the bytes. + /// The applied artifact BYTES — what actually gets stored. + /// The apply's already-parsed artifact, when the caller has one, so the cluster + /// scope is read off that parse instead of re-parsing the same bytes. Null on the boot/restore paths, + /// which fall back to the one-shot static resolve. + private void CacheAppliedArtifact( + DeploymentId deploymentId, RevisionHash revision, byte[]? blob, ParsedDeploymentArtifact? parsed = null) { if (_deploymentArtifactCache is null) return; @@ -1922,28 +2254,65 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers // The node's ClusterId is carried inside the artifact (ClusterNode rows), not in config // or on IClusterRoleInfo. A single-cluster or unscoped artifact has none, so it shares // one sentinel key — the pair still converges on a single pointer row either way. - var scope = DeploymentArtifact.ResolveClusterScope(blob, _localNode.Value); + // + // Resolved HERE, synchronously, and not one step later: `parsed` is owned by the calling + // apply and DISPOSED in its finally, which runs long before a slow store completes. The + // async continuation below must therefore capture only these scalars — never `parsed`. + var scope = parsed is not null + ? parsed.ResolveClusterScope(_localNode.Value) + : DeploymentArtifact.ResolveClusterScope(blob, _localNode.Value); var clusterId = scope.Mode == ClusterFilterMode.ScopeTo && !string.IsNullOrWhiteSpace(scope.ClusterId) ? scope.ClusterId : SingleClusterCacheKey; + _log.Debug("DriverHost {Node}: caching deployment {Id} (rev {Rev}, cluster {ClusterId}, {Bytes} bytes)", + _localNode, deploymentId, revision, clusterId, blob.Length); + _deploymentArtifactCache .StoreAsync(clusterId, deploymentId.ToString(), revision.Value, blob) - .GetAwaiter() - .GetResult(); - - _log.Debug("DriverHost {Node}: cached deployment {Id} (rev {Rev}, cluster {ClusterId}, {Bytes} bytes)", - _localNode, deploymentId, revision, clusterId, blob.Length); + .PipeTo( + Self, + success: () => new ArtifactCached(deploymentId), + failure: ex => new ArtifactCacheFailed(deploymentId, ex)); } catch (Exception ex) { - _log.Error(ex, - "DriverHost {Node}: failed to cache applied deployment {Id}. The apply itself succeeded " + - "and is unaffected; this node simply has no local fallback for that revision.", - _localNode, deploymentId); + // A cache that throws SYNCHRONOUSLY (before returning a task) never produces a pipe-back, + // so route it to the same handler by hand rather than letting it unwind into the apply's + // catch — which would send a second, contradictory Failed ACK for a live deployment. + Self.Tell(new ArtifactCacheFailed(deploymentId, ex)); } } + /// The node-local artifact cache accepted this deployment's bytes. + /// + /// Piped back from . Carries the id only — deliberately no + /// ParsedDeploymentArtifact and no other apply-owned state, since the apply that started + /// the store has long since disposed its parse by the time this lands. + /// + private sealed record ArtifactCached(DeploymentId DeploymentId); + + /// The node-local artifact cache could not store this deployment's bytes. + /// + /// Carries the exception, not a pre-flattened message string. A LocalDb/SQLite failure is + /// routinely a wrapped inner exception, and ex.Message alone ("An error occurred while + /// …") names neither the SQLite result code nor the frame that produced it — the detail an + /// operator needs to tell a full disk from a locked file from a schema mismatch. Logging it as + /// the log event's Cause also lets a test assert on the exception TYPE rather than on + /// prose. + /// + private sealed record ArtifactCacheFailed(DeploymentId DeploymentId, Exception Cause); + + private void HandleArtifactCached(ArtifactCached msg) => + _log.Info("DriverHost {Node}: cached applied deployment {Id} as this node's local fallback", + _localNode, msg.DeploymentId); + + private void HandleArtifactCacheFailed(ArtifactCacheFailed msg) => + _log.Warning(msg.Cause, + "DriverHost {Node}: deployment {Id} could not be cached locally ({Reason}). The apply itself " + + "succeeded and is unaffected; this node simply has no local fallback for that revision.", + _localNode, msg.DeploymentId, msg.Cause.Message); + /// /// Restore the served state for an already-applied deployment after a process restart. /// recovers from NodeDeploymentState, @@ -1987,10 +2356,18 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers /// the OPC UA sink and resume after reconnects. Drivers with no configured tags get an empty set /// (which clears any stale subscription from a previous deployment). /// + /// + /// The bootstrap-restore entry point only. Neither apply path reaches it any more: both + /// and already hold the apply's single + /// parse and call directly, so this ConfigDb read + /// exists for , which restores from a deployment id recovered out of + /// NodeDeploymentState and may hold no blob of its own. + /// private void PushDesiredSubscriptions(DeploymentId deploymentId) { - // Phase 4 belt-and-suspenders: Direct-only read (the FetchAndCache path calls - // PushDesiredSubscriptionsFromArtifact with bytes in hand). No ConfigDb ⇒ nothing to load. + // Phase 4 belt-and-suspenders: a ConfigDb read, so it can only be the restore path on a node that + // has one (every apply path calls PushDesiredSubscriptionsFromParsed with its parse in hand). + // No ConfigDb ⇒ nothing to load. if (_dbFactory is null) { _log.Warning("DriverHost {Node}: no ConfigDb to load artifact for SubscribeBulk ({Id}); skipping", _localNode, deploymentId); @@ -2048,6 +2425,44 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers return; } + ApplyDesiredSubscriptions(composition); + } + + /// + /// The apply paths' SubscribeBulk entry point: reads the composition off the apply's SINGLE parse + /// instead of re-parsing the same bytes a second time. + /// + /// + /// No empty-artifact guard here, unlike : a + /// only exists for a non-empty blob (the apply's reconcile + /// already applied that guard and never opened one otherwise). + /// + /// The deployment being applied — diagnostics only. + /// The apply's parsed artifact. + private void PushDesiredSubscriptionsFromParsed(DeploymentId deploymentId, ParsedDeploymentArtifact parsed) + { + AddressSpaceComposition composition; + try + { + composition = parsed.ParseComposition(_localNode.Value); + } + catch (Exception ex) + { + _log.Warning(ex, "DriverHost {Node}: failed to parse composition for SubscribeBulk ({Id})", _localNode, deploymentId); + return; + } + + ApplyDesiredSubscriptions(composition); + } + + /// + /// Hand each running driver child its desired value + alarm subscription set for + /// , rebuild the live-value/write routing maps, and forward the + /// Equipment VirtualTag + ScriptedAlarm plans to their hosts. + /// + /// The composition this node should serve. + private void ApplyDesiredSubscriptions(AddressSpaceComposition composition) + { // v3 Batch 4: the driver subscribes + publishes by RawPath (the wire-ref == RawPath == RawTagPlan.NodeId // per the v3 identity contract). Value-subscription set = the non-alarm raw tags' RawPaths; alarm-bearing // raw tags are Part 9 conditions, fed via the native alarm event stream (ForwardNativeAlarm), so they are @@ -2434,7 +2849,7 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers if (_fetchAndCacheMode || _dbFactory is null) { Timers.Cancel("retry-db"); - Become(Steady); + BecomeSteady(); return; } @@ -2457,7 +2872,7 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers UpsertNodeDeploymentState(new DeploymentId(latestSealed.DeploymentId), NodeDeploymentStatus.Applied, failureReason: null); } - Become(Steady); + BecomeSteady(); } catch (Exception ex) { 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 890a0282..f0c9a193 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/OpcUa/OpcUaPublishActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/OpcUa/OpcUaPublishActor.cs @@ -462,18 +462,30 @@ public sealed class OpcUaPublishActor : ReceiveActor, IWithTimers return; } + // Parse the artifact ONCE for this rebuild. Every consumer below (data-plane authority, then the + // composition) reads this one JsonDocument instead of rebuilding it per question — the rebuild + // used to pay a full parse-and-throw-away per static call. Disposed when the handler returns. + // + // Open() throws on bytes that are not valid JSON, where the static wrappers swallowed to an + // EMPTY result — and an empty composition is not a benign no-op here: the planner diffs it as a + // PureRemove and the applier tears the served address space down. So the throw is the honest + // outcome, and it lands in this method's catch below: logged at Error, _lastApplied untouched, + // last-known-good still served. Same "no answer ≠ empty configuration" rule as the #485 guard + // immediately above, applied to bytes that arrived unreadable rather than absent. + using var parsed = DeploymentArtifact.Open(artifact); + // #520 stage 4 — install this artifact's API keys + ACL grants BEFORE the address-space work // below, and deliberately ahead of the `plan.IsEmpty` early-return: an artifact that revokes a // key or edits a grant without touching a single node produces an EMPTY address-space plan, so // feeding this after that return would leave a revoked key authenticating until some unrelated // structural change happened to redeploy. Authorization and topology change on their own // schedules. - FeedDataPlaneAuthority(artifact); + FeedDataPlaneAuthority(parsed, artifact); var composition = _localNode is { } ln - ? DeploymentArtifact.ParseComposition(artifact, ln.Value, + ? parsed.ParseComposition(ln.Value, inconsistency => _log.Warning("OpcUaPublish {Node}: cross-cluster binding — {Message}", ln, inconsistency)) - : DeploymentArtifact.ParseComposition(artifact); + : parsed.ParseComposition(); var plan = AddressSpacePlanner.Compute(_lastApplied, composition); if (plan.IsEmpty) @@ -484,38 +496,52 @@ public sealed class OpcUaPublishActor : ReceiveActor, IWithTimers } var outcome = _applier.Apply(plan); - _lastApplied = composition; - _hasAppliedComposition = true; // Sum swallowed per-node materialise failures across every pass together with Apply's own // removal-pass tally (archreview 01/S-1). A degraded apply used to vanish into per-node // Warnings + an optimistic Info line; now it surfaces at Error + a dedicated meter. var failedNodes = outcome.FailedNodes; + // The UNS Area/Line/Equipment folder hierarchy is materialised in FULL on every apply, rebuild or + // not: it is O(areas + lines + equipment) — trivial next to the tag counts the delta passes below + // exist to bound — and every node the delta passes add needs its ancestor folders to already + // exist, so narrowing it would mean re-deriving each add's ancestor closure (strictly more work). failedNodes += _applier.MaterialiseHierarchy(composition); - // v3 Batch 4 — the Raw device subtree (ns=Raw): containers (Folder→Driver→Device→TagGroup) then - // tag Variables keyed by RawPath. Materialised BEFORE the UNS references so each UNS Organizes→Raw - // edge finds its raw target. The driver binds live values to these raw NodeIds. - failedNodes += _applier.MaterialiseRawSubtree(composition); - // v3 Batch 4 — the UNS reference Variables (ns=UNS): each projects a raw tag under its equipment - // folder (created by MaterialiseHierarchy) with an Organizes→Raw edge; values fan out from the raw - // node. Runs AFTER both the equipment folders AND the raw subtree exist. - failedNodes += _applier.MaterialiseUnsReferences(composition); - // T14 — scripted alarms get their own pass right after the hierarchy so the equipment - // folders they parent under already exist. Materialises real Part 9 AlarmConditionState - // nodes (keyed by ScriptedAlarmId so AlarmStateUpdate writes target them); disabled - // alarms are skipped. - failedNodes += _applier.MaterialiseScriptedAlarms(composition); - // Equipment-namespace tags get their own pass: ensures each signal's Variable (and any - // FolderPath sub-folder) exists under its already-materialised equipment folder so - // clients can browse them. Live values are pushed by DriverHostActor.ForwardBatchToMux after - // each subscription cycle; variables show BadWaitingForInitialData only until the first - // publish interval fires. - failedNodes += _applier.MaterialiseEquipmentTags(composition); - // Equipment-namespace VirtualTags get their own pass right after the equipment tags: - // ensures each computed signal's Variable (and any FolderPath sub-folder) exists under its - // equipment folder with a folder-scoped NodeId. VirtualTagHostActor.OnResult pushes live - // values once the first dependency update arrives; until then variables show BadWaitingForInitialData. - failedNodes += _applier.MaterialiseEquipmentVirtualTags(composition); + failedNodes += MaterialiseNodes(plan, composition, fullComposition: outcome.RebuildCalled); + + // 2026-08-15 surgical-deploys review C1 — record the composition as applied ONLY when the apply was + // clean, or when a rebuild ran. Two reasons this sits HERE (after the materialise passes) rather + // than immediately after Apply: + // + // • A pass that THROWS outright leaves _lastApplied at the previous composition (the outer catch + // below takes over), so the next deploy re-diffs against what is actually materialised. + // • A DEGRADED non-rebuild apply (failedNodes > 0, no rebuild) must not advance either. Before the + // delta materialisation the full passes re-ensured every node on every apply, which silently + // repaired any prior swallowed sink fault — an accidental self-healing mechanism. The delta path + // removed it: a node whose EnsureVariable failed is inside _lastApplied, never appears in a diff + // set again, and EnsureVariable early-returns on the ones that DID land — so the failed node is + // lost until some unrelated change happens to force a rebuild. Holding the composition back is + // the replacement: the next deploy re-diffs the same composition and re-ensures what failed. + // + // What the re-diff costs, stated honestly: it also re-emits the previous apply's REMOVES. A + // re-remove of an already-removed node returns false from the surgical sink, which trips the + // applier's one-way ratchet into a full rebuild — the correct resync for an apply that has already + // been logged at Error as degraded, and a bounded one (it happens once, then the address space is + // known-good again). + // + // A rebuild DOES advance even with failures: it already re-materialised the whole composition, and + // the ratchet is the resync path for anything it could not place. Holding it back would make every + // subsequent deploy re-diff the entire address space as added. + if (outcome.RebuildCalled || failedNodes == 0) + { + _lastApplied = composition; + _hasAppliedComposition = true; + } + else + { + _log.Error( + "OpcUaPublish: degraded non-rebuild apply (correlation={Correlation}, failedNodes={FailedNodes}) — NOT recording this composition as applied, so the next deploy re-diffs it and repairs the missing node(s)", + msg.Correlation, failedNodes); + } // 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 @@ -525,6 +551,16 @@ public sealed class OpcUaPublishActor : ReceiveActor, IWithTimers // 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. + // + // KNOWN GAP, and NOT a neutral pre-existing one — the delta work made it matter. Before this + // branch, a plan carrying raw adds went through a full RebuildAddressSpace, so every client had to + // re-browse anyway and the missing announcement changed nothing observable. Now those adds are + // materialised surgically while existing subscriptions stay alive, so an unannounced add is + // genuinely INVISIBLE until a client happens to browse again. + // Added UNS reference variables ARE announced (their equipment folder is the parent, which is the + // realm AnnounceAddedNodes hardcodes). Added RAW containers/tags are still not: the announce is + // Uns-only, and a raw parent needs a realm-aware RaiseNodesAddedModelChange call. That is a real + // follow-up, not an accepted limitation. if (!outcome.RebuildCalled) _applier.AnnounceAddedNodes(plan); OtOpcUaTelemetry.OpcUaSinkWrite.Add(1, new KeyValuePair("kind", "rebuild")); @@ -549,6 +585,152 @@ public sealed class OpcUaPublishActor : ReceiveActor, IWithTimers } } + /// + /// Run the five node-materialising applier passes — Raw subtree, UNS references, scripted alarms, + /// equipment tags, equipment VirtualTags — either over the FULL composition (after a rebuild) or over + /// just this plan's delta (surgical-deploys Task 6). MaterialiseHierarchy is NOT one of them: + /// the caller always runs it in full, because parents must exist before any add. + /// + /// + /// Why the delta path exists. The passes are idempotent, but idempotent is not free. + /// EnsureFolder / EnsureVariable early-return on a ConcurrentDictionary hit + /// BEFORE taking the node manager's lock, so an already-standing node costs a dictionary probe rather + /// than a lock acquisition — but MaterialiseAlarmCondition DOES take the lock every time, and + /// the probes are still O(address space) per apply plus the composition walk that produces them. + /// Running the passes over the whole composition made the cost of ANY apply proportional to the size + /// of the served address space rather than to the size of the change: adding one tag to a 50k-node + /// server was ~50k sink calls on the actor's pinned publish thread, with every client's publish + /// blocked behind it. The delta path makes it O(change). + /// What the full pass was silently doing, and its replacement. Re-ensuring every node on + /// every apply also REPAIRED any node a previous apply had swallowed a sink fault on — accidental + /// self-healing nobody designed. The caller's degraded-apply no-advance rule (a non-rebuild apply + /// reporting failedNodes > 0 does not record its composition in _lastApplied, so the + /// next deploy re-diffs and re-ensures it) is the deliberate replacement for it. + /// Why is the branch key rather than + /// the classifier's kind: a surgical apply that hits a false/throw from the surgical sink falls + /// back to a full RebuildAddressSpace() INSIDE Apply, which wipes every node. That plan + /// is still classified non-Rebuild, so branching on the kind would re-materialise only the delta onto + /// a wiped address space. The outcome flag reports what actually happened. + /// Why changed UNS references are in the subset. Apply repairs a + /// UnsReferenceDelta by REMOVING the previous node and leaving the recreate to this pass, so + /// the subset must carry the current side of every changed reference as well as the added ones + /// — otherwise a re-pointed, renamed, or backing-edited UNS node is removed and never comes back. + /// Accepted narrowing. The plan diffs the THIN ScriptedAlarmPlans projection, so + /// the added-alarm subset is the rich EquipmentScriptedAlarms rows whose id the plan reports as + /// added. An alarm already present in both compositions whose predicate SCRIPT row only now arrived + /// (making it materialisable for the first time) is not in that set — but neither is it in any diff + /// set at all, so on its own it already produced an empty plan and materialised nothing. The delta + /// path extends that pre-existing blindness to the case where it rides along with another change. + /// + /// The plan just applied — the source of the delta subsets. + /// The full composition, always passed as the passes' lookup context. + /// When true, materialise the whole composition (post-rebuild). + /// The summed count of swallowed per-node sink failures across the passes that ran. + private int MaterialiseNodes(AddressSpacePlan plan, AddressSpaceComposition composition, bool fullComposition) + { + var applier = _applier!; + var failedNodes = 0; + + // v3 Batch 4 — the Raw device subtree (ns=Raw): containers (Folder→Driver→Device→TagGroup) then + // tag Variables keyed by RawPath. Materialised BEFORE the UNS references so each UNS Organizes→Raw + // edge finds its raw target. The driver binds live values to these raw NodeIds. + if (fullComposition) + { + failedNodes += applier.MaterialiseRawSubtree(composition); + } + else if (plan.AddedRawContainers.Count > 0 || plan.AddedRawTags.Count > 0) + { + failedNodes += applier.MaterialiseRawSubtree(composition, plan.AddedRawContainers, plan.AddedRawTags); + } + + // v3 Batch 4 — the UNS reference Variables (ns=UNS): each projects a raw tag under its equipment + // folder (created by MaterialiseHierarchy) with an Organizes→Raw edge; values fan out from the raw + // node. Runs AFTER both the equipment folders AND the raw subtree exist. + if (fullComposition) + { + failedNodes += applier.MaterialiseUnsReferences(composition); + } + else + { + var unsReferences = DeltaUnsReferences(plan); + if (unsReferences.Count > 0) failedNodes += applier.MaterialiseUnsReferences(composition, unsReferences); + } + + // T14 — scripted alarms get their own pass right after the hierarchy so the equipment + // folders they parent under already exist. Materialises real Part 9 AlarmConditionState + // nodes (keyed by ScriptedAlarmId so AlarmStateUpdate writes target them); disabled + // alarms are skipped. + if (fullComposition) + { + failedNodes += applier.MaterialiseScriptedAlarms(composition); + } + else + { + var addedAlarms = DeltaScriptedAlarms(plan, composition); + if (addedAlarms.Count > 0) failedNodes += applier.MaterialiseScriptedAlarms(composition, addedAlarms); + } + + // Equipment-namespace tags get their own pass: ensures each signal's Variable (and any + // FolderPath sub-folder) exists under its already-materialised equipment folder so + // clients can browse them. Live values are pushed by DriverHostActor.ForwardBatchToMux after + // each subscription cycle; variables show BadWaitingForInitialData only until the first + // publish interval fires. + if (fullComposition) + { + failedNodes += applier.MaterialiseEquipmentTags(composition); + } + else if (plan.AddedEquipmentTags.Count > 0) + { + failedNodes += applier.MaterialiseEquipmentTags(composition, plan.AddedEquipmentTags); + } + + // Equipment-namespace VirtualTags get their own pass right after the equipment tags: + // ensures each computed signal's Variable (and any FolderPath sub-folder) exists under its + // equipment folder with a folder-scoped NodeId. VirtualTagHostActor.OnResult pushes live + // values once the first dependency update arrives; until then variables show BadWaitingForInitialData. + if (fullComposition) + { + failedNodes += applier.MaterialiseEquipmentVirtualTags(composition); + } + else if (plan.AddedEquipmentVirtualTags.Count > 0) + { + failedNodes += applier.MaterialiseEquipmentVirtualTags(composition, plan.AddedEquipmentVirtualTags); + } + + return failedNodes; + } + + /// The UNS reference variables a non-rebuild apply must materialise: the plan's ADDED references + /// plus the CURRENT side of every CHANGED one (whose previous node the applier already removed in place). + /// + /// The plan just applied. + /// The references to materialise (possibly empty). + private static IReadOnlyList DeltaUnsReferences(AddressSpacePlan plan) + { + if (plan.ChangedUnsReferenceVariables.Count == 0) return plan.AddedUnsReferenceVariables; + var list = new List( + plan.AddedUnsReferenceVariables.Count + plan.ChangedUnsReferenceVariables.Count); + list.AddRange(plan.AddedUnsReferenceVariables); + foreach (var d in plan.ChangedUnsReferenceVariables) list.Add(d.Current); + return list; + } + + /// The scripted-alarm host plans a non-rebuild apply must materialise: the rich + /// EquipmentScriptedAlarms rows whose ScriptedAlarmId the plan reports as ADDED (the plan + /// diffs only the thin projection — see the narrowing note on ). + /// The plan just applied. + /// The full composition carrying the rich alarm plans. + /// The alarms to materialise (possibly empty). + private static IReadOnlyList DeltaScriptedAlarms( + AddressSpacePlan plan, AddressSpaceComposition composition) + { + if (plan.AddedAlarms.Count == 0 || composition.EquipmentScriptedAlarms.Count == 0) + return Array.Empty(); + + var addedIds = new HashSet(plan.AddedAlarms.Select(a => a.ScriptedAlarmId), StringComparer.Ordinal); + return composition.EquipmentScriptedAlarms.Where(a => addedIds.Contains(a.ScriptedAlarmId)).ToList(); + } + /// /// Decode the artifact's data-plane authorization inputs (API keys + this cluster's ACL grants) /// and install them for OPC UA session activation (#520 stage 4). No-op when nothing consumes @@ -560,17 +742,22 @@ public sealed class OpcUaPublishActor : ReceiveActor, IWithTimers /// the safe direction for availability but means a revocation carried by an artifact this node /// could not decode has not taken effect. It is logged at Error for exactly that reason. /// - private void FeedDataPlaneAuthority(byte[] artifact) + /// The rebuild's single parse of the artifact, read for the keys + grants. + /// The RAW artifact bytes — hashed for the revision id, which must be + /// SHA-256 of the exact blob (see below); never re-parsed here. + private void FeedDataPlaneAuthority(ParsedDeploymentArtifact parsed, byte[] artifactBytes) { if (_dataPlaneAuthority is null) return; try { var nodeId = _localNode?.Value ?? string.Empty; - var projection = DeploymentArtifact.ParseDataPlaneAuthority(artifact, nodeId); + var projection = parsed.ParseDataPlaneAuthority(nodeId); // The revision hash is SHA-256 of the artifact bytes — the same derivation ConfigComposer uses, // which is what lets a node recognise a re-applied revision without the hash riding the wire. - var revision = Convert.ToHexStringLower(System.Security.Cryptography.SHA256.HashData(artifact)); + // Hashed off the RAW bytes for exactly that reason: a re-serialisation of the parsed document + // would be a different byte sequence and would not match the deployment's RevisionHash. + var revision = Convert.ToHexStringLower(System.Security.Cryptography.SHA256.HashData(artifactBytes)); var snapshot = _dataPlaneAuthority.Apply( projection.ClusterId, revision, projection.Keys, projection.Grants, projection.ScopePaths); diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ZB.MOM.WW.OtOpcUa.Runtime.csproj b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ZB.MOM.WW.OtOpcUa.Runtime.csproj index 094b2e67..d45ea40d 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ZB.MOM.WW.OtOpcUa.Runtime.csproj +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ZB.MOM.WW.OtOpcUa.Runtime.csproj @@ -5,6 +5,15 @@ true + + + + + diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/ConfigComposerTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/ConfigComposerTests.cs index ac747ee3..4e431151 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/ConfigComposerTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/ConfigComposerTests.cs @@ -1,3 +1,6 @@ +using System.IO.Compression; +using System.Security.Cryptography; +using System.Text; using Shouldly; using Xunit; using ZB.MOM.WW.OtOpcUa.Configuration.Entities; @@ -86,6 +89,43 @@ public sealed class ConfigComposerTests : ControlPlaneActorTestBase artifact.RevisionHash.ShouldMatch("^[0-9a-f]{64}$"); } + /// + /// Task 8: the emitted blob is gzip AND the revision hash is the SHA-256 of THOSE bytes. The second + /// half is the load-bearing one — a FetchAndCache node verifies the bytes it fetched against + /// the RevisionHash its dispatch carried, so hashing the uncompressed JSON instead would fail + /// every fetch in the fleet while every hash-shape test above stayed green. + /// + [Fact] + public async Task Blob_is_gzip_and_the_revision_hash_is_over_the_compressed_bytes() + { + var f = NewInMemoryDbFactory(); + await using (var db = f.CreateDbContext()) + { + db.ServerClusters.Add(NewCluster("c1")); + await db.SaveChangesAsync(); + } + + var artifact = await ConfigComposer.SnapshotAndFlattenAsync(f.CreateDbContext()); + + artifact.Blob[0].ShouldBe((byte)0x1F); + artifact.Blob[1].ShouldBe((byte)0x8B); + artifact.RevisionHash.ShouldBe(Convert.ToHexStringLower(SHA256.HashData(artifact.Blob))); + // …which is exactly what ConfigComposer.HashOf (the fetcher's derivation) computes over the same bytes. + artifact.RevisionHash.ShouldBe(ConfigComposer.HashOf(artifact.Blob)); + + // Falsifiability control: the hash is NOT over the JSON. Decompress and hash that — it must differ, + // or "hash of the compressed bytes" would be an untested claim on a blob that happened to round-trip. + using var inflated = new MemoryStream(); + await using (var gz = new GZipStream(new MemoryStream(artifact.Blob), CompressionMode.Decompress)) + { + await gz.CopyToAsync(inflated); + } + var json = inflated.ToArray(); + Encoding.UTF8.GetString(json).ShouldStartWith("{"); + Convert.ToHexStringLower(SHA256.HashData(json)).ShouldNotBe(artifact.RevisionHash); + artifact.Blob.Length.ShouldBeLessThan(json.Length); + } + /// /// v3: verifies that serialises a /// 's endpoint into the artifact blob and that diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierRawUnsTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierRawUnsTests.cs index 2f4715a3..c8f2244f 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierRawUnsTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierRawUnsTests.cs @@ -253,7 +253,8 @@ public sealed class AddressSpaceApplierRawUnsTests UnsReferenceVariables = new[] { new UnsReferenceVariable("r1", "EQ-1", "filling/line1/station1/speed", "speed", - "Plant/A/dev/speed", "Float", Writable: true), + "Plant/A/dev/speed", "Float", Writable: true, + BackingIsHistorized: true, BackingHistorianTagname: "WW.Speed"), }, }; @@ -276,11 +277,173 @@ public sealed class AddressSpaceApplierRawUnsTests r.ReferenceType.ShouldBe("Organizes"); } + /// + /// 2026-08-15 review — the inherited historian tagname + array shape come from the + /// RECORD, never from a RawTags back-lookup by RawPath. The + /// fixture makes the two disagree DELIBERATELY (the raw tag says de-historized + scalar; the reference + /// record says historized "WW.Record" + array-of-4) so the assertion can only pass on the record. That + /// matters because the record is what the planner diffed to produce the repair delta: materialising + /// from anything else would recreate the node with values nobody diffed. + /// + [Fact] + public void MaterialiseUnsReferences_inherits_shape_and_historian_from_the_record_not_the_raw_tag() + { + var sink = new RealmRecordingSink(); + var composition = new AddressSpaceComposition( + Array.Empty(), Array.Empty(), Array.Empty()) + { + RawTags = new[] + { + new RawTagPlan("t1", "Plant/A/dev/speed", "Plant/A/dev", "drv", "speed", "Float", + Writable: true, Alarm: null, ReferencingEquipmentPaths: new[] { "filling/line1/station1" }, + IsHistorized: false, HistorianTagname: "WW.RawTag", IsArray: false, ArrayLength: null), + }, + UnsReferenceVariables = new[] + { + new UnsReferenceVariable("r1", "EQ-1", "filling/line1/station1/speed", "speed", + "Plant/A/dev/speed", "Float", Writable: true, + BackingIsHistorized: true, BackingHistorianTagname: "WW.Record", + BackingIsArray: true, BackingArrayLength: 4), + }, + }; + + NewApplier(sink).MaterialiseUnsReferences(composition); + + var v = sink.Variables.ShouldHaveSingleItem(); + v.HistorianTagname.ShouldBe("WW.Record"); // the record wins; the raw tag says not historized at all + v.IsArray.ShouldBeTrue(); + v.ArrayLength.ShouldBe(4u); + v.Writable.ShouldBeFalse(); // Writable && !IsArray ⇒ an array mirror is read-only + } + + /// The historian-resolution semantics carried over verbatim from the old back-lookup: a BLANK + /// BackingHistorianTagname on a historized reference falls back to the BackingRawPath (which + /// IS the backing raw tag's NodeId, the raw side's own default), and a non-historized reference registers + /// no tagname at all even when an override string is present. + [Theory] + [InlineData(true, null, "Plant/A/dev/speed")] + [InlineData(true, " ", "Plant/A/dev/speed")] + [InlineData(true, "WW.Override", "WW.Override")] + [InlineData(false, "WW.Override", null)] + public void MaterialiseUnsReferences_resolves_the_historian_tagname_like_the_raw_side( + bool historized, string? tagname, string? expected) + { + var sink = new RealmRecordingSink(); + var composition = new AddressSpaceComposition( + Array.Empty(), Array.Empty(), Array.Empty()) + { + UnsReferenceVariables = new[] + { + new UnsReferenceVariable("r1", "EQ-1", "filling/line1/station1/speed", "speed", + "Plant/A/dev/speed", "Float", Writable: true, + BackingIsHistorized: historized, BackingHistorianTagname: tagname), + }, + }; + + NewApplier(sink).MaterialiseUnsReferences(composition); + + sink.Variables.ShouldHaveSingleItem().HistorianTagname.ShouldBe(expected); + } + + /// + /// 2026-08-15 surgical-deploys (Task 6) — the subset overload materialises ONLY the tags/containers it + /// is handed, while still resolving native-alarm notifier targets from the FULL composition's equipment + /// map. Both halves matter: narrowing the iteration is the whole point (a one-tag deploy used to re-walk + /// every node on the server), and the map has to stay full because the equipment a raw alarm fans to is + /// not part of any raw-tag delta — building it from the subset would silently drop the fan-out. + /// + [Fact] + public void MaterialiseRawSubtree_with_a_subset_ensures_only_the_subset_but_resolves_notifiers_from_the_full_composition() + { + var sink = new RealmRecordingSink(); + var untouched = new RawTagPlan("t0", "Plant/A/dev/speed", "Plant/A/dev", "drv", "speed", "Float", + Writable: true, Alarm: null, ReferencingEquipmentPaths: Array.Empty()); + var added = new RawTagPlan("t1", "Plant/A/dev/OverTemp", "Plant/A/dev", "drv", "OverTemp", "Boolean", + Writable: false, Alarm: new EquipmentTagAlarmInfo("OffNormalAlarm", 700), + ReferencingEquipmentPaths: new[] { "filling/line1/station1" }); + var composition = new AddressSpaceComposition( + new[] { new UnsAreaProjection("a1", "filling") }, + new[] { new UnsLineProjection("l1", "a1", "line1") }, + new[] { new EquipmentNode("EQ-1", "station1", "l1") }, + Array.Empty(), + Array.Empty()) + { + RawContainers = new[] + { + new RawContainerNode("Plant", null, "Plant", RawNodeKind.Folder), + new RawContainerNode("Plant/A", "Plant", "A", RawNodeKind.Driver), + new RawContainerNode("Plant/A/dev", "Plant/A", "dev", RawNodeKind.Device), + }, + RawTags = new[] { untouched, added }, + }; + + // The delta of a one-tag add: no new containers, exactly one new tag. + var failed = NewApplier(sink).MaterialiseRawSubtree( + composition, + onlyContainers: Array.Empty(), + onlyTags: new[] { added }); + + failed.ShouldBe(0); + sink.Folders.ShouldBeEmpty(); // the three existing containers were NOT re-ensured + sink.Variables.ShouldBeEmpty(); // the untouched value tag was NOT re-ensured + sink.Conditions.ShouldHaveSingleItem().NodeId.ShouldBe("Plant/A/dev/OverTemp"); + // …and the notifier still resolved through the FULL composition's Area/Line/Equipment map. + sink.NotifierWirings.ShouldHaveSingleItem().FolderNodeIds.ShouldBe(new[] { "EQ-1" }); + } + + /// A null subset (the rebuild path) still means "the whole composition" — the overload is additive + /// and every pre-existing call site keeps its behaviour. + [Fact] + public void MaterialiseRawSubtree_with_no_subset_still_materialises_the_whole_composition() + { + var sink = new RealmRecordingSink(); + var composition = new AddressSpaceComposition( + Array.Empty(), Array.Empty(), Array.Empty()) + { + RawContainers = new[] { new RawContainerNode("Plant", null, "Plant", RawNodeKind.Folder) }, + RawTags = new[] + { + new RawTagPlan("t0", "Plant/a", "Plant", "drv", "a", "Float", Writable: true, Alarm: null, + ReferencingEquipmentPaths: Array.Empty()), + new RawTagPlan("t1", "Plant/b", "Plant", "drv", "b", "Float", Writable: true, Alarm: null, + ReferencingEquipmentPaths: Array.Empty()), + }, + }; + + NewApplier(sink).MaterialiseRawSubtree(composition); + + sink.Folders.Select(f => f.NodeId).ShouldBe(new[] { "Plant" }); + sink.Variables.Select(v => v.NodeId).ShouldBe(new[] { "Plant/a", "Plant/b" }); + } + + /// 2026-08-15 surgical-deploys (Task 6) — the UNS-reference subset overload materialises only the + /// references it is handed (with their Organizes edges), leaving every other reference in the composition + /// untouched. + [Fact] + public void MaterialiseUnsReferences_with_a_subset_ensures_only_the_subset() + { + var sink = new RealmRecordingSink(); + var untouched = new UnsReferenceVariable("r0", "EQ-1", "filling/line1/station1/speed", "speed", + "Plant/A/dev/speed", "Float", Writable: true); + var added = new UnsReferenceVariable("r1", "EQ-1", "filling/line1/station1/temp", "temp", + "Plant/A/dev/temp", "Float", Writable: false); + var composition = new AddressSpaceComposition( + Array.Empty(), Array.Empty(), Array.Empty()) + { + UnsReferenceVariables = new[] { untouched, added }, + }; + + NewApplier(sink).MaterialiseUnsReferences(composition, only: new[] { added }); + + sink.Variables.ShouldHaveSingleItem().NodeId.ShouldBe("filling/line1/station1/temp"); + sink.References.ShouldHaveSingleItem().Target.ShouldBe("Plant/A/dev/temp"); + } + /// A capturing sink that records (nodeId, realm) for every materialise + the Organizes edges. private sealed class RealmRecordingSink : IOpcUaAddressSpaceSink { public List<(string NodeId, string? Parent, AddressSpaceRealm Realm)> Folders { get; } = new(); - public List<(string NodeId, string? Parent, bool Writable, string? HistorianTagname, AddressSpaceRealm Realm)> Variables { get; } = new(); + public List<(string NodeId, string? Parent, bool Writable, string? HistorianTagname, AddressSpaceRealm Realm, bool IsArray, uint? ArrayLength)> Variables { get; } = new(); public List<(string NodeId, string? Parent, bool IsNative, AddressSpaceRealm Realm)> Conditions { get; } = new(); public List<(string Source, AddressSpaceRealm SourceRealm, string Target, AddressSpaceRealm TargetRealm, string ReferenceType)> References { get; } = new(); public List<(string AlarmNodeId, AddressSpaceRealm AlarmRealm, IReadOnlyList FolderNodeIds, AddressSpaceRealm FolderRealm)> NotifierWirings { get; } = new(); @@ -289,7 +452,7 @@ public sealed class AddressSpaceApplierRawUnsTests => Folders.Add((folderNodeId, parentNodeId, realm)); public void EnsureVariable(string variableNodeId, string? parentFolderNodeId, string displayName, string dataType, bool writable, AddressSpaceRealm realm, string? historianTagname = null, bool isArray = false, uint? arrayLength = null) - => Variables.Add((variableNodeId, parentFolderNodeId, writable, historianTagname, realm)); + => Variables.Add((variableNodeId, parentFolderNodeId, writable, historianTagname, realm, isArray, arrayLength)); public void MaterialiseAlarmCondition(string alarmNodeId, string equipmentNodeId, string displayName, string alarmType, int severity, AddressSpaceRealm realm, bool isNative = false) => Conditions.Add((alarmNodeId, equipmentNodeId, isNative, realm)); diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierTests.cs index 7151a280..1251f581 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierTests.cs @@ -147,8 +147,8 @@ public sealed class AddressSpaceApplierTests sink.RebuildCalls.ShouldBe(0); var nodeId = V3NodeIds.Uns("eq-1", "Speed"); // Terminal Bad written to the removed variable BEFORE the removal. - sink.ValueWrites.ShouldContain((nodeId, OpcUaStatus.Bad)); - sink.RemoveCalls.ShouldHaveSingleItem().ShouldBe(("var", nodeId)); + sink.ValueWrites.ShouldContain((nodeId, OpcUaStatus.Bad, AddressSpaceRealm.Uns)); + sink.RemoveCalls.ShouldHaveSingleItem().ShouldBe(("var", nodeId, AddressSpaceRealm.Uns)); } /// R2-07 T11 — a removed alarm-bearing tag writes the terminal RemovedConditionState (closing @@ -178,7 +178,7 @@ public sealed class AddressSpaceApplierTests 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.RemoveCalls.ShouldHaveSingleItem().ShouldBe(("alarm", nodeId, AddressSpaceRealm.Uns)); sink.ValueWrites.ShouldNotContain(w => w.NodeId == nodeId); } @@ -207,7 +207,7 @@ public sealed class AddressSpaceApplierTests 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")); + sink.RemoveCalls.ShouldHaveSingleItem().ShouldBe(("equipment", "eq-1", AddressSpaceRealm.Uns)); } /// R2-07 T11 — if a surgical remove reports the node unknown (RemoveReturns=false), the applier @@ -283,7 +283,7 @@ public sealed class AddressSpaceApplierTests 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", V3NodeIds.Uns("eq-1", "Old"))); + sink.RemoveCalls.ShouldHaveSingleItem().ShouldBe(("var", V3NodeIds.Uns("eq-1", "Old"), AddressSpaceRealm.Uns)); outcome.AddedNodes.ShouldBe(1); outcome.RemovedNodes.ShouldBe(1); } @@ -314,7 +314,7 @@ public sealed class AddressSpaceApplierTests var outcome = applier.Apply(plan); outcome.RebuildCalled.ShouldBeFalse(); - sink.RemoveCalls.ShouldHaveSingleItem().ShouldBe(("var", nodeId)); + sink.RemoveCalls.ShouldHaveSingleItem().ShouldBe(("var", nodeId, AddressSpaceRealm.Uns)); } /// R2-07 T13 — a remove failure inside an AddRemoveMix still trips the rebuild ratchet. @@ -787,8 +787,8 @@ public sealed class AddressSpaceApplierTests sink.RebuildCalls.ShouldBe(0); outcome.RemovedNodes.ShouldBe(2); // both removals counted sink.RemoveCalls.Count.ShouldBe(2); - sink.RemoveCalls.ShouldContain(("var", V3NodeIds.Uns("eq-1", "Speed"))); - sink.RemoveCalls.ShouldContain(("var", V3NodeIds.Uns("eq-1", "Efficiency"))); + sink.RemoveCalls.ShouldContain(("var", V3NodeIds.Uns("eq-1", "Speed"), AddressSpaceRealm.Uns)); + sink.RemoveCalls.ShouldContain(("var", V3NodeIds.Uns("eq-1", "Efficiency"), AddressSpaceRealm.Uns)); } // ----- F10b: surgical in-place tag-attribute writes (Writable / IsHistorized / HistorianTagname) ----- @@ -1401,6 +1401,31 @@ public sealed class AddressSpaceApplierTests applier.ComputeAddAnnouncements(noLine).ShouldBe(new[] { "eq-2" }); } + /// + /// 2026-08-15 review I5 — an added UNS reference variable announces its EQUIPMENT folder, which is the + /// parent MaterialiseUnsReferences hangs it under and a UNS-realm node (the realm + /// AnnounceAddedNodes hardcodes). Before the surgical-deploy branch a plan carrying adds went + /// through a full rebuild, so every client re-browsed regardless and the missing announcement was + /// invisible; now the add is surgical and an unannounced node stays unseen until some later browse. + /// Two references under one equipment dedup to a single announcement. + /// + [Fact] + public void ComputeAddAnnouncements_added_uns_reference_announces_its_equipment() + { + var applier = new AddressSpaceApplier(new RecordingSink(), NullLogger.Instance); + + var plan = EmptyPlan with + { + AddedUnsReferenceVariables = new[] + { + UnsRef("filling/line-1/station-1/speed", "speed"), + UnsRef("filling/line-1/station-1/temp", "temp"), + }, + }; + + applier.ComputeAddAnnouncements(plan).ShouldBe(new[] { "eq-1" }); + } + /// A plan with no additions yields no announcements. [Fact] public void ComputeAddAnnouncements_empty_add_plan_yields_none() @@ -1434,6 +1459,307 @@ public sealed class AddressSpaceApplierTests new[] { "eq-1", V3NodeIds.Uns("eq-1", "Diag"), "line-7" }.OrderBy(x => x)); } + // ----- Surgical v3 deltas: raw-tag attribute edits (Raw realm) + UNS-reference remove ----- + // Task 4 taught AddressSpaceChangeClassifier that a non-alarm RawTagDelta and any UnsReferenceDelta are + // surgical-eligible; these pin the applier actually APPLYING them (before this, such a plan classified + // AttributeOnly and fell through the surgical branch as a silent no-op). + + /// A non-alarm raw-tag delta (historize flipped on) is applied IN PLACE on the raw node via + /// ISurgicalAddressSpaceSink.UpdateTagAttributes in the Raw realm — no rebuild, so every + /// client subscription survives. The arguments match + /// exactly: a blank historian override resolves to the RawPath (== NodeId). + [Fact] + public void Surgical_raw_tag_delta_updates_in_place_raw_realm() + { + var sink = new RecordingSink(); + var applier = new AddressSpaceApplier(sink, NullLogger.Instance); + + var previous = RawTag("Plant/Modbus/dev1/speed"); + var current = previous with { IsHistorized = true }; + + var plan = AddressSpacePlanner.Compute(CompositionWithRawTags(previous), CompositionWithRawTags(current)); + plan.ChangedRawTags.Count.ShouldBe(1); + plan.AddedRawTags.ShouldBeEmpty(); + plan.RemovedRawTags.ShouldBeEmpty(); + + var outcome = applier.Apply(plan); + + outcome.RebuildCalled.ShouldBeFalse(); + sink.RebuildCalls.ShouldBe(0); + var call = sink.SurgicalCalls.ShouldHaveSingleItem(); + call.NodeId.ShouldBe("Plant/Modbus/dev1/speed"); + call.Writable.ShouldBeTrue(); // Writable && !IsArray + call.Historian.ShouldBe("Plant/Modbus/dev1/speed"); // blank override ⇒ the RawPath + call.DataType.ShouldBe("Float"); + call.IsArray.ShouldBeFalse(); + call.ArrayLength.ShouldBeNull(); + call.Realm.ShouldBe(AddressSpaceRealm.Raw); // the raw node, NOT a UNS projection + outcome.ChangedNodes.ShouldBe(1); + } + + /// MaterialiseRawSubtree parity for the two computed arguments: an ARRAY tag is forced read-only + /// (the driver write path can't handle arrays) and a non-blank HistorianTagname override wins over + /// the RawPath default. + [Fact] + public void Surgical_raw_tag_delta_forces_array_read_only_and_honours_historian_override() + { + var sink = new RecordingSink(); + var applier = new AddressSpaceApplier(sink, NullLogger.Instance); + + var previous = RawTag("Plant/Modbus/dev1/block") with + { + Writable = true, IsArray = true, ArrayLength = 8, IsHistorized = true, HistorianTagname = "WW.Old", + }; + var current = previous with { HistorianTagname = "WW.New", ArrayLength = 16 }; + + var plan = AddressSpacePlanner.Compute(CompositionWithRawTags(previous), CompositionWithRawTags(current)); + plan.ChangedRawTags.Count.ShouldBe(1); + + applier.Apply(plan); + + sink.RebuildCalls.ShouldBe(0); + var call = sink.SurgicalCalls.ShouldHaveSingleItem(); + call.Writable.ShouldBeFalse(); // Writable && !IsArray ⇒ an array tag is read-only + call.Historian.ShouldBe("WW.New"); // override verbatim + call.IsArray.ShouldBeTrue(); + call.ArrayLength.ShouldBe(16u); + call.Realm.ShouldBe(AddressSpaceRealm.Raw); + } + + /// The headline UNS-reference case: adding/dropping ONE UnsTagReference flips its backing + /// raw tag to Changed purely through ReferencingEquipmentPaths, which is node-INERT for a value tag. + /// The applier must recognise that and make NO sink call at all — not a rebuild, and not even an + /// idempotent in-place update. + [Fact] + public void Reference_only_raw_tag_delta_skips_the_sink_call() + { + var sink = new RecordingSink(); + var applier = new AddressSpaceApplier(sink, NullLogger.Instance); + + var previous = RawTag("Plant/Modbus/dev1/speed"); + var current = previous with { ReferencingEquipmentPaths = new[] { "filling/line-1/station-1" } }; + + var plan = AddressSpacePlanner.Compute(CompositionWithRawTags(previous), CompositionWithRawTags(current)); + plan.ChangedRawTags.Count.ShouldBe(1); + + var outcome = applier.Apply(plan); + + outcome.RebuildCalled.ShouldBeFalse(); + sink.RebuildCalls.ShouldBe(0); + sink.SurgicalCalls.ShouldBeEmpty(); // node-inert ⇒ nothing to say to the sink + } + + /// A UNS-reference delta (here a display-name-override edit, which moves the NodeId) tears the + /// PREVIOUS node down in place: a terminal Bad so an in-flight MonitoredItem observes the removal, then + /// RemoveVariableNode — both in the Uns realm, no rebuild. The RECREATE of the current node is NOT the + /// applier's job (it rides the materialise pass after Apply returns). + [Fact] + public void Uns_reference_delta_removes_previous_node() + { + var sink = new RecordingSink(); + var applier = new AddressSpaceApplier(sink, NullLogger.Instance); + + var previous = UnsRef("filling/line-1/station-1/speed", "speed"); + var current = previous with { NodeId = "filling/line-1/station-1/Line Speed", EffectiveName = "Line Speed" }; + + var plan = AddressSpacePlanner.Compute(CompositionWithUnsRefs(previous), CompositionWithUnsRefs(current)); + plan.ChangedUnsReferenceVariables.Count.ShouldBe(1); + plan.AddedUnsReferenceVariables.ShouldBeEmpty(); + plan.RemovedUnsReferenceVariables.ShouldBeEmpty(); + + var outcome = applier.Apply(plan); + + outcome.RebuildCalled.ShouldBeFalse(); + sink.RebuildCalls.ShouldBe(0); + sink.ValueWrites.ShouldContain((previous.NodeId, OpcUaStatus.Bad, AddressSpaceRealm.Uns)); + sink.RemoveCalls.ShouldHaveSingleItem().ShouldBe(("var", previous.NodeId, AddressSpaceRealm.Uns)); + // The applier never recreates — no EnsureVariable for the new NodeId here. + sink.VariableCalls.ShouldBeEmpty(); + outcome.ChangedNodes.ShouldBe(1); + } + + /// + /// 2026-08-15 review — the combined plan a backing-tag HISTORIAN edit produces: a surgical raw-tag + /// delta AND the coincident UNS-reference delta (the reference record carries the backing historian + /// intent, so it goes unequal too). Both are non-rebuild kinds, so ONE apply must update the raw node + /// in place AND repair the stale UNS mirror — with NO rebuild. Without the coincident delta the UNS + /// mirror kept the OLD historian tagname and HistoryRead through the UNS NodeId served a different + /// series than through the raw NodeId. + /// The mirror is repaired IN PLACE (review I2), not by remove+recreate. This delta's + /// IDENTITY is unchanged (same NodeId, same EquipmentId, same BackingRawPath) — only the inherited + /// Backing* attributes moved — so UpdateTagAttributes in the Uns realm says everything that + /// needs saying and the mirror's client MonitoredItems survive. Remove+recreate stays for the deltas + /// that genuinely change identity (a re-point or an effective-name edit). + /// + [Fact] + public void Backing_historian_edit_updates_both_the_raw_node_and_the_uns_mirror_in_place() + { + var sink = new RecordingSink(); + var applier = new AddressSpaceApplier(sink, NullLogger.Instance); + + var rawBefore = RawTag("Plant/Modbus/dev1/speed") with { IsHistorized = true, HistorianTagname = "WW.Old" }; + var rawAfter = rawBefore with { HistorianTagname = "WW.New" }; + var unsBefore = UnsRef("filling/line-1/station-1/speed", "speed") with + { + BackingIsHistorized = true, BackingHistorianTagname = "WW.Old", + }; + var unsAfter = unsBefore with { BackingHistorianTagname = "WW.New" }; + + var previous = CompositionWith(rawBefore, unsBefore); + var current = CompositionWith(rawAfter, unsAfter); + var plan = AddressSpacePlanner.Compute(previous, current); + + // The plan shape the fix produces: one delta in EACH realm, nothing structural. + plan.ChangedRawTags.Count.ShouldBe(1); + plan.ChangedUnsReferenceVariables.Count.ShouldBe(1); + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.AttributeOnly); + + var outcome = applier.Apply(plan); + + outcome.RebuildCalled.ShouldBeFalse(); + sink.RebuildCalls.ShouldBe(0); + + // Raw node: updated in place with the NEW tagname, in the Raw realm. + var raw = sink.SurgicalCalls.Single(c => c.Realm == AddressSpaceRealm.Raw); + raw.NodeId.ShouldBe("Plant/Modbus/dev1/speed"); + raw.Historian.ShouldBe("WW.New"); + + // UNS mirror: the SAME in-place update, in the Uns realm, carrying the new inherited tagname. + var uns = sink.SurgicalCalls.Single(c => c.Realm == AddressSpaceRealm.Uns); + uns.NodeId.ShouldBe(unsAfter.NodeId); + uns.Historian.ShouldBe("WW.New"); + + // …and NOT torn down: no remove, and no terminal Bad on the mirror's subscribers. + sink.RemoveCalls.ShouldBeEmpty(); + sink.ValueWrites.ShouldNotContain((unsBefore.NodeId, OpcUaStatus.Bad, AddressSpaceRealm.Uns)); + } + + /// The same combined-plan repair for a scalar→ARRAY flip on the backing tag: without it the UNS + /// mirror stays scalar AND writable, bypassing the array read-only write gate through the UNS NodeId. + /// Identity is unchanged, so the mirror is repaired in place too (review I2) — and the in-place call must + /// carry the array shape AND the forced read-only, exactly as a recreate would have. + [Fact] + public void Backing_shape_flip_updates_the_uns_mirror_in_place_without_rebuilding() + { + var sink = new RecordingSink(); + var applier = new AddressSpaceApplier(sink, NullLogger.Instance); + + var rawBefore = RawTag("Plant/Modbus/dev1/block"); + var rawAfter = rawBefore with { IsArray = true, ArrayLength = 8 }; + var unsBefore = UnsRef("filling/line-1/station-1/block", "block"); + var unsAfter = unsBefore with { BackingIsArray = true, BackingArrayLength = 8 }; + + var plan = AddressSpacePlanner.Compute( + CompositionWith(rawBefore, unsBefore), CompositionWith(rawAfter, unsAfter)); + + plan.ChangedRawTags.Count.ShouldBe(1); + plan.ChangedUnsReferenceVariables.Count.ShouldBe(1); + + var outcome = applier.Apply(plan); + + outcome.RebuildCalled.ShouldBeFalse(); + sink.RebuildCalls.ShouldBe(0); + sink.SurgicalCalls.Single(c => c.Realm == AddressSpaceRealm.Raw).IsArray.ShouldBeTrue(); + + var uns = sink.SurgicalCalls.Single(c => c.Realm == AddressSpaceRealm.Uns); + uns.NodeId.ShouldBe(unsAfter.NodeId); + uns.IsArray.ShouldBeTrue(); + uns.ArrayLength.ShouldBe(8u); + // The reference row is authored Writable, but an array node is forced read-only — the same + // `Writable && !BackingIsArray` rule MaterialiseUnsReferences applies when it creates the node. + uns.Writable.ShouldBeFalse(); + sink.RemoveCalls.ShouldBeEmpty(); + } + + /// The one-way rebuild ratchet holds for the new raw-tag path: a surgical update that reports the + /// node missing (false) falls back to a full rebuild. + [Fact] + public void Surgical_raw_tag_false_falls_back_to_rebuild() + { + var sink = new RecordingSink { SurgicalReturns = false }; + var applier = new AddressSpaceApplier(sink, NullLogger.Instance); + + var previous = RawTag("Plant/Modbus/dev1/speed"); + var current = previous with { IsHistorized = true }; + + var outcome = applier.Apply( + AddressSpacePlanner.Compute(CompositionWithRawTags(previous), CompositionWithRawTags(current))); + + sink.SurgicalCalls.ShouldHaveSingleItem(); // attempted in place first + outcome.RebuildCalled.ShouldBeTrue(); + sink.RebuildCalls.ShouldBe(1); + } + + /// The same ratchet for the UNS-reference remove: a remove reporting the node missing (false) + /// falls back to a full rebuild. + [Fact] + public void Uns_reference_remove_false_falls_back_to_rebuild() + { + var sink = new RecordingSink { RemoveReturns = false }; + var applier = new AddressSpaceApplier(sink, NullLogger.Instance); + + var previous = UnsRef("filling/line-1/station-1/speed", "speed"); + var current = previous with { BackingRawPath = "Plant/Modbus/dev1/speed2" }; + + var outcome = applier.Apply( + AddressSpacePlanner.Compute(CompositionWithUnsRefs(previous), CompositionWithUnsRefs(current))); + + sink.RemoveCalls.ShouldHaveSingleItem(); // attempted in place first + outcome.RebuildCalled.ShouldBeTrue(); + sink.RebuildCalls.ShouldBe(1); + } + + /// A sink lacking the surgical capability cannot apply a raw-tag delta in place, so it must + /// REBUILD. This is what closes the Task-4 gap for the fallback case: before the branch trigger learned + /// about raw-tag deltas, such a plan classified AttributeOnly and applied nothing at all. + [Fact] + public void Raw_tag_delta_with_non_surgical_sink_rebuilds() + { + var sink = new PlainRecordingSink(); + var applier = new AddressSpaceApplier(sink, NullLogger.Instance); + + var previous = RawTag("Plant/Modbus/dev1/speed"); + var current = previous with { IsHistorized = true }; + + var outcome = applier.Apply( + AddressSpacePlanner.Compute(CompositionWithRawTags(previous), CompositionWithRawTags(current))); + + outcome.RebuildCalled.ShouldBeTrue(); + sink.RebuildCalls.ShouldBe(1); + } + + private static RawTagPlan RawTag(string rawPath) => + new("tag-" + rawPath, rawPath, ParentNodeId: "Plant/Modbus/dev1", DriverInstanceId: "drv-1", + Name: rawPath[(rawPath.LastIndexOf('/') + 1)..], DataType: "Float", Writable: true, Alarm: null, + ReferencingEquipmentPaths: Array.Empty()); + + private static UnsReferenceVariable UnsRef(string nodeId, string effectiveName) => + new("ref-1", "eq-1", nodeId, effectiveName, "Plant/Modbus/dev1/speed", "Float", Writable: true); + + private static AddressSpaceComposition CompositionWithRawTags(params RawTagPlan[] tags) => + new( + Array.Empty(), Array.Empty(), Array.Empty()) + { + RawTags = tags, + }; + + // Both realms in one composition — the shape a backing-tag edit produces (a raw delta AND its coincident + // UNS-reference delta) needs the raw tag and the reference to it side by side. + private static AddressSpaceComposition CompositionWith(RawTagPlan tag, UnsReferenceVariable reference) => + new( + Array.Empty(), Array.Empty(), Array.Empty()) + { + RawTags = new[] { tag }, + UnsReferenceVariables = new[] { reference }, + }; + + private static AddressSpaceComposition CompositionWithUnsRefs(params UnsReferenceVariable[] refs) => + new( + Array.Empty(), Array.Empty(), Array.Empty()) + { + UnsReferenceVariables = refs, + }; + private static AddressSpaceComposition CompositionWithAreas(params UnsAreaProjection[] areas) => new( areas, Array.Empty(), Array.Empty(), @@ -1486,9 +1812,9 @@ public sealed class AddressSpaceApplierTests private sealed class RecordingSink : IOpcUaAddressSpaceSink, ISurgicalAddressSpaceSink { /// Gets the queue of surgical in-place tag-attribute update calls (F10b + FB-7). - public ConcurrentQueue<(string NodeId, bool Writable, string? Historian, string DataType, bool IsArray, uint? ArrayLength)> SurgicalQueue { get; } = new(); + public ConcurrentQueue<(string NodeId, bool Writable, string? Historian, string DataType, bool IsArray, uint? ArrayLength, AddressSpaceRealm Realm)> SurgicalQueue { get; } = new(); /// Gets the list of recorded surgical in-place tag-attribute update calls. - public List<(string NodeId, bool Writable, string? Historian, string DataType, bool IsArray, uint? ArrayLength)> SurgicalCalls => SurgicalQueue.ToList(); + public List<(string NodeId, bool Writable, string? Historian, string DataType, bool IsArray, uint? ArrayLength, AddressSpaceRealm Realm)> SurgicalCalls => SurgicalQueue.ToList(); /// When false, reports the node missing (returns false), /// driving the applier's rebuild fallback. Defaults to true (node present, update succeeds). public bool SurgicalReturns { get; init; } = true; @@ -1502,7 +1828,7 @@ public sealed class AddressSpaceApplierTests /// The new 1-D array length when is true. public bool UpdateTagAttributes(string variableNodeId, bool writable, string? historianTagname, string dataType, bool isArray, uint? arrayLength, AddressSpaceRealm realm = AddressSpaceRealm.Uns) { - SurgicalQueue.Enqueue((variableNodeId, writable, historianTagname, dataType, isArray, arrayLength)); + SurgicalQueue.Enqueue((variableNodeId, writable, historianTagname, dataType, isArray, arrayLength, realm)); return SurgicalReturns; } @@ -1524,9 +1850,9 @@ public sealed class AddressSpaceApplierTests } /// 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(); + public ConcurrentQueue<(string Kind, string NodeId, AddressSpaceRealm Realm)> RemoveQueue { get; } = new(); /// Gets the list of recorded surgical remove calls. - public List<(string Kind, string NodeId)> RemoveCalls => RemoveQueue.ToList(); + public List<(string Kind, string NodeId, AddressSpaceRealm Realm)> 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; @@ -1534,21 +1860,21 @@ public sealed class AddressSpaceApplierTests /// Records a surgical variable-node removal; returns . public bool RemoveVariableNode(string variableNodeId, AddressSpaceRealm realm = AddressSpaceRealm.Uns) { - RemoveQueue.Enqueue(("var", variableNodeId)); + RemoveQueue.Enqueue(("var", variableNodeId, realm)); return RemoveReturns; } /// Records a surgical alarm-condition-node removal; returns . public bool RemoveAlarmConditionNode(string alarmNodeId, AddressSpaceRealm realm = AddressSpaceRealm.Uns) { - RemoveQueue.Enqueue(("alarm", alarmNodeId)); + RemoveQueue.Enqueue(("alarm", alarmNodeId, realm)); return RemoveReturns; } /// Records a surgical equipment-subtree removal; returns . public bool RemoveEquipmentSubtree(string equipmentNodeId, AddressSpaceRealm realm = AddressSpaceRealm.Uns) { - RemoveQueue.Enqueue(("equipment", equipmentNodeId)); + RemoveQueue.Enqueue(("equipment", equipmentNodeId, realm)); return RemoveReturns; } @@ -1583,9 +1909,9 @@ public sealed class AddressSpaceApplierTests public List<(string AlarmNodeId, string EquipmentNodeId, string DisplayName, string AlarmType, int Severity, bool IsNative)> AlarmConditionCalls => AlarmConditionQueue.ToList(); /// Gets the queue of value writes (NodeId, quality) — used to assert the PureRemove terminal Bad. - public ConcurrentQueue<(string NodeId, uint Quality)> ValueWriteQueue { get; } = new(); + public ConcurrentQueue<(string NodeId, uint Quality, AddressSpaceRealm Realm)> ValueWriteQueue { get; } = new(); /// Gets the list of recorded value writes. - public List<(string NodeId, uint Quality)> ValueWrites => ValueWriteQueue.ToList(); + public List<(string NodeId, uint Quality, AddressSpaceRealm Realm)> ValueWrites => ValueWriteQueue.ToList(); /// Records a value write (NodeId + quality). /// The node ID. @@ -1593,7 +1919,7 @@ public sealed class AddressSpaceApplierTests /// The OPC UA status code, sub-code included. /// The source timestamp in UTC. public void WriteValue(string nodeId, object? value, uint statusCode, DateTime sourceTimestampUtc, AddressSpaceRealm realm = AddressSpaceRealm.Uns) - => ValueWriteQueue.Enqueue((nodeId, statusCode)); + => ValueWriteQueue.Enqueue((nodeId, statusCode, realm)); /// 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/AddressSpaceChangeClassifierDualNamespaceTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceChangeClassifierDualNamespaceTests.cs index 28f758df..0f388b92 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceChangeClassifierDualNamespaceTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceChangeClassifierDualNamespaceTests.cs @@ -6,8 +6,11 @@ namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests; /// /// v3 Batch 4 (B4-WP1) — the classifier folds the Raw + UNS subtree diff sets into its routing policy: -/// raw/uns adds count as adds (PureAdd), removes as removes (PureRemove), and any Changed raw/uns delta -/// routes to Rebuild (the safe default — WP3's applier owns surgical refinement). +/// raw/uns adds count as adds (PureAdd), removes as removes (PureRemove). +/// Surgical deploys (2026-08-15 Task 4) narrowed the Changed sets: a NON-alarm raw-tag delta whose +/// only differences are the in-place-updatable attributes and/or its ReferencingEquipmentPaths is +/// surgical (NOT a rebuild), and every UNS-reference delta is applied remove+recreate. Alarm-bearing and +/// identity-field raw-tag deltas still route to Rebuild — the default-closed WP4 notifier guard. /// public sealed class AddressSpaceChangeClassifierDualNamespaceTests { @@ -71,8 +74,12 @@ public sealed class AddressSpaceChangeClassifierDualNamespaceTests AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.AddRemoveMix); } + /// The headline surgical case: adding/dropping a UnsTagReference flips the BACKING raw tag + /// to Changed (ReferencingEquipmentPaths is part of RawTagPlan's equality). For a non-alarm VALUE tag that + /// set is node-inert — it only drives native-alarm notifier wiring — so it must NOT tear down the world. + /// (Was Changed_raw_tag_is_Rebuild; deliberately re-pointed by 2026-08-15 Task 4.) [Fact] - public void Changed_raw_tag_is_Rebuild() + public void Reference_only_raw_tag_delta_is_not_a_rebuild() { var plan = Empty() with { @@ -80,14 +87,129 @@ public sealed class AddressSpaceChangeClassifierDualNamespaceTests { new AddressSpacePlan.RawTagDelta( RawTag("Plant/Modbus1/PLC-A/Speed"), - RawTag("Plant/Modbus1/PLC-A/Speed") with { IsHistorized = true }), + RawTag("Plant/Modbus1/PLC-A/Speed") with + { + ReferencingEquipmentPaths = new[] { "filling/line-1/station-1" }, + }), + }, + }; + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.AttributeOnly); + } + + /// The in-place-updatable attribute whitelist (Writable / IsHistorized / HistorianTagname / + /// DataType / IsArray / ArrayLength) on a non-alarm raw tag is surgical, not a rebuild. + [Theory] + [InlineData("writable")] + [InlineData("historized")] + [InlineData("historianTagname")] + [InlineData("dataType")] + [InlineData("array")] + [InlineData("arrayLength")] + public void Attribute_raw_tag_delta_is_not_a_rebuild(string field) + { + var baseline = RawTag("Plant/Modbus1/PLC-A/Speed"); + // ArrayLength moving ALONE (a resize of an already-array tag) — the whitelist must carry ArrayLength + // in its own right, not only as a passenger of an IsArray flip. Its "previous" is already an array. + var prev = field == "arrayLength" ? baseline with { IsArray = true, ArrayLength = 8 } : baseline; + var cur = field switch + { + "writable" => prev with { Writable = true }, + "historized" => prev with { IsHistorized = true }, + "historianTagname" => prev with { IsHistorized = true, HistorianTagname = "Plant.PLC_A.Speed" }, + "dataType" => prev with { DataType = "Double" }, + "array" => prev with { IsArray = true, ArrayLength = 8 }, + "arrayLength" => prev with { ArrayLength = 16 }, + _ => throw new ArgumentOutOfRangeException(nameof(field)), + }; + + var plan = Empty() with { ChangedRawTags = new[] { new AddressSpacePlan.RawTagDelta(prev, cur) } }; + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.AttributeOnly); + } + + /// WP4 binding guard: a raw tag carrying a native alarm on EITHER side still rebuilds — the + /// rebuild is what clears + re-wires the per-equipment alarm notifiers. + [Theory] + [InlineData(true, false)] + [InlineData(false, true)] + [InlineData(true, true)] + public void Alarm_bearing_raw_tag_delta_still_rebuilds(bool alarmBefore, bool alarmAfter) + { + var alarm = new EquipmentTagAlarmInfo("OffNormalAlarm", 700); + var prev = RawTag("Plant/Modbus1/PLC-A/Speed") with { Alarm = alarmBefore ? alarm : null }; + var cur = RawTag("Plant/Modbus1/PLC-A/Speed") with + { + Alarm = alarmAfter ? alarm : null, + ReferencingEquipmentPaths = new[] { "filling/line-1/station-1" }, + }; + + var plan = Empty() with { ChangedRawTags = new[] { new AddressSpacePlan.RawTagDelta(prev, cur) } }; + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.Rebuild); + } + + /// WP4 binding guard, CONTENT edition: an alarm present on BOTH sides whose severity (or type) + /// moved still rebuilds. The null-check pair alone does not decide it — the with-equality whitelist has to + /// leave Alarm out, so the two values compare unequal. + [Fact] + public void Alarm_content_only_raw_tag_delta_still_rebuilds() + { + var prev = RawTag("Plant/Modbus1/PLC-A/Speed") with { Alarm = new EquipmentTagAlarmInfo("OffNormalAlarm", 500) }; + var cur = prev with { Alarm = new EquipmentTagAlarmInfo("OffNormalAlarm", 900) }; + + var plan = Empty() with { ChangedRawTags = new[] { new AddressSpacePlan.RawTagDelta(prev, cur) } }; + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.Rebuild); + } + + /// The QUANTIFIER: the rebuild gate is .Any(d => !eligible(d)), so ONE ineligible raw-tag + /// delta must rebuild even when every other delta in the same plan is surgical. Pins against a regression + /// to .All(eligible) on the wrong side, or to inspecting only First() — either of which would + /// apply an identity/alarm change as an in-place attribute update and leave the node mis-wired. + [Fact] + public void One_ineligible_raw_tag_delta_among_eligible_ones_still_rebuilds() + { + var eligiblePrev = RawTag("Plant/Modbus1/PLC-A/Speed"); + var ineligiblePrev = RawTag("Plant/Modbus1/PLC-A/Temp") with { TagId = "t-2" }; + + var plan = Empty() with + { + ChangedRawTags = new[] + { + // Surgical: a plain historize toggle. Listed FIRST so a First()-only check would pass it. + new AddressSpacePlan.RawTagDelta(eligiblePrev, eligiblePrev with { IsHistorized = true }), + // Ineligible: DriverInstanceId re-routes the node to a different driver point. + new AddressSpacePlan.RawTagDelta(ineligiblePrev, ineligiblePrev with { DriverInstanceId = "drv-2" }), }, }; AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.Rebuild); } + /// Default-closed: an identity field outside the whitelist (DriverInstanceId re-routes the node to + /// another driver point; Name/TagId/ParentNodeId re-shape identity) still forces a full rebuild. + [Theory] + [InlineData("driver")] + [InlineData("name")] + [InlineData("tagId")] + [InlineData("parent")] + public void Identity_field_raw_tag_delta_still_rebuilds(string field) + { + var prev = RawTag("Plant/Modbus1/PLC-A/Speed"); + var cur = field switch + { + "driver" => prev with { DriverInstanceId = "drv-2" }, + "name" => prev with { Name = "Velocity" }, + "tagId" => prev with { TagId = "t-2" }, + "parent" => prev with { ParentNodeId = "Plant/Modbus1/PLC-B" }, + _ => throw new ArgumentOutOfRangeException(nameof(field)), + }; + + var plan = Empty() with { ChangedRawTags = new[] { new AddressSpacePlan.RawTagDelta(prev, cur) } }; + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.Rebuild); + } + + /// A UNS-reference re-point (BackingRawPath moved) is applied surgically (remove-previous + + /// recreate-current), NOT as a rebuild. (Was Uns_reference_repoint_is_Rebuild; deliberately + /// re-pointed by 2026-08-15 Task 4.) [Fact] - public void Uns_reference_repoint_is_Rebuild() + public void Uns_reference_repoint_delta_is_not_a_rebuild() { var plan = Empty() with { @@ -98,6 +220,40 @@ public sealed class AddressSpaceChangeClassifierDualNamespaceTests UnsRef("filling/line-1/station-1/Speed") with { BackingRawPath = "Plant/Modbus1/PLC-A/Velocity" }), }, }; - AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.Rebuild); + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.AttributeOnly); + } + + /// The other UnsReferenceDelta variant — a display-name override edit keeps the row id while the + /// NodeId + EffectiveName change. Also surgical (remove-previous + recreate-current). + [Fact] + public void Uns_reference_display_name_delta_is_not_a_rebuild() + { + var plan = Empty() with + { + ChangedUnsReferenceVariables = new[] + { + new AddressSpacePlan.UnsReferenceDelta( + UnsRef("filling/line-1/station-1/Speed"), + UnsRef("filling/line-1/station-1/LineSpeed") with { EffectiveName = "LineSpeed" }), + }, + }; + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.AttributeOnly); + } + + /// The headline end-to-end shape: adding ONE UNS reference produces an added UNS variable PLUS the + /// induced reference-only delta on the backing raw tag. That must classify PureAdd — one node created, + /// zero teardown, every client subscription server-wide survives. + [Fact] + public void Uns_reference_add_with_induced_reference_only_raw_delta_is_pure_add() + { + var prev = RawTag("Plant/Modbus1/PLC-A/Speed"); + var cur = prev with { ReferencingEquipmentPaths = new[] { "filling/line-1/station-1" } }; + + var plan = Empty() with + { + AddedUnsReferenceVariables = new[] { UnsRef("filling/line-1/station-1/Speed") }, + ChangedRawTags = new[] { new AddressSpacePlan.RawTagDelta(prev, cur) }, + }; + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.PureAdd); } } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceComposerDualNamespaceTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceComposerDualNamespaceTests.cs index c22cdf79..f1e51249 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceComposerDualNamespaceTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceComposerDualNamespaceTests.cs @@ -178,6 +178,70 @@ public sealed class AddressSpaceComposerDualNamespaceTests statusRef.Writable.ShouldBeFalse(); // inherited from the Read raw tag } + /// + /// 2026-08-15 surgical-deploys review — a UNS reference also carries its backing tag's HISTORIAN + /// intent + ARRAY shape (Backing{IsHistorized,HistorianTagname,IsArray,ArrayLength}). They are + /// on the record so a raw-side edit to any of them makes the reference record unequal, producing the + /// coincident UnsReferenceDelta that repairs the mirror node; without them the UNS node kept the + /// old historian tagname / scalar shape after a surgical raw edit. + /// + [Fact] + public void Uns_reference_variables_carry_backing_historian_intent_and_array_shape() + { + var c = ComposeFullFixture(); + + // Backing tag historized WITH an override → both fields ride the reference verbatim. + var speedRef = c.UnsReferenceVariables.Single(v => v.UnsTagReferenceId == "ref-speed"); + speedRef.BackingIsHistorized.ShouldBeTrue(); + speedRef.BackingHistorianTagname.ShouldBe("speed_hist"); + speedRef.BackingIsArray.ShouldBeFalse(); + speedRef.BackingArrayLength.ShouldBeNull(); + + // Backing tag neither historized nor array → the defaults. + var statusRef = c.UnsReferenceVariables.Single(v => v.UnsTagReferenceId == "ref-status"); + statusRef.BackingIsHistorized.ShouldBeFalse(); + statusRef.BackingHistorianTagname.ShouldBeNull(); + statusRef.BackingIsArray.ShouldBeFalse(); + statusRef.BackingArrayLength.ShouldBeNull(); + } + + /// The array + historize-without-override branches of the same inheritance: an ARRAY backing tag + /// projects its shape onto the reference (so the UNS mirror is created as an array, and therefore + /// read-only), and a historized tag with a BLANK override leaves BackingHistorianTagname null — the + /// applier resolves that to the BackingRawPath, exactly as the raw side defaults to its NodeId. + [Fact] + public void Uns_reference_variables_carry_array_shape_and_blank_historian_override() + { + var driver = new DriverInstance + { + DriverInstanceId = "drv-modbus", ClusterId = "c1", RawFolderId = null, + Name = "Modbus1", DriverType = "Modbus", DriverConfig = "{}", + }; + var device = new Device { DeviceId = "dev-1", DriverInstanceId = "drv-modbus", Name = "PLC-A", DeviceConfig = "{}" }; + var levels = new Tag + { + TagId = "t-levels", DeviceId = "dev-1", Name = "Levels", DataType = "Int32", + AccessLevel = TagAccessLevel.ReadWrite, + TagConfig = "{\"isArray\":true,\"arrayLength\":8,\"isHistorized\":true}", + }; + var area = new UnsArea { UnsAreaId = "area-1", ClusterId = "c1", Name = "filling" }; + var line = new UnsLine { UnsLineId = "line-1", UnsAreaId = "area-1", Name = "line-1" }; + var equip = new Equipment { EquipmentId = "eq-1", UnsLineId = "line-1", Name = "station-1", MachineCode = "STATION_001" }; + var reference = new UnsTagReference { UnsTagReferenceId = "ref-levels", EquipmentId = "eq-1", TagId = "t-levels" }; + + var c = AddressSpaceComposer.Compose( + new[] { area }, new[] { line }, new[] { equip }, + new[] { driver }, Array.Empty(), + unsTagReferences: new[] { reference }, tags: new[] { levels }, + rawFolders: Array.Empty(), devices: new[] { device }); + + var levelsRef = c.UnsReferenceVariables.ShouldHaveSingleItem(); + levelsRef.BackingIsArray.ShouldBeTrue(); + levelsRef.BackingArrayLength.ShouldBe(8u); + levelsRef.BackingIsHistorized.ShouldBeTrue(); + levelsRef.BackingHistorianTagname.ShouldBeNull(); // blank override ⇒ resolved to BackingRawPath later + } + /// A composition with no raw/UNS inputs leaves all three new subtree sets empty (the legacy /// convenience overloads + earlier callers keep working). [Fact] diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpacePlannerDualNamespaceTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpacePlannerDualNamespaceTests.cs index b63807f9..dc7156cc 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpacePlannerDualNamespaceTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpacePlannerDualNamespaceTests.cs @@ -20,7 +20,7 @@ public sealed class AddressSpacePlannerDualNamespaceTests // Compose a fixture with one raw tag (given name) under Plant/Modbus1/PLC-A and one UNS reference to it // (with an optional display-name override so the effective name can be held stable across a raw rename). - private static AddressSpaceComposition Compose(string tagName, string? displayOverride) + private static AddressSpaceComposition Compose(string tagName, string? displayOverride, string tagConfig = "{}") { var folder = new RawFolder { RawFolderId = "raw-plant", ClusterId = "c1", Name = RawFolderName }; var driver = new DriverInstance @@ -32,7 +32,7 @@ public sealed class AddressSpacePlannerDualNamespaceTests var tag = new Tag { TagId = "t-speed", DeviceId = "dev-1", Name = tagName, - DataType = "Float", AccessLevel = TagAccessLevel.ReadWrite, TagConfig = "{}", + DataType = "Float", AccessLevel = TagAccessLevel.ReadWrite, TagConfig = tagConfig, }; var area = new UnsArea { UnsAreaId = "area-1", ClusterId = "c1", Name = "filling" }; var line = new UnsLine { UnsLineId = "line-1", UnsAreaId = "area-1", Name = "line-1" }; @@ -219,6 +219,55 @@ public sealed class AddressSpacePlannerDualNamespaceTests _ = prev; } + /// + /// THE PIN (2026-08-15 surgical-deploys review): a RAW-side edit to the backing tag's historian + /// intent or array shape must produce a ChangedRawTags delta AND a coincident + /// ChangedUnsReferenceVariables delta on every reference to it. The coincident delta is the ONLY + /// repair trigger for the UNS mirror node: the raw node is updated surgically in place and + /// EnsureVariable early-returns on the existing UNS node, so without this delta the mirror keeps + /// the OLD historian tagname (HistoryRead via the UNS NodeId serves a different series than via the raw + /// NodeId) and the OLD shape (a scalar→array flip leaves it scalar AND writable). Nothing about the + /// reference ROW changed here — only the backing tag's TagConfig — so the delta exists purely because + /// the four Backing* fields are on . + /// + [Theory] + [InlineData("{\"isHistorized\":true,\"historianTagname\":\"WW.Old\"}", "{\"isHistorized\":true,\"historianTagname\":\"WW.New\"}")] + [InlineData("{\"isHistorized\":true}", "{}")] + [InlineData("{}", "{\"isArray\":true,\"arrayLength\":8}")] + [InlineData("{\"isArray\":true,\"arrayLength\":8}", "{\"isArray\":true,\"arrayLength\":16}")] + public void Backing_tag_historian_or_shape_edit_produces_a_coincident_uns_reference_delta(string before, string after) + { + var prev = Compose("Speed", displayOverride: null, tagConfig: before); + var next = Compose("Speed", displayOverride: null, tagConfig: after); + + var plan = AddressSpacePlanner.Compute(prev, next); + + // Raw side: an attribute edit on a stable RawPath ⇒ a Changed delta (not remove+add). + var rawDelta = plan.ChangedRawTags.ShouldHaveSingleItem(); + rawDelta.Previous.NodeId.ShouldBe(rawDelta.Current.NodeId); + + // UNS side: the SAME reference row, same NodeId + BackingRawPath — flagged Changed ONLY because the + // inherited backing historian/shape moved. This is what drives the applier's remove+recreate repair. + var unsDelta = plan.ChangedUnsReferenceVariables.ShouldHaveSingleItem(); + unsDelta.Previous.UnsTagReferenceId.ShouldBe("ref-1"); + unsDelta.Previous.NodeId.ShouldBe(unsDelta.Current.NodeId); + unsDelta.Previous.BackingRawPath.ShouldBe(unsDelta.Current.BackingRawPath); + unsDelta.Previous.ShouldNotBe(unsDelta.Current); + + // And the inherited values on the CURRENT side match the backing tag's new intent, so the recreate + // materialises the mirror from the same truth the raw node was updated with. + unsDelta.Current.BackingIsHistorized.ShouldBe(rawDelta.Current.IsHistorized); + unsDelta.Current.BackingHistorianTagname.ShouldBe(rawDelta.Current.HistorianTagname); + unsDelta.Current.BackingIsArray.ShouldBe(rawDelta.Current.IsArray); + unsDelta.Current.BackingArrayLength.ShouldBe(rawDelta.Current.ArrayLength); + + // Neither realm sees a structural add/remove — this stays a surgical, subscription-preserving deploy. + plan.AddedRawTags.ShouldBeEmpty(); + plan.RemovedRawTags.ShouldBeEmpty(); + plan.AddedUnsReferenceVariables.ShouldBeEmpty(); + plan.RemovedUnsReferenceVariables.ShouldBeEmpty(); + } + /// Identical compositions diff to an empty plan across both realms (fresh list instances must /// not spuriously flag RawTags/UnsReferenceVariables as changed). [Fact] diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/SdkAddressSpaceSinkTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/SdkAddressSpaceSinkTests.cs index 699915a1..0e5ae264 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/SdkAddressSpaceSinkTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/SdkAddressSpaceSinkTests.cs @@ -190,6 +190,51 @@ public sealed class SdkAddressSpaceSinkTests : IDisposable await host.DisposeAsync(); } + /// + /// 2026-08-15 review I3 — RemoveVariableNode must sever the INVERSE half of the cross-tree + /// Organizes edge it holds, not just drop its own node. AddReference wires the edge + /// bidirectionally (forward on the UNS reference variable, inverse on the backing RAW node), and the + /// raw node SURVIVES a scoped remove — so without this the raw node keeps a browsable, unresolvable + /// reference to a node that no longer exists. Rebuilds used to sweep these up; surgical applies made + /// rebuilds rare, so they now accumulate one per re-point for the life of the process. + /// + [Fact] + public async Task RemoveVariableNode_severs_the_inverse_Organizes_edge_on_the_raw_target() + { + var (host, server) = await BootAsync(); + try + { + var nm = server.NodeManager!; + var sink = new SdkAddressSpaceSink(nm); + + sink.EnsureVariable("MAIN/modbus/dev/grp/temp", parentFolderNodeId: null, displayName: "Temp", + dataType: "Float", writable: false, realm: AddressSpaceRealm.Raw); + sink.EnsureVariable("Area/Line/Eq/Temperature", parentFolderNodeId: null, displayName: "Temperature", + dataType: "Float", writable: false, realm: AddressSpaceRealm.Uns); + sink.AddReference("Area/Line/Eq/Temperature", AddressSpaceRealm.Uns, + "MAIN/modbus/dev/grp/temp", AddressSpaceRealm.Raw); + + var unsVar = nm.TryGetVariable("Area/Line/Eq/Temperature", AddressSpaceRealm.Uns); + unsVar.ShouldNotBeNull(); + // Precondition: the raw node really does hold the inverse half. + nm.GetNodeReferences("MAIN/modbus/dev/grp/temp", AddressSpaceRealm.Raw) + .Any(r => r.ReferenceTypeId == ReferenceTypeIds.Organizes && r.IsInverse && + r.TargetId == unsVar.NodeId).ShouldBeTrue(); + + nm.RemoveVariableNode("Area/Line/Eq/Temperature", AddressSpaceRealm.Uns).ShouldBeTrue(); + + // The raw node survives… + nm.TryGetVariable("MAIN/modbus/dev/grp/temp", AddressSpaceRealm.Raw).ShouldNotBeNull(); + // …carrying NO reference back to the node that just went away. + nm.GetNodeReferences("MAIN/modbus/dev/grp/temp", AddressSpaceRealm.Raw) + .Any(r => r.TargetId == unsVar.NodeId).ShouldBeFalse(); + } + finally + { + await host.DisposeAsync(); + } + } + /// Verifies that RebuildAddressSpace clears all registered variables. [Fact] public async Task RebuildAddressSpace_clears_all_registered_variables() diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DeploymentArtifactRawUnsParityTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DeploymentArtifactRawUnsParityTests.cs index 21acd7ff..c80884ed 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DeploymentArtifactRawUnsParityTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DeploymentArtifactRawUnsParityTests.cs @@ -61,11 +61,14 @@ public sealed class DeploymentArtifactRawUnsParityTests var line = new UnsLine { UnsLineId = "l1", UnsAreaId = "a1", Name = "line1" }; var equip = new Equipment { EquipmentId = "EQ-1", UnsLineId = "l1", Name = "station1", MachineCode = "M1" }; var reference = new UnsTagReference { UnsTagReferenceId = "ref-1", EquipmentId = "EQ-1", TagId = "tag-speed", DisplayNameOverride = "MotorSpeed" }; + // A second reference onto the ARRAY tag: the UNS reference record carries the backing tag's array + // shape + historian intent (2026-08-15 review), so both branches must survive the artifact round-trip. + var arrayReference = new UnsTagReference { UnsTagReferenceId = "ref-2", EquipmentId = "EQ-1", TagId = "tag-profile", DisplayNameOverride = null }; var composed = AddressSpaceComposer.Compose( new[] { area }, new[] { line }, new[] { equip }, new[] { driver }, Array.Empty(), - unsTagReferences: new[] { reference }, tags: new[] { speed, run, profile, tempHi }, + unsTagReferences: new[] { reference, arrayReference }, tags: new[] { speed, run, profile, tempHi }, rawFolders: new[] { folder }, devices: new[] { device }, tagGroups: new[] { group }); // Artifact JSON — the same shape ConfigComposer serializes (enums numeric: ReadWrite = 1, Read = 0). @@ -85,7 +88,11 @@ public sealed class DeploymentArtifactRawUnsParityTests UnsAreas = new[] { new { UnsAreaId = "a1", Name = "filling", ClusterId = "c1" } }, UnsLines = new[] { new { UnsLineId = "l1", UnsAreaId = "a1", Name = "line1" } }, Equipment = new[] { new { EquipmentId = "EQ-1", UnsLineId = "l1", Name = "station1", MachineCode = "M1" } }, - UnsTagReferences = new[] { new { UnsTagReferenceId = "ref-1", EquipmentId = "EQ-1", TagId = "tag-speed", DisplayNameOverride = "MotorSpeed" } }, + UnsTagReferences = new[] + { + new { UnsTagReferenceId = "ref-1", EquipmentId = "EQ-1", TagId = "tag-speed", DisplayNameOverride = (string?)"MotorSpeed" }, + new { UnsTagReferenceId = "ref-2", EquipmentId = "EQ-1", TagId = "tag-profile", DisplayNameOverride = (string?)null }, + }, }); var decoded = DeploymentArtifact.ParseComposition(blob); @@ -103,9 +110,20 @@ public sealed class DeploymentArtifactRawUnsParityTests composedAlarm.Alarm.ShouldNotBeNull(); composedAlarm.Alarm!.AlarmType.ShouldBe("OffNormalAlarm"); composedAlarm.Writable.ShouldBeTrue(); - composed.UnsReferenceVariables.Count.ShouldBe(1); - composed.UnsReferenceVariables[0].NodeId.ShouldBe("filling/line1/station1/MotorSpeed"); - composed.UnsReferenceVariables[0].BackingRawPath.ShouldBe("Cell1/Modbus/Dev1/Fast/Speed"); + composed.UnsReferenceVariables.Count.ShouldBe(2); + var composedSpeedRef = composed.UnsReferenceVariables.Single(v => v.UnsTagReferenceId == "ref-1"); + composedSpeedRef.NodeId.ShouldBe("filling/line1/station1/MotorSpeed"); + composedSpeedRef.BackingRawPath.ShouldBe("Cell1/Modbus/Dev1/Fast/Speed"); + + // Same "both right, not both blind" sanity for the backing historian intent + array shape the UNS + // reference inherits — a composer that dropped them entirely would parity-match a decode that did too. + composedSpeedRef.BackingIsHistorized.ShouldBeTrue(); + composedSpeedRef.BackingHistorianTagname.ShouldBe("WW.Speed"); + composedSpeedRef.BackingIsArray.ShouldBeFalse(); + var composedProfileRef = composed.UnsReferenceVariables.Single(v => v.UnsTagReferenceId == "ref-2"); + composedProfileRef.BackingIsArray.ShouldBeTrue(); + composedProfileRef.BackingArrayLength.ShouldBe(16u); + composedProfileRef.BackingIsHistorized.ShouldBeFalse(); // Byte-parity: the three dual-namespace node sets are record-equal between the two seams. decoded.RawContainers.ShouldBe(composed.RawContainers); diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorApplyingStashTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorApplyingStashTests.cs new file mode 100644 index 00000000..4ca52a95 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorApplyingStashTests.cs @@ -0,0 +1,385 @@ +using System.Text.Json; +using Akka.Actor; +using Akka.Event; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Commons.Interfaces; +using ZB.MOM.WW.OtOpcUa.Commons.Messages.Deploy; +using ZB.MOM.WW.OtOpcUa.Commons.Messages.Fleet; +using ZB.MOM.WW.OtOpcUa.Commons.Types; +using ZB.MOM.WW.OtOpcUa.Runtime.DeploymentCache; +using ZB.MOM.WW.OtOpcUa.Runtime.Drivers; +using ZB.MOM.WW.OtOpcUa.Runtime.Tests.Harness; + +namespace ZB.MOM.WW.OtOpcUa.Runtime.Tests.Drivers; + +/// +/// Surgical-deploys Task 7 Part B — a dispatch that arrives while an apply is in flight is +/// STASHED, not re-forwarded to Self. +/// +/// +/// +/// Self.Forward put the message back at the tail of the actor's OWN mailbox, so with +/// nothing else to do the host spun dequeue → log → enqueue for the entire fetch window — +/// thousands of iterations and thousands of log lines for one deferred deployment, burning a +/// core and starving nothing but itself of useful work. The stash holds it outside the mailbox +/// until the apply finishes. +/// +/// +/// Runs at DEBUG because the assertion IS a log line: a spin is only distinguishable +/// from a single deferral by how many times it is announced. A log event below +/// akka.loglevel never reaches the EventStream at all, so at the default WARNING this +/// test would count zero either way and pass vacuously. +/// +/// +public sealed class DriverHostActorApplyingStashTests() + : RuntimeActorTestBase(AkkaTestHoconAtLevel("DEBUG")) +{ + private static readonly NodeId TestNode = NodeId.Parse("driver-test"); + private static readonly RevisionHash RevA = RevisionHash.Parse(new string('a', 64)); + private static readonly RevisionHash RevB = RevisionHash.Parse(new string('b', 64)); + private static readonly RevisionHash RevC = RevisionHash.Parse(new string('c', 64)); + + /// How long a forward-spin gets to announce itself thousands of times if it is back. + private static readonly TimeSpan SpinWindow = TimeSpan.FromSeconds(1.5); + + /// Minimal parseable artifact — no drivers. + private static byte[] ArtifactBytes() => + JsonSerializer.SerializeToUtf8Bytes(new { DriverInstances = Array.Empty() }); + + [Fact] + public void ADispatchArrivingMidApply_IsDeferredOnce_AndAppliedAfterTheApplyCompletes() + { + var db = NewInMemoryDbFactory(); + var fetcher = new GatedFetcher(ArtifactBytes()); + var deploymentA = DeploymentId.NewId(); + var deploymentB = DeploymentId.NewId(); + + var coordinator = CreateTestProbe(); + var actor = Sys.ActorOf(DriverHostActor.Props( + db, TestNode, coordinator.Ref, + localRoles: new HashSet { "driver" }, + deploymentArtifactCache: new NullArtifactCache(), + fetchAndCacheMode: true, + artifactFetcher: fetcher)); + + try + { + // A is in flight: the fetch has started and its task will not complete until released. + actor.Tell(new DispatchDeployment(deploymentA, RevA, CorrelationId.NewId())); + AwaitAssert(() => fetcher.Calls.ShouldBe(1), duration: PresenceBudget); + + var logs = SubscribeToLogs("deferring"); + + actor.Tell(new DispatchDeployment(deploymentB, RevB, CorrelationId.NewId())); + + // PRESENCE first, on the shared budget: the deferral must actually be ANNOUNCED. Counting + // only over a fixed window would let "the dispatch was silently dropped" score zero and + // then pass the ≤1 half below, which is the wrong bug to be blind to. + logs.DrainUntilAllSeen(PresenceBudget); + logs["deferring"].ShouldBeGreaterThanOrEqualTo(1); + + // ABSENCE second: over a window in which a forward-spin would announce itself thousands of + // times, no further deferral appears. + logs.Drain(SpinWindow); + logs["deferring"].ShouldBe(1); + + // Nothing was lost: B re-enters exactly once, after A's apply transitions back to Steady. + fetcher.Release(); + coordinator.ExpectMsg(TimeSpan.FromSeconds(10)).Outcome.ShouldBe(ApplyAckOutcome.Applied); + coordinator.ExpectMsg(TimeSpan.FromSeconds(10)).Outcome.ShouldBe(ApplyAckOutcome.Applied); + fetcher.Calls.ShouldBe(2); + } + finally + { + // Never leave the fetch parked: a blocked apply would hang ActorSystem shutdown. + fetcher.Release(); + } + } + + /// + /// The SAME dispatch arriving twice mid-apply is stashed ONCE — the stash is bounded by + /// distinct deployments, not by how often the coordinator re-dispatches. + /// + /// + /// + /// Verified RED against the pinned Akka (1.5.62): with the dedup disabled, both copies are + /// stashed and the deferral is announced twice. That is the failure this pins — a + /// coordinator that re-dispatches on a timer would otherwise grow the stash with retries + /// rather than with work, and every extra copy is re-delivered on the way back to Steady + /// only to short-circuit as a duplicate. + /// + /// + /// It is NOT a crash today, and the review's suggestion that it might be was checked rather + /// than assumed. AbstractStash.Stash() is documented to throw + /// IllegalActorStateException when an envelope equals the last stashed one, and that + /// message string is in the shipped assembly — but a probe actor on this package stashed the + /// same instance twice, and two value-equal instances, with and without an explicit sender, + /// and never threw. The RED above is a count, not an exception, for exactly that reason. + /// + /// + [Fact] + public void TheSameDispatchArrivingTwiceMidApply_IsStashedOnce_AndAppliedOnce() + { + var db = NewInMemoryDbFactory(); + var fetcher = new GatedFetcher(ArtifactBytes()); + var deploymentA = DeploymentId.NewId(); + var deploymentB = DeploymentId.NewId(); + + var coordinator = CreateTestProbe(); + var actor = Sys.ActorOf(DriverHostActor.Props( + db, TestNode, coordinator.Ref, + localRoles: new HashSet { "driver" }, + deploymentArtifactCache: new NullArtifactCache(), + fetchAndCacheMode: true, + artifactFetcher: fetcher)); + + try + { + actor.Tell(new DispatchDeployment(deploymentA, RevA, CorrelationId.NewId())); + AwaitAssert(() => fetcher.Calls.ShouldBe(1), duration: PresenceBudget); + + var logs = SubscribeToLogs("deferring", "already deferred"); + + // The IDENTICAL message object, twice — a record, so the two envelopes are value-equal. + var dispatchB = new DispatchDeployment(deploymentB, RevB, CorrelationId.NewId()); + actor.Tell(dispatchB); + actor.Tell(dispatchB); + + logs.DrainUntilAllSeen(PresenceBudget); + logs["deferring"].ShouldBe(1); + logs["already deferred"].ShouldBe(1); + + // No second stash, therefore no second re-delivery: exactly one Applied ack for B. + fetcher.Release(); + coordinator.ExpectMsg(TimeSpan.FromSeconds(10)).Outcome.ShouldBe(ApplyAckOutcome.Applied); + coordinator.ExpectMsg(TimeSpan.FromSeconds(10)).Outcome.ShouldBe(ApplyAckOutcome.Applied); + coordinator.ExpectNoMsg(TimeSpan.FromMilliseconds(500)); + fetcher.Calls.ShouldBe(2); + } + finally + { + fetcher.Release(); + } + } + + /// + /// Two successive deferrals apply in arrival order, and the second one proves the dedup set is + /// CLEARED on the way back to Steady rather than accumulating across applies. + /// + /// + /// C is stashed by B's apply, i.e. after A's apply already unstashed B and cleared the set. A + /// dedup set that was never cleared would still hold nothing for C, so what this really pins is + /// the re-stash CYCLE working at all — plus the ordering the coordinator depends on: the node + /// must end on C's revision, not on B's. + /// + [Fact] + public void ASecondDeferralAppliesAfterTheFirst_AndTheNodeEndsOnTheLastRevision() + { + var db = NewInMemoryDbFactory(); + // Gate the first TWO fetches so C can arrive while B is applying. + var fetcher = new GatedFetcher(ArtifactBytes(), gatedCalls: 2); + var deploymentA = DeploymentId.NewId(); + var deploymentB = DeploymentId.NewId(); + var deploymentC = DeploymentId.NewId(); + + var coordinator = CreateTestProbe(); + var actor = Sys.ActorOf(DriverHostActor.Props( + db, TestNode, coordinator.Ref, + localRoles: new HashSet { "driver" }, + deploymentArtifactCache: new NullArtifactCache(), + fetchAndCacheMode: true, + artifactFetcher: fetcher)); + + try + { + actor.Tell(new DispatchDeployment(deploymentA, RevA, CorrelationId.NewId())); + AwaitAssert(() => fetcher.Calls.ShouldBe(1), duration: PresenceBudget); + + actor.Tell(new DispatchDeployment(deploymentB, RevB, CorrelationId.NewId())); + + // A completes; B unstashes and starts its own (still gated) fetch. + fetcher.Release(1); + coordinator.ExpectMsg(TimeSpan.FromSeconds(10)).DeploymentId.ShouldBe(deploymentA); + AwaitAssert(() => fetcher.Calls.ShouldBe(2), duration: PresenceBudget); + + // C arrives mid-B — a fresh stash cycle, on a set that was cleared when B was released. + actor.Tell(new DispatchDeployment(deploymentC, RevC, CorrelationId.NewId())); + + fetcher.Release(2); + coordinator.ExpectMsg(TimeSpan.FromSeconds(10)).DeploymentId.ShouldBe(deploymentB); + coordinator.ExpectMsg(TimeSpan.FromSeconds(10)).DeploymentId.ShouldBe(deploymentC); + + actor.Tell(new GetDiagnostics(CorrelationId.NewId()), coordinator.Ref); + coordinator.ExpectMsg(TimeSpan.FromSeconds(5)) + .CurrentRevision.ShouldBe(RevC); + } + finally + { + fetcher.Release(); + } + } + + /// + /// A host wedged in Applying says so. The pre-stash forward-spin was accidentally the + /// only alarm for this — a flood of log lines nobody could miss — and removing it removed the + /// signal with it. The watchdog is the deliberate replacement. + /// + /// + /// The wedge delay is a constructor parameter (defaulting to + /// ), shortened here so the fire is + /// observable in a test's lifetime — the same seam every other interval on this actor uses. + /// Nothing is cancelled or failed by the fire, so the assertion is purely the Warning, and it + /// must NAME the wedged deployment or it is useless during an incident. + /// + [Fact] + public void AnApplyThatNeverCompletes_IsReportedAsWedged() + { + var db = NewInMemoryDbFactory(); + var fetcher = new GatedFetcher(ArtifactBytes()); + var deploymentA = DeploymentId.NewId(); + + var coordinator = CreateTestProbe(); + var actor = Sys.ActorOf(DriverHostActor.Props( + db, TestNode, coordinator.Ref, + localRoles: new HashSet { "driver" }, + deploymentArtifactCache: new NullArtifactCache(), + fetchAndCacheMode: true, + artifactFetcher: fetcher, + applyWedgeTimeout: TimeSpan.FromMilliseconds(300))); + + try + { + EventFilter.Warning(contains: deploymentA.ToString()).ExpectOne(TimeSpan.FromSeconds(10), () => + actor.Tell(new DispatchDeployment(deploymentA, RevA, CorrelationId.NewId()))); + } + finally + { + fetcher.Release(); + } + } + + /// + /// A completed apply does NOT report a wedge, however short the delay — the timer is cancelled + /// on the way back to Steady, and a fire already in the mailbox drops at Debug. + /// + /// + /// The falsification control for the test above: a watchdog that fires unconditionally would + /// pass it and would then cry wedge after every ordinary deployment. + /// + [Fact] + public void ACompletedApply_NeverReportsAWedge() + { + var db = NewInMemoryDbFactory(); + var deploymentA = DeploymentId.NewId(); + + var coordinator = CreateTestProbe(); + var actor = Sys.ActorOf(DriverHostActor.Props( + db, TestNode, coordinator.Ref, + localRoles: new HashSet { "driver" }, + deploymentArtifactCache: new NullArtifactCache(), + fetchAndCacheMode: true, + // Ungated: the fetch answers immediately. + artifactFetcher: new GatedFetcher(ArtifactBytes(), gatedCalls: 0), + applyWedgeTimeout: TimeSpan.FromMilliseconds(200))); + + var logs = SubscribeToLogs("appears WEDGED"); + actor.Tell(new DispatchDeployment(deploymentA, RevA, CorrelationId.NewId())); + coordinator.ExpectMsg(TimeSpan.FromSeconds(10)).Outcome.ShouldBe(ApplyAckOutcome.Applied); + + // Well past the (200 ms) delay — a live timer would have fired several times over. + logs.Drain(SpinWindow); + logs["appears WEDGED"].ShouldBe(0); + } + + /// Subscribes a fresh counter to the EventStream's log events. + /// + /// Every fragment of interest must be declared up front, because draining is destructive: a + /// probe read for one fragment discards every other log event it passes over, so counting two + /// fragments with two independent drains would always score zero on the second. + /// + private LogCounter SubscribeToLogs(params string[] fragments) + { + var probe = CreateTestProbe(); + Sys.EventStream.Subscribe(probe.Ref, typeof(LogEvent)); + return new LogCounter(probe, fragments); + } + + /// Counts occurrences of a fixed set of log-message fragments off one EventStream probe. + private sealed class LogCounter(Akka.TestKit.TestProbe logs, string[] fragments) + { + private readonly Dictionary _counts = + fragments.ToDictionary(f => f, _ => 0, StringComparer.Ordinal); + + public int this[string fragment] => _counts[fragment]; + + /// Drains the probe for a fixed window, tallying every tracked fragment. + public void Drain(TimeSpan window) + { + var deadline = DateTime.UtcNow + window; + while (DateTime.UtcNow < deadline) + { + if (logs.ReceiveOne(TimeSpan.FromMilliseconds(100)) is LogEvent e && + e.Message?.ToString() is { } text) + { + foreach (var fragment in fragments) + if (text.Contains(fragment, StringComparison.Ordinal)) + _counts[fragment]++; + } + } + } + + /// Drains until every tracked fragment has been seen at least once, or the budget expires. + public void DrainUntilAllSeen(TimeSpan budget) + { + var deadline = DateTime.UtcNow + budget; + while (DateTime.UtcNow < deadline && _counts.Values.Any(n => n == 0)) + Drain(TimeSpan.FromMilliseconds(200)); + } + } + + /// + /// Gates the first gatedCalls fetches on the test; later fetches complete immediately. + /// + private sealed class GatedFetcher(byte[] bytes, int gatedCalls = 1) : IDeploymentArtifactFetcher + { + private readonly TaskCompletionSource[] _gates = Enumerable.Range(0, gatedCalls) + .Select(_ => new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously)) + .ToArray(); + + private int _calls; + + public int Calls => Volatile.Read(ref _calls); + + /// Releases every gate — also the test-teardown safety net. + public void Release() + { + foreach (var gate in _gates) gate.TrySetResult(); + } + + /// Releases the gate on the -th fetch (1-based). + public void Release(int call) => _gates[call - 1].TrySetResult(); + + public async Task FetchAsync(string deploymentId, string expectedRevisionHash, CancellationToken ct) + { + var call = Interlocked.Increment(ref _calls); + if (call <= _gates.Length) + await _gates[call - 1].Task.ConfigureAwait(false); + return bytes; + } + } + + /// Accepts every store; this class asserts on dispatch handling, not on caching. + private sealed class NullArtifactCache : IDeploymentArtifactCache + { + public Task StoreAsync(string clusterId, string deploymentId, string revisionHash, + byte[] artifact, CancellationToken ct = default) + => Task.CompletedTask; + + public Task GetCurrentAsync(string clusterId, CancellationToken ct = default) + => Task.FromResult(null); + + public Task GetCurrentUnkeyedAsync(CancellationToken ct = default) + => Task.FromResult(null); + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorAsyncCacheWriteTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorAsyncCacheWriteTests.cs new file mode 100644 index 00000000..a0857409 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorAsyncCacheWriteTests.cs @@ -0,0 +1,185 @@ +using System.Text.Json; +using Akka.Actor; +using Akka.Event; +using Microsoft.EntityFrameworkCore; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Commons.Interfaces; +using ZB.MOM.WW.OtOpcUa.Commons.Messages.Deploy; +using ZB.MOM.WW.OtOpcUa.Commons.Messages.Fleet; +using ZB.MOM.WW.OtOpcUa.Commons.Types; +using ZB.MOM.WW.OtOpcUa.Configuration; +using ZB.MOM.WW.OtOpcUa.Configuration.Entities; +using ZB.MOM.WW.OtOpcUa.Configuration.Enums; +using ZB.MOM.WW.OtOpcUa.Runtime.DeploymentCache; +using ZB.MOM.WW.OtOpcUa.Runtime.Drivers; +using ZB.MOM.WW.OtOpcUa.Runtime.Tests.Harness; + +namespace ZB.MOM.WW.OtOpcUa.Runtime.Tests.Drivers; + +/// +/// Surgical-deploys Task 7 Part A — the artifact-cache write is I/O (hundreds of chunk inserts +/// into LocalDb) and must not run on the actor thread. +/// +/// +/// +/// The ACK was never the thing at risk: ApplyAndAck sends it BEFORE it caches, so a +/// blocking store still acked promptly. What it blocked was everything after — the host +/// could not dequeue a driver value burst, a diagnostics request or the next dispatch until +/// SQLite was done. So the load-bearing assertion here is that the actor answers an unrelated +/// message while the store is still in flight, not that the ACK arrived. +/// +/// +/// Runs at INFO so the completion self-message's log line is observable — the pipe-back +/// is private, and its visible effect is that line. +/// +/// +public sealed class DriverHostActorAsyncCacheWriteTests() + : RuntimeActorTestBase(AkkaTestHoconAtLevel("INFO")) +{ + private static readonly NodeId TestNode = NodeId.Parse("driver-test"); + private static readonly RevisionHash RevA = RevisionHash.Parse(new string('a', 64)); + + [Fact] + public void AStillRunningCacheStore_DoesNotBlockTheHostMailbox() + { + var db = NewInMemoryDbFactory(); + var cache = new GatedArtifactCache(); + var deploymentId = SeedDeployment(db, RevA); + + var coordinator = CreateTestProbe(); + var actor = Sys.ActorOf(DriverHostActor.Props( + db, TestNode, coordinator.Ref, + localRoles: new HashSet { "driver" }, + deploymentArtifactCache: cache)); + + try + { + actor.Tell(new DispatchDeployment(deploymentId, RevA, CorrelationId.NewId())); + coordinator.ExpectMsg(TimeSpan.FromSeconds(5)).Outcome.ShouldBe(ApplyAckOutcome.Applied); + + // The store was entered and has NOT completed — the gate is still closed. + AwaitAssert(() => cache.StoresEntered.ShouldBe(1), duration: PresenceBudget); + cache.StoresCompleted.ShouldBe(0); + + // THE assertion: with the store still in flight the host is still processing its mailbox. + // A blocking `.GetAwaiter().GetResult()` parks the actor thread here and this times out. + actor.Tell(new GetDiagnostics(CorrelationId.NewId()), coordinator.Ref); + coordinator.ExpectMsg(TimeSpan.FromSeconds(3)); + + // Completion is observed rather than fire-and-forgotten: the store pipes back to Self and + // the handler logs it, so an operator can tell a cached apply from an uncached one. + EventFilter.Info(contains: "cached applied deployment") + .ExpectOne(TimeSpan.FromSeconds(5), () => cache.Release()); + } + finally + { + // Never leave the fake gated: a parked store would hang ActorSystem shutdown. + cache.Release(); + } + } + + [Fact] + public void AFailingCacheStore_WarnsAndLeavesTheApplyApplied() + { + // The cache is a boot fallback, not part of the deployment protocol. A failed store must + // surface as a Warning and nothing else — emphatically NOT as a second, contradictory Failed + // ACK for a deployment the fleet already believes is live. + var db = NewInMemoryDbFactory(); + var deploymentId = SeedDeployment(db, RevA); + + var coordinator = CreateTestProbe(); + var actor = Sys.ActorOf(DriverHostActor.Props( + db, TestNode, coordinator.Ref, + localRoles: new HashSet { "driver" }, + deploymentArtifactCache: new AsyncFailingArtifactCache())); + + // Matched on the EXCEPTION, not just the prose: the pipe-back carries the Exception itself and + // the handler logs it as the event's Cause. A SQLite failure is routinely a wrapped inner + // exception whose outer Message names neither the result code nor the failing frame, so a + // Reason-string-only carrier would have thrown that away before the log line was written. + // (Custom rather than EventFilter.Exception, which only ever matches ERROR-level events — + // a failed cache write is deliberately a Warning, since the apply itself succeeded.) + EventFilter.Custom(e => + e.Cause is InvalidOperationException && + e.Message?.ToString()?.Contains("could not be cached", StringComparison.Ordinal) == true) + .ExpectOne(TimeSpan.FromSeconds(5), () => + { + actor.Tell(new DispatchDeployment(deploymentId, RevA, CorrelationId.NewId())); + coordinator.ExpectMsg(TimeSpan.FromSeconds(5)).Outcome.ShouldBe(ApplyAckOutcome.Applied); + }); + + coordinator.ExpectNoMsg(TimeSpan.FromMilliseconds(300)); + } + + /// Seeds a sealed, parseable deployment and returns its id. + private static DeploymentId SeedDeployment(IDbContextFactory db, RevisionHash rev) + { + var artifact = JsonSerializer.SerializeToUtf8Bytes(new { DriverInstances = Array.Empty() }); + var id = DeploymentId.NewId(); + using var ctx = db.CreateDbContext(); + ctx.Deployments.Add(new Deployment + { + DeploymentId = id.Value, + RevisionHash = rev.Value, + Status = DeploymentStatus.Sealed, + CreatedBy = "test", + SealedAtUtc = DateTime.UtcNow, + ArtifactBlob = artifact, + }); + ctx.SaveChanges(); + return id; + } + + /// + /// A store that never completes until the test releases it — the fake shape of a slow LocalDb + /// write. It must not complete synchronously, or it would prove nothing about the mailbox. + /// + private sealed class GatedArtifactCache : IDeploymentArtifactCache + { + private readonly TaskCompletionSource _gate = + new(TaskCreationOptions.RunContinuationsAsynchronously); + private int _entered; + private int _completed; + + public int StoresEntered => Volatile.Read(ref _entered); + public int StoresCompleted => Volatile.Read(ref _completed); + + public void Release() => _gate.TrySetResult(); + + public async Task StoreAsync(string clusterId, string deploymentId, string revisionHash, + byte[] artifact, CancellationToken ct = default) + { + Interlocked.Increment(ref _entered); + await _gate.Task.ConfigureAwait(false); + Interlocked.Increment(ref _completed); + } + + public Task GetCurrentAsync(string clusterId, CancellationToken ct = default) + => Task.FromResult(null); + + public Task GetCurrentUnkeyedAsync(CancellationToken ct = default) + => Task.FromResult(null); + } + + /// + /// Fails the store ASYNCHRONOUSLY (a faulted task), which is how a real LocalDb write fails — + /// as distinct from DriverHostActorArtifactCacheTests's synchronously-throwing fake, + /// which exercises the other half of the guard. + /// + private sealed class AsyncFailingArtifactCache : IDeploymentArtifactCache + { + public async Task StoreAsync(string clusterId, string deploymentId, string revisionHash, + byte[] artifact, CancellationToken ct = default) + { + await Task.Yield(); + throw new InvalidOperationException("disk full"); + } + + public Task GetCurrentAsync(string clusterId, CancellationToken ct = default) + => Task.FromResult(null); + + public Task GetCurrentUnkeyedAsync(CancellationToken ct = default) + => Task.FromResult(null); + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorTests.cs index 8f192617..a712a339 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorTests.cs @@ -8,6 +8,7 @@ using ZB.MOM.WW.OtOpcUa.Commons.Types; using ZB.MOM.WW.OtOpcUa.Configuration; using ZB.MOM.WW.OtOpcUa.Configuration.Enums; using ZB.MOM.WW.OtOpcUa.Runtime.Drivers; +using ZB.MOM.WW.OtOpcUa.Runtime.OpcUa; using ZB.MOM.WW.OtOpcUa.Runtime.ScriptedAlarms; using ZB.MOM.WW.OtOpcUa.Runtime.Tests.Harness; @@ -199,6 +200,47 @@ public sealed class DriverHostActorTests : RuntimeActorTestBase plan.PredicateSource.ShouldContain("ctx.GetTag"); } + /// Verifies that a Direct-mode apply hands the artifact bytes it has already read to the + /// address-space rebuild instead of leaving the publish actor to re-read the same row from central SQL. + /// + /// The Direct apply reads Deployment.ArtifactBlob once (ReconcileDrivers returns it as + /// appliedBlob) and then used to Tell RebuildAddressSpace with NO bytes — so + /// OpcUaPublishActor.HandleRebuild re-read the row, and PushDesiredSubscriptions read it + /// a third time: three central-SQL reads of one immutable blob per apply. Carrying the in-hand bytes + /// collapses that to one. The FetchAndCache path has always been shaped this way, because it has no + /// ConfigDb to re-read from — this asserts Direct mode now matches it. Byte-equality (rather than + /// merely non-null) is the assertion that matters: a rebuild fed the WRONG artifact would materialise + /// a configuration nobody deployed. + /// + [Fact] + public void Direct_mode_apply_hands_the_already_read_artifact_to_the_rebuild() + { + var db = NewInMemoryDbFactory(); + var deploymentId = V3ArtifactSeeder.SeedV3Deployment(db, RevA, + rawTags: new[] { (Driver: "drv-1", DriverName: "Modbus", Device: "dev1", Tag: "speed", DataType: "Double", Writable: false) }, + unsRefs: new[] { (Area: "filling", Line: "line1", Equip: "station1", BackingTag: "speed", Effective: (string?)null) }); + + byte[] seeded; + using (var seedCtx = db.CreateDbContext()) + seeded = seedCtx.Deployments.Single(d => d.DeploymentId == deploymentId.Value).ArtifactBlob!; + seeded.Length.ShouldBeGreaterThan(0); + + var coordinator = CreateTestProbe(); + var publish = CreateTestProbe(); + var actor = Sys.ActorOf(DriverHostActor.Props( + db, TestNode, coordinator.Ref, + localRoles: new HashSet { "driver" }, + opcUaPublishActor: publish.Ref)); + + actor.Tell(new DispatchDeployment(deploymentId, RevA, CorrelationId.NewId())); + coordinator.ExpectMsg(PresenceBudget).Outcome.ShouldBe(ApplyAckOutcome.Applied); + + var rebuild = publish.ExpectMsg(PresenceBudget); + rebuild.DeploymentId.ShouldBe(deploymentId); + rebuild.Artifact.ShouldNotBeNull(); + rebuild.Artifact.ShouldBe(seeded); + } + private static DeploymentId SeedDeploymentWithScriptedAlarm( IDbContextFactory db, RevisionHash rev) { diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/ParsedDeploymentArtifactTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/ParsedDeploymentArtifactTests.cs new file mode 100644 index 00000000..4c43610b --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/ParsedDeploymentArtifactTests.cs @@ -0,0 +1,491 @@ +using System.IO.Compression; +using System.Text; +using System.Text.Json; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Authorization; +using ZB.MOM.WW.OtOpcUa.OpcUaServer; +using ZB.MOM.WW.OtOpcUa.Runtime.Drivers; + +namespace ZB.MOM.WW.OtOpcUa.Runtime.Tests.Drivers; + +/// +/// Parity coverage for / — +/// the "parse the artifact JSON once per apply" seam. Every public static entry point on +/// is now a thin wrapper over the instance method of the same name, +/// so the ONLY thing that must be proved is that the two produce identical output for the same bytes. +/// +/// +/// +/// These are not "does the parser work" tests — that is what the eight existing +/// DeploymentArtifact*Tests suites are for, and they now exercise the instance path through +/// the wrappers. These pin the refactor itself: same bytes ⇒ same output through either door. +/// +/// +/// Record equality is NOT usable for the composition/projection results: their members are +/// IReadOnlyList/IReadOnlyDictionary, which records compare by REFERENCE, so two +/// separately-produced-but-identical results are never ==. Every assertion here therefore +/// compares element-wise (the element types are records with scalar members, which do compare by +/// value). +/// +/// +public sealed class ParsedDeploymentArtifactTests +{ + private const string NodeC1 = "c1-1:4053"; + private const string NodeC2 = "c2-1:4053"; + private const string NodeUnlisted = "stranger:4053"; + + /// + /// A deliberately rich MULTI-CLUSTER artifact: two clusters + their node rows, the full raw chain + /// (folder → driver → device → tag group → tags, including a historized tag, an array tag and a + /// native-alarm tag), the UNS chain (area → line → equipment) with a tag reference carrying a + /// display-name override, scripts joined by both a VirtualTag and a ScriptedAlarm (with a + /// {{equip}} reference so the equipment-reference map is genuinely exercised), plus ApiKeys + /// and NodeAcls so the data-plane projection has something to scope. + /// + private const string Artifact = """ + { + "Clusters": [{"ClusterId":"C1"},{"ClusterId":"C2"}], + "Nodes": [ + {"NodeId":"c1-1:4053","ClusterId":"C1"}, + {"NodeId":"c2-1:4053","ClusterId":"C2"} + ], + "RawFolders": [ + {"RawFolderId":"RF-plant","ClusterId":"C1","ParentRawFolderId":null,"Name":"Plant"}, + {"RawFolderId":"RF-line1","ClusterId":"C1","ParentRawFolderId":"RF-plant","Name":"Line1"}, + {"RawFolderId":"RF-c2","ClusterId":"C2","ParentRawFolderId":null,"Name":"Site2"} + ], + "DriverInstances": [ + {"DriverInstanceRowId":"11111111-1111-1111-1111-111111111111","DriverInstanceId":"DI-modbus","ClusterId":"C1","RawFolderId":"RF-line1","Name":"ModbusDrv","DriverType":"Modbus","Enabled":true,"DriverConfig":"{\"unitId\":1}","ResilienceConfig":"{\"bulkheadMaxConcurrent\":16}"}, + {"DriverInstanceRowId":"22222222-2222-2222-2222-222222222222","DriverInstanceId":"DI-s7","ClusterId":"C2","RawFolderId":"RF-c2","Name":"S7Drv","DriverType":"S7","Enabled":true,"DriverConfig":"{}"} + ], + "Devices": [ + {"DeviceId":"DV-1","DriverInstanceId":"DI-modbus","Name":"dev1","DeviceConfig":"{\"endpoint\":\"10.0.0.1:502\"}"}, + {"DeviceId":"DV-2","DriverInstanceId":"DI-s7","Name":"dev2","DeviceConfig":"{}"} + ], + "TagGroups": [{"TagGroupId":"TG-fast","DeviceId":"DV-1","ParentTagGroupId":null,"Name":"Fast"}], + "Tags": [ + {"TagId":"TAG-speed","DeviceId":"DV-1","TagGroupId":"TG-fast","Name":"Speed","DataType":"Float","AccessLevel":1,"WriteIdempotent":true,"TagConfig":"{\"isHistorized\":true,\"historianTagname\":\"WW.Speed\"}"}, + {"TagId":"TAG-run","DeviceId":"DV-1","TagGroupId":null,"Name":"Run","DataType":"Boolean","AccessLevel":0,"TagConfig":"{}"}, + {"TagId":"TAG-profile","DeviceId":"DV-1","TagGroupId":"TG-fast","Name":"Profile","DataType":"Float","AccessLevel":0,"TagConfig":"{\"isArray\":true,\"arrayLength\":16}"}, + {"TagId":"TAG-temphi","DeviceId":"DV-1","TagGroupId":null,"Name":"TempHi","DataType":"Boolean","AccessLevel":1,"TagConfig":"{\"alarm\":{\"alarmType\":\"OffNormalAlarm\",\"severity\":700}}"}, + {"TagId":"TAG-c2","DeviceId":"DV-2","TagGroupId":null,"Name":"Pressure","DataType":"Float","AccessLevel":0,"TagConfig":"{}"} + ], + "UnsAreas": [ + {"UnsAreaId":"UA-w","ClusterId":"C1","Name":"West"}, + {"UnsAreaId":"UA-e","ClusterId":"C2","Name":"East"} + ], + "UnsLines": [ + {"UnsLineId":"UL-1","UnsAreaId":"UA-w","Name":"L1"}, + {"UnsLineId":"UL-2","UnsAreaId":"UA-e","Name":"L2"} + ], + "Equipment": [ + {"EquipmentId":"EQ-mixer","UnsLineId":"UL-1","Name":"Mixer","MachineCode":"MX1"}, + {"EquipmentId":"EQ-press","UnsLineId":"UL-2","Name":"Press","MachineCode":"PR1"} + ], + "UnsTagReferences": [ + {"UnsTagReferenceId":"REF-1","EquipmentId":"EQ-mixer","TagId":"TAG-speed","DisplayNameOverride":"MixerSpeed"}, + {"UnsTagReferenceId":"REF-2","EquipmentId":"EQ-mixer","TagId":"TAG-run","DisplayNameOverride":null} + ], + "Scripts": [ + {"ScriptId":"SCR-vt","SourceCode":"return ctx.GetTag(\"{{equip}}/MixerSpeed\") * 2;"}, + {"ScriptId":"SCR-alarm","SourceCode":"return ctx.GetTag(\"{{equip}}/Run\") == false;"} + ], + "VirtualTags": [ + {"VirtualTagId":"VT-1","EquipmentId":"EQ-mixer","Name":"DoubleSpeed","DataType":"Float","ScriptId":"SCR-vt","Historize":true} + ], + "ScriptedAlarms": [ + {"ScriptedAlarmId":"SA-1","EquipmentId":"EQ-mixer","Name":"Stopped","AlarmType":"OffNormalAlarm","Severity":500,"MessageTemplate":"Mixer stopped","PredicateScriptId":"SCR-alarm","HistorizeToAveva":true,"Retain":true,"Enabled":true} + ], + "ApiKeys": [ + {"ApiKeyId":"k1","Name":"key-one","SecretHash":"hash-one","Principal":"p","Enabled":true,"CreatedUtc":"2026-01-01T00:00:00","ExpiresUtc":"2030-06-15T12:00:00"}, + {"ApiKeyId":"k2","Name":"key-two","SecretHash":"hash-two","Principal":"anonymous","Enabled":false,"CreatedUtc":"2026-01-01T00:00:00"} + ], + "NodeAcls": [ + {"NodeAclId":"acl-dev","ClusterId":"C1","Principal":"p","ScopeKind":9,"ScopeId":"DV-1","PermissionFlags":2}, + {"NodeAclId":"acl-eq","ClusterId":"C1","Principal":"p","ScopeKind":4,"ScopeId":"EQ-mixer","PermissionFlags":16}, + {"NodeAclId":"acl-c2","ClusterId":"C2","Principal":"p","ScopeKind":4,"ScopeId":"EQ-press","PermissionFlags":16} + ] + } + """; + + private static byte[] Blob => Encoding.UTF8.GetBytes(Artifact); + + [Theory] + [InlineData(NodeC1)] + [InlineData(NodeC2)] + [InlineData(NodeUnlisted)] + public void ResolveClusterScope_is_identical_through_the_parsed_artifact(string nodeId) + { + var blob = Blob; + using var parsed = DeploymentArtifact.Open(blob); + + parsed.ResolveClusterScope(nodeId).ShouldBe(DeploymentArtifact.ResolveClusterScope(blob, nodeId)); + } + + [Fact] + public void The_scope_fixture_actually_distinguishes_its_nodes() + { + // Falsifiability control for the parity theory above: if every node resolved to the same scope, + // a memoization cache keyed on nothing at all would still pass it. + var blob = Blob; + + DeploymentArtifact.ResolveClusterScope(blob, NodeC1) + .ShouldBe(new ClusterScope(ClusterFilterMode.ScopeTo, "C1")); + DeploymentArtifact.ResolveClusterScope(blob, NodeC2) + .ShouldBe(new ClusterScope(ClusterFilterMode.ScopeTo, "C2")); + DeploymentArtifact.ResolveClusterScope(blob, NodeUnlisted) + .ShouldBe(new ClusterScope(ClusterFilterMode.Suppress, null)); + } + + [Fact] + public void ParseDriverInstances_unscoped_is_identical_through_the_parsed_artifact() + { + var blob = Blob; + using var parsed = DeploymentArtifact.Open(blob); + + var expected = DeploymentArtifact.ParseDriverInstances(blob); + expected.Count.ShouldBe(2); // sanity: an empty-vs-empty comparison proves nothing + + parsed.ParseDriverInstances().ShouldBe(expected); + } + + [Theory] + [InlineData(NodeC1)] + [InlineData(NodeC2)] + [InlineData(NodeUnlisted)] + public void ParseDriverInstances_scoped_is_identical_through_the_parsed_artifact(string nodeId) + { + var blob = Blob; + using var parsed = DeploymentArtifact.Open(blob); + + parsed.ParseDriverInstances(nodeId).ShouldBe(DeploymentArtifact.ParseDriverInstances(blob, nodeId)); + } + + [Fact] + public void The_scoped_driver_fixture_actually_filters() + { + var blob = Blob; + + DeploymentArtifact.ParseDriverInstances(blob, NodeC1) + .Select(s => s.DriverInstanceId).ShouldBe(new[] { "DI-modbus" }); + DeploymentArtifact.ParseDriverInstances(blob, NodeC2) + .Select(s => s.DriverInstanceId).ShouldBe(new[] { "DI-s7" }); + DeploymentArtifact.ParseDriverInstances(blob, NodeUnlisted).ShouldBeEmpty(); + } + + [Theory] + [InlineData(NodeC1)] + [InlineData(NodeC2)] + [InlineData(NodeUnlisted)] + public void ParseDataPlaneAuthority_is_identical_through_the_parsed_artifact(string nodeId) + { + var blob = Blob; + using var parsed = DeploymentArtifact.Open(blob); + + AssertSameAuthority(parsed.ParseDataPlaneAuthority(nodeId), + DeploymentArtifact.ParseDataPlaneAuthority(blob, nodeId)); + } + + [Fact] + public void The_authority_fixture_carries_keys_grants_and_resolved_scope_paths() + { + // Sanity on the parity theory above: it compares three collections, and three empties compare equal. + var projection = DeploymentArtifact.ParseDataPlaneAuthority(Blob, NodeC1); + + projection.ClusterId.ShouldBe("C1"); + projection.Keys.Select(k => k.ApiKeyId).ShouldBe(new[] { "k1", "k2" }); + projection.Grants.Select(g => g.NodeAclId).ShouldBe(new[] { "acl-dev", "acl-eq" }); + projection.ScopePaths.ShouldNotBeEmpty(); + } + + [Fact] + public void ParseComposition_unscoped_is_identical_through_the_parsed_artifact() + { + var blob = Blob; + using var parsed = DeploymentArtifact.Open(blob); + + var expected = DeploymentArtifact.ParseComposition(blob); + expected.RawTags.ShouldNotBeEmpty(); + expected.UnsReferenceVariables.ShouldNotBeEmpty(); + expected.EquipmentVirtualTags.ShouldNotBeEmpty(); + expected.EquipmentScriptedAlarms.ShouldNotBeEmpty(); + + AssertSameComposition(parsed.ParseComposition(), expected); + } + + [Theory] + [InlineData(NodeC1)] + [InlineData(NodeC2)] + [InlineData(NodeUnlisted)] + public void ParseComposition_scoped_is_identical_through_the_parsed_artifact(string nodeId) + { + var blob = Blob; + using var parsed = DeploymentArtifact.Open(blob); + + AssertSameComposition(parsed.ParseComposition(nodeId), DeploymentArtifact.ParseComposition(blob, nodeId)); + } + + [Fact] + public void The_scoped_composition_fixture_actually_filters() + { + // BuildClusterSets is the piece this refactor de-duplicated (it used to re-parse the blob), so the + // scoped composition has to keep filtering rather than degrade to "everything" or "nothing". + var blob = Blob; + + var c1 = DeploymentArtifact.ParseComposition(blob, NodeC1); + c1.UnsAreas.Select(a => a.UnsAreaId).ShouldBe(new[] { "UA-w" }); + c1.EquipmentNodes.Select(e => e.EquipmentId).ShouldBe(new[] { "EQ-mixer" }); + c1.RawTags.Select(t => t.DriverInstanceId).Distinct().ShouldBe(new[] { "DI-modbus" }); + c1.UnsReferenceVariables.Select(v => v.EquipmentId).Distinct().ShouldBe(new[] { "EQ-mixer" }); + + var c2 = DeploymentArtifact.ParseComposition(blob, NodeC2); + c2.UnsAreas.Select(a => a.UnsAreaId).ShouldBe(new[] { "UA-e" }); + c2.EquipmentNodes.Select(e => e.EquipmentId).ShouldBe(new[] { "EQ-press" }); + + DeploymentArtifact.ParseComposition(blob, NodeUnlisted).EquipmentNodes.ShouldBeEmpty(); + } + + [Fact] + public void The_scoped_composition_inconsistency_callback_fires_identically_through_both_doors() + { + var blob = Blob; + using var parsed = DeploymentArtifact.Open(blob); + + var viaStatic = new List(); + DeploymentArtifact.ParseComposition(blob, NodeC1, viaStatic.Add); + var viaInstance = new List(); + parsed.ParseComposition(NodeC1, viaInstance.Add); + + viaInstance.ShouldBe(viaStatic); + } + + [Fact] + public void A_second_call_on_the_same_parsed_artifact_returns_equal_results() + { + // Memoization sanity: ResolveClusterScope is cached per node id, and the cache must not leak the + // first caller's answer to a different node — nor stale the same caller's answer. + using var parsed = DeploymentArtifact.Open(Blob); + + var firstC1 = parsed.ResolveClusterScope(NodeC1); + var firstC2 = parsed.ResolveClusterScope(NodeC2); + parsed.ResolveClusterScope(NodeC1).ShouldBe(firstC1); + parsed.ResolveClusterScope(NodeC2).ShouldBe(firstC2); + firstC1.ShouldNotBe(firstC2); + + parsed.ParseDriverInstances().ShouldBe(parsed.ParseDriverInstances()); + parsed.ParseDriverInstances(NodeC1).ShouldBe(parsed.ParseDriverInstances(NodeC1)); + AssertSameAuthority(parsed.ParseDataPlaneAuthority(NodeC1), parsed.ParseDataPlaneAuthority(NodeC1)); + AssertSameComposition(parsed.ParseComposition(), parsed.ParseComposition()); + AssertSameComposition(parsed.ParseComposition(NodeC1), parsed.ParseComposition(NodeC1)); + } + + [Fact] + public void Results_survive_disposing_the_parsed_artifact() + { + // Everything returned is fully materialised — no JsonElement is retained, so a caller that disposes + // the artifact the moment the apply's parsing is done cannot be handed a use-after-dispose. + AddressSpaceComposition composition; + DataPlaneAuthorityProjection authority; + IReadOnlyList specs; + + using (var parsed = DeploymentArtifact.Open(Blob)) + { + composition = parsed.ParseComposition(NodeC1); + authority = parsed.ParseDataPlaneAuthority(NodeC1); + specs = parsed.ParseDriverInstances(NodeC1); + } + + // Touch every collection AFTER disposal — a retained JsonElement would throw ObjectDisposedException. + composition.RawTags.Select(t => t.NodeId).ShouldNotBeEmpty(); + composition.UnsReferenceVariables.Select(v => v.NodeId).ShouldNotBeEmpty(); + composition.EquipmentVirtualTags.Select(v => v.Expression).ShouldNotBeEmpty(); + composition.EquipmentScriptedAlarms.Select(a => a.PredicateSource).ShouldNotBeEmpty(); + authority.Keys.Select(k => k.SecretHash).ShouldNotBeEmpty(); + authority.ScopePaths.SelectMany(kv => kv.Value).Select(p => p.Segments.Count).ShouldNotBeEmpty(); + specs.Select(s => s.DriverConfig).ShouldNotBeEmpty(); + } + + [Fact] + public void Disposing_twice_is_harmless() + { + var parsed = DeploymentArtifact.Open(Blob); + parsed.Dispose(); + Should.NotThrow(() => parsed.Dispose()); + } + + [Fact] + public void A_malformed_blob_still_degrades_leniently_through_every_static_wrapper() + { + // The statics own the lenient contract (empty list / empty composition / None scope) — Open() itself + // throws, because a caller that opens an artifact wants to know the bytes were unusable. + var bad = Encoding.UTF8.GetBytes("{ this is not json"); + + DeploymentArtifact.ParseDriverInstances(bad).ShouldBeEmpty(); + DeploymentArtifact.ParseDriverInstances(bad, NodeC1).ShouldBeEmpty(); + DeploymentArtifact.ResolveClusterScope(bad, NodeC1).Mode.ShouldBe(ClusterFilterMode.None); + DeploymentArtifact.ParseDataPlaneAuthority(bad, NodeC1).Keys.ShouldBeEmpty(); + DeploymentArtifact.ParseComposition(bad).EquipmentNodes.ShouldBeEmpty(); + DeploymentArtifact.ParseComposition(bad, NodeC1).EquipmentNodes.ShouldBeEmpty(); + } + + [Fact] + public void An_empty_blob_still_degrades_leniently_through_every_static_wrapper() + { + var empty = Array.Empty(); + + DeploymentArtifact.ParseDriverInstances(empty).ShouldBeEmpty(); + DeploymentArtifact.ParseDriverInstances(empty, NodeC1).ShouldBeEmpty(); + DeploymentArtifact.ResolveClusterScope(empty, NodeC1).Mode.ShouldBe(ClusterFilterMode.None); + DeploymentArtifact.ParseDataPlaneAuthority(empty, NodeC1).ShouldBe(DataPlaneAuthorityProjection.Empty); + DeploymentArtifact.ParseComposition(empty).EquipmentNodes.ShouldBeEmpty(); + DeploymentArtifact.ParseComposition(empty, NodeC1).EquipmentNodes.ShouldBeEmpty(); + } + + [Fact] + public void Open_reads_gzip_compressed_artifacts() + { + // Task 8: ConfigComposer now gzips the artifact BEFORE hashing it, so every blob that reaches a node + // (SQL row, LocalDb cache chunk, gRPC fetch) is gzip. Decompression lives in exactly one place — + // Open() — so this parity assertion covers every static wrapper and every consumer at once. + var plain = Blob; + var gzipped = Gzip(plain); + + gzipped[0].ShouldBe((byte)0x1F); + gzipped[1].ShouldBe((byte)0x8B); + gzipped.Length.ShouldBeLessThan(plain.Length); // the whole point + + using var parsedGzip = DeploymentArtifact.Open(gzipped); + using var parsedPlain = DeploymentArtifact.Open(plain); + + parsedGzip.ResolveClusterScope(NodeC1).ShouldBe(parsedPlain.ResolveClusterScope(NodeC1)); + parsedGzip.ParseDriverInstances().ShouldBe(parsedPlain.ParseDriverInstances()); + parsedGzip.ParseDriverInstances(NodeC1).ShouldBe(parsedPlain.ParseDriverInstances(NodeC1)); + AssertSameAuthority(parsedGzip.ParseDataPlaneAuthority(NodeC1), parsedPlain.ParseDataPlaneAuthority(NodeC1)); + AssertSameComposition(parsedGzip.ParseComposition(), parsedPlain.ParseComposition()); + AssertSameComposition(parsedGzip.ParseComposition(NodeC1), parsedPlain.ParseComposition(NodeC1)); + + // …and through the statics, which is what most call sites still use. + DeploymentArtifact.ParseDriverInstances(gzipped, NodeC1) + .ShouldBe(DeploymentArtifact.ParseDriverInstances(plain, NodeC1)); + AssertSameComposition( + DeploymentArtifact.ParseComposition(gzipped, NodeC1), + DeploymentArtifact.ParseComposition(plain, NodeC1)); + } + + [Fact] + public void Open_still_reads_uncompressed_artifacts() + { + // Backward compatibility: every pre-upgrade Deployment row (and every cached artifact chunk written + // before the upgrade) holds raw JSON. A JSON artifact can never start with 0x1F, so the magic sniff + // is unambiguous and these bytes take the identical path they always did. + using var parsed = DeploymentArtifact.Open(Blob); + + parsed.ParseDriverInstances().Select(s => s.DriverInstanceId).ShouldBe(new[] { "DI-modbus", "DI-s7" }); + parsed.ParseComposition(NodeC1).RawTags.ShouldNotBeEmpty(); + } + + [Fact] + public void Static_wrappers_stay_lenient_on_corrupt_gzip() + { + // Corrupt/truncated gzip must degrade exactly like malformed JSON does — the decompressor's + // InvalidDataException is translated to JsonException so the wrappers' existing lenient catch + // (and every apply path's failure handling) covers it unchanged. + var truncated = Gzip(Blob)[..24]; + + DeploymentArtifact.ParseDriverInstances(truncated).ShouldBeEmpty(); + DeploymentArtifact.ParseDriverInstances(truncated, NodeC1).ShouldBeEmpty(); + DeploymentArtifact.ResolveClusterScope(truncated, NodeC1).Mode.ShouldBe(ClusterFilterMode.None); + DeploymentArtifact.ParseDataPlaneAuthority(truncated, NodeC1).Keys.ShouldBeEmpty(); + DeploymentArtifact.ParseComposition(truncated).EquipmentNodes.ShouldBeEmpty(); + DeploymentArtifact.ParseComposition(truncated, NodeC1).EquipmentNodes.ShouldBeEmpty(); + + // Open() itself is explicit about unusable bytes, same as it is for malformed JSON. + Should.Throw(() => DeploymentArtifact.Open(truncated)); + } + + [Fact] + public void Decompression_size_guard_refuses_bombs() + { + // A hostile/corrupt blob must not be able to OOM the node: gzip of a highly repetitive payload + // reaches ~1000:1, so a few-hundred-KB artifact row could inflate to hundreds of MB. The production + // limit is 512 MB, which no honest artifact approaches — crafting a real 512 MB expansion in a unit + // test would burn seconds and gigabytes, so the ceiling is a parameter on the internal Inflate seam + // and this test passes a small one. + // + // It used to lower a mutable DeploymentArtifact.MaxDecompressedBytes static and restore it in a + // finally. xUnit runs test CLASSES in parallel, so that window was visible to every sibling class + // parsing a >4 KB artifact concurrently — they would fail with "refusing to inflate" for reasons + // having nothing to do with them. Do not reintroduce a process-wide mutation here. + var payload = new byte[64 * 1024]; // all zeroes: compresses to a few dozen bytes + var bomb = Gzip(payload); + bomb.Length.ShouldBeLessThan(1024); // the small-in / huge-out shape the guard exists for + + var ex = Should.Throw(() => DeploymentArtifact.Inflate(bomb, maxBytes: 4096)); + ex.Message.ShouldContain("4096"); + ex.Message.ShouldContain("refusing to inflate"); + + // Falsifiability control: the SAME bytes at the production ceiling inflate to the full payload — so + // the throw above is the guard firing, not the gzip stream being unreadable. + DeploymentArtifact.Inflate(bomb, DeploymentArtifact.MaxDecompressedBytes).Length.ShouldBe(payload.Length); + + // …and the guard is actually wired into the door every consumer comes through: Open() at the + // production ceiling gets past decompression and fails as JSON (64 KB of NULs is not JSON), never + // as a bomb. + Should.Throw(() => DeploymentArtifact.Open(bomb)) + .Message.ShouldNotContain("refusing to inflate"); + + // The over-limit JsonException lands in the same lenient catch the static wrappers already own — + // that leniency is proved on the corrupt-gzip path in Static_wrappers_stay_lenient_on_corrupt_gzip, + // which reaches it through the identical `catch (JsonException)` in every wrapper. Asserting it here + // with these bytes would be vacuous: 64 KB of NULs comes back empty from a lenient wrapper whether + // the guard fired or the JSON parse did. + } + + private static byte[] Gzip(byte[] plain) + { + using var output = new MemoryStream(); + using (var gz = new GZipStream(output, CompressionLevel.Fastest, leaveOpen: true)) + { + gz.Write(plain, 0, plain.Length); + } + return output.ToArray(); + } + + private static void AssertSameComposition(AddressSpaceComposition actual, AddressSpaceComposition expected) + { + actual.UnsAreas.ShouldBe(expected.UnsAreas); + actual.UnsLines.ShouldBe(expected.UnsLines); + actual.EquipmentNodes.ShouldBe(expected.EquipmentNodes); + actual.DriverInstancePlans.ShouldBe(expected.DriverInstancePlans); + actual.ScriptedAlarmPlans.ShouldBe(expected.ScriptedAlarmPlans); + actual.EquipmentTags.ShouldBe(expected.EquipmentTags); + actual.EquipmentVirtualTags.ShouldBe(expected.EquipmentVirtualTags); + actual.EquipmentScriptedAlarms.ShouldBe(expected.EquipmentScriptedAlarms); + actual.RawContainers.ShouldBe(expected.RawContainers); + actual.RawTags.ShouldBe(expected.RawTags); + actual.UnsReferenceVariables.ShouldBe(expected.UnsReferenceVariables); + } + + private static void AssertSameAuthority(DataPlaneAuthorityProjection actual, DataPlaneAuthorityProjection expected) + { + actual.ClusterId.ShouldBe(expected.ClusterId); + actual.Keys.Select(Describe).ShouldBe(expected.Keys.Select(Describe)); + actual.Grants.Select(Describe).ShouldBe(expected.Grants.Select(Describe)); + + actual.ScopePaths.Keys.OrderBy(k => k.ToString()).ShouldBe(expected.ScopePaths.Keys.OrderBy(k => k.ToString())); + foreach (var (scope, paths) in expected.ScopePaths) + { + actual.ScopePaths.ShouldContainKey(scope); + actual.ScopePaths[scope].Select(p => string.Join('/', p.Segments)) + .ShouldBe(paths.Select(p => string.Join('/', p.Segments))); + } + } + + // ApiKey / NodeAcl are EF entities (reference equality), so compare their fields as a flat string. + private static string Describe(ZB.MOM.WW.OtOpcUa.Configuration.Entities.ApiKey k) => + $"{k.ApiKeyId}|{k.Name}|{k.SecretHash}|{k.Principal}|{k.Enabled}|{k.CreatedUtc:O}|{k.ExpiresUtc:O}|{k.Notes}"; + + private static string Describe(ZB.MOM.WW.OtOpcUa.Configuration.Entities.NodeAcl a) => + $"{a.NodeAclId}|{a.ClusterId}|{a.Principal}|{a.ScopeKind}|{a.ScopeId}|{a.PermissionFlags}|{a.Notes}"; +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Harness/RuntimeActorTestBase.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Harness/RuntimeActorTestBase.cs index c547321f..d6f8fafe 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Harness/RuntimeActorTestBase.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Harness/RuntimeActorTestBase.cs @@ -55,8 +55,30 @@ akka { } }"; + /// + /// The same single-node HOCON at a caller-chosen loglevel. + /// + /// + /// A log event below akka.loglevel is never published to the EventStream at all, so + /// neither EventFilter nor an EventStream-subscribed probe can see it. A test whose + /// subject IS a log line (e.g. "this Debug line must be emitted exactly once, not in a spin") + /// therefore has to raise the level for its own ActorSystem; the default stays + /// WARNING so the other 40-odd classes keep their quiet output. + /// + /// An Akka log level — DEBUG, INFO, WARNING, ERROR. + /// The base test HOCON with its loglevel replaced. + protected static string AkkaTestHoconAtLevel(string level) => + AkkaTestHocon.Replace(@"loglevel = ""WARNING""", $@"loglevel = ""{level}""", StringComparison.Ordinal); + /// Initializes a new instance of the RuntimeActorTestBase class with Akka test setup. - protected RuntimeActorTestBase() : base(AkkaTestHocon) + protected RuntimeActorTestBase() : this(AkkaTestHocon) + { + } + + /// Initializes a new instance of the RuntimeActorTestBase class with caller-supplied HOCON. + /// The Akka configuration for this class's ActorSystem — usually + /// . + protected RuntimeActorTestBase(string hocon) : base(hocon) { var cluster = Akka.Cluster.Cluster.Get(Sys); cluster.Join(cluster.SelfAddress); 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 af17654b..f113ea06 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 @@ -23,6 +23,16 @@ namespace ZB.MOM.WW.OtOpcUa.Runtime.Tests.OpcUa; /// otopcua.opcua.apply.failed meter and log at Error — instead of the optimistic Info line. The /// happy path must stay Info-only (zero increments). /// +/// +/// Shares the opcua-publish-meters collection with OpcUaPublishActorRebuildTests so the two +/// classes NEVER run in parallel. The zero-increment assertions below (recorder.Total.ShouldBe(0)) +/// read a process-wide filtered only on the instrument NAME, and +/// otopcua.opcua.apply.failed carries no tag identifying the emitting actor instance (only +/// kind=rebuild|nodes) — so the assertion cannot be scoped to this test's own actor. The sibling +/// class's degraded-apply tests deliberately increment the same counter, which intermittently made these +/// zero assertions fail when the two classes ran concurrently in different xUnit collections. +/// +[Collection("opcua-publish-meters")] public sealed class OpcUaPublishActorApplyFailureTests : RuntimeActorTestBase { /// A rebuild whose sink throws increments otopcua.opcua.apply.failed (kind=rebuild). R2-07 — a 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 227b683b..5851527b 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 @@ -16,6 +16,18 @@ using ZB.MOM.WW.OtOpcUa.Runtime.Tests.Harness; namespace ZB.MOM.WW.OtOpcUa.Runtime.Tests.OpcUa; +/// +/// Shares the opcua-publish-meters collection with +/// so the two classes NEVER run in parallel. MeterRecorder is a process-wide +/// filtered only on the instrument NAME, and +/// otopcua.opcua.apply.failed carries no tag identifying which actor instance emitted it (only +/// kind=rebuild|nodes) — so it cannot be scoped to one test. The degraded-apply tests here +/// ( and +/// ) deliberately increment that counter, +/// which intermittently broke the sibling class's Total.ShouldBe(0) happy-path assertions when the +/// two classes ran concurrently in different collections. Serialising them is the fix. +/// +[Collection("opcua-publish-meters")] public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase { /// R2-07 — RebuildAddressSpace with dbFactory loads the artifact, composes, and applies. Two @@ -607,12 +619,392 @@ public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase return id; } + // --------------------------------------------------------------------------------------------------- + // 2026-08-15 surgical-deploys (Task 6) — a NON-rebuild apply materialises the plan's DELTA, not the whole + // composition. The passes are idempotent, but idempotent is not free. EnsureVariable / EnsureFolder + // early-return on a ConcurrentDictionary hit BEFORE taking the node manager's lock, so an already-standing + // node costs a dictionary probe, not a lock acquisition — but MaterialiseAlarmCondition DOES take the lock + // every time, and the probes are still O(address space) per apply plus the composition walk producing them. + // So re-walking the composition made the cost of ANY apply proportional to the size of the served address + // space (a one-tag add on a 50k-node server was ~50k sink calls on the publish thread) instead of to the + // size of the change. + // --------------------------------------------------------------------------------------------------- + + /// THE headline test. Deploy three raw tags, then deploy the same three PLUS one more: the + /// second apply must touch ONLY the added tag. Everything already standing — the three container folders + /// and the three existing variables — must not be re-ensured. + [Fact] + public void Pure_add_materialises_only_the_added_nodes() + { + var db = NewInMemoryDbFactory(); + var sink = new RecordingSink(); + var applier = new AddressSpaceApplier(sink, NullLogger.Instance); + var dep1 = SeedRawTagDeployment(db, new[] { "A", "B", "C" }); + + 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.ShouldContain("EV:Plant/Modbus/dev1/C"), duration: PresenceBudget); + var afterFirst = sink.Calls.Count; + + // One new tag on an otherwise identical configuration. + var dep2 = SeedRawTagDeployment(db, new[] { "A", "B", "C", "D" }); + actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId(), new DeploymentId(dep2))); + AwaitAssert(() => sink.Calls.ShouldContain("EV:Plant/Modbus/dev1/D"), duration: PresenceBudget); + Thread.Sleep(200); // let any (unwanted) extra pass finish before counting + + var second = sink.Calls.Skip(afterFirst).ToList(); + sink.RebuildCalls.ShouldBe(0); + // Exactly one variable ensured — the added tag. The three that were already standing are untouched. + second.Where(c => c.StartsWith("EV:")).ShouldBe(new[] { "EV:Plant/Modbus/dev1/D" }); + // …and no container folder was re-ensured either (the raw containers did not change). + second.ShouldNotContain(c => c.StartsWith("EF:")); + } + + /// The falsification control for the headline test: when the plan DOES force a full rebuild, the + /// address space has just been wiped, so every node in the composition must be re-materialised — the delta + /// subsets would leave the server serving almost nothing. + [Fact] + public void Rebuild_still_materialises_the_full_composition() + { + var db = NewInMemoryDbFactory(); + var sink = new RecordingSink(); + var applier = new AddressSpaceApplier(sink, NullLogger.Instance); + var dep1 = SeedUnsRawDeployment(db, equipmentName: "station1", referencedTag: "a"); + + 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.ShouldContain("EV:Plant/Modbus/dev1/b"), duration: PresenceBudget); + var afterFirst = sink.Calls.Count; + + // Renaming the equipment is a ChangedEquipment delta ⇒ the classifier's default-closed Rebuild. + var dep2 = SeedUnsRawDeployment(db, equipmentName: "station1-renamed", referencedTag: "a"); + actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId(), new DeploymentId(dep2))); + AwaitAssert(() => sink.RebuildCalls.ShouldBe(1), duration: PresenceBudget); + AwaitAssert( + () => sink.Calls.Skip(afterFirst).ShouldContain("EV:Plant/Modbus/dev1/b"), + duration: PresenceBudget); + + var second = sink.Calls.Skip(afterFirst).ToList(); + // The FULL raw subtree came back: both containers and both tags, not just a delta. + second.ShouldContain("EF:Plant"); + second.ShouldContain("EF:Plant/Modbus/dev1"); + second.ShouldContain("EV:Plant/Modbus/dev1/a"); + second.ShouldContain("EV:Plant/Modbus/dev1/b"); + // …and so did the UNS reference variable (whose own row never changed). + second.ShouldContain("EV:filling/line1/station1-renamed/a"); + } + + /// + /// The delta subset MUST carry the CURRENT side of every changed UNS reference, not just the added + /// ones. AddressSpaceApplier.Apply repairs a UnsReferenceDelta by REMOVING the previous + /// node in place and leaving the recreate to the materialise pass; a subset built from + /// AddedUnsReferenceVariables alone would remove a re-pointed reference and never bring it back. + /// Here the reference is re-pointed from raw tag a to raw tag b (which also moves the + /// effective name, and so the UNS NodeId). + /// + [Fact] + public void Changed_uns_reference_is_recreated_after_surgical_remove() + { + var db = NewInMemoryDbFactory(); + var sink = new RecordingSink(); + var applier = new AddressSpaceApplier(sink, NullLogger.Instance); + var dep1 = SeedUnsRawDeployment(db, equipmentName: "station1", referencedTag: "a"); + + 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.ShouldContain("EV:filling/line1/station1/a"), duration: PresenceBudget); + var afterFirst = sink.Calls.Count; + + // Re-point the SAME reference row at raw tag "b". + var dep2 = SeedUnsRawDeployment(db, equipmentName: "station1", referencedTag: "b"); + actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId(), new DeploymentId(dep2))); + AwaitAssert(() => sink.Calls.ShouldContain("EV:filling/line1/station1/b"), duration: PresenceBudget); + + var second = sink.Calls.Skip(afterFirst).ToList(); + sink.RebuildCalls.ShouldBe(0); // repaired in place, not by wiping the server + second.ShouldContain("RV:filling/line1/station1/a"); // the previous node was removed… + second.ShouldContain("EV:filling/line1/station1/b"); // …and the current one recreated + // …with its Organizes edge pointing at the NEW backing raw path. + sink.ReferenceEdges.ShouldContain(("filling/line1/station1/b", "Plant/Modbus/dev1/b")); + } + + /// + /// The same repair for the shape the 2026-08-15 review found (commit 875ec9fe): the reference row is + /// untouched, but its BACKING raw tag becomes historized. The coincident UnsReferenceDelta must + /// still be applied, or the UNS mirror keeps serving no history while the raw node serves it — the + /// exact split the Backing* fields exist to close. + /// Re-pointed by review I2, which changed HOW this shape is repaired: the delta's identity + /// is unchanged (same NodeId, same equipment, same backing RawPath — only the inherited attributes + /// moved), so it is now an IN-PLACE UpdateTagAttributes in the Uns realm rather than a + /// remove+recreate. The client MonitoredItems on the UNS mirror survive an operator historizing the raw + /// tag behind them. The historian-value assertion is unchanged — that is the actual repair being + /// pinned; only the mechanism that delivers it moved. + /// + [Fact] + public void Changed_uns_reference_is_updated_in_place_when_the_backing_tag_flips_historian() + { + var db = NewInMemoryDbFactory(); + var sink = new RecordingSink(); + var applier = new AddressSpaceApplier(sink, NullLogger.Instance); + var dep1 = SeedUnsRawDeployment(db, equipmentName: "station1", referencedTag: "a"); + + 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.ShouldContain("EV:filling/line1/station1/a"), duration: PresenceBudget); + var afterFirst = sink.Calls.Count; + + var dep2 = SeedUnsRawDeployment(db, equipmentName: "station1", referencedTag: "a", + historianTagnameForA: "WW.A"); + actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId(), new DeploymentId(dep2))); + + // The mirror now carries the backing tag's historian tagname — the repair itself, unchanged by I2. + AwaitAssert( + () => sink.UpdateTagDetails.ShouldContain( + u => u.NodeId == "filling/line1/station1/a" && u.HistorianTagname == "WW.A" && + u.Realm == AddressSpaceRealm.Uns), + duration: PresenceBudget); + Thread.Sleep(200); // let any (unwanted) remove+recreate land before asserting its absence + + var second = sink.Calls.Skip(afterFirst).ToList(); + sink.RebuildCalls.ShouldBe(0); + second.ShouldContain("UT:filling/line1/station1/a"); // repaired in place… + second.ShouldNotContain("RV:filling/line1/station1/a"); // …NOT torn down (subscriptions survive) + } + + /// An AttributeOnly apply (here a raw tag gaining a historian override, with no UNS reference to + /// repair) is handled entirely by the surgical in-place update: no materialise pass has anything to do, so + /// not a single node is re-ensured. + [Fact] + public void Attribute_only_apply_skips_all_materialise_passes_except_hierarchy() + { + var db = NewInMemoryDbFactory(); + var sink = new RecordingSink(); + var applier = new AddressSpaceApplier(sink, NullLogger.Instance); + var dep1 = SeedRawTagDeployment(db, new[] { "A", "B" }); + + 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.ShouldContain("EV:Plant/Modbus/dev1/B"), duration: PresenceBudget); + var afterFirst = sink.Calls.Count; + + var dep2 = SeedRawTagDeployment(db, new[] { "A", "B" }, historizedTag: "A"); + actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId(), new DeploymentId(dep2))); + AwaitAssert(() => sink.Calls.ShouldContain("UT:Plant/Modbus/dev1/A"), duration: PresenceBudget); + Thread.Sleep(200); // let any (unwanted) extra pass finish before counting + + var second = sink.Calls.Skip(afterFirst).ToList(); + sink.RebuildCalls.ShouldBe(0); + second.Where(c => c.StartsWith("EV:")).ShouldBeEmpty(); // nothing re-ensured + second.Where(c => c.StartsWith("EF:")).ShouldBeEmpty(); + second.ShouldContain("UT:Plant/Modbus/dev1/A"); // the in-place attribute update DID happen + } + + /// + /// 2026-08-15 surgical-deploys review C1 — THE headline degraded-apply test. Before the delta + /// materialisation the full passes re-ensured EVERY node on EVERY apply, so a swallowed + /// EnsureVariable fault silently repaired itself on the next deploy. The delta path removed that + /// accidental self-healing: once _lastApplied records a composition, the failed node is never in + /// a diff set again and EnsureVariable's existing-node early-return means it is never re-created + /// — the node is lost until a rebuild happens to fire. So a non-rebuild apply that reports + /// failedNodes > 0 must NOT advance _lastApplied: the next deploy re-diffs the same + /// composition and re-ensures what failed. + /// + [Fact] + public void Degraded_non_rebuild_apply_does_not_advance_last_applied() + { + var db = NewInMemoryDbFactory(); + // The sink faults on exactly one of the two added variables — a PureAdd apply that lands B but + // loses A, with no rebuild anywhere in sight. + var sink = new RecordingSink { FailEnsureVariableFor = "Plant/Modbus/dev1/A" }; + var applier = new AddressSpaceApplier(sink, NullLogger.Instance); + var dep1 = SeedRawTagDeployment(db, new[] { "A", "B" }); + + 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.ShouldContain("EV:Plant/Modbus/dev1/B"), duration: PresenceBudget); + Thread.Sleep(200); + sink.RebuildCalls.ShouldBe(0); // genuinely a non-rebuild (PureAdd) apply + var afterFirst = sink.Calls.Count; + + // The fault clears (a transient sink fault, an SDK hiccup). Re-deploy the SAME composition: because + // the degraded apply did not record it as applied, the planner re-diffs A (and B) as ADDED and the + // materialise pass re-ensures them. With _lastApplied advanced this second apply would diff to an + // EMPTY plan and A would be gone from the served address space forever. + sink.FailEnsureVariableFor = null; + var dep2 = SeedRawTagDeployment(db, new[] { "A", "B" }); + actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId(), new DeploymentId(dep2))); + + AwaitAssert( + () => sink.Calls.Skip(afterFirst).ShouldContain("EV:Plant/Modbus/dev1/A"), + duration: PresenceBudget); + sink.RebuildCalls.ShouldBe(0); + } + + /// + /// The other half of C1's rule: a REBUILD that also reports failed nodes DOES advance + /// _lastApplied. A rebuild already re-materialised the whole composition, and the applier's + /// one-way rebuild ratchet is the resync mechanism for anything it could not place — holding the + /// composition back here would make every subsequent deploy re-diff the entire address space as added. + /// Proven by the third apply of the same composition being an empty-plan no-op. + /// + [Fact] + public void Rebuild_with_failures_still_advances_last_applied() + { + var db = NewInMemoryDbFactory(); + var sink = new RecordingSink(); + var applier = new AddressSpaceApplier(sink, NullLogger.Instance); + var dep1 = SeedUnsRawDeployment(db, equipmentName: "station1", referencedTag: "a"); + + 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.ShouldContain("EV:Plant/Modbus/dev1/b"), duration: PresenceBudget); + + // Renaming the equipment is ChangedEquipment ⇒ the classifier's default-closed Rebuild; the sink + // faults on one variable of the full re-materialise that follows. + sink.FailEnsureVariableFor = "Plant/Modbus/dev1/b"; + var dep2 = SeedUnsRawDeployment(db, equipmentName: "station1-renamed", referencedTag: "a"); + actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId(), new DeploymentId(dep2))); + AwaitAssert(() => sink.RebuildCalls.ShouldBe(1), duration: PresenceBudget); + Thread.Sleep(200); + var afterSecond = sink.Calls.Count; + + // Re-deploy the SAME composition. _lastApplied advanced despite the failure, so this diffs to an + // empty plan and adds no sink calls at all. + sink.FailEnsureVariableFor = null; + var dep3 = SeedUnsRawDeployment(db, equipmentName: "station1-renamed", referencedTag: "a"); + actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId(), new DeploymentId(dep3))); + Thread.Sleep(300); + + sink.Calls.Count.ShouldBe(afterSecond); + } + + /// Seal a deployment whose raw tree is Plant/Modbus/dev1 carrying one tag per name in + /// (RawPath Plant/Modbus/dev1/<name>). There is no UNS topology, so + /// the composition is raw-only. names one tag that carries an + /// isHistorized TagConfig, which is an in-place-updatable (surgical) attribute change. + /// Deliberately NOT a params method, and deliberately not overloaded. The first draft + /// was (dbFactory, string? historizedTag = null, params string[] tagNames) alongside a convenience + /// (dbFactory, params string[] tagNames); Seed(db, "A", "B") then bound to the FIRST overload + /// in expanded form — historizing a tag called "A" and seeding only "B" — which compiled clean and made a + /// delta test silently assert against a one-tag composition. + private static Guid SeedRawTagDeployment( + IDbContextFactory dbFactory, + string[] tagNames, + string? historizedTag = null) + { + var artifact = JsonSerializer.SerializeToUtf8Bytes(new + { + UnsAreas = Array.Empty(), + UnsLines = Array.Empty(), + Equipment = Array.Empty(), + RawFolders = new[] { new { RawFolderId = "rf-1", ParentRawFolderId = (string?)null, Name = "Plant", ClusterId = "c1" } }, + DriverInstances = new[] { new { DriverInstanceId = "drv-1", DriverType = "Modbus", DriverConfig = "{}", Name = "Modbus", RawFolderId = "rf-1", ClusterId = "c1" } }, + Devices = new[] { new { DeviceId = "dev-1", DriverInstanceId = "drv-1", Name = "dev1", DeviceConfig = "{}" } }, + TagGroups = Array.Empty(), + Tags = tagNames.Select(n => new + { + TagId = $"t-{n}", + DeviceId = "dev-1", + TagGroupId = (string?)null, + Name = n, + DataType = "Float", + AccessLevel = 0, + TagConfig = string.Equals(n, historizedTag, StringComparison.Ordinal) + ? "{\"isHistorized\":true}" + : "{}", + }).ToArray(), + UnsTagReferences = Array.Empty(), + ScriptedAlarms = Array.Empty(), + }); + + return SealArtifact(dbFactory, artifact); + } + + /// Seal a deployment carrying BOTH a raw tree (Plant/Modbus/dev1 with tags a + + /// b) and a UNS topology (filling / line1 / ) whose + /// single UnsTagReference row (id ref-1, stable across deploys) points at + /// . Re-pointing that row across two deploys yields a + /// UnsReferenceDelta; renaming the equipment yields a rebuild-forcing ChangedEquipment. + /// historizes raw tag a with that override, which makes the + /// reference's Backing* mirror fields differ (the 875ec9fe repair path). + private static Guid SeedUnsRawDeployment( + IDbContextFactory dbFactory, + string equipmentName, + string referencedTag, + string? historianTagnameForA = null) + { + var artifact = JsonSerializer.SerializeToUtf8Bytes(new + { + UnsAreas = new[] { new { UnsAreaId = "area-1", ClusterId = "c1", Name = "filling" } }, + UnsLines = new[] { new { UnsLineId = "line-1", UnsAreaId = "area-1", Name = "line1" } }, + Equipment = new[] { new { EquipmentId = "eq-1", UnsLineId = "line-1", Name = equipmentName, MachineCode = "EQ-1" } }, + RawFolders = new[] { new { RawFolderId = "rf-1", ParentRawFolderId = (string?)null, Name = "Plant", ClusterId = "c1" } }, + DriverInstances = new[] { new { DriverInstanceId = "drv-1", DriverType = "Modbus", DriverConfig = "{}", Name = "Modbus", RawFolderId = "rf-1", ClusterId = "c1" } }, + Devices = new[] { new { DeviceId = "dev-1", DriverInstanceId = "drv-1", Name = "dev1", DeviceConfig = "{}" } }, + TagGroups = Array.Empty(), + Tags = new[] + { + new + { + TagId = "t-a", DeviceId = "dev-1", TagGroupId = (string?)null, Name = "a", + DataType = "Float", AccessLevel = 0, + TagConfig = historianTagnameForA is null + ? "{}" + : $"{{\"isHistorized\":true,\"historianTagname\":\"{historianTagnameForA}\"}}", + }, + new + { + TagId = "t-b", DeviceId = "dev-1", TagGroupId = (string?)null, Name = "b", + DataType = "Float", AccessLevel = 0, TagConfig = "{}", + }, + }, + UnsTagReferences = new[] + { + new { UnsTagReferenceId = "ref-1", EquipmentId = "eq-1", TagId = $"t-{referencedTag}" }, + }, + ScriptedAlarms = Array.Empty(), + }); + + return SealArtifact(dbFactory, artifact); + } + + /// Seal as a new deployment and return its id. + private static Guid SealArtifact(IDbContextFactory dbFactory, byte[] artifact) + { + var id = Guid.NewGuid(); + using var ctx = dbFactory.CreateDbContext(); + ctx.Deployments.Add(new Deployment + { + DeploymentId = id, + RevisionHash = new string('e', 64), + Status = DeploymentStatus.Sealed, + CreatedBy = "test", + SealedAtUtc = DateTime.UtcNow, + ArtifactBlob = artifact, + }); + ctx.SaveChanges(); + return id; + } + private sealed class RecordingSink : IOpcUaAddressSpaceSink, ISurgicalAddressSpaceSink { /// Gets the list of recorded sink calls. public ConcurrentQueue Calls { get; } = new(); + /// Gets every EnsureVariable call with the attributes the applier resolved for it (the + /// ordered queue only carries node ids). + public ConcurrentQueue<(string NodeId, string? Parent, bool Writable, string? HistorianTagname, AddressSpaceRealm Realm)> VariableDetails { get; } = new(); + /// Gets every AddReference (Organizes) edge the applier wired. + public ConcurrentQueue<(string Source, string Target)> ReferenceEdges { get; } = new(); /// Gets or sets the count of rebuild address space calls. public int RebuildCalls; + /// Gets or sets the node id whose throws (null ⇒ none). The + /// applier's SafeEnsureVariable swallows the throw into its failed-node tally, which is exactly + /// the degraded-apply shape the C1 no-advance rule exists for. Settable mid-test so a transient fault + /// can clear between deploys. + public volatile string? FailEnsureVariableFor; + /// Gets every surgical in-place tag-attribute update with the attributes the applier resolved + /// for it (the ordered queue only carries node ids). + public ConcurrentQueue<(string NodeId, bool Writable, string? HistorianTagname, AddressSpaceRealm Realm)> UpdateTagDetails { get; } = new(); /// Gets the queue of surgical in-place folder display-name update calls (OpcUaServer-001). public ConcurrentQueue<(string FolderNodeId, string DisplayName)> FolderRenameQueue { get; } = new(); /// Gets the list of recorded surgical folder display-name update calls. @@ -653,18 +1045,25 @@ public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase /// Whether the node is created read/write. /// The resolved historian tagname (null ⇒ not historized). public void EnsureVariable(string variableNodeId, string? parentFolderNodeId, string displayName, string dataType, bool writable, AddressSpaceRealm realm, string? historianTagname = null, bool isArray = false, uint? arrayLength = null) - => Calls.Enqueue($"EV:{variableNodeId}"); + { + if (string.Equals(FailEnsureVariableFor, variableNodeId, StringComparison.Ordinal)) + throw new InvalidOperationException($"simulated EnsureVariable fault for {variableNodeId}"); + Calls.Enqueue($"EV:{variableNodeId}"); + VariableDetails.Enqueue((variableNodeId, parentFolderNodeId, writable, historianTagname, realm)); + } /// Records a rebuild address space call. public void RebuildAddressSpace() => Interlocked.Increment(ref RebuildCalls); /// Records a NodeAdded model-change announcement. /// The node under which discovered nodes were added. public void RaiseNodesAddedModelChange(string affectedNodeId, AddressSpaceRealm realm = AddressSpaceRealm.Uns) => Calls.Enqueue($"NA:{affectedNodeId}"); public void WireAlarmNotifiers(string alarmNodeId, AddressSpaceRealm alarmRealm, IReadOnlyList notifierFolderNodeIds, AddressSpaceRealm notifierFolderRealm) { } - public void AddReference(string sourceNodeId, AddressSpaceRealm sourceRealm, string targetNodeId, AddressSpaceRealm targetRealm, string referenceType = "Organizes") { } + public void AddReference(string sourceNodeId, AddressSpaceRealm sourceRealm, string targetNodeId, AddressSpaceRealm targetRealm, string referenceType = "Organizes") + => ReferenceEdges.Enqueue((sourceNodeId, targetNodeId)); /// Records a surgical in-place tag-attribute update (always succeeds in this recording sink). public bool UpdateTagAttributes(string variableNodeId, bool writable, string? historianTagname, string dataType, bool isArray, uint? arrayLength, AddressSpaceRealm realm = AddressSpaceRealm.Uns) { Calls.Enqueue($"UT:{variableNodeId}"); + UpdateTagDetails.Enqueue((variableNodeId, writable, historianTagname, realm)); return true; } /// Records a surgical in-place folder display-name update (always succeeds in this recording sink).