From 20a816dbf464fda7ced569f7232f03f394915f01 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Sun, 9 Aug 2026 20:11:55 -0400 Subject: [PATCH] feat(galaxy): consume the gateway's correlated write statuses; close archreview 06/S-1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The mxaccessgw OnWriteComplete correlation is live (server b948e69, worker 53f69cd — all four unary write kinds), so every write reply's statuses[0] now carries the real COM commit outcome. Live-gating the cutover on windev found and fixed three defects: - StatusCodeMap.FromMxStatus truncated the correlated failure row's MxStatus detail code to a byte (1007 & 0xFF = 239 -> OPC DA Good band), mapping a REFUSED write to Good. New invariant: a row that is not IsSuccess() never maps Good — details > 0xFF (and contradictory Good-band substatuses) route by category (SECURITY_ERROR -> BadUserAccessDenied, named const covered by StatusCodeParityTests; COMMUNICATION/CONFIGURATION keep their codes). - The borrow-from-subscription-registry write optimization could never commit: a subscription handle is plain-advised, and MXAccess refuses both the no-login write through it (SECURITY_ERROR detail 1008, live-captured) and the supervisory advise upgrade (E_INVALIDARG) — one advise per HANDLE, not per attribute. Removed: writer subscribedHandleSource ctor param + borrow branch, GalaxyDriver wiring, SubscriptionRegistry.TryResolveItemHandle + its forward index and test file. The writer always AddItems its own supervisory-advised handle, which commits fine while the same attribute is subscribed (live-proven; pinned by the new write-while-subscribed smoke). - (gateway-side, fixed there) the correlation initially covered WriteSecured only, while the dominant no-login path emits plain Write. Live gate, all legs PASSED against windev: happy write Good off a confirmed status row with galaxy.writes.unconfirmed + advise_failed both FLAT; read-only refusal surfaces Bad-family (the #5 node-revert input Galaxy could never produce before); WriteCompletionWaitMilliseconds=0 control -> provisional Good + metered unconfirmed (override removed after, restoration re-verified). New env-gated smokes: Live_correlated_write_statuses_confirm_commit_and_surface_refusal, Live_write_to_subscribed_tag_commits_via_own_handle (replaces the borrow smoke), GatewayGalaxyWriteMeterLiveTests (run filtered — meters are process-wide). --- deferment.md | 2 +- docs/Redundancy.md | 4 +- .../GalaxyDriver.cs | 10 +- .../Runtime/GatewayGalaxyDataWriter.cs | 77 +++------ .../Runtime/StatusCodeMap.cs | 42 ++++- .../Runtime/SubscriptionRegistry.cs | 46 ------ .../Runtime/GatewayGalaxyDataWriterTests.cs | 98 +---------- .../GatewayGalaxyLiveReopenAndWriteTests.cs | 97 +++++++---- .../GatewayGalaxyWriteMeterLiveTests.cs | 133 +++++++++++++++ .../Runtime/StatusCodeMapTests.cs | 56 +++++++ .../SubscriptionRegistryHandleResolveTests.cs | 156 ------------------ 11 files changed, 337 insertions(+), 384 deletions(-) create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/GatewayGalaxyWriteMeterLiveTests.cs delete mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/SubscriptionRegistryHandleResolveTests.cs diff --git a/deferment.md b/deferment.md index bbe3711e..f4b3792a 100644 --- a/deferment.md +++ b/deferment.md @@ -56,7 +56,7 @@ So everything below is **untracked work**. Nothing in it is a data-loss or outag | **4 live gates blocked on hardware/licence** — ~~archreview R2-03 (VT Good→Bad on a data-fed rig)~~ ✅ *PASSED 2026-08-07*, ~~R2-10 (force breaker-OPEN)~~ ✅ *PASSED 2026-08-08 by fixing the structural gap (`DriverCapability.Initialize`)*, Galaxy `ScanStateProbeParityTests` (licence-gated), v2 GA exit criteria (~~FOCAS live-CNC smoke #54~~ *closed — live-validated on a real 31i-B 2026-06-25, see `docs/plans/2026-06-25-focas-pdu-v3-30i-b-support.md`*, OPC UA CTT pass, Ignition 8.3 cutover) | Cannot be closed from this box. Not decisions — inputs. *(The two archreview gates turned out to be closable from this box after all — §5.)* | §5 | | ~~**1 deferred live-verify task** — `2026-06-14-driver-reconfigure-while-faulted` Task 3~~ ✅ **PASSED 2026-08-07.** A faulted driver ("initialize failed: Connection refused") adopted a corrected config and connected while faulted, with the node's `startedAt` byte-identical before and after and `RestartCount=0` — no restart. | §5 | | **G-7 residual** — Sql / MTConnect / Calculation / OpcUaClient have no Contracts-side `Inspect` | **The deploy gate is weaker than the authoring gate.** `TagConfigValidator` covers 11 of 12, so config arriving by **CSV import or API** — the paths that skip the UI — meets only the weaker gate. | §1.2 | -| ~~**3 carried-open archreview findings** — the failure-visibility trio~~ **Update 2026-08-07 (truth sweep): mostly CLOSED.** 01/S-1 **FIXED** — `AddressSpaceApplier.cs:95-97` tallies swallowed removal-pass failures into `AddressSpaceApplyOutcome.FailedNodes` (comment cites 01/S-1). 03/S4 **FIXED** — `PrimaryGatePolicy.cs` default-denies on unknown role when a driver peer exists (doc-comment "Fixes archreview 03/S4"), live-proven R2-04 T15. 06/S-1 **PARTIAL** — fail-closed advise path + `galaxy.writes.unconfirmed` meter shipped in `GatewayGalaxyDataWriter.cs`; residual = gateway-side WriteComplete correlation (cross-repo, mxaccessgw). | Residual is only the 06/S-1 cross-repo half. | §6.3 | +| ~~**3 carried-open archreview findings** — the failure-visibility trio~~ **Update 2026-08-07 (truth sweep): mostly CLOSED.** 01/S-1 **FIXED** — `AddressSpaceApplier.cs:95-97` tallies swallowed removal-pass failures into `AddressSpaceApplyOutcome.FailedNodes` (comment cites 01/S-1). 03/S4 **FIXED** — `PrimaryGatePolicy.cs` default-denies on unknown role when a driver peer exists (doc-comment "Fixes archreview 03/S4"), live-proven R2-04 T15. 06/S-1 **PARTIAL** — fail-closed advise path + `galaxy.writes.unconfirmed` meter shipped in `GatewayGalaxyDataWriter.cs`; residual = gateway-side WriteComplete correlation (cross-repo, mxaccessgw). **Update 2026-08-09: the gateway half SHIPPED** — mxaccessgw main `b948e69` correlates the worker's `OnWriteComplete` onto the unary `WriteSecured` reply (`statuses[0]`; bounded wait default **1.5 s** — deliberately under OtOpcUa's 2 s Tier A Write budget, configurable via `MxGateway:Worker:WriteCompletionWaitMilliseconds`; timeout falls back to today's empty-statuses shape, still metered unconfirmed). **Update 2026-08-09 (later): 06/S-1 CLOSED — live-gated end to end, and the gate found + fixed three defects.** The user-authorized redeploy exposed (1) the correlation covered `WriteSecured` only while OtOpcUa's dominant no-login path emits plain `Write` — fixed gateway-side (worker `53f69cd`, all four unary write kinds, redeployed both hosts); (2) OtOpcUa's `StatusCodeMap.FromMxStatus` truncated the correlated failure row's MxStatus detail code to a byte (`1007 & 0xFF = 239` → OPC DA Good band), mapping a REFUSED write to Good — fixed with the not-IsSuccess-never-Good invariant + category routing (`SECURITY_ERROR → BadUserAccessDenied`); (3) the writer's borrow-from-subscription-registry optimization could never commit (a subscription handle is plain-advised; MXAccess refuses the no-login write `SECURITY_ERROR 1008` AND the advise upgrade `E_INVALIDARG` — one advise per HANDLE, not per attribute) — borrow path REMOVED, writer always owns a supervisory-advised handle. Live legs all PASSED on windev: happy write Good off confirmed `statuses[0]` + both meters flat; read-only refusal surfaces Bad-family + node-revert input; `WriteCompletionWaitMilliseconds=0` control → provisional Good + metered unconfirmed (override removed after). Pinned by `Live_correlated_write_statuses_confirm_commit_and_surface_refusal`, `Live_write_to_subscribed_tag_commits_via_own_handle`, `GatewayGalaxyWriteMeterLiveTests`, and the new `StatusCodeMapTests` rows. | ~~Residual is only the 06/S-1 live leg~~ **CLOSED 2026-08-09** — nothing remains. | §6.3 | | ~~**1 genuinely unexecuted plan** — `2026-05-29-adminui-followups`, **19 pending / 0 completed**~~ ✅ **CLOSED 2026-08-07 by the overlap audit it asked for — it was bookkeeping drift, not backlog.** All 19 tasks are shipped, superseded by v3, or moot; **no code was needed.** The warning was right and then some: WS1's whole *premise* died with v3 (driver-embedded tag lists in `DriverConfig` are no longer the runtime source of truth — the artifact populates `RawTags`), and its capability was delivered in the v3 shape (`TagConfigEditorMap` + `DeviceFormMap`, both verified **reachable**, not merely present). WS2/WS3 shipped; WS4's target files were deleted by v3 Batch 2 + #542. Per-task evidence in the `.tasks.json`. | §6.2 | | **11 other open plan tasks** — ~~5 akka-hosting partials (likely v3-superseded)~~ *(verified superseded 2026-08-07 — F8/F9/F10/F13/F14 marked completed; `Phase7Composer`/`ScriptedAlarmActor` deleted, evaluators/SDK sinks/security policies all production-wired)*, 4+1 followups-batch (open *by choice*), fixedtree Task 11, 1 manual smoke checklist | Small, and two of the groups are decisions already taken. | §6.2 | | **Review-coverage gap** — 10 shipped projects never reviewed (Calculation, Historian.Gateway, Mqtt +Browser/Contracts, MTConnect +Contracts, Sql +Browser/Contracts) while **3 modules still review the retired Wonderware backend** | Re-verified 2026-08-04 against `code-reviews/`. Unchanged since the audit. | §6.3 | diff --git a/docs/Redundancy.md b/docs/Redundancy.md index 0312c30e..650ef264 100644 --- a/docs/Redundancy.md +++ b/docs/Redundancy.md @@ -643,11 +643,11 @@ The OtOpcUa Client CLI at `src/Client/ZB.MOM.WW.OtOpcUa.Client.CLI` supports `-F | `otopcua.redundancy.primary_gate_denied` | Counter (`{denial}`) | Operations denied by the Primary data-plane gate (archreview 03/S4). Tagged `site` = `write` \| `ack` \| `alarm-emit` and `reason` = `secondary` \| `detached` \| `role-unknown`. A non-zero `role-unknown` rate on a healthy multi-node cluster flags a stuck boot window (redundancy snapshot not delivering). | | `otopcua.opcua.apply.failed` | Counter (`{apply}`) | Address-space apply/materialise passes that swallowed at least one sink failure (archreview 01/S-1). Tagged `kind` = `rebuild` (the rebuild threw) \| `nodes` (per-node materialise failures). A non-zero rate means the running server holds a stale or partial address space despite a reported-successful deploy — `OpcUaPublishActor` also logs the degraded apply at **Error**. | | `galaxy.writes.advise_failed` | Counter (`{write}`) | Galaxy writes short-circuited to `Bad` because `AdviseSupervisory` failed — the value could not have committed, so the write **fails closed** (archreview 06/S-1) and the Bad status fires the #5 node revert instead of leaving a phantom-Good node. On the Galaxy driver meter `ZB.MOM.WW.OtOpcUa.Driver.Galaxy`. | -| `galaxy.writes.unconfirmed` | Counter (`{write}`) | Galaxy writes reported `Good` off an **empty** gateway statuses array (command accepted; the COM-side commit is unconfirmed pending the mxaccessgw `WriteComplete` correlation follow-up). The honest signal for the optimistic-Good rate until that cross-repo fix lands. On the Galaxy driver meter. | +| `galaxy.writes.unconfirmed` | Counter (`{write}`) | Galaxy writes reported `Good` off an **empty** gateway statuses array (worker-side acceptance only — the COM commit is unconfirmed). Against a correlated gateway (mxaccessgw server ≥ `b948e69`, worker ≥ `53f69cd` — every unary write reply carries the worker's `OnWriteComplete` outcome as `statuses[0]`) this stays **flat**; an increment means the bounded completion wait (`MxGateway:Worker:WriteCompletionWaitMilliseconds`, default 1500 ms, deliberately under the 2 s Tier A Write timeout) timed out, or the gateway pre-dates the correlation. On the Galaxy driver meter. | The redundancy/apply meters are defined on `OtOpcUaTelemetry` (`src/Core/ZB.MOM.WW.OtOpcUa.Commons/Observability/OtOpcUaTelemetry.cs`); the Galaxy write meters hang off the driver's own meter (`EventPump.MeterName`). All surface through whatever OpenTelemetry exporter the host configures. -**Galaxy fail-closed write semantics (archreview 06/S-1):** a non-secured Galaxy write commits only when its item is `AdviseSupervisory`-advised first (the writer runs with no login). If the advise fails, the write is **not** issued — `GatewayGalaxyDataWriter` returns `BadCommunicationError` (a knowingly-lost write is never reported as Good). An empty statuses array on the write reply is still treated as `Good` but metered as `galaxy.writes.unconfirmed`. See [Historian.md](Historian.md) / the Galaxy driver notes for the write path. +**Galaxy fail-closed write semantics (archreview 06/S-1, closed 2026-08-09):** a non-secured Galaxy write commits only when its item is `AdviseSupervisory`-advised first (the writer runs with no login) — live-confirmed: a write through a merely plain-advised handle is refused by the automation object (`SECURITY_ERROR` detail 1008 on the correlated reply). If the advise fails, the write is **not** issued — `GatewayGalaxyDataWriter` returns `BadCommunicationError` (a knowingly-lost write is never reported as Good). With the gateway-side `OnWriteComplete` correlation live, the write reply's `statuses[0]` carries the real COM commit outcome, so a refused write surfaces as a Bad-family code and fires the #5 node revert — the writer always AddItems its **own** supervisory-advised handle (the former borrow-from-subscription-registry optimization was removed: MXAccess enforces one advise per handle, so a subscription's plain-advised handle can neither commit a no-login write nor be upgraded). An empty statuses array (completion-wait timeout / pre-correlation gateway) is still treated as `Good` but metered as `galaxy.writes.unconfirmed`. See [Historian.md](Historian.md) / the Galaxy driver notes for the write path. ## Depth reference diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/GalaxyDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/GalaxyDriver.cs index 153160c6..b3d8d537 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/GalaxyDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/GalaxyDriver.cs @@ -378,11 +378,13 @@ public sealed class GalaxyDriver _subscriber = new TracedGalaxySubscriber( new GatewayGalaxySubscriber(_ownedMxSession, _logger), _options.MxAccess.ClientName); _dataWriter = new TracedGalaxyDataWriter( - // Let the writer borrow live MXAccess item handles the subscription registry already - // holds, so the first write to an already-subscribed tag skips a redundant AddItem. + // The writer always AddItems its own handle — a subscription's handle is plain-advised, + // and MXAccess both refuses a no-login write through it (SECURITY_ERROR 1008) and refuses + // upgrading its advise mode (E_INVALIDARG), so the former borrow-from-registry + // optimization could never commit a write. The advise limit is per handle, not per + // attribute: the writer's own supervisory-advised handle coexists with the subscription's. new GatewayGalaxyDataWriter( - _ownedMxSession, _options.MxAccess.WriteUserId, _logger, - _subscriptions.TryResolveItemHandle), + _ownedMxSession, _options.MxAccess.WriteUserId, _logger), _options.MxAccess.ClientName); _supervisor = new ReconnectSupervisor( diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/Runtime/GatewayGalaxyDataWriter.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/Runtime/GatewayGalaxyDataWriter.cs index 282803ec..9a25a321 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/Runtime/GatewayGalaxyDataWriter.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/Runtime/GatewayGalaxyDataWriter.cs @@ -18,19 +18,19 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Runtime; /// /// Item handle cache survives across writes — repeated writes to the same tag avoid /// re-AddItem. Per-tag failures are isolated: one bad write doesn't fail the batch. -/// When a subscribedHandleSource delegate is supplied, the first write to an -/// already-subscribed tag borrows the live handle from the subscription registry and -/// skips the AddItem round-trip entirely. Borrowed handles are intentionally NOT stored -/// in _itemHandles — the subscription registry owns their lifecycle (including -/// reconnect rebind), so each write re-borrows the fresh handle and no stale-cache -/// window is introduced. +/// The writer always owns its handles: it never borrows a subscription's live handle, +/// because a subscription handle is plain-advised and MXAccess enforces one advise per +/// handle — a no-login write through a plain-advised handle is refused (SECURITY_ERROR +/// detail 1008) and re-advising it supervisory is rejected (E_INVALIDARG), both +/// live-captured on the correlated write replies (06/S-1). The advise limit is per +/// handle, not per attribute, so the writer's own supervisory-advised handle coexists +/// with the subscription's plain-advised one on the same attribute. /// public sealed class GatewayGalaxyDataWriter : IGalaxyDataWriter { private readonly GalaxyMxSession _session; private readonly int _writeUserId; private readonly ILogger _logger; - private readonly Func? _subscribedHandleSource; private readonly ConcurrentDictionary _itemHandles = new(StringComparer.OrdinalIgnoreCase); // Item handles we've already AdviseSupervisory'd this session — supervisory advise is @@ -41,7 +41,9 @@ public sealed class GatewayGalaxyDataWriter : IGalaxyDataWriter // Write failure-visibility meters (archreview 06/S-1). Reuse the pump's meter name so the single host // listener catches both. advise_failed = writes short-circuited to Bad because AdviseSupervisory failed // (the value could not have committed); unconfirmed = writes reported Good off an EMPTY gateway statuses - // array (command accepted, COM commit unconfirmed pending the gateway WriteComplete correlation follow-up). + // array. A correlated gateway (mxaccessgw ≥ 53f69cd rolls the worker's OnWriteComplete into every unary + // write reply's statuses[0]) keeps this counter FLAT — an increment means the gateway is pre-correlation + // or the completion wait timed out (WriteCompletionWaitMilliseconds), so the commit is unconfirmed. private static readonly Meter WriterMeter = new(EventPump.MeterName); private static readonly Counter WriteAdviseFailed = WriterMeter.CreateCounter("galaxy.writes.advise_failed", unit: "{write}", @@ -54,21 +56,11 @@ public sealed class GatewayGalaxyDataWriter : IGalaxyDataWriter /// The MXAccess gateway session. /// The user ID for write operations. /// Optional logger for tracing. - /// - /// Optional delegate that resolves a live MXAccess item handle from the subscription - /// registry for a given tag full reference. When the delegate returns a positive handle - /// the writer borrows it and skips the AddItem gateway round-trip. A return value of - /// null, 0, or negative is treated as "not available" and the writer falls back - /// to its own AddItem. Borrowed handles are NOT cached in this writer — the subscription - /// registry owns their lifecycle. - /// - public GatewayGalaxyDataWriter(GalaxyMxSession session, int writeUserId, ILogger? logger = null, - Func? subscribedHandleSource = null) + public GatewayGalaxyDataWriter(GalaxyMxSession session, int writeUserId, ILogger? logger = null) { _session = session ?? throw new ArgumentNullException(nameof(session)); _writeUserId = writeUserId; _logger = logger ?? NullLogger.Instance; - _subscribedHandleSource = subscribedHandleSource; } /// @@ -94,7 +86,7 @@ public sealed class GatewayGalaxyDataWriter : IGalaxyDataWriter internal int CachedSupervisedHandleCount => _supervisedHandles.Count; /// Count of real gateway AddItem round-trips this writer has issued. Stays zero - /// when every handle was served from cache or borrowed from the subscription registry. Test seam. + /// when every handle was served from the cache. Test seam. internal int AddItemCallCount => Volatile.Read(ref _addItemCallCount); /// @@ -111,23 +103,6 @@ public sealed class GatewayGalaxyDataWriter : IGalaxyDataWriter if (supervised) _supervisedHandles.TryAdd(itemHandle, 0); } - /// - /// Resolve an item handle WITHOUT touching the gateway: a handle this writer already AddItem'd - /// (_itemHandles), else a live subscription handle borrowed via - /// _subscribedHandleSource. Returns null when neither is available (the caller must - /// AddItem). A BORROWED handle is intentionally NOT stored in _itemHandles — the - /// subscription registry owns the borrowed handle's lifecycle (including reconnect rebind), - /// so the next write re-borrows the fresh handle and no stale-cache window is introduced. - /// - /// The dotted tag full reference. - /// A usable item handle, or null when an AddItem is required. - internal int? TryResolveCachedOrBorrowed(string fullRef) - { - if (_itemHandles.TryGetValue(fullRef, out var existing)) return existing; - if (_subscribedHandleSource?.Invoke(fullRef) is int borrowed && borrowed > 0) return borrowed; - return null; - } - /// public async Task> WriteAsync( IReadOnlyList writes, @@ -190,13 +165,12 @@ public sealed class GatewayGalaxyDataWriter : IGalaxyDataWriter { // A raw Write runs with NO user login (WriteUserId is typically 0), so MXAccess // only COMMITS the value when the item is advised in SUPERVISORY mode. Without it - // the gateway's Write call doesn't throw (reply looks OK) but the value never - // reaches the galaxy — a knowingly-lost write. FAIL-CLOSED (archreview 06/S-1): when + // the write is refused by the automation object (SECURITY_ERROR detail 1008 on the + // correlated reply — live-captured). FAIL-CLOSED (archreview 06/S-1): when // AdviseSupervisory fails we do NOT issue the raw Write; we return Bad so the // #5 write-outcome self-correction reverts the phantom-Good node instead of leaving it. - // The real fix is a gateway-side WriteComplete correlation (mxaccessgw backlog) so the - // unary reply's Statuses carry the actual COM commit outcome; until then advise-status + - // the galaxy.writes.* meters are the honest signal. + // A correlated gateway (mxaccessgw ≥ 53f69cd) would also surface the refusal in the + // reply's statuses[0], but the short-circuit avoids a write we know cannot commit. if (!await EnsureSupervisoryAdvisedAsync(ops, itemHandle, ct).ConfigureAwait(false)) { WriteAdviseFailed.Add(1); @@ -229,7 +203,7 @@ public sealed class GatewayGalaxyDataWriter : IGalaxyDataWriter private async Task EnsureItemHandleAsync( IMxWriteOps ops, string fullRef, CancellationToken ct) { - if (TryResolveCachedOrBorrowed(fullRef) is int resolved) return resolved; + if (_itemHandles.TryGetValue(fullRef, out var existing)) return existing; var handle = await ops.AddItemAsync(fullRef, ct).ConfigureAwait(false); Interlocked.Increment(ref _addItemCallCount); _itemHandles[fullRef] = handle; @@ -323,12 +297,13 @@ public sealed class GatewayGalaxyDataWriter : IGalaxyDataWriter /// /// Translate a gateway into an OPC UA /// . Honours the protocol-level Status field first - /// (transport / dispatch failures), then the first MXAccess status row. - /// An EMPTY statuses array is treated as Good but is PROVISIONAL: the reply proves - /// worker-side command acceptance, not the COM-side commit (the gateway does not yet correlate a - /// WriteComplete row onto the unary reply — mxaccessgw backlog). Each such reply increments - /// galaxy.writes.unconfirmed so the unconfirmed-write rate is operator-visible today - /// (archreview 06/S-1). + /// (transport / dispatch failures), then the first MXAccess status row — which, against a + /// correlated gateway (mxaccessgw ≥ 53f69cd), is the worker's OnWriteComplete outcome: + /// the actual COM-side commit verdict, including refusals (archreview 06/S-1, closed). + /// An EMPTY statuses array is treated as Good but is PROVISIONAL: it means the + /// gateway's bounded completion wait timed out (or the gateway pre-dates the correlation), so + /// the reply proves worker-side command acceptance only. Each such reply increments + /// galaxy.writes.unconfirmed so the unconfirmed-write rate stays operator-visible. /// private WriteResult TranslateReply(MxCommandReply reply, string fullRef) { @@ -350,8 +325,8 @@ public sealed class GatewayGalaxyDataWriter : IGalaxyDataWriter return new WriteResult(StatusCodeMap.FromMxStatus(status, _logger)); } - // Empty statuses ⇒ provisional Good. Meter it so the unconfirmed-write rate is visible until the - // gateway-side WriteComplete correlation lands (archreview 06/S-1 cross-repo follow-up). + // Empty statuses ⇒ the gateway's completion wait timed out (or the gateway pre-dates the + // 53f69cd correlation) ⇒ provisional Good. Meter it so the unconfirmed-write rate is visible. WriteUnconfirmed.Add(1); return new WriteResult(StatusCodeMap.Good); } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/Runtime/StatusCodeMap.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/Runtime/StatusCodeMap.cs index 67b7f4f5..d5dcb754 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/Runtime/StatusCodeMap.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/Runtime/StatusCodeMap.cs @@ -52,6 +52,7 @@ internal static class StatusCodeMap public const uint BadOutOfService = 0x808D0000u; public const uint BadWaitingForInitialData = 0x80320000u; public const uint BadInternalError = 0x80020000u; + public const uint BadUserAccessDenied = 0x801F0000u; /// /// Fills a still-pending read when the caller's token fires before the gateway answers. Named here @@ -111,10 +112,21 @@ internal static class StatusCodeMap if (status is null) return Good; if (status.IsSuccess()) return Good; - // Detail field carries the substatus when the worker translated MX-style codes; - // when zero, infer from detected_by. - var detail = (byte)(status.Detail & 0xFF); - if (detail != 0) return FromQualityByte(detail, logger); + // A detail in [1, 255] is an OPC DA quality substatus (read/data-change rows). Anything + // larger is an MXAccess MxStatus detail code — the write-completion rows the gateway + // correlates onto write replies carry these (e.g. 1007 operational, 1008 security) — and + // truncating one into the quality table maps a REFUSED WRITE to Good whenever the low byte + // lands in the Good band (1007 → 239 ≥ 192). Route those on the status category instead. + var detail = status.Detail; + if (detail is > 0 and <= 0xFF) + { + var mapped = FromQualityByte((byte)detail, logger); + // Invariant: a row that is not IsSuccess() must never surface as Good — a + // contradictory Good-band substatus on a failure row falls back to the category. + if ((mapped >> 30) != 0u) return mapped; + return FromCategory(status, logger); + } + if (detail > 0xFF) return FromCategory(status, logger); // detected_by != Mxaccess (raw_detected_by != the MXAccess source enum) implies // the failure happened pre-call (gateway, worker, transport) — surface as a @@ -124,6 +136,28 @@ internal static class StatusCodeMap return Bad; } + /// + /// Map a failure row by its — used when the detail field is + /// an MxStatus detail code rather than an OPC DA quality byte (or contradicts the failure). + /// Category-level only: the MxStatus detail vocabulary (1007, 1008, …) is not mapped + /// per-code yet, so the log line carries the raw detail for field capture. + /// + private static uint FromCategory(MxStatusProxy status, ILogger? logger) + { + var code = status.Category switch + { + MxStatusCategory.SecurityError => BadUserAccessDenied, + MxStatusCategory.CommunicationError => BadCommunicationError, + MxStatusCategory.ConfigurationError => BadConfigurationError, + _ => Bad, + }; + logger?.LogWarning( + "MxStatus failure row category {Category} detail {Detail} mapped to 0x{Code:X8} at category " + + "level. Field capture welcome — extend StatusCodeMap.FromCategory with the detail code.", + status.Category, status.Detail, code); + return code; + } + /// /// Convert an OPC UA status-code uint back to the OPC DA quality category /// byte — Good=192, Uncertain=64, Bad=0 — by extracting the top-two bits of the diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/Runtime/SubscriptionRegistry.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/Runtime/SubscriptionRegistry.cs index b2fd411a..512099f5 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/Runtime/SubscriptionRegistry.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/Runtime/SubscriptionRegistry.cs @@ -20,12 +20,6 @@ internal sealed class SubscriptionRegistry { private readonly ConcurrentDictionary _bySubscriptionId = new(); private readonly ConcurrentDictionary> _subscribersByItemHandle = new(); - // Forward index for the Galaxy writer: fullRef (case-insensitive) → live item handle. - // Maintained in lock-step with _subscribersByItemHandle; entries are cleaned up when - // the last subscriber for a handle is removed, and TryResolveItemHandle guards against - // stale entries by cross-checking _subscribersByItemHandle at read time. - private readonly ConcurrentDictionary _itemHandleByFullRef = - new(StringComparer.OrdinalIgnoreCase); private long _nextSubscriptionId; /// Gets the number of tracked subscriptions. @@ -55,7 +49,6 @@ internal sealed class SubscriptionRegistry binding.ItemHandle, _ => [subscriptionId], (_, set) => set.Add(subscriptionId)); - _itemHandleByFullRef[binding.FullReference] = binding.ItemHandle; } } @@ -77,10 +70,6 @@ internal sealed class SubscriptionRegistry if (remaining.IsEmpty) { _subscribersByItemHandle.TryRemove(binding.ItemHandle, out _); - // Clean up the forward index only when this handle is still the current - // mapping — a concurrent re-add for the same ref must not be clobbered. - if (_itemHandleByFullRef.TryGetValue(binding.FullReference, out var fwd) && fwd == binding.ItemHandle) - _itemHandleByFullRef.TryRemove(binding.FullReference, out _); } else _subscribersByItemHandle[binding.ItemHandle] = remaining; } @@ -110,36 +99,6 @@ internal sealed class SubscriptionRegistry return result; } - /// - /// Resolve the live MXAccess item handle a current subscription holds for , - /// or null when no live subscription covers it. The Galaxy writer borrows this handle to skip a - /// redundant AddItem round-trip on the first write to an already-subscribed tag. - /// - /// - /// The _itemHandleByFullRef forward map is only a hint — resolution is authoritative: - /// it confirms a live subscription actually binds this to - /// the candidate handle (via the per-entry reverse index), so a stale forward entry can never - /// hand the writer a dead handle, nor a handle that now belongs to a different tag. This keeps - /// a tag resolvable while ANY subscription still binds it (the legitimate - /// multiple-driver-subscriptions-per-tag case) yet returns null the moment the last one drops. - /// - /// The dotted tag full reference (e.g. TestMachine_002.TestFloat). - /// The live item handle, or null when none is currently subscribed. - public int? TryResolveItemHandle(string fullRef) - { - if (fullRef is null) return null; - if (!_itemHandleByFullRef.TryGetValue(fullRef, out var handle)) return null; - if (!_subscribersByItemHandle.TryGetValue(handle, out var subs)) return null; // dead handle - - // Confirm a live subscription genuinely binds fullRef -> handle (not a stale forward entry - // pointing at a handle the gateway has since associated with another tag). - foreach (var subId in subs) - if (_bySubscriptionId.TryGetValue(subId, out var entry) - && entry.FullRefByItemHandle.TryGetValue(handle, out var liveRef) - && string.Equals(liveRef, fullRef, StringComparison.OrdinalIgnoreCase)) - return handle; - return null; - } /// Snapshot every active binding for diagnostic output. /// A flattened list of every tag binding across all tracked subscriptions. @@ -177,10 +136,6 @@ internal sealed class SubscriptionRegistry if (remaining.IsEmpty) { _subscribersByItemHandle.TryRemove(binding.ItemHandle, out _); - // Clean up the forward index only when this handle is still the current - // mapping — the add loop below will overwrite it with the fresh handle. - if (_itemHandleByFullRef.TryGetValue(binding.FullReference, out var fwd) && fwd == binding.ItemHandle) - _itemHandleByFullRef.TryRemove(binding.FullReference, out _); } else _subscribersByItemHandle[binding.ItemHandle] = remaining; } @@ -193,7 +148,6 @@ internal sealed class SubscriptionRegistry binding.ItemHandle, _ => [subscriptionId], (_, set) => set.Add(subscriptionId)); - _itemHandleByFullRef[binding.FullReference] = binding.ItemHandle; } } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/GatewayGalaxyDataWriterTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/GatewayGalaxyDataWriterTests.cs index 7c3f00fe..107a0b31 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/GatewayGalaxyDataWriterTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/GatewayGalaxyDataWriterTests.cs @@ -89,96 +89,10 @@ public sealed class GatewayGalaxyDataWriterTests writer.CachedSupervisedHandleCount.ShouldBe(0); } - // ===== subscribedHandleSource / TryResolveCachedOrBorrowed tests ===== - - /// - /// When a subscribedHandleSource delegate returns a positive handle for the - /// requested ref, must - /// return that handle. Critically, the borrowed handle must NOT be stored in the writer's - /// own item-handle cache (the subscription registry owns its lifecycle), and - /// must remain zero (no real - /// gateway round-trip issued). - /// - [Fact] - public void TryResolveCachedOrBorrowed_returns_borrowed_handle_without_caching_it() - { - var writer = new GatewayGalaxyDataWriter( - MinimalSession(), writeUserId: 0, logger: null, - subscribedHandleSource: fr => fr == "Tag.X" ? 7 : (int?)null); - - var result = writer.TryResolveCachedOrBorrowed("Tag.X"); - - result.ShouldBe(7); - writer.CachedItemHandleCount.ShouldBe(0); // borrow NOT stored in _itemHandles - writer.AddItemCallCount.ShouldBe(0); // no real AddItem issued - } - - /// - /// When a handle is present in the writer's own _itemHandles cache (seeded via - /// ), that cached handle must - /// win over the subscribedHandleSource delegate — the writer's own earlier AddItem - /// takes priority. - /// - [Fact] - public void TryResolveCachedOrBorrowed_cached_handle_wins_over_source() - { - // Source would return 99 for "Tag.Y", but the writer already cached handle 42 for it. - var writer = new GatewayGalaxyDataWriter( - MinimalSession(), writeUserId: 0, logger: null, - subscribedHandleSource: fr => fr == "Tag.Y" ? 99 : (int?)null); - - writer.SeedHandleCachesForTest("Tag.Y", itemHandle: 42, supervised: false); - - var result = writer.TryResolveCachedOrBorrowed("Tag.Y"); - - result.ShouldBe(42); // cached wins, not 99 from the source - } - - /// - /// When no subscribedHandleSource is provided (null), the method must - /// return null for a ref that has never been cached — preserving today's - /// AddItem-required behavior. - /// - [Fact] - public void TryResolveCachedOrBorrowed_null_source_returns_null_when_not_cached() - { - var writer = new GatewayGalaxyDataWriter(MinimalSession(), writeUserId: 0); - - var result = writer.TryResolveCachedOrBorrowed("Tag.Z"); - - result.ShouldBeNull(); - } - - /// - /// A subscribedHandleSource that returns 0 (a sentinel meaning "subscribe - /// pending / failed") must be treated as no borrow — the method returns null - /// so the caller issues a fresh AddItem. - /// - [Fact] - public void TryResolveCachedOrBorrowed_source_returning_zero_treated_as_no_borrow() - { - var writer = new GatewayGalaxyDataWriter( - MinimalSession(), writeUserId: 0, logger: null, - subscribedHandleSource: _ => 0); - - var result = writer.TryResolveCachedOrBorrowed("Tag.W"); - - result.ShouldBeNull(); - } - - /// - /// A subscribedHandleSource that returns a negative value (another failure - /// sentinel) must also be treated as no borrow — the method returns null. - /// - [Fact] - public void TryResolveCachedOrBorrowed_source_returning_negative_treated_as_no_borrow() - { - var writer = new GatewayGalaxyDataWriter( - MinimalSession(), writeUserId: 0, logger: null, - subscribedHandleSource: _ => -5); - - var result = writer.TryResolveCachedOrBorrowed("Tag.V"); - - result.ShouldBeNull(); - } + // The former subscribedHandleSource / TryResolveCachedOrBorrowed tests are gone with the + // borrow-for-write optimization itself: a subscription's handle is plain-advised, and MXAccess + // both refuses a no-login write through it (SECURITY_ERROR detail 1008) and refuses upgrading + // its advise mode (E_INVALIDARG) — live-captured on the correlated write replies (06/S-1). The + // writer now always AddItems its own supervisory-advised handle, which MXAccess accepts even + // while the same attribute is subscribed (the advise limit is per handle, not per attribute). } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/GatewayGalaxyLiveReopenAndWriteTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/GatewayGalaxyLiveReopenAndWriteTests.cs index 95a2e68c..5027c0f7 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/GatewayGalaxyLiveReopenAndWriteTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/GatewayGalaxyLiveReopenAndWriteTests.cs @@ -125,52 +125,43 @@ public sealed class GatewayGalaxyLiveReopenAndWriteTests } /// - /// Borrow — the writer reuses a live MXAccess item handle the subscription registry - /// already holds, so the first write to an already-subscribed tag skips a redundant AddItem. - /// Proves the load-bearing premise (a subscription handle is usable for a no-login supervisory - /// write that commits) AND the optimization (zero AddItem on the borrow path). Control: a writer - /// with no registry source on the same session AddItems exactly once. + /// Write-while-subscribed — the writer must commit a no-login supervisory write to a + /// tag that is ALREADY live-subscribed on the same session, via its own AddItem'd handle. + /// This replaces the retired borrow smoke: the borrow optimization's premise ("a subscription + /// handle is usable for a committing no-login write") was disproven by the correlated write + /// replies — a plain-advised subscription handle refuses the write (SECURITY_ERROR detail + /// 1008) and refuses an advise upgrade (E_INVALIDARG). What this pins instead is the fix's + /// premise: MXAccess's one-advise-per-handle limit is per HANDLE, not per attribute, so the + /// writer's own supervisory-advised handle commits while the subscription keeps its own. /// [Fact] - public async Task Live_writer_borrows_subscription_handle_and_skips_AddItem() + public async Task Live_write_to_subscribed_tag_commits_via_own_handle() { var (endpoint, apiKey) = RequireLiveGatewayOrSkip(); var ct = TestContext.Current.CancellationToken; var clientOptions = BuildClientOptions(endpoint, apiKey); - await using var session = new GalaxyMxSession(new GalaxyMxAccessOptions(ClientName: "OtOpcUaBorrowSmoke")); + await using var session = new GalaxyMxSession(new GalaxyMxAccessOptions(ClientName: "OtOpcUaSubWriteSmoke")); await session.ConnectAsync(clientOptions, ct); - // Subscribe the tag so the registry holds the gateway's REAL item handle for it. + // Live subscription on the tag — the state the borrow path used to piggyback on. var subscriber = new GatewayGalaxySubscriber(session); var subResults = await subscriber.SubscribeBulkAsync([WriteRef], bufferedUpdateIntervalMs: 0, ct); var match = subResults.SingleOrDefault(r => string.Equals(r.TagAddress, WriteRef, StringComparison.OrdinalIgnoreCase) && r.WasSuccessful); match.ShouldNotBeNull($"the gateway should accept a subscription on {WriteRef}"); - match!.ItemHandle.ShouldBeGreaterThan(0, "a successful subscribe must return a positive item handle"); - var registry = new SubscriptionRegistry(); - registry.Register(1, [new TagBinding(WriteRef, match.ItemHandle)]); - - // Borrow case: a writer wired to the registry must commit the write WITHOUT a fresh AddItem, - // and must NOT cache the borrowed handle (the registry owns its lifecycle). - var borrowingWriter = new GatewayGalaxyDataWriter( - session, writeUserId: 0, logger: null, subscribedHandleSource: registry.TryResolveItemHandle); - var borrowed = await borrowingWriter.WriteAsync( + // The production writer must AddItem its own handle and commit, despite the live subscription. + var writer = new GatewayGalaxyDataWriter(session, writeUserId: 0); + var result = await writer.WriteAsync( [new WriteRequest(WriteRef, 2727.0f)], _ => SecurityClassification.FreeAccess, ct); - borrowed.ShouldHaveSingleItem().StatusCode.ShouldBe(0u, "the borrowed-handle write should commit Good (0)"); - borrowingWriter.AddItemCallCount.ShouldBe(0, "the writer should reuse the subscription handle, not AddItem"); - borrowingWriter.CachedItemHandleCount.ShouldBe(0, "a borrowed handle must not be cached in the writer"); - - // Control: a writer with NO source on the same session must AddItem exactly once for the same tag. - var plainWriter = new GatewayGalaxyDataWriter(session, writeUserId: 0); - var control = await plainWriter.WriteAsync( - [new WriteRequest(WriteRef, 3838.0f)], _ => SecurityClassification.FreeAccess, ct); - control.ShouldHaveSingleItem().StatusCode.ShouldBe(0u, "the control write should commit Good (0)"); - plainWriter.AddItemCallCount.ShouldBe(1, "with no borrow source the writer must AddItem once"); + result.ShouldHaveSingleItem().StatusCode.ShouldBe(0u, + "a write to an already-subscribed tag must commit Good via the writer's own handle"); + writer.AddItemCallCount.ShouldBe(1, "the writer owns its handle — exactly one AddItem"); + writer.CachedSupervisedHandleCount.ShouldBe(1, "the writer's own handle should be supervisory-advised"); TestContext.Current.SendDiagnosticMessage( - $"borrow smoke: subscribed {WriteRef} -> handle {match.ItemHandle}; borrowed write Good with AddItemCallCount=0; control AddItemCallCount=1"); + $"write-while-subscribed smoke: subscribed {WriteRef} (handle {match!.ItemHandle}); own-handle write Good, AddItemCallCount=1"); } /// @@ -201,6 +192,56 @@ public sealed class GatewayGalaxyLiveReopenAndWriteTests $"fail-closed guard smoke: normal advised write {WriteRef}=5151 returned Good; handle supervised"); } + /// + /// 06/S-1 confirmation smoke — the gateway (mxaccessgw ≥ 53f69cd) correlates the worker's + /// OnWriteComplete onto the unary write reply as statuses[0], so the production + /// writer's verdict is now the real COM commit outcome, not worker-side acceptance: + /// + /// a committed write returns Good off a CONFIRMED status row + /// (success:-1, category:OK) — pre-correlation this was a provisional Good off an + /// empty statuses array; + /// a REFUSED write (read-only attribute) returns a Bad-family code off + /// the correlated failure row (live shape: category:OPERATIONAL_ERROR, detail:1007) + /// — the capability that never existed before: pre-correlation the refusal was invisible + /// and reported Good, so the #5 write-outcome self-correction never fired for Galaxy. + /// + /// + /// Requires both deployed gateway halves at the correlation build (server ≥ b948e69, worker ≥ + /// 53f69cd); against an older gateway the refusal leg fails — that is the point of the gate. + /// + [Fact] + public async Task Live_correlated_write_statuses_confirm_commit_and_surface_refusal() + { + var (endpoint, apiKey) = RequireLiveGatewayOrSkip(); + var ct = TestContext.Current.CancellationToken; + var clientOptions = BuildClientOptions(endpoint, apiKey); + + await using var session = new GalaxyMxSession(new GalaxyMxAccessOptions(ClientName: "OtOpcUa06S1Smoke")); + await session.ConnectAsync(clientOptions, ct); + var writer = new GatewayGalaxyDataWriter(session, writeUserId: 0); + + // Happy path: the confirmed commit comes back Good (statuses[0] success row, not empty-array + // provisional). The commit itself is proven by Live_supervisory_write_commits_and_persists_to_galaxy. + var good = await writer.WriteAsync( + [new WriteRequest(WriteRef, 6262.0f)], _ => SecurityClassification.FreeAccess, ct); + good.ShouldHaveSingleItem().StatusCode.ShouldBe(0u, + "a committed write should return Good off the correlated status row"); + + // Refusal path: Tagname is read-only, so the automation object refuses the write and the + // correlated failure row must surface as a Bad-family code (top two bits 10b). Pre-correlation + // this returned Good. Assert the family, not the exact code — the detail→code mapping may + // gain per-code specificity (StatusCodeMap.FromCategory) without invalidating the gate. + var refused = await writer.WriteAsync( + [new WriteRequest("TestMachine_002.Tagname", "refused-write-probe")], + _ => SecurityClassification.FreeAccess, ct); + var refusedCode = refused.ShouldHaveSingleItem().StatusCode; + ((refusedCode >> 30) & 0x3u).ShouldBe(2u, + $"a refused write must surface a Bad-family code off the correlated failure row (got 0x{refusedCode:X8})"); + + TestContext.Current.SendDiagnosticMessage( + $"06/S-1 smoke: {WriteRef}=6262 -> Good confirmed; Tagname write refused with 0x{refusedCode:X8}"); + } + private static (string Endpoint, string ApiKey) RequireLiveGatewayOrSkip() { var endpoint = Environment.GetEnvironmentVariable("MXGW_ENDPOINT"); diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/GatewayGalaxyWriteMeterLiveTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/GatewayGalaxyWriteMeterLiveTests.cs new file mode 100644 index 00000000..7e024575 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/GatewayGalaxyWriteMeterLiveTests.cs @@ -0,0 +1,133 @@ +using ZB.MOM.WW.MxGateway.Client; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Config; +using ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Runtime; + +namespace ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests.Runtime; + +/// +/// Live meter legs for the 06/S-1 write-confirmation gate — the observation that closed the +/// archreview finding: against a correlated gateway (mxaccessgw server ≥ b948e69, worker ≥ +/// 53f69cd) every unary write reply carries the worker's OnWriteComplete outcome as +/// statuses[0], so galaxy.writes.unconfirmed — which used to increment on EVERY +/// write — stays flat, and only a completion-wait timeout (or a pre-correlation gateway) meters. +/// +/// ⚠️ Run these FILTERED (one test class), never as part of a full-assembly run with the env vars +/// set: the offline exercise the unconfirmed path +/// against fakes, and xunit runs test classes in parallel, so their increments would pollute the +/// process-wide counters these tests listen to. +/// +/// +[Trait("Category", "Integration")] +public sealed class GatewayGalaxyWriteMeterLiveTests +{ + private const string WriteRef = "TestMachine_002.TestFloat"; + + /// + /// Confirmed writes keep both write meters flat: five committed writes and one refusal, all + /// carrying a correlated status row — zero unconfirmed (the counter is presence-based, + /// so the refusal must not increment it either) and zero advise_failed. + /// + [Fact] + public async Task Live_confirmed_writes_keep_unconfirmed_meter_flat() + { + var (endpoint, apiKey) = RequireLiveGatewayOrSkip(); + + long unconfirmed = 0; + long adviseFailed = 0; + using var listener = new System.Diagnostics.Metrics.MeterListener(); + listener.InstrumentPublished = (instrument, l) => + { + if (instrument.Name is "galaxy.writes.unconfirmed" or "galaxy.writes.advise_failed") + l.EnableMeasurementEvents(instrument); + }; + listener.SetMeasurementEventCallback((instrument, value, _, _) => + { + if (instrument.Name == "galaxy.writes.unconfirmed") Interlocked.Add(ref unconfirmed, value); + else Interlocked.Add(ref adviseFailed, value); + }); + listener.Start(); + + var ct = TestContext.Current.CancellationToken; + await using var session = new GalaxyMxSession(new GalaxyMxAccessOptions(ClientName: "OtOpcUa06S1Meter")); + await session.ConnectAsync(BuildClientOptions(endpoint, apiKey), ct); + var writer = new GatewayGalaxyDataWriter(session, writeUserId: 0); + + for (var i = 0; i < 5; i++) + { + var r = await writer.WriteAsync( + [new WriteRequest(WriteRef, 7000.0f + i)], _ => SecurityClassification.FreeAccess, ct); + Assert.Equal(0u, r[0].StatusCode); + } + var refused = await writer.WriteAsync( + [new WriteRequest("TestMachine_002.Tagname", "meter-leg-refusal")], + _ => SecurityClassification.FreeAccess, ct); + Assert.Equal(2u, (refused[0].StatusCode >> 30) & 0x3u); + + listener.Dispose(); + Assert.Equal(0, Interlocked.Read(ref unconfirmed)); + Assert.Equal(0, Interlocked.Read(ref adviseFailed)); + } + + /// + /// Timeout-control leg — run ONLY while the target gateway has the completion wait disabled + /// (MxGateway__Worker__WriteCompletionWaitMilliseconds=0, an NSSM env-var flip + + /// service restart on a deployed instance): every reply is then empty-statuses, so the writer + /// reports provisional Good and meters each write unconfirmed. Gated on + /// MXGW_WRITE_WAIT_DISABLED=1 so it can never run (and fail) against a + /// normally-configured gateway. Remove the override (same nssm command with -) and + /// restart afterwards; + /// passing again proves the restoration. + /// + [Fact] + public async Task Live_wait_disabled_gateway_yields_provisional_good_and_metered_unconfirmed() + { + if (Environment.GetEnvironmentVariable("MXGW_WRITE_WAIT_DISABLED") != "1") + Assert.Skip("Set MXGW_WRITE_WAIT_DISABLED=1 (with the gateway's completion wait set to 0) to run the timeout leg."); + var (endpoint, apiKey) = RequireLiveGatewayOrSkip(); + + long unconfirmed = 0; + using var listener = new System.Diagnostics.Metrics.MeterListener(); + listener.InstrumentPublished = (instrument, l) => + { + if (instrument.Name == "galaxy.writes.unconfirmed") l.EnableMeasurementEvents(instrument); + }; + listener.SetMeasurementEventCallback((_, value, _, _) => Interlocked.Add(ref unconfirmed, value)); + listener.Start(); + + var ct = TestContext.Current.CancellationToken; + await using var session = new GalaxyMxSession(new GalaxyMxAccessOptions(ClientName: "OtOpcUa06S1Leg3")); + await session.ConnectAsync(BuildClientOptions(endpoint, apiKey), ct); + var writer = new GatewayGalaxyDataWriter(session, writeUserId: 0); + + for (var i = 0; i < 3; i++) + { + var r = await writer.WriteAsync( + [new WriteRequest(WriteRef, 7100.0f + i)], _ => SecurityClassification.FreeAccess, ct); + Assert.Equal(0u, r[0].StatusCode); // provisional Good off the empty-statuses reply + } + + listener.Dispose(); + Assert.Equal(3, Interlocked.Read(ref unconfirmed)); + } + + private static (string Endpoint, string ApiKey) RequireLiveGatewayOrSkip() + { + var endpoint = Environment.GetEnvironmentVariable("MXGW_ENDPOINT"); + var apiKey = Environment.GetEnvironmentVariable("GALAXY_MXGW_API_KEY"); + if (string.IsNullOrWhiteSpace(endpoint) || string.IsNullOrWhiteSpace(apiKey)) + Assert.Skip("Set MXGW_ENDPOINT + GALAXY_MXGW_API_KEY to run the live write-meter legs."); + return (endpoint!, apiKey!); + } + + private static MxGatewayClientOptions BuildClientOptions(string endpoint, string apiKey) => new() + { + Endpoint = new Uri(endpoint, UriKind.Absolute), + ApiKey = apiKey, + UseTls = false, + ConnectTimeout = TimeSpan.FromSeconds(10), + DefaultCallTimeout = TimeSpan.FromSeconds(30), + StreamTimeout = TimeSpan.FromSeconds(30), + }; +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/StatusCodeMapTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/StatusCodeMapTests.cs index 3ec18e17..f3c67194 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/StatusCodeMapTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/StatusCodeMapTests.cs @@ -108,6 +108,62 @@ public sealed class StatusCodeMapTests StatusCodeMap.FromMxStatus(s).ShouldBe(StatusCodeMap.Bad); } + // ===== 06/S-1 write-completion rows: detail is an MxStatus detail code, NOT a quality byte ===== + // The gateway (≥ 53f69cd) correlates the worker's OnWriteComplete onto the unary write reply as + // statuses[0]. Those rows carry MxStatus detail codes (live-captured on windev: 1007 operational + // refusal on a read-only attribute, 1008 security refusal on a plain-advised handle). The old + // truncation `(byte)(Detail & 0xFF)` pushed 1007 → 239 and 1008 → 240 into the OPC DA Good band, + // mapping a REFUSED write to Good — no node revert, silently-lost write reported committed. + + /// A refused write (operational error, MxStatus detail code) must map to a Bad-family code. + [Fact] + public void FromMxStatus_WriteRefusal_OperationalError_Detail1007_IsBad() + { + // Live shape: writeRaw to read-only TestMachine_002.Tagname. + var s = new MxStatusProxy { Success = 0, Category = MxStatusCategory.OperationalError, Detail = 1007, RawDetectedBy = 5 }; + StatusCodeMap.FromMxStatus(s).ShouldBe(StatusCodeMap.Bad); + } + + /// A security-refused write must map to BadUserAccessDenied, not Good. + [Fact] + public void FromMxStatus_WriteRefusal_SecurityError_Detail1008_IsUserAccessDenied() + { + // Live shape: writeRaw through a plain-advised (subscription) handle — no supervisory advise. + var s = new MxStatusProxy { Success = 0, Category = MxStatusCategory.SecurityError, Detail = 1008, RawDetectedBy = 5 }; + StatusCodeMap.FromMxStatus(s).ShouldBe(StatusCodeMap.BadUserAccessDenied); + } + + /// Category-level routing for large details preserves communication/configuration specificity. + [Theory] + [InlineData(MxStatusCategory.CommunicationError, 0x80050000u)] // BadCommunicationError + [InlineData(MxStatusCategory.ConfigurationError, 0x80890000u)] // BadConfigurationError + [InlineData(MxStatusCategory.SoftwareError, 0x80000000u)] // unmapped category → generic Bad + public void FromMxStatus_LargeDetail_RoutesByCategory(MxStatusCategory category, uint expected) + { + var s = new MxStatusProxy { Success = 0, Category = category, Detail = 2001 }; + StatusCodeMap.FromMxStatus(s).ShouldBe(expected); + } + + /// + /// Invariant: a row that is not IsSuccess() must NEVER map to Good — a Good-band quality + /// byte (e.g. 216 = GoodLocalOverride) on a failure row is contradictory and falls back to + /// the category route. Uncertain-band substatuses remain allowed (uncertain is not Good). + /// + [Fact] + public void FromMxStatus_FailureRow_GoodBandQualityByte_NeverMapsGood() + { + var s = new MxStatusProxy { Success = 0, Category = MxStatusCategory.SecurityError, Detail = 216 }; + StatusCodeMap.FromMxStatus(s).ShouldBe(StatusCodeMap.BadUserAccessDenied); + } + + /// An Uncertain-band quality substatus on a failure row still maps to its specific Uncertain code. + [Fact] + public void FromMxStatus_FailureRow_UncertainBandQualityByte_KeepsUncertainCode() + { + var s = new MxStatusProxy { Success = 0, Category = MxStatusCategory.OperationalError, Detail = 68 }; + StatusCodeMap.FromMxStatus(s).ShouldBe(StatusCodeMap.UncertainLastUsableValue); + } + /// Verifies that top two bits of status codes follow OPC UA convention. [Fact] public void TopByteCategoryBits_StayWithinOpcUaConvention() diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/SubscriptionRegistryHandleResolveTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/SubscriptionRegistryHandleResolveTests.cs deleted file mode 100644 index e1e71991..00000000 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/SubscriptionRegistryHandleResolveTests.cs +++ /dev/null @@ -1,156 +0,0 @@ -using Shouldly; -using Xunit; -using ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Runtime; - -namespace ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests.Runtime; - -/// -/// Tests for — the forward -/// fullRef → live item-handle lookup the Galaxy writer uses to skip a redundant -/// AddItem round-trip when an already-subscribed tag is written. -/// -public sealed class SubscriptionRegistryHandleResolveTests -{ - /// - /// Verifies that after registering a binding, TryResolveItemHandle returns the correct handle. - /// - [Fact] - public void Register_ThenResolve_ReturnsHandle() - { - var registry = new SubscriptionRegistry(); - registry.Register(1, [new TagBinding("Tag.A", 5)]); - - registry.TryResolveItemHandle("Tag.A").ShouldBe(5); - } - - /// - /// Verifies that TryResolveItemHandle is case-insensitive on the full reference. - /// - [Fact] - public void Register_ThenResolve_IsCaseInsensitive() - { - var registry = new SubscriptionRegistry(); - registry.Register(1, [new TagBinding("Tag.A", 5)]); - - registry.TryResolveItemHandle("tag.a").ShouldBe(5); - } - - /// - /// Verifies that a full reference that was never registered resolves to null. - /// - [Fact] - public void NeverRegistered_ReturnsNull() - { - var registry = new SubscriptionRegistry(); - - registry.TryResolveItemHandle("Tag.NotHere").ShouldBeNull(); - } - - /// - /// Verifies that after Remove(), the forward lookup returns null. - /// - [Fact] - public void Remove_ThenResolve_ReturnsNull() - { - var registry = new SubscriptionRegistry(); - registry.Register(1, [new TagBinding("Tag.A", 5)]); - - registry.Remove(1); - - registry.TryResolveItemHandle("Tag.A").ShouldBeNull(); - } - - /// - /// Verifies that after Rebind() the forward lookup returns the new handle, not the old one. - /// - [Fact] - public void Rebind_ThenResolve_ReturnsNewHandle() - { - var registry = new SubscriptionRegistry(); - registry.Register(1, [new TagBinding("Tag.A", 5)]); - - registry.Rebind(1, [new TagBinding("Tag.A", 99)]); - - registry.TryResolveItemHandle("Tag.A").ShouldBe(99); - } - - /// - /// Verifies that a binding with ItemHandle <= 0 (gateway-rejected) is not resolvable. - /// - [Fact] - public void FailedBinding_ZeroHandle_IsNotResolvable() - { - var registry = new SubscriptionRegistry(); - registry.Register(1, [new TagBinding("Tag.Failed", 0)]); - - registry.TryResolveItemHandle("Tag.Failed").ShouldBeNull(); - } - - /// - /// Verifies that a binding with a negative ItemHandle is not resolvable. - /// - [Fact] - public void FailedBinding_NegativeHandle_IsNotResolvable() - { - var registry = new SubscriptionRegistry(); - registry.Register(1, [new TagBinding("Tag.Failed", -1)]); - - registry.TryResolveItemHandle("Tag.Failed").ShouldBeNull(); - } - - /// - /// Verifies the liveness guard: after the only subscriber of a handle is removed, - /// the forward lookup returns null even if a stale forward-map entry lingers. - /// - [Fact] - public void Remove_OnlySubscriber_LivenessGuard_ReturnsNull() - { - var registry = new SubscriptionRegistry(); - registry.Register(1, [new TagBinding("Tag.A", 5)]); - - registry.Remove(1); - - // After removal the subscriber set for handle 5 is gone, so TryResolveItemHandle - // must return null regardless of whether the forward entry was cleaned up. - registry.TryResolveItemHandle("Tag.A").ShouldBeNull(); - } - - /// - /// A tag may legitimately appear in multiple driver subscriptions (separate OPC UA monitored - /// items on the same Galaxy attribute) — they share one gw item handle. Removing ONE of them - /// must keep the tag resolvable while another subscription still binds it, so the writer keeps - /// borrowing instead of falling back to AddItem. - /// - [Fact] - public void Remove_OneOfTwoSubscribersForSameRef_StillResolves() - { - var registry = new SubscriptionRegistry(); - registry.Register(1, [new TagBinding("Tag.A", 5)]); - registry.Register(2, [new TagBinding("Tag.A", 5)]); - - registry.Remove(1); - - // sub2 still binds Tag.A -> 5, so the borrow must still be offered. - registry.TryResolveItemHandle("Tag.A").ShouldBe(5); - } - - /// - /// Authoritative resolution: if two DIFFERENT refs ever map to the same numeric handle and the - /// subscription for one is removed, that ref must NOT resolve to the handle that now belongs to - /// the other ref — a wrong-tag write would be the worst outcome. Resolution confirms a live - /// subscription genuinely binds fullRef -> handle, not just that the handle is alive. - /// - [Fact] - public void Remove_CrossRefSameHandle_DoesNotResolveToTheOtherRefsHandle() - { - var registry = new SubscriptionRegistry(); - registry.Register(1, [new TagBinding("Tag.A", 5)]); - registry.Register(2, [new TagBinding("Tag.B", 5)]); - - registry.Remove(1); - - // Tag.A's subscription is gone; handle 5 is still alive but now only Tag.B binds it. - registry.TryResolveItemHandle("Tag.A").ShouldBeNull(); - registry.TryResolveItemHandle("Tag.B").ShouldBe(5); - } -}