diff --git a/docs/plans/2026-08-15-surgical-deploys.md b/docs/plans/2026-08-15-surgical-deploys.md new file mode 100644 index 00000000..24b6ea63 --- /dev/null +++ b/docs/plans/2026-08-15-surgical-deploys.md @@ -0,0 +1,318 @@ +# Surgical Deploys Implementation Plan + +> **For Claude:** REQUIRED SUB-SKILL: Use superpowers-extended-cc:executing-plans to implement this plan task-by-task. + +**Goal:** Make routine config edits non-events instead of fleet-wide outages: a raw-tag attribute edit or a UNS-reference add/remove/re-point no longer tears down the whole address space (killing every client subscription), the artifact is read from SQL once per apply instead of three times, fully JSON-parsed once per actor instead of ~8–11 times, only the delta is materialised instead of the whole composition, blocking apply I/O leaves the hot actor mailboxes, and the artifact blob is deflate-compressed inside the hashed bytes. + +**Architecture:** Three prongs. (1) **Surgical routing:** `AddressSpaceChangeClassifier` learns that a non-alarm `RawTagDelta` and any `UnsReferenceDelta` are surgical-eligible; `AddressSpaceApplier` applies raw-tag attribute edits in place via the existing `ISurgicalAddressSpaceSink.UpdateTagAttributes` (Raw realm) and handles UNS-reference deltas as remove(previous-node)+recreate(current) — the recreate rides the delta materialise pass. Alarm-bearing raw tags keep the full-rebuild path (the WP4 notifier-wiring guard). (2) **Parse/read-once:** Direct mode threads the already-read blob into the rebuild + subscription pushes (the FetchAndCache path is the template, `DriverHostActor.cs:1650-1654`); a new `DeploymentArtifact.Open()` returns a `ParsedDeploymentArtifact` that parses the JSON once and serves all consumers; the materialise passes gain subset overloads so non-rebuild applies materialise only the plan's added/changed nodes. (3) **Mechanics:** the LocalDb cache write leaves the mailbox via `PipeTo`, the Applying-state dispatch deferral stops busy-spinning, and `ConfigComposer` emits a deflate-compressed blob (magic-sniffed on the read side, hash computed over the compressed bytes so `RevisionHash == SHA-256(stored/wire bytes)` stays true). + +**Tech Stack:** C# / .NET 10, System.Text.Json (`JsonDocument`), Akka.NET, System.IO.Compression, xUnit + Shouldly. + +**Branch:** `perf/surgical-deploys` (branched off `perf/value-pipeline-batching` — plans 1 and 2 touch the same actors; do NOT branch off master). + +--- + +## Context for implementers (read before any task) + +Verified facts — do not re-derive: + +- **The rebuild cliff:** `AddressSpaceChangeClassifier.Classify` (`src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceChangeClassifier.cs:69-78`) routes ANY non-empty `ChangedRawTags` / `ChangedUnsReferenceVariables` (among others) to `Rebuild`. `RawTagPlan.ReferencingEquipmentPaths` participates in record equality (`AddressSpaceComposer.cs`, `RawTagPlan.Equals`), so adding/removing a `UnsTagReference` flips the backing raw tag to Changed ⇒ full rebuild. The WP4 comment block (`AddressSpaceChangeClassifier.cs:62-68`) is the binding guard: any surgical `ChangedRawTags` path for ALARM tags must re-wire notifiers — we sidestep it by keeping alarm-bearing deltas on the Rebuild path. +- **The applier already has every surgical primitive:** `ISurgicalAddressSpaceSink` (`src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/ISurgicalAddressSpaceSink.cs`) — `UpdateTagAttributes(nodeId, writable, historianTagname, dataType, isArray, arrayLength, realm)` (realm-aware, returns false ⇒ caller rebuilds), `RemoveVariableNode`, `RemoveAlarmConditionNode`, `RemoveEquipmentSubtree`, `UpdateFolderDisplayName`. The applier's surgical branch is `AddressSpaceApplier.Apply` (`AddressSpaceApplier.cs:151-224`); its false/throw ⇒ full-rebuild ratchet must be preserved for every new surgical operation. +- **Apply flow (publish actor):** `OpcUaPublishActor.HandleRebuild` (`OpcUaPublishActor.cs:361-512`): load blob (msg.Artifact wins; else ConfigDb) → `FeedDataPlaneAuthority(artifact)` (MUST stay ahead of the `plan.IsEmpty` return — #520 stage 4) → `ParseComposition` → `AddressSpacePlanner.Compute(_lastApplied, composition)` → `plan.IsEmpty` return → `_applier.Apply(plan)` → six full-composition materialise passes (`:456-480`) → `AnnounceAddedNodes` when `!outcome.RebuildCalled`. `_lastApplied` advances only after Apply. +- **Apply flow (host actor):** Direct mode `ApplyAndAck` (`DriverHostActor.cs:1680-1754`): `ReconcileDrivers(deploymentId)` reads the blob from SQL and RETURNS it (`appliedBlob`), then `RebuildAddressSpace(correlation, deploymentId)` (NO bytes ⇒ publish actor re-reads SQL at `OpcUaPublishActor.cs:578-593`) and `PushDesiredSubscriptions(deploymentId)` (re-reads SQL again). FetchAndCache mode `HandleFetchedForApply` (`:1609-1674`) already does it right: `ReconcileDriversFromBlob(id, bytes)` → `RebuildAddressSpace(corr, id, appliedBlob)` → `PushDesiredSubscriptionsFromArtifact(id, appliedBlob)` → `CacheAppliedArtifact`. +- **Parse entry points** (`src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DeploymentArtifact.cs`): `ParseDriverInstances(ReadOnlySpan)` `:89`, `ResolveClusterScope(ReadOnlySpan, string)` `:513`, `ParseDriverInstances(blob, nodeId)` `:555` (scope+parse), `ParseDataPlaneAuthority(blob, nodeId)` `:591`, `ParseComposition(blob)` `:893`, `ParseComposition(blob, nodeId, onInconsistency)` `:955` (scope+parse+`BuildClusterSets` `:1018`). EVERY one does `JsonDocument.Parse(blob.ToArray())` — a full copy per parse. The private `Build*` helpers already take `JsonElement root`, which is what makes the parse-once refactor mechanical. +- **Per-apply parse count today:** host: `ReconcileDrivers*` (scope+instances = 2) + `PushDesiredSubscriptions*` (scope+composition+clustersets = 3) + `CacheAppliedArtifact` (scope = 1); publish actor: `FeedDataPlaneAuthority` (authority+scope = 2) + `ParseComposition` (3). Target: ONE `JsonDocument` per actor per apply. +- **Blocking I/O on mailboxes:** `CacheAppliedArtifact` does `.GetAwaiter().GetResult()` (`DriverHostActor.cs:1901-1904`, comment at `:1870-1875` acknowledges it); the Applying-state deferral re-`Forward`s to Self in a hot loop (`DriverHostActor.cs:944-955`); `BeginFetchAndCacheApply` (`:1571-1602`) is the `PipeTo` template. +- **Compression constraint (CLAUDE.md, load-bearing):** `RevisionHash == SHA-256(artifact bytes)` — `ConfigComposer` computes the hash of the exact blob, `GrpcDeploymentArtifactFetcher` verifies fetched bytes against it, chunked LocalDb cache stores those bytes. Compressing BEFORE hashing preserves the invariant everywhere (hash of the compressed bytes). The dispatch stays payload-free. +- **Invariants to preserve:** + 1. **#485 "no bytes is no answer"** — a null/empty artifact fails the apply and keeps last-known-good; never let compression or parse-once break that path. + 2. **`FeedDataPlaneAuthority` runs ahead of `plan.IsEmpty`** — a key-revocation-only artifact must still land. + 3. **The full-rebuild ratchet** — any surgical false/throw falls back to `SafeRebuild()`. + 4. **`_lastApplied` advances only on a successful Apply** — the retry diffs against what is actually materialised. + 5. **Deterministic composition** — `ConfigComposer`'s deterministic sort is what makes revision-hash no-ops real; compression must not reorder anything (it can't — it wraps the final bytes). + 6. **The classifier stays default-closed** — any delta shape not explicitly whitelisted still lands on `Rebuild`. +- **Key records:** `RawTagPlan(TagId, NodeId, ParentNodeId, DriverInstanceId, Name, DataType, Writable, Alarm, ReferencingEquipmentPaths, IsHistorized, HistorianTagname, IsArray, ArrayLength, Realm)` with custom `Equals` (`AddressSpaceComposer.cs:141+`); `UnsReferenceVariable(UnsTagReferenceId, EquipmentId, NodeId, EffectiveName, BackingRawPath, DataType, Writable, Realm)` (`:204+`); `AddressSpacePlan.RawTagDelta(Previous, Current)` / `UnsReferenceDelta(Previous, Current)` (`AddressSpacePlan.cs:134-138`). +- **Materialise passes** (`AddressSpaceApplier.cs`): `MaterialiseHierarchy(composition)` `:643`, `MaterialiseRawSubtree(composition)` `:682` (needs `BuildEquipmentIdByFolderPath(composition)` for alarm notifier wiring), `MaterialiseUnsReferences(composition)` `:776` (builds `rawByPath` from `composition.RawTags` for inherited shape/historian), `MaterialiseScriptedAlarms`, `MaterialiseEquipmentTags`, `MaterialiseEquipmentVirtualTags` (in the `:820+` region). All idempotent loops over composition lists. +- **Existing tests to keep green:** `tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceChangeClassifierTests.cs` + `AddressSpaceChangeClassifierDualNamespaceTests.cs` (some assert `ChangedRawTags ⇒ Rebuild` / `ChangedUnsReferenceVariables ⇒ Rebuild` — those assertions CHANGE deliberately in Task 4; update them to assert the new contract, alarm-delta cases still Rebuild), applier tests, `OpcUaPublishActorApplyFailureTests`, `DriverHostActorFetchAndCacheBootstrapTests`, `tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/DeploymentArtifactProtoTests.cs`, and `tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/SubscriptionSurvivalTests.cs` (the live gate shape for "subscriptions survive"). +- **Commit style:** `feat(...)` / `perf(...)` / `refactor(...)` with scope; one commit per task. + +--- + +### Task 1: Direct mode threads the already-read blob + +**Classification:** small +**Estimated implement time:** ~3 min +**Parallelizable with:** Task 2 + +**Files:** +- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs:1726-1730` +- Test: `tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/` — extend the existing apply-path test class that exercises `ApplyAndAck` (grep for it; the FetchAndCache tests show the probe idiom) + +**Step 1: Failing test** — drive a Direct-mode apply with a real blob in the fake ConfigDb; assert the publish-actor probe's `RebuildAddressSpace` message carries `Artifact != null` with the exact blob bytes (today it carries null). + +**Step 3: Implement** — in `ApplyAndAck`, replace lines `:1726` and `:1729`: + +```csharp +_opcUaPublishActor?.Tell(new ZB.MOM.WW.OtOpcUa.Runtime.OpcUa.OpcUaPublishActor.RebuildAddressSpace(correlation, deploymentId, appliedBlob)); +PushDesiredSubscriptionsFromArtifact(deploymentId, appliedBlob); +``` + +(`appliedBlob` is non-null here — the null case returned at `:1706`.) Leave `PushDesiredSubscriptions(deploymentId)` in place for any other caller; if it becomes call-site-dead, delete it and its SQL read. Update the comment block: the publish actor no longer reads central SQL on the Direct path either — `LoadArtifact`/`LoadLatestArtifact` remain only as the legacy/dev fallback for a bytes-less `RebuildAddressSpace`. + +**Step 4:** `dotnet test tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests` — PASS. + +**Step 5:** `git add -A && git commit -m "perf(runtime): Direct-mode apply threads the in-hand artifact (1 SQL read per apply, not 3)"` + +--- + +### Task 2: `DeploymentArtifact.Open()` — parse once + +**Classification:** high-risk (config-parsing seam; behavior must be byte-identical) +**Estimated implement time:** ~5 min +**Parallelizable with:** Task 1 + +**Files:** +- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DeploymentArtifact.cs` +- Test: `tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/ParsedDeploymentArtifactTests.cs` (create) + +**Step 1: Failing test** — build a representative artifact JSON (reuse/borrow the builder from existing `DeploymentArtifact` tests — grep `ParseComposition` under tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests for the fixture); assert parity: `using var parsed = DeploymentArtifact.Open(blob);` then `parsed.ResolveClusterScope(node)`, `parsed.ParseDriverInstances(node)`, `parsed.ParseComposition(node, onInconsistency)`, `parsed.ParseDataPlaneAuthority(node)` each equal the static counterparts' results (deep equality on the records; compositions compare with their record equality). + +**Step 3: Implement** +- Add: + +```csharp +/// One-parse view over an artifact blob: a single JsonDocument serves every consumer of an +/// apply (driver reconcile, subscription push, composition, data-plane authority, cluster scope) instead +/// of each entry point re-parsing — at production sizes each static parse was a full LOH copy + document +/// build, and one apply performed ~8-11 of them. Dispose when the apply completes. +public sealed class ParsedDeploymentArtifact : IDisposable +{ + private readonly JsonDocument _doc; + private readonly byte[] _blob; // decoded (post-decompression) bytes + private ClusterScope? _scope; // memoized per node — every consumer resolves the same node + private string? _scopeNodeId; + internal ParsedDeploymentArtifact(JsonDocument doc, byte[] blob) { _doc = doc; _blob = blob; } + public JsonElement Root => _doc.RootElement; + public void Dispose() => _doc.Dispose(); +} +``` + +- `public static ParsedDeploymentArtifact Open(byte[] blob)` — parse ONCE (`JsonDocument.Parse(blob)` — no `.ToArray()` copy; the caller's array is not mutated afterward on any call path). +- Move the body of each static entry point into an instance method on `ParsedDeploymentArtifact` operating on `Root` (the private `Build*` helpers already take `JsonElement root` — pass `Root` straight in). Memoize `ResolveClusterScope(nodeId)` per node id (`_scope`/`_scopeNodeId`) since `ParseDriverInstances`/`ParseComposition`/`ParseDataPlaneAuthority` each begin with the same scope resolution. +- Re-express every existing static as a thin wrapper: `using var parsed = Open(blob.ToArray-if-span-else-array); return parsed.X(...)` — public signatures may stay `ReadOnlySpan` for compatibility (the wrapper pays one copy; the hot path stops using the statics in Task 3). Do not change any output shape. + +**Step 4:** `dotnet test tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests` + `dotnet test tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests` — PASS. + +**Step 5:** `git commit -m "refactor(runtime): ParsedDeploymentArtifact — one JsonDocument per apply"` + +--- + +### Task 3: Actors consume `Open()` + +**Classification:** standard +**Estimated implement time:** ~4 min +**Parallelizable with:** none (needs 1 + 2) + +**Files:** +- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs` (`ReconcileDriversFromBlob`, `PushDesiredSubscriptionsFromArtifact`, `CacheAppliedArtifact` call sites within one apply) +- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/OpcUa/OpcUaPublishActor.cs` (`HandleRebuild` + `FeedDataPlaneAuthority`) + +**Implement (behavior-preserving):** +- Host: in `ApplyAndAck` / `HandleFetchedForApply`, `using var parsed = DeploymentArtifact.Open(appliedBlob);` and pass `parsed` down (add internal overloads `ReconcileDriversFromParsed`, `PushDesiredSubscriptionsFromParsed`, `CacheAppliedArtifact(deploymentId, revision, blob, parsed)` — cache still stores the BYTES; it only needs `parsed` for the scope lookup it currently re-parses for). The existing blob-based methods stay as wrappers for the boot paths. +- Publish actor: `HandleRebuild` opens once after the #485 guard; `FeedDataPlaneAuthority(parsed, artifactBytes)` (the SHA-256 at `:535` still hashes the raw bytes); the composition parse uses `parsed.ParseComposition(...)`. +- Tests: the Task 2 parity tests + the existing apply suites ARE the net; add one assertion-level test only if a seam demands it. + +Run both Runtime + OpcUaServer test projects. Commit: `perf(runtime): apply path parses the artifact once per actor` + +--- + +### Task 4: Classifier — surgical eligibility for raw-tag and UNS-reference deltas + +**Classification:** high-risk (the outage-per-edit fix; default-closed semantics) +**Estimated implement time:** ~5 min +**Parallelizable with:** none + +**Files:** +- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceChangeClassifier.cs` +- Test: `tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceChangeClassifierTests.cs` + `AddressSpaceChangeClassifierDualNamespaceTests.cs` (update the deliberately-changed assertions; add the new cases) + +**Step 1: Write the failing tests** (these define the new contract): +1. `Reference_only_raw_tag_delta_is_not_a_rebuild` — a `RawTagDelta` whose ONLY difference is `ReferencingEquipmentPaths`, `Alarm == null` both sides ⇒ kind is NOT `Rebuild` (with no other deltas ⇒ `AttributeOnly`). +2. `Attribute_raw_tag_delta_is_not_a_rebuild` — non-alarm delta differing in `Writable`/`IsHistorized`/`HistorianTagname`/`DataType`/`IsArray`/`ArrayLength` ⇒ not Rebuild. +3. `Alarm_bearing_raw_tag_delta_still_rebuilds` — `Alarm != null` on either side ⇒ `Rebuild` (the WP4 guard). +4. `Identity_field_raw_tag_delta_still_rebuilds` — e.g. `DriverInstanceId` or `Name` differs ⇒ `Rebuild` (default-closed). +5. `Uns_reference_delta_is_not_a_rebuild` — a `UnsReferenceDelta` (re-point or display-name case) ⇒ not Rebuild. +6. Combination: UNS-ref ADD + the induced reference-only raw-tag delta ⇒ `PureAdd` (the headline case — one added UNS variable, zero teardown). + +**Step 3: Implement** +- Add the predicate (mirror `TagDeltaIsSurgicalEligible`'s whitelist-via-`with` idiom, which auto-closes over future fields): + +```csharp +// 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); +``` + +- In `Classify` rule 2 (`:69-78`): replace `plan.ChangedRawTags.Count > 0` with `plan.ChangedRawTags.Any(d => !RawTagDeltaIsSurgicalEligible(d))`, and DELETE `plan.ChangedUnsReferenceVariables.Count > 0` from the rebuild condition (every `UnsReferenceDelta` is handled surgically in Tasks 5-6: remove-previous + recreate-current). +- Rewrite the WP4 comment block to state the NEW contract: alarm-bearing raw-tag deltas still rebuild (notifier wiring); value-tag deltas are surgical; the applier owns `UpdateTagAttributes` for them; UNS-ref deltas are remove+recreate. + +**Step 4:** `dotnet test tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests --filter "FullyQualifiedName~AddressSpaceChangeClassifier"` — PASS (including the updated legacy assertions). + +**Step 5:** `git commit -m "feat(opcuaserver): classifier — non-alarm raw-tag + UNS-reference deltas are surgical, not rebuilds"` + +--- + +### Task 5: Applier — apply the new surgical deltas + +**Classification:** high-risk (rebuild ratchet semantics) +**Estimated implement time:** ~5 min +**Parallelizable with:** none (needs 4) + +**Files:** +- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceApplier.cs` (`Apply`, `:151-224`) +- Test: `tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/` — extend the applier test class (find it via grep `AddressSpaceApplier` in that project; use its recording-fake surgical sink) + +**Step 1: Failing tests** +1. `Surgical_raw_tag_delta_updates_in_place_raw_realm` — plan with one eligible `RawTagDelta` (historize flip); fake surgical sink records `UpdateTagAttributes(d.Current.NodeId, writable, historian, dataType, isArray, arrayLength, AddressSpaceRealm.Raw)` with historian resolved as `MaterialiseRawSubtree` resolves it (override else NodeId, `AddressSpaceApplier.cs:748-750`) and `writable = Writable && !IsArray`; NO `RebuildAddressSpace` call. +2. `Reference_only_raw_tag_delta_is_a_noop_update` — only `ReferencingEquipmentPaths` differs ⇒ either no sink call or one idempotent `UpdateTagAttributes` (assert no rebuild; prefer SKIPPING the call when the with-comparison excluding the referencing set shows attribute equality — implement that skip). +3. `Uns_reference_delta_removes_previous_node` — plan with one `UnsReferenceDelta`; sink records terminal-Bad `WriteValue(Previous.NodeId, ..., Uns)` then `RemoveVariableNode(Previous.NodeId, Uns)`; no rebuild. (The recreate happens in the publish actor's delta materialise pass — Task 6 — NOT here.) +4. `Surgical_false_falls_back_to_rebuild` — surgical sink returns false for the raw-tag update ⇒ `RebuildAddressSpace` called (ratchet preserved). + +**Step 3: Implement** in `Apply`: +- Compute `var surgicalRawTagDeltas = plan.ChangedRawTags.Where(AddressSpaceChangeClassifier.RawTagDeltaIsSurgicalEligible).ToList();` (when kind != Rebuild this equals ALL of ChangedRawTags — the classifier guarantees it; still filter, defense-in-depth) and `var unsRefDeltas = plan.ChangedUnsReferenceVariables;` +- Extend the surgical-branch trigger (`:171`): `surgicalTagDeltas.Count > 0 || renamedFolders.Count > 0 || hasRemovePass || surgicalRawTagDeltas.Count > 0 || unsRefDeltas.Count > 0`. +- Inside the `ISurgicalAddressSpaceSink` branch, after the existing equipment-tag loop: + - Raw-tag loop: skip when attribute-equal apart from `ReferencingEquipmentPaths` (test 2); else `surgical.UpdateTagAttributes(d.Current.NodeId, d.Current.Writable && !d.Current.IsArray, d.Current.IsHistorized ? (string.IsNullOrWhiteSpace(d.Current.HistorianTagname) ? d.Current.NodeId : d.Current.HistorianTagname) : null, d.Current.DataType, d.Current.IsArray, d.Current.ArrayLength, AddressSpaceRealm.Raw)` with the same try/catch + `allApplied` ratchet as the equipment-tag loop. + - UNS-ref loop: `SafeWriteValue(d.Previous.NodeId, null, OpcUaStatus.Bad, ts, AddressSpaceRealm.Uns);` then `SafeRemoveVariable(surgical, d.Previous.NodeId, AddressSpaceRealm.Uns)` into the ratchet. Doc-comment WHY remove+recreate (a re-point must move the `Organizes` edge and the sink has no RemoveReference; a display-name edit changes the NodeId; remove+recreate covers both, at the cost of that one node's monitored items — vs. every subscription server-wide today). +- `changedCount` accounting already includes both sets (`:115-121`) — verify, don't double-count. +- Also update the `FeedHistorizedRefs` changed-raw-tags handling: it already handles `plan.ChangedRawTags` (`:571-578`) — now those deltas reach it on the surgical path too; confirm the logic is delta-shape-agnostic (it is) and leave it. + +**Step 4:** run the OpcUaServer.Tests project — PASS. + +**Step 5:** `git commit -m "feat(opcuaserver): applier applies raw-tag + UNS-reference deltas surgically"` + +--- + +### Task 6: Delta materialisation (publish actor materialises the plan, not the world) + +**Classification:** high-risk (the O(fleet)-per-apply fix) +**Estimated implement time:** ~5 min +**Parallelizable with:** none (needs 5) + +**Files:** +- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceApplier.cs` (subset overloads on the six passes) +- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/OpcUa/OpcUaPublishActor.cs:456-480` +- Test: `tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/` applier tests + `tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/` publish-actor tests + +**Step 1: Failing tests** +1. `Pure_add_materialises_only_the_added_nodes` — publish-actor-level: apply composition A (N nodes), then A+1 tag; the fake sink's `EnsureVariable`/`EnsureFolder` calls after the second apply cover ONLY the added tag (+ its added containers), not all N. (This is the headline test.) +2. `Rebuild_still_materialises_the_full_composition` — force a rebuild-kind delta; full-composition passes run as today. +3. `Uns_reference_delta_recreates_current_node` — a re-point delta: after Task 5's remove, the UNS pass re-ensures `Current.NodeId` with shape inherited from the FULL composition's `rawByPath` (backing tag NOT in the delta), plus the `AddReference` Organizes edge to the NEW backing path. +4. Applier-level: `MaterialiseRawSubtree(full, subsetTags, subsetContainers)` wires alarm notifiers using the FULL composition's equipment map. + +**Step 3: Implement** +- Applier: give the passes subset parameters, full-composition context retained for lookups: + - `MaterialiseRawSubtree(AddressSpaceComposition composition, IReadOnlyList? onlyContainers = null, IReadOnlyList? onlyTags = null)` — iterate `onlyContainers ?? composition.RawContainers` / `onlyTags ?? composition.RawTags`; `BuildEquipmentIdByFolderPath(composition)` stays full. + - `MaterialiseUnsReferences(AddressSpaceComposition composition, IReadOnlyList? only = null)` — iterate `only ?? composition.UnsReferenceVariables`; `rawByPath` stays full. + - `MaterialiseScriptedAlarms` / `MaterialiseEquipmentTags` / `MaterialiseEquipmentVirtualTags`: same `only` pattern over their respective lists. + - `MaterialiseHierarchy` stays full-composition ALWAYS (folders are O(areas+lines+equipment), cheap, and parents must exist for any add). +- Publish actor (`:456-480`): after `var outcome = _applier.Apply(plan);` branch on `outcome.RebuildCalled`: + - Rebuild ⇒ the six passes exactly as today (full composition). + - Non-rebuild ⇒ `MaterialiseHierarchy(composition)` full; then subsets: containers = `plan.AddedRawContainers`, raw tags = `plan.AddedRawTags`, UNS refs = `plan.AddedUnsReferenceVariables` concat `plan.ChangedUnsReferenceVariables.Select(d => d.Current)`, scripted alarms = `plan.AddedAlarms`, equipment tags = `plan.AddedEquipmentTags`, vtags = `plan.AddedEquipmentVirtualTags`. Skip a pass entirely when its subset is empty. + - Comment WHY: the passes are idempotent but not free — each call is a lock acquisition; a one-tag add on a 50k-node server was ~50k sink calls. +- `AnnounceAddedNodes` stays as-is (known pre-existing gap: raw/UNS adds are not announced — out of scope, note it in the comment). + +**Step 4:** OpcUaServer.Tests + Runtime.Tests — PASS (watch `OpcUaPublishActorApplyFailureTests` + `DarkAddressSpaceReasons` for full-pass assumptions; update only assertions that pinned the OLD full-pass behavior explicitly, preserving their intent). + +**Step 5:** `git commit -m "perf(opcuaserver): non-rebuild applies materialise the delta, not the composition"` + +--- + +### Task 7: Blocking I/O off the host mailbox + +**Classification:** high-risk (actor lifecycle) +**Estimated implement time:** ~5 min +**Parallelizable with:** Task 8 + +**Files:** +- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs` (`CacheAppliedArtifact` `:1896-1904` region; Applying-state deferral `:944-955`) +- Test: `tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/` — extend the apply/dispatch test classes + +**Part A — async cache write.** `CacheAppliedArtifact`'s `.GetAwaiter().GetResult()` becomes fire-and-forget with observed completion: + +```csharp +_artifactCache.StoreAsync(...).PipeTo(Self, + success: () => new ArtifactCached(deploymentId), + failure: ex => new ArtifactCacheFailed(deploymentId, ex.Message)); +``` + +(private records; handlers just log Info/Warning — the cache is a boot fallback, not apply-critical; the docstring at `:1870-1875` already debates exactly this and the apply outcome does not depend on it). CHECK first whether `BootstrapFromCache` (`:788, :859`) races a concurrent store — it runs in PreStart before any apply, so no; note that in the comment. Keep the store idempotent-skip (`IsAlreadyCachedAsync`) inside the async flow. +Failing test: apply completes and acks BEFORE a deliberately-slow fake cache resolves; a follow-up `ArtifactCached` self-message is observed (TestKit probe or log assertion per the project's idiom). + +**Part B — stash instead of forward-spin.** The Applying-state deferral for a DIFFERENT deployment (`:944-955`): make `DriverHostActor` implement `IWithUnboundedStash` (`public IStash Stash { get; set; }`), replace `Self.Forward(msg)` with `Stash.Stash()`, and call `Stash.UnstashAll()` at every `Become(Steady)` transition (both `finally` blocks + the fetch-failure path). Failing test: send dispatch B while A's fetch is in flight (slow fake fetcher); assert no busy-loop (the log does NOT contain the deferral line more than once) and B applies after A completes. + +**Step 4:** Runtime.Tests — PASS. **Step 5:** `git commit -m "perf(runtime): apply-path cache write off-mailbox; stash deferred dispatches"` + +--- + +### Task 8: Compressed artifact blob + +**Classification:** high-risk (wire/storage format; hash invariant) +**Estimated implement time:** ~5 min +**Parallelizable with:** Task 7 + +**Files:** +- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/AdminOperations/ConfigComposer.cs` (emit compressed; hash unchanged in derivation — SHA-256 of the emitted bytes) +- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DeploymentArtifact.cs` (`Open()` + static wrappers sniff-and-decompress) +- Test: `tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/ParsedDeploymentArtifactTests.cs` + the ConfigComposer test class (grep `SnapshotAndFlattenAsync` under tests/Server) + +**Format decision (locked):** gzip via `System.IO.Compression.GZipStream` (`CompressionLevel.Fastest`), detected by the gzip magic bytes `0x1F 0x8B` at offset 0 (a JSON artifact can never start with 0x1F). No custom framing. Compression happens INSIDE `ConfigComposer` immediately after `SerializeToUtf8Bytes`, BEFORE the revision hash is computed — so `RevisionHash == SHA-256(stored bytes)` holds and `GrpcDeploymentArtifactFetcher`'s verification (`GrpcDeploymentArtifactFetcher.cs:89-90`) is untouched. The LocalDb cache + gRPC serve stores/ships the compressed bytes as-is (they never look inside). Decompression lives in exactly one place: `DeploymentArtifact.Open()` (and thus every static wrapper). `OpcUaPublishActor.FeedDataPlaneAuthority`'s SHA at `OpcUaPublishActor.cs:535` hashes the RAW (compressed) bytes it received — verify that call site hashes the bytes as-carried, not a decompressed copy. + +**Step 1: Failing tests** +1. `Open_reads_gzip_compressed_artifacts` — compress a fixture blob, parity-assert `ParseComposition` vs the uncompressed parse. +2. `Open_still_reads_uncompressed_artifacts` (backward compat — pre-upgrade rows). +3. `Composer_emits_gzip_and_hash_of_compressed_bytes` — `ArtifactBlob` starts with `1F 8B`; the deployment's `RevisionHash` equals `SHA256(ArtifactBlob)` lowercase hex. + +**Step 3: Implement** — decompress with a size guard (refuse decompressed output > 512 MB — a corrupt/hostile blob must not OOM the node; treat over-limit exactly like unparseable JSON: the existing catch paths). Note in a comment: gzip output is not guaranteed byte-stable across .NET releases; the hash is computed from the emitted bytes at compose time and never re-derived, so the only consequence of an impl change is one spurious "changed" revision after a runtime upgrade. + +**Step 4:** Runtime.Tests + ControlPlane tests (if the composer test project is DB-backed and the DB is unreachable, run the artifact round-trip tests and note the skip). **Step 5:** `git commit -m "perf(controlplane): gzip artifact blob inside the hashed bytes (~10-20x smaller)"` + +--- + +### Task 9: Full verification + docs + +**Classification:** standard +**Estimated implement time:** ~5 min +**Parallelizable with:** none (last) + +**Files:** +- Modify: `CLAUDE.md` — the "v3 OPC UA Address Space" / classifier-adjacent statements that now describe the old behavior (grep CLAUDE.md for `ChangedRawTags` and `Rebuild`); keep the edits surgical and truthful to the new contract (alarm-bearing deltas still rebuild). +- Modify: `docs/Uns.md` + `docs/Raw.md` — one short paragraph each: reference add/remove and attribute edits are now surgical; what still rebuilds (alarm deltas, container renames, identity changes). + +Steps: full `dotnet build ZB.MOM.WW.OtOpcUa.slnx`; run every non-DB test project (same list as plan 1 Task 12 + `tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests`); record skipped DB-backed suites in the commit message. Commit: `docs: record surgical-deploy semantics (plan 2026-08-15)`. + +--- + +## Explicit non-goals + +- **No surgical raw-container (folder/device/group) renames** — still rebuild (no surgical folder-remove on the sink; pre-existing). +- **No alarm-bearing raw-tag surgical path** — the WP4 notifier-rewire stays owned by the rebuild. +- **No central-side deploy-start dedup** (`AdminOperationsActor` triple DB read) — separate follow-up. +- **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. diff --git a/docs/plans/2026-08-15-surgical-deploys.md.tasks.json b/docs/plans/2026-08-15-surgical-deploys.md.tasks.json new file mode 100644 index 00000000..347c712e --- /dev/null +++ b/docs/plans/2026-08-15-surgical-deploys.md.tasks.json @@ -0,0 +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]} + ], + "lastUpdated": "2026-08-15T00:00:00Z" +} diff --git a/docs/plans/2026-08-15-value-pipeline-batching.md b/docs/plans/2026-08-15-value-pipeline-batching.md new file mode 100644 index 00000000..218c346d --- /dev/null +++ b/docs/plans/2026-08-15-value-pipeline-batching.md @@ -0,0 +1,631 @@ +# Value-Pipeline Batching Implementation Plan + +> **For Claude:** REQUIRED SUB-SKILL: Use superpowers-extended-cc:executing-plans to implement this plan task-by-task. + +**Goal:** Batch the hot value path end to end — one message per poll cycle instead of one per tag at every hop (driver → DriverInstanceActor → DriverHostActor → OpcUaPublishActor → node manager), one SDK-lock acquisition per batch instead of per value, and zero per-write key allocations in the node manager. + +**Architecture:** A new optional `IBatchSubscribable` capability lets a driver raise one `OnDataChangeBatch` per production burst (poll cycle); `PollGroupEngine` gains a batch callback so the four engine-based poll drivers get it for free. `DriverInstanceActor` forwards one `AttributeValueBatchPublished` per burst (wrapping per-tag events from non-batch drivers as single-item batches); `DriverHostActor` resolves the whole batch's NodeRealmRefs and sends ONE `AttributeValueBatchUpdate` to `OpcUaPublishActor`, plus ONE batch Tell to the dependency mux. The sink gains `WriteValues(IReadOnlyList)` (default-interface-method fallback loops `WriteValue`, so fakes/Null sink are untouched); `OtOpcUaNodeManager` overrides it taking `Lock` once per batch, and its node maps are re-keyed by a `(realm, id)` struct so the per-write `new NodeId(...).ToString()` allocations disappear. The pinned dispatcher's `throughput` rises from 1 to 100 (same single thread, fewer mailbox re-schedules). + +**Tech Stack:** C# / .NET 10, Akka.NET (classic actors), OPC Foundation UA .NET Standard, xUnit + Shouldly, Akka.TestKit. + +**Branch:** `perf/value-pipeline-batching` (off `master`). + +--- + +## Context for implementers (read before any task) + +Verified facts about the current code — do not re-derive, and do not violate the invariants: + +- **The per-tag path today:** `PollGroupEngine.PollOnceAsync` (`src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/PollGroupEngine.cs:202-226`) diffs per tag and invokes `_onChange` per changed tag. Drivers forward that to `ISubscribable.OnDataChange` (per-tag event, `src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/ISubscribable.cs:36`). `DriverInstanceActor.HandleSubscribeAsync` attaches `_dataChangeHandler` at `DriverInstanceActor.cs:880` (one `DataChangeForward` Tell per tag), `OnDataChangeForward` (`:1138-1143`) Tells parent one `AttributeValuePublished` per tag. `DriverHostActor.ForwardToMux` (`DriverHostActor.cs:976-1006`) Tells the mux per value and Tells `OpcUaPublishActor` one `AttributeValueUpdate` **per NodeRealmRef** (UNS fan-out at `:997-999`). `OpcUaPublishActor.HandleAttributeUpdate` (`OpcUaPublishActor.cs:319-331`) calls `_sink.WriteValue` per message. `OtOpcUaNodeManager.WriteValue` (`OtOpcUaNodeManager.cs:508-533`) takes the manager-wide `lock (Lock)` per value and allocates via `MapKey(realm, id)` = `new NodeId(id, ns).ToString()` (`:133`). +- **Invariants to preserve:** + 1. **#519 status codes ride verbatim** — the full `uint` status code, sub-code included, must reach the SDK unchanged at every hop. + 2. **Timestamps are carried from the driver** — never stamp `DateTime.UtcNow` on the value path. + 3. **PollGroupEngine's no-overlap guarantee** (delay-after-completion, one loop task per subscription) must not change in this plan. + 4. **Per-tag `OnDataChange` keeps working** — Galaxy's cold-read path and driver CLIs subscribe it. A batch-capable driver raises BOTH events (batch once per cycle + per-tag per item); a consumer subscribes to exactly ONE of the two. `DriverInstanceActor` prefers batch when the driver implements `IBatchSubscribable`. + 5. **`DriverInstanceActor.DetachSubscription`** (`:934-943`) must detach the batch handler too — the detach is load-bearing (the `IDriver` outlives the actor across respawns; see CLAUDE.md §DeployWatcher). + 6. **The mux's per-item receive stays** — `DriverHostActor.FanCommsLossToDependencyMux` (`:1135-1142`) fabricates per-item `AttributeValuePublished` messages; `DependencyMuxActor` keeps that receive and gains a batch receive alongside. + 7. **`OpcUaPublishActor` keeps its per-item `AttributeValueUpdate` receive** — `VirtualTagHostActor` and the alarm paths still send it. The batch message is additive. + 8. **The pinned dispatcher stays pinned** — the OPC UA SDK requires a single thread. Only `throughput` changes. + 9. **`_writes` increments** (`OpcUaPublishActor.cs:324`) and the `OtOpcUaTelemetry.OpcUaSinkWrite` meter must count per-VALUE (per item), not per batch, so dashboards don't drop by 100×. +- **Message-record homes:** `AttributeValuePublished` is declared in `DriverInstanceActor` (`:109`); `AttributeValueUpdate` in `OpcUaPublishActor` (`:52`); `NodeRealmRef` is a private struct of `DriverHostActor` (`:183`). +- **Sink implementors that must keep compiling:** `NullOpcUaAddressSpaceSink` (same file as the interface), `DeferredAddressSpaceSink` (`src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/DeferredAddressSpaceSink.cs`), `SdkAddressSpaceSink` (`src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/SdkAddressSpaceSink.cs`), plus test fakes. Using a **default interface method** for `WriteValues` means only the ones that want batching override it. +- **Commands:** build `dotnet build ZB.MOM.WW.OtOpcUa.slnx`; single project `dotnet test tests//`; single test `dotnet test --filter "FullyQualifiedName~."`. DB-backed suites need the central SQL Server — on this Mac, prefer running the specific unit projects named per task, not the whole solution, until the final task. +- **Commit style:** follow recent history (`feat(...)`, `fix(...)`, `chore(...)` with a scope). One commit per task. + +--- + +### Task 1: Batch contract types in Core.Abstractions + +**Classification:** small +**Estimated implement time:** ~3 min +**Parallelizable with:** Task 10 + +**Files:** +- Modify: `src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/ISubscribable.cs` +- Test: `tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/DataChangeBatchContractTests.cs` (create) + +**Step 1: Write the failing test** + +```csharp +using Shouldly; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using Xunit; + +namespace ZB.MOM.WW.OtOpcUa.Core.Tests; + +public class DataChangeBatchContractTests +{ + private sealed record FakeHandle(string DiagnosticId) : ISubscriptionHandle; + + [Fact] + public void Batch_args_carry_handle_and_items() + { + var handle = new FakeHandle("h1"); + var snap = new DataValueSnapshot(42, 0u, DateTime.UtcNow, DateTime.UtcNow); + var args = new DataChangeBatchEventArgs(handle, new[] { new DataChangeItem("plc/tag1", snap) }); + args.SubscriptionHandle.ShouldBe(handle); + args.Items.Count.ShouldBe(1); + args.Items[0].FullReference.ShouldBe("plc/tag1"); + args.Items[0].Snapshot.ShouldBe(snap); + } +} +``` + +Note: check `DataValueSnapshot`'s actual constructor shape in `src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/` first and adjust the test's construction to it (positional record with Value/StatusCode/timestamps). Do not change `DataValueSnapshot`. + +**Step 2: Run test to verify it fails** + +Run: `dotnet test tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests --filter "FullyQualifiedName~DataChangeBatchContractTests"` +Expected: FAIL — `DataChangeBatchEventArgs` / `DataChangeItem` not defined. + +**Step 3: Implement** + +Append to `src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/ISubscribable.cs`: + +```csharp +/// One changed attribute within a . +/// Driver-side full reference of the changed attribute. +/// New value + quality + timestamps. +public sealed record DataChangeItem(string FullReference, DataValueSnapshot Snapshot); + +/// Event payload for — every attribute +/// that changed within ONE production burst (a poll cycle, an inbound message, a stream chunk). +/// The handle returned by the original call. +/// The changed attributes, one entry per changed tag. Never empty. +public sealed record DataChangeBatchEventArgs( + ISubscriptionHandle SubscriptionHandle, + IReadOnlyList Items); + +/// +/// Optional driver capability: data changes delivered one event per production burst instead of +/// one event per tag. A driver implementing this raises once per +/// burst AND continues raising the per-tag for each item +/// (legacy consumers: Galaxy cold reads, driver CLIs). A consumer subscribes to EXACTLY ONE of the +/// two events — subscribing both double-delivers. DriverInstanceActor prefers the batch event. +/// +public interface IBatchSubscribable : ISubscribable +{ + /// Batched data-change notification — one raise per production burst. + event EventHandler? OnDataChangeBatch; +} +``` + +**Step 4: Run test to verify it passes** + +Run: same filter. Expected: PASS. + +**Step 5: Commit** + +```bash +git add src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/ISubscribable.cs tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/DataChangeBatchContractTests.cs +git commit -m "feat(core): add IBatchSubscribable batch data-change contract" +``` + +--- + +### Task 2: PollGroupEngine batch callback + +**Classification:** standard +**Estimated implement time:** ~5 min +**Parallelizable with:** none (Task 1 first) + +**Files:** +- Modify: `src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/PollGroupEngine.cs` +- Test: `tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/PollGroupEngineBatchTests.cs` (create) + +**Step 1: Write the failing tests** (find the existing PollGroupEngine test file first — if one exists in `tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests`, put these in a new class beside it and copy its reader-fake idiom): + +Test cases (write all four): +1. `Batch_fires_once_per_cycle_with_only_changed_items` — reader returns 3 refs; first poll (initial push) batch has all 3; then reader changes only 1 value; next batch has exactly that 1 item; per-tag `onChange` also fired for the same items (both callbacks see identical content). +2. `No_batch_when_nothing_changed` — second poll with identical snapshots ⇒ no batch invocation (and no per-tag invocation). +3. `Batch_is_not_invoked_when_callback_absent` — construct without `onBatch`; engine behaves exactly as today (per-tag only). Assert per-tag count. +4. `Batch_items_are_in_reader_order`. + +Use a short interval (e.g. 30 ms) + `TaskCompletionSource`/polling-with-timeout to await callbacks, mirroring the existing engine tests' style. Never `Thread.Sleep` long constants. + +**Step 2: Run to verify failure** + +Run: `dotnet test tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests --filter "FullyQualifiedName~PollGroupEngineBatchTests"` +Expected: FAIL — constructor has no `onBatch` parameter. + +**Step 3: Implement** + +In `PollGroupEngine`: +- Add field `private readonly Action>? _onBatch;` +- Add optional ctor param `Action>? onBatch = null` (AFTER the existing optional params, so every existing call site compiles unchanged) and assign it. +- In `PollOnceAsync` (currently `:202-226`): collect changed items lazily and raise the batch after the loop: + +```csharp +List? changed = null; +for (var i = 0; i < state.TagReferences.Count; i++) +{ + var tagRef = state.TagReferences[i]; + var current = snapshots[i]; + var lastSeen = state.LastValues.TryGetValue(tagRef, out var prev) ? prev : default; + + if (forceRaise || ValuesAreDifferent(lastSeen?.Value, current.Value) || lastSeen?.StatusCode != current.StatusCode) + { + state.LastValues[tagRef] = current; + _onChange(state.Handle, tagRef, current); + if (_onBatch is not null) (changed ??= new List()).Add(new DataChangeItem(tagRef, current)); + } +} +if (changed is { Count: > 0 }) _onBatch!(state.Handle, changed); +``` + +Keep the surrounding contract-violation check and everything else byte-identical. Update the class remarks to document the batch callback (one raise per cycle, only when non-empty, after all per-tag callbacks of that cycle). + +**Step 4: Run tests** + +Run: `dotnet test tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests` (whole project — the existing engine tests must stay green). +Expected: PASS. + +**Step 5: Commit** + +```bash +git add -A && git commit -m "feat(core): PollGroupEngine optional per-cycle batch callback" +``` + +--- + +### Task 3a: Modbus + S7 raise the batch event + +**Classification:** standard +**Estimated implement time:** ~5 min +**Parallelizable with:** Task 3b, Task 5 + +**Files:** +- Modify: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs` +- Modify: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7Driver.cs` +- Test: `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusBatchSubscriptionTests.cs` (create; mirror the fixture idiom of `ModbusSubscriptionTests.cs`) + +**Step 1: Write the failing test** — subscribe via the driver against the existing in-test transport fake (copy `ModbusSubscriptionTests`' setup); assert: +1. `ModbusDriver` is `IBatchSubscribable`. +2. After one poll cycle with N changed registers, `OnDataChangeBatch` fired once with N items AND `OnDataChange` fired N times (both events, identical content — contract from Task 1). + +**Step 2:** Run: `dotnet test tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests --filter "FullyQualifiedName~ModbusBatchSubscriptionTests"` — FAIL (interface not implemented). + +**Step 3: Implement** — in each driver: +- Change the class declaration to implement `IBatchSubscribable` (it already implements `ISubscribable`). +- Add `public event EventHandler? OnDataChangeBatch;` +- Find the `new PollGroupEngine(...)` construction; pass `onBatch: (handle, items) => OnDataChangeBatch?.Invoke(this, new DataChangeBatchEventArgs(handle, items))`. + +Per-tag raising is already wired via the engine's `onChange` — leave it. + +**Step 4:** Run the Modbus + S7 test projects: +`dotnet test tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests && dotnet test tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests` — PASS (all existing tests too). + +**Step 5:** `git add -A && git commit -m "feat(drivers): Modbus + S7 implement IBatchSubscribable via PollGroupEngine batch callback"` + +**Deviation (landed):** Modbus does **not** forward the engine's `items` argument — it ignores it and +raises the batch from its own per-cycle buffer of the items that passed `ShouldPublish`. The engine +collects its batch list unconditionally and has no knowledge of the driver's deadband filter, so a +straight pass-through would carry changes the per-tag event suppressed, and a consumer that preferred +the batch event would see deadband silently stop working (review finding). `onBatch` is therefore used +only as the end-of-cycle signal, draining the buffer; a cycle whose every change was suppressed raises +nothing. Because that buffer is the only piece of cross-cycle state any of the four poll drivers holds, +the driver's `onChange` bridge also wraps `OnDataChange?.Invoke(...)` in a swallowing try/catch +(mirroring `PollGroupEngine.ReportError`): the engine invokes the bridge unguarded, so a throwing +external subscriber would unwind the cycle before `onBatch` ran and the undrained items would merge +into a later cycle's batch. The buffer append stays outside the catch — the item was published as far +as the batch contract is concerned; only the per-tag subscriber faulted. S7/AbCip/FOCAS forward the +engine's list as this task describes and hold no such state. + +--- + +### Task 3b: AbCip + FOCAS raise the batch event + +**Classification:** standard +**Estimated implement time:** ~4 min +**Parallelizable with:** Task 3a, Task 5 + +**Files:** +- Modify: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs` +- Modify: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs` + +Same mechanical change as Task 3a (interface + event + `onBatch` at the `new PollGroupEngine(...)` site). If either driver constructs the engine differently (check first), adapt but keep the identical contract. No new per-driver test required (the engine behavior is pinned by Task 2; the wiring pattern is pinned by Task 3a's test) — but the projects' existing suites must stay green. + +Run: `dotnet test tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests && dotnet test tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests` + +Commit: `feat(drivers): AbCip + FOCAS implement IBatchSubscribable` + +--- + +### Task 4: DriverInstanceActor batch forwarding + +**Classification:** high-risk (actor model, subscription lifecycle) +**Estimated implement time:** ~5 min +**Parallelizable with:** Task 5 + +**Files:** +- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverInstanceActor.cs` +- Test: `tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorBatchForwardTests.cs` (create; copy the TestKit + stub-driver idiom from the existing `DriverInstanceActor*Tests` in that directory) + +**Step 1: Write the failing tests** +1. `Batch_driver_forwards_one_batch_message` — stub driver implementing `IBatchSubscribable`; raise `OnDataChangeBatch` with 3 items after subscribe; parent probe receives exactly ONE `AttributeValueBatchPublished` with 3 items (driver id + per-item FullReference/Value/StatusCode/timestamp mapped as `OnDataChangeForward` maps them today: `ts = Snapshot.SourceTimestampUtc ?? Snapshot.ServerTimestampUtc`). +2. `Batch_driver_per_tag_event_not_double_forwarded` — the stub raises BOTH events (per contract); parent receives only the batch message, no legacy `AttributeValuePublished`. +3. `Per_tag_driver_forwards_single_item_batches` — stub implementing only `ISubscribable`; one `OnDataChange` ⇒ parent receives one `AttributeValueBatchPublished` with 1 item. +4. `Detach_removes_batch_handler` — after Unsubscribe (or Connected→Reconnecting, whichever the existing detach tests exercise), raising the batch event produces no message. + +**Step 2:** Run: `dotnet test tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests --filter "FullyQualifiedName~DriverInstanceActorBatchForwardTests"` — FAIL. + +**Step 3: Implement** +- Add records next to `AttributeValuePublished` (`:109`): + +```csharp +/// One value within an . +public sealed record AttributeValueItem(string FullReference, object? Value, uint StatusCode, DateTime TimestampUtc); +/// Batched form of — every changed attribute of ONE +/// driver production burst in one message. The parent fans the whole batch to the mux and the OPC UA +/// publish path in one hop each. Status codes ride verbatim (#519). +public sealed record AttributeValueBatchPublished(string DriverInstanceId, IReadOnlyList Items); +``` + +- Private message `private sealed record DataChangeBatchForward(IReadOnlyList Items);` + field `private EventHandler? _batchChangeHandler;` +- In `HandleSubscribeAsync` (`:878-882`): when `subscribable is IBatchSubscribable batchable`, attach ONLY the batch handler (`_batchChangeHandler = (_, args) => self.Tell(new DataChangeBatchForward(args.Items)); batchable.OnDataChangeBatch += _batchChangeHandler;`) and do NOT attach `_dataChangeHandler`; else attach `_dataChangeHandler` exactly as today. +- `Receive(OnDataChangeBatchForward);` beside the existing receive (`:480`); handler builds the items list once and Tells parent one `AttributeValueBatchPublished`. +- `OnDataChangeForward` (`:1138`) becomes a single-item batch: `Context.Parent.Tell(new AttributeValueBatchPublished(_driverInstanceId, new[] { new AttributeValueItem(msg.FullReference, msg.Snapshot.Value, msg.Snapshot.StatusCode, ts) }));` — keep the `AttributeValuePublished` record declared (the mux comms-loss path and existing tests use the type), but the actor no longer sends it. +- `DetachSubscription` (`:934-943`): also detach `_batchChangeHandler` from `IBatchSubscribable` and null it. + +**IMPORTANT — downstream compile/behavior:** `DriverHostActor` currently receives `AttributeValuePublished` (`DriverHostActor.cs:960`). Until Task 8 lands, add a minimal bridge in THIS task so the system stays green: in `DriverHostActor`, add `Receive(batch => { foreach (var i in batch.Items) ForwardToMux(new DriverInstanceActor.AttributeValuePublished(batch.DriverInstanceId, i.FullReference, i.Value, i.StatusCode, i.TimestampUtc)); });` — Task 8 replaces this bridge with the real batched fan-out. Existing `DriverHostActor` tests that send `AttributeValuePublished` directly stay green (the receive stays). + +**Step 4:** Run: `dotnet test tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests` — PASS (existing DriverInstanceActor forward tests may need updating IF they assert the parent receives `AttributeValuePublished` from a live driver event — update those to expect the single-item batch; do NOT weaken assertions on content). + +**Step 5:** `git add -A && git commit -m "feat(runtime): DriverInstanceActor forwards batched data changes"` + +--- + +### Task 5: `ValueWrite` + sink `WriteValues` default method + +**Classification:** small +**Estimated implement time:** ~4 min +**Parallelizable with:** Task 3a, 3b, 4 + +**Files:** +- Modify: `src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/IOpcUaAddressSpaceSink.cs` +- Modify: `src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/DeferredAddressSpaceSink.cs` +- Test: `tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/AddressSpaceSinkBatchTests.cs` (create) + +**Step 1: Failing tests** +1. `Default_WriteValues_loops_WriteValue` — a minimal fake implementing only the existing members; call `WriteValues` with 3 items; the fake's recorded `WriteValue` calls match in order/content. +2. `Deferred_sink_forwards_WriteValues_to_inner_once` — a fake inner that overrides `WriteValues` (records one batch call); `DeferredAddressSpaceSink.WriteValues` reaches the inner's override (NOT 3 `WriteValue` calls). + +**Step 3: Implement** + +In `IOpcUaAddressSpaceSink.cs`: + +```csharp +/// One value write within a batch. +/// Same semantics per field as (#519: status verbatim). +public readonly record struct ValueWrite( + string NodeId, object? Value, uint StatusCode, DateTime SourceTimestampUtc, AddressSpaceRealm Realm); +``` + +Add to the interface (after `WriteValue`): + +```csharp +/// Write a batch of Variable values. Semantically identical to calling +/// per item; an SDK-backed sink overrides this to take its internal +/// lock ONCE per batch instead of once per value (the hot-path win). The default implementation +/// loops so existing sinks and fakes behave unchanged. +void WriteValues(IReadOnlyList values) +{ + ArgumentNullException.ThrowIfNull(values); + for (var i = 0; i < values.Count; i++) + { + var v = values[i]; + WriteValue(v.NodeId, v.Value, v.StatusCode, v.SourceTimestampUtc, v.Realm); + } +} +``` + +In `DeferredAddressSpaceSink`, add the forwarding override: `public void WriteValues(IReadOnlyList values) => _inner.WriteValues(values);` + +**Step 4:** `dotnet test tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests` — PASS. + +**Step 5:** `git add -A && git commit -m "feat(commons): batched WriteValues on the address-space sink"` + +--- + +### Task 6: OtOpcUaNodeManager batched write under one lock + +**Classification:** high-risk (SDK locking) +**Estimated implement time:** ~4 min +**Parallelizable with:** Task 7 + +**Files:** +- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs` (WriteValue region, `:508-533`) +- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/SdkAddressSpaceSink.cs` (forward `WriteValues` to the manager) +- Test: `tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerBatchWriteTests.cs` (create; copy the manager-construction idiom from an existing OpcUaServer.Tests file that instantiates `OtOpcUaNodeManager`) + +**Step 1: Failing tests** +1. `WriteValues_lands_every_value` — materialise 3 variables (or rely on create-on-first-write), `WriteValues` a 3-item batch, read each variable's Value/StatusCode/Timestamp back via the manager's public surface. +2. `WriteValues_creates_missing_variables` — same lazy-create semantics as `WriteValue`. + +**Step 3: Implement** +- Extract the body of `WriteValue`'s `lock (Lock)` block into `private void WriteValueLocked(string nodeId, ushort ns, string key, object? value, uint statusCode, DateTime ts)` (verbatim move). +- `WriteValue` computes `ns`/`key` then `lock (Lock) { WriteValueLocked(...); }` — behavior identical. +- Add: + +```csharp +/// Batched WriteValue: one Lock acquisition for the whole batch. Same per-item semantics. +public void WriteValues(IReadOnlyList values) +{ + ArgumentNullException.ThrowIfNull(values); + if (values.Count == 0) return; + EnsureAddressSpaceCreated(); + lock (Lock) + { + for (var i = 0; i < values.Count; i++) + { + var v = values[i]; + WriteValueLocked(v.NodeId, NamespaceIndexForRealm(v.Realm), MapKey(v.Realm, v.NodeId), v.Value, v.StatusCode, v.SourceTimestampUtc); + } + } +} +``` + +- In `SdkAddressSpaceSink`, override `WriteValues` forwarding to the manager's method (it currently forwards `WriteValue` 1:1 — mirror that). + +**Step 4:** `dotnet test tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests` — PASS. + +**Step 5:** `git add -A && git commit -m "feat(opcuaserver): node manager batched WriteValues under one lock"` + +--- + +### Task 7: OpcUaPublishActor batch message + +**Classification:** standard +**Estimated implement time:** ~4 min +**Parallelizable with:** Task 6 + +**Files:** +- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/OpcUa/OpcUaPublishActor.cs` +- Test: `tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorBatchTests.cs` (create; copy the actor-construction idiom from `OpcUaPublishActorApplyFailureTests.cs`) + +**Step 1: Failing tests** +1. `Batch_message_reaches_sink_as_one_WriteValues_call` — fake sink records `WriteValues` batches; send `AttributeValueBatchUpdate` with 3 items; sink saw one batch of 3. +2. `Batch_increments_write_counter_per_item` — whatever surface exposes `_writes` (check for an existing status/metrics accessor in the actor's tests; if none, assert via the telemetry meter or skip the counter assertion and note it) — at minimum assert behavior parity: 3 items produce the same observable effect as 3 single `AttributeValueUpdate` messages. + +**Step 3: Implement** +- Add next to `AttributeValueUpdate` (`:52`): + +```csharp +/// Batched form of : every fan-out write of one driver +/// production burst in one message — the sink takes its lock once for the whole batch. +public sealed record AttributeValueBatchUpdate(IReadOnlyList Items); +``` + +(`ValueWrite` comes from `ZB.MOM.WW.OtOpcUa.Commons.OpcUa` — already imported.) +- Register `Receive(HandleAttributeBatchUpdate);` beside `:288`. +- Handler mirrors `HandleAttributeUpdate` (`:319-331`): try/catch WARN on throw, `_sink.WriteValues(msg.Items)`, then `_writes += msg.Items.Count` via `Interlocked.Add(ref _writes, msg.Items.Count)` and `OtOpcUaTelemetry.OpcUaSinkWrite.Add(msg.Items.Count, new KeyValuePair("kind", "value"))` — per-item counting (invariant 9). + +**Step 4:** `dotnet test tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests --filter "FullyQualifiedName~OpcUaPublishActorBatch"` then the whole project — PASS. + +**Step 5:** `git add -A && git commit -m "feat(runtime): OpcUaPublishActor batched value updates"` + +--- + +### Task 8: DriverHostActor batched fan-out (replaces the Task 4 bridge) + +**Classification:** high-risk (the hottest routing seam) +**Estimated implement time:** ~5 min +**Parallelizable with:** none (needs 4 + 7; Task 9 lands with it) + +**Files:** +- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs` +- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/DependencyMuxActor.cs` (Task 9 content — same commit is fine if splitting is awkward, but prefer two commits) +- Test: `tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorBatchFanOutTests.cs` (create; copy the host-actor fixture idiom from `DriverHostActorNativeAlarmTests.cs`) + +**Step 1: Failing tests** +1. `Batch_fans_out_to_all_node_realm_refs_in_one_publish_message` — arrange the host with an applied composition where tag A maps to 1 raw + 1 UNS NodeId and tag B maps to 1 raw; send `AttributeValueBatchPublished` with A+B; the publish-actor probe receives exactly ONE `AttributeValueBatchUpdate` containing 3 `ValueWrite`s with correct NodeId/Realm/value/status/timestamp. +2. `Unmapped_items_are_dropped_not_faulted` — batch with one unmapped ref: other items still land; no crash. +3. `Batch_reaches_mux_once` — mux probe receives ONE batch message carrying all items (not N). + +**Step 3: Implement** + +Replace the Task 4 bridge with a real handler: + +```csharp +private void ForwardBatchToMux(DriverInstanceActor.AttributeValueBatchPublished msg) +{ + _dependencyMux?.Tell(msg); // one envelope per burst; the mux filters per item (Task 9) + + if (_opcUaPublishActor is null) return; + + List? writes = null; + foreach (var item in msg.Items) + { + if (_nodeIdByDriverRef.TryGetValue((msg.DriverInstanceId, item.FullReference), out var nodeIds)) + { + writes ??= new List(msg.Items.Count * 2); + foreach (var n in nodeIds) + writes.Add(new ValueWrite(n.NodeId, item.Value, item.StatusCode, item.TimestampUtc, n.Realm)); + } + else if (_log.IsDebugEnabled) + { + _log.Debug("DriverHost {Node}: no bound NodeId for ({Driver},{Ref}) — value dropped", + _localNode, msg.DriverInstanceId, item.FullReference); + } + } + if (writes is not null) + _opcUaPublishActor.Tell(new ZB.MOM.WW.OtOpcUa.Runtime.OpcUa.OpcUaPublishActor.AttributeValueBatchUpdate(writes)); +} +``` + +Keep `ForwardToMux` and its `Receive` registration untouched (comms-loss fan + legacy tests). Register `Receive(ForwardBatchToMux);` in place of the Task 4 bridge. + +**Step 4:** `dotnet test tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests` — PASS. + +**Step 5:** `git add -A && git commit -m "feat(runtime): DriverHostActor batched value fan-out"` + +--- + +### Task 9: DependencyMuxActor batch receive + +**Classification:** small +**Estimated implement time:** ~3 min +**Parallelizable with:** Task 8 (same PR-let; do after 8's records exist) + +**Files:** +- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/DependencyMuxActor.cs` +- Test: `tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/` — add a batch case beside the existing mux tests (find them via `CalculationDependencyFlowTests.cs` / grep `DependencyMuxActor` in that project) + +**Step 1: Failing test** — register a subscriber for ref X; send a batch containing X and an uninteresting Y; subscriber receives exactly one `DependencyValueChanged` for X, nothing for Y. + +**Step 3: Implement** — add: + +```csharp +Receive(OnAttributeValueBatchPublished); +``` + +```csharp +private void OnAttributeValueBatchPublished(DriverInstanceActor.AttributeValueBatchPublished msg) +{ + foreach (var item in msg.Items) + { + if (!_byRef.TryGetValue(item.FullReference, out var subscribers) || subscribers.Count == 0) continue; + var dep = new VirtualTagActor.DependencyValueChanged(item.FullReference, item.Value, item.TimestampUtc, item.StatusCode); + foreach (var sub in subscribers) sub.Tell(dep); + } +} +``` + +Keep the per-item receive (comms-loss path). + +**Step 4/5:** project tests green; `git commit -m "feat(runtime): dependency mux accepts batched publishes"` + +--- + +### Task 10: Raise pinned-dispatcher throughput + +**Classification:** trivial +**Estimated implement time:** ~2 min +**Parallelizable with:** Task 1 + +**Files:** +- Modify: `src/Core/ZB.MOM.WW.OtOpcUa.Cluster/Resources/akka.conf:112-116` +- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ServiceCollectionExtensions.cs:301-312` (the inline HOCON copy) + +Change `throughput = 1` → `throughput = 100` in BOTH places (they must match — the inline HOCON exists for consumers without akka.conf; see the comment at `ServiceCollectionExtensions.cs:301-303`). Update the comments: pinning alone guarantees the SDK's single-thread requirement; throughput only sets how many queued messages drain per scheduling run, so 100 removes a mailbox re-schedule round-trip per message with zero concurrency change. + +Run: `dotnet test tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests --filter "FullyQualifiedName~ServiceCollectionExtensionsTests"` + +Commit: `perf(runtime): raise opcua pinned dispatcher throughput 1 -> 100` + +--- + +### Task 11: Re-key node-manager maps (kill per-write key allocations) + +**Classification:** high-risk (38 call sites, mechanical but wide) +**Estimated implement time:** ~5 min (compiler-driven) +**Parallelizable with:** none (after Task 6 — same file) + +**Files:** +- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs` + +**Step 1: Baseline** — run `dotnet test tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests` green BEFORE starting (this task relies on the existing suite as its net; it should need no new tests, but the whole suite must pass unchanged after). + +**Step 2: Implement** +- Add near `MapKey` (`:126-139`): + +```csharp +/// Zero-allocation realm-qualified map key. Replaces the formatted "ns=N;s=id" string key — +/// the (realm, id) pair is the same identity without the per-write NodeId + string allocations. +private readonly record struct RealmKey(AddressSpaceRealm Realm, string Id); +``` + +- Change `MapKey(AddressSpaceRealm realm, string id)` to return `new RealmKey(realm, id)`; change `MapKey(NodeId nodeId)` to `new RealmKey(RealmOf(nodeId.NamespaceIndex), (string)nodeId.Identifier)` — `RealmOf` already exists (~`:123`); confirm its exact name/signature and that every `MapKey(NodeId)` call site only ever passes nodes from the two owned namespaces (they do — SDK-facing seams on owned handles). If `nodeId.Identifier` can be non-string on any call path (numeric-id nodes), guard with `as string ?? nodeId.Identifier?.ToString() ?? string.Empty`. +- Change every map/set keyed by the old string key to `RealmKey`: `_variables`, `_folders`, `_alarmConditions`, `_historizedTagnames`, `_eventNotifierSources`, `_nativeAlarmNodeIds`, and the notifier-wiring bookkeeping — let the compiler find all of them (38 `MapKey` call sites). +- Prefix scans (`RemoveEquipmentSubtree` ~`:2539-2596`, `InScope` usages): match on `key.Realm == realm && key.Id.StartsWith(prefix, ...)` — semantics identical (the old string key embedded the realm as `ns=N;`). +- `ResolveNodeState(string key)` (~`:2240-2261`): change its parameter to `RealmKey`. +- Do NOT change public API surface or any behavior — this is a pure key-representation change. + +**Step 3:** `dotnet build ZB.MOM.WW.OtOpcUa.slnx` then `dotnet test tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests` and `dotnet test tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests` — ALL green, zero test edits expected (tests drive the public surface). If a test pokes the key format directly, stop and reassess rather than bending the test. + +**Step 4:** `git add -A && git commit -m "perf(opcuaserver): struct realm keys replace formatted NodeId string keys"` + +--- + +### Task 12: Full verification + docs + +**Classification:** standard +**Estimated implement time:** ~5 min +**Parallelizable with:** none (last) + +**Files:** +- Modify: `CLAUDE.md` (only if a statement it makes is now false — check the pinned-dispatcher/`throughput` mentions; keep edits surgical) +- Modify: `docs/v2/Galaxy.Performance.md` — add a short "value-path batching" note (one paragraph: the batch seams, the invariants, the dispatcher change) + +Steps: +1. `dotnet build ZB.MOM.WW.OtOpcUa.slnx` — zero warnings introduced. +2. `dotnet test ZB.MOM.WW.OtOpcUa.slnx` — run what the environment can (DB-backed suites need `10.100.0.35`; if unreachable, run every non-DB project under tests/Core, tests/Drivers (unit), tests/Server/Runtime.Tests, tests/Server/OpcUaServer.Tests and record which suites were skipped and why in the commit message). +3. Docs + commit: `docs: record value-pipeline batching (plan 2026-08-15)`. + +--- + +## Explicit non-goals (do NOT let scope creep into these) + +- **No conflation / bounded mailboxes** — batching per burst is this plan's backpressure story; keyed conflation is a follow-up. +- **No fan-out-map push-down to DriverInstanceActor** — the host stays the router. +- **No MQTT / MTConnect / Sparkplug / Galaxy / OpcUaClient batch implementations** — they flow through the single-item-batch path unchanged. +- **No change to alarm paths** (`AttributeAlarmPublished`, `AlarmStateUpdate`) — value path only. +- **No PollGroupEngine cadence change** (fixed-rate scheduling is a separate item). + +--- + +## Follow-ups (recorded, not implemented) + +Raised during Tasks 1–12 review and deliberately left out of this plan's scope. None is a defect in the +delivered work; each is a place where the next change in the area could go wrong quietly. + +1. **`PollGroupEngine` collects batch items unconditionally — the batch event is only as trustworthy as + the driver's `onChange`.** The engine has no knowledge of a driver's publish filter, so its items list + is pre-filter by construction. That is safe *today* only because S7, AbCip and FOCAS forward every + `onChange` unconditionally, and Modbus — the one driver with a deadband — ignores the engine's list and + raises from its own filtered buffer. The trap: a future per-tag filter added to S7/AbCip/FOCAS's + `onChange` would silently reintroduce the exact deadband-bypass class of bug fixed for Modbus in + `cd4ce305`, with a green test suite (the two events would simply disagree, and only a consumer + preferring the batch would notice). Consider changing the engine's `onChange` contract to + `Func<..., bool>` so the engine collects an item only when the driver reports it published — making the + filter structurally impossible to skip rather than a convention each driver must remember. +2. **Two-level `driverId → (rawPath → refs)` routing map in `DriverHostActor`.** The fan-out map is keyed + by a composite of driver id + raw path, so resolving a batch re-hashes the driver id once per item even + though a batch is by definition single-driver. Nesting the map would hash the driver id once per batch + and the raw path once per item. Measure first — batches are small enough that this may be noise + relative to the message-count win already banked. +3. **Null/empty guard consistency on the internal batch messages.** The guards on + `AttributeValueBatchPublished` / `AttributeValueBatchUpdate` grew per task and are not uniform — some + seams drop an empty batch early, others tolerate it downstream. These are internal messages with + trusted senders so nothing is unsafe today; worth one pass to make the contract identical at every hop + and stated once, rather than re-derived per reader. +4. **Possible merge of `DriverHostActorLiveValueTests` into the batch suite.** After Task 8 both files + drive the same fan-out through the same message; the split is now historical rather than meaningful. + Merging would remove a genuine ambiguity about where a new fan-out test belongs — but check first that + the older suite's per-item comms-loss cases really are covered, since those enter via a different seam. +5. **Aborted-cycle tail-loss when a per-tag handler throws, for the three unbuffered drivers.** If a + consumer's `onChange` throws mid-cycle in S7/AbCip/FOCAS, the cycle aborts and the items already + collected for that cycle never reach a batch raise. **Pre-existing and narrower than the Modbus case** + (Modbus's own buffer is per-handle and is dropped on unsubscribe/dispose, so it cannot leak), and it + predates this plan — the per-tag path lost the same tail before batching existed. Recorded so it is not + mistaken for a regression introduced here. diff --git a/docs/plans/2026-08-15-value-pipeline-batching.md.tasks.json b/docs/plans/2026-08-15-value-pipeline-batching.md.tasks.json new file mode 100644 index 00000000..f90c182e --- /dev/null +++ b/docs/plans/2026-08-15-value-pipeline-batching.md.tasks.json @@ -0,0 +1,19 @@ +{ + "planPath": "docs/plans/2026-08-15-value-pipeline-batching.md", + "tasks": [ + {"id": 1, "subject": "Task 1: Batch contract types in Core.Abstractions", "status": "completed"}, + {"id": 2, "subject": "Task 2: PollGroupEngine batch callback", "status": "completed", "blockedBy": [1]}, + {"id": 3, "subject": "Task 3a: Modbus + S7 raise the batch event", "status": "completed", "blockedBy": [2]}, + {"id": 4, "subject": "Task 3b: AbCip + FOCAS raise the batch event", "status": "completed", "blockedBy": [2]}, + {"id": 5, "subject": "Task 4: DriverInstanceActor batch forwarding", "status": "completed", "blockedBy": [1]}, + {"id": 6, "subject": "Task 5: ValueWrite + sink WriteValues default method", "status": "completed"}, + {"id": 7, "subject": "Task 6: OtOpcUaNodeManager batched write under one lock", "status": "completed", "blockedBy": [6]}, + {"id": 8, "subject": "Task 7: OpcUaPublishActor batch message", "status": "completed", "blockedBy": [6]}, + {"id": 9, "subject": "Task 8: DriverHostActor batched fan-out", "status": "completed", "blockedBy": [5, 8]}, + {"id": 10, "subject": "Task 9: DependencyMuxActor batch receive", "status": "completed", "blockedBy": [9]}, + {"id": 11, "subject": "Task 10: Raise pinned-dispatcher throughput", "status": "completed"}, + {"id": 12, "subject": "Task 11: Re-key node-manager maps", "status": "completed", "blockedBy": [7]}, + {"id": 13, "subject": "Task 12: Full verification + docs", "status": "completed", "blockedBy": [3, 4, 9, 10, 11, 12]} + ], + "lastUpdated": "2026-08-15T17:08:35Z" +} diff --git a/docs/v2/Galaxy.Performance.md b/docs/v2/Galaxy.Performance.md index d83e60a8..7fabe0a2 100644 --- a/docs/v2/Galaxy.Performance.md +++ b/docs/v2/Galaxy.Performance.md @@ -181,3 +181,56 @@ Safety: reuse is serialised under `_evalGate`, so two threads can never observe - `Reevaluation_reuses_the_same_read_cache_dictionary` — asserts dictionary instance identity across two evaluations. - `Reevaluation_reuses_the_same_predicate_context` — same, for the context. - `LoadAsync_drops_the_prior_generations_scratch` — asserts a publish resets the scratch. + +## Value-path batching (2026-08-15) + +The hot value path is batched end to end: one message per driver production burst (a poll cycle, an +inbound message, a stream chunk) instead of one per changed tag at every hop, one SDK-lock acquisition +per batch instead of one per value, and no per-write key allocation in the node manager. Nothing about +*what* is published changed — only how many messages and lock takes it costs. Plan + +task-by-task rationale: [`docs/plans/2026-08-15-value-pipeline-batching.md`](../plans/2026-08-15-value-pipeline-batching.md). + +**The seams, in order:** + +- **`IBatchSubscribable` / `PollGroupEngine`'s `onBatch`** — an optional driver capability raising + `OnDataChangeBatch` once per burst. The four engine-based poll drivers (Modbus, S7, AbCip, FOCAS) get + it from the engine's batch callback. The per-tag `ISubscribable.OnDataChange` is unchanged and still + raised — Galaxy cold reads and the driver CLIs subscribe it. A consumer subscribes to **exactly one** + of the two; subscribing both double-delivers. +- **`DriverInstanceActor` → `AttributeValueBatchPublished`** — one per burst, preferring the batch event + when the driver implements `IBatchSubscribable` and wrapping a non-batch driver's per-tag events as + single-item batches, so downstream sees one shape. The detach path tears the batch handler down too + (load-bearing: the `IDriver` outlives the actor across host respawns). +- **`DriverHostActor` → `AttributeValueBatchUpdate`** — resolves the whole batch's `NodeRealmRef`s + (raw NodeId + every referencing UNS NodeId) and sends **one** message to `OpcUaPublishActor`, plus one + batch Tell to the dependency mux. The mux keeps its per-item receive — the comms-loss fan-out + fabricates per-item messages. +- **`OtOpcUaNodeManager.WriteValues`** — the sink's batch override takes the manager-wide `Lock` once per + chunk of **512** items rather than once per value, with each item isolated so one bad node cannot lose + the rest of the batch. + +**Invariants the batching preserves — do not break them:** + +- **#519 status codes ride verbatim.** The full `uint` status code, sub-code included, reaches the SDK + unchanged at every hop; batching moved the values, it did not re-derive them. Timestamps likewise come + from the driver — nothing on this path stamps `DateTime.UtcNow`. +- **Metering stays per ITEM, not per batch** (`OpcUaPublishActor`'s write tally + the + `OtOpcUaTelemetry.OpcUaSinkWrite` meter). Counting per message would have silently dropped the + dashboards by up to 100×. +- **Deadband is honored on the batch event.** ⚠️ `PollGroupEngine` collects its batch list + **unconditionally** — it has no knowledge of a driver's publish filter — so `ModbusDriver` ignores the + engine's list and raises from its own buffer of what actually passed `ShouldPublish`, using `onBatch` + purely as the end-of-cycle signal. Forwarding the engine's list instead would carry changes the per-tag + event suppressed, and a consumer preferring the batch event would see deadband silently stop working. +- **Partial failure is typed, not swallowed.** A sink that applies some of a batch and fails the rest + reports `ValueWriteBatchException` (first failed NodeId + failed/succeeded counts); the publish actor's + typed catch advances the tally by the **succeeded** count — not zero (the general catch's behaviour) and + not the whole batch. + +**Two supporting changes:** the node manager's variable / folder / alarm / historized-tagname maps are +re-keyed by a `RealmKey(Realm, Id)` struct, so a write no longer allocates `new NodeId(id, ns).ToString()` +per value (the realm still travels explicitly — it is never parsed back out of an id string). And the +pinned `opcua-synchronized-dispatcher`'s `throughput` rose **1 → 100**: the actor keeps its own dedicated +thread (that is what the `PinnedDispatcher` guarantees, not `throughput`), so this only removes a mailbox +re-schedule round-trip per message on the hottest actor in the process, with zero change to concurrency. +Keep it in sync with the inline HOCON copy in `ServiceCollectionExtensions.WithOtOpcUaRuntimeActors`. diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/Resources/akka.conf b/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/Resources/akka.conf index 56648f0b..93f1a0aa 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/Resources/akka.conf +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/Resources/akka.conf @@ -109,8 +109,15 @@ akka { # Pinned dispatcher used by OpcUaPublishActor (Task 44) so the OPC UA SDK sees # only one thread per actor instance — its session/subscription locks expect # strict single-threaded access. +# +# throughput is NOT what provides that guarantee: the PinnedDispatcher does, by +# giving the actor its own dedicated thread. throughput only sets how many queued +# messages drain per scheduling run, so 100 removes a mailbox re-schedule +# round-trip per message on the hottest actor in the process with zero change to +# concurrency. Must stay in sync with the inline HOCON copy in +# ServiceCollectionExtensions.WithOtOpcUaRuntimeActors. opcua-synchronized-dispatcher { type = "PinnedDispatcher" executor = "thread-pool-executor" - throughput = 1 + throughput = 100 } diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/DeferredAddressSpaceSink.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/DeferredAddressSpaceSink.cs index b0fd6325..5c917dc0 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/DeferredAddressSpaceSink.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/DeferredAddressSpaceSink.cs @@ -26,6 +26,12 @@ public sealed class DeferredAddressSpaceSink : IOpcUaAddressSpaceSink, ISurgical public void WriteValue(string nodeId, object? value, uint statusCode, DateTime sourceTimestampUtc, AddressSpaceRealm realm) => _inner.WriteValue(nodeId, value, statusCode, sourceTimestampUtc, realm); + /// + // Forward the batch to the inner sink. Without this the interface's default per-item loop runs here and + // the one-lock-per-batch win is silently degraded back to one lock per value on every driver-role host + // (actors inject THIS wrapper, not the inner SdkAddressSpaceSink) — same trap as WireAlarmNotifiers above. + public void WriteValues(IReadOnlyList values) => _inner.WriteValues(values); + /// public void WriteAlarmCondition(string alarmNodeId, AlarmConditionSnapshot state, DateTime sourceTimestampUtc, AddressSpaceRealm realm) => _inner.WriteAlarmCondition(alarmNodeId, state, sourceTimestampUtc, realm); diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/IOpcUaAddressSpaceSink.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/IOpcUaAddressSpaceSink.cs index 78d2f720..2e240a6d 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/IOpcUaAddressSpaceSink.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/IOpcUaAddressSpaceSink.cs @@ -22,6 +22,26 @@ public interface IOpcUaAddressSpaceSink /// calls this once per NodeId (raw + every referencing UNS node) with the matching realm. void WriteValue(string nodeId, object? value, uint statusCode, DateTime sourceTimestampUtc, AddressSpaceRealm realm); + /// Write a batch of Variable values. Semantically identical to calling + /// per item; an SDK-backed sink overrides this to take its internal + /// lock ONCE per batch instead of once per value (the hot-path win). The default implementation + /// loops so existing sinks and fakes behave unchanged. + /// A batching implementation MUST isolate per item — one bad item may not drop the tail of the + /// batch — and report the aggregate as a ; see that type for why. + /// The default loop above predates the contract and is only used by fakes / the no-op sink. + /// The value writes to apply, in order. + /// A batching implementation throws this when some — but not + /// necessarily all — items failed; every other item has been applied. + void WriteValues(IReadOnlyList values) + { + ArgumentNullException.ThrowIfNull(values); + for (var i = 0; i < values.Count; i++) + { + var v = values[i]; + WriteValue(v.NodeId, v.Value, v.StatusCode, v.SourceTimestampUtc, v.Realm); + } + } + /// Write an alarm-condition's full Part 9 state. When a real condition node has been /// materialised for via , /// this projects the whole @@ -165,6 +185,35 @@ public interface IOpcUaAddressSpaceSink string referenceType = "Organizes"); } +/// One value write within a batch. +/// Same semantics per field as (#519: status verbatim). +public readonly record struct ValueWrite( + string NodeId, object? Value, uint StatusCode, DateTime SourceTimestampUtc, AddressSpaceRealm Realm); + +/// Aggregate outcome of a partially-failed batch: +/// every non-failing item HAS been applied; this carries what the caller needs to log and meter honestly. +/// A batched sink must NOT abandon the tail of a batch when one item throws: the poll engine +/// updates its last-value cache BEFORE raising, so a dropped item is never re-raised until that tag's value +/// changes again — the node would serve a stale value indefinitely. Hence the sink isolates per item and +/// reports the aggregate here rather than letting the first failure escape. +/// The NodeId of the first item that failed (empty when that item's own +/// NodeId was null/empty — the validation failure this type most often reports). +/// How many items of the batch failed. +/// How many items of the batch were applied. +/// The exception the first failing item produced; the inner exception. +public sealed class ValueWriteBatchException(string firstFailedNodeId, int failedCount, int succeededCount, Exception firstFailure) + : Exception($"WriteValues batch: {failedCount} of {failedCount + succeededCount} items failed (first: '{firstFailedNodeId}')", firstFailure) +{ + /// Gets the NodeId of the first item that failed. + public string FirstFailedNodeId { get; } = firstFailedNodeId; + + /// Gets the number of items of the batch that failed. + public int FailedCount { get; } = failedCount; + + /// Gets the number of items of the batch that were successfully applied. + public int SucceededCount { get; } = succeededCount; +} + /// No-op sink. Bound by default so the actors are safe to run in dev / Mac / /// integration tests without a real SDK behind them. public sealed class NullOpcUaAddressSpaceSink : IOpcUaAddressSpaceSink diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/ISubscribable.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/ISubscribable.cs index 9787bf2d..a1dd8bae 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/ISubscribable.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/ISubscribable.cs @@ -51,3 +51,31 @@ public sealed record DataChangeEventArgs( ISubscriptionHandle SubscriptionHandle, string FullReference, DataValueSnapshot Snapshot); + +/// One changed attribute within a . +/// Driver-side full reference of the changed attribute. +/// New value + quality + timestamps. +public sealed record DataChangeItem(string FullReference, DataValueSnapshot Snapshot); + +/// Event payload for — every attribute +/// that changed within ONE production burst (a poll cycle, an inbound message, a stream chunk). +/// The handle returned by the original call. +/// The changed attributes, one entry per changed tag. Never empty. Ownership transfers +/// to the consumer, which may retain the list past the raise (it is marshalled onto another thread) — so the +/// driver MUST NOT mutate or reuse the list instance after raising; allocate a fresh one per burst. +public sealed record DataChangeBatchEventArgs( + ISubscriptionHandle SubscriptionHandle, + IReadOnlyList Items); + +/// +/// Optional driver capability: data changes delivered one event per production burst instead of +/// one event per tag. A driver implementing this raises once per +/// burst AND continues raising the per-tag for each item +/// (legacy consumers: Galaxy cold reads, driver CLIs). A consumer subscribes to EXACTLY ONE of the +/// two events — subscribing both double-delivers. DriverInstanceActor prefers the batch event. +/// +public interface IBatchSubscribable : ISubscribable +{ + /// Batched data-change notification — one raise per production burst. + event EventHandler? OnDataChangeBatch; +} diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/PollGroupEngine.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/PollGroupEngine.cs index e0d29342..8447bda4 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/PollGroupEngine.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/PollGroupEngine.cs @@ -24,6 +24,15 @@ namespace ZB.MOM.WW.OtOpcUa.Core.Abstractions; /// can record the failure. Without an onError callback the exception is silently /// swallowed (preserves the original behaviour for drivers that have not opted in yet). /// +/// An optional onBatch callback delivers the SAME changes collected per poll +/// cycle: it is raised at most once per cycle, only when that cycle produced at least one +/// change, and always AFTER every per-tag onChange callback of that same cycle. Drivers +/// forward it to so downstream consumers can +/// take one message per cycle instead of one per tag. Both callbacks fire by design — a +/// consumer subscribes to exactly one of the two. Without an onBatch callback the engine +/// behaves byte-identically to before: no batch is collected and no per-cycle allocation is +/// made. +/// /// Programmer errors and obviously-fatal exceptions (, /// , , /// ) are NOT caught — they propagate and tear the poll @@ -34,6 +43,7 @@ public sealed class PollGroupEngine : IAsyncDisposable private readonly Func, CancellationToken, Task>> _reader; private readonly Action _onChange; private readonly Action? _onError; + private readonly Action>? _onBatch; private readonly TimeSpan _minInterval; private readonly TimeSpan? _backoffCap; private readonly ConcurrentDictionary _subscriptions = new(); @@ -58,12 +68,18 @@ public sealed class PollGroupEngine : IAsyncDisposable /// (interval, 2×, 4×, … saturating at this cap) instead of hammering a dead device every /// tick; a successful poll resets to the base interval. null (default) keeps the /// byte-identical fixed-interval cadence for consumers that have not opted in. + /// Optional per-cycle batch callback — invoked once per poll cycle that + /// produced at least one change, carrying exactly that cycle's changed items in reader + /// order, after all of the cycle's per-tag callbacks. The + /// driver forwards it to its own event. + /// null (default) collects nothing and allocates nothing per cycle. public PollGroupEngine( Func, CancellationToken, Task>> reader, Action onChange, TimeSpan? minInterval = null, Action? onError = null, - TimeSpan? backoffCap = null) + TimeSpan? backoffCap = null, + Action>? onBatch = null) { ArgumentNullException.ThrowIfNull(reader); ArgumentNullException.ThrowIfNull(onChange); @@ -72,6 +88,7 @@ public sealed class PollGroupEngine : IAsyncDisposable _onError = onError; _minInterval = minInterval ?? DefaultMinInterval; _backoffCap = backoffCap; + _onBatch = onBatch; } /// Register a new polled subscription and start its background loop. @@ -211,6 +228,10 @@ public sealed class PollGroupEngine : IAsyncDisposable $"Reader contract violation: expected {state.TagReferences.Count} snapshots but received {snapshots.Count}. " + "The reader delegate must return one snapshot per input reference in input order."); + // Collected lazily: nothing is allocated for a cycle with no changes, and nothing at all + // when no batch callback is wired. + List? changed = null; + for (var i = 0; i < state.TagReferences.Count; i++) { var tagRef = state.TagReferences[i]; @@ -221,8 +242,15 @@ public sealed class PollGroupEngine : IAsyncDisposable { state.LastValues[tagRef] = current; _onChange(state.Handle, tagRef, current); + // Capacity hint: a cycle can produce at most one item per subscribed reference, so + // sizing the list once avoids the doubling re-allocations a large group would incur. + if (_onBatch is not null) + (changed ??= new List(state.TagReferences.Count)).Add(new DataChangeItem(tagRef, current)); } } + + // One raise per cycle, after every per-tag callback of that cycle, never empty. + if (changed is { Count: > 0 }) _onBatch!(state.Handle, changed); } /// diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs index 2d6c6d06..25e2625e 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs @@ -24,7 +24,7 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip; /// (called in ); the library's own finalizer /// handles GC-collected tags. /// -public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, ISubscribable, +public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, IBatchSubscribable, IHostConnectivityProbe, IPerCallHostResolver, IAlarmSource, ISkippedTagReporter, IDisposable, IAsyncDisposable { /// Authored tags this driver could not build into its poll table (Gitea #541). @@ -114,6 +114,14 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, /// Occurs when a subscribed tag's value changes. public event EventHandler? OnDataChange; + /// + /// Occurs once per poll cycle that produced at least one change, carrying that cycle's + /// changed tags. Both data-change events fire by design — this one once per cycle, + /// once per changed tag — so a consumer subscribes to EXACTLY + /// ONE of the two; subscribing both double-delivers. + /// + public event EventHandler? OnDataChangeBatch; + /// Occurs when a device's host connectivity status changes. public event EventHandler? OnHostStatusChanged; @@ -151,7 +159,9 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, onChange: (handle, tagRef, snapshot) => OnDataChange?.Invoke(this, new DataChangeEventArgs(handle, tagRef, snapshot)), onError: HandlePollError, - backoffCap: PollBackoffCap); + backoffCap: PollBackoffCap, + onBatch: (handle, items) => + OnDataChangeBatch?.Invoke(this, new DataChangeBatchEventArgs(handle, items))); _alarmProjection = new AbCipAlarmProjection(this, _options.AlarmPollInterval, _logger); } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs index 8c9aabfc..67aef9eb 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs @@ -18,7 +18,7 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.FOCAS; /// + the default makes misconfigured servers /// fail fast. /// -public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, ISubscribable, +public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, IBatchSubscribable, IHostConnectivityProbe, IPerCallHostResolver, IAlarmSource, ISkippedTagReporter, IDisposable, IAsyncDisposable { /// Authored tags this driver could not build into its poll table (Gitea #541). FOCAS has three @@ -80,6 +80,13 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, /// Occurs when data changes on a subscribed tag. public event EventHandler? OnDataChange; + /// + /// Occurs once per poll cycle that produced at least one change, carrying that cycle's + /// changed tags. Both data-change events fire by design — this one once per cycle, + /// once per changed tag — so a consumer subscribes to EXACTLY + /// ONE of the two; subscribing both double-delivers. + /// + public event EventHandler? OnDataChangeBatch; /// Occurs when a device host connection status changes. public event EventHandler? OnHostStatusChanged; /// Occurs when an alarm event is raised. @@ -112,7 +119,9 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, onChange: (handle, tagRef, snapshot) => OnDataChange?.Invoke(this, new DataChangeEventArgs(handle, tagRef, snapshot)), onError: HandlePollError, - backoffCap: PollBackoffCap); + backoffCap: PollBackoffCap, + onBatch: (handle, items) => + OnDataChangeBatch?.Invoke(this, new DataChangeBatchEventArgs(handle, items))); } /// True when the supplied DriverConfig JSON carries a real body. The bootstrapper always passes diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs index 45d358f5..c937c893 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs @@ -19,7 +19,7 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Modbus; /// since Modbus has no native push model. /// public sealed class ModbusDriver - : IDriver, ITagDiscovery, IReadable, IWritable, ISubscribable, IHostConnectivityProbe, IPerCallHostResolver, ISkippedTagReporter, IDisposable, IAsyncDisposable + : IDriver, ITagDiscovery, IReadable, IWritable, IBatchSubscribable, IHostConnectivityProbe, IPerCallHostResolver, ISkippedTagReporter, IDisposable, IAsyncDisposable { /// Authored tags this driver could not build into its poll table (Gitea #541). private readonly SkippedTagLedger _skippedTags = new(); @@ -53,6 +53,17 @@ public sealed class ModbusDriver // does a typed cast inside. private readonly ConcurrentDictionary _lastPublishedByRef = new(StringComparer.OrdinalIgnoreCase); + // Per-cycle buffer of the items this driver actually PUBLISHED (i.e. that passed ShouldPublish), + // keyed by subscription handle. The poll engine collects its own batch list unconditionally — it + // has no knowledge of the deadband filter — so the batch event is raised from this buffer instead, + // using the engine's onBatch purely as the end-of-cycle signal. + // + // No lock is needed on the per-handle List: the engine runs one loop task per subscription, so for + // ONE handle all of a cycle's onChange calls and that cycle's single onBatch run sequentially on + // that subscription's own task. Different subscriptions run concurrently — hence the per-handle + // ConcurrentDictionary. + private readonly ConcurrentDictionary> _cycleBatchItems = new(); + // Last-written value per tag for the WriteOnChangeOnly suppression. Invalidated by reads // that return a different value (so an HMI-side change doesn't get masked). private readonly Dictionary _lastWrittenByRef = new(StringComparer.OrdinalIgnoreCase); @@ -80,6 +91,13 @@ public sealed class ModbusDriver /// Occurs when a subscribed tag value changes. public event EventHandler? OnDataChange; + /// + /// Occurs once per poll cycle that produced at least one change, carrying that cycle's + /// changed tags. Both data-change events fire by design — this one once per cycle, + /// once per changed tag — so a consumer subscribes to EXACTLY + /// ONE of the two; subscribing both double-delivers. + /// + public event EventHandler? OnDataChangeBatch; /// Occurs when host connectivity status changes. public event EventHandler? OnHostStatusChanged; @@ -122,10 +140,35 @@ public sealed class ModbusDriver onChange: (handle, tagRef, snapshot) => { if (!ShouldPublish(tagRef, snapshot)) return; - OnDataChange?.Invoke(this, new DataChangeEventArgs(handle, tagRef, snapshot)); + // Buffer what we publish so the cycle-end batch carries exactly the same items. + // The append stays OUTSIDE the catch below on purpose: as far as the batch contract + // is concerned the item WAS published — only the per-tag subscriber faulted — so it + // must still appear in this cycle's batch, keeping the two events content-identical. + _cycleBatchItems.GetOrAdd(handle, _ => new List()) + .Add(new DataChangeItem(tagRef, snapshot)); + // Defensive, mirroring PollGroupEngine.ReportError: the engine invokes this bridge + // UNGUARDED, so a throwing external subscriber would unwind PollOnceAsync mid-loop + // and the cycle would never reach onBatch. Because the buffer above persists across + // cycles (it is drained only by onBatch), those items would then silently MERGE into + // a later cycle's batch — cross-cycle contamination unique to this driver. Swallowing + // is what keeps the cycle running so onBatch drains the buffer on schedule. + try { OnDataChange?.Invoke(this, new DataChangeEventArgs(handle, tagRef, snapshot)); } + catch { /* a buggy subscriber must not corrupt the poll cycle */ } }, onError: HandlePollError, - backoffCap: PollBackoffCap); + backoffCap: PollBackoffCap, + // The engine's own items list is deliberately IGNORED: it is collected pre-deadband + // (PollGroupEngine has no knowledge of ShouldPublish), so forwarding it would carry + // changes the per-tag event suppressed — and a consumer that prefers the batch event + // would see deadband silently stop working. The buffer holds exactly what the per-tag + // event published, keeping the two events content-identical per the IBatchSubscribable + // contract. onBatch is used only as the end-of-cycle signal. A cycle whose every change + // was suppressed leaves the buffer empty and raises nothing. + onBatch: (handle, _) => + { + if (_cycleBatchItems.TryRemove(handle, out var published) && published.Count > 0) + OnDataChangeBatch?.Invoke(this, new DataChangeBatchEventArgs(handle, published)); + }); } /// Upper bound on the poll-loop failure backoff — adopts the S7-proven 30 s cap fleet-wide (05/STAB-8). @@ -1207,6 +1250,9 @@ public sealed class ModbusDriver public Task UnsubscribeAsync(ISubscriptionHandle handle, CancellationToken cancellationToken) { _poll.Unsubscribe(handle); + // Drop any partially-collected cycle buffer so a torn-down subscription leaks nothing. + // Unsubscribe awaits the loop task, so no onChange can re-add after this point. + _cycleBatchItems.TryRemove(handle, out _); return Task.CompletedTask; } @@ -1648,6 +1694,8 @@ public sealed class ModbusDriver lock (_autoProhibitedLock) _autoProhibited.Clear(); await _poll.DisposeAsync().ConfigureAwait(false); + // After the engine's loops are drained, no onChange can re-populate the per-cycle buffers. + _cycleBatchItems.Clear(); if (_transport is not null) await _transport.DisposeAsync().ConfigureAwait(false); _transport = null; diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7Driver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7Driver.cs index 04ebf8ae..14378ca3 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7Driver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7Driver.cs @@ -30,7 +30,7 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.S7; /// /// public sealed class S7Driver - : IDriver, ITagDiscovery, IReadable, IWritable, ISubscribable, IHostConnectivityProbe, ISkippedTagReporter, IDisposable, IAsyncDisposable + : IDriver, ITagDiscovery, IReadable, IWritable, IBatchSubscribable, IHostConnectivityProbe, ISkippedTagReporter, IDisposable, IAsyncDisposable { /// Authored tags this driver could not build into its poll table (Gitea #541). private readonly SkippedTagLedger _skippedTags = new(); @@ -68,7 +68,8 @@ public sealed class S7Driver OnDataChange?.Invoke(this, new DataChangeEventArgs(handle, tagRef, snapshot)), minInterval: TimeSpan.FromMilliseconds(100), onError: HandlePollError, - backoffCap: TimeSpan.FromSeconds(30)); + backoffCap: TimeSpan.FromSeconds(30), + onBatch: (handle, items) => OnDataChangeBatch?.Invoke(this, new DataChangeBatchEventArgs(handle, items))); } // ---- ISubscribable + IHostConnectivityProbe state ---- @@ -93,6 +94,13 @@ public sealed class S7Driver /// Occurs when a subscribed tag value or status code changes. public event EventHandler? OnDataChange; + /// + /// Occurs once per poll cycle that produced at least one change, carrying that cycle's + /// changed tags. Both data-change events fire by design — this one once per cycle, + /// once per changed tag — so a consumer subscribes to EXACTLY + /// ONE of the two; subscribing both double-delivers. + /// + public event EventHandler? OnDataChangeBatch; /// Occurs when host connectivity status changes. public event EventHandler? OnHostStatusChanged; diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs index ed397b0a..45248cde 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs @@ -52,21 +52,21 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 /// v3 URIs are the real namespaces; this alias is removed once those tests target Raw/UNS explicitly. public const string DefaultNamespaceUri = V3NodeIds.UnsNamespaceUri; - private readonly ConcurrentDictionary _variables = new(StringComparer.Ordinal); - private readonly ConcurrentDictionary _folders = new(StringComparer.Ordinal); - private readonly ConcurrentDictionary _alarmConditions = new(StringComparer.Ordinal); + private readonly ConcurrentDictionary _variables = new(); + private readonly ConcurrentDictionary _folders = new(); + private readonly ConcurrentDictionary _alarmConditions = new(); /// H6a: the subset of node ids materialised as NATIVE /// (driver-fed, e.g. Galaxy equipment-tag alarms) rather than scripted. A later task routes a native /// condition's inbound Acknowledge to the driver instead of the scripted engine, so the node manager /// must know which conditions are native. Maintained in lock-step with : /// a native re-materialise adds, and clears it alongside /// so a re-materialise as the other kind is correct. - private readonly HashSet _nativeAlarmNodeIds = new(StringComparer.Ordinal); + private readonly HashSet _nativeAlarmNodeIds = new(); /// Phase C: NodeId → resolved historian tagname for every variable materialised /// Historizing. Populated by when a historian tagname is supplied; the /// (later) HistoryRead override resolves a HistoryRead request's NodeId against this map. Cleared on /// . - private readonly ConcurrentDictionary _historizedTagnames = new(StringComparer.Ordinal); + private readonly ConcurrentDictionary _historizedTagnames = new(); /// Folders we have already promoted to event-notifiers + registered as root notifiers, /// so repeated calls don't double-add (idempotent guard). /// Keyed by NodeId → the actual so can @@ -81,7 +81,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 /// each wired pair is torn down bidirectionally (RemoveNotifier(bidirectional:true)) so an /// inverse-notifier entry never leaks across redeploys. Guarded by the same Lock as /// / . - private readonly Dictionary _alarmNotifierWiring = new(StringComparer.Ordinal); + private readonly Dictionary _alarmNotifierWiring = new(); /// Phase C: event-notifier folder NodeId-identifier → the event-history source /// name passed to . The equipment-folder NodeId /// identifier IS the equipment id, which IS the sourceName, so key and value are the same string; @@ -90,7 +90,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 /// and the override resolves an inbound request's notifier NodeId /// against it (a miss ⇒ BadHistoryOperationUnsupported). Cleared on /// . - private readonly ConcurrentDictionary _eventNotifierSources = new(StringComparer.Ordinal); + private readonly ConcurrentDictionary _eventNotifierSources = new(); private FolderState? _root; /// Initializes a new instance of the class with the OPC UA server and configuration. @@ -123,20 +123,43 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 private AddressSpaceRealm RealmOf(NodeId nodeId) => nodeId.NamespaceIndex == NamespaceIndexes[0] ? AddressSpaceRealm.Raw : AddressSpaceRealm.Uns; - /// Build the internal map key for a realm-qualified string id. The key is the node's full - /// namespace-qualified NodeId string (e.g. ns=2;s=<id>) so a Raw node and a UNS node that - /// share the same s= identifier are distinct keys — a bare id is no longer globally unique across - /// the two namespaces. Matches for the same node. + /// Zero-allocation realm-qualified map key. Replaces the formatted "ns=N;s=id" string key — + /// the (realm, id) pair is the same identity without the per-write NodeId + string allocations + /// (the id string is already in hand at every call site). + /// The realm the id lives in. + /// The bare s= identifier. + private readonly record struct RealmKey(AddressSpaceRealm Realm, string Id); + + /// Build the internal map key for a realm-qualified string id. The key pairs the realm with the + /// bare s= identifier so a Raw node and a UNS node that share the same identifier are distinct + /// keys — a bare id is not globally unique across the two namespaces. Matches + /// for the same node. /// The realm the id lives in. /// The bare s= identifier. - /// The namespace-qualified map key. - private string MapKey(AddressSpaceRealm realm, string id) => new NodeId(id, NamespaceIndexForRealm(realm)).ToString(); + /// The realm-qualified map key. + private static RealmKey MapKey(AddressSpaceRealm realm, string id) => new(realm, id); /// Build the internal map key for an already-resolved SDK (the SDK-facing - /// seams). Equal to for the same node. + /// seams). Equal to for the same node. The SDK-facing + /// seams only ever pass nodes this manager minted (string s= identifiers); a non-string + /// identifier falls back to its string form rather than throwing — and says so loudly, because such a + /// key cannot match anything the manager stored, so the lookup would silently miss. /// The resolved SDK node id. - /// The namespace-qualified map key. - private static string MapKey(NodeId nodeId) => nodeId.ToString(); + /// The realm-qualified map key. + private RealmKey MapKey(NodeId nodeId) + { + if (nodeId.Identifier is string s) return new RealmKey(RealmOf(nodeId), s); + + // Unreachable by construction — every call site passes a node this manager minted with a string + // `s=` identifier. If it ever fires, the derived key resolves to nothing and the caller sees an + // unexplained miss, so name the seam rather than collapsing quietly. +#pragma warning disable CS0618 // Utils.LogWarning is [Obsolete] in favour of an ITelemetryContext this manager doesn't carry. + Utils.LogWarning( + "OtOpcUaNodeManager.MapKey: non-string NodeId identifier reached the map-key seam (node {0}, identifier type {1}) — the derived key will not resolve", + nodeId, nodeId.Identifier?.GetType().Name ?? "null"); +#pragma warning restore CS0618 + return new RealmKey(RealmOf(nodeId), nodeId.Identifier?.ToString() ?? string.Empty); + } /// Gets the count of variable nodes currently managed. public int VariableCount => _variables.Count; @@ -503,8 +526,10 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 /// /// The node identifier of the variable. /// The new value to write. - /// The OPC UA quality status code. + /// The full OPC UA status code, sub-code included (#519 — passed to the SDK verbatim). /// The timestamp of the value in UTC. + /// Which of the two v3 namespaces the lives in; the + /// namespace index is resolved from this rather than parsed out of the id string. public void WriteValue(string nodeId, object? value, uint statusCode, DateTime sourceTimestampUtc, AddressSpaceRealm realm) { ArgumentException.ThrowIfNullOrEmpty(nodeId); @@ -513,23 +538,135 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 var ns = NamespaceIndexForRealm(realm); var key = MapKey(realm, nodeId); lock (Lock) + WriteValueLocked(nodeId, ns, key, value, statusCode, sourceTimestampUtc); + } + + /// The most items applies under a SINGLE Lock acquisition before + /// releasing and re-taking it. + /// WHY a cap at all: the lock take is what batching amortises, and the amortisation curve is + /// brutally front-loaded — going from one take per value to one per 512 already removes >99.8% of the + /// takes, so an unbounded hold buys essentially nothing more. What it DOES buy is a stall: the manager-wide + /// Lock is the same lock every SDK service thread (Read / Browse / subscription publish / + /// ConditionRefresh) contends for, and the first poll cycle force-raises EVERY tag (the initial-data push), + /// so batch #1 is a whole device × its UNS fan-out — thousands of CreateVariable calls, each mutating + /// the SDK address space, under one acquisition. 512 is therefore "effectively unbounded for the win, + /// bounded for the stall": chunking hands the SDK a scheduling point between chunks. + private const int MaxWritesPerLockAcquisition = 512; + + /// Batched : one Lock acquisition per chunk of + /// items instead of one per value — the publish path's per-value + /// lock take was the steady-state throughput ceiling. Per-item semantics are identical to + /// (create-on-first-write, #519 verbatim status codes, ClearChangeMasks per node). + /// Items are isolated. A bad item (empty NodeId, or a throw out of the SDK) does not abandon + /// the rest of the batch: it is recorded and the loop continues, and the aggregate surfaces afterwards as + /// a . Dropping the tail would be silent and durable — the poll + /// engine updates its last-value cache BEFORE raising, so a dropped item is not re-raised until that tag's + /// value changes again, leaving the node stale indefinitely. + /// The value writes to apply, in order. + /// When one or more items failed. Every other item HAS been + /// applied; the exception carries the first failing NodeId plus the failed/succeeded split so the caller + /// can log and meter honestly. + public void WriteValues(IReadOnlyList values) + { + ArgumentNullException.ThrowIfNull(values); + // Guard BEFORE the empty-batch early-return, so a pre-start call fails the same legible way every + // other mutator does regardless of what it happens to carry. + EnsureAddressSpaceCreated(); + if (values.Count == 0) return; + + // Pre-pass, OUTSIDE the lock: validate each item and resolve its namespace index + map key into a + // scratch array. Both jobs belong here — validating outside keeps a malformed item from ever reaching + // the SDK, and resolving outside moves the MapKey string allocation (a NodeId + ToString per item) + // off the critical section entirely. + var resolved = new ResolvedWrite[values.Count]; + string? firstFailedNodeId = null; + Exception? firstFailure = null; + var failed = 0; + + for (var i = 0; i < values.Count; i++) { - // CreateVariable mutates the SDK address space (_root.AddChild + AddPredefinedNode), - // so it MUST run under Lock — the SDK's subscription/ConditionRefresh threads take it too. - if (!_variables.TryGetValue(key, out var variable)) + var nodeId = values[i].NodeId; + if (string.IsNullOrEmpty(nodeId)) { - variable = CreateVariable(nodeId, ns); - _variables[key] = variable; + failed++; + firstFailedNodeId ??= nodeId ?? string.Empty; + firstFailure ??= new ArgumentException( + $"WriteValues item {i} has a null or empty NodeId.", nameof(values)); + continue; } - variable.Value = value; - // #519 — the caller's status code reaches the SDK verbatim, sub-code included. This used to - // re-expand a 3-state enum, so every Bad became a bare StatusCodes.Bad (0x80000000) and a - // driver's BadNoCommunication was indistinguishable from BadTypeMismatch at the client. - variable.StatusCode = new StatusCode(statusCode); - variable.Timestamp = sourceTimestampUtc; - variable.ClearChangeMasks(SystemContext, includeChildren: false); + resolved[i] = new ResolvedWrite( + nodeId, NamespaceIndexForRealm(values[i].Realm), MapKey(values[i].Realm, nodeId), IsValid: true); } + + var succeeded = 0; + for (var start = 0; start < values.Count; start += MaxWritesPerLockAcquisition) + { + var end = Math.Min(start + MaxWritesPerLockAcquisition, values.Count); + lock (Lock) + { + for (var i = start; i < end; i++) + { + if (!resolved[i].IsValid) continue; + var v = values[i]; + try + { + WriteValueLocked(resolved[i].NodeId, resolved[i].NamespaceIndex, resolved[i].Key, + v.Value, v.StatusCode, v.SourceTimestampUtc); + succeeded++; + } + catch (Exception ex) + { + failed++; + firstFailedNodeId ??= v.NodeId; + firstFailure ??= ex; + } + } + } + } + + if (failed > 0) + { + throw new ValueWriteBatchException( + firstFailedNodeId ?? string.Empty, failed, succeeded, firstFailure!); + } + } + + /// One item after the outside-the-lock pre-pass: its namespace index and + /// map key already resolved, or false when the item failed validation. + /// The item's bare s= identifier. + /// The namespace index resolved from the item's realm. + /// The realm-qualified map key. + /// False when the item failed the pre-pass and must be skipped. + private readonly record struct ResolvedWrite(string NodeId, ushort NamespaceIndex, RealmKey Key, bool IsValid); + + /// The body of a single value write. Caller MUST hold Lock — this is factored out + /// solely so can apply a whole batch under one acquisition; it is byte-identical + /// to what has always done inside its own lock. + /// The bare s= identifier of the variable. + /// The namespace index resolved from the write's realm. + /// The realm-qualified map key for . + /// The new value to write. + /// The full OPC UA status code, sub-code included. + /// The timestamp of the value in UTC. + private void WriteValueLocked(string nodeId, ushort ns, RealmKey key, object? value, uint statusCode, DateTime sourceTimestampUtc) + { + // CreateVariable mutates the SDK address space (_root.AddChild + AddPredefinedNode), + // so it MUST run under Lock — the SDK's subscription/ConditionRefresh threads take it too. + if (!_variables.TryGetValue(key, out var variable)) + { + variable = CreateVariable(nodeId, ns); + _variables[key] = variable; + } + + variable.Value = value; + // #519 — the caller's status code reaches the SDK verbatim, sub-code included. This used to + // re-expand a 3-state enum, so every Bad became a bare StatusCodes.Bad (0x80000000) and a + // driver's BadNoCommunication was indistinguishable from BadTypeMismatch at the client. + variable.StatusCode = new StatusCode(statusCode); + variable.Timestamp = sourceTimestampUtc; + // Stays INSIDE the lock, per node: this is how the SDK notifies monitored items of the change. + variable.ClearChangeMasks(SystemContext, includeChildren: false); } /// @@ -1138,7 +1275,10 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 // ReportEvent catch below for the same pattern). Utils.LogInfo is [Obsolete] in 1.5.378 in // favour of an ITelemetryContext this manager doesn't wire — suppress the deprecation. #pragma warning disable CS0618 // Type or member is obsolete - Utils.LogInfo("OtOpcUaNodeManager.WireAlarmNotifiers: condition {0} not materialised — skipping notifier wiring", alarmKey); + // Interpolate the key's PARTS, not the RealmKey struct — a struct dump would render a shape + // no log-grep pattern in this file matches. + Utils.LogInfo("OtOpcUaNodeManager.WireAlarmNotifiers: condition {0} (realm {1}) not materialised — skipping notifier wiring", + alarmKey.Id, alarmKey.Realm); #pragma warning restore CS0618 return; } @@ -1155,7 +1295,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 // supplied, regardless of whether each is currently materialised. Reconcile against it below. var desiredKeys = notifierFolderNodeIds .Select(id => MapKey(notifierFolderRealm, id)) - .ToHashSet(StringComparer.Ordinal); + .ToHashSet(); // Reconcile (M3): unwire any previously-wired folder whose id is NO LONGER in the supplied set — a // genuine de-reference (the equipment dropped its reference to the alarm's raw tag). Matched by the @@ -1173,8 +1313,8 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 { // Referencing-equipment folder not (yet) materialised — skip this one (logged, never thrown). #pragma warning disable CS0618 // Type or member is obsolete - Utils.LogInfo("OtOpcUaNodeManager.WireAlarmNotifiers: notifier folder {0} for condition {1} not present — skipped", - folderNodeId, alarmKey); + Utils.LogInfo("OtOpcUaNodeManager.WireAlarmNotifiers: notifier folder {0} for condition {1} (realm {2}) not present — skipped", + folderNodeId, alarmKey.Id, alarmKey.Realm); #pragma warning restore CS0618 continue; } @@ -1200,7 +1340,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 /// Tear down (bidirectionally) every notifier wired for the condition at /// and drop its tracking entry. MUST be called under Lock. Used on condition-removal + full rebuild /// so no inverse-notifier entry leaks. A no-op when the condition has no wired notifiers. - private void UnwireAlarmNotifiers(string alarmKey) + private void UnwireAlarmNotifiers(RealmKey alarmKey) { if (!_alarmNotifierWiring.TryGetValue(alarmKey, out var wiring)) return; foreach (var folder in wiring.Folders) @@ -1817,8 +1957,8 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 // A historian is wired: advertise event history on this notifier and register it as a source. // The equipment-folder NodeId identifier IS the equipment id IS the ReadEventsAsync sourceName. folder.EventNotifier = (byte)(folder.EventNotifier | EventNotifiers.HistoryRead); - // Key by the full (namespace-qualified) NodeId string so a Raw and a UNS folder that share the - // same bare id are distinct sources; the VALUE stays the bare id — that is the historian + // Key by the realm-qualified RealmKey built from the NodeId so a Raw and a UNS folder that share + // the same bare id are distinct sources; the VALUE stays the bare id — that is the historian // ReadEventsAsync sourceName (== the equipment id). var sourceName = folder.NodeId.Identifier?.ToString() ?? string.Empty; _eventNotifierSources[MapKey(folder.NodeId)] = sourceName; @@ -2239,7 +2379,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 /// to link two already-materialised nodes regardless of their kind. /// The namespace-qualified map key (see ). /// The node state, or null when unknown. - private NodeState? ResolveNodeState(string key) + private NodeState? ResolveNodeState(RealmKey key) { if (_variables.TryGetValue(key, out var variable)) return variable; if (_folders.TryGetValue(key, out var folder)) return folder; @@ -2527,13 +2667,14 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 // Unknown equipment id ⇒ map drift ⇒ false (caller rebuilds). if (!_folders.ContainsKey(equipKey)) return false; - // Maps key on the full namespace-qualified NodeId string (e.g. "ns=3;s="). Every - // descendant in the SAME realm shares that key exactly or begins with "/" (sub-folders, - // variables, condition nodes); the "ns=N;s=" prefix makes the match realm-scoped for free, so a - // node in the OTHER namespace that happens to share a bare id is never in scope. Snapshot each - // map's in-scope keys first (ToList) so we don't mutate while enumerating. - var prefix = equipKey + "/"; - bool InScope(string key) => key == equipKey || key.StartsWith(prefix, StringComparison.Ordinal); + // Maps key on the (realm, bare id) pair. Every descendant in the SAME realm shares that id exactly + // or begins with "/" (sub-folders, variables, condition nodes); the explicit realm + // compare is what makes the match realm-scoped — it replaces the old string key's leading "ns=N;s=" + // — so a node in the OTHER namespace that happens to share a bare id is never in scope. Snapshot + // each map's in-scope keys first (ToList) so we don't mutate while enumerating. + var prefix = equipmentNodeId + "/"; + bool InScope(RealmKey key) => key.Realm == realm + && (key.Id == equipmentNodeId || key.Id.StartsWith(prefix, StringComparison.Ordinal)); // Value variables (+ their historized-tagname registrations). foreach (var id in _variables.Keys.Where(InScope).ToList()) @@ -2575,8 +2716,8 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 // Notifier demotion BEFORE dropping the folders: sever the Server↔folder HasNotifier ref for // every promoted folder in the subtree (an equipment folder, or a sub-folder that hosted an // alarm), else the removal leaks an orphaned root-notifier reference on the Server object. - // _notifierFolders is keyed by the folder's NodeId; match InScope on its full (ns-qualified) string. - foreach (var kvp in _notifierFolders.Where(kv => InScope(kv.Key.ToString())).ToList()) + // _notifierFolders is keyed by the folder's NodeId; match InScope on its realm-qualified map key. + foreach (var kvp in _notifierFolders.Where(kv => InScope(MapKey(kv.Key))).ToList()) { RemoveRootNotifier(kvp.Value); _notifierFolders.Remove(kvp.Key); @@ -2612,7 +2753,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 /// Guard the address-space mutators against a too-early call. _root /// is only assigned in , which the SDK invokes during - /// StandardServer start; every public mutator (, + /// StandardServer start; every public mutator (, , /// , , , /// ) and assume it has run. /// If one is invoked before the server has started (a sink wired or a publish replayed before diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/SdkAddressSpaceSink.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/SdkAddressSpaceSink.cs index b2df9615..dcb135f5 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/SdkAddressSpaceSink.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/SdkAddressSpaceSink.cs @@ -24,6 +24,13 @@ public sealed class SdkAddressSpaceSink : IOpcUaAddressSpaceSink, ISurgicalAddre public void WriteValue(string nodeId, object? value, uint statusCode, DateTime sourceTimestampUtc, AddressSpaceRealm realm) => _nodeManager.WriteValue(nodeId, value, statusCode, sourceTimestampUtc, realm); + /// + /// Overrides the interface's default implementation on purpose: that default loops + /// , which would re-take the node manager's SDK-wide Lock once per value. + /// Forwarding to the manager's own batched entry point takes it ONCE for the whole batch. + public void WriteValues(IReadOnlyList values) + => _nodeManager.WriteValues(values); + /// public void WriteAlarmCondition(string alarmNodeId, AlarmConditionSnapshot state, DateTime sourceTimestampUtc, AddressSpaceRealm realm) => _nodeManager.WriteAlarmCondition(alarmNodeId, state, sourceTimestampUtc, realm); 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 4b865dab..e56b1c31 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs @@ -48,7 +48,11 @@ namespace ZB.MOM.WW.OtOpcUa.Runtime.Drivers; /// Children (DriverInstance/VirtualTag/etc.) are fully wired: DispatchDeployment drives /// which runs the spawn-plan via , /// maintains the FullName→NodeId live-value routing map, and forwards results to the -/// OPC UA publish actor via ForwardToMux. +/// OPC UA publish actor via ForwardBatchToMux — one whole driver production burst per message, +/// and the ONLY value ingress from driver children ( wraps even a +/// single data change in an AttributeValueBatchPublished). The per-value +/// 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 { @@ -185,7 +189,7 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers /// /// Driver live-value routing map: (DriverInstanceId, RawPath) → materialised NodeId(s) (realm-tagged). /// Rebuilt every apply by from the composition's RawTags - /// ∪ UnsReferenceVariables, and resolved in so a driver value + /// ∪ UnsReferenceVariables, and resolved in so a driver value /// published by wire-ref RawPath fans to its raw node AND every referencing UNS node with identical /// value/quality/timestamp — the single-source-fan-out (no independent buffer, so no drift). A set /// because one driver ref can back the raw node plus N UNS references; the per-apply rebuild dedups. @@ -923,7 +927,7 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers { Receive(HandleDispatchFromSteady); Receive(HandleGetDiagnostics); - Receive(ForwardToMux); + Receive(ForwardBatchToMux); Receive(ForwardNativeAlarm); Receive(OnDriverConnectivityChanged); Receive(HandleDeltaApplied); @@ -957,7 +961,7 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers // then Become(Applying)); completing the apply transitions back to Steady. Receive(HandleFetchedForApply); Receive(HandleGetDiagnostics); - Receive(ForwardToMux); + Receive(ForwardBatchToMux); Receive(ForwardNativeAlarm); Receive(OnDriverConnectivityChanged); Receive(HandleDeltaApplied); @@ -973,42 +977,67 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers Receive(_ => { /* PubSub ack */ }); } - private void ForwardToMux(DriverInstanceActor.AttributeValuePublished msg) + /// + /// Routes one batched driver production burst — the ONLY value ingress from a driver child, since + /// wraps even a single data change in an + /// . + /// + /// Each item's FullReference (the tag's RawPath) resolves through + /// — rebuilt every apply from the composition's RawTags + /// ∪ UnsReferenceVariables — to the v3 Batch 4 single-source fan-out: the RAW + /// NodeId () AND every referencing UNS NodeId + /// (), each carrying IDENTICAL value/status/timestamp, so + /// there is no independent buffer to drift. An unmapped ref is Debug-logged and dropped without + /// disturbing the rest of the burst. + /// + /// + /// The whole burst costs ONE hop to the dependency mux (which filters per item itself, feeding + /// VirtualTag inputs keyed by RawPath) and ONE + /// to + /// the publish actor — never one message per value. Both sinks are optional: without a mux, + /// VirtualTagActor evaluation can't fire (the dev/Mac path, no virtual tags registered); + /// production binds both via the RuntimeActors extension. + /// + /// + private void ForwardBatchToMux(DriverInstanceActor.AttributeValueBatchPublished msg) { - // Pass driver-published values to the dependency mux when one is wired (VirtualTag inputs, - // keyed by FullReference). Without a mux, VirtualTagActor evaluation can't fire — that's the - // dev/Mac path (no virtual tags registered); production binds the mux via the RuntimeActors - // extension. KEEP this unchanged — VirtualTag inputs are still keyed by FullReference. + // One envelope per burst; the mux filters per item (it registered a batch receive in Task 9). _dependencyMux?.Tell(msg); if (_opcUaPublishActor is null) return; - // Route the value to the OPC UA sink at the variable's ACTUAL NodeId. Equipment-tag variables - // are materialised with folder-scoped NodeIds (EquipmentId/FolderPath/Name), while the driver - // publishes keyed by its wire-ref FullName (FullReference). The _nodeIdByDriverRef map — built - // each apply from the composition's EquipmentTags — resolves (DriverInstanceId, FullName) to - // the folder-scoped NodeId(s) the materialiser placed the variable(s) at, so the value lands - // instead of leaving the variable at BadWaitingForInitialData. One driver ref can back several - // equipment variables (identical machines sharing a register), hence the fan-out. - if (_nodeIdByDriverRef.TryGetValue((msg.DriverInstanceId, msg.FullReference), out var nodeIds)) + // Indexed for-loop, not foreach: IReadOnlyList enumerates through an interface + // enumerator that would allocate once per burst on the hottest routing seam in the server. + var count = msg.Items.Count; + List? writes = null; + for (var idx = 0; idx < count; idx++) { - // v3 Batch 4 single-source fan-out: one driver publish for a RawPath lands on the raw NodeId AND - // every referencing UNS NodeId, each with its realm, carrying IDENTICAL value/status/timestamp. - foreach (var n in nodeIds) - _opcUaPublishActor.Tell(new ZB.MOM.WW.OtOpcUa.Runtime.OpcUa.OpcUaPublishActor.AttributeValueUpdate( - n.NodeId, msg.Value, msg.StatusCode, msg.TimestampUtc, n.Realm)); - } - else - { - _log.Debug("DriverHost {Node}: no bound NodeId for ({Driver},{Ref}) — value dropped", - _localNode, msg.DriverInstanceId, msg.FullReference); + var item = msg.Items[idx]; + if (_nodeIdByDriverRef.TryGetValue((msg.DriverInstanceId, item.FullReference), out var nodeIds)) + { + // Sized for the dominant case — every item resolving to its raw node alone, no UNS + // reference — where this is exact and the list never grows. A burst that does fan out + // pays at most a doubling or two, which beats over-allocating on every burst that doesn't. + writes ??= new List(count); + foreach (var n in nodeIds) + writes.Add(new ValueWrite(n.NodeId, item.Value, item.StatusCode, item.TimestampUtc, n.Realm)); + } + else if (_log.IsDebugEnabled) + { + _log.Debug("DriverHost {Node}: no bound NodeId for ({Driver},{Ref}) — value dropped", + _localNode, msg.DriverInstanceId, item.FullReference); + } } + + // Nothing resolved ⇒ no message at all, rather than an empty batch for the sink to unpack. + if (writes is not null) + _opcUaPublishActor.Tell(new ZB.MOM.WW.OtOpcUa.Runtime.OpcUa.OpcUaPublishActor.AttributeValueBatchUpdate(writes)); } /// /// Routes a native alarm transition (published by a driver child as /// ) to its materialised Part 9 condition - /// node(s). The alarm path analogue of : the transition's ConditionId + /// node(s). The alarm path analogue of : the transition's ConditionId /// (the dotted alarm full-reference, which equals the authored equipment-tag FullName — NOT the bare /// SourceNodeId owning object) is resolved by the map — /// built each apply from the alarm-bearing EquipmentTags — to the folder-scoped condition NodeId(s) diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverInstanceActor.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverInstanceActor.cs index 4ab6af32..98040273 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverInstanceActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverInstanceActor.cs @@ -101,13 +101,28 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers /// state — a no-op when already Reconnecting or Connecting. /// public sealed record ForceReconnect; - /// Published to the actor's parent whenever the subscribed IDriver fires - /// . The parent forwards to OpcUaPublishActor. + /// One driver-published value. NOT sent to the parent by the subscription path any more — the + /// actor batches every data change into an . Still live as the + /// per-value message the parent fans onward (the dependency mux consumes it, and + /// DriverHostActor.FanCommsLossToDependencyMux fabricates it). /// is the driver's full OPC UA status code including its sub-code /// (#519) — not a severity bucket. Classify it with and friends rather /// than comparing it to . public sealed record AttributeValuePublished(string DriverInstanceId, string FullReference, object? Value, uint StatusCode, DateTime TimestampUtc); + /// One value within an . + /// A struct deliberately: an AttributeValueItem[] then holds its items inline, so a burst of + /// N values costs one array allocation instead of one array plus N boxes. Indexed access through + /// does not box. + public readonly record struct AttributeValueItem(string FullReference, object? Value, uint StatusCode, DateTime TimestampUtc); + /// Batched form of — every changed attribute of ONE + /// driver production burst in one message. The parent fans the whole batch to the mux and the OPC UA + /// publish path in one hop each. Status codes ride verbatim (#519). + public sealed record AttributeValueBatchPublished(string DriverInstanceId, IReadOnlyList Items); private sealed record DataChangeForward(string FullReference, DataValueSnapshot Snapshot); + /// Self-sent when a batch-capable driver () raises + /// — marshals the whole burst off the driver's + /// thread onto the actor thread in one hop. Parallels . + private sealed record DataChangeBatchForward(IReadOnlyList Items); /// Published to the parent whenever the subscribed driver (an ) fires /// . The parent () projects + routes it /// to the materialised Part 9 condition. Parallels . @@ -205,6 +220,9 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers /// do not need to re-send subscription requests after a reconnect. private ISubscriptionHandle? _subscriptionHandle; private EventHandler? _dataChangeHandler; + /// The batch data-change handler, attached INSTEAD of when the + /// driver is an . Exactly one of the two is ever non-null. + private EventHandler? _batchChangeHandler; private EventHandler? _alarmEventHandler; private EventHandler? _rediscoveryHandler; private EventHandler? _hostStatusHandler; @@ -478,8 +496,9 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers }); ReceiveAsync(HandleSubscribeAlarmsAsync); Receive(OnDataChangeForward); + Receive(OnDataChangeBatchForward); // Native alarm transition marshaled onto the actor thread from the driver's OnAlarmEvent; - // project it to the parent the same way DataChangeForward projects AttributeValuePublished. + // project it to the parent the same way DataChangeForward projects AttributeValueBatchPublished. Receive(m => Context.Parent.Tell(new AttributeAlarmPublished(_driverInstanceId, m.Args))); // ResubscribeDesired self-Tells Subscribe; HandleSubscribeAsync replies SubscriptionEstablished to the // sender, which on the self-resubscribe path is Self. Swallow it (trace only) so it doesn't dead-letter. @@ -877,8 +896,27 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers try { - _dataChangeHandler = (_, args) => self.Tell(new DataChangeForward(args.FullReference, args.Snapshot)); - subscribable.OnDataChange += _dataChangeHandler; + // EXACTLY ONE of the two data-change events is subscribed. An IBatchSubscribable driver raises + // OnDataChangeBatch once per production burst AND the per-tag OnDataChange for every item in + // that same burst (the per-tag event is kept for legacy consumers — Galaxy cold reads, driver + // CLIs). Attaching both here would forward every value twice. The batch event is preferred: one + // Tell, one parent hop, one mux pass per burst instead of per tag. + if (subscribable is IBatchSubscribable batchable) + { + _batchChangeHandler = (_, args) => + { + // A null/empty raise from a buggy driver must never enter the mailbox: an unguarded null + // would throw in the handler, restart the actor, and tear down the driver transport. + if (args?.Items is not { Count: > 0 }) return; + self.Tell(new DataChangeBatchForward(args.Items)); + }; + batchable.OnDataChangeBatch += _batchChangeHandler; + } + else + { + _dataChangeHandler = (_, args) => self.Tell(new DataChangeForward(args.FullReference, args.Snapshot)); + subscribable.OnDataChange += _dataChangeHandler; + } using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(10)); // A bulk subscribe spans potentially many hosts, so key the pipeline on the driver @@ -928,9 +966,12 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers } } - /// Tear down the data-change + native-alarm event handlers + null the handle. Called from the - /// Unsubscribe path, on PostStop, and on Connected → Reconnecting transitions so a stale handler doesn't - /// push data-change / alarm events to an actor that has lost its driver connection. + /// Tear down the data-change (per-tag AND batch) + native-alarm event handlers + null the handle. + /// Called from the Unsubscribe path, on PostStop, and on Connected → Reconnecting transitions so a stale + /// handler doesn't push data-change / alarm events to an actor that has lost its driver connection. + /// Both data-change fields are cleared unconditionally even though only one is ever attached — the + /// IDriver object outlives the actor when the host respawns a child around it, so a handler left on the + /// driver's event would keep Telling a dead actor. private void DetachSubscription() { if (_driver is ISubscribable subscribable && _dataChangeHandler is not null) @@ -938,6 +979,11 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers subscribable.OnDataChange -= _dataChangeHandler; } _dataChangeHandler = null; + if (_driver is IBatchSubscribable batchable && _batchChangeHandler is not null) + { + batchable.OnDataChangeBatch -= _batchChangeHandler; + } + _batchChangeHandler = null; _subscriptionHandle = null; DetachAlarmSource(); } @@ -1130,7 +1176,8 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers } } - /// Forward a driver's data change to the parent for OPC UA publication. + /// Forward a per-tag-only driver's data change to the parent for OPC UA publication, as a + /// single-item batch — so the parent sees ONE message shape whether or not the driver batches. /// The driver's status code rides through UNCHANGED (#519). This used to project it onto a /// 3-state Good/Uncertain/Bad enum here, which the node manager then re-expanded to a bare /// 0x80000000 — so no driver's sub-code ever reached a client, and the status constants each @@ -1138,8 +1185,25 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers private void OnDataChangeForward(DataChangeForward msg) { var ts = msg.Snapshot.SourceTimestampUtc ?? msg.Snapshot.ServerTimestampUtc; - Context.Parent.Tell(new AttributeValuePublished( - _driverInstanceId, msg.FullReference, msg.Snapshot.Value, msg.Snapshot.StatusCode, ts)); + Context.Parent.Tell(new AttributeValueBatchPublished( + _driverInstanceId, + new[] { new AttributeValueItem(msg.FullReference, msg.Snapshot.Value, msg.Snapshot.StatusCode, ts) })); + } + + /// Forward one batch-capable driver production burst to the parent as a single message. + /// Every item's status code and timestamp ride through exactly as + /// carries them for a per-tag driver (#519; timestamps come from the + /// driver's snapshot, never from this actor's clock). + private void OnDataChangeBatchForward(DataChangeBatchForward msg) + { + var items = new AttributeValueItem[msg.Items.Count]; + for (var i = 0; i < msg.Items.Count; i++) + { + var item = msg.Items[i]; + var ts = item.Snapshot.SourceTimestampUtc ?? item.Snapshot.ServerTimestampUtc; + items[i] = new AttributeValueItem(item.FullReference, item.Snapshot.Value, item.Snapshot.StatusCode, ts); + } + Context.Parent.Tell(new AttributeValueBatchPublished(_driverInstanceId, items)); } private static bool IsGoodStatus(uint statusCode) => OpcUaStatus.IsGood(statusCode); 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 63292de0..890a0282 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/OpcUa/OpcUaPublishActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/OpcUa/OpcUaPublishActor.cs @@ -51,6 +51,11 @@ public sealed class OpcUaPublishActor : ReceiveActor, IWithTimers /// hands it to the SDK verbatim. public sealed record AttributeValueUpdate(string NodeId, object? Value, uint StatusCode, DateTime TimestampUtc, AddressSpaceRealm Realm); + /// Batched form of : every fan-out write of one driver + /// production burst in one message — the sink takes its lock once for the whole batch. Items carry + /// their realm each (); status codes ride verbatim (#519). + public sealed record AttributeValueBatchUpdate(IReadOnlyList Items); + /// Carries the full Part 9 condition state for a scripted alarm to the sink. The /// snapshot is the Commons projection the Runtime host maps from the engine's /// Core AlarmConditionState + severity/message — the actor stays decoupled from @@ -286,6 +291,7 @@ public sealed class OpcUaPublishActor : ReceiveActor, IWithTimers _healthTickInterval = healthTickInterval ?? TimeSpan.FromSeconds(5); Receive(HandleAttributeUpdate); + Receive(HandleAttributeBatchUpdate); Receive(HandleAlarmUpdate); Receive(HandleAlarmQualityUpdate); Receive(HandleRebuild); @@ -330,6 +336,38 @@ public sealed class OpcUaPublishActor : ReceiveActor, IWithTimers } } + /// Mirrors for a whole burst: ONE + /// call, but the write tally and the + /// otopcua.opcua.sink.write meter still advance PER ITEM — batching is a transport change, so + /// dashboards must not drop by the batch factor. + /// A is a PARTIAL failure: the sink applied every item it + /// could and is telling us which one it couldn't. So it is caught ahead of the general catch to (a) name + /// the offending node in the log — a bare "the batch threw" cannot be acted on — and (b) still count the + /// items that DID land, keeping the per-item counting invariant honest instead of silently under-reporting + /// a whole batch because one item was bad. + private void HandleAttributeBatchUpdate(AttributeValueBatchUpdate msg) + { + try + { + _sink.WriteValues(msg.Items); + Interlocked.Add(ref _writes, msg.Items.Count); + OtOpcUaTelemetry.OpcUaSinkWrite.Add(msg.Items.Count, new KeyValuePair("kind", "value")); + } + catch (ValueWriteBatchException ex) + { + _log.Warning(ex, + "OpcUaPublish: sink.WriteValues partially failed for a {Count}-item batch — {Failed} failed " + + "(first: {Node}), {Succeeded} applied", + msg.Items.Count, ex.FailedCount, ex.FirstFailedNodeId, ex.SucceededCount); + Interlocked.Add(ref _writes, ex.SucceededCount); + OtOpcUaTelemetry.OpcUaSinkWrite.Add(ex.SucceededCount, new KeyValuePair("kind", "value")); + } + catch (Exception ex) + { + _log.Warning(ex, "OpcUaPublish: sink.WriteValues threw for a {Count}-item batch", msg.Items.Count); + } + } + private void HandleAlarmUpdate(AlarmStateUpdate msg) { try @@ -469,7 +507,7 @@ public sealed class OpcUaPublishActor : ReceiveActor, IWithTimers 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.ForwardToMux after + // 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); diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ServiceCollectionExtensions.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ServiceCollectionExtensions.cs index 3ce81372..98daf31a 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ServiceCollectionExtensions.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ServiceCollectionExtensions.cs @@ -302,12 +302,18 @@ public static class ServiceCollectionExtensions // bootstrap their own HOCON (e.g. ServiceCollectionExtensionsTests) wouldn't pick it up // — OpcUaPublishActor.Props pins itself to opcua-synchronized-dispatcher and Akka throws // ConfigurationException if it doesn't exist. Prepend a fallback so the runtime extension - // is self-contained. + // is self-contained. Keep this block identical to the akka.conf one. + // + // The OPC UA SDK's single-threaded session/subscription requirement is guaranteed by the + // PinnedDispatcher alone (a dedicated thread per actor instance), not by throughput. + // throughput only sets how many queued messages drain per scheduling run, so 100 removes a + // mailbox re-schedule round-trip per message on the hottest actor in the process with zero + // change to concurrency. builder.AddHocon(@" opcua-synchronized-dispatcher { type = ""PinnedDispatcher"" executor = ""thread-pool-executor"" - throughput = 1 + throughput = 100 } ", HoconAddMode.Prepend); diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/DependencyMuxActor.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/DependencyMuxActor.cs index 2820df66..cf06b01f 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/DependencyMuxActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/DependencyMuxActor.cs @@ -12,9 +12,15 @@ namespace ZB.MOM.WW.OtOpcUa.Runtime.VirtualTags; /// forwards a to each interested /// subscriber. /// -/// DriverHostActor forwards every AttributeValuePublished it receives from its -/// DriverInstanceActor children to this mux (one mux per driver-role node). The mux is -/// deliberately not a DPS subscriber — virtual-tag evaluation is local to each node and +/// DriverHostActor forwards driver-child value publishes to this mux (one mux per driver-role +/// node). The ONLY driver ingress is the batched +/// — one envelope per driver +/// production burst, which the mux filters per item. The per-value +/// receive stays for the comms-loss fan +/// ALONE: DriverHostActor.FanCommsLossToDependencyMux fabricates that message and Tells this +/// mux directly, never through the host's own receive. Both +/// produce the same per interested subscriber. +/// The mux is deliberately not a DPS subscriber — virtual-tag evaluation is local to each node and /// would over-emit if it spanned the cluster. /// public sealed class DependencyMuxActor : ReceiveActor @@ -43,6 +49,7 @@ public sealed class DependencyMuxActor : ReceiveActor Receive(OnRegister); Receive(msg => RemoveSubscriber(msg.Subscriber)); Receive(OnAttributeValuePublished); + Receive(OnAttributeValueBatchPublished); Receive(msg => RemoveSubscriber(msg.ActorRef)); } @@ -107,4 +114,27 @@ public sealed class DependencyMuxActor : ReceiveActor } } + /// Filters one batched driver production burst per item — the batched analogue of + /// . Items nobody registered for are dropped (the common case: + /// only a fraction of an address space feeds virtual-tag expressions), so a burst of N values costs one + /// message plus one DependencyValueChanged per genuinely interested (item, subscriber) pair. + /// The registered set is NOT just virtual-tag inputs: ContinuousHistorizationRecorder + /// registers interest in every historized ref, so on a historizing node can be + /// large and a sizeable fraction of each burst resolves. + private void OnAttributeValueBatchPublished(DriverInstanceActor.AttributeValueBatchPublished msg) + { + // Nobody has registered anything — skip the whole burst rather than probing an empty map per item. + if (_byRef.Count == 0) return; + + // Indexed for-loop, not foreach: this is the hot value ingress and an IReadOnlyList enumerator + // would allocate once per burst. + var count = msg.Items.Count; + for (var i = 0; i < count; i++) + { + var item = msg.Items[i]; + if (!_byRef.TryGetValue(item.FullReference, out var subscribers) || subscribers.Count == 0) continue; + var dep = new VirtualTagActor.DependencyValueChanged(item.FullReference, item.Value, item.TimestampUtc, item.StatusCode); + foreach (var sub in subscribers) sub.Tell(dep); + } + } } diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/AddressSpaceSinkBatchTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/AddressSpaceSinkBatchTests.cs new file mode 100644 index 00000000..ec1b5a5a --- /dev/null +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/AddressSpaceSinkBatchTests.cs @@ -0,0 +1,112 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Commons.OpcUa; + +namespace ZB.MOM.WW.OtOpcUa.Commons.Tests; + +/// +/// The batched seam. It exists so an SDK-backed sink +/// can take its internal lock ONCE per batch instead of once per value, and it ships as a default +/// interface method so every existing sink and test fake keeps compiling unchanged — this suite +/// pins both halves of that bargain: the default loops +/// verbatim (#519: the status code rides through untouched), and +/// forwards the batch to an overriding inner rather than +/// silently degrading it to per-item calls on every driver-role host. +/// +public class AddressSpaceSinkBatchTests +{ + [Fact] + public void Default_WriteValues_loops_WriteValue() + { + // A sink that implements ONLY the pre-existing interface members — exactly the shape of every + // sink and fake that predates the batch seam. The default implementation must fan the batch out + // to WriteValue, in order, with every field carried through unchanged. + var fake = new PerItemOnlySink(); + var t0 = new DateTime(2026, 8, 15, 10, 0, 0, DateTimeKind.Utc); + + ((IOpcUaAddressSpaceSink)fake).WriteValues(new[] + { + new ValueWrite("Plant/Modbus/dev1/Speed", 42, OpcUaStatus.Good, t0, AddressSpaceRealm.Raw), + new ValueWrite("Area/Line/Eq/Speed", 43.5, OpcUaStatus.BadNoCommunication, t0.AddSeconds(1), AddressSpaceRealm.Uns), + new ValueWrite("Plant/Modbus/dev1/Temp", null, OpcUaStatus.Bad, t0.AddSeconds(2), AddressSpaceRealm.Raw), + }); + + fake.Calls.Count.ShouldBe(3); + + fake.Calls[0].ShouldBe(("Plant/Modbus/dev1/Speed", (object?)42, OpcUaStatus.Good, t0, AddressSpaceRealm.Raw)); + fake.Calls[1].ShouldBe(("Area/Line/Eq/Speed", (object?)43.5, OpcUaStatus.BadNoCommunication, t0.AddSeconds(1), AddressSpaceRealm.Uns)); + fake.Calls[2].ShouldBe(("Plant/Modbus/dev1/Temp", (object?)null, OpcUaStatus.Bad, t0.AddSeconds(2), AddressSpaceRealm.Raw)); + } + + [Fact] + public void Deferred_sink_forwards_WriteValues_to_inner_once() + { + // The whole point of the seam is the ONE-lock-per-batch win, and actors inject the deferred + // wrapper — not the inner SdkAddressSpaceSink. A missing forward here would fall back to the + // interface default and re-explode the batch into per-item WriteValue calls on every + // driver-role host: the batching would ship inert, exactly the F10b / PR#423 trap class. + var inner = new BatchingSpySink(); + var sink = new DeferredAddressSpaceSink(); + sink.SetSink(inner); + + var t0 = new DateTime(2026, 8, 15, 11, 0, 0, DateTimeKind.Utc); + var batch = new[] + { + new ValueWrite("a", 1, OpcUaStatus.Good, t0, AddressSpaceRealm.Raw), + new ValueWrite("b", 2, OpcUaStatus.Good, t0, AddressSpaceRealm.Uns), + new ValueWrite("c", 3, OpcUaStatus.Good, t0, AddressSpaceRealm.Raw), + }; + + sink.WriteValues(batch); + + inner.Batches.Count.ShouldBe(1, "the batch must reach the inner sink as ONE call, not be re-looped"); + inner.Batches[0].Count.ShouldBe(3); + inner.Batches[0].Select(v => v.NodeId).ShouldBe(new[] { "a", "b", "c" }); + inner.PerItemCalls.ShouldBe(0, "the inner sink's per-item WriteValue must not be reached at all"); + } + + // ---- test doubles ---- + + /// Implements only the members that existed before the batch seam, so the interface's + /// default WriteValues is what runs. + private sealed class PerItemOnlySink : IOpcUaAddressSpaceSink + { + public List<(string NodeId, object? Value, uint StatusCode, DateTime SourceTimestampUtc, AddressSpaceRealm Realm)> Calls { get; } = new(); + + public void WriteValue(string nodeId, object? value, uint statusCode, DateTime sourceTimestampUtc, AddressSpaceRealm realm) + => Calls.Add((nodeId, value, statusCode, sourceTimestampUtc, realm)); + + public void WriteAlarmCondition(string alarmNodeId, AlarmConditionSnapshot state, DateTime sourceTimestampUtc, AddressSpaceRealm realm) { } + public void WriteAlarmQuality(string alarmNodeId, uint statusCode, DateTime sourceTimestampUtc, AddressSpaceRealm realm) { } + public void MaterialiseAlarmCondition(string alarmNodeId, string equipmentNodeId, string displayName, string alarmType, int severity, AddressSpaceRealm realm, bool isNative = false) { } + public void WireAlarmNotifiers(string alarmNodeId, AddressSpaceRealm alarmRealm, IReadOnlyList notifierFolderNodeIds, AddressSpaceRealm notifierFolderRealm) { } + public void EnsureFolder(string folderNodeId, string? parentNodeId, string displayName, AddressSpaceRealm 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) { } + public void RebuildAddressSpace() { } + public void RaiseNodesAddedModelChange(string affectedNodeId, AddressSpaceRealm realm) { } + public void AddReference(string sourceNodeId, AddressSpaceRealm sourceRealm, string targetNodeId, AddressSpaceRealm targetRealm, string referenceType = "Organizes") { } + } + + /// Overrides WriteValues the way a real SDK-backed sink does — records the batch and + /// does NOT loop — so a wrapper that dropped the forward is distinguishable from one that kept it. + private sealed class BatchingSpySink : IOpcUaAddressSpaceSink + { + public List> Batches { get; } = new(); + public int PerItemCalls { get; private set; } + + public void WriteValue(string nodeId, object? value, uint statusCode, DateTime sourceTimestampUtc, AddressSpaceRealm realm) + => PerItemCalls++; + + public void WriteValues(IReadOnlyList values) => Batches.Add(values); + + public void WriteAlarmCondition(string alarmNodeId, AlarmConditionSnapshot state, DateTime sourceTimestampUtc, AddressSpaceRealm realm) { } + public void WriteAlarmQuality(string alarmNodeId, uint statusCode, DateTime sourceTimestampUtc, AddressSpaceRealm realm) { } + public void MaterialiseAlarmCondition(string alarmNodeId, string equipmentNodeId, string displayName, string alarmType, int severity, AddressSpaceRealm realm, bool isNative = false) { } + public void WireAlarmNotifiers(string alarmNodeId, AddressSpaceRealm alarmRealm, IReadOnlyList notifierFolderNodeIds, AddressSpaceRealm notifierFolderRealm) { } + public void EnsureFolder(string folderNodeId, string? parentNodeId, string displayName, AddressSpaceRealm 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) { } + public void RebuildAddressSpace() { } + public void RaiseNodesAddedModelChange(string affectedNodeId, AddressSpaceRealm realm) { } + public void AddReference(string sourceNodeId, AddressSpaceRealm sourceRealm, string targetNodeId, AddressSpaceRealm targetRealm, string referenceType = "Organizes") { } + } +} diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/OpcUa/DeferredSinkForwardingReflectionTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/OpcUa/DeferredSinkForwardingReflectionTests.cs index 5a005ee0..7a455cec 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/OpcUa/DeferredSinkForwardingReflectionTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/OpcUa/DeferredSinkForwardingReflectionTests.cs @@ -86,12 +86,28 @@ public class DeferredSinkForwardingReflectionTests { if (method.Name == nameof(IOpcUaAddressSpaceSink.RebuildAddressSpace)) continue; - method.GetParameters().Any(p => p.ParameterType == typeof(AddressSpaceRealm)).ShouldBeTrue( + method.GetParameters().Any(p => CarriesRealm(p.ParameterType)).ShouldBeTrue( $"{method.DeclaringType!.Name}.{method.Name} names a node but has no AddressSpaceRealm parameter — " + "a bare node id is ambiguous across the Raw and UNS namespaces (B4-WP2). Add the realm discriminator."); } } + /// Whether a parameter type supplies the B4-WP2 realm discriminator — either directly, or + /// carried per element by a batch parameter (WriteValues(IReadOnlyList<ValueWrite>) is + /// realm-qualified per item, since one batch legitimately spans both namespaces). Looking THROUGH the + /// element type rather than exempting the method by name keeps the guard mechanical: a future batch + /// item struct that forgets its realm still trips it. + private static bool CarriesRealm(Type t) + { + if (t == typeof(AddressSpaceRealm)) return true; + + if (t.IsGenericType && t.GetGenericArguments().Any(CarriesRealm)) return true; + + return t.Namespace == typeof(IOpcUaAddressSpaceSink).Namespace + && (t.GetProperties().Any(p => p.PropertyType == typeof(AddressSpaceRealm)) + || t.GetFields().Any(f => f.FieldType == typeof(AddressSpaceRealm))); + } + [Fact] public void Every_IServiceLevelPublisher_method_reaches_the_inner_publisher() { diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/DataChangeBatchContractTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/DataChangeBatchContractTests.cs new file mode 100644 index 00000000..80c86802 --- /dev/null +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/DataChangeBatchContractTests.cs @@ -0,0 +1,27 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +namespace ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests; + +/// Contract tests for the batched data-change payload types. +[Trait("Category", "Unit")] +public sealed class DataChangeBatchContractTests +{ + private sealed record FakeHandle(string DiagnosticId) : ISubscriptionHandle; + + /// Verifies the batch payload carries the subscription handle and its per-tag items. + [Fact] + public void Batch_args_carry_handle_and_items() + { + var handle = new FakeHandle("h1"); + var snap = new DataValueSnapshot(42, 0u, DateTime.UtcNow, DateTime.UtcNow); + + var args = new DataChangeBatchEventArgs(handle, new[] { new DataChangeItem("plc/tag1", snap) }); + + args.SubscriptionHandle.ShouldBe(handle); + args.Items.Count.ShouldBe(1); + args.Items[0].FullReference.ShouldBe("plc/tag1"); + args.Items[0].Snapshot.ShouldBe(snap); + } +} diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/PollGroupEngineBatchTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/PollGroupEngineBatchTests.cs new file mode 100644 index 00000000..4d014197 --- /dev/null +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/PollGroupEngineBatchTests.cs @@ -0,0 +1,335 @@ +using System.Collections.Concurrent; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +namespace ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests; + +/// +/// Guards for PollGroupEngine's optional per-cycle batch callback: one raise per poll +/// cycle carrying exactly the items that changed in that cycle, raised after the per-tag +/// callbacks of the same cycle and only when non-empty. Drivers forward it to +/// . +/// +[Trait("Category", "Unit")] +public sealed class PollGroupEngineBatchTests +{ + private static readonly TimeSpan Interval = TimeSpan.FromMilliseconds(30); + private static readonly TimeSpan WaitTimeout = TimeSpan.FromSeconds(5); + + /// + /// A reader over a mutable value map. Snapshots are returned in the requested reference + /// order (the engine's reader contract); lets a test await poll + /// cycles instead of sleeping a fixed wall-clock span. + /// + private sealed class FakeReader + { + private readonly Lock _gate = new(); + private readonly Dictionary _values = new(StringComparer.Ordinal); + private readonly Dictionary _statusCodes = new(StringComparer.Ordinal); + private int _pollCount; + + public FakeReader(params (string Reference, object? Value)[] seed) + { + foreach (var (reference, value) in seed) _values[reference] = value; + } + + /// Gets the number of completed reader invocations. + public int PollCount => Volatile.Read(ref _pollCount); + + /// + /// Overwrites the values a subsequent poll will observe. Applied under the same lock the + /// reader holds, so a multi-reference change is observed by ONE poll cycle rather than + /// being torn across two. + /// + public void Set(params (string Reference, object? Value)[] changes) + { + lock (_gate) + { + foreach (var (reference, value) in changes) _values[reference] = value; + } + } + + /// + /// Overwrites the status code a subsequent poll will report for a reference, leaving its + /// value untouched — the shape a device fault takes on the wire (#519). + /// + public void SetStatus(string reference, uint statusCode) + { + lock (_gate) _statusCodes[reference] = statusCode; + } + + /// The reader delegate handed to the engine. + public Task> ReadAsync(IReadOnlyList references, CancellationToken ct) + { + var now = DateTime.UtcNow; + var snapshots = new List(references.Count); + lock (_gate) + { + foreach (var reference in references) + snapshots.Add(new DataValueSnapshot( + _values.GetValueOrDefault(reference), + _statusCodes.GetValueOrDefault(reference), + now, + now)); + } + Interlocked.Increment(ref _pollCount); + return Task.FromResult>(snapshots); + } + } + + private static async Task WaitForAsync(Func condition, string because) + { + var deadline = DateTime.UtcNow + WaitTimeout; + while (DateTime.UtcNow < deadline) + { + if (condition()) return; + await Task.Delay(10, TestContext.Current.CancellationToken); + } + throw new TimeoutException($"Timed out after {WaitTimeout.TotalSeconds}s waiting for: {because}"); + } + + /// + /// The initial-data cycle batches every subscribed tag; a later cycle in which exactly one + /// value moved batches exactly that one item — and the per-tag callback fired for the same + /// items, so both callbacks see identical content. + /// + [Fact] + public async Task Batch_fires_once_per_cycle_with_only_changed_items() + { + var reader = new FakeReader(("a", 1), ("b", 2), ("c", 3)); + var perTag = new ConcurrentQueue<(string Reference, DataValueSnapshot Snapshot)>(); + var batches = new ConcurrentQueue>(); + + await using var engine = new PollGroupEngine( + reader.ReadAsync, + (_, reference, snapshot) => perTag.Enqueue((reference, snapshot)), + minInterval: Interval, + onBatch: (_, items) => batches.Enqueue(items)); + + engine.Subscribe(["a", "b", "c"], Interval); + + await WaitForAsync(() => batches.Count >= 1, "the initial-data batch"); + batches.TryDequeue(out var initial).ShouldBeTrue(); + initial!.Count.ShouldBe(3); + initial.Select(i => i.FullReference).ShouldBe(["a", "b", "c"]); + perTag.Count.ShouldBe(3); + + perTag.Clear(); + batches.Clear(); + reader.Set(("b", 99)); + + await WaitForAsync(() => batches.Count >= 1, "the batch for the single changed tag"); + batches.TryDequeue(out var changed).ShouldBeTrue(); + changed!.Count.ShouldBe(1); + changed[0].FullReference.ShouldBe("b"); + changed[0].Snapshot.Value.ShouldBe(99); + + // The per-tag callback saw the same single item, with the same snapshot instance. + perTag.Count.ShouldBe(1); + perTag.TryDequeue(out var perTagItem).ShouldBeTrue(); + perTagItem.Reference.ShouldBe("b"); + perTagItem.Snapshot.ShouldBeSameAs(changed[0].Snapshot); + } + + /// A cycle in which no value moved raises neither the batch nor any per-tag callback. + [Fact] + public async Task No_batch_when_nothing_changed() + { + var reader = new FakeReader(("a", 1), ("b", 2)); + var perTag = new ConcurrentQueue(); + var batches = new ConcurrentQueue>(); + + await using var engine = new PollGroupEngine( + reader.ReadAsync, + (_, reference, _) => perTag.Enqueue(reference), + minInterval: Interval, + onBatch: (_, items) => batches.Enqueue(items)); + + engine.Subscribe(["a", "b"], Interval); + + await WaitForAsync(() => batches.Count >= 1, "the initial-data batch"); + var pollsAfterInitial = reader.PollCount; + perTag.Clear(); + batches.Clear(); + + // Several further cycles over identical snapshots, plus a grace window for the callbacks + // of the last counted poll to have run. + await WaitForAsync(() => reader.PollCount >= pollsAfterInitial + 5, "five further poll cycles"); + await Task.Delay(50, TestContext.Current.CancellationToken); + + batches.ShouldBeEmpty(); + perTag.ShouldBeEmpty(); + } + + /// + /// Constructed without onBatch, the engine behaves exactly as before: per-tag + /// callbacks only, with no batch collection and no change to the raise pattern. + /// + [Fact] + public async Task Batch_is_not_invoked_when_callback_absent() + { + var reader = new FakeReader(("a", 1), ("b", 2), ("c", 3)); + var perTag = new ConcurrentQueue(); + + await using var engine = new PollGroupEngine( + reader.ReadAsync, + (_, reference, _) => perTag.Enqueue(reference), + minInterval: Interval); + + engine.Subscribe(["a", "b", "c"], Interval); + + await WaitForAsync(() => perTag.Count >= 3, "the initial-data per-tag callbacks"); + perTag.Count.ShouldBe(3); + + reader.Set(("c", 42)); + await WaitForAsync(() => perTag.Count >= 4, "the per-tag callback for the changed tag"); + + // Let further cycles run: nothing else changed, so the count must stay at 4. + var polls = reader.PollCount; + await WaitForAsync(() => reader.PollCount >= polls + 3, "three further poll cycles"); + await Task.Delay(50, TestContext.Current.CancellationToken); + perTag.Count.ShouldBe(4); + perTag.ToArray()[3].ShouldBe("c"); + } + + /// Batch items follow the reader's reference order, not any incidental sort order. + [Fact] + public async Task Batch_items_are_in_reader_order() + { + // Deliberately non-alphabetical subscription order: an alphabetical (or insertion-ordered + // dictionary) collection would produce z/a/m in a different sequence. + var reader = new FakeReader(("z", 1), ("a", 2), ("m", 3)); + var batches = new ConcurrentQueue>(); + + await using var engine = new PollGroupEngine( + reader.ReadAsync, + (_, _, _) => { }, + minInterval: Interval, + onBatch: (_, items) => batches.Enqueue(items)); + + engine.Subscribe(["z", "a", "m"], Interval); + + await WaitForAsync(() => batches.Count >= 1, "the initial-data batch"); + batches.TryDequeue(out var initial).ShouldBeTrue(); + initial!.Select(i => i.FullReference).ShouldBe(["z", "a", "m"]); + + // A cycle in which two of the three moved keeps the reader's relative order. + batches.Clear(); + reader.Set(("m", 30), ("z", 10)); + + await WaitForAsync(() => batches.Count >= 1, "the batch for the two changed tags"); + batches.TryDequeue(out var changed).ShouldBeTrue(); + changed!.Select(i => i.FullReference).ShouldBe(["z", "m"]); + } + + /// + /// Both callbacks write into ONE ordered log, so the batch's position relative to the + /// per-tag raises of the same cycle is directly observable: in a cycle where three tags + /// moved, the batch marker must land strictly after all three per-tag markers. + /// + /// + /// This asserts ORDER, not final state — a batch raised mid-loop (after the first + /// change, say) still produces exactly the same four log entries, and is caught here only + /// because their sequence differs. A test that merely counted the entries, or compared the + /// two collections' contents, would pass against that implementation. + /// + /// The thread assertion covers the other way the guarantee can be lost: a batch handed + /// to a detached Task.Run would usually still land last in the log (nothing else is + /// appended after it), so position alone does not catch it. The engine raises both callbacks + /// in one synchronous stretch of PollOnceAsync with no await between them, so a + /// correct implementation is on the same thread by construction and this can never flake red; + /// a detached raise lands on a different pool thread. + /// + [Fact] + public async Task Batch_is_raised_after_every_per_tag_callback_of_the_same_cycle() + { + var reader = new FakeReader(("a", 1), ("b", 2), ("c", 3)); + var log = new ConcurrentQueue(); + var perTagThreads = new ConcurrentQueue(); + var batchThreads = new ConcurrentQueue(); + + await using var engine = new PollGroupEngine( + reader.ReadAsync, + // The thread id is recorded BEFORE the log marker in both callbacks: the test awaits the + // "batch" marker, so recording it second would let the assertions run against a + // not-yet-enqueued thread id. + (_, reference, _) => + { + perTagThreads.Enqueue(Environment.CurrentManagedThreadId); + log.Enqueue($"per-tag:{reference}"); + }, + minInterval: Interval, + onBatch: (_, _) => + { + batchThreads.Enqueue(Environment.CurrentManagedThreadId); + log.Enqueue("batch"); + }); + + engine.Subscribe(["a", "b", "c"], Interval); + + // Drain the initial-data cycle so the log holds exactly one cycle's markers. + await WaitForAsync(() => log.Contains("batch"), "the initial-data batch"); + log.Clear(); + perTagThreads.Clear(); + batchThreads.Clear(); + + // All three move under the reader's lock, so ONE poll cycle observes all three changes. + reader.Set(("a", 10), ("b", 20), ("c", 30)); + await WaitForAsync(() => log.Contains("batch"), "the batch for the three changed tags"); + + var ordered = log.ToArray(); + var batchIndex = Array.IndexOf(ordered, "batch"); + + // Exactly one cycle's worth: three per-tag raises then the single batch, in that order. + ordered.Count(e => e == "batch").ShouldBe(1); + ordered.Take(batchIndex).ShouldBe(["per-tag:a", "per-tag:b", "per-tag:c"]); + batchIndex.ShouldBe(3); + + // Raised inline on the polling thread, not posted to a detached task. + batchThreads.Count.ShouldBe(1); + batchThreads.Single().ShouldBe(perTagThreads.Last()); + } + + /// + /// A non-zero status code is a change in its own right (the value need not move), and the + /// snapshot rides into untouched — the batch path + /// never re-derives or normalises quality (#519 at its origin). + /// + [Fact] + public async Task Batch_carries_a_bad_status_snapshot_unchanged() + { + const uint BadStatus = 0x80010000u; // Bad_UnexpectedError + + var reader = new FakeReader(("a", 1), ("b", 2)); + var perTag = new ConcurrentQueue<(string Reference, DataValueSnapshot Snapshot)>(); + var batches = new ConcurrentQueue>(); + + await using var engine = new PollGroupEngine( + reader.ReadAsync, + (_, reference, snapshot) => perTag.Enqueue((reference, snapshot)), + minInterval: Interval, + onBatch: (_, items) => batches.Enqueue(items)); + + engine.Subscribe(["a", "b"], Interval); + + await WaitForAsync(() => batches.Count >= 1, "the initial-data batch"); + perTag.Clear(); + batches.Clear(); + + // The device faults on "a": same value, bad quality. + reader.SetStatus("a", BadStatus); + + await WaitForAsync(() => batches.Count >= 1, "the batch for the newly-bad tag"); + batches.TryDequeue(out var bad).ShouldBeTrue(); + bad!.Count.ShouldBe(1); + bad[0].FullReference.ShouldBe("a"); + bad[0].Snapshot.StatusCode.ShouldBe(BadStatus); + bad[0].Snapshot.Value.ShouldBe(1); // value unchanged — the status alone drove the raise + + // The per-tag callback saw the identical snapshot instance. + perTag.TryDequeue(out var perTagItem).ShouldBeTrue(); + perTagItem.Reference.ShouldBe("a"); + perTagItem.Snapshot.ShouldBeSameAs(bad[0].Snapshot); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipSubscriptionTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipSubscriptionTests.cs index 901589a1..c4625681 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipSubscriptionTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipSubscriptionTests.cs @@ -20,6 +20,14 @@ public sealed class AbCipSubscriptionTests return (drv, factory); } + /// The driver advertises the batched-subscription capability (which implies ISubscribable). + [Fact] + public void Driver_is_batch_subscribable() + { + var (drv, _) = NewDriver(new AbCipTagDefinition("Speed", "ab://10.0.0.5/1,0", "Speed", AbCipDataType.DInt)); + (drv is IBatchSubscribable).ShouldBeTrue(); + } + /// Verifies that the initial poll raises OnDataChange events for every subscribed tag. [Fact] public async Task Initial_poll_raises_OnDataChange_for_every_tag() diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasCapabilityTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasCapabilityTests.cs index 3bbad4e8..a6cc2623 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasCapabilityTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasCapabilityTests.cs @@ -47,6 +47,20 @@ public sealed class FocasCapabilityTests // ---- ISubscribable ---- + /// The driver advertises the batched-subscription capability (which implies ISubscribable). + [Fact] + public void Driver_is_batch_subscribable() + { + var drv = new FocasDriver(new FocasDriverOptions + { + Devices = [new FocasDeviceOptions("focas://10.0.0.5:8193")], + RawTags = [FocasRawTags.Entry(new FocasTagDefinition("X", "focas://10.0.0.5:8193", "R100", FocasDataType.Byte))], + Probe = new FocasProbeOptions { Enabled = false }, + }, "drv-1", new FakeFocasClientFactory()); + + (drv is IBatchSubscribable).ShouldBeTrue(); + } + /// Verifies that the initial subscription poll raises an OnDataChange event. [Fact] public async Task Subscribe_initial_poll_raises_OnDataChange() diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusBatchSubscriptionTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusBatchSubscriptionTests.cs new file mode 100644 index 00000000..9f40e07d --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusBatchSubscriptionTests.cs @@ -0,0 +1,274 @@ +using System.Collections.Concurrent; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Driver.Modbus; + +namespace ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests; + +/// +/// Pins the driver-side half of the batched value pipeline: the Modbus driver is an +/// and forwards the PollGroupEngine's per-cycle batch +/// callback to . Both events fire by design — +/// the batch once per poll cycle, the per-tag one per changed tag — so a consumer subscribes to +/// exactly one of the two. +/// +[Trait("Category", "Unit")] +public sealed class ModbusBatchSubscriptionTests +{ + /// + /// Lightweight fake transport the subscription tests drive through — only the FC03 + /// (Read Holding Registers) path is used. Mutating + /// between polls is how each test simulates a PLC value change. + /// + private sealed class FakeTransport : IModbusTransport + { + public readonly ushort[] HoldingRegisters = new ushort[256]; + /// Simulates connecting to the Modbus device. + /// The cancellation token. + public Task ConnectAsync(CancellationToken ct) => Task.CompletedTask; + + /// Simulates sending a Modbus PDU. + /// The Modbus unit ID. + /// The protocol data unit. + /// The cancellation token. + public Task SendAsync(byte unitId, byte[] pdu, CancellationToken ct) + { + if (pdu[0] != 0x03) return Task.FromException(new NotSupportedException("FC not supported")); + var addr = (ushort)((pdu[1] << 8) | pdu[2]); + var qty = (ushort)((pdu[3] << 8) | pdu[4]); + var resp = new byte[2 + qty * 2]; + resp[0] = 0x03; + resp[1] = (byte)(qty * 2); + for (var i = 0; i < qty; i++) + { + resp[2 + i * 2] = (byte)(HoldingRegisters[addr + i] >> 8); + resp[3 + i * 2] = (byte)(HoldingRegisters[addr + i] & 0xFF); + } + return Task.FromResult(resp); + } + /// Disposes the transport asynchronously. + public ValueTask DisposeAsync() => ValueTask.CompletedTask; + } + + private static (ModbusDriver drv, FakeTransport fake) NewDriver(params ModbusTagDefinition[] tags) + { + var fake = new FakeTransport(); + var opts = new ModbusDriverOptions { Host = "fake", RawTags = ModbusRawTags.Entries(tags)}; + return (new ModbusDriver(opts, "modbus-1", _ => fake), fake); + } + + /// The driver advertises the batched-subscription capability. + [Fact] + public void Driver_is_batch_subscribable() + { + var (drv, _) = NewDriver(new ModbusTagDefinition("Level", ModbusRegion.HoldingRegisters, 0, ModbusDataType.Int16)); + (drv is IBatchSubscribable).ShouldBeTrue(); + } + + /// + /// One poll cycle that changed N tags raises the batch event ONCE carrying N items, and + /// raises the per-tag event N times with identical content. The invariant asserted across + /// the whole run — the concatenation of every batch's items equals the per-tag event + /// sequence — is what makes the two events interchangeable for a downstream consumer. + /// + [Fact] + public async Task Batch_event_fires_once_per_cycle_with_per_tag_events() + { + var (drv, fake) = NewDriver( + new ModbusTagDefinition("Level", ModbusRegion.HoldingRegisters, 0, ModbusDataType.Int16), + new ModbusTagDefinition("Temp", ModbusRegion.HoldingRegisters, 1, ModbusDataType.Int16), + new ModbusTagDefinition("Flow", ModbusRegion.HoldingRegisters, 2, ModbusDataType.Int16)); + await drv.InitializeAsync("{}", CancellationToken.None); + fake.HoldingRegisters[0] = 100; + fake.HoldingRegisters[1] = 200; + fake.HoldingRegisters[2] = 300; + + var perTag = new ConcurrentQueue(); + var batches = new ConcurrentQueue(); + drv.OnDataChange += (_, e) => perTag.Enqueue(e); + ((IBatchSubscribable)drv).OnDataChangeBatch += (_, e) => batches.Enqueue(e); + + var handle = await drv.SubscribeAsync(["Level", "Temp", "Flow"], TimeSpan.FromMilliseconds(100), CancellationToken.None); + + // Cycle 1 — the initial-data push: all three tags raise. + await WaitForCountAsync(batches, 1, TimeSpan.FromSeconds(2)); + batches.Count.ShouldBe(1); + batches.Single().Items.Count.ShouldBe(3); + perTag.Count.ShouldBe(3); + + // Cycle 2 — every tag changes, so the next cycle again batches all three. + fake.HoldingRegisters[0] = 101; + fake.HoldingRegisters[1] = 201; + fake.HoldingRegisters[2] = 301; + await WaitForCountAsync(batches, 2, TimeSpan.FromSeconds(3)); + + // Unsubscribe drains the in-flight poll loop, so the queues are stable from here. + await drv.UnsubscribeAsync(handle, CancellationToken.None); + + var allBatches = batches.ToArray(); + allBatches.Length.ShouldBeGreaterThanOrEqualTo(2); + allBatches[1].Items.Count.ShouldBe(3); + + // Every batch belongs to the subscription it was raised for, and is never empty. + foreach (var b in allBatches) + { + b.SubscriptionHandle.ShouldBeSameAs(handle); + b.Items.ShouldNotBeEmpty(); + } + + // Identical content: flattening the batches reproduces the per-tag event stream exactly + // (same references, same snapshots, same order). + var flattened = allBatches.SelectMany(b => b.Items).ToArray(); + var perTagArray = perTag.ToArray(); + flattened.Length.ShouldBe(perTagArray.Length); + for (var i = 0; i < flattened.Length; i++) + { + flattened[i].FullReference.ShouldBe(perTagArray[i].FullReference); + flattened[i].Snapshot.ShouldBe(perTagArray[i].Snapshot); + } + } + + /// + /// The batch event honours the driver's own per-tag deadband filter. The + /// PollGroupEngine collects batch items unconditionally — it knows nothing about + /// ShouldPublish — so a straight pass-through of the engine's list would carry + /// changes the per-tag event suppressed, and a downstream consumer that PREFERS the batch + /// event would see deadband silently stop working. A cycle whose only changes are all + /// suppressed raises no batch at all. + /// + [Fact] + public async Task Deadband_suppressed_changes_are_excluded_from_the_batch() + { + var (drv, fake) = NewDriver( + new ModbusTagDefinition("Temp", ModbusRegion.HoldingRegisters, 0, ModbusDataType.Int16, Deadband: 5.0)); + await drv.InitializeAsync("{}", CancellationToken.None); + fake.HoldingRegisters[0] = 100; + + var perTag = new ConcurrentQueue(); + var batches = new ConcurrentQueue(); + drv.OnDataChange += (_, e) => perTag.Enqueue(e); + ((IBatchSubscribable)drv).OnDataChangeBatch += (_, e) => batches.Enqueue(e); + + var handle = await drv.SubscribeAsync(["Temp"], TimeSpan.FromMilliseconds(100), CancellationToken.None); + try + { + // Cycle 1 — initial-data push. First sample seeds the deadband baseline and publishes + // through both events. + await WaitForCountAsync(batches, 1, TimeSpan.FromSeconds(2)); + batches.Count.ShouldBe(1); + batches.Single().Items.Count.ShouldBe(1); + batches.Single().Items[0].Snapshot.Value.ShouldBe((short)100); + perTag.Count.ShouldBe(1); + + // Under-threshold change (delta 2 < 5): the ENGINE sees a value change and would offer + // it as a batch item, but the driver's deadband suppresses the publish — so neither + // event fires and no batch is raised for that cycle at all. + fake.HoldingRegisters[0] = 102; + await Task.Delay(600); + perTag.Count.ShouldBe(1, "under-threshold change is suppressed on the per-tag event"); + batches.Count.ShouldBe(1, "a cycle whose only change is deadband-suppressed raises no batch"); + + // Over-threshold change (delta 10 ≥ 5 against the 100 baseline): publishes on both. + fake.HoldingRegisters[0] = 110; + await WaitForCountAsync(batches, 2, TimeSpan.FromSeconds(3)); + } + finally + { + // Unsubscribe drains the in-flight poll loop, so the queues are stable from here. + await drv.UnsubscribeAsync(handle, CancellationToken.None); + } + + var allBatches = batches.ToArray(); + allBatches.Length.ShouldBe(2); + allBatches[1].Items.Count.ShouldBe(1); + allBatches[1].Items[0].Snapshot.Value.ShouldBe((short)110); + + // Content equality: flattening the batches reproduces the per-tag event stream exactly. + // The suppressed 102 appears in NEITHER. + var flattened = allBatches.SelectMany(b => b.Items).ToArray(); + var perTagArray = perTag.ToArray(); + flattened.Length.ShouldBe(perTagArray.Length); + for (var i = 0; i < flattened.Length; i++) + { + flattened[i].FullReference.ShouldBe(perTagArray[i].FullReference); + flattened[i].Snapshot.ShouldBe(perTagArray[i].Snapshot); + } + flattened.ShouldNotContain(item => Equals(item.Snapshot.Value, (short)102)); + } + + /// + /// A per-tag subscriber that throws must not corrupt + /// the poll cycle. PollGroupEngine.PollOnceAsync invokes the driver's onChange bridge + /// UNGUARDED, so without the driver-side try/catch the throw unwinds the whole cycle before + /// onBatch runs — and because Modbus buffers its published items across the cycle + /// (the deadband-honouring buffer the tests above pin), the undrained items would silently + /// MERGE into a later cycle's batch. This failure mode is unique to Modbus: the other poll + /// drivers forward the engine's own per-cycle list and hold no state between cycles. + /// Asserted on both the shape (one batch per cycle, three items each) and the VALUES, so a + /// merged or dropped batch fails rather than coincidentally matching on count. + /// + [Fact] + public async Task A_throwing_per_tag_subscriber_does_not_break_or_contaminate_the_cycle_batch() + { + var (drv, fake) = NewDriver( + new ModbusTagDefinition("Level", ModbusRegion.HoldingRegisters, 0, ModbusDataType.Int16), + new ModbusTagDefinition("Temp", ModbusRegion.HoldingRegisters, 1, ModbusDataType.Int16), + new ModbusTagDefinition("Flow", ModbusRegion.HoldingRegisters, 2, ModbusDataType.Int16)); + await drv.InitializeAsync("{}", CancellationToken.None); + fake.HoldingRegisters[0] = 100; + fake.HoldingRegisters[1] = 200; + fake.HoldingRegisters[2] = 300; + + var thrown = 0; + var batches = new ConcurrentQueue(); + // The buggy external subscriber: every per-tag delivery faults. + drv.OnDataChange += (_, _) => + { + Interlocked.Increment(ref thrown); + throw new InvalidOperationException("buggy per-tag subscriber"); + }; + ((IBatchSubscribable)drv).OnDataChangeBatch += (_, e) => batches.Enqueue(e); + + var handle = await drv.SubscribeAsync(["Level", "Temp", "Flow"], TimeSpan.FromMilliseconds(100), CancellationToken.None); + try + { + // Cycle 1 — the initial-data push. All three tags publish despite every per-tag + // delivery faulting, and the cycle still reaches its batch. + await WaitForCountAsync(batches, 1, TimeSpan.FromSeconds(2)); + batches.Count.ShouldBe(1, "the cycle must still reach onBatch after a subscriber throws"); + + // Cycle 2 — every tag changes again. + fake.HoldingRegisters[0] = 101; + fake.HoldingRegisters[1] = 201; + fake.HoldingRegisters[2] = 301; + await WaitForCountAsync(batches, 2, TimeSpan.FromSeconds(3)); + } + finally + { + // Unsubscribe drains the in-flight poll loop, so the queues are stable from here. + await drv.UnsubscribeAsync(handle, CancellationToken.None); + } + + var allBatches = batches.ToArray(); + allBatches.Length.ShouldBeGreaterThanOrEqualTo(2); + + allBatches[0].Items.Count.ShouldBe(3); + allBatches[0].Items.Select(i => i.Snapshot.Value).ShouldBe(new object[] { (short)100, (short)200, (short)300 }); + + // The load-bearing assertion: cycle 2's batch carries EXACTLY cycle 2's items. A cycle + // aborted mid-loop would have left its items in the buffer to merge in here. + allBatches[1].Items.Count.ShouldBe(3, "a cycle aborted before onBatch would merge its buffered items into the next batch"); + allBatches[1].Items.Select(i => i.Snapshot.Value).ShouldBe(new object[] { (short)101, (short)201, (short)301 }); + + // Sanity: the throwing subscriber really was invoked for every published item. + thrown.ShouldBeGreaterThanOrEqualTo(6); + } + + private static async Task WaitForCountAsync(ConcurrentQueue q, int target, TimeSpan timeout) + { + var deadline = DateTime.UtcNow + timeout; + while (q.Count < target && DateTime.UtcNow < deadline) + await Task.Delay(25); + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerBatchWriteTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerBatchWriteTests.cs new file mode 100644 index 00000000..6c3e9c19 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerBatchWriteTests.cs @@ -0,0 +1,264 @@ +using Microsoft.Extensions.Logging.Abstractions; +using Opc.Ua; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Commons.OpcUa; + +namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests; + +/// +/// Value-pipeline batching (Task 6) — applies a batch under +/// one acquisition of the manager-wide SDK Lock per 512-item chunk instead of one per value (the +/// per-value take was the steady-state throughput ceiling on the publish path; the chunk cap keeps the +/// first poll cycle's force-raise-everything batch from stalling every SDK service thread). +/// These tests pin the observable contract rather than the locking: every item lands with +/// its own Value / StatusCode / Timestamp, an item for an undeclared NodeId is lazily created exactly +/// as does, and an empty batch does nothing at all. +/// A non-Good status is included so the #519 verbatim-sub-code passthrough is proven on the batched +/// path too — a batch that re-projected status through a 3-state enum would collapse it to a bare +/// Bad. +/// Boots a real through — the +/// same harness uses — so the assertions read the real SDK node +/// state an OPC UA client would receive. +/// +public sealed class NodeManagerBatchWriteTests : IDisposable +{ + private static CancellationToken Ct => TestContext.Current.CancellationToken; + + private readonly string _pkiRoot = Path.Combine( + Path.GetTempPath(), + $"otopcua-batchwrite-{Guid.NewGuid():N}"); + + /// Every item of a multi-item batch lands on its own node with its own value, status code + /// (including a non-Good sub-code) and source timestamp — no item is dropped, and none overwrites + /// another's state. + [Fact] + public async Task WriteValues_lands_every_value() + { + var (host, server) = await BootAsync(); + try + { + var nm = server.NodeManager!; + + foreach (var name in new[] { "a", "b", "c" }) + { + nm.EnsureVariable($"eq-1/{name}", parentFolderNodeId: null, displayName: name, dataType: "Double", + writable: false, realm: AddressSpaceRealm.Uns); + } + + var t1 = new DateTime(2026, 8, 15, 10, 0, 0, DateTimeKind.Utc); + var t2 = new DateTime(2026, 8, 15, 10, 0, 1, DateTimeKind.Utc); + var t3 = new DateTime(2026, 8, 15, 10, 0, 2, DateTimeKind.Utc); + + nm.WriteValues(new List + { + new("eq-1/a", 1.5d, OpcUaStatus.Good, t1, AddressSpaceRealm.Uns), + new("eq-1/b", null, OpcUaStatus.BadNoCommunication, t2, AddressSpaceRealm.Uns), + new("eq-1/c", 3.5d, 0x80740000u /* BadTypeMismatch */, t3, AddressSpaceRealm.Uns), + }); + + var a = nm.TryGetVariable("eq-1/a"); + a.ShouldNotBeNull(); + a!.Value.ShouldBe(1.5d); + a.StatusCode.ShouldBe((StatusCode)StatusCodes.Good); + a.Timestamp.ShouldBe(t1); + + var b = nm.TryGetVariable("eq-1/b"); + b.ShouldNotBeNull(); + b!.Value.ShouldBeNull(); + b.StatusCode.ShouldBe((StatusCode)StatusCodes.BadNoCommunication); + b.StatusCode.ShouldNotBe((StatusCode)StatusCodes.Bad, + "the batched path must hand the status sub-code to the SDK verbatim (#519)"); + b.Timestamp.ShouldBe(t2); + + var c = nm.TryGetVariable("eq-1/c"); + c.ShouldNotBeNull(); + c!.Value.ShouldBe(3.5d); + c.StatusCode.ShouldBe((StatusCode)StatusCodes.BadTypeMismatch); + c.Timestamp.ShouldBe(t3); + } + finally + { + await host.DisposeAsync(); + } + } + + /// A batch item naming a NodeId that was never declared lazily creates the variable, exactly + /// as the single-value create-on-first-write path does — + /// and it does so PER REALM. Both items here carry the SAME s= id and differ only by realm, so a + /// key that ignored the realm would create one node and have the second item overwrite the first. The + /// assertion is therefore two nodes with DISTINCT namespace indices, not merely "both reads returned + /// something" (which a realm-blind map satisfies, since the shared key resolves for either lookup). + [Fact] + public async Task WriteValues_creates_missing_variables() + { + var (host, server) = await BootAsync(); + try + { + var nm = server.NodeManager!; + + const string SharedId = "Shared/lazy/id"; + nm.TryGetVariable(SharedId, AddressSpaceRealm.Uns).ShouldBeNull(); + nm.TryGetVariable(SharedId, AddressSpaceRealm.Raw).ShouldBeNull(); + + var ts = new DateTime(2026, 8, 15, 11, 0, 0, DateTimeKind.Utc); + nm.WriteValues(new List + { + new(SharedId, 42, OpcUaStatus.Good, ts, AddressSpaceRealm.Uns), + new(SharedId, 7, OpcUaStatus.Good, ts, AddressSpaceRealm.Raw), + }); + + var uns = nm.TryGetVariable(SharedId, AddressSpaceRealm.Uns); + uns.ShouldNotBeNull(); + uns!.Value.ShouldBe(42); + uns.StatusCode.ShouldBe((StatusCode)StatusCodes.Good); + uns.Timestamp.ShouldBe(ts); + + var raw = nm.TryGetVariable(SharedId, AddressSpaceRealm.Raw); + raw.ShouldNotBeNull(); + raw!.Value.ShouldBe(7); + + // Two DISTINCT nodes, one per namespace — the realm-blind failure mode collapses these to one. + raw.NodeId.NamespaceIndex.ShouldNotBe(uns.NodeId.NamespaceIndex); + raw.NodeId.NamespaceIndex.ShouldBe(nm.NamespaceIndexForRealm(AddressSpaceRealm.Raw)); + uns.NodeId.NamespaceIndex.ShouldBe(nm.NamespaceIndexForRealm(AddressSpaceRealm.Uns)); + } + finally + { + await host.DisposeAsync(); + } + } + + /// A bad item does NOT drop the tail of the batch. Item 2's NodeId is empty; items 1 and 3 must + /// still land, and the aggregate surfaces as a naming the offender + /// and the failed/succeeded split. + /// This is the durability-critical half: the poll engine updates its last-value cache BEFORE + /// raising, so an item dropped here is never re-raised until that tag's value changes again — the node + /// would serve a stale value indefinitely, with nothing in the log naming it. + [Fact] + public async Task A_failing_item_neither_drops_the_tail_nor_hides_which_node_failed() + { + var (host, server) = await BootAsync(); + try + { + var nm = server.NodeManager!; + var ts = new DateTime(2026, 8, 15, 12, 0, 0, DateTimeKind.Utc); + + var ex = Should.Throw(() => nm.WriteValues(new List + { + new("eq-iso/first", 1, OpcUaStatus.Good, ts, AddressSpaceRealm.Uns), + new(string.Empty, 2, OpcUaStatus.Good, ts, AddressSpaceRealm.Uns), + new("eq-iso/third", 3, OpcUaStatus.Good, ts, AddressSpaceRealm.Uns), + })); + + ex.FailedCount.ShouldBe(1); + ex.SucceededCount.ShouldBe(2); + ex.FirstFailedNodeId.ShouldBe(string.Empty); + + // The load-bearing half: BOTH good items applied — including the one AFTER the failure. + var first = nm.TryGetVariable("eq-iso/first"); + first.ShouldNotBeNull(); + first!.Value.ShouldBe(1); + + var third = nm.TryGetVariable("eq-iso/third"); + third.ShouldNotBeNull("the item after the failing one must not be dropped"); + third!.Value.ShouldBe(3); + } + finally + { + await host.DisposeAsync(); + } + } + + /// A batch larger than MaxWritesPerLockAcquisition (512) spans several lock acquisitions; + /// chunking must be invisible in the outcome — every item still lands exactly once, with its own value. + /// Chunk boundaries are the natural off-by-one hazard, so the size is deliberately not a multiple of 512. + /// + [Fact] + public async Task A_batch_spanning_several_chunks_lands_every_item() + { + var (host, server) = await BootAsync(); + try + { + var nm = server.NodeManager!; + var ts = new DateTime(2026, 8, 15, 13, 0, 0, DateTimeKind.Utc); + + const int Count = 1300; // > 2 chunks of 512, and not a multiple of it + var items = new List(Count); + for (var i = 0; i < Count; i++) + { + items.Add(new ValueWrite($"eq-chunk/{i}", i, OpcUaStatus.Good, ts, AddressSpaceRealm.Uns)); + } + + nm.WriteValues(items); + + nm.VariableCount.ShouldBe(Count); + foreach (var probe in new[] { 0, 511, 512, 1023, 1024, Count - 1 }) + { + var v = nm.TryGetVariable($"eq-chunk/{probe}"); + v.ShouldNotBeNull($"item {probe} (a chunk boundary) must have landed"); + v!.Value.ShouldBe(probe); + } + } + finally + { + await host.DisposeAsync(); + } + } + + /// An empty batch neither throws nor touches the address space. + [Fact] + public async Task Empty_batch_is_a_no_op() + { + var (host, server) = await BootAsync(); + try + { + var nm = server.NodeManager!; + + var before = nm.VariableCount; + Should.NotThrow(() => nm.WriteValues([])); + nm.VariableCount.ShouldBe(before); + } + finally + { + await host.DisposeAsync(); + } + } + + private async Task<(OpcUaApplicationHost Host, OtOpcUaSdkServer Server)> BootAsync() + { + var host = new OpcUaApplicationHost( + new OpcUaApplicationHostOptions + { + ApplicationName = "OtOpcUa.BatchWriteTest", + ApplicationUri = $"urn:OtOpcUa.BatchWriteTest:{Guid.NewGuid():N}", + OpcUaPort = AllocateFreePort(), + PublicHostname = "localhost", + PkiStoreRoot = _pkiRoot, + }, + NullLogger.Instance); + + var server = new OtOpcUaSdkServer(); + await host.StartAsync(server, Ct); + return (host, server); + } + + private static int AllocateFreePort() + { + using var listener = new System.Net.Sockets.TcpListener(System.Net.IPAddress.Loopback, 0); + listener.Start(); + var port = ((System.Net.IPEndPoint)listener.LocalEndpoint).Port; + listener.Stop(); + return port; + } + + /// Cleans up the PKI root directory. + public void Dispose() + { + if (Directory.Exists(_pkiRoot)) + { + try { Directory.Delete(_pkiRoot, recursive: true); } + catch { /* best-effort cleanup */ } + } + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerPreStartGuardTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerPreStartGuardTests.cs index 718aa73e..afcbea51 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerPreStartGuardTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerPreStartGuardTests.cs @@ -9,7 +9,8 @@ namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests; /// /// OpcUaServer-004 — a fresh whose CreateAddressSpace has NOT /// yet run (i.e. the server has not started) has a null _root. Every public address-space mutator -/// (, , +/// (, , +/// , /// , , /// ) must now fail with a legible /// instead of a bare NRE out of ResolveParentFolder / @@ -78,6 +79,28 @@ public sealed class NodeManagerPreStartGuardTests : IDisposable } } + /// The batched mutator guards the same way — and it does so BEFORE its empty-batch early-return, + /// so a pre-start caller gets the legible failure regardless of what the batch happens to carry (an empty + /// batch returning quietly would make the guard depend on payload rather than on server state). + [Fact] + public async Task WriteValues_before_CreateAddressSpace_throws_InvalidOperationException() + { + var (host, nm) = await BuildPreStartNodeManagerAsync(); + try + { + Should.Throw(() => nm.WriteValues(new[] + { + new ValueWrite("eq-1/temp", 1.0, OpcUaStatus.Good, DateTime.UtcNow, AddressSpaceRealm.Uns), + })); + + Should.Throw(() => nm.WriteValues([])); + } + finally + { + await host.DisposeAsync(); + } + } + [Fact] public async Task MaterialiseAlarmCondition_before_CreateAddressSpace_throws_InvalidOperationException() { 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 bd3f8b85..699915a1 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/SdkAddressSpaceSinkTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/SdkAddressSpaceSinkTests.cs @@ -36,6 +36,83 @@ public sealed class SdkAddressSpaceSinkTests : IDisposable await host.DisposeAsync(); } + /// The sink's override forwards the batch to the + /// node manager's own batched entry point — every item lands, with its own value and realm. + [Fact] + public async Task WriteValues_forwards_the_batch_to_the_node_manager() + { + var (host, server) = await BootAsync(); + try + { + var nm = server.NodeManager!; + // Through the INTERFACE on purpose: that is the seam production uses, and it is the only call + // shape under which deleting the sink's override silently degrades to the per-value default loop + // (a concrete-typed call would simply stop compiling, which is not the regression being pinned). + IOpcUaAddressSpaceSink sink = new SdkAddressSpaceSink(nm); + var ts = new DateTime(2026, 8, 15, 9, 0, 0, DateTimeKind.Utc); + + sink.WriteValues(new List + { + new("Plant/dev1/Speed", 42, OpcUaStatus.Good, ts, AddressSpaceRealm.Raw), + new("eq-1/Speed", 42, OpcUaStatus.Good, ts, AddressSpaceRealm.Uns), + }); + + nm.VariableCount.ShouldBe(2); + + var raw = nm.TryGetVariable("Plant/dev1/Speed", AddressSpaceRealm.Raw); + raw.ShouldNotBeNull(); + raw!.Value.ShouldBe(42); + raw.Timestamp.ShouldBe(ts); + + var uns = nm.TryGetVariable("eq-1/Speed", AddressSpaceRealm.Uns); + uns.ShouldNotBeNull(); + uns!.Value.ShouldBe(42); + } + finally + { + await host.DisposeAsync(); + } + } + + /// Pins that the sink actually OVERRIDES WriteValues rather than inheriting the + /// interface's per-value default loop. The values-landed assertion above cannot tell the two apart — the + /// default loop lands them too — so this uses the one place their behaviour diverges: the manager's + /// batched path isolates per item and reports a aggregate, whereas + /// the default loop lets the first item's ArgumentException escape and never reaches the tail. + /// A test seam (a recording subclass) is not available here — is sealed + /// and its WriteValues is not virtual — so the forward is pinned by its observable contract + /// instead, which is the stronger assertion anyway. + [Fact] + public async Task WriteValues_forward_surfaces_the_node_managers_partial_failure_contract() + { + var (host, server) = await BootAsync(); + try + { + var nm = server.NodeManager!; + IOpcUaAddressSpaceSink sink = new SdkAddressSpaceSink(nm); + var ts = new DateTime(2026, 8, 15, 9, 30, 0, DateTimeKind.Utc); + + var ex = Should.Throw(() => sink.WriteValues(new List + { + new("eq-fwd/first", 1, OpcUaStatus.Good, ts, AddressSpaceRealm.Uns), + new(string.Empty, 2, OpcUaStatus.Good, ts, AddressSpaceRealm.Uns), + new("eq-fwd/third", 3, OpcUaStatus.Good, ts, AddressSpaceRealm.Uns), + })); + + ex.FailedCount.ShouldBe(1); + ex.SucceededCount.ShouldBe(2); + + // The tail item landed — the default per-value loop would have thrown out on item 2 and skipped it. + var third = nm.TryGetVariable("eq-fwd/third", AddressSpaceRealm.Uns); + third.ShouldNotBeNull(); + third!.Value.ShouldBe(3); + } + finally + { + await host.DisposeAsync(); + } + } + /// Verifies that WriteAlarmCondition (un-materialised fallback) creates a dedicated node /// distinct from value writes. [Fact] diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/CalculationDependencyFlowTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/CalculationDependencyFlowTests.cs index a76c00a5..b70d82a5 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/CalculationDependencyFlowTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/CalculationDependencyFlowTests.cs @@ -17,7 +17,7 @@ namespace ZB.MOM.WW.OtOpcUa.Runtime.Tests.Drivers; /// path (not merely that exists). /// A source value published to the mux is fed into the calc driver, which computes and publishes a /// value; and a calc tag reading ANOTHER calc tag's output (calc-of-calc) recomputes when the first -/// tag's output re-enters the mux — the exact re-entry the host's ForwardToMux performs, mirrored +/// tag's output re-enters the mux — the exact re-entry the host's ForwardBatchToMux performs, mirrored /// here by looping the driver's back into the mux. /// public sealed class CalculationDependencyFlowTests : RuntimeActorTestBase @@ -58,7 +58,7 @@ public sealed class CalculationDependencyFlowTests : RuntimeActorTestBase var published = new ConcurrentQueue(); var mux = Sys.ActorOf(DependencyMuxActor.Props(), "dep-mux"); - // Loop the driver's outputs back into the mux exactly as DriverHostActor.ForwardToMux does — this + // Loop the driver's outputs back into the mux exactly as DriverHostActor.ForwardBatchToMux does — this // is what makes calc-of-calc chains work. driver.OnDataChange += (_, e) => { diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorBatchFanOutTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorBatchFanOutTests.cs new file mode 100644 index 00000000..51e3ee87 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorBatchFanOutTests.cs @@ -0,0 +1,198 @@ +using Akka.Actor; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Commons.OpcUa; +using ZB.MOM.WW.OtOpcUa.Commons.Types; +using ZB.MOM.WW.OtOpcUa.Runtime.Drivers; +using ZB.MOM.WW.OtOpcUa.Runtime.OpcUa; +using ZB.MOM.WW.OtOpcUa.Runtime.Tests.Harness; +using static ZB.MOM.WW.OtOpcUa.Runtime.Tests.Harness.V3ArtifactSeeder; + +namespace ZB.MOM.WW.OtOpcUa.Runtime.Tests.Drivers; + +/// +/// Batched value fan-out in : a driver child's +/// burst costs exactly ONE mux hop and +/// ONE OPC UA publish hop, regardless of how many values it carries — the point of the batching work. +/// +/// The routing itself is unchanged from the per-value path: each item's FullReference +/// (== the tag's RawPath) resolves through the host's _nodeIdByDriverRef map to the RAW NodeId +/// AND every referencing UNS NodeId, each carrying IDENTICAL value/status/timestamp (v3 Batch 4 +/// single-source fan-out), and an unmapped ref is dropped rather than faulting the burst. These pin +/// the message COUNT as well as the contents, because a per-item unwrap satisfies the contents alone. +/// +/// +/// Arrangement mirrors — a real apply through the seeded +/// v3 artifact () builds the map; sink + mux are +/// s. +/// +/// +public sealed class DriverHostActorBatchFanOutTests : RuntimeActorTestBase +{ + private static readonly NodeId TestNode = NodeId.Parse("driver-batch-test"); + private static readonly RevisionHash RevA = RevisionHash.Parse(new string('b', 64)); + private static readonly DateTime TsA = new(2026, 8, 15, 9, 0, 0, DateTimeKind.Utc); + private static readonly DateTime TsB = new(2026, 8, 15, 9, 0, 1, DateTimeKind.Utc); + + /// A two-item burst whose first item is referenced by one equipment lands as a SINGLE + /// AttributeValueBatchUpdate carrying all three (raw, uns, raw) writes verbatim — including a non-Good + /// sub-code (#519), which rides through unbucketed. + [Fact] + public void Batch_fans_out_to_all_node_realm_refs_in_one_publish_message() + { + var db = NewInMemoryDbFactory(); + var deploymentId = SeedV3Deployment(db, RevA, + rawTags: new[] + { + (Driver: "drv-1", DriverName: "Modbus", Device: "dev1", Tag: "speed", DataType: "Double", Writable: false), + (Driver: "drv-1", DriverName: "Modbus", Device: "dev1", Tag: "temp", DataType: "Double", Writable: false), + }, + unsRefs: new[] { (Area: "filling", Line: "line1", Equip: "station1", BackingTag: "speed", Effective: (string?)null) }); + + var (actor, publish, _) = SpawnHostAndApply(this, db, TestNode, deploymentId, RevA); + + actor.Tell(new DriverInstanceActor.AttributeValueBatchPublished("drv-1", new[] + { + new DriverInstanceActor.AttributeValueItem("Plant/Modbus/dev1/speed", 42.0, OpcUaStatus.Good, TsA), + new DriverInstanceActor.AttributeValueItem("Plant/Modbus/dev1/temp", 7.5, OpcUaStatus.BadNoCommunication, TsB), + })); + + var batch = publish.ExpectMsg(PresenceBudget); + batch.Items.Count.ShouldBe(3); + + var byId = batch.Items.ToDictionary(w => w.NodeId); + byId.Keys.ShouldBe(new[] + { + "Plant/Modbus/dev1/speed", + "filling/line1/station1/speed", + "Plant/Modbus/dev1/temp", + }, ignoreOrder: true); + + // Item A: raw + UNS, identical value/quality/timestamp (fan-out drift guard). + foreach (var id in new[] { "Plant/Modbus/dev1/speed", "filling/line1/station1/speed" }) + { + byId[id].Value.ShouldBe(42.0); + byId[id].StatusCode.ShouldBe(OpcUaStatus.Good); + byId[id].SourceTimestampUtc.ShouldBe(TsA); + } + byId["Plant/Modbus/dev1/speed"].Realm.ShouldBe(AddressSpaceRealm.Raw); + byId["filling/line1/station1/speed"].Realm.ShouldBe(AddressSpaceRealm.Uns); + + // Item B: raw only, and its own status sub-code + timestamp — not item A's. + byId["Plant/Modbus/dev1/temp"].Value.ShouldBe(7.5); + byId["Plant/Modbus/dev1/temp"].StatusCode.ShouldBe(OpcUaStatus.BadNoCommunication); + byId["Plant/Modbus/dev1/temp"].SourceTimestampUtc.ShouldBe(TsB); + byId["Plant/Modbus/dev1/temp"].Realm.ShouldBe(AddressSpaceRealm.Raw); + + // ONE message, not one per value. + publish.ExpectNoMsg(TimeSpan.FromMilliseconds(300)); + } + + /// An item whose ref is bound to no NodeId is dropped; the rest of the burst still lands in the + /// one message and nothing faults. + [Fact] + public void Unmapped_items_are_dropped_not_faulted() + { + var db = NewInMemoryDbFactory(); + var deploymentId = SeedV3Deployment(db, RevA, + rawTags: new[] { (Driver: "drv-1", DriverName: "Modbus", Device: "dev1", Tag: "speed", DataType: "Double", Writable: false) }, + unsRefs: Array.Empty<(string, string, string, string, string?)>()); + + var (actor, publish, _) = SpawnHostAndApply(this, db, TestNode, deploymentId, RevA); + + actor.Tell(new DriverInstanceActor.AttributeValueBatchPublished("drv-1", new[] + { + new DriverInstanceActor.AttributeValueItem("Plant/Modbus/dev1/nope", 99.0, OpcUaStatus.Good, TsA), + new DriverInstanceActor.AttributeValueItem("Plant/Modbus/dev1/speed", 42.0, OpcUaStatus.Good, TsA), + })); + + var batch = publish.ExpectMsg(PresenceBudget); + batch.Items.Count.ShouldBe(1); + batch.Items[0].NodeId.ShouldBe("Plant/Modbus/dev1/speed"); + batch.Items[0].Value.ShouldBe(42.0); + + // The host is still alive and routing after the unmapped item. + actor.Tell(new DriverInstanceActor.AttributeValueBatchPublished("drv-1", new[] + { + new DriverInstanceActor.AttributeValueItem("Plant/Modbus/dev1/speed", 43.0, OpcUaStatus.Good, TsB), + })); + publish.ExpectMsg(PresenceBudget) + .Items[0].Value.ShouldBe(43.0); + } + + /// The dependency mux gets ONE envelope carrying the whole burst — not N per-item messages. + /// The mux filters per item itself. + [Fact] + public void Batch_reaches_mux_once() + { + var db = NewInMemoryDbFactory(); + var deploymentId = SeedV3Deployment(db, RevA, + rawTags: new[] + { + (Driver: "drv-1", DriverName: "Modbus", Device: "dev1", Tag: "speed", DataType: "Double", Writable: false), + (Driver: "drv-1", DriverName: "Modbus", Device: "dev1", Tag: "temp", DataType: "Double", Writable: false), + }, + unsRefs: Array.Empty<(string, string, string, string, string?)>()); + + var (actor, _, mux) = SpawnHostAndApply(this, db, TestNode, deploymentId, RevA); + + actor.Tell(new DriverInstanceActor.AttributeValueBatchPublished("drv-1", new[] + { + new DriverInstanceActor.AttributeValueItem("Plant/Modbus/dev1/speed", 42.0, OpcUaStatus.Good, TsA), + new DriverInstanceActor.AttributeValueItem("Plant/Modbus/dev1/temp", 7.5, OpcUaStatus.Good, TsB), + })); + + var fwd = mux.ExpectMsg(PresenceBudget); + fwd.DriverInstanceId.ShouldBe("drv-1"); + fwd.Items.Select(i => i.FullReference).ShouldBe( + new[] { "Plant/Modbus/dev1/speed", "Plant/Modbus/dev1/temp" }); + mux.ExpectNoMsg(TimeSpan.FromMilliseconds(300)); + } + + /// A burst in which NOTHING resolves sends no publish message at all — an empty batch would be + /// wasted work at the sink — while the mux still gets its one envelope (virtual-tag inputs are keyed by + /// ref, independent of whether the ref is materialised as a node). + [Fact] + public void All_unmapped_batch_sends_nothing_to_publish_actor() + { + var db = NewInMemoryDbFactory(); + var deploymentId = SeedV3Deployment(db, RevA, + rawTags: new[] { (Driver: "drv-1", DriverName: "Modbus", Device: "dev1", Tag: "speed", DataType: "Double", Writable: false) }, + unsRefs: Array.Empty<(string, string, string, string, string?)>()); + + var (actor, publish, mux) = SpawnHostAndApply(this, db, TestNode, deploymentId, RevA); + + actor.Tell(new DriverInstanceActor.AttributeValueBatchPublished("drv-1", new[] + { + new DriverInstanceActor.AttributeValueItem("Plant/Modbus/dev1/nope", 1.0, OpcUaStatus.Good, TsA), + new DriverInstanceActor.AttributeValueItem("Plant/Modbus/dev1/alsonope", 2.0, OpcUaStatus.Good, TsB), + })); + + mux.ExpectMsg(PresenceBudget) + .Items.Count.ShouldBe(2); + publish.ExpectNoMsg(TimeSpan.FromMilliseconds(500)); + } + + /// A host with NO OPC UA publish actor wired (the ctor default — the dev/Mac shape) still + /// makes the mux hop: the batch fan-out returns early on a null sink, and returning early BEFORE the + /// mux Tell would silently strand virtual-tag and continuous-historization inputs. + [Fact] + public void Mux_still_receives_batch_when_no_publish_actor_is_wired() + { + var db = NewInMemoryDbFactory(); + var deploymentId = SeedV3Deployment(db, RevA, + rawTags: new[] { (Driver: "drv-1", DriverName: "Modbus", Device: "dev1", Tag: "speed", DataType: "Double", Writable: false) }, + unsRefs: Array.Empty<(string, string, string, string, string?)>()); + + var (actor, mux) = SpawnHostAndApplyWithoutPublishActor(this, db, TestNode, deploymentId, RevA); + + actor.Tell(new DriverInstanceActor.AttributeValueBatchPublished("drv-1", new[] + { + new DriverInstanceActor.AttributeValueItem("Plant/Modbus/dev1/speed", 42.0, OpcUaStatus.Good, TsA), + })); + + var fwd = mux.ExpectMsg(PresenceBudget); + fwd.DriverInstanceId.ShouldBe("drv-1"); + fwd.Items.Single().FullReference.ShouldBe("Plant/Modbus/dev1/speed"); + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorLiveValueTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorLiveValueTests.cs index df113425..d86e4ce9 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorLiveValueTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorLiveValueTests.cs @@ -1,19 +1,12 @@ -using System.Text.Json; using Akka.Actor; -using Microsoft.EntityFrameworkCore; using Shouldly; using Xunit; -using ZB.MOM.WW.OtOpcUa.Commons.Engines; -using ZB.MOM.WW.OtOpcUa.Commons.Messages.Deploy; -using ZB.MOM.WW.OtOpcUa.Commons.Messages.Fleet; using ZB.MOM.WW.OtOpcUa.Commons.OpcUa; 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.Drivers; using ZB.MOM.WW.OtOpcUa.Runtime.OpcUa; using ZB.MOM.WW.OtOpcUa.Runtime.Tests.Harness; +using static ZB.MOM.WW.OtOpcUa.Runtime.Tests.Harness.V3ArtifactSeeder; namespace ZB.MOM.WW.OtOpcUa.Runtime.Tests.Drivers; @@ -22,14 +15,22 @@ namespace ZB.MOM.WW.OtOpcUa.Runtime.Tests.Drivers; /// . The driver publishes a value keyed by its wire-ref (== the tag's /// RawPath); the host's _nodeIdByDriverRef map — rebuilt each apply from the composition's /// RawTags ∪ UnsReferenceVariables — resolves (DriverInstanceId, RawPath) to the RAW -/// NodeId AND every referencing UNS NodeId, so ForwardToMux lands ONE publish on the raw node +/// NodeId AND every referencing UNS NodeId, so ForwardBatchToMux lands ONE write on the raw node /// () plus every UNS node () with /// IDENTICAL value/quality/timestamp (no independent buffer ⇒ no drift). The raw value is still forwarded /// to the dependency mux (VirtualTag inputs), keyed by the RawPath. /// -/// Drives a real apply through the existing harness: the seeded artifact carries the v3 raw-tag chain -/// (RawFolder → DriverInstance(RawFolderId) → Device → Tag) + an optional UnsTagReference projecting -/// each raw tag into an equipment. DispatchDeployment triggers the +/// Driven over the BATCH ingress, because that is the only one a driver child has: since the +/// value-pipeline batching work DriverInstanceActor.OnDataChangeForward wraps even a single +/// data change in an , and the host +/// no longer receives the per-value message at all. These fan-out cases are therefore stated as +/// single-item bursts; what they pin is the ROUTING, while +/// pins the message counts of a multi-item burst. +/// +/// +/// Drives a real apply through : the seeded artifact carries the v3 +/// raw-tag chain (RawFolder → DriverInstance(RawFolderId) → Device → Tag) + an optional +/// UnsTagReference projecting each raw tag into an equipment. DispatchDeployment triggers the /// ApplyAndAck → PushDesiredSubscriptions pass that builds the map; the sink and mux are /// injected as s. /// @@ -40,6 +41,12 @@ public sealed class DriverHostActorLiveValueTests : RuntimeActorTestBase private static readonly RevisionHash RevA = RevisionHash.Parse(new string('a', 64)); private static readonly DateTime Ts = new(2026, 6, 13, 10, 0, 0, DateTimeKind.Utc); + /// The single-item burst a driver child emits for one data change — every value ingress into + /// the host is an AttributeValueBatchPublished, so the fan-out cases below state their input that + /// way. + private static DriverInstanceActor.AttributeValueBatchPublished Burst((string Ref, object Value) item) => + new("drv-1", new[] { new DriverInstanceActor.AttributeValueItem(item.Ref, item.Value, OpcUaStatus.Good, Ts) }); + /// A driver value published by RawPath fans to the RAW node AND the referencing UNS node, each /// carrying identical value/quality/timestamp — the fan-out drift guard. [Fact] @@ -50,15 +57,14 @@ public sealed class DriverHostActorLiveValueTests : RuntimeActorTestBase 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) }); - var (actor, publish, mux) = SpawnHostAndApply(db, deploymentId); + var (actor, publish, mux) = SpawnHostAndApply(this, db, TestNode, deploymentId, RevA); - // The driver publishes by its wire-ref RawPath. - actor.Tell(new DriverInstanceActor.AttributeValuePublished( - "drv-1", "Plant/Modbus/dev1/speed", 42.0, OpcUaStatus.Good, Ts)); + // The driver publishes by its wire-ref RawPath, in the single-item burst DriverInstanceActor + // emits for one data change. + actor.Tell(Burst(("Plant/Modbus/dev1/speed", 42.0))); - var a = publish.ExpectMsg(TimeSpan.FromSeconds(5)); - var b = publish.ExpectMsg(TimeSpan.FromSeconds(5)); - var byId = new[] { a, b }.ToDictionary(u => u.NodeId); + var byId = publish.ExpectMsg(PresenceBudget) + .Items.ToDictionary(u => u.NodeId); byId.Keys.ShouldBe(new[] { "Plant/Modbus/dev1/speed", "filling/line1/station1/speed" }, ignoreOrder: true); // Realm travels with each update so the sink resolves the right namespace. @@ -69,12 +75,12 @@ public sealed class DriverHostActorLiveValueTests : RuntimeActorTestBase { u.Value.ShouldBe(42.0); u.StatusCode.ShouldBe(OpcUaStatus.Good); - u.TimestampUtc.ShouldBe(Ts); + u.SourceTimestampUtc.ShouldBe(Ts); } // The raw publish still reaches the dependency mux (VirtualTag inputs), keyed by RawPath. - mux.ExpectMsg(TimeSpan.FromSeconds(5)) - .FullReference.ShouldBe("Plant/Modbus/dev1/speed"); + mux.ExpectMsg(PresenceBudget) + .Items.Single().FullReference.ShouldBe("Plant/Modbus/dev1/speed"); } /// A raw tag with NO UNS reference fans to ONLY its raw NodeId (Raw realm). @@ -86,12 +92,12 @@ public sealed class DriverHostActorLiveValueTests : RuntimeActorTestBase rawTags: new[] { (Driver: "drv-1", DriverName: "Modbus", Device: "dev1", Tag: "speed", DataType: "Double", Writable: false) }, unsRefs: Array.Empty<(string, string, string, string, string?)>()); - var (actor, publish, _) = SpawnHostAndApply(db, deploymentId); + var (actor, publish, _) = SpawnHostAndApply(this, db, TestNode, deploymentId, RevA); - actor.Tell(new DriverInstanceActor.AttributeValuePublished( - "drv-1", "Plant/Modbus/dev1/speed", 5.0, OpcUaStatus.Good, Ts)); + actor.Tell(Burst(("Plant/Modbus/dev1/speed", 5.0))); - var only = publish.ExpectMsg(TimeSpan.FromSeconds(5)); + var only = publish.ExpectMsg(PresenceBudget) + .Items.ShouldHaveSingleItem(); only.NodeId.ShouldBe("Plant/Modbus/dev1/speed"); only.Realm.ShouldBe(AddressSpaceRealm.Raw); publish.ExpectNoMsg(TimeSpan.FromMilliseconds(300)); @@ -111,17 +117,13 @@ public sealed class DriverHostActorLiveValueTests : RuntimeActorTestBase (Area: "filling", Line: "line1", Equip: "station2", BackingTag: "speed", Effective: (string?)null), }); - var (actor, publish, _) = SpawnHostAndApply(db, deploymentId); + var (actor, publish, _) = SpawnHostAndApply(this, db, TestNode, deploymentId, RevA); - actor.Tell(new DriverInstanceActor.AttributeValuePublished( - "drv-1", "Plant/Modbus/dev1/speed", 7.0, OpcUaStatus.Good, Ts)); + actor.Tell(Burst(("Plant/Modbus/dev1/speed", 7.0))); - var updates = new[] - { - publish.ExpectMsg(TimeSpan.FromSeconds(5)), - publish.ExpectMsg(TimeSpan.FromSeconds(5)), - publish.ExpectMsg(TimeSpan.FromSeconds(5)), - }; + // 3-wide fan-out inside ONE batch message — the batch path's own 1:N coverage. + var updates = publish.ExpectMsg(PresenceBudget).Items; + updates.Count.ShouldBe(3); updates.Select(u => u.NodeId).ShouldBe(new[] { "Plant/Modbus/dev1/speed", @@ -141,14 +143,13 @@ public sealed class DriverHostActorLiveValueTests : RuntimeActorTestBase rawTags: new[] { (Driver: "drv-1", DriverName: "Modbus", Device: "dev1", Tag: "speed", DataType: "Double", Writable: false) }, unsRefs: Array.Empty<(string, string, string, string, string?)>()); - var (actor, publish, mux) = SpawnHostAndApply(db, deploymentId); + var (actor, publish, mux) = SpawnHostAndApply(this, db, TestNode, deploymentId, RevA); - actor.Tell(new DriverInstanceActor.AttributeValuePublished( - "drv-1", "Plant/Modbus/dev1/nope", 99.0, OpcUaStatus.Good, Ts)); + actor.Tell(Burst(("Plant/Modbus/dev1/nope", 99.0))); publish.ExpectNoMsg(TimeSpan.FromMilliseconds(500)); - mux.ExpectMsg(TimeSpan.FromSeconds(5)) - .FullReference.ShouldBe("Plant/Modbus/dev1/nope"); + mux.ExpectMsg(PresenceBudget) + .Items.Single().FullReference.ShouldBe("Plant/Modbus/dev1/nope"); } // ---- #481 (Layer 4): comms loss reaches the dependency mux ------------------------------------ @@ -172,14 +173,14 @@ public sealed class DriverHostActorLiveValueTests : RuntimeActorTestBase }, unsRefs: Array.Empty<(string, string, string, string, string?)>()); - var (actor, _, mux) = SpawnHostAndApply(db, deploymentId); + var (actor, _, mux) = SpawnHostAndApply(this, db, TestNode, deploymentId, RevA); actor.Tell(new DriverInstanceActor.ConnectivityChanged("drv-1", Connected: false)); var pushes = new[] { - mux.ExpectMsg(TimeSpan.FromSeconds(5)), - mux.ExpectMsg(TimeSpan.FromSeconds(5)), + mux.ExpectMsg(PresenceBudget), + mux.ExpectMsg(PresenceBudget), }; pushes.Select(p => p.FullReference).ShouldBe( @@ -215,7 +216,7 @@ public sealed class DriverHostActorLiveValueTests : RuntimeActorTestBase rawTags: new[] { (Driver: "drv-1", DriverName: "Modbus", Device: "dev1", Tag: "speed", DataType: "Double", Writable: false) }, unsRefs: Array.Empty<(string, string, string, string, string?)>()); - var (actor, _, mux) = SpawnHostAndApply(db, deploymentId); + var (actor, _, mux) = SpawnHostAndApply(this, db, TestNode, deploymentId, RevA); actor.Tell(new DriverInstanceActor.ConnectivityChanged("drv-1", Connected: true)); @@ -232,110 +233,11 @@ public sealed class DriverHostActorLiveValueTests : RuntimeActorTestBase rawTags: new[] { (Driver: "drv-1", DriverName: "Modbus", Device: "dev1", Tag: "speed", DataType: "Double", Writable: false) }, unsRefs: Array.Empty<(string, string, string, string, string?)>()); - var (actor, _, mux) = SpawnHostAndApply(db, deploymentId); + var (actor, _, mux) = SpawnHostAndApply(this, db, TestNode, deploymentId, RevA); actor.Tell(new DriverInstanceActor.ConnectivityChanged("drv-OTHER", Connected: false)); mux.ExpectNoMsg(TimeSpan.FromMilliseconds(500)); } - /// Spawns the host with publish + mux probes, dispatches the deployment, and waits for the - /// Applied ACK so the map build in PushDesiredSubscriptions has completed before the test publishes a - /// value. A VirtualTag-host probe is injected so the real host isn't spawned. - private (IActorRef Actor, Akka.TestKit.TestProbe Publish, Akka.TestKit.TestProbe Mux) SpawnHostAndApply( - IDbContextFactory db, DeploymentId deploymentId) - { - var coordinator = CreateTestProbe(); - var publish = CreateTestProbe(); - var mux = CreateTestProbe(); - var vtHost = CreateTestProbe(); - - var actor = Sys.ActorOf(DriverHostActor.Props( - db, TestNode, coordinator.Ref, - localRoles: new HashSet { "driver" }, - dependencyMux: mux.Ref, - opcUaPublishActor: publish.Ref, - virtualTagEvaluator: NullVirtualTagEvaluator.Instance, - virtualTagHostOverride: vtHost.Ref)); - - actor.Tell(new DispatchDeployment(deploymentId, RevA, CorrelationId.NewId())); - coordinator.ExpectMsg(TimeSpan.FromSeconds(5)).Outcome.ShouldBe(ApplyAckOutcome.Applied); - publish.ExpectMsg(TimeSpan.FromSeconds(5)); - - return (actor, publish, mux); - } - - /// Seeds a Sealed deployment whose artifact carries the v3 raw-tag chain (RawFolder "Plant" → - /// DriverInstance(RawFolderId) → Device → Tag) plus optional UnsTagReferences projecting a raw tag into an - /// Area/Line/Equipment. Each raw tag's RawPath is Plant/{DriverName}/{Device}/{Tag}; each UNS - /// reference's NodeId is {Area}/{Line}/{Equip}/{Effective ?? backing tag Name}. Enums serialize - /// numerically (AccessLevel: ReadWrite = 1). - private static DeploymentId SeedV3Deployment( - IDbContextFactory db, RevisionHash rev, - (string Driver, string DriverName, string Device, string Tag, string DataType, bool Writable)[] rawTags, - (string Area, string Line, string Equip, string BackingTag, string? Effective)[] unsRefs) - { - var drivers = rawTags - .Select(t => (t.Driver, t.DriverName)) - .Distinct() - .Select(d => new { DriverInstanceId = d.Driver, RawFolderId = "rf-plant", Name = d.DriverName, DriverType = "Modbus", DriverConfig = "{}", ClusterId = "c1", Enabled = false }) - .ToArray(); - var devices = rawTags - .Select(t => (t.Driver, t.Device)) - .Distinct() - .Select(d => new { DeviceId = $"{d.Driver}:{d.Device}", DriverInstanceId = d.Driver, Name = d.Device, DeviceConfig = "{}" }) - .ToArray(); - var tags = rawTags.Select(t => new - { - TagId = t.Tag, // tag id == name for test simplicity (unique per driver here) - DeviceId = $"{t.Driver}:{t.Device}", - TagGroupId = (string?)null, - Name = t.Tag, - DataType = t.DataType, - AccessLevel = t.Writable ? 1 : 0, // TagAccessLevel.ReadWrite = 1 - TagConfig = "{}", - }).ToArray(); - - // UNS topology: one area/line per distinct (Area, Line); one equipment per (Area, Line, Equip). - var areas = unsRefs.Select(r => r.Area).Distinct(StringComparer.Ordinal) - .Select(a => new { UnsAreaId = $"a-{a}", Name = a, ClusterId = "c1" }).ToArray(); - var lines = unsRefs.Select(r => (r.Area, r.Line)).Distinct() - .Select(l => new { UnsLineId = $"l-{l.Area}-{l.Line}", UnsAreaId = $"a-{l.Area}", Name = l.Line }).ToArray(); - var equipment = unsRefs.Select(r => (r.Area, r.Line, r.Equip)).Distinct() - .Select(e => new { EquipmentId = $"EQ-{e.Area}-{e.Line}-{e.Equip}", UnsLineId = $"l-{e.Area}-{e.Line}", Name = e.Equip, MachineCode = e.Equip }).ToArray(); - var unsTagReferences = unsRefs.Select((r, i) => new - { - UnsTagReferenceId = $"ref-{i}", - EquipmentId = $"EQ-{r.Area}-{r.Line}-{r.Equip}", - TagId = r.BackingTag, - DisplayNameOverride = r.Effective, - }).ToArray(); - - var artifact = JsonSerializer.SerializeToUtf8Bytes(new - { - RawFolders = new[] { new { RawFolderId = "rf-plant", ParentRawFolderId = (string?)null, Name = "Plant", ClusterId = "c1" } }, - DriverInstances = drivers, - Devices = devices, - TagGroups = Array.Empty(), - Tags = tags, - UnsAreas = areas, - UnsLines = lines, - Equipment = equipment, - UnsTagReferences = unsTagReferences, - }); - - 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; - } } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorBatchForwardTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorBatchForwardTests.cs new file mode 100644 index 00000000..922a12fc --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorBatchForwardTests.cs @@ -0,0 +1,332 @@ +using Akka.Actor; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Commons.OpcUa; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Runtime.Drivers; +using ZB.MOM.WW.OtOpcUa.Runtime.Tests.Harness; + +namespace ZB.MOM.WW.OtOpcUa.Runtime.Tests.Drivers; + +/// +/// Covers the ingress seam of the value-pipeline batching work: +/// forwards data changes to its parent as +/// — ONE message per driver production burst. +/// A driver implementing gets the batch handler attached and the +/// per-tag handler NOT attached: such a driver raises BOTH +/// events per the contract, so attaching both would double-deliver every value. A per-tag-only +/// driver keeps working unchanged — each raise is wrapped as a +/// single-item batch, so the parent sees exactly one message shape either way. +/// Status codes ride verbatim (#519); timestamps come from the driver's snapshot +/// (SourceTimestampUtc ?? ServerTimestampUtc), never from the actor's clock. +/// +public sealed class DriverInstanceActorBatchForwardTests : RuntimeActorTestBase +{ + /// + /// A batch-capable driver's burst of 3 changed tags reaches the parent as exactly ONE + /// carrying all 3 items, each mapped + /// verbatim (reference / value / status code) with the timestamp derived from the snapshot. + /// + [Fact] + public async Task Batch_driver_forwards_one_batch_message() + { + var driver = new BatchSubscribableStubDriver(); + var parent = CreateTestProbe(); + // #477: DriverInstanceActor Tells the parent ConnectivityChanged on each connect/disconnect; + // this suite asserts on the value forwards only, so ignore it. + parent.IgnoreMessages(m => m is DriverInstanceActor.ConnectivityChanged); + var actor = parent.ChildActorOf(DriverInstanceActor.Props(driver)); + + actor.Tell(new DriverInstanceActor.InitializeRequested("{}")); + AwaitCondition(() => driver.InitializeCount > 0, PresenceBudget); + + await actor.Ask( + new DriverInstanceActor.Subscribe(new[] { "tag-a", "tag-b", "tag-c" }, TimeSpan.FromMilliseconds(100)), + TimeSpan.FromSeconds(3)); + + var sourceTs = new DateTime(2026, 8, 15, 10, 0, 0, DateTimeKind.Utc); + var serverTs = new DateTime(2026, 8, 15, 10, 0, 5, DateTimeKind.Utc); + driver.FireBatch(includePerTagRaises: false, + new DataChangeItem("tag-a", new DataValueSnapshot(3.14, OpcUaStatus.Good, sourceTs, serverTs)), + new DataChangeItem("tag-b", new DataValueSnapshot(7, 0x80740000u /* BadTypeMismatch */, sourceTs.AddSeconds(1), serverTs)), + // Null source timestamp — the server timestamp is the fallback, per OnDataChangeForward. + new DataChangeItem("tag-c", new DataValueSnapshot(null, OpcUaStatus.BadNoCommunication, null, serverTs))); + + var batch = parent.ExpectMsg(TimeSpan.FromSeconds(2)); + batch.DriverInstanceId.ShouldBe(driver.DriverInstanceId); + batch.Items.Count.ShouldBe(3); + + batch.Items[0].FullReference.ShouldBe("tag-a"); + batch.Items[0].Value.ShouldBe(3.14); + batch.Items[0].StatusCode.ShouldBe(OpcUaStatus.Good); + batch.Items[0].TimestampUtc.ShouldBe(sourceTs); + + batch.Items[1].FullReference.ShouldBe("tag-b"); + batch.Items[1].Value.ShouldBe(7); + // #519: the sub-code rides through, not a collapsed OpcUaStatus.Bad. + batch.Items[1].StatusCode.ShouldBe(0x80740000u); + batch.Items[1].TimestampUtc.ShouldBe(sourceTs.AddSeconds(1)); + + batch.Items[2].FullReference.ShouldBe("tag-c"); + batch.Items[2].Value.ShouldBeNull(); + batch.Items[2].StatusCode.ShouldBe(OpcUaStatus.BadNoCommunication); + batch.Items[2].TimestampUtc.ShouldBe(serverTs, "a null SourceTimestampUtc falls back to the server timestamp"); + + // Exactly ONE message for the burst — no per-item trailer. + parent.ExpectNoMsg(TimeSpan.FromMilliseconds(300)); + } + + /// + /// An driver raises BOTH the batch event and the per-tag + /// for every item (that is the documented driver contract, + /// kept for legacy consumers). The actor must attach ONE handler only, so the parent sees the batch + /// and nothing else — attaching both would publish every value twice. + /// + [Fact] + public async Task Batch_driver_per_tag_event_not_double_forwarded() + { + var driver = new BatchSubscribableStubDriver(); + var parent = CreateTestProbe(); + parent.IgnoreMessages(m => m is DriverInstanceActor.ConnectivityChanged); + var actor = parent.ChildActorOf(DriverInstanceActor.Props(driver)); + + actor.Tell(new DriverInstanceActor.InitializeRequested("{}")); + AwaitCondition(() => driver.InitializeCount > 0, PresenceBudget); + + await actor.Ask( + new DriverInstanceActor.Subscribe(new[] { "tag-a", "tag-b" }, TimeSpan.FromMilliseconds(100)), + TimeSpan.FromSeconds(3)); + + // Deterministic half of the assertion: the per-tag handler was never attached at all. ExpectNoMsg + // below only proves nothing arrived within its window; this proves it structurally cannot. + driver.OnDataChangeSubscriberCount.ShouldBe(0, "a batch-capable driver must NOT get the per-tag handler"); + + var ts = new DateTime(2026, 8, 15, 11, 0, 0, DateTimeKind.Utc); + driver.FireBatch(includePerTagRaises: true, + new DataChangeItem("tag-a", new DataValueSnapshot(1, OpcUaStatus.Good, ts, ts)), + new DataChangeItem("tag-b", new DataValueSnapshot(2, OpcUaStatus.Good, ts, ts))); + + var batch = parent.ExpectMsg(TimeSpan.FromSeconds(2)); + batch.Items.Count.ShouldBe(2); + + // The per-tag raises for the SAME burst produced nothing extra — no second batch, no + // AttributeValuePublished. Anything here means both handlers were attached. + parent.ExpectNoMsg(TimeSpan.FromMilliseconds(400)); + } + + /// + /// A driver that implements only keeps its per-tag handler and each raise + /// arrives at the parent as a single-item batch — the whole fleet minus the batch-capable drivers + /// stays on one message shape. + /// + [Fact] + public async Task Per_tag_driver_forwards_single_item_batches() + { + var driver = new SubscribableStubDriver(); + var parent = CreateTestProbe(); + parent.IgnoreMessages(m => m is DriverInstanceActor.ConnectivityChanged); + var actor = parent.ChildActorOf(DriverInstanceActor.Props(driver)); + + actor.Tell(new DriverInstanceActor.InitializeRequested("{}")); + AwaitCondition(() => driver.InitializeCount > 0, PresenceBudget); + + await actor.Ask( + new DriverInstanceActor.Subscribe(new[] { "tag-a" }, TimeSpan.FromMilliseconds(100)), + TimeSpan.FromSeconds(3)); + + driver.FireDataChange("tag-a", value: 3.14, statusCode: OpcUaStatus.Good); + + var batch = parent.ExpectMsg(TimeSpan.FromSeconds(2)); + batch.DriverInstanceId.ShouldBe(driver.DriverInstanceId); + batch.Items.Count.ShouldBe(1); + batch.Items[0].FullReference.ShouldBe("tag-a"); + batch.Items[0].Value.ShouldBe(3.14); + batch.Items[0].StatusCode.ShouldBe(OpcUaStatus.Good); + } + + /// + /// The detach is load-bearing — the object outlives the actor across respawns, + /// so a batch handler left attached would push into a dead/stale actor. After Unsubscribe, a burst + /// produces nothing at the parent. + /// + [Fact] + public async Task Detach_removes_batch_handler() + { + var driver = new BatchSubscribableStubDriver(); + var parent = CreateTestProbe(); + parent.IgnoreMessages(m => m is DriverInstanceActor.ConnectivityChanged); + var actor = parent.ChildActorOf(DriverInstanceActor.Props(driver)); + + actor.Tell(new DriverInstanceActor.InitializeRequested("{}")); + AwaitCondition(() => driver.InitializeCount > 0, PresenceBudget); + + await actor.Ask( + new DriverInstanceActor.Subscribe(new[] { "tag-a" }, TimeSpan.FromMilliseconds(100)), + TimeSpan.FromSeconds(3)); + driver.OnDataChangeBatchSubscriberCount.ShouldBe(1, "the batch handler is attached for an IBatchSubscribable driver"); + + actor.Tell(new DriverInstanceActor.Unsubscribe()); + + AwaitCondition(() => driver.OnDataChangeBatchSubscriberCount == 0, PresenceBudget); + + var ts = new DateTime(2026, 8, 15, 12, 0, 0, DateTimeKind.Utc); + driver.FireBatch(includePerTagRaises: true, + new DataChangeItem("tag-a", new DataValueSnapshot(42, OpcUaStatus.Good, ts, ts))); + + parent.ExpectNoMsg(TimeSpan.FromMilliseconds(400)); + } + + /// + /// Comms loss (, Connected → Reconnecting) detaches + /// the batch handler too. The object outlives the actor across a host respawn, so a + /// handler surviving the disconnect would push a burst produced by the dead transport into a stale actor. + /// + [Fact] + public async Task DisconnectObserved_detaches_batch_handler_so_late_bursts_are_dropped() + { + var driver = new BatchSubscribableStubDriver(); + var parent = CreateTestProbe(); + parent.IgnoreMessages(m => m is DriverInstanceActor.ConnectivityChanged); + var actor = parent.ChildActorOf(DriverInstanceActor.Props(driver, reconnectInterval: TimeSpan.FromSeconds(30))); + + actor.Tell(new DriverInstanceActor.InitializeRequested("{}")); + AwaitCondition(() => driver.InitializeCount > 0, PresenceBudget); + + await actor.Ask( + new DriverInstanceActor.Subscribe(new[] { "tag-a" }, TimeSpan.FromMilliseconds(100)), + TimeSpan.FromSeconds(3)); + driver.OnDataChangeBatchSubscriberCount.ShouldBe(1); + + actor.Tell(new DriverInstanceActor.DisconnectObserved("backend went away")); + + // Race window — once the disconnect is processed the handler is gone and a late burst is inert. + AwaitCondition(() => driver.OnDataChangeBatchSubscriberCount == 0, PresenceBudget); + + var ts = new DateTime(2026, 8, 15, 13, 0, 0, DateTimeKind.Utc); + driver.FireBatch(includePerTagRaises: true, + new DataChangeItem("tag-a", new DataValueSnapshot(42, OpcUaStatus.Good, ts, ts))); + + parent.ExpectNoMsg(TimeSpan.FromMilliseconds(300)); + } + + /// + /// Stopping the actor detaches the batch handler in PostStop — the same leak guard as the + /// per-tag path, and the one that matters when the host respawns a child around the same + /// instance. + /// + [Fact] + public async Task PostStop_detaches_batch_handler() + { + var driver = new BatchSubscribableStubDriver(); + var parent = CreateTestProbe(); + parent.IgnoreMessages(m => m is DriverInstanceActor.ConnectivityChanged); + var actor = parent.ChildActorOf(DriverInstanceActor.Props(driver)); + + actor.Tell(new DriverInstanceActor.InitializeRequested("{}")); + AwaitCondition(() => driver.InitializeCount > 0, PresenceBudget); + + await actor.Ask( + new DriverInstanceActor.Subscribe(new[] { "tag-a" }, TimeSpan.FromMilliseconds(100)), + TimeSpan.FromSeconds(3)); + driver.OnDataChangeBatchSubscriberCount.ShouldBe(1); + + Sys.Stop(actor); + + AwaitCondition(() => driver.OnDataChangeBatchSubscriberCount == 0, PresenceBudget); + } + + /// + /// A malformed raise from a buggy driver — a null payload, or one carrying no items — is dropped at + /// the handler and never reaches the mailbox. Unguarded, the null would throw inside the event + /// handler, restart the actor, and take the driver transport down with it; the still-attached handler + /// (count unchanged) and the burst that lands afterwards prove neither happened. + /// + [Fact] + public async Task Malformed_batch_raise_is_dropped_without_restarting_the_actor() + { + var driver = new BatchSubscribableStubDriver(); + var parent = CreateTestProbe(); + parent.IgnoreMessages(m => m is DriverInstanceActor.ConnectivityChanged); + var actor = parent.ChildActorOf(DriverInstanceActor.Props(driver)); + + actor.Tell(new DriverInstanceActor.InitializeRequested("{}")); + AwaitCondition(() => driver.InitializeCount > 0, PresenceBudget); + + await actor.Ask( + new DriverInstanceActor.Subscribe(new[] { "tag-a" }, TimeSpan.FromMilliseconds(100)), + TimeSpan.FromSeconds(3)); + + driver.FireBatch(includePerTagRaises: false /* no items — an empty burst */); + driver.FireNullBatch(); + + parent.ExpectNoMsg(TimeSpan.FromMilliseconds(300)); + driver.OnDataChangeBatchSubscriberCount.ShouldBe(1, + "a restart would have detached the handler in PostStop"); + + // …and the subscription is still live: a well-formed burst still lands. + var ts = new DateTime(2026, 8, 15, 14, 0, 0, DateTimeKind.Utc); + driver.FireBatch(includePerTagRaises: false, + new DataChangeItem("tag-a", new DataValueSnapshot(5, OpcUaStatus.Good, ts, ts))); + + var batch = parent.ExpectMsg(TimeSpan.FromSeconds(2)); + batch.Items.Count.ShouldBe(1); + batch.Items[0].FullReference.ShouldBe("tag-a"); + batch.Items[0].Value.ShouldBe(5); + } + + /// + /// A that implements — it raises + /// once per burst AND (optionally, per the documented driver + /// contract) the per-tag for each item of that same burst. + /// + private sealed class BatchSubscribableStubDriver : StubDriver, IBatchSubscribable + { + private readonly StubHandle _handle = new(); + + /// Occurs when a single attribute changes (legacy per-tag contract). + public event EventHandler? OnDataChange; + + /// Occurs once per production burst with every attribute that changed in it. + public event EventHandler? OnDataChangeBatch; + + /// Gets the number of handlers attached to . + public int OnDataChangeSubscriberCount => OnDataChange?.GetInvocationList().Length ?? 0; + + /// Gets the number of handlers attached to . + public int OnDataChangeBatchSubscriberCount => OnDataChangeBatch?.GetInvocationList().Length ?? 0; + + /// Subscribes to the specified full references. + /// The full references to subscribe to. + /// The publishing interval. + /// Cancellation token for the operation. + public Task SubscribeAsync( + IReadOnlyList fullReferences, TimeSpan publishingInterval, CancellationToken cancellationToken) => + Task.FromResult(_handle); + + /// Unsubscribes from the specified subscription handle. + /// The subscription handle. + /// Cancellation token for the operation. + public Task UnsubscribeAsync(ISubscriptionHandle handle, CancellationToken cancellationToken) => + Task.CompletedTask; + + /// Raises one production burst. + /// + /// When true, also raises the per-tag for every item — the documented + /// contract a real batch-capable driver honours for legacy consumers. + /// + /// The changed attributes of this burst. + public void FireBatch(bool includePerTagRaises, params DataChangeItem[] items) + { + OnDataChangeBatch?.Invoke(this, new DataChangeBatchEventArgs(_handle, items)); + if (!includePerTagRaises) return; + foreach (var item in items) + { + OnDataChange?.Invoke(this, new DataChangeEventArgs(_handle, item.FullReference, item.Snapshot)); + } + } + + /// Raises the batch event with a null payload — what a buggy driver does, and what the + /// handler's guard exists to absorb. + public void FireNullBatch() => OnDataChangeBatch?.Invoke(this, null!); + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorNativeAlarmTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorNativeAlarmTests.cs index e1f22d7c..1e1ce406 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorNativeAlarmTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorNativeAlarmTests.cs @@ -15,7 +15,7 @@ namespace ZB.MOM.WW.OtOpcUa.Runtime.Tests.Drivers; /// (when the driver is an ) /// and forwards every transition to its parent as /// — mirroring the OnDataChange → -/// forward. The driver fires +/// forward. The driver fires /// OnAlarmEvent on its OWN thread; the actor marshals it onto the actor thread via Self. /// public sealed class DriverInstanceActorNativeAlarmTests : RuntimeActorTestBase @@ -63,8 +63,8 @@ public sealed class DriverInstanceActorNativeAlarmTests : RuntimeActorTestBase // The same driver's OnDataChange still flows independently — alarm + value events coexist. driver.FireDataChange("tag-z", value: 9.0, statusCode: 0u); - parent.ExpectMsg(TimeSpan.FromSeconds(2)) - .FullReference.ShouldBe("tag-z"); + parent.ExpectMsg(TimeSpan.FromSeconds(2)) + .Items[0].FullReference.ShouldBe("tag-z"); } /// @@ -131,8 +131,8 @@ public sealed class DriverInstanceActorNativeAlarmTests : RuntimeActorTestBase driver.FireDataChange("tag-a", value: 1.5, statusCode: 0u); // Data-change forwarding still works (no crash on AttachAlarmSource for a non-IAlarmSource driver). - var dc = parent.ExpectMsg(TimeSpan.FromSeconds(2)); - dc.FullReference.ShouldBe("tag-a"); + var dc = parent.ExpectMsg(TimeSpan.FromSeconds(2)); + dc.Items[0].FullReference.ShouldBe("tag-a"); // An AttributeAlarmPublished can never be produced for a non-IAlarmSource driver. parent.ExpectNoMsg(TimeSpan.FromMilliseconds(300)); } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorTests.cs index 7cc4e9b3..6f1c4958 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorTests.cs @@ -121,14 +121,16 @@ public sealed class DriverInstanceActorTests : RuntimeActorTestBase new DriverInstanceActor.Subscribe(new[] { "tag-a", "tag-b" }, TimeSpan.FromMilliseconds(250)), TimeSpan.FromSeconds(3)); - // Driver fires an OnDataChange — actor should forward it to its parent as - // AttributeValuePublished carrying the driver's status code. + // Driver fires an OnDataChange — actor should forward it to its parent as an + // AttributeValueBatchPublished (single-item, since this driver is per-tag-only) carrying the + // driver's status code. driver.FireDataChange("tag-a", value: 3.14, statusCode: 0u); - var published = parent.ExpectMsg(TimeSpan.FromSeconds(2)); - published.FullReference.ShouldBe("tag-a"); - published.Value.ShouldBe(3.14); - published.StatusCode.ShouldBe(OpcUaStatus.Good); + var published = parent.ExpectMsg(TimeSpan.FromSeconds(2)); + published.Items.Count.ShouldBe(1); + published.Items[0].FullReference.ShouldBe("tag-a"); + published.Items[0].Value.ShouldBe(3.14); + published.Items[0].StatusCode.ShouldBe(OpcUaStatus.Good); } /// @@ -162,17 +164,19 @@ public sealed class DriverInstanceActorTests : RuntimeActorTestBase // Uncertain — severity bits 01 (top 2 bits = 01). driver.FireDataChange("tag-1", value: 1, statusCode: 0x40000000u); - parent.ExpectMsg().StatusCode.ShouldBe(OpcUaStatus.Uncertain); + parent.ExpectMsg().Items[0].StatusCode + .ShouldBe(OpcUaStatus.Uncertain); // Bad — severity bits 10. driver.FireDataChange("tag-1", value: 2, statusCode: 0x80000000u); - parent.ExpectMsg().StatusCode.ShouldBe(OpcUaStatus.Bad); + parent.ExpectMsg().Items[0].StatusCode + .ShouldBe(OpcUaStatus.Bad); // The regression itself: a Bad SUB-code must arrive intact, not flattened to OpcUaStatus.Bad. // This is the exact code MTConnect maps the Agent's UNAVAILABLE sentinel to, and the live gate // that opened #519 observed it reaching a client as 0x80000000. driver.FireDataChange("tag-1", value: null, statusCode: OpcUaStatus.BadNoCommunication); - var noComms = parent.ExpectMsg(); + var noComms = parent.ExpectMsg().Items[0]; noComms.StatusCode.ShouldBe(OpcUaStatus.BadNoCommunication); noComms.StatusCode.ShouldNotBe(OpcUaStatus.Bad, "the sub-code was collapsed to a generic Bad"); OpcUaStatus.IsBad(noComms.StatusCode).ShouldBeTrue("it must still classify as Bad"); @@ -180,7 +184,8 @@ public sealed class DriverInstanceActorTests : RuntimeActorTestBase // A second, DIFFERENT Bad sub-code — the two were indistinguishable before the fix, so asserting // one alone would not prove the sub-code is carried rather than hard-coded. driver.FireDataChange("tag-1", value: null, statusCode: 0x80740000u /* BadTypeMismatch */); - parent.ExpectMsg().StatusCode.ShouldBe(0x80740000u); + parent.ExpectMsg().Items[0].StatusCode + .ShouldBe(0x80740000u); } /// @@ -209,7 +214,8 @@ public sealed class DriverInstanceActorTests : RuntimeActorTestBase // The auto-subscription is live — a data change reaches the parent. driver.FireDataChange("tag-a", value: 7, statusCode: 0u); - parent.ExpectMsg(TimeSpan.FromSeconds(2)).Value.ShouldBe(7); + parent.ExpectMsg(TimeSpan.FromSeconds(2)) + .Items[0].Value.ShouldBe(7); // Reconnect → the desired set is re-established without any new host message. actor.Tell(new DriverInstanceActor.DisconnectObserved("backend blip")); 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 be6fd400..c547321f 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 @@ -27,8 +27,11 @@ public abstract class RuntimeActorTestBase : TestKit /// idle machine are comfortable in isolation and marginal under that contention: three consecutive /// 30-run verification rounds each surfaced a different test failing on one, in three /// different files. Fixing them one at a time was chasing a distribution rather than a defect. + /// Public rather than protected so Harness helpers that are not themselves + /// subclasses — — budget their waits from the + /// same constant instead of re-deriving one. /// - protected static readonly TimeSpan PresenceBudget = TimeSpan.FromSeconds(15); + public static readonly TimeSpan PresenceBudget = TimeSpan.FromSeconds(15); /// Gets the Akka test HOCON configuration string for single-node cluster setup. protected static string AkkaTestHocon => @" diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Harness/V3ArtifactSeeder.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Harness/V3ArtifactSeeder.cs new file mode 100644 index 00000000..4ffc9a1d --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Harness/V3ArtifactSeeder.cs @@ -0,0 +1,173 @@ +using System.Text.Json; +using Akka.Actor; +using Akka.TestKit; +using Microsoft.EntityFrameworkCore; +using Shouldly; +using ZB.MOM.WW.OtOpcUa.Commons.Engines; +using ZB.MOM.WW.OtOpcUa.Commons.Messages.Deploy; +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.Drivers; +using ZB.MOM.WW.OtOpcUa.Runtime.OpcUa; + +namespace ZB.MOM.WW.OtOpcUa.Runtime.Tests.Harness; + +/// +/// Shared v3 deployment fixture for the value-routing suites: seeds a +/// Sealed artifact carrying the raw-tag chain (+ optional UNS references) and spawns a host that has +/// actually applied it, so _nodeIdByDriverRef is populated before the test publishes anything. +/// +/// Kept here rather than in either test class because the per-value and batched fan-out suites +/// assert different deliveries over the SAME arrangement — duplicating the seeder let the two +/// copies drift. Deliberately narrow: the assembly's other artifact fixtures seed their own +/// shapes and are not folded in. +/// +/// +public static class V3ArtifactSeeder +{ + /// Seeds a Sealed deployment whose artifact carries the v3 raw-tag chain (RawFolder "Plant" → + /// DriverInstance(RawFolderId) → Device → Tag) plus optional UnsTagReferences projecting a raw tag into an + /// Area/Line/Equipment. Each raw tag's RawPath is Plant/{DriverName}/{Device}/{Tag}; each UNS + /// reference's NodeId is {Area}/{Line}/{Equip}/{Effective ?? backing tag Name}. Enums serialize + /// numerically (AccessLevel: ReadWrite = 1). + /// The in-memory config-db factory to seed. + /// The revision hash the deployment is sealed under. + /// The raw tags to materialise, one per (driver, device, tag). + /// UNS references projecting a raw tag (by BackingTag) into an equipment. + /// The seeded deployment's id. + public static DeploymentId SeedV3Deployment( + IDbContextFactory db, RevisionHash rev, + (string Driver, string DriverName, string Device, string Tag, string DataType, bool Writable)[] rawTags, + (string Area, string Line, string Equip, string BackingTag, string? Effective)[] unsRefs) + { + var drivers = rawTags + .Select(t => (t.Driver, t.DriverName)) + .Distinct() + .Select(d => new { DriverInstanceId = d.Driver, RawFolderId = "rf-plant", Name = d.DriverName, DriverType = "Modbus", DriverConfig = "{}", ClusterId = "c1", Enabled = false }) + .ToArray(); + var devices = rawTags + .Select(t => (t.Driver, t.Device)) + .Distinct() + .Select(d => new { DeviceId = $"{d.Driver}:{d.Device}", DriverInstanceId = d.Driver, Name = d.Device, DeviceConfig = "{}" }) + .ToArray(); + var tags = rawTags.Select(t => new + { + TagId = t.Tag, // tag id == name for test simplicity (unique per driver here) + DeviceId = $"{t.Driver}:{t.Device}", + TagGroupId = (string?)null, + Name = t.Tag, + DataType = t.DataType, + AccessLevel = t.Writable ? 1 : 0, // TagAccessLevel.ReadWrite = 1 + TagConfig = "{}", + }).ToArray(); + + // UNS topology: one area/line per distinct (Area, Line); one equipment per (Area, Line, Equip). + var areas = unsRefs.Select(r => r.Area).Distinct(StringComparer.Ordinal) + .Select(a => new { UnsAreaId = $"a-{a}", Name = a, ClusterId = "c1" }).ToArray(); + var lines = unsRefs.Select(r => (r.Area, r.Line)).Distinct() + .Select(l => new { UnsLineId = $"l-{l.Area}-{l.Line}", UnsAreaId = $"a-{l.Area}", Name = l.Line }).ToArray(); + var equipment = unsRefs.Select(r => (r.Area, r.Line, r.Equip)).Distinct() + .Select(e => new { EquipmentId = $"EQ-{e.Area}-{e.Line}-{e.Equip}", UnsLineId = $"l-{e.Area}-{e.Line}", Name = e.Equip, MachineCode = e.Equip }).ToArray(); + var unsTagReferences = unsRefs.Select((r, i) => new + { + UnsTagReferenceId = $"ref-{i}", + EquipmentId = $"EQ-{r.Area}-{r.Line}-{r.Equip}", + TagId = r.BackingTag, + DisplayNameOverride = r.Effective, + }).ToArray(); + + var artifact = JsonSerializer.SerializeToUtf8Bytes(new + { + RawFolders = new[] { new { RawFolderId = "rf-plant", ParentRawFolderId = (string?)null, Name = "Plant", ClusterId = "c1" } }, + DriverInstances = drivers, + Devices = devices, + TagGroups = Array.Empty(), + Tags = tags, + UnsAreas = areas, + UnsLines = lines, + Equipment = equipment, + UnsTagReferences = unsTagReferences, + }); + + 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; + } + + /// Spawns the host with publish + mux probes, dispatches the deployment, and waits for the + /// Applied ACK so the map build in PushDesiredSubscriptions has completed before the test publishes a + /// value. A VirtualTag-host probe is injected so the real host isn't spawned. + /// The calling TestKit, supplying the ActorSystem + probes. + /// The factory whose deployment was seeded by . + /// The host's local node id. + /// The deployment to dispatch. + /// The revision hash the deployment was sealed under. + /// The host actor plus the publish and mux probes wired into it. + public static (IActorRef Actor, TestProbe Publish, TestProbe Mux) SpawnHostAndApply( + TestKitBase kit, IDbContextFactory db, + NodeId localNode, DeploymentId deploymentId, RevisionHash rev) + { + var publish = kit.CreateTestProbe(); + var (actor, mux, coordinator) = Spawn(kit, db, localNode, publish.Ref); + + Dispatch(actor, coordinator, deploymentId, rev); + publish.ExpectMsg(RuntimeActorTestBase.PresenceBudget); + + return (actor, publish, mux); + } + + /// The same arrangement with NO OPC UA publish actor wired (the ctor default) — the dev/Mac + /// shape, where the mux hop must still happen. Returns only the mux probe, since there is no sink. + /// The calling TestKit, supplying the ActorSystem + probes. + /// The factory whose deployment was seeded by . + /// The host's local node id. + /// The deployment to dispatch. + /// The revision hash the deployment was sealed under. + /// The host actor plus the mux probe wired into it. + public static (IActorRef Actor, TestProbe Mux) SpawnHostAndApplyWithoutPublishActor( + TestKitBase kit, IDbContextFactory db, + NodeId localNode, DeploymentId deploymentId, RevisionHash rev) + { + var (actor, mux, coordinator) = Spawn(kit, db, localNode, opcUaPublishActor: null); + Dispatch(actor, coordinator, deploymentId, rev); + return (actor, mux); + } + + private static (IActorRef Actor, TestProbe Mux, TestProbe Coordinator) Spawn( + TestKitBase kit, IDbContextFactory db, NodeId localNode, + IActorRef? opcUaPublishActor) + { + var coordinator = kit.CreateTestProbe(); + var mux = kit.CreateTestProbe(); + var vtHost = kit.CreateTestProbe(); + + var actor = kit.Sys.ActorOf(DriverHostActor.Props( + db, localNode, coordinator.Ref, + localRoles: new HashSet { "driver" }, + dependencyMux: mux.Ref, + opcUaPublishActor: opcUaPublishActor, + virtualTagEvaluator: NullVirtualTagEvaluator.Instance, + virtualTagHostOverride: vtHost.Ref)); + + return (actor, mux, coordinator); + } + + private static void Dispatch(IActorRef actor, TestProbe coordinator, DeploymentId deploymentId, RevisionHash rev) + { + actor.Tell(new DispatchDeployment(deploymentId, rev, CorrelationId.NewId())); + coordinator.ExpectMsg(RuntimeActorTestBase.PresenceBudget) + .Outcome.ShouldBe(ApplyAckOutcome.Applied); + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorBatchTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorBatchTests.cs new file mode 100644 index 00000000..08052bb7 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorBatchTests.cs @@ -0,0 +1,225 @@ +using Akka.Actor; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Commons.OpcUa; +using ZB.MOM.WW.OtOpcUa.Runtime.OpcUa; +using ZB.MOM.WW.OtOpcUa.Runtime.Tests.Harness; + +namespace ZB.MOM.WW.OtOpcUa.Runtime.Tests.OpcUa; + +/// +/// Task 7 of the value-pipeline batching work: +/// must reach the sink as exactly ONE call — that single +/// call is the whole point (the SDK-backed sink takes its lock once per batch instead of once per value), +/// so a handler that looped WriteValue would pass a "the values arrived" assertion while delivering +/// none of the win. Hence the fake counts per-item WriteValue calls separately and the assertions +/// require that count to be zero. +/// The per-item receive stays — VirtualTagHostActor +/// and the alarm paths still send it — so it is pinned here too. +/// +public sealed class OpcUaPublishActorBatchTests : RuntimeActorTestBase +{ + /// A 3-item batch lands as one WriteValues call carrying all three items verbatim (field-exact, + /// realm included), and never decomposes into per-item WriteValue calls. + [Fact] + public void Batch_message_reaches_sink_as_one_WriteValues_call() + { + var sink = new RecordingSink(); + var actor = Sys.ActorOf(OpcUaPublishActor.PropsForTests(sink: sink)); + + var t0 = new DateTime(2026, 8, 15, 10, 0, 0, DateTimeKind.Utc); + var items = new[] + { + new ValueWrite("Plant/Modbus/dev1/Speed", 42, OpcUaStatus.Good, t0, AddressSpaceRealm.Raw), + new ValueWrite("AreaA/Line1/Eq1/Speed", 42, OpcUaStatus.Good, t0, AddressSpaceRealm.Uns), + new ValueWrite("Plant/Modbus/dev1/Temp", 7.5d, OpcUaStatus.BadNoCommunication, + t0.AddSeconds(1), AddressSpaceRealm.Raw), + }; + + actor.Tell(new OpcUaPublishActor.AttributeValueBatchUpdate(items)); + + AwaitAssert(() => sink.Batches.Count.ShouldBe(1), PresenceBudget); + + var batch = sink.Batches[0]; + batch.Count.ShouldBe(3); + batch[0].ShouldBe(items[0]); + batch[1].ShouldBe(items[1]); + batch[2].ShouldBe(items[2]); + + // The load-bearing half: one batched call, NOT three per-item calls. + sink.SingleWrites.Count.ShouldBe(0); + } + + /// A sink that throws out of WriteValues is swallowed and logged — the actor survives and keeps + /// processing, rather than restarting and dropping traffic. + /// The survival assertion alone does NOT prove the catch exists: Akka's default supervision + /// RESTARTS a throwing actor and the restart preserves the mailbox, so a follow-up message lands either + /// way. The load-bearing assertions are therefore the two event filters — the exception must never reach + /// supervision (no Error carrying it is ever logged), and the handler's own Warning must be logged so the + /// fault is swallowed but not silent. Deleting the catch turns BOTH red. + [Fact] + public void Sink_throw_is_swallowed_and_logged() + { + var sink = new ThrowOnWriteValuesSink(); + var actor = Sys.ActorOf(OpcUaPublishActor.PropsForTests(sink: sink)); + + var t0 = new DateTime(2026, 8, 15, 10, 0, 0, DateTimeKind.Utc); + + // Expect(0, …) counts matches at the END of the action, so everything that must have happened by + // then goes INSIDE it — otherwise the absence assertion races the actor and passes vacuously. + EventFilter.Exception().Expect(0, () => + { + // The handler caught it AND said so: its own Warning, not supervision's Error. + EventFilter.Warning(contains: "sink.WriteValues threw").ExpectOne(() => + actor.Tell(new OpcUaPublishActor.AttributeValueBatchUpdate(new[] + { + new ValueWrite("Plant/Modbus/dev1/Speed", 1, OpcUaStatus.Good, t0, AddressSpaceRealm.Raw), + }))); + + // Survival: a follow-up on the untouched per-item path lands only if the actor is still pumping. + actor.Tell(new OpcUaPublishActor.AttributeValueUpdate( + "Plant/Modbus/dev1/Temp", 2, OpcUaStatus.Good, t0, AddressSpaceRealm.Raw)); + + AwaitAssert(() => sink.SingleWrites.Count.ShouldBe(1), PresenceBudget); + }); + + sink.Threw.ShouldBeTrue(); + } + + /// A PARTIAL failure is metered honestly: a sink that applies 2 of 3 items and reports the + /// aggregate as a must advance the write tally by the SUCCEEDED + /// count, not by zero (the general catch's behaviour) and not by the whole batch. The invariant the + /// batching work must not break is that the tally counts per ITEM — so a batch that half-failed reports + /// exactly the half that landed. + [Fact] + public void Partial_failure_still_counts_the_items_that_landed() + { + var sink = new PartialFailureSink(); + // TestActorRef so the actor's own WriteCount tally is readable — the counter the handler advances is + // the thing under test, and nothing else observes it. + var actor = ActorOfAsTestActorRef(OpcUaPublishActor.PropsForTests(sink: sink)); + + var t0 = new DateTime(2026, 8, 15, 10, 0, 0, DateTimeKind.Utc); + actor.Tell(new OpcUaPublishActor.AttributeValueBatchUpdate(new[] + { + new ValueWrite("Plant/Modbus/dev1/Speed", 1, OpcUaStatus.Good, t0, AddressSpaceRealm.Raw), + new ValueWrite("", 2, OpcUaStatus.Good, t0, AddressSpaceRealm.Raw), + new ValueWrite("Plant/Modbus/dev1/Temp", 3, OpcUaStatus.Good, t0, AddressSpaceRealm.Raw), + })); + + sink.Threw.ShouldBeTrue(); + actor.UnderlyingActor.WriteCount.ShouldBe(2, + "the two items the sink DID apply must still be counted — not 0 (the general catch) and not 3"); + + // The actor survived the partial failure and keeps pumping. + actor.Tell(new OpcUaPublishActor.AttributeValueUpdate( + "Plant/Modbus/dev1/Other", 4, OpcUaStatus.Good, t0, AddressSpaceRealm.Raw)); + sink.SingleWrites.Count.ShouldBe(1); + actor.UnderlyingActor.WriteCount.ShouldBe(3); + } + + /// The pre-existing single-item AttributeValueUpdate path is untouched by the batch addition. + [Fact] + public void Single_item_message_still_works() + { + var sink = new RecordingSink(); + var actor = Sys.ActorOf(OpcUaPublishActor.PropsForTests(sink: sink)); + + var t0 = new DateTime(2026, 8, 15, 10, 0, 0, DateTimeKind.Utc); + actor.Tell(new OpcUaPublishActor.AttributeValueUpdate( + "AreaA/Line1/Eq1/Speed", 99, OpcUaStatus.Good, t0, AddressSpaceRealm.Uns)); + + AwaitAssert(() => sink.SingleWrites.Count.ShouldBe(1), PresenceBudget); + sink.SingleWrites[0].ShouldBe(new ValueWrite("AreaA/Line1/Eq1/Speed", 99, OpcUaStatus.Good, t0, AddressSpaceRealm.Uns)); + sink.Batches.Count.ShouldBe(0); + } + + private void AwaitAssert(Action assertion, TimeSpan duration) + { + var deadline = DateTime.UtcNow + duration; + Exception? last = null; + while (DateTime.UtcNow < deadline) + { + try { assertion(); return; } + catch (Exception ex) { last = ex; Thread.Sleep(25); } + } + if (last is not null) throw last; + } + + /// Records batched and per-item value writes separately, so a handler that looped WriteValue + /// instead of calling WriteValues once is distinguishable from one that batched. + private class RecordingSink : IOpcUaAddressSpaceSink + { + private readonly object _gate = new(); + private readonly List> _batches = new(); + private readonly List _singles = new(); + + public IReadOnlyList> Batches { get { lock (_gate) return _batches.ToArray(); } } + public IReadOnlyList SingleWrites { get { lock (_gate) return _singles.ToArray(); } } + + public void WriteValue(string nodeId, object? value, uint statusCode, DateTime sourceTimestampUtc, AddressSpaceRealm realm = AddressSpaceRealm.Uns) + { + lock (_gate) _singles.Add(new ValueWrite(nodeId, value, statusCode, sourceTimestampUtc, realm)); + } + + // Overrides the interface's default WriteValues so the batch is observed as one call, not N. + public virtual void WriteValues(IReadOnlyList values) + { + lock (_gate) _batches.Add(values.ToArray()); + } + + public void WriteAlarmQuality(string alarmNodeId, uint statusCode, DateTime sourceTimestampUtc, AddressSpaceRealm realm) { } + public void WriteAlarmCondition(string alarmNodeId, AlarmConditionSnapshot state, DateTime occurredUtc, AddressSpaceRealm realm = AddressSpaceRealm.Uns) { } + public void MaterialiseAlarmCondition(string alarmNodeId, string equipmentNodeId, string displayName, string alarmType, int severity, AddressSpaceRealm realm, bool isNative = false) { } + public void EnsureFolder(string folderNodeId, string? parentNodeId, string displayName, AddressSpaceRealm realm = AddressSpaceRealm.Uns) { } + public void EnsureVariable(string variableNodeId, string? parentFolderNodeId, string displayName, string dataType, bool writable, AddressSpaceRealm realm, string? historianTagname = null, bool isArray = false, uint? arrayLength = null) { } + public void RebuildAddressSpace() { } + public void RaiseNodesAddedModelChange(string affectedNodeId, AddressSpaceRealm realm = AddressSpaceRealm.Uns) { } + 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") { } + } + + /// A sink whose batched write throws a PLAIN exception — drives the handler's GENERAL catch (an + /// unrecognised sink failure, where nothing is known about what landed, so nothing is counted). + private sealed class ThrowOnWriteValuesSink : RecordingSink + { + private int _threw; + + public bool Threw => Volatile.Read(ref _threw) != 0; + + public override void WriteValues(IReadOnlyList values) + { + Interlocked.Exchange(ref _threw, 1); + throw new InvalidOperationException("simulated batched-write fault"); + } + } + + /// A sink modelling the real SDK-backed one's PARTIAL failure: it applies every item with a + /// non-empty NodeId, then reports the aggregate as a . Drives the + /// handler's typed catch. + private sealed class PartialFailureSink : RecordingSink + { + private int _threw; + + public bool Threw => Volatile.Read(ref _threw) != 0; + + public override void WriteValues(IReadOnlyList values) + { + var applied = values.Where(v => !string.IsNullOrEmpty(v.NodeId)).ToArray(); + var failedCount = values.Count - applied.Length; + if (failedCount == 0) + { + base.WriteValues(values); + return; + } + + base.WriteValues(applied); + Interlocked.Exchange(ref _threw, 1); + throw new ValueWriteBatchException( + firstFailedNodeId: values.First(v => string.IsNullOrEmpty(v.NodeId)).NodeId, + failedCount: failedCount, + succeededCount: applied.Length, + firstFailure: new ArgumentException("empty NodeId")); + } + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/VirtualTags/DependencyMuxActorTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/VirtualTags/DependencyMuxActorTests.cs index c5960c68..0ea82630 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/VirtualTags/DependencyMuxActorTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/VirtualTags/DependencyMuxActorTests.cs @@ -124,13 +124,14 @@ public sealed class DependencyMuxActorTests : RuntimeActorTestBase parent.ExpectNoMsg(TimeSpan.FromMilliseconds(200)); } - /// Verifies that the DriverHostActor forwards AttributeValuePublished messages through the dependency mux. + /// Verifies that the DriverHostActor forwards a driver burst through the dependency mux to the + /// interested subscriber. [Fact] - public void DriverHostActor_forwards_AttributeValuePublished_through_to_mux() + public void DriverHostActor_forwards_AttributeValueBatchPublished_through_to_mux() { - // Spin a mux + a stand-in for DriverHostActor that wraps the real DriverHostActor's - // forward path. We feed it AttributeValuePublished directly to verify the routing — - // exercising the actual DriverHost spawn would require a deployment artifact + DI. + // Spin a mux + a real DriverHostActor and feed it the batch a driver child would send — + // exercising a full DriverHost apply would require a deployment artifact + DI, and the mux hop + // does not depend on one (it precedes the NodeId resolution). var mux = Sys.ActorOf(DependencyMuxActor.Props()); var subscriber = CreateTestProbe(); mux.Tell(new DependencyMuxActor.RegisterInterest(new[] { "ref-1" }, subscriber.Ref)); @@ -142,13 +143,44 @@ public sealed class DependencyMuxActorTests : RuntimeActorTestBase ackRouter: hostProbe.Ref, dependencyMux: mux)); - // Tell the host an AttributeValuePublished — it should fan out to the mux + subscriber. - hostActor.Tell(new DriverInstanceActor.AttributeValuePublished( - "driver-1", "ref-1", 42, OpcUaStatus.Good, DateTime.UtcNow)); + hostActor.Tell(new DriverInstanceActor.AttributeValueBatchPublished("driver-1", new[] + { + new DriverInstanceActor.AttributeValueItem("ref-1", 42, OpcUaStatus.Good, DateTime.UtcNow), + })); subscriber.ExpectMsg().TagId.ShouldBe("ref-1"); } + /// A batched driver burst (the primary ingress from DriverHostActor since the value-pipeline + /// batching work) is filtered per item: a subscriber hears exactly one DependencyValueChanged for the + /// ref it registered, carrying that item's own value/timestamp/quality, and nothing for the rest of the + /// burst. + [Fact] + public void AttributeValueBatchPublished_is_filtered_per_item_to_interested_subscribers() + { + var mux = Sys.ActorOf(DependencyMuxActor.Props()); + var sub = CreateTestProbe(); + mux.Tell(new DependencyMuxActor.RegisterInterest(new[] { "tag-x" }, sub.Ref)); + + var tsX = new DateTime(2026, 8, 15, 9, 0, 0, DateTimeKind.Utc); + var tsY = new DateTime(2026, 8, 15, 9, 0, 1, DateTimeKind.Utc); + mux.Tell(new DriverInstanceActor.AttributeValueBatchPublished("driver-1", new[] + { + new DriverInstanceActor.AttributeValueItem("tag-y", 99, OpcUaStatus.Good, tsY), + new DriverInstanceActor.AttributeValueItem("tag-x", 10, OpcUaStatus.BadNoCommunication, tsX), + })); + + var dep = sub.ExpectMsg(PresenceBudget); + dep.TagId.ShouldBe("tag-x"); + dep.Value.ShouldBe(10); + dep.TimestampUtc.ShouldBe(tsX); + // #519: the item's own status sub-code rides through, not a Good/Bad bucket. + dep.StatusCode.ShouldBe(OpcUaStatus.BadNoCommunication); + + // Nothing for tag-y — no subscriber registered for it. + sub.ExpectNoMsg(TimeSpan.FromMilliseconds(200)); + } + /// #478 — the mux forwards the published source quality onto DependencyValueChanged so the /// scripted-alarm host can derive a condition's worst-of-input quality. [Fact]