diff --git a/CLAUDE.md b/CLAUDE.md index a95b328..bf44902 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -76,11 +76,11 @@ powershell -ExecutionPolicy Bypass -File scripts/run-client-e2e-tests.ps1 - **Style guides** in `docs/style-guides/` are authoritative. Follow `CSharpStyleGuide.md` for gateway/worker/.NET-client code: file-scoped namespaces, `sealed` by default, `Async` suffix on Task-returning methods, MXAccess-aligned names (`MxStatusProxy`, `ServerHandle`, `ItemHandle`, `HResult`). - **MXAccess parity is the contract.** Don't "fix" surprising MXAccess behavior (e.g., `WriteSecured` failing before a value-bearing NMX body, distinct `OperationComplete` semantics, invalid-handle exceptions) unless the client explicitly opts into a non-parity mode. The installed MXAccess COM component is the baseline. - **Don't synthesize events.** The gateway forwards only events the worker emits; it never invents `OperationComplete` from write completion or command replies. -- **One worker per session** (invariant). Multi-subscriber event fan-out and reconnect-with-replay have shipped and are config-gated: `AllowMultipleEventSubscribers` (default `false`) enables fan-out up to `MaxEventSubscribersPerSession` (default `8`); `DetachGraceSeconds` (default `30`) retains a session after its last subscriber drops so clients can reconnect; `ReplayBufferCapacity` / `ReplayRetentionSeconds` control how much event history the replay ring keeps. Default config is single-subscriber (`AllowMultipleEventSubscribers` off), but detach-grace and replay retention are **on** by default (`DetachGraceSeconds=30`, `ReplayBufferCapacity=1024`, `ReplayRetentionSeconds=300`): a detached session is retained for 30 s and recent events are buffered for reconnect. See `docs/DesignDecisions.md` and `docs/Sessions.md`. +- **One worker per session** (invariant). Multi-subscriber event fan-out and reconnect-with-replay have shipped and are config-gated: `AllowMultipleEventSubscribers` (default `false`) enables fan-out up to `MaxEventSubscribersPerSession` (default `8`); `DetachGraceSeconds` (default `30`) retains a session after its last subscriber drops so clients can reconnect; `ReplayBufferCapacity` / `ReplayRetentionSeconds` control how much event history the replay ring keeps. Default config is single-subscriber (`AllowMultipleEventSubscribers` off), but detach-grace and replay retention are **on** by default (`DetachGraceSeconds=30`, `ReplayBufferCapacity=1024`, `ReplayRetentionSeconds=300`): a detached session is retained for 30 s and recent events are buffered for reconnect. The reconnect protocol is consumable end-to-end: a resuming `StreamEvents` (via `after_worker_sequence`) that predates the retained ring gets a `ReplayGap` sentinel, and all five official clients surface it as a typed signal. Orphan-worker reattach after a gateway restart is **deferred, not planned** — see `oldtasks.md` (session-resilience epic Phase 5); the invariant on the next line stands. See `docs/DesignDecisions.md` and `docs/Sessions.md`. - **Gateway restart does not reattach orphan workers.** The first version terminates orphaned workers on startup; do not design code paths that assume reattachment. - **No Blazor UI component libraries.** Dashboard uses local Bootstrap CSS/JS only — do not introduce MudBlazor, Radzen, FluentUI, etc. - **Don't log secrets or full tag values by default.** API keys, passwords, `WriteSecured` payloads, and `AuthenticateUser` credentials must never reach logs. Value logging is opt-in and redacted. -- **Generated code** under `src/ZB.MOM.WW.MxGateway.Contracts/Generated/`, `clients/*/generated*/`, `clients/python/src/mxgateway/generated/`, etc., is build output. Don't hand-edit. To regenerate, build the contracts project (`dotnet build src/ZB.MOM.WW.MxGateway.Contracts/ZB.MOM.WW.MxGateway.Contracts.csproj`) or run the per-client generation step in that client's README. +- **Generated code** under `src/ZB.MOM.WW.MxGateway.Contracts/Generated/`, `clients/*/generated*/`, `clients/python/src/zb_mom_ww_mxgateway/generated/`, etc., is build output. Don't hand-edit. To regenerate, build the contracts project (`dotnet build src/ZB.MOM.WW.MxGateway.Contracts/ZB.MOM.WW.MxGateway.Contracts.csproj`) or run the per-client generation step in that client's README. - **Documentation style** (`StyleGuide.md`): PascalCase filenames, no marketing language, present tense, explain *why* not *what*. - **Update docs in the same change as the source.** When public APIs, contracts, configuration, build steps, security behavior, event shapes, value conversion, status mapping, or lifecycle rules change, the affected docs (`gateway.md`, `docs/`, client READMEs, design docs) must change in the same commit. Don't leave stale prose describing old behavior. @@ -123,7 +123,7 @@ Gateway gRPC clients authenticate with an API key in metadata: `authorization: B Session event streaming is **owner-scoped**: the API key that opened a session is recorded on the session, and every `StreamEvents` attach/reattach is rejected with `PermissionDenied` unless the caller's key id matches the owner. Possessing the `event` scope and knowing a session id is not sufficient — this closes the reconnect/fan-out trust boundary (detach-grace and replay retention are on by default) so an `event`-scoped key cannot attach to another key's retained session. -Dashboard auth is LDAP-backed (separate from the gRPC API-key model). `/login` binds against `MxGateway:Ldap` and maps the user's LDAP groups to `Admin` or `Viewer` via `MxGateway:Dashboard:GroupToRole`, then issues an HTTP-only secure `__Host-MxGatewayDashboard` cookie. SignalR hubs at `/hubs/{snapshot,alarms,events}` accept either the cookie or a 5-minute bearer minted at `/hubs/token`. `Dashboard:AllowAnonymousLocalhost` (default `true`) grants **read-only** loopback access — it satisfies the Viewer requirement but never the Admin-only requirement, so anonymous localhost can view the dashboard but not reach API-key CRUD or session Close/Kill (`Authentication:Mode=Disabled` is scoped the same way). `Dashboard:DisableLogin` (default `false`) auto-authenticates every dashboard request — including remote browsers — as `Dashboard:AutoLoginUser` (default `multi-role`) with both Admin and Viewer roles; dev/test only, never enable in production. +Dashboard auth is LDAP-backed (separate from the gRPC API-key model). `/login` binds against `MxGateway:Ldap` and maps the user's LDAP groups to `Admin` or `Viewer` via `MxGateway:Dashboard:GroupToRole`, then issues an HTTP-only secure cookie named `__Host-MxGatewayDashboard` when `Dashboard:RequireHttpsCookie` is true (default) and no `Dashboard:CookieName` override is set, else the plain `MxGatewayDashboard` (the `__Host-` prefix requires a Secure cookie). SignalR hubs at `/hubs/{snapshot,alarms,events}` accept either the cookie or a 5-minute bearer minted at `/hubs/token`. `Dashboard:AllowAnonymousLocalhost` (default `true`) grants **read-only** loopback access — it satisfies the Viewer requirement but never the Admin-only requirement, so anonymous localhost can view the dashboard but not reach API-key CRUD or session Close/Kill (`Authentication:Mode=Disabled` is scoped the same way). `Dashboard:DisableLogin` (default `false`) auto-authenticates every dashboard request — including remote browsers — as `Dashboard:AutoLoginUser` (default `multi-role`) with both Admin and Viewer roles; dev/test only, never enable in production. ## Process / Platform Notes diff --git a/archreview/remediation/00-tracking.md b/archreview/remediation/00-tracking.md index c5c3a63..8bf4044 100644 --- a/archreview/remediation/00-tracking.md +++ b/archreview/remediation/00-tracking.md @@ -64,16 +64,16 @@ Full design + implementation for each row lives in the linked domain doc under i | GWC-03 | High | P0 | S | — | Done | Documented sparse-array max-length bound is unimplemented | | GWC-04 | Medium | P1 | M | — | Done | Full event channel stalls the worker read loop behind command replies | | GWC-05 | Medium | — | S | — | Not started | Worker pipe created with no ACL / no CurrentUserOnly | -| GWC-06 | Medium | P2 | S | GWC-07,08 | Not started | Stopwatch allocated per streamed event | -| GWC-07 | Medium | P2 | S | GWC-06,08 | Not started | Every mapped event is deep-cloned | -| GWC-08 | Medium | P2 | M | GWC-06,07 | Not started | Pipe framing allocates per frame and writes twice | +| GWC-06 | Medium | P2 | S | GWC-07,08 | Done | Stopwatch allocated per streamed event | +| GWC-07 | Medium | P2 | S | GWC-06,08 | Done | Every mapped event is deep-cloned | +| GWC-08 | Medium | P2 | M | GWC-06,07 | Done | Pipe framing allocates per frame and writes twice | | GWC-09 | Medium | — | M | — | Not started | Startup probe is a no-op; its retry pipeline can never retry | | GWC-10 | Medium | — | S | — | Not started | Envelope `sequence` monotonicity specified but not enforced | | GWC-11 | Low | — | S | — | Not started | `_workerClient` written under lock but read lock-free | | GWC-12 | Low | — | S | — | Not started | `WorkerClient.DisposeAsync` not safe against double-dispose | | GWC-13 | Low | — | M | — | Not started | Worker-ready wait is a 25 ms poll loop | -| GWC-14 | Low | P2 | M | — | Not started | Replay ring is a `LinkedList` with a node alloc per event | -| GWC-15 | Low | P2 | S | — | Not started | Per-event gauge reads `ChannelReader.Count` every event | +| GWC-14 | Low | P2 | M | — | Done | Replay ring is a `LinkedList` with a node alloc per event | +| GWC-15 | Low | P2 | S | — | Done | Per-event gauge reads `ChannelReader.Count` every event | | GWC-16 | Low | — | S | — | Not started | `Invoke` resolves the session twice per command | | GWC-17 | Low | — | M | — | Not started | Sessions layer throws `Grpc.Core.RpcException` (layering leak) | | GWC-18 | Low | — | S | — | Not started | `GatewaySession` implements `DisposeAsync` without `IAsyncDisposable` | @@ -92,16 +92,16 @@ Full design + implementation for each row lives in the linked domain doc under i | WRK-03 | Medium | — | S | WRK-02 | Not started | Commands after shutdown starts are dropped with no reply | | WRK-04 | Medium | — | M | — | Done | Envelope `sequence` can appear out of order on the wire | | WRK-05 | Medium | — | M | — | Not started | One transient alarm-poll failure kills the whole session | -| WRK-06 | Medium | P2 | S | — | Not started | `MXSTATUS_PROXY` conversion reflects per field, per event | +| WRK-06 | Medium | P2 | S | — | Done | `MXSTATUS_PROXY` conversion reflects per field, per event | | WRK-07 | Medium | P1 | M | WRK-04 | Done | Documented outbound write priority not implemented | | WRK-08 | Low | — | S | — | Not started | Residual event queue discarded at graceful shutdown, undocumented | | WRK-09 | Low | — | S | — | Not started | No `AppDomain.UnhandledException` hook | | WRK-10 | Low | — | S | — | Not started | Top-level catch logs exception type but never message | -| WRK-11 | Low | P2 | S | — | Not started | Every accepted event is defensively cloned on enqueue | -| WRK-12 | Low | P2 | M | WRK-04 | Not started | No event batching per envelope; one flush per event; 25 ms poll | +| WRK-11 | Low | P2 | S | — | Done | Every accepted event is defensively cloned on enqueue | +| WRK-12 | Low | P2 | M | WRK-04 | Done | No event batching per envelope; one flush per event; 25 ms poll | | WRK-13 | Low | — | S | — | Not started | Frame writer allocates a fresh buffer per frame; reader pools | | WRK-14 | Low | — | M | — | Not started | Private-field naming split `_camelCase` vs `camelCase` | -| WRK-15 | Low | P2 | S | — | Not started | Doc drift: STA thread name and heartbeat-counter note | +| WRK-15 | Low | P2 | S | — | Done | Doc drift: STA thread name and heartbeat-counter note | | WRK-16 | Low | — | S | — | Not started | Boilerplate duplication in IPC envelope/ctor overloads | | WRK-17 | Low | — | S | — | Not started | Gateway death exits with wrong exit code (6 not 5) | | WRK-18 | Low | — | S | — | Not started | Event-queue overflow exits as generic `UnexpectedFailure` | @@ -116,23 +116,23 @@ Full design + implementation for each row lives in the linked domain doc under i | IPC-02 | Medium | P1 | M | IPC-03 | Done | Worker max frame size hard-coded, cannot follow gateway config | | IPC-03 | Medium | P1 | M | IPC-02 | Done | gRPC max == pipe max with zero headroom; oversized write faults whole session | | IPC-04 | Medium | P1 | S | IPC-03 | Done | `DrainEvents max_events=0` packs entire queue into one reply frame | -| IPC-05 | Medium | P2 | S | — | Not started | Redundant deep copies on command and event hot paths | -| IPC-06 | Medium | P2 | S | — | Not started | `gateway.md` Worker Envelope sketch no longer matches the contract | -| IPC-07 | Medium | P2 | S | — | Not started | `docs/Grpc.md` says six RPCs; there are seven | +| IPC-05 | Medium | P2 | S | — | Done | Redundant deep copies on command and event hot paths | +| IPC-06 | Medium | P2 | S | — | Done | `gateway.md` Worker Envelope sketch no longer matches the contract | +| IPC-07 | Medium | P2 | S | — | Done | `docs/Grpc.md` says six RPCs; there are seven | | IPC-08 | Medium | — | M | — | Not started | `WorkerCancel` defined and handled but never sent | | IPC-09 | Medium | P1 | M | IPC-01 | Done | Known codegen fragilities have no in-repo guards | | IPC-10 | Low | — | S | — | Not started | Envelope `sequence` is write-only; monotonicity never validated | | IPC-11 | Low | — | S | — | Not started | No protocol version negotiation despite `supported_protocol_version` name | | IPC-12 | Low | — | S | — | Not started | Gateway frame writer has no write lock; integrity rests on undocumented invariant | -| IPC-13 | Low | P2 | S | IPC-05 | Not started | Gateway writer serializes twice and issues two stream writes | -| IPC-14 | Low | P2 | S | IPC-05 | Not started | Gateway reader allocates fresh array per frame; worker rents from `ArrayPool` | -| IPC-15 | Low | P2 | M | — | Not started | Worker event delivery 25 ms poll with per-event write+flush, no batching | +| IPC-13 | Low | P2 | S | IPC-05 | Done | Gateway writer serializes twice and issues two stream writes | +| IPC-14 | Low | P2 | S | IPC-05 | Done | Gateway reader allocates fresh array per frame; worker rents from `ArrayPool` | +| IPC-15 | Low | P2 | M | — | Done | Worker event delivery 25 ms poll with per-event write+flush, no batching | | IPC-16 | Low | — | S | — | N/A | Correlation id carried twice per reply (Info) | -| IPC-17 | Low | P2 | S | — | Not started | Two docs name the wrong Python generated-output directory | +| IPC-17 | Low | P2 | S | — | Done | Two docs name the wrong Python generated-output directory | | IPC-18 | Low | — | S | IPC-01 | N/A | Generated-code hygiene verified good — preserve under change (positive) | | IPC-19 | Low | P1 | S | IPC-01 | Done | Generated/-must-be-committed rule for net48 undocumented and unguarded | | IPC-20 | Low | P1 | S | IPC-01 | Done | Descriptor `-Check` byte-compares source-info bytes; protoc-version-sensitive | -| IPC-21 | Low | P2 | S | IPC-06 | Not started | `gateway.md` presents unimplemented `Session` RPC as live | +| IPC-21 | Low | P2 | S | IPC-06 | Done | `gateway.md` presents unimplemented `Session` RPC as live | | IPC-22 | Low | — | S | — | N/A | Public error-detail model stops at status codes plus prose (Info) | ### Security & dashboard — [40-security-dashboard.md](40-security-dashboard.md) @@ -141,13 +141,13 @@ Full design + implementation for each row lives in the linked domain doc under i |---|---|:-:|:-:|---|---|---| | SEC-01 | Medium | P1 | M | — | Done | Windows-absolute default paths become relative files off-Windows | | SEC-02 | Medium | P1 | S | — | Done | `AllowAnonymousLocalhost` satisfies AdminOnly, not just Viewer | -| SEC-03 | Medium | P2 | S | — | Not started | Dashboard cookie lost its `__Host-` prefix; four docs still promise it | +| SEC-03 | Medium | P2 | S | — | Done | Dashboard cookie lost its `__Host-` prefix; four docs still promise it | | SEC-04 | Medium | P1 | S | SEC-03 | Done | `DisableLogin` has no production guard | | SEC-05 | Medium | P1 | M | — | Done | Hub bearer tokens irrevocable for 30 min, carried in query string | | SEC-06 | Medium | P1 | M | — | Done | LDAP plaintext-by-default with a committed service password | | SEC-07 | Medium | P1 | S | — | Done | `QueryActiveAlarmsRequest` missing from scope resolver; tests mislabeled | | SEC-08 | Medium | P1 | L | — | Done | Per-RPC SQLite read + `last_used_utc` write; no verification cache | -| SEC-09 | Medium | P2 | S | — | Not started | Dashboard design-doc `GroupToRole` sample now fails startup validation | +| SEC-09 | Medium | P2 | S | — | Done | Dashboard design-doc `GroupToRole` sample now fails startup validation | | SEC-10 | Medium | P1 | L | — | Done | No API-key expiry | | SEC-11 | Medium | P1 | M | SEC-08 | Done | No rate limiting or lockout on either auth surface | | SEC-12 | Medium | P1 | M | — | Done | Dashboard Close/Kill bypass the canonical audit store | @@ -160,15 +160,15 @@ Full design + implementation for each row lives in the linked domain doc under i | SEC-19 | Low | — | S | — | Not started | Canonical audit store re-issues `CREATE TABLE` per write/read | | SEC-20 | Low | P1 | S | — | Done | `session_id` metric tag is unbounded cardinality | | SEC-21 | Low | — | M | — | Not started | Snapshot publisher works every second regardless of audience | -| SEC-22 | Low | P2 | M | — | Not started | `docs/Authentication.md` documents types no longer in this repo | +| SEC-22 | Low | P2 | M | — | Done | `docs/Authentication.md` documents types no longer in this repo | | SEC-23 | Low | — | S | SEC-03,04 | Not started | Validator ignores `DisableLogin`/`AutoLoginUser`/`RequireHttpsCookie`/`CookieName` | | SEC-24 | Low | — | S | SEC-04 | Not started | Effective-config view omits the riskiest dashboard flags | -| SEC-25 | Low | P2 | M | SEC-02 | Not started | Per-session EventsHub ACL is an acknowledged TODO | +| SEC-25 | Low | P2 | M | SEC-02 | Done | Per-session EventsHub ACL is an acknowledged TODO | | SEC-26 | Low | — | M | — | Not started | Audit trail has no retention or pruning | | SEC-27 | Low | — | S | — | Not started | Dashboard `GatewayStatus` is hardcoded Healthy | | SEC-28 | Info | — | S | — | N/A | Positive security observations (preserve under change) | | SEC-29 | Info | — | S | — | N/A | UI-stack rule verified compliant | -| SEC-30 | Info | P2 | S | SEC-13 | Not started | Value-logging feature is unwired end-to-end | +| SEC-30 | Info | P2 | S | SEC-13 | Done | Value-logging feature is unwired end-to-end | ### Clients — [50-clients.md](50-clients.md) @@ -177,7 +177,7 @@ Full design + implementation for each row lives in the linked domain doc under i | CLI-01 | High | P0 | M | — | Done | Go `Session.Events()` silently closes stream on 16-slot overflow | | CLI-02 | High | P1 | M | — | Done | Rust crate unbuildable outside the repo (`build.rs` path + `--no-verify`) | | CLI-03 | High | P0 | M | — | Done | Rust `invoke` never validates HRESULT / MXSTATUS_PROXY | -| CLI-04 | High | P2 | L | CLI-15 | Not started | Typed-command parity gap across all five clients | +| CLI-04 | High | P2 | L | CLI-15 | Done | Typed-command parity gap across all five clients | | CLI-05 | Medium | — | S | — | Not started | .NET session cannot be re-attached to an existing session id | | CLI-06 | Medium | — | S | — | Not started | .NET `DisposeAsync` blocks/throws on unreachable gateway | | CLI-07 | Medium | — | S | — | Not started | .NET retry budget self-defeats on `DeadlineExceeded` | @@ -185,25 +185,25 @@ Full design + implementation for each row lives in the linked domain doc under i | CLI-09 | Medium | — | M | — | Not started | Go has no typed auth-error mapping (Unauthenticated vs PermissionDenied) | | CLI-10 | Medium | — | M | — | Not started | Go uses deprecated `grpc.DialContext` + `grpc.WithBlock()` | | CLI-11 | Medium | — | S | — | Not started | Go CLI cannot opt into strict TLS validation | -| CLI-12 | Medium | P2 | S | — | Not started | Java docs still say "Java 21" after the JDK 17 retarget | +| CLI-12 | Medium | P2 | S | — | Done | Java docs still say "Java 21" after the JDK 17 retarget | | CLI-13 | Medium | — | M | — | Not started | Java event-stream buffer hardcoded 16, cancel-on-overflow | | CLI-14 | Medium | — | S | — | Not started | Python default TLS is blocking TOFU pin with silent `localhost` SNI | -| CLI-15 | Medium | P2 | M | — | Not started | No client handles `ReplayGap` or offers a reconnect helper | -| CLI-16 | Medium | P2 | S | CLI-12 | Not started | `docs/ClientPackaging.md` drifted from Python naming and .NET `.slnx` | +| CLI-15 | Medium | P2 | M | — | Done | No client handles `ReplayGap` or offers a reconnect helper | +| CLI-16 | Medium | P2 | S | CLI-12 | Done | `docs/ClientPackaging.md` drifted from Python naming and .NET `.slnx` | | CLI-17 | Medium | — | M | CLI-14 | Not started | TLS default posture inconsistent across the five clients | -| CLI-18 | Low | P2 | S | — | Not started | .NET csproj records no `` | +| CLI-18 | Low | P2 | S | — | Done | .NET csproj records no `` | | CLI-19 | Low | — | S | — | Not started | .NET duplicate `InternalsVisibleTo` (AssemblyInfo + csproj) | | CLI-20 | Low | — | S | CLI-17 | Not started | .NET `--tls` without CA installs accept-all callback | -| CLI-21 | Low | P2 | S | — | Not started | Go `ClientVersion = "0.1.0-dev"` stale vs tagged releases | +| CLI-21 | Low | P2 | S | — | Done | Go `ClientVersion = "0.1.0-dev"` stale vs tagged releases | | CLI-22 | Low | — | S | — | Not started | Go `newCorrelationID` swallows `crypto/rand` error → empty id | | CLI-23 | Low | — | S | — | Not started | Go nil-vs-empty bulk short-circuit asymmetry | | CLI-24 | Low | — | S | — | Not started | Java `MxEventStream` single-consumer constraint undocumented | | CLI-25 | Low | — | S | — | Not started | Java `close()` does not await channel termination | -| CLI-26 | Low | P2 | S | — | Not started | Python `version.py` (0.1.0) ≠ `pyproject.toml` (0.1.2) | +| CLI-26 | Low | P2 | S | — | Done | Python `version.py` (0.1.0) ≠ `pyproject.toml` (0.1.2) | | CLI-27 | Low | — | S | — | Not started | Python `Session.close()` not concurrency-safe; synthesizes reply | | CLI-28 | Low | — | S | — | Not started | Python circular-import workaround at end of `session.py` | -| CLI-29 | Low | P2 | S | — | Not started | Rust `CLIENT_VERSION` (0.1.0-dev) ≠ `Cargo.toml` (0.1.2) | -| CLI-30 | Low | — | S | CLI-04 | Not started | Rust has no `unregister` typed helper | +| CLI-29 | Low | P2 | S | — | Done | Rust `CLIENT_VERSION` (0.1.0-dev) ≠ `Cargo.toml` (0.1.2) | +| CLI-30 | Low | — | S | CLI-04 | Done | Rust has no `unregister` typed helper | | CLI-31 | Low | — | M | — | Not started | Rust CLI is a single 2,699-line `main.rs` | | CLI-32 | Low | — | S | — | Not started | Client-side bulk caps differ (.NET/Java unbounded) | | CLI-33 | Low | — | S | CLI-01,13 | Not started | Per-language event backpressure semantics undocumented | @@ -213,19 +213,19 @@ Full design + implementation for each row lives in the linked domain doc under i | ID | Sev | Tier | Eff | Dep | Status | Title | |---|---|:-:|:-:|---|---|---| -| TST-01 | High | P2 | L | TST-04 | Not started | Reconnect/replay has no e2e test and no client consumer | +| TST-01 | High | P2 | L | TST-04 | Done | Reconnect/replay has no e2e test and no client consumer | | TST-02 | High | P0 | M | TST-04 | Done | Reconnect owner re-validation not implemented | | TST-03 | High | P1 | M | — | In review | No CI exists | -| TST-04 | High | P2 | L | — | Not started | Session-resilience epic 16/28 tasks unfinished | +| TST-04 | High | P2 | L | — | Done | Session-resilience epic 16/28 tasks unfinished | | TST-05 | Medium | P1 | S | TST-03 | Not started | Real-worker control/COM paths verified opt-in only | | TST-06 | Medium | — | M | — | Not started | Dashboard live-data path untested | | TST-07 | Medium | — | S | — | Not started | Real-clock sleeps with negative assertions are latent flakes | | TST-08 | Medium | P1 | M | — | Done | Full-suite orphaned testhost processes (does not reproduce; doc de-stale) | | TST-09 | Medium | — | M | — | Not started | Health checks cover only the auth store | | TST-10 | Medium | — | M | — | Not started | Deployment/upgrade undocumented and hand-rolled | -| TST-11 | Medium | P2 | M | — | Not started | No version discipline on server; client versions drift | +| TST-11 | Medium | P2 | M | — | Done | No version discipline on server; client versions drift | | TST-12 | Medium | P0 | S | — | Done | CLAUDE.md misstates default retention behaviour | -| TST-13 | Medium | P2 | S | — | Not started | gateway.md carries stale design-era sketches | +| TST-13 | Medium | P2 | S | — | Done | gateway.md carries stale design-era sketches | | TST-14 | Medium | P2 | S | — | Not started | Repo-root working artifacts need triage | | TST-15 | Medium | P2 | M | TST-04 | Not started | Dashboard EventsHub has no per-session ACL | | TST-16 | Medium | — | S | — | Not started | `Dashboard:ShowTagValues` is a dead flag | @@ -235,7 +235,7 @@ Full design + implementation for each row lives in the linked domain doc under i | TST-20 | Low | — | S | — | Not started | E2E script papers over a real advise-without-consumer sharp edge | | TST-21 | Low | — | S | — | Not started | Log rotation configured but minimal | | TST-22 | Low | — | S | — | Not started | Config-shape JSON block omits documented keys | -| TST-23 | Low | P2 | S | — | Not started | Bidirectional `Session` RPC never built | +| TST-23 | Low | P2 | S | — | Done | Bidirectional `Session` RPC never built | | TST-24 | Low | P2 | M | TST-03 | Not started | Client wire behaviour has no automated verification | ## Cross-cutting clusters @@ -253,8 +253,21 @@ Findings the review flagged as one coordinated design pass — sequence them tog | Date | Change | |---|---| +| 2026-07-09 | **P2 Epic wrap — user decision: DEFER TST-15 + TST-24, close the epic.** Epic bucket result: 5 of 7 findings `Done` (CLI-15, CLI-04, CLI-30, TST-01, TST-04); **TST-15** and **TST-24** intentionally deferred to a follow-up (kept `Not started`, not `Won't fix` — they are gated, not rejected). **TST-15** (dashboard EventsHub per-session ACL) is epic Phase 4 — a real feature needing a new session-"tag" mechanism + dashboard group→tag config, not a mechanical fix; the `EventsHub` `TODO(per-session-acl)` stays, and the already-shipped **SEC-25** mitigation (tag *values* redacted from the dashboard mirror by default) means no sensitive payload leaks through the hub today regardless of the missing ACL — so deferring carries no value-leak risk. **TST-24** (per-client wire tests) depends on **TST-03** (CI), which is `In review` (YAML authored, never run on a Gitea runner) — no point wiring client tests into a pipeline that isn't live yet. Net P2: 35/38 `Done`; remaining = TST-15 (deferred feature), TST-24 (deferred, CI-gated), TST-14 (user deletes their own untracked gitignored `*-docs-*.md` files). | +| 2026-07-09 | P2 Epic — **Java client completes CLI-15 + CLI-04 locally** (commit `1cc0fa4`); **CLI-15, CLI-04, CLI-30, TST-01 all → `Done` (5/5 clients + server e2e)**. Java CLI-15: `MxEventStreamItem` record + `MxEventStream.nextItem()` (`isReplayGap()`/`replayGap()`/`event()`); existing `Iterator` path unchanged, sentinel never swallowed. Java CLI-04: Phase 1 `adviseSupervisory`/`writeSecured`/`writeSecured2`/`authenticateUser`/`archestrAUserToId` + Phase 2 `addBufferedItem`/`setBufferedUpdateInterval`/`suspend`/`activate` (unregister already present) on `MxGatewaySession`, each through `invokeCommand` → `ensureProtocolSuccess`+`ensureMxAccessSuccess`; credentials scrubbed via `MxGatewaySecrets.redactCredentials` (tests assert absent from message/toString/CLI). `gradle test` 106/0 (58 client + 48 cli), no generated churn. Built locally with `JAVA_HOME=/opt/homebrew/opt/openjdk@17` — Java toolchain now works on the Mac (see prior note). Shared docs `ClientLibrariesDesign.md` + CLAUDE.md updated to "all five clients". **TST-01 → Done** (server e2e `fed0685` + all 5 client `ReplayGap` consumers). This closes session-resilience epic Phase 3 fully. | +| 2026-07-09 | P2 Epic Wave E2 — typed-command parity (commit `bde042b`). CLI-04 → `In progress` (4/5 clients; Java pending in the SAME session — see next note), CLI-30 → `Done`. Every parity-critical single-item command now has a typed session helper across .NET/Go/Rust/Python: Phase 1 `AdviseSupervisory`/`WriteSecured`/`WriteSecured2`/`AuthenticateUser`/`ArchestrAUserToId`, Phase 2 `AddBufferedItem`/`SetBufferedUpdateInterval`/`Suspend`/`Activate`, plus `Unregister` (CLI-30: Rust + .NET added; Go/Java/Python already had it — CLI-30 fully Done). Each wraps existing raw-command machinery (no new wire surface) and runs the client's MXAccess-level reply validation (`hresult < 0` + `MxStatusProxy`). MXAccess parity preserved (WriteSecured-before-authenticate surfaces the native failure). **Credentials route through each client's secret-redaction seam** (never in logs/exceptions/ToString/Debug/Display; each suite asserts a distinctive credential is absent from surfaced errors); new CLI subcommands take credentials via flag/env, never echoed. Verified per toolchain: .NET build clean 102 passed; Go gofmt/vet/build/test clean; Rust fmt/check/test/clippy clean; Python 145 passed. Shared doc: `ClientLibrariesDesign.md` "Typed Command Parity" section. | +| 2026-07-09 | **Java client toolchain now works locally on the Mac** (user-flagged + verified): homebrew `openjdk@17` (17.0.19), @21, @26 are installed (off PATH); `JAVA_HOME=/opt/homebrew/opt/openjdk@17 gradle test` → BUILD SUCCESSFUL. Retires the "Java client must be built on windev" constraint — the remaining Java halves of CLI-15 + CLI-04 are done locally this session (no windev batch). Memory `project_java_build_host` rewritten. | +| 2026-07-09 | P2 Epic — TST-04 (session-resilience epic governance) → `Done` (commit `ed3c6c6`, docs-only umbrella). Resolved the epic's shipped-vs-planned entanglement into three decisions: **Phase 3 finished** (Task 13=TST-02 owner-scoped attach P0, Task 15=TST-01 reconnect e2e, Task 14=CLI-15 4/5 clients w/ Java pending); **Phase 4 scoped** as TST-15 with the Viewer-default decision settled (admin-sees-all, Viewer strict per owned/granted session, matching TST-02's gRPC owner binding); **Phase 5 (orphan-worker reattach) DEFERRED, not planned** — the CLAUDE.md "gateway restart does not reattach orphan workers" invariant stands and the `EnableOrphanReattach` flag does not exist / must not be referenced. Updated `oldtasks.md`, the `tasks.json` mirror (per-task statuses + governance note), and the CLAUDE.md reconnect paragraph (clients consume ReplayGap; reattach deferred). The actionable slices carry their own verification: TST-01 (done), TST-02 (done, P0), TST-15 (pending, E3). | +| 2026-07-09 | P2 Epic Wave E1 — reconnect-replay: CLI-15 + TST-01 → `In progress` (both gated on the Java client, deferred to the windev batch). **CLI-15 (commit `0c6e5b3`)**: four clients now surface the gateway's `ReplayGap` reconnect sentinel as a distinct, typed, non-terminal signal (never synthesized, never swallowed) — .NET `MxEventStreamItem`/`StreamEventItemsAsync` (build clean, 87 passed), Go `EventResult.ReplayGap`+`IsReplayGap()` (build/vet/test clean), Rust `EventItem::ReplayGap` enum (`EventStream` now yields `Result`; fmt/check/test/clippy clean), Python `ReplayGap` dataclass yielded from `stream_events` (131 passed). Shared docs: `ClientLibrariesDesign.md` non-goals reframed (reconnect-replay protocol is consumable; auto-reconnect stays a non-goal) + `CrossLanguageSmokeMatrix.md` resume-gap note. Resume contract uniform: `after_worker_sequence = oldest_available_sequence - 1`. **TST-01 server half (commit `fed0685`)**: `GatewayEndToEndReconnectReplayTests` (2 facts) proves the default-on reconnect protocol end-to-end over the real gRPC StreamEvents path via the fake worker — replay-tail-no-gap (capacity 16, mid-batch cursor) and stale-cursor-emits-ReplayGap-first (capacity 3, 6 events force eviction, cursor=1); single-subscriber detach→reconnect, ring survives detach. macOS-verified 2/2 (needs `TMPDIR=/tmp` — default macOS TMPDIR pushes the CoreFxPipe Unix-socket path past the 104-byte `sun_path` limit; Windows CI unaffected). No product bugs; server behavior matched the contract exactly. Remaining to close both: Java `ReplayGap` surface (windev) + Java client consumer for TST-01. | +| 2026-07-09 | P2 Wave B — worker event hot-path (commit `f61c816`), **windev-verified**. WRK-06 → `Done`: `MxStatusProxyConverter` caches the four resolved `FieldInfo` per status type in a static `ConcurrentDictionary` (the `GetField` metadata scan ran 4× per status per event on the STA path); `GetValue`+`Convert.ToInt32` still run per event (late-bound RCW); both exception messages byte-identical (missing-field via `ResolveField`, not cached on throw through `GetOrAdd`; null-value unchanged). WRK-11 → `Done`: `MxAccessEventQueue.Enqueue` takes ownership of the passed `MxEvent` (stamps `WorkerSequence`/`WorkerTimestamp` in place, no `Clone()`); audited all 3 callers (base/alarm event sinks + provider-mode handler) — each builds a fresh event, none reuse it; `MxAccessValueCache.Set` now deep-copies its retained `Value`/`SourceTimestamp`/`Statuses` so the cache snapshot never aliases the queue-owned (later serialized) event. WRK-12 → `Done`: `WorkerFrameWriter` coalesces the flush across a drained batch (write each frame, one `FlushAsync` after the batch, then complete all) — preserves the written+flushed completion contract; a burst of N events costs 1 flush not N; on failure the in-flight batch + queue all fail so no caller hangs. IPC-15 → `Done`: the multi-event `WorkerEnvelope` body stays unimplemented (wire still one event per `worker_event` frame); gateway.md Performance section now distinguishes the shipped flush-coalescing from that deferred additive-proto change. **Windev:** x86 Worker builds clean (fixed a Windows-only `TreatWarningsAsErrors` CS8604 in the value-cache null-guard that macOS can't surface); Worker.Tests **356 passed / 0 failed / 11 skipped** (+4 new: converter cache-reuse, queue ownership-transfer, value-cache snapshot independence, writer batch-flush-once); gateway Tests **815 passed / 1 failed** — the 1 is the known windev-environmental SelfSigned-SAN assertion (passes on macOS), and all pipe-harness tests (which also exercise the earlier G-frame gateway frame change + WRK-12 end-to-end over a real pipe) pass. | +| 2026-07-09 | P2 Wave B — quick Mac items (commit `ec6f82b`). TST-11 → `Done`: `src/Directory.Build.props` single-sources the .NET-side version (Server/Worker/Contracts/tests stamped 0.1.2, was SDK-default 1.0.0) and appends the git short SHA to InformationalVersion (`0.1.2+`, guarded so a git-less build still works); verified Server assembly stamps `0.1.2+579282f`, Server+Tests build clean. Kept 0.1.2 (matches Contracts + aligned Python/Rust/Go); converging all to a single 0.2.0 cadence left as a release decision. WRK-15 → `Done` (docs-only path, no x86 build): STA thread-name refs corrected to the actual `MxGateway.Worker.STA` in `docs/WorkerSta.md` + `docs/MxAccessWorkerInstanceDesign.md`, and the stale heartbeat-counter note fixed (CaptureHeartbeat populates queue depth + sequence from the live queue). TST-23 → `Done`: already resolved by the Wave A gateway.md edit — the bidi `Session` RPC now sits under a "Future work: not implemented" heading (no "best long-term shape" framing), consistent with the proto. **TST-14 left open by design**: its only concrete step is deleting the user's untracked, gitignored `*-docs-*.md` working files (zero repo impact) — surfaced to the user rather than deleting files not created here. | +| 2026-07-09 | P2 Wave B — dashboard/security (commit `e15c3cb`). SEC-25 → `Done` (near-term hardening only; full per-session EventsHub ACL stays deferred to roadmap item 12 / TST-15): `DashboardEventBroadcaster` redacts tag values from a deep clone of the event before mirroring to SignalR when `Dashboard:ShowTagValues` is false (default), so the hub seam cannot leak values regardless of the missing ACL. Clears `MxEvent.value` (the OnDataChange/OnWriteComplete/OperationComplete/OnBufferedDataChange bodies are empty discriminators — values ride in the top-level field, incl. buffered arrays) + the alarm body `current_value`/`limit_value`; source event never mutated (shared with gRPC path + replay ring; verified). Makes the formerly-dead `ShowTagValues` flag live for the mirror. `EventsHub` TODO(per-session-acl) kept + tied to roadmap item 12. Test `DashboardEventBroadcasterTests` (3). SEC-30 → `Done`: `docs/Diagnostics.md` trimmed to mark opt-in command-value logging NOT YET IMPLEMENTED (no `LogCommandValues` knob, `RedactCommandValue` unwired, no values logged); wiring deferred pending secured-bulk redaction (SEC-13), not wired now. Server build clean; Dashboard tests 152/152. | +| 2026-07-09 | P2 Wave B — gateway event/frame hot-path performance (8 findings). **Frame I/O (commit `baae59c`):** GWC-08/IPC-13 gateway `WorkerFrameWriter` serializes once into one ArrayPool-rented buffer (LE length prefix + payload) and issues a single write (was: second serialization via `ToByteArray`, separate prefix array, two writes); IPC-14 `WorkerFrameReader` rents the payload buffer from ArrayPool instead of `new byte[]` per frame, read/parse length-bounded, returned in finally. Wire format byte-identical, cross-checked against the worker writer/reader. **Event/command hot path (commit `5e2e40a`):** GWC-06 `StreamEvents` timing via `Stopwatch.GetTimestamp()`/`GetElapsedTime()` (no per-event Stopwatch alloc); GWC-07/IPC-05 `MapEvent` transfers ownership of the inner `MxEvent` instead of cloning (safe — single-consumer `WorkerEvent`, MapEvent runs once pre-fan-out, all downstream consumers read-only; verified no post-mapping mutation) and `CreateCommandEnvelope` drops its redundant second clone (`MapCommand` already isolated the graph); every load-bearing isolation clone kept; GWC-15 `grpc_stream_queue.depth` converted from a per-event push counter to an `ObservableGauge` summing registered channel sources only at scrape time (name/semantics unchanged). **Replay ring (commit `cf1b3d4`):** GWC-14 replaces the `LinkedList` (node alloc per retained event) with a preallocated circular `ReplayEntry[]` (head+count), preserving ascending order, capacity eviction, time trim, oldest-read/ReplayGap math, both query paths, and the lock. → all `Done`. Server build clean (0 warnings); per-cluster tests green (WorkerFrameProtocol 10/10 incl. new near-cap round-trip, EventStream/Metrics/Distributor/Mapper 62/62, Distributor/Replay 33/33 incl. 2 new wrap-around cases). Full-suite macOS checkpoint: 769 passed / 44 failed, all 44 the known named-pipe/fake-worker-harness UDS-104-char limit (0 failures in any touched area). **Pending: windev run** to exercise the frame I/O over a real pipe (the 44 pipe-harness tests are macOS-blind). | +| 2026-07-09 | P2 Wave B — SEC-03 (dashboard `__Host-` cookie prefix) → `Done` (commit `7d42c85`). Conditionally restore the prefix instead of a pure doc fix: `DashboardServiceCollectionExtensions` PostConfigure resolves the cookie name as explicit `MxGateway:Dashboard:CookieName` override → else `__Host-MxGatewayDashboard` when `SecurePolicy==Always` (`RequireHttpsCookie` true) → else the plain `MxGatewayDashboard` default; guard never applies `__Host-` without Secure (browsers drop it). `DashboardAuthenticationDefaults.SecureCookieName` added; plain name kept as the non-secure fallback. Five stale docs corrected to the conditional contract (gateway.md, GatewayProcessDesign, ImplementationPlanGateway, GatewayDashboardDesign, CLAUDE.md; GatewayConfiguration phrasing tightened). Test `DashboardCookieOptionsTests` asserts the name flips with `RequireHttpsCookie` and that an override wins. Server build clean; Dashboard tests 149/149. | +| 2026-07-09 | P2 Wave A (doc-drift sweep + client version alignment) via parallel agents, on `fix/archreview-p2` off `main`. **Version alignment (commit `4fe1917`):** CLI-18 (.NET `0.1.2`), CLI-21 (Go `ClientVersion` 0.1.0-dev→0.1.2), CLI-26 (Python `__version__`→0.1.2), CLI-29 (Rust `CLIENT_VERSION`→`env!(CARGO_PKG_VERSION)`) → `Done`; verified dotnet build + `go build`/`test` + `cargo check`/`test`/`clippy` + pytest 127 pass. **Doc-drift (commit `06e1046`):** IPC-06/07/21 + TST-13 (gateway.md envelope sketch → proto-as-truth, six→seven RPCs incl. `QueryActiveAlarms` in `docs/Grpc.md`, `Session` RPC marked not-implemented, stale sketches removed), SEC-09 (dashboard `GroupToRole` sample `Administrator` so it passes the validator), SEC-22 (`docs/Authentication.md` rewritten to the shipped `ZB.MOM.WW.Auth.ApiKeys`-package pipeline; 18 stale type names removed, grep-verified absent; named gateway types re-verified present), IPC-17 (wrong Python gen dir `mxgateway`→`zb_mom_ww_mxgateway` in CLAUDE.md + 3 docs), CLI-12 (Java 21→17), CLI-16 (`docs/ClientPackaging.md` reconciled with `.slnx`/Python package name/gradle project names) → `Done`. Docs-only claims verified against source. 13 findings closed. | | 2026-07-09 | Initial tracking doc generated from the six domain remediation designs. All 153 findings `Not started`. | | 2026-07-09 | P0 tier executed via parallel agents. GWC-01/02/03, TST-02, TST-12, CLI-01, CLI-03 → `Done` (NonWindows build clean; gateway 135/135 targeted tests pass, Go `go test ./...` + `-race` pass, Rust `test`/`clippy -D warnings` pass). | +| 2026-07-09 | P0 tier executed via parallel agents. GWC-01/02/03, TST-02, TST-12, CLI-01, CLI-03 → `Done` (NonWindows build clean; gateway 135/135 targeted tests pass, Go `go test ./...` + `-race` pass, Rust `test`/`clippy -D warnings` pass). | | 2026-07-09 | Windows full-suite temp-file-lock flakiness (discovered via TST-08) → **fixed** (commit `11a716a`). Two shared-state parallel collisions that macOS never surfaces: (1) self-signed cert generation — `SelfSignedCertificateProvider` now stages the PFX in a unique `..tmp` (was a fixed name that concurrent/interrupted writers collided on); `TestHostEnvironmentInitializer` defaults the cert path to a per-process temp dir (so the suite never writes shared `ProgramData` or fights the deployed service); `GatewayTlsBootstrapTests` moved to a `DisableParallelization` collection so its process-global env-var mutation can't bleed into parallel tests. (2) `AuthStoreHealthCheckTests` — reuse the existing `TempDatabaseDirectory` helper (`SqliteConnection.ClearAllPools()` before delete) so the Microsoft.Data.Sqlite pool releases the temp `.db` handle. **windev-verified:** 3 consecutive full-suite runs all **793 passed / 2 failed, 0 new orphans** (was flaky 2–4 fails, count-varying). The 2 stable remaining failures are pre-existing windev-environmental (`SelfSignedCertificateProviderTests.GenerateCertificate_HasExpectedSansEkuAndValidity` machine-name/FQDN SAN assertion; `EventStreamServiceTests…TracksAggregateQueueDepth` timing) — both pass on macOS, unrelated to this change. | | 2026-07-09 | P1 TST-08 (orphaned testhost) → `Done` by evidence: the claimed leak does **not** reproduce. Full `ZB.MOM.WW.MxGateway.Tests` suite exits cleanly on macOS (0 survivors) and on the Windows dev box (windev, `origin/main` worktree: 0 new `testhost` and 0 new `ZB.MOM.WW.MxGateway.Worker` processes after a full run, isolated by StartTime). Static audit confirms the prime-suspect fixtures already dispose deterministically (WorkerClient cancels its `_stopCts` + awaits read/write/heartbeat tasks with a 5 s timeout; GatewaySession disposes the client; E2E/harness fixtures use `await using`). CLAUDE.md "Source Update Workflow" updated to drop the stale "leaves orphaned testhost processes" rationale (filtered runs kept as a speed guideline). **Discovered separately (NOT TST-08, still open):** Windows-only temp-file-lock flakiness in ~4 full-host test classes — `AuthStoreHealthCheckTests` (Microsoft.Data.Sqlite connection-pool holds the temp `.db` handle after `await using`, so the finally `File.Delete` throws "used by another process"), `GatewayTlsBootstrapTests`/`DashboardCookieOptionsTests`/`DashboardHubsRegistrationTests` (a `gw.pfx.tmp` handle from `SelfSignedCertificateProvider`'s fixed-name `path+".tmp"` atomic-write racing teardown). Count varies run-to-run (2–4 fails); macOS never sees it (Unix allows deleting open files). Fix = `SqliteConnection.ClearAllPools()` before temp delete + a unique/guarded `.tmp` name; verify on windev. | | 2026-07-09 | SEC-10 polish (gateway-side follow-up to the G-2 auth-lib core) → done. `apikey create-key` gains an optional `--expires` (absolute ISO-8601 UTC or relative `d`/`h`; omit = non-expiring, opt-in) threaded through `ApiKeyAdminCommand`/parser/runner into the library's `CreateKeyAsync(..., expiresUtc, ...)`; `list-keys` shows an expiry column + `active`/`expired`/`revoked` status. Dashboard API Keys page surfaces expiry: an `Expires` column (`Never` when unset) and a status badge reading `Expired` (red) / `Expiring` (≤7 days, amber) / `Revoked` / `Active` (`DashboardApiKeySummary.ExpiresUtc` projected in `DashboardSnapshotService`; `StatusBadge` maps the new states). Server build clean; targeted tests 32/32 (parser abs/relative/invalid + end-to-end past-expiry rejection via the live verifier) + dashboard 28/28. Docs: `docs/Authentication.md` (verification flow expiry step, CLI table + examples, dashboard badge). SEC-10 was already `Done` (core); this closes the tracked polish. | diff --git a/clients/dotnet/README.md b/clients/dotnet/README.md index 71ecb15..ebdd94c 100644 --- a/clients/dotnet/README.md +++ b/clients/dotnet/README.md @@ -84,6 +84,48 @@ messages. `MxGatewaySession.OpenSessionReply` keeps the raw session-open reply available, and command helpers have `*RawAsync` variants when callers need the complete `MxCommandReply`. +### Event Streaming And Replay Gaps + +`StreamEventsAsync(afterWorkerSequence)` yields raw generated `MxEvent` +messages. Passing a non-zero `afterWorkerSequence` resumes a session's event +stream after a known worker sequence — this is the reconnect cursor. If that +cursor is *stale* — older than the oldest event the gateway still retains in the +session replay ring — the events in between were evicted and cannot be replayed. +The gateway signals this by emitting a single **replay-gap sentinel** at the head +of the resumed stream: an `MxEvent` with its `ReplayGap` field set, `Family` +unspecified, and no body. It means "you missed events — discard local state and +re-snapshot." + +Rather than force callers to inspect the raw sentinel, the client exposes a +typed surface, `StreamEventItemsAsync`, which yields `MxEventStreamItem` values: + +```csharp +await foreach (MxEventStreamItem item in session.StreamEventItemsAsync( + afterWorkerSequence: lastSeenSequence)) +{ + if (item.IsReplayGap) + { + // We missed events: throw away local state and re-snapshot. + ReplayGap gap = item.ReplayGap!; + // Resume without incurring another gap: + lastSeenSequence = gap.OldestAvailableSequence - 1; + await ReSnapshotAsync(); + continue; + } + + HandleEvent(item.Event); // normal MXAccess event, IsReplayGap == false + lastSeenSequence = item.Event.WorkerSequence; +} +``` + +The typed surface never synthesizes or drops events — it only makes the +gateway's own sentinel observable. Normal events pass through with +`IsReplayGap == false` and `ReplayGap == null`. The gap is only ever produced by +`StreamEvents`; the diagnostic drain path never emits it. If you already consume +the raw `StreamEventsAsync` (or the client-level stream), the +`AsStreamItemsAsync()` extension projects any `IAsyncEnumerable` into +the same `MxEventStreamItem` surface. + For alarms, the client exposes `QueryActiveAlarmsAsync` (one-shot snapshot of the active alarms the gateway's central monitor currently holds), `StreamAlarmsAsync` (server-streaming feed of alarm-state-change messages diff --git a/clients/dotnet/ZB.MOM.WW.MxGateway.Client.Cli/MxGatewayCliSecretRedactor.cs b/clients/dotnet/ZB.MOM.WW.MxGateway.Client.Cli/MxGatewayCliSecretRedactor.cs index d4e3003..7119c85 100644 --- a/clients/dotnet/ZB.MOM.WW.MxGateway.Client.Cli/MxGatewayCliSecretRedactor.cs +++ b/clients/dotnet/ZB.MOM.WW.MxGateway.Client.Cli/MxGatewayCliSecretRedactor.cs @@ -1,18 +1,31 @@ namespace ZB.MOM.WW.MxGateway.Client.Cli; -/// Utility to redact API keys from error messages for safe output. +/// Utility to redact secrets (API keys, MXAccess credentials) from error messages for safe output. internal static class MxGatewayCliSecretRedactor { - /// Replaces occurrences of the API key in the value with a redacted placeholder. + /// + /// Replaces every occurrence of any supplied secret in the value with a + /// redacted placeholder. Null or empty secrets are ignored, so callers can + /// pass optional credentials without pre-filtering. + /// /// The message text to redact. - /// The API key to remove; no redaction if null or empty. - public static string Redact(string value, string? apiKey) + /// The secret values to remove (API key, verify-user password, secured payloads). + public static string Redact(string value, params string?[] secrets) { - if (string.IsNullOrEmpty(value) || string.IsNullOrEmpty(apiKey)) + if (string.IsNullOrEmpty(value) || secrets is null) { return value; } - return value.Replace(apiKey, "[redacted]", StringComparison.Ordinal); + string redacted = value; + foreach (string? secret in secrets) + { + if (!string.IsNullOrEmpty(secret)) + { + redacted = redacted.Replace(secret, "[redacted]", StringComparison.Ordinal); + } + } + + return redacted; } } diff --git a/clients/dotnet/ZB.MOM.WW.MxGateway.Client.Cli/MxGatewayClientCli.cs b/clients/dotnet/ZB.MOM.WW.MxGateway.Client.Cli/MxGatewayClientCli.cs index e8d56a8..bb7250d 100644 --- a/clients/dotnet/ZB.MOM.WW.MxGateway.Client.Cli/MxGatewayClientCli.cs +++ b/clients/dotnet/ZB.MOM.WW.MxGateway.Client.Cli/MxGatewayClientCli.cs @@ -112,6 +112,24 @@ public static class MxGatewayClientCli .ConfigureAwait(false), "advise-supervisory" => await AdviseSupervisoryAsync(arguments, client, standardOutput, cancellation.Token) .ConfigureAwait(false), + "unregister" => await UnregisterAsync(arguments, client, standardOutput, cancellation.Token) + .ConfigureAwait(false), + "add-buffered-item" => await AddBufferedItemAsync(arguments, client, standardOutput, cancellation.Token) + .ConfigureAwait(false), + "set-buffered-update-interval" => await SetBufferedUpdateIntervalAsync(arguments, client, standardOutput, cancellation.Token) + .ConfigureAwait(false), + "suspend" => await SuspendAsync(arguments, client, standardOutput, cancellation.Token) + .ConfigureAwait(false), + "activate" => await ActivateAsync(arguments, client, standardOutput, cancellation.Token) + .ConfigureAwait(false), + "write-secured" => await WriteSecuredAsync(arguments, client, standardOutput, cancellation.Token) + .ConfigureAwait(false), + "write-secured2" => await WriteSecured2Async(arguments, client, standardOutput, cancellation.Token) + .ConfigureAwait(false), + "authenticate-user" => await AuthenticateUserAsync(arguments, client, standardOutput, cancellation.Token) + .ConfigureAwait(false), + "archestra-user-to-id" => await ArchestraUserToIdAsync(arguments, client, standardOutput, cancellation.Token) + .ConfigureAwait(false), "subscribe-bulk" => await SubscribeBulkAsync(arguments, client, standardOutput, cancellation.Token) .ConfigureAwait(false), "unsubscribe-bulk" => await UnsubscribeBulkAsync(arguments, client, standardOutput, cancellation.Token) @@ -157,9 +175,15 @@ public static class MxGatewayClientCli { // Client.Dotnet-028: redact the *effective* key — from --api-key or the // --api-key-env environment variable — so an env-var-sourced key echoed - // in a transport error never reaches stderr unredacted. + // in a transport error never reaches stderr unredacted. CLI-04: also + // redact MXAccess credentials (AuthenticateUser password, WriteSecured + // payloads) that could otherwise be echoed back in a surfaced error. string? apiKey = TryResolveApiKey(arguments); - string message = MxGatewayCliSecretRedactor.Redact(exception.Message, apiKey); + string message = MxGatewayCliSecretRedactor.Redact( + exception.Message, + apiKey, + TryResolveVerifyUserPassword(arguments), + arguments.GetOptional("value")); if (forceJsonErrors || arguments.HasFlag("json")) { @@ -319,6 +343,48 @@ public static class MxGatewayClientCli return Environment.GetEnvironmentVariable(apiKeyEnvironmentName); } + /// + /// Resolves the effective MXAccess verify-user credential from + /// --verify-user-password or, failing that, the + /// --verify-user-password-env-named environment variable (default + /// MXGATEWAY_VERIFY_USER_PASSWORD). The credential is never echoed; + /// this resolver exists so the error-redaction catch block can strip it + /// from any surfaced error (CLI-04), mirroring . + /// + private static string? TryResolveVerifyUserPassword(CliArguments arguments) + { + string? password = arguments.GetOptional("verify-user-password"); + if (!string.IsNullOrEmpty(password)) + { + return password; + } + + string passwordEnvironmentName = arguments.GetOptional("verify-user-password-env") + ?? "MXGATEWAY_VERIFY_USER_PASSWORD"; + + return Environment.GetEnvironmentVariable(passwordEnvironmentName); + } + + /// + /// Resolves the verify-user credential for authenticate-user, throwing + /// a redaction-safe error when neither the flag nor the env var is set. The + /// thrown message names only the option/env var, never the value. + /// + private static string ResolveVerifyUserPassword(CliArguments arguments) + { + string? password = TryResolveVerifyUserPassword(arguments); + if (!string.IsNullOrEmpty(password)) + { + return password; + } + + string passwordEnvironmentName = arguments.GetOptional("verify-user-password-env") + ?? "MXGATEWAY_VERIFY_USER_PASSWORD"; + + throw new ArgumentException( + $"Verify-user password is required. Pass --verify-user-password or set {passwordEnvironmentName}."); + } + private static CancellationTokenSource CreateCancellation(CliArguments arguments, string command) { var cancellation = new CancellationTokenSource(); @@ -475,6 +541,215 @@ public static class MxGatewayClientCli cancellationToken); } + private static Task UnregisterAsync( + CliArguments arguments, + IMxGatewayCliClient client, + TextWriter output, + CancellationToken cancellationToken) + { + return InvokeAndWriteAsync( + arguments, + client, + output, + new MxCommand + { + Kind = MxCommandKind.Unregister, + Unregister = new UnregisterCommand + { + ServerHandle = arguments.GetInt32("server-handle"), + }, + }, + cancellationToken); + } + + private static Task AddBufferedItemAsync( + CliArguments arguments, + IMxGatewayCliClient client, + TextWriter output, + CancellationToken cancellationToken) + { + return InvokeAndWriteAsync( + arguments, + client, + output, + new MxCommand + { + Kind = MxCommandKind.AddBufferedItem, + AddBufferedItem = new AddBufferedItemCommand + { + ServerHandle = arguments.GetInt32("server-handle"), + ItemDefinition = arguments.GetRequired("item"), + ItemContext = arguments.GetOptional("item-context") ?? string.Empty, + }, + }, + cancellationToken); + } + + private static Task SetBufferedUpdateIntervalAsync( + CliArguments arguments, + IMxGatewayCliClient client, + TextWriter output, + CancellationToken cancellationToken) + { + return InvokeAndWriteAsync( + arguments, + client, + output, + new MxCommand + { + Kind = MxCommandKind.SetBufferedUpdateInterval, + SetBufferedUpdateInterval = new SetBufferedUpdateIntervalCommand + { + ServerHandle = arguments.GetInt32("server-handle"), + UpdateIntervalMilliseconds = arguments.GetInt32("interval-ms"), + }, + }, + cancellationToken); + } + + private static Task SuspendAsync( + CliArguments arguments, + IMxGatewayCliClient client, + TextWriter output, + CancellationToken cancellationToken) + { + return InvokeAndWriteAsync( + arguments, + client, + output, + new MxCommand + { + Kind = MxCommandKind.Suspend, + Suspend = new SuspendCommand + { + ServerHandle = arguments.GetInt32("server-handle"), + ItemHandle = arguments.GetInt32("item-handle"), + }, + }, + cancellationToken); + } + + private static Task ActivateAsync( + CliArguments arguments, + IMxGatewayCliClient client, + TextWriter output, + CancellationToken cancellationToken) + { + return InvokeAndWriteAsync( + arguments, + client, + output, + new MxCommand + { + Kind = MxCommandKind.Activate, + Activate = new ActivateCommand + { + ServerHandle = arguments.GetInt32("server-handle"), + ItemHandle = arguments.GetInt32("item-handle"), + }, + }, + cancellationToken); + } + + private static Task WriteSecuredAsync( + CliArguments arguments, + IMxGatewayCliClient client, + TextWriter output, + CancellationToken cancellationToken) + { + return InvokeAndWriteAsync( + arguments, + client, + output, + new MxCommand + { + Kind = MxCommandKind.WriteSecured, + WriteSecured = new WriteSecuredCommand + { + ServerHandle = arguments.GetInt32("server-handle"), + ItemHandle = arguments.GetInt32("item-handle"), + CurrentUserId = arguments.GetInt32("current-user-id"), + VerifierUserId = arguments.GetInt32("verifier-user-id", 0), + Value = ParseValue(arguments), + }, + }, + cancellationToken); + } + + private static Task WriteSecured2Async( + CliArguments arguments, + IMxGatewayCliClient client, + TextWriter output, + CancellationToken cancellationToken) + { + return InvokeAndWriteAsync( + arguments, + client, + output, + new MxCommand + { + Kind = MxCommandKind.WriteSecured2, + WriteSecured2 = new WriteSecured2Command + { + ServerHandle = arguments.GetInt32("server-handle"), + ItemHandle = arguments.GetInt32("item-handle"), + CurrentUserId = arguments.GetInt32("current-user-id"), + VerifierUserId = arguments.GetInt32("verifier-user-id", 0), + Value = ParseValue(arguments), + TimestampValue = ParseTimestampValue(arguments), + }, + }, + cancellationToken); + } + + private static Task AuthenticateUserAsync( + CliArguments arguments, + IMxGatewayCliClient client, + TextWriter output, + CancellationToken cancellationToken) + { + // The credential is resolved from --verify-user-password or its env var and + // is never echoed. On any surfaced error the RunCoreAsync catch block routes + // it through MxGatewayCliSecretRedactor so it cannot reach stderr (CLI-04). + return InvokeAndWriteAsync( + arguments, + client, + output, + new MxCommand + { + Kind = MxCommandKind.AuthenticateUser, + AuthenticateUser = new AuthenticateUserCommand + { + ServerHandle = arguments.GetInt32("server-handle"), + VerifyUser = arguments.GetRequired("verify-user"), + VerifyUserPassword = ResolveVerifyUserPassword(arguments), + }, + }, + cancellationToken); + } + + private static Task ArchestraUserToIdAsync( + CliArguments arguments, + IMxGatewayCliClient client, + TextWriter output, + CancellationToken cancellationToken) + { + return InvokeAndWriteAsync( + arguments, + client, + output, + new MxCommand + { + Kind = MxCommandKind.ArchestraUserToId, + ArchestraUserToId = new ArchestrAUserToIdCommand + { + ServerHandle = arguments.GetInt32("server-handle"), + UserIdGuid = arguments.GetRequired("user-guid"), + }, + }, + cancellationToken); + } + private static Task SubscribeBulkAsync( CliArguments arguments, IMxGatewayCliClient client, @@ -2029,6 +2304,15 @@ public static class MxGatewayClientCli or "add-item" or "advise" or "advise-supervisory" + or "unregister" + or "add-buffered-item" + or "set-buffered-update-interval" + or "suspend" + or "activate" + or "write-secured" + or "write-secured2" + or "authenticate-user" + or "archestra-user-to-id" or "subscribe-bulk" or "unsubscribe-bulk" or "read-bulk" @@ -2092,6 +2376,15 @@ public static class MxGatewayClientCli writer.WriteLine("mxgw-dotnet add-item --session-id --server-handle --item [--json]"); writer.WriteLine("mxgw-dotnet advise --session-id --server-handle --item-handle [--json]"); writer.WriteLine("mxgw-dotnet advise-supervisory --session-id --server-handle --item-handle [--json]"); + writer.WriteLine("mxgw-dotnet unregister --session-id --server-handle [--json]"); + writer.WriteLine("mxgw-dotnet add-buffered-item --session-id --server-handle --item [--item-context ] [--json]"); + writer.WriteLine("mxgw-dotnet set-buffered-update-interval --session-id --server-handle --interval-ms [--json]"); + writer.WriteLine("mxgw-dotnet suspend --session-id --server-handle --item-handle [--json]"); + writer.WriteLine("mxgw-dotnet activate --session-id --server-handle --item-handle [--json]"); + writer.WriteLine("mxgw-dotnet write-secured --session-id --server-handle --item-handle --type --value --current-user-id [--verifier-user-id ] [--json]"); + writer.WriteLine("mxgw-dotnet write-secured2 --session-id --server-handle --item-handle --type --value --current-user-id [--verifier-user-id ] [--timestamp ] [--json]"); + writer.WriteLine("mxgw-dotnet authenticate-user --session-id --server-handle --verify-user (--verify-user-password | --verify-user-password-env ) [--json]"); + writer.WriteLine("mxgw-dotnet archestra-user-to-id --session-id --server-handle --user-guid [--json]"); writer.WriteLine("mxgw-dotnet subscribe-bulk --session-id --server-handle --items [--json]"); writer.WriteLine("mxgw-dotnet unsubscribe-bulk --session-id --server-handle --item-handles [--json]"); writer.WriteLine("mxgw-dotnet read-bulk --session-id --server-handle --items [--timeout-ms ] [--json]"); diff --git a/clients/dotnet/ZB.MOM.WW.MxGateway.Client.Tests/MxGatewayClientCliTests.cs b/clients/dotnet/ZB.MOM.WW.MxGateway.Client.Tests/MxGatewayClientCliTests.cs index bd39120..54642a7 100644 --- a/clients/dotnet/ZB.MOM.WW.MxGateway.Client.Tests/MxGatewayClientCliTests.cs +++ b/clients/dotnet/ZB.MOM.WW.MxGateway.Client.Tests/MxGatewayClientCliTests.cs @@ -129,6 +129,120 @@ public sealed class MxGatewayClientCliTests Assert.Equal(string.Empty, error.ToString()); } + /// Verifies that write-secured builds a WriteSecured command with the value and user ids. + [Fact] + public async Task RunAsync_WriteSecured_BuildsWriteSecuredCommand() + { + using var output = new StringWriter(); + using var error = new StringWriter(); + FakeCliClient fakeClient = new(); + fakeClient.InvokeReplies.Enqueue(new MxCommandReply + { + SessionId = "session-fixture", + Kind = MxCommandKind.WriteSecured, + ProtocolStatus = new ProtocolStatus { Code = ProtocolStatusCode.Ok }, + }); + + int exitCode = await MxGatewayClientCli.RunAsync( + [ + "write-secured", + "--endpoint", "http://localhost:5000", + "--api-key", "test-api-key", + "--session-id", "session-fixture", + "--server-handle", "12", + "--item-handle", "34", + "--type", "int32", + "--value", "123", + "--current-user-id", "5", + "--verifier-user-id", "6", + "--json", + ], + output, + error, + _ => fakeClient); + + Assert.Equal(0, exitCode); + MxCommandRequest request = Assert.Single(fakeClient.InvokeRequests); + Assert.Equal(MxCommandKind.WriteSecured, request.Command.Kind); + Assert.Equal(123, request.Command.WriteSecured.Value.Int32Value); + Assert.Equal(5, request.Command.WriteSecured.CurrentUserId); + Assert.Equal(6, request.Command.WriteSecured.VerifierUserId); + Assert.Equal(string.Empty, error.ToString()); + } + + /// + /// Verifies that authenticate-user builds an AuthenticateUser command sourcing the + /// credential from the flag, and that the credential never appears in stdout/stderr. + /// + [Fact] + public async Task RunAsync_AuthenticateUser_BuildsCommandAndDoesNotEchoCredential() + { + const string password = "cli-secret-credential-987"; + using var output = new StringWriter(); + using var error = new StringWriter(); + FakeCliClient fakeClient = new(); + fakeClient.InvokeReplies.Enqueue(new MxCommandReply + { + SessionId = "session-fixture", + Kind = MxCommandKind.AuthenticateUser, + ProtocolStatus = new ProtocolStatus { Code = ProtocolStatusCode.Ok }, + AuthenticateUser = new AuthenticateUserReply { UserId = 4242 }, + }); + + int exitCode = await MxGatewayClientCli.RunAsync( + [ + "authenticate-user", + "--endpoint", "http://localhost:5000", + "--api-key", "test-api-key", + "--session-id", "session-fixture", + "--server-handle", "12", + "--verify-user", "operator", + "--verify-user-password", password, + "--json", + ], + output, + error, + _ => fakeClient); + + Assert.Equal(0, exitCode); + MxCommandRequest request = Assert.Single(fakeClient.InvokeRequests); + Assert.Equal(MxCommandKind.AuthenticateUser, request.Command.Kind); + Assert.Equal("operator", request.Command.AuthenticateUser.VerifyUser); + Assert.Equal(password, request.Command.AuthenticateUser.VerifyUserPassword); + Assert.DoesNotContain(password, output.ToString()); + Assert.DoesNotContain(password, error.ToString()); + } + + /// + /// CLI-04: a surfaced error for authenticate-user must have the credential + /// redacted (never echoed to stderr), mirroring the API-key redaction seam. + /// + [Fact] + public async Task RunAsync_AuthenticateUser_ErrorOutput_RedactsCredential() + { + const string password = "leaky-credential-value"; + using var output = new StringWriter(); + using var error = new StringWriter(); + + int exitCode = await MxGatewayClientCli.RunAsync( + [ + "authenticate-user", + "--endpoint", "http://localhost:5000", + "--api-key", "test-api-key", + "--session-id", "session-fixture", + "--server-handle", "12", + "--verify-user", "operator", + "--verify-user-password", password, + ], + output, + error, + _ => throw new InvalidOperationException($"boom {password}")); + + Assert.Equal(1, exitCode); + Assert.DoesNotContain(password, error.ToString()); + Assert.Contains("[redacted]", error.ToString()); + } + /// Verifies that error output redacts sensitive API key values. [Fact] public async Task RunAsync_ErrorOutput_RedactsApiKey() diff --git a/clients/dotnet/ZB.MOM.WW.MxGateway.Client.Tests/MxGatewayClientSessionTests.cs b/clients/dotnet/ZB.MOM.WW.MxGateway.Client.Tests/MxGatewayClientSessionTests.cs index d827be1..0b83237 100644 --- a/clients/dotnet/ZB.MOM.WW.MxGateway.Client.Tests/MxGatewayClientSessionTests.cs +++ b/clients/dotnet/ZB.MOM.WW.MxGateway.Client.Tests/MxGatewayClientSessionTests.cs @@ -215,6 +215,55 @@ public sealed class MxGatewayClientSessionTests Assert.Equal("session-fixture", request.SessionId); } + /// + /// Verifies that a reconnect-replay gap sentinel is surfaced as a typed + /// with the gap populated, while normal + /// events pass through unchanged with IsReplayGap false. + /// + [Fact] + public async Task StreamEventItemsAsync_SurfacesReplayGapSentinel() + { + FakeGatewayTransport transport = CreateTransport(); + transport.AddEvent(new MxEvent + { + SessionId = "session-fixture", + ReplayGap = new ReplayGap + { + RequestedAfterSequence = 5, + OldestAvailableSequence = 42, + }, + }); + transport.AddEvent(new MxEvent + { + SessionId = "session-fixture", + Family = MxEventFamily.OnDataChange, + WorkerSequence = 42, + }); + await using MxGatewayClient client = CreateClient(transport); + MxGatewaySession session = await client.OpenSessionAsync(); + + List items = []; + await foreach (MxEventStreamItem item in session.StreamEventItemsAsync(afterWorkerSequence: 5)) + { + items.Add(item); + } + + Assert.Equal(2, items.Count); + + MxEventStreamItem gap = items[0]; + Assert.True(gap.IsReplayGap); + Assert.NotNull(gap.ReplayGap); + Assert.Equal(5UL, gap.ReplayGap!.RequestedAfterSequence); + Assert.Equal(42UL, gap.ReplayGap.OldestAvailableSequence); + Assert.Same(gap.ReplayGap, gap.Event.ReplayGap); + + MxEventStreamItem normal = items[1]; + Assert.False(normal.IsReplayGap); + Assert.Null(normal.ReplayGap); + Assert.Equal(42UL, normal.Event.WorkerSequence); + Assert.Equal(MxEventFamily.OnDataChange, normal.Event.Family); + } + /// Verifies that close is explicit and idempotent. [Fact] public async Task CloseAsync_IsExplicitAndIdempotent() @@ -366,6 +415,297 @@ public sealed class MxGatewayClientSessionTests Assert.Equal(7, el.Value.Int32Value); } + /// Verifies that unregister builds an unregister command with the server handle. + [Fact] + public async Task UnregisterAsync_BuildsUnregisterCommand() + { + FakeGatewayTransport transport = CreateTransport(); + transport.AddInvokeReply(new MxCommandReply + { + SessionId = "session-fixture", + Kind = MxCommandKind.Unregister, + ProtocolStatus = new ProtocolStatus { Code = ProtocolStatusCode.Ok }, + }); + await using MxGatewayClient client = CreateClient(transport); + MxGatewaySession session = await client.OpenSessionAsync(); + + await session.UnregisterAsync(12); + + MxCommandRequest request = Assert.Single(transport.InvokeCalls).Request; + Assert.Equal(MxCommandKind.Unregister, request.Command.Kind); + Assert.Equal(12, request.Command.Unregister.ServerHandle); + } + + /// Verifies that advise-supervisory builds the supervisory advise command. + [Fact] + public async Task AdviseSupervisoryAsync_BuildsAdviseSupervisoryCommand() + { + FakeGatewayTransport transport = CreateTransport(); + transport.AddInvokeReply(new MxCommandReply + { + SessionId = "session-fixture", + Kind = MxCommandKind.AdviseSupervisory, + ProtocolStatus = new ProtocolStatus { Code = ProtocolStatusCode.Ok }, + }); + await using MxGatewayClient client = CreateClient(transport); + MxGatewaySession session = await client.OpenSessionAsync(); + + await session.AdviseSupervisoryAsync(12, 34); + + MxCommandRequest request = Assert.Single(transport.InvokeCalls).Request; + Assert.Equal(MxCommandKind.AdviseSupervisory, request.Command.Kind); + Assert.Equal(12, request.Command.AdviseSupervisory.ServerHandle); + Assert.Equal(34, request.Command.AdviseSupervisory.ItemHandle); + } + + /// Verifies that add-buffered-item returns the item handle from the typed reply. + [Fact] + public async Task AddBufferedItemAsync_BuildsCommandAndReturnsItemHandle() + { + FakeGatewayTransport transport = CreateTransport(); + transport.AddInvokeReply(new MxCommandReply + { + SessionId = "session-fixture", + Kind = MxCommandKind.AddBufferedItem, + ProtocolStatus = new ProtocolStatus { Code = ProtocolStatusCode.Ok }, + AddBufferedItem = new AddBufferedItemReply { ItemHandle = 77 }, + }); + await using MxGatewayClient client = CreateClient(transport); + MxGatewaySession session = await client.OpenSessionAsync(); + + int itemHandle = await session.AddBufferedItemAsync(12, "Area001.Pump001.Speed", "runtime"); + + Assert.Equal(77, itemHandle); + MxCommandRequest request = Assert.Single(transport.InvokeCalls).Request; + Assert.Equal(MxCommandKind.AddBufferedItem, request.Command.Kind); + Assert.Equal(12, request.Command.AddBufferedItem.ServerHandle); + Assert.Equal("Area001.Pump001.Speed", request.Command.AddBufferedItem.ItemDefinition); + Assert.Equal("runtime", request.Command.AddBufferedItem.ItemContext); + } + + /// Verifies that set-buffered-update-interval builds the command with the interval. + [Fact] + public async Task SetBufferedUpdateIntervalAsync_BuildsCommandWithInterval() + { + FakeGatewayTransport transport = CreateTransport(); + transport.AddInvokeReply(new MxCommandReply + { + SessionId = "session-fixture", + Kind = MxCommandKind.SetBufferedUpdateInterval, + ProtocolStatus = new ProtocolStatus { Code = ProtocolStatusCode.Ok }, + }); + await using MxGatewayClient client = CreateClient(transport); + MxGatewaySession session = await client.OpenSessionAsync(); + + await session.SetBufferedUpdateIntervalAsync(12, 500); + + MxCommandRequest request = Assert.Single(transport.InvokeCalls).Request; + Assert.Equal(MxCommandKind.SetBufferedUpdateInterval, request.Command.Kind); + Assert.Equal(12, request.Command.SetBufferedUpdateInterval.ServerHandle); + Assert.Equal(500, request.Command.SetBufferedUpdateInterval.UpdateIntervalMilliseconds); + } + + /// Verifies that suspend builds the command and returns the reply status. + [Fact] + public async Task SuspendAsync_BuildsCommandAndReturnsStatus() + { + FakeGatewayTransport transport = CreateTransport(); + transport.AddInvokeReply(new MxCommandReply + { + SessionId = "session-fixture", + Kind = MxCommandKind.Suspend, + ProtocolStatus = new ProtocolStatus { Code = ProtocolStatusCode.Ok }, + Suspend = new SuspendReply { Status = new MxStatusProxy { Success = 1, Category = MxStatusCategory.Ok } }, + }); + await using MxGatewayClient client = CreateClient(transport); + MxGatewaySession session = await client.OpenSessionAsync(); + + MxStatusProxy? status = await session.SuspendAsync(12, 34); + + Assert.NotNull(status); + Assert.Equal(1, status!.Success); + MxCommandRequest request = Assert.Single(transport.InvokeCalls).Request; + Assert.Equal(MxCommandKind.Suspend, request.Command.Kind); + Assert.Equal(12, request.Command.Suspend.ServerHandle); + Assert.Equal(34, request.Command.Suspend.ItemHandle); + } + + /// Verifies that activate builds the command and returns the reply status. + [Fact] + public async Task ActivateAsync_BuildsCommandAndReturnsStatus() + { + FakeGatewayTransport transport = CreateTransport(); + transport.AddInvokeReply(new MxCommandReply + { + SessionId = "session-fixture", + Kind = MxCommandKind.Activate, + ProtocolStatus = new ProtocolStatus { Code = ProtocolStatusCode.Ok }, + Activate = new ActivateReply { Status = new MxStatusProxy { Success = 1, Category = MxStatusCategory.Ok } }, + }); + await using MxGatewayClient client = CreateClient(transport); + MxGatewaySession session = await client.OpenSessionAsync(); + + MxStatusProxy? status = await session.ActivateAsync(12, 34); + + Assert.NotNull(status); + MxCommandRequest request = Assert.Single(transport.InvokeCalls).Request; + Assert.Equal(MxCommandKind.Activate, request.Command.Kind); + Assert.Equal(34, request.Command.Activate.ItemHandle); + } + + /// Verifies that write-secured builds a WriteSecured command with value and user ids. + [Fact] + public async Task WriteSecuredAsync_BuildsWriteSecuredCommand() + { + FakeGatewayTransport transport = CreateTransport(); + transport.AddInvokeReply(new MxCommandReply + { + SessionId = "session-fixture", + Kind = MxCommandKind.WriteSecured, + ProtocolStatus = new ProtocolStatus { Code = ProtocolStatusCode.Ok }, + }); + await using MxGatewayClient client = CreateClient(transport); + MxGatewaySession session = await client.OpenSessionAsync(); + MxValue value = 123.ToMxValue(); + + await session.WriteSecuredAsync(12, 34, value, currentUserId: 5, verifierUserId: 6); + + MxCommandRequest request = Assert.Single(transport.InvokeCalls).Request; + Assert.Equal(MxCommandKind.WriteSecured, request.Command.Kind); + Assert.Equal(12, request.Command.WriteSecured.ServerHandle); + Assert.Equal(34, request.Command.WriteSecured.ItemHandle); + Assert.Same(value, request.Command.WriteSecured.Value); + Assert.Equal(5, request.Command.WriteSecured.CurrentUserId); + Assert.Equal(6, request.Command.WriteSecured.VerifierUserId); + } + + /// + /// MXAccess parity: WriteSecured issued before a prior AuthenticateUser fails + /// natively. The client must surface that scripted failure (as an + /// ) rather than pre-validating it away. + /// + [Fact] + public async Task WriteSecuredAsync_SurfacesNativeFailureWhenNotAuthenticated() + { + FakeGatewayTransport transport = CreateTransport(); + transport.AddInvokeReply(new MxCommandReply + { + SessionId = "session-fixture", + Kind = MxCommandKind.WriteSecured, + ProtocolStatus = new ProtocolStatus { Code = ProtocolStatusCode.MxaccessFailure }, + Hresult = unchecked((int)0x80040200), + }); + await using MxGatewayClient client = CreateClient(transport); + MxGatewaySession session = await client.OpenSessionAsync(); + + MxAccessException exception = await Assert.ThrowsAsync( + async () => await session.WriteSecuredAsync(12, 34, 123.ToMxValue(), currentUserId: 0, verifierUserId: 0)); + + // The native HRESULT is surfaced; the request payload is never in the message. + Assert.Contains("WriteSecured", exception.Message); + Assert.Single(transport.InvokeCalls); + } + + /// Verifies that write-secured2 builds a WriteSecured2 command with value and timestamp. + [Fact] + public async Task WriteSecured2Async_BuildsWriteSecured2Command() + { + FakeGatewayTransport transport = CreateTransport(); + transport.AddInvokeReply(new MxCommandReply + { + SessionId = "session-fixture", + Kind = MxCommandKind.WriteSecured2, + ProtocolStatus = new ProtocolStatus { Code = ProtocolStatusCode.Ok }, + }); + await using MxGatewayClient client = CreateClient(transport); + MxGatewaySession session = await client.OpenSessionAsync(); + MxValue value = 123.ToMxValue(); + MxValue timestampValue = DateTimeOffset.Parse("2026-01-01T00:00:00Z").ToMxValue(); + + await session.WriteSecured2Async(12, 34, value, timestampValue, currentUserId: 5, verifierUserId: 6); + + MxCommandRequest request = Assert.Single(transport.InvokeCalls).Request; + Assert.Equal(MxCommandKind.WriteSecured2, request.Command.Kind); + Assert.Same(value, request.Command.WriteSecured2.Value); + Assert.Same(timestampValue, request.Command.WriteSecured2.TimestampValue); + Assert.Equal(5, request.Command.WriteSecured2.CurrentUserId); + Assert.Equal(6, request.Command.WriteSecured2.VerifierUserId); + } + + /// Verifies that authenticate-user builds the command and returns the resolved user id. + [Fact] + public async Task AuthenticateUserAsync_BuildsCommandAndReturnsUserId() + { + FakeGatewayTransport transport = CreateTransport(); + transport.AddInvokeReply(new MxCommandReply + { + SessionId = "session-fixture", + Kind = MxCommandKind.AuthenticateUser, + ProtocolStatus = new ProtocolStatus { Code = ProtocolStatusCode.Ok }, + AuthenticateUser = new AuthenticateUserReply { UserId = 4242 }, + }); + await using MxGatewayClient client = CreateClient(transport); + MxGatewaySession session = await client.OpenSessionAsync(); + + int userId = await session.AuthenticateUserAsync(12, "operator", "s3cr3t-p@ss"); + + Assert.Equal(4242, userId); + MxCommandRequest request = Assert.Single(transport.InvokeCalls).Request; + Assert.Equal(MxCommandKind.AuthenticateUser, request.Command.Kind); + Assert.Equal("operator", request.Command.AuthenticateUser.VerifyUser); + Assert.Equal("s3cr3t-p@ss", request.Command.AuthenticateUser.VerifyUserPassword); + } + + /// + /// SECRET REDACTION: when AuthenticateUser fails, the surfaced exception message + /// must never contain the credential — the error path is built only from + /// reply-derived diagnostics, not the request payload. + /// + [Fact] + public async Task AuthenticateUserAsync_FailureDoesNotLeakCredentialInErrorMessage() + { + const string password = "super-secret-credential-123"; + FakeGatewayTransport transport = CreateTransport(); + transport.AddInvokeReply(new MxCommandReply + { + SessionId = "session-fixture", + Kind = MxCommandKind.AuthenticateUser, + ProtocolStatus = new ProtocolStatus { Code = ProtocolStatusCode.MxaccessFailure }, + Hresult = unchecked((int)0x80040210), + }); + await using MxGatewayClient client = CreateClient(transport); + MxGatewaySession session = await client.OpenSessionAsync(); + + MxAccessException exception = await Assert.ThrowsAsync( + async () => await session.AuthenticateUserAsync(12, "operator", password)); + + Assert.DoesNotContain(password, exception.Message); + Assert.DoesNotContain(password, exception.ToString()); + } + + /// Verifies that archestra-user-to-id builds the command and returns the resolved user id. + [Fact] + public async Task ArchestraUserToIdAsync_BuildsCommandAndReturnsUserId() + { + FakeGatewayTransport transport = CreateTransport(); + transport.AddInvokeReply(new MxCommandReply + { + SessionId = "session-fixture", + Kind = MxCommandKind.ArchestraUserToId, + ProtocolStatus = new ProtocolStatus { Code = ProtocolStatusCode.Ok }, + ArchestraUserToId = new ArchestrAUserToIdReply { UserId = 909 }, + }); + await using MxGatewayClient client = CreateClient(transport); + MxGatewaySession session = await client.OpenSessionAsync(); + + int userId = await session.ArchestraUserToIdAsync(12, "BCC47053-9542-4D65-BDAA-BCDEA6A32A73"); + + Assert.Equal(909, userId); + MxCommandRequest request = Assert.Single(transport.InvokeCalls).Request; + Assert.Equal(MxCommandKind.ArchestraUserToId, request.Command.Kind); + Assert.Equal("BCC47053-9542-4D65-BDAA-BCDEA6A32A73", request.Command.ArchestraUserToId.UserIdGuid); + } + private static MxGatewayClient CreateClient(FakeGatewayTransport transport) { return new MxGatewayClient(transport.Options, transport); diff --git a/clients/dotnet/ZB.MOM.WW.MxGateway.Client/MxEventStreamExtensions.cs b/clients/dotnet/ZB.MOM.WW.MxGateway.Client/MxEventStreamExtensions.cs new file mode 100644 index 0000000..67b376a --- /dev/null +++ b/clients/dotnet/ZB.MOM.WW.MxGateway.Client/MxEventStreamExtensions.cs @@ -0,0 +1,40 @@ +using System.Runtime.CompilerServices; +using ZB.MOM.WW.MxGateway.Contracts.Proto; + +namespace ZB.MOM.WW.MxGateway.Client; + +/// +/// Extension methods that project a raw stream into the +/// typed surface, making the gateway's +/// reconnect-replay gap sentinel observable. +/// +public static class MxEventStreamExtensions +{ + /// + /// Projects a raw stream (e.g. + /// or + /// ) into typed + /// values. Normal events pass through with + /// false; the gateway's + /// reconnect-replay gap sentinel is surfaced with + /// true and + /// populated. The stream is + /// forwarded faithfully — no event is synthesized or dropped. + /// + /// The raw event stream to wrap. + /// Cancellation token for the enumeration. + /// The same events, each wrapped as an . + public static async IAsyncEnumerable AsStreamItemsAsync( + this IAsyncEnumerable source, + [EnumeratorCancellation] CancellationToken cancellationToken = default) + { + ArgumentNullException.ThrowIfNull(source); + + await foreach (MxEvent gatewayEvent in source + .WithCancellation(cancellationToken) + .ConfigureAwait(false)) + { + yield return MxEventStreamItem.From(gatewayEvent); + } + } +} diff --git a/clients/dotnet/ZB.MOM.WW.MxGateway.Client/MxEventStreamItem.cs b/clients/dotnet/ZB.MOM.WW.MxGateway.Client/MxEventStreamItem.cs new file mode 100644 index 0000000..c2f79df --- /dev/null +++ b/clients/dotnet/ZB.MOM.WW.MxGateway.Client/MxEventStreamItem.cs @@ -0,0 +1,82 @@ +using ZB.MOM.WW.MxGateway.Contracts.Proto; + +namespace ZB.MOM.WW.MxGateway.Client; + +/// +/// One item yielded by the typed event stream. It is either a normal MXAccess +/// or a reconnect-replay gap sentinel. +/// +/// +/// +/// The gateway emits a single sentinel at the head of a +/// StreamEvents stream that was resumed via +/// StreamEventsRequest.after_worker_sequence when the requested sequence +/// predates the oldest event still retained in the session replay ring — i.e. +/// events were evicted and cannot be replayed. On that sentinel the +/// MxEvent.replay_gap field is set, is +/// , the body oneof is unset, and +/// no per-item fields are populated. +/// +/// +/// This wrapper makes that sentinel observable instead of forcing the consumer +/// to inspect the raw . It never synthesizes or swallows an +/// event: the gap is exactly the gateway's own sentinel, exposed with +/// set and populated. Every +/// other event flows through unchanged with false. +/// +/// +/// When is the consumer has +/// missed events and MUST discard local state and re-snapshot. To resume without +/// incurring another gap, reconnect with +/// after_worker_sequence = ReplayGap.OldestAvailableSequence - 1. +/// +/// +public sealed class MxEventStreamItem +{ + private MxEventStreamItem(MxEvent gatewayEvent, ReplayGap? replayGap) + { + Event = gatewayEvent; + ReplayGap = replayGap; + } + + /// + /// The underlying raw . For a normal event this is the + /// MXAccess event itself; for a replay-gap item this is the gateway's + /// sentinel event whose only meaningful payload is . + /// Never . + /// + public MxEvent Event { get; } + + /// + /// The reconnect-replay gap payload when this item is a gap sentinel; + /// otherwise . Read + /// and + /// to learn + /// which events were lost. + /// + public ReplayGap? ReplayGap { get; } + + /// + /// when this item is a reconnect-replay gap sentinel: + /// the consumer missed events and must discard local state and re-snapshot. + /// Resume without another gap by reconnecting with + /// after_worker_sequence = ReplayGap.OldestAvailableSequence - 1. + /// + public bool IsReplayGap => ReplayGap is not null; + + /// + /// Wraps a raw stream as a typed item, classifying it + /// as a replay-gap sentinel when MxEvent.replay_gap is present. + /// + /// The raw event from the StreamEvents stream. + /// A typed item exposing either the normal event or the replay gap. + internal static MxEventStreamItem From(MxEvent gatewayEvent) + { + ArgumentNullException.ThrowIfNull(gatewayEvent); + + // For a message-typed proto3 field, presence is a non-null reference. + return gatewayEvent.ReplayGap is { } replayGap + ? new MxEventStreamItem(gatewayEvent, replayGap) + : new MxEventStreamItem(gatewayEvent, replayGap: null); + } +} diff --git a/clients/dotnet/ZB.MOM.WW.MxGateway.Client/MxGatewaySession.cs b/clients/dotnet/ZB.MOM.WW.MxGateway.Client/MxGatewaySession.cs index e0c6f24..a8ca738 100644 --- a/clients/dotnet/ZB.MOM.WW.MxGateway.Client/MxGatewaySession.cs +++ b/clients/dotnet/ZB.MOM.WW.MxGateway.Client/MxGatewaySession.cs @@ -842,6 +842,525 @@ public sealed class MxGatewaySession : IAsyncDisposable cancellationToken); } + /// + /// Unregisters a previously registered client from the MXAccess session + /// (MXAccess Unregister), releasing its ServerHandle. + /// + /// The ServerHandle from register. + /// Cancellation token. + public async Task UnregisterAsync( + int serverHandle, + CancellationToken cancellationToken = default) + { + MxCommandReply reply = await UnregisterRawAsync(serverHandle, cancellationToken) + .ConfigureAwait(false); + reply.EnsureProtocolSuccess().EnsureMxAccessSuccess(); + } + + /// + /// Unregisters a previously registered client without error checking. + /// + /// The ServerHandle from register. + /// Cancellation token. + /// The raw server reply. + public Task UnregisterRawAsync( + int serverHandle, + CancellationToken cancellationToken = default) + { + return InvokeCommandAsync( + new MxCommand + { + Kind = MxCommandKind.Unregister, + Unregister = new UnregisterCommand { ServerHandle = serverHandle }, + }, + cancellationToken); + } + + /// + /// Subscribes to supervisory events for an item (MXAccess AdviseSupervisory). + /// + /// The ServerHandle from register. + /// The ItemHandle from add-item. + /// Cancellation token. + public async Task AdviseSupervisoryAsync( + int serverHandle, + int itemHandle, + CancellationToken cancellationToken = default) + { + MxCommandReply reply = await AdviseSupervisoryRawAsync(serverHandle, itemHandle, cancellationToken) + .ConfigureAwait(false); + reply.EnsureProtocolSuccess().EnsureMxAccessSuccess(); + } + + /// + /// Subscribes to supervisory events for an item without error checking. + /// + /// The ServerHandle from register. + /// The ItemHandle from add-item. + /// Cancellation token. + /// The raw server reply. + public Task AdviseSupervisoryRawAsync( + int serverHandle, + int itemHandle, + CancellationToken cancellationToken = default) + { + return InvokeCommandAsync( + new MxCommand + { + Kind = MxCommandKind.AdviseSupervisory, + AdviseSupervisory = new AdviseSupervisoryCommand + { + ServerHandle = serverHandle, + ItemHandle = itemHandle, + }, + }, + cancellationToken); + } + + /// + /// Adds a buffered item to the MXAccess session (MXAccess AddBufferedItem), + /// returning an ItemHandle. + /// + /// The ServerHandle from register. + /// The item tag address. + /// Additional context for the item. + /// Cancellation token. + /// The item handle assigned to the new buffered item. + public async Task AddBufferedItemAsync( + int serverHandle, + string itemDefinition, + string itemContext, + CancellationToken cancellationToken = default) + { + MxCommandReply reply = await AddBufferedItemRawAsync( + serverHandle, + itemDefinition, + itemContext, + cancellationToken) + .ConfigureAwait(false); + reply.EnsureProtocolSuccess().EnsureMxAccessSuccess(); + return reply.AddBufferedItem?.ItemHandle ?? reply.ReturnValue.Int32Value; + } + + /// + /// Adds a buffered item to the MXAccess session without error checking. + /// + /// The ServerHandle from register. + /// The item tag address. + /// Additional context for the item. + /// Cancellation token. + /// The raw server reply. + public Task AddBufferedItemRawAsync( + int serverHandle, + string itemDefinition, + string itemContext, + CancellationToken cancellationToken = default) + { + ArgumentException.ThrowIfNullOrWhiteSpace(itemDefinition); + + return InvokeCommandAsync( + new MxCommand + { + Kind = MxCommandKind.AddBufferedItem, + AddBufferedItem = new AddBufferedItemCommand + { + ServerHandle = serverHandle, + ItemDefinition = itemDefinition, + ItemContext = itemContext ?? string.Empty, + }, + }, + cancellationToken); + } + + /// + /// Sets the buffered-item update interval on the MXAccess session + /// (MXAccess SetBufferedUpdateInterval). + /// + /// The ServerHandle from register. + /// The buffered update interval, in milliseconds. + /// Cancellation token. + public async Task SetBufferedUpdateIntervalAsync( + int serverHandle, + int updateIntervalMilliseconds, + CancellationToken cancellationToken = default) + { + MxCommandReply reply = await SetBufferedUpdateIntervalRawAsync( + serverHandle, + updateIntervalMilliseconds, + cancellationToken) + .ConfigureAwait(false); + reply.EnsureProtocolSuccess().EnsureMxAccessSuccess(); + } + + /// + /// Sets the buffered-item update interval without error checking. + /// + /// The ServerHandle from register. + /// The buffered update interval, in milliseconds. + /// Cancellation token. + /// The raw server reply. + public Task SetBufferedUpdateIntervalRawAsync( + int serverHandle, + int updateIntervalMilliseconds, + CancellationToken cancellationToken = default) + { + return InvokeCommandAsync( + new MxCommand + { + Kind = MxCommandKind.SetBufferedUpdateInterval, + SetBufferedUpdateInterval = new SetBufferedUpdateIntervalCommand + { + ServerHandle = serverHandle, + UpdateIntervalMilliseconds = updateIntervalMilliseconds, + }, + }, + cancellationToken); + } + + /// + /// Suspends updates for an item (MXAccess Suspend). + /// + /// The ServerHandle from register. + /// The ItemHandle from add-item. + /// Cancellation token. + /// The item's MXSTATUS_PROXY as reported by the worker, or null if the reply omitted it. + public async Task SuspendAsync( + int serverHandle, + int itemHandle, + CancellationToken cancellationToken = default) + { + MxCommandReply reply = await SuspendRawAsync(serverHandle, itemHandle, cancellationToken) + .ConfigureAwait(false); + reply.EnsureProtocolSuccess().EnsureMxAccessSuccess(); + return reply.Suspend?.Status; + } + + /// + /// Suspends updates for an item without error checking. + /// + /// The ServerHandle from register. + /// The ItemHandle from add-item. + /// Cancellation token. + /// The raw server reply. + public Task SuspendRawAsync( + int serverHandle, + int itemHandle, + CancellationToken cancellationToken = default) + { + return InvokeCommandAsync( + new MxCommand + { + Kind = MxCommandKind.Suspend, + Suspend = new SuspendCommand + { + ServerHandle = serverHandle, + ItemHandle = itemHandle, + }, + }, + cancellationToken); + } + + /// + /// Resumes updates for a suspended item (MXAccess Activate). + /// + /// The ServerHandle from register. + /// The ItemHandle from add-item. + /// Cancellation token. + /// The item's MXSTATUS_PROXY as reported by the worker, or null if the reply omitted it. + public async Task ActivateAsync( + int serverHandle, + int itemHandle, + CancellationToken cancellationToken = default) + { + MxCommandReply reply = await ActivateRawAsync(serverHandle, itemHandle, cancellationToken) + .ConfigureAwait(false); + reply.EnsureProtocolSuccess().EnsureMxAccessSuccess(); + return reply.Activate?.Status; + } + + /// + /// Resumes updates for a suspended item without error checking. + /// + /// The ServerHandle from register. + /// The ItemHandle from add-item. + /// Cancellation token. + /// The raw server reply. + public Task ActivateRawAsync( + int serverHandle, + int itemHandle, + CancellationToken cancellationToken = default) + { + return InvokeCommandAsync( + new MxCommand + { + Kind = MxCommandKind.Activate, + Activate = new ActivateCommand + { + ServerHandle = serverHandle, + ItemHandle = itemHandle, + }, + }, + cancellationToken); + } + + /// + /// Writes a secured value to an item on the MXAccess server (MXAccess WriteSecured). + /// + /// + /// MXAccess parity: WriteSecured fails when it is issued before a value-bearing + /// NMX body or before a prior AuthenticateUser + AdviseSupervisory. That + /// native failure is surfaced unchanged — the client does not pre-validate or reorder it. + /// The is credential-sensitive and must never reach logs; the + /// client mirrors the single-item WriteSecured redaction contract. + /// + /// The ServerHandle from register. + /// The ItemHandle from add-item. + /// The secured value to write. + /// The current operator user id. + /// The verifier (secondary approver) user id. + /// Cancellation token. + public async Task WriteSecuredAsync( + int serverHandle, + int itemHandle, + MxValue value, + int currentUserId, + int verifierUserId, + CancellationToken cancellationToken = default) + { + MxCommandReply reply = await WriteSecuredRawAsync( + serverHandle, + itemHandle, + value, + currentUserId, + verifierUserId, + cancellationToken) + .ConfigureAwait(false); + reply.EnsureProtocolSuccess().EnsureMxAccessSuccess(); + } + + /// + /// Writes a secured value to an item without error checking. See + /// for the parity and redaction contract. + /// + /// The ServerHandle from register. + /// The ItemHandle from add-item. + /// The secured value to write. + /// The current operator user id. + /// The verifier (secondary approver) user id. + /// Cancellation token. + /// The raw server reply. + public Task WriteSecuredRawAsync( + int serverHandle, + int itemHandle, + MxValue value, + int currentUserId, + int verifierUserId, + CancellationToken cancellationToken = default) + { + ArgumentNullException.ThrowIfNull(value); + + return InvokeCommandAsync( + new MxCommand + { + Kind = MxCommandKind.WriteSecured, + WriteSecured = new WriteSecuredCommand + { + ServerHandle = serverHandle, + ItemHandle = itemHandle, + CurrentUserId = currentUserId, + VerifierUserId = verifierUserId, + Value = value, + }, + }, + cancellationToken); + } + + /// + /// Writes a secured value and timestamp to an item (MXAccess WriteSecured2). + /// + /// + /// Same parity and redaction contract as : the native + /// failure that occurs before a value-bearing NMX body or a prior authenticate is + /// surfaced unchanged, and the credential-sensitive must never + /// reach logs. + /// + /// The ServerHandle from register. + /// The ItemHandle from add-item. + /// The secured value to write. + /// The timestamp to write with the value. + /// The current operator user id. + /// The verifier (secondary approver) user id. + /// Cancellation token. + public async Task WriteSecured2Async( + int serverHandle, + int itemHandle, + MxValue value, + MxValue timestampValue, + int currentUserId, + int verifierUserId, + CancellationToken cancellationToken = default) + { + MxCommandReply reply = await WriteSecured2RawAsync( + serverHandle, + itemHandle, + value, + timestampValue, + currentUserId, + verifierUserId, + cancellationToken) + .ConfigureAwait(false); + reply.EnsureProtocolSuccess().EnsureMxAccessSuccess(); + } + + /// + /// Writes a secured value and timestamp to an item without error checking. See + /// for the parity and redaction contract. + /// + /// The ServerHandle from register. + /// The ItemHandle from add-item. + /// The secured value to write. + /// The timestamp to write with the value. + /// The current operator user id. + /// The verifier (secondary approver) user id. + /// Cancellation token. + /// The raw server reply. + public Task WriteSecured2RawAsync( + int serverHandle, + int itemHandle, + MxValue value, + MxValue timestampValue, + int currentUserId, + int verifierUserId, + CancellationToken cancellationToken = default) + { + ArgumentNullException.ThrowIfNull(value); + ArgumentNullException.ThrowIfNull(timestampValue); + + return InvokeCommandAsync( + new MxCommand + { + Kind = MxCommandKind.WriteSecured2, + WriteSecured2 = new WriteSecured2Command + { + ServerHandle = serverHandle, + ItemHandle = itemHandle, + CurrentUserId = currentUserId, + VerifierUserId = verifierUserId, + Value = value, + TimestampValue = timestampValue, + }, + }, + cancellationToken); + } + + /// + /// Authenticates an MXAccess verify-user (MXAccess AuthenticateUser), returning + /// the resolved user id used by WriteSecured / WriteSecured2. + /// + /// + /// The is a raw MXAccess credential. It is never + /// logged and never placed on the exception path: gateway/MXAccess failures surface only + /// reply-derived diagnostics (kind, HRESULT, MXSTATUS_PROXY), never the request payload. + /// + /// The ServerHandle from register. + /// The user to verify. + /// The verify-user credential. Never logged. + /// Cancellation token. + /// The authenticated user id. + public async Task AuthenticateUserAsync( + int serverHandle, + string verifyUser, + string verifyUserPassword, + CancellationToken cancellationToken = default) + { + MxCommandReply reply = await AuthenticateUserRawAsync( + serverHandle, + verifyUser, + verifyUserPassword, + cancellationToken) + .ConfigureAwait(false); + reply.EnsureProtocolSuccess().EnsureMxAccessSuccess(); + return reply.AuthenticateUser?.UserId ?? reply.ReturnValue.Int32Value; + } + + /// + /// Authenticates an MXAccess verify-user without error checking. See + /// for the credential-handling contract. + /// + /// The ServerHandle from register. + /// The user to verify. + /// The verify-user credential. Never logged. + /// Cancellation token. + /// The raw server reply. + public Task AuthenticateUserRawAsync( + int serverHandle, + string verifyUser, + string verifyUserPassword, + CancellationToken cancellationToken = default) + { + ArgumentNullException.ThrowIfNull(verifyUser); + ArgumentNullException.ThrowIfNull(verifyUserPassword); + + return InvokeCommandAsync( + new MxCommand + { + Kind = MxCommandKind.AuthenticateUser, + AuthenticateUser = new AuthenticateUserCommand + { + ServerHandle = serverHandle, + VerifyUser = verifyUser, + VerifyUserPassword = verifyUserPassword, + }, + }, + cancellationToken); + } + + /// + /// Resolves an ArchestrA user GUID to its MXAccess user id + /// (MXAccess ArchestrAUserToId). + /// + /// The ServerHandle from register. + /// The ArchestrA user GUID to resolve. + /// Cancellation token. + /// The resolved MXAccess user id. + public async Task ArchestraUserToIdAsync( + int serverHandle, + string userIdGuid, + CancellationToken cancellationToken = default) + { + MxCommandReply reply = await ArchestraUserToIdRawAsync(serverHandle, userIdGuid, cancellationToken) + .ConfigureAwait(false); + reply.EnsureProtocolSuccess().EnsureMxAccessSuccess(); + return reply.ArchestraUserToId?.UserId ?? reply.ReturnValue.Int32Value; + } + + /// + /// Resolves an ArchestrA user GUID to its MXAccess user id without error checking. + /// + /// The ServerHandle from register. + /// The ArchestrA user GUID to resolve. + /// Cancellation token. + /// The raw server reply. + public Task ArchestraUserToIdRawAsync( + int serverHandle, + string userIdGuid, + CancellationToken cancellationToken = default) + { + ArgumentException.ThrowIfNullOrWhiteSpace(userIdGuid); + + return InvokeCommandAsync( + new MxCommand + { + Kind = MxCommandKind.ArchestraUserToId, + ArchestraUserToId = new ArchestrAUserToIdCommand + { + ServerHandle = serverHandle, + UserIdGuid = userIdGuid, + }, + }, + cancellationToken); + } + /// /// Invokes an MXAccess command on this session. /// @@ -875,6 +1394,34 @@ public sealed class MxGatewaySession : IAsyncDisposable cancellationToken); } + /// + /// Streams events as typed values, surfacing + /// the gateway's reconnect-replay gap sentinel as an observable, typed signal. + /// + /// + /// When resuming with a stale (older than + /// the oldest event still retained in the session replay ring), the gateway emits + /// a single gap sentinel at the head of the stream. It arrives here as an item + /// with true and + /// populated, meaning the consumer + /// missed events and MUST discard local state and re-snapshot. To resume without + /// incurring another gap, reconnect with + /// afterWorkerSequence = item.ReplayGap.OldestAvailableSequence - 1. + /// All other events pass through with + /// false. Use when raw generated + /// messages are needed instead. + /// + /// The sequence number to stream from. Defaults to 0. + /// Cancellation token. + /// An async enumerable of typed event items. + public IAsyncEnumerable StreamEventItemsAsync( + ulong afterWorkerSequence = 0, + CancellationToken cancellationToken = default) + { + return StreamEventsAsync(afterWorkerSequence, cancellationToken) + .AsStreamItemsAsync(cancellationToken); + } + /// /// Closes the session and releases resources. /// diff --git a/clients/dotnet/ZB.MOM.WW.MxGateway.Client/ZB.MOM.WW.MxGateway.Client.csproj b/clients/dotnet/ZB.MOM.WW.MxGateway.Client/ZB.MOM.WW.MxGateway.Client.csproj index 4df7458..2da0c6f 100644 --- a/clients/dotnet/ZB.MOM.WW.MxGateway.Client/ZB.MOM.WW.MxGateway.Client.csproj +++ b/clients/dotnet/ZB.MOM.WW.MxGateway.Client/ZB.MOM.WW.MxGateway.Client.csproj @@ -19,6 +19,7 @@ true ZB.MOM.WW.MxGateway.Client + 0.1.2 .NET 10 gRPC client for the MxAccessGateway service. Provides typed wrappers, retry, and a lazy-browse walker over the Galaxy Repository hierarchy. README.md + + 0.1.2 + + + + + + + + $(_StampedGitSha.Trim()) + + + diff --git a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardAuthenticationDefaults.cs b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardAuthenticationDefaults.cs index 1566d4a..f00caf3 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardAuthenticationDefaults.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardAuthenticationDefaults.cs @@ -35,5 +35,23 @@ public static class DashboardAuthenticationDefaults public const string LdapGroupClaimType = "mxgateway:ldap_group"; public const string KeyPrefixClaimType = "mxgateway:key_prefix"; + + /// + /// Dashboard auth cookie name used when the cookie is not guaranteed to be Secure + /// (RequireHttpsCookie=false → ) + /// or when an explicit MxGateway:Dashboard:CookieName override is absent but the + /// secure default cannot be applied. This plain name carries no browser-enforced guarantees. + /// public const string CookieName = "MxGatewayDashboard"; + + /// + /// Dashboard auth cookie name applied when the cookie is guaranteed Secure + /// (RequireHttpsCookie=true → ) + /// and no explicit MxGateway:Dashboard:CookieName override is set. The __Host- + /// prefix instructs browsers to enforce Secure, no Domain, and Path=/; those + /// guarantees only hold for a Secure cookie, so this name must never be applied unless + /// is in + /// effect — a __Host- cookie without Secure is silently dropped by browsers. + /// + public const string SecureCookieName = "__Host-MxGatewayDashboard"; } diff --git a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardServiceCollectionExtensions.cs b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardServiceCollectionExtensions.cs index a50b502..658d42e 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardServiceCollectionExtensions.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardServiceCollectionExtensions.cs @@ -123,13 +123,22 @@ public static class DashboardServiceCollectionExtensions ? CookieSecurePolicy.Always : CookieSecurePolicy.SameAsRequest; - // Config-driven cookie name (MxGateway:Dashboard:CookieName). Null/blank keeps - // the canonical default set above, so a misconfiguration cannot unname the cookie. + // Config-driven cookie name (MxGateway:Dashboard:CookieName). An explicit override + // always wins. With no override, restore the __Host- prefix when the cookie is + // guaranteed Secure (RequireHttpsCookie true → SecurePolicy Always): the __Host- + // browser guarantees (Secure required, no Domain, Path=/) hold only for a Secure + // cookie, and a __Host- cookie without Secure is silently dropped — so the prefix + // is never applied unless SecurePolicy is Always. Otherwise keep the plain + // canonical default set by AddCookie above, so a misconfiguration cannot unname it. var cookieName = gatewayOptions.Value.Dashboard.CookieName; if (!string.IsNullOrWhiteSpace(cookieName)) { cookieOptions.Cookie.Name = cookieName; } + else if (cookieOptions.Cookie.SecurePolicy == CookieSecurePolicy.Always) + { + cookieOptions.Cookie.Name = DashboardAuthenticationDefaults.SecureCookieName; + } }); services.AddAuthorization(authorization => diff --git a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/DashboardEventBroadcaster.cs b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/DashboardEventBroadcaster.cs index ac44ec7..b81125e 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/DashboardEventBroadcaster.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/DashboardEventBroadcaster.cs @@ -1,5 +1,7 @@ using Microsoft.AspNetCore.SignalR; +using Microsoft.Extensions.Options; using ZB.MOM.WW.MxGateway.Contracts.Proto; +using ZB.MOM.WW.MxGateway.Server.Configuration; namespace ZB.MOM.WW.MxGateway.Server.Dashboard.Hubs; @@ -10,10 +12,22 @@ namespace ZB.MOM.WW.MxGateway.Server.Dashboard.Hubs; /// Errors are logged once and dropped — keeping the SignalR mirror best-effort /// preserves the gRPC contract that exists today. /// +/// +/// When MxGateway:Dashboard:ShowTagValues is false (the default), tag +/// values are stripped from a redacted copy of the event before it reaches any +/// dashboard client. The source is shared with the gRPC +/// event path and the reconnect replay ring, so it is never mutated in place — +/// the redaction is applied to a deep clone. This closes the value-leak seam at +/// the mirror independently of the still-outstanding per-session hub ACL +/// (see ). +/// public sealed class DashboardEventBroadcaster( IHubContext hubContext, + IOptions options, ILogger logger) : IDashboardEventBroadcaster { + private readonly bool _showTagValues = options.Value.Dashboard.ShowTagValues; + /// public void Publish(string sessionId, MxEvent mxEvent) { @@ -22,6 +36,8 @@ public sealed class DashboardEventBroadcaster( return; } + MxEvent outbound = _showTagValues ? mxEvent : RedactValues(mxEvent); + // Wrap the Task acquisition in a try/catch so a hypothetical synchronous throw // from SendAsync (e.g. an implementation that throws before returning the Task) // cannot escape Publish. The interface contract is never-throw; fire-and-forget. @@ -30,7 +46,7 @@ public sealed class DashboardEventBroadcaster( { send = hubContext.Clients .Group(EventsHub.GroupName(sessionId)) - .SendAsync(EventsHub.EventMessage, mxEvent); + .SendAsync(EventsHub.EventMessage, outbound); } catch (Exception ex) { @@ -51,4 +67,27 @@ public sealed class DashboardEventBroadcaster( TaskScheduler.Default); } } + + /// + /// Produces a deep clone of with every tag-value + /// field cleared, leaving tag reference, quality, status, and timestamps + /// intact so the dashboard still renders the event without the value. The + /// source event is left untouched because it is shared downstream with the + /// gRPC stream and the replay ring. + /// + /// The source event to redact a copy of. + /// A redacted deep clone of the event. + private static MxEvent RedactValues(MxEvent source) + { + MxEvent redacted = source.Clone(); + redacted.Value = null; + + if (redacted.BodyCase == MxEvent.BodyOneofCase.OnAlarmTransition) + { + redacted.OnAlarmTransition.CurrentValue = null; + redacted.OnAlarmTransition.LimitValue = null; + } + + return redacted; + } } diff --git a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/EventsHub.cs b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/EventsHub.cs index 6d93a85..1c5863c 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/EventsHub.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/EventsHub.cs @@ -32,15 +32,19 @@ public sealed class EventsHub : Hub /// the dashboard roles (Admin or Viewer); both roles may subscribe to /// any session id they choose. This is acceptable today because (a) the /// dashboard's per-session views show non-secret session metadata that - /// any authenticated dashboard user can already see, and (b) value - /// logging in the source gRPC stream is gated by the same redaction - /// policy that protects logs. The per-session ACL that gates the gRPC + /// any authenticated dashboard user can already see, and (b) tag values + /// are stripped from the mirrored events by + /// when + /// MxGateway:Dashboard:ShowTagValues is false (the default), so the + /// most sensitive payload cannot leak through this seam regardless of the + /// still-missing ACL. The per-session ACL that gates the gRPC /// StreamEvents RPC is intentionally not yet mirrored here. - /// TODO(per-session-acl): once a role/scope is introduced that scopes a - /// Viewer to a specific session or tenant, add a session-access check - /// at this seam — either inline (consult the per-user allowed-session - /// set on Context.User claims / Context.Items) or via a - /// dedicated authorization policy applied to the hub method itself. + /// TODO(per-session-acl): tracked as remediation roadmap item 12 + /// (SEC-25). Once a role/scope is introduced that scopes a Viewer to a + /// specific session or tenant, add a session-access check at this seam — + /// either inline (consult the per-user allowed-session set on + /// Context.User claims / Context.Items) or via a dedicated + /// authorization policy applied to the hub method itself. /// /// Session id to subscribe the caller to. /// A task representing the subscription operation. diff --git a/src/ZB.MOM.WW.MxGateway.Server/Grpc/EventStreamService.cs b/src/ZB.MOM.WW.MxGateway.Server/Grpc/EventStreamService.cs index 5afce48..ae0c3fe 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Grpc/EventStreamService.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Grpc/EventStreamService.cs @@ -116,11 +116,23 @@ public sealed class EventStreamService( options.Value.Sessions.MaxEventSubscribersPerSession); } - int streamQueueDepth = 0; IAsyncEnumerator reader = subscriber.Reader .ReadAllAsync(cancellationToken) .GetAsyncEnumerator(cancellationToken); + // GWC-15: register this subscriber's channel as a live backlog source instead of + // reconciling the queue-depth gauge on every event. The gauge previously read the + // bounded channel's Count (which takes the channel's internal lock) and adjusted the + // metric under its own lock on every streamed event. Now the metric reads Count only + // when it is scraped (ObservableGauge callback) or projected (GetSnapshot), summing the + // live backlog across every registered subscriber — the same "buffered, not yet + // delivered" aggregate the per-event push reported, but with no per-event lock traffic. + // Disposing the registration in the finally removes this subscriber's contribution, so + // the gauge returns to the other subscribers' backlog (zero when none remain) on + // disconnect. CanCount guards a channel that ever cannot report Count (contributes 0). + IDisposable backlogRegistration = metrics.RegisterEventStreamBacklogSource( + () => subscriber.Reader.CanCount ? subscriber.Reader.Count : 0); + try { // Emit order for a resume: the ReplayGap sentinel FIRST (only when events were @@ -179,32 +191,21 @@ public sealed class EventStreamService( continue; } - // Queue-depth gauge tracks events the pump has fanned into this subscriber's - // channel but the client has not yet consumed — the same "buffered, not yet - // delivered" quantity the original per-RPC channel reported. The bounded - // subscriber channel supports counting, so reconcile the gauge to the current - // backlog; falling back to a no-op delta if a channel ever cannot count. - int backlog = subscriber.Reader.CanCount ? subscriber.Reader.Count : streamQueueDepth; - int delta = backlog - streamQueueDepth; - if (delta != 0) - { - streamQueueDepth = backlog; - metrics.AdjustGrpcEventStreamQueueDepth(delta); - } - + // The queue-depth gauge is maintained lazily via the backlog registration above + // (GWC-15): the metric reads this subscriber's channel Count only when scraped, + // so there is no per-event gauge bookkeeping on this hot path. yield return mxEvent; } } finally { await reader.DisposeAsync().ConfigureAwait(false); - subscriber.Dispose(); - if (streamQueueDepth != 0) - { - metrics.AdjustGrpcEventStreamQueueDepth(-streamQueueDepth); - streamQueueDepth = 0; - } + // Remove this subscriber's live backlog contribution before disposing the lease so + // the gauge stops counting a channel that is about to be completed; after this the + // gauge reflects only the remaining subscribers (zero when none remain). + backlogRegistration.Dispose(); + subscriber.Dispose(); metrics.StreamDisconnected("Detached"); } diff --git a/src/ZB.MOM.WW.MxGateway.Server/Grpc/MxAccessGatewayService.cs b/src/ZB.MOM.WW.MxGateway.Server/Grpc/MxAccessGatewayService.cs index 22675f1..7e756fa 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Grpc/MxAccessGatewayService.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Grpc/MxAccessGatewayService.cs @@ -152,9 +152,15 @@ public sealed class MxAccessGatewayService( .WithCancellation(context.CancellationToken) .ConfigureAwait(false)) { - Stopwatch stopwatch = Stopwatch.StartNew(); + // GWC-06: measure send latency with the allocation-free timestamp API rather + // than allocating a Stopwatch object per event per subscriber on the highest- + // volume gateway path. Stopwatch.GetTimestamp/GetElapsedTime measure the exact + // same wall-clock span the former Stopwatch.StartNew()/.Elapsed did. + long sendStartTimestamp = Stopwatch.GetTimestamp(); await responseStream.WriteAsync(publicEvent).ConfigureAwait(false); - metrics.RecordEventStreamSend(publicEvent.Family.ToString(), stopwatch.Elapsed); + metrics.RecordEventStreamSend( + publicEvent.Family.ToString(), + Stopwatch.GetElapsedTime(sendStartTimestamp)); } } catch (Exception exception) when (exception is not RpcException) diff --git a/src/ZB.MOM.WW.MxGateway.Server/Grpc/MxAccessGrpcMapper.cs b/src/ZB.MOM.WW.MxGateway.Server/Grpc/MxAccessGrpcMapper.cs index 4333b8c..ea6d0f7 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Grpc/MxAccessGrpcMapper.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Grpc/MxAccessGrpcMapper.cs @@ -65,7 +65,16 @@ public sealed class MxAccessGrpcMapper { ArgumentNullException.ThrowIfNull(workerEvent); - return workerEvent.Event?.Clone() ?? new MxEvent + // GWC-07 / IPC-05: ownership transfer, not a deep clone. The enclosing WorkerEvent is + // parsed fresh from a single pipe frame in WorkerClient's read loop and is discarded + // immediately after this mapping — the SessionEventDistributor pump is its single + // consumer (GWC-01 claims the worker event channel as single-reader), so nothing else + // aliases or mutates workerEvent.Event. We therefore move the inner MxEvent into the + // outbound graph instead of cloning it. Downstream the pump fans this one MxEvent to + // every subscriber and retains it in the replay ring, but that sharing is READ-ONLY + // (subscribers only yield/filter it), so a single shared instance is safe. If a second + // consumer of WorkerEvent is ever added, restore a .Clone() here to re-isolate. + return workerEvent.Event ?? new MxEvent { Family = MxEventFamily.Unspecified, RawStatus = "Worker event did not contain a public event payload.", diff --git a/src/ZB.MOM.WW.MxGateway.Server/Metrics/GatewayMetrics.cs b/src/ZB.MOM.WW.MxGateway.Server/Metrics/GatewayMetrics.cs index c42468f..859b73d 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Metrics/GatewayMetrics.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Metrics/GatewayMetrics.cs @@ -32,10 +32,17 @@ public sealed class GatewayMetrics : IDisposable private readonly ConcurrentDictionary _eventsBySession = new(StringComparer.Ordinal); private readonly Dictionary _retryAttemptsByArea = new(StringComparer.OrdinalIgnoreCase); + // GWC-15: live backlog sources for the gRPC event-stream queue-depth gauge. Each active + // StreamEvents subscriber registers a delegate returning its channel's current backlog; the + // gauge sums them ONLY when scraped or projected (GetGrpcEventStreamQueueDepth), so nothing is + // read on the per-event streaming hot path. Concurrent because subscribers register/unregister + // from arbitrary gRPC threads while the gauge callback / GetSnapshot enumerate. + private readonly ConcurrentDictionary> _eventStreamBacklogSources = new(); + private long _nextEventStreamBacklogSourceId; + private int _openSessions; private int _workersRunning; private int _workerEventQueueDepth; - private int _grpcEventStreamQueueDepth; private int _alarmProviderMode; private long _sessionsOpened; private long _sessionsClosed; @@ -292,15 +299,24 @@ public sealed class GatewayMetrics : IDisposable } /// - /// Adjusts the gRPC event stream queue depth by the given delta. + /// Registers a live backlog source for the gRPC event-stream queue-depth gauge and + /// returns a handle that removes it when disposed. The gauge sums every registered + /// source's current value on demand (when scraped or via ), + /// rather than tracking a pushed running total, so the streaming hot path does no + /// per-event gauge bookkeeping (GWC-15). /// - /// Amount to adjust the queue depth by. - public void AdjustGrpcEventStreamQueueDepth(int delta) + /// + /// Returns this subscriber's current channel backlog. Invoked only at collection time; + /// must be cheap and non-blocking (a bounded channel's Count). Negative returns + /// are clamped to zero when summed. + /// + /// A handle whose disposal unregisters the source. Safe to dispose more than once. + public IDisposable RegisterEventStreamBacklogSource(Func backlog) { - lock (_syncRoot) - { - _grpcEventStreamQueueDepth = Math.Max(0, _grpcEventStreamQueueDepth + delta); - } + ArgumentNullException.ThrowIfNull(backlog); + long id = Interlocked.Increment(ref _nextEventStreamBacklogSourceId); + _eventStreamBacklogSources[id] = backlog; + return new EventStreamBacklogRegistration(this, id); } /// @@ -429,13 +445,16 @@ public sealed class GatewayMetrics : IDisposable /// The current metrics snapshot. public GatewayMetricsSnapshot GetSnapshot() { + // Compute the live gRPC stream backlog outside _syncRoot: the sources are the subscriber + // channels' Count (their own locks) and must not run under this lock. GWC-15. + int grpcEventStreamQueueDepth = GetGrpcEventStreamQueueDepth(); lock (_syncRoot) { return new GatewayMetricsSnapshot( OpenSessions: _openSessions, WorkersRunning: _workersRunning, WorkerEventQueueDepth: _workerEventQueueDepth, - GrpcEventStreamQueueDepth: _grpcEventStreamQueueDepth, + GrpcEventStreamQueueDepth: grpcEventStreamQueueDepth, SessionsOpened: _sessionsOpened, SessionsClosed: _sessionsClosed, CommandsStarted: _commandsStarted, @@ -495,12 +514,28 @@ public sealed class GatewayMetrics : IDisposable } } + // Sums the live backlog across every registered event-stream subscriber. Runs at collection + // time (ObservableGauge scrape) or when GetSnapshot projects the value — never on the + // per-event path. Enumerating ConcurrentDictionary.Values never throws on concurrent + // register/unregister; a source removed mid-enumeration simply drops from this sample. private int GetGrpcEventStreamQueueDepth() { - lock (_syncRoot) + int total = 0; + foreach (Func source in _eventStreamBacklogSources.Values) { - return _grpcEventStreamQueueDepth; + int value = source(); + if (value > 0) + { + total += value; + } } + + return total; + } + + private void UnregisterEventStreamBacklogSource(long id) + { + _eventStreamBacklogSources.TryRemove(id, out _); } private int GetAlarmProviderMode() @@ -521,4 +556,19 @@ public sealed class GatewayMetrics : IDisposable { values.AddOrUpdate(key, 1, static (_, currentValue) => currentValue + 1); } + + // Handle returned by RegisterEventStreamBacklogSource. Disposal (once) removes the source + // from the gauge's live sum. Idempotent so a double dispose from a stream teardown is safe. + private sealed class EventStreamBacklogRegistration(GatewayMetrics metrics, long id) : IDisposable + { + private int _disposed; + + public void Dispose() + { + if (Interlocked.Exchange(ref _disposed, 1) == 0) + { + metrics.UnregisterEventStreamBacklogSource(id); + } + } + } } diff --git a/src/ZB.MOM.WW.MxGateway.Server/Sessions/SessionEventDistributor.cs b/src/ZB.MOM.WW.MxGateway.Server/Sessions/SessionEventDistributor.cs index 8d94da3..29e910a 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Sessions/SessionEventDistributor.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Sessions/SessionEventDistributor.cs @@ -98,14 +98,21 @@ public sealed class SessionEventDistributor : IAsyncDisposable private readonly object _lifecycleLock = new(); // Replay ring buffer. Appended on the pump thread and queried from arbitrary - // threads via TryGetReplayFrom, so every access is under _replayLock. The deque - // keeps events in ascending WorkerSequence order (the pump fans in source order), - // so the oldest retained event is always at the front. Capacity == 0 disables - // retention; RetentionSeconds <= 0 disables age-based eviction. + // threads via TryGetReplayFrom, so every access is under _replayLock. Backed by a + // fixed-size circular array preallocated to the capacity so appending a retained + // event allocates no node (the LinkedList this replaced allocated one node per + // event on the fan-out hot path). Events are kept in ascending WorkerSequence order + // (the pump fans in source order): _replayHead is the logical front (oldest retained + // event) and _replayCount entries follow it, wrapping modulo the array length. The + // logical entry at position i is _replayBuffer[(_replayHead + i) % Length]. Capacity + // == 0 disables retention (the array is empty and never indexed); RetentionSeconds + // <= 0 disables age-based eviction. private readonly int _replayBufferCapacity; private readonly TimeSpan _replayRetention; private readonly bool _ageEvictionEnabled; - private readonly LinkedList _replayBuffer = new(); + private readonly ReplayEntry[] _replayBuffer; + private int _replayHead; + private int _replayCount; private readonly object _replayLock = new(); private bool _anyEventSeen; private ulong _highestSequenceSeen; @@ -245,6 +252,7 @@ public sealed class SessionEventDistributor : IAsyncDisposable _overflowHandler = overflowHandler; _shutdownTimeout = DefaultShutdownTimeout; _replayBufferCapacity = replayBufferCapacity; + _replayBuffer = new ReplayEntry[replayBufferCapacity]; _ageEvictionEnabled = replayRetentionSeconds > 0; _replayRetention = _ageEvictionEnabled ? TimeSpan.FromSeconds(replayRetentionSeconds) @@ -452,24 +460,25 @@ public sealed class SessionEventDistributor : IAsyncDisposable List newer = []; ulong highestReplayed = afterSequence; - if (_replayBuffer.Count == 0) + if (_replayCount == 0) { gap = _anyEventSeen && afterSequence < _highestSequenceSeen; oldestAvailableSequence = 0; // meaningful only when gap == true; 0 here since nothing is retained } else { - ulong oldestRetained = _replayBuffer.First!.Value.Event.WorkerSequence; + ulong oldestRetained = ReplayEntryAt(0).Event.WorkerSequence; gap = oldestRetained > 0 && afterSequence < oldestRetained - 1; // Per the contract on OldestAvailableSequence: meaningful only when gap == true. oldestAvailableSequence = gap ? oldestRetained : 0; - foreach (ReplayEntry entry in _replayBuffer) + for (int i = 0; i < _replayCount; i++) { - if (entry.Event.WorkerSequence > afterSequence) + MxEvent retained = ReplayEntryAt(i).Event; + if (retained.WorkerSequence > afterSequence) { - newer.Add(entry.Event); - highestReplayed = entry.Event.WorkerSequence; + newer.Add(retained); + highestReplayed = retained.WorkerSequence; } } } @@ -729,7 +738,7 @@ public sealed class SessionEventDistributor : IAsyncDisposable { EvictAged(); - if (_replayBuffer.Count == 0) + if (_replayCount == 0) { events = []; // Nothing retained. The caller missed events only if it is behind the @@ -738,7 +747,7 @@ public sealed class SessionEventDistributor : IAsyncDisposable return true; } - ulong oldestRetained = _replayBuffer.First!.Value.Event.WorkerSequence; + ulong oldestRetained = ReplayEntryAt(0).Event.WorkerSequence; // A gap exists when at least one event newer than afterSequence was evicted, // i.e. afterSequence sits below the oldest-retained-minus-one boundary. @@ -750,11 +759,12 @@ public sealed class SessionEventDistributor : IAsyncDisposable // O(n) scan over the retained buffer — acceptable because TryGetReplayFrom // is only called on subscriber reconnect, never on the hot fan-out path. List newer = []; - foreach (ReplayEntry entry in _replayBuffer) + for (int i = 0; i < _replayCount; i++) { - if (entry.Event.WorkerSequence > afterSequence) + MxEvent retained = ReplayEntryAt(i).Event; + if (retained.WorkerSequence > afterSequence) { - newer.Add(entry.Event); + newer.Add(retained); } } @@ -780,30 +790,43 @@ public sealed class SessionEventDistributor : IAsyncDisposable return; } - _replayBuffer.AddLast(new ReplayEntry(mxEvent, _timeProvider.GetUtcNow())); - - // Capacity eviction: drop oldest until within bound. - while (_replayBuffer.Count > _replayBufferCapacity) + // Append at the logical tail. When the ring is full the oldest entry is + // overwritten in place (its slot becomes the new tail) and the head advances, + // so the newest _replayBufferCapacity events are retained with no allocation. + ReplayEntry entry = new(mxEvent, _timeProvider.GetUtcNow()); + if (_replayCount < _replayBufferCapacity) { - _replayBuffer.RemoveFirst(); + _replayBuffer[(_replayHead + _replayCount) % _replayBufferCapacity] = entry; + _replayCount++; + } + else + { + _replayBuffer[_replayHead] = entry; + _replayHead = (_replayHead + 1) % _replayBufferCapacity; } EvictAged(); } } - // Must be called under _replayLock. Drops entries older than the retention window. + // Returns the logical entry at position i (0 == oldest retained). Must be called + // under _replayLock with 0 <= i < _replayCount (so the array length is nonzero). + private ReplayEntry ReplayEntryAt(int i) => _replayBuffer[(_replayHead + i) % _replayBuffer.Length]; + + // Must be called under _replayLock. Drops entries older than the retention window + // by advancing the head past them (no dealloc; slots are reused on the next append). private void EvictAged() { - if (!_ageEvictionEnabled || _replayBuffer.Count == 0) + if (!_ageEvictionEnabled || _replayCount == 0) { return; } DateTimeOffset cutoff = _timeProvider.GetUtcNow() - _replayRetention; - while (_replayBuffer.First is { } first && first.Value.RetainedAt < cutoff) + while (_replayCount > 0 && _replayBuffer[_replayHead].RetainedAt < cutoff) { - _replayBuffer.RemoveFirst(); + _replayHead = (_replayHead + 1) % _replayBuffer.Length; + _replayCount--; } } diff --git a/src/ZB.MOM.WW.MxGateway.Server/Workers/WorkerClient.cs b/src/ZB.MOM.WW.MxGateway.Server/Workers/WorkerClient.cs index 342ae6d..0a61536 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Workers/WorkerClient.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Workers/WorkerClient.cs @@ -997,9 +997,14 @@ public sealed class WorkerClient : IWorkerClient string correlationId, WorkerCommand command) { + // IPC-05: no second clone. MxAccessGrpcMapper.MapCommand already deep-cloned the command + // out of the caller-owned gRPC MxCommandRequest, so this WorkerCommand is a fresh graph + // owned by the invoke pipeline and referenced by no other consumer. The envelope is built + // and owned entirely inside WorkerClient and the command is not touched again after this + // point, so transferring it into the envelope introduces no aliasing hazard. return CreateEnvelope( correlationId, - envelope => envelope.WorkerCommand = command.Clone()); + envelope => envelope.WorkerCommand = command); } /// Creates shutdown envelope. diff --git a/src/ZB.MOM.WW.MxGateway.Server/Workers/WorkerFrameReader.cs b/src/ZB.MOM.WW.MxGateway.Server/Workers/WorkerFrameReader.cs index 076d5e9..95cbe13 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Workers/WorkerFrameReader.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Workers/WorkerFrameReader.cs @@ -1,3 +1,4 @@ +using System.Buffers; using System.Buffers.Binary; using Google.Protobuf; using ZB.MOM.WW.MxGateway.Contracts.Proto; @@ -47,20 +48,34 @@ public sealed class WorkerFrameReader $"Worker frame payload length {payloadLength} exceeds the configured maximum of {_options.MaxMessageBytes} bytes."); } - byte[] payload = new byte[payloadLength]; - await ReadExactlyOrThrowAsync(payload, cancellationToken).ConfigureAwait(false); - + // Rent the payload buffer from the shared pool rather than allocating a fresh byte[] per + // frame; large event frames near the cap would otherwise allocate an LOH buffer each time + // (IPC-14). ParseFrom copies whatever it needs into the parsed message, so the rented buffer + // can be returned as soon as parsing completes without the envelope aliasing it. The rented + // buffer may be larger than requested, so the read and parse are bounded to length. + int length = checked((int)payloadLength); + byte[] payload = ArrayPool.Shared.Rent(length); WorkerEnvelope envelope; try { - envelope = WorkerEnvelope.Parser.ParseFrom(payload); + await ReadExactlyOrThrowAsync(new Memory(payload, 0, length), cancellationToken) + .ConfigureAwait(false); + + try + { + envelope = WorkerEnvelope.Parser.ParseFrom(payload, 0, length); + } + catch (InvalidProtocolBufferException exception) + { + throw new WorkerFrameProtocolException( + WorkerFrameProtocolErrorCode.InvalidEnvelope, + "Worker frame payload is not a valid WorkerEnvelope protobuf message.", + exception); + } } - catch (InvalidProtocolBufferException exception) + finally { - throw new WorkerFrameProtocolException( - WorkerFrameProtocolErrorCode.InvalidEnvelope, - "Worker frame payload is not a valid WorkerEnvelope protobuf message.", - exception); + ArrayPool.Shared.Return(payload); } WorkerEnvelopeValidator.Validate(envelope, _options); diff --git a/src/ZB.MOM.WW.MxGateway.Server/Workers/WorkerFrameWriter.cs b/src/ZB.MOM.WW.MxGateway.Server/Workers/WorkerFrameWriter.cs index 728a611..124e58f 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Workers/WorkerFrameWriter.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Workers/WorkerFrameWriter.cs @@ -1,3 +1,4 @@ +using System.Buffers; using System.Buffers.Binary; using Google.Protobuf; using ZB.MOM.WW.MxGateway.Contracts.Proto; @@ -53,10 +54,25 @@ public sealed class WorkerFrameWriter $"Worker envelope payload length {payloadLength} exceeds the configured maximum of {_options.MaxMessageBytes} bytes."); } - byte[] lengthPrefix = new byte[sizeof(uint)]; - BinaryPrimitives.WriteUInt32LittleEndian(lengthPrefix, (uint)payloadLength); + // Serialize once into a single pooled buffer that carries the 4-byte little-endian length + // prefix followed by the payload, then issue one stream write. This avoids a second + // serialization pass (ToByteArray re-runs CalculateSize), a separate prefix array and its + // own write, and any per-frame heap allocation (GWC-08, IPC-13). The rented buffer may be + // larger than requested, so only the first frameLength bytes are ever written. + int frameLength = sizeof(uint) + payloadLength; + byte[] frame = ArrayPool.Shared.Rent(frameLength); + try + { + BinaryPrimitives.WriteUInt32LittleEndian(frame, (uint)payloadLength); + envelope.WriteTo(new Span(frame, sizeof(uint), payloadLength)); - await _stream.WriteAsync(lengthPrefix, cancellationToken).ConfigureAwait(false); - await _stream.WriteAsync(envelope.ToByteArray(), cancellationToken).ConfigureAwait(false); + await _stream + .WriteAsync(new ReadOnlyMemory(frame, 0, frameLength), cancellationToken) + .ConfigureAwait(false); + } + finally + { + ArrayPool.Shared.Return(frame); + } } } diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/DashboardCookieOptionsTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/DashboardCookieOptionsTests.cs index 435a859..bbfeb6b 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/DashboardCookieOptionsTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/DashboardCookieOptionsTests.cs @@ -10,7 +10,12 @@ namespace ZB.MOM.WW.MxGateway.Tests.Gateway.Dashboard; public sealed class DashboardCookieOptionsTests { - /// Verifies that the application configures secure dashboard authentication cookies. + /// + /// Verifies that the application configures secure dashboard authentication cookies. + /// With RequireHttpsCookie defaulting to and no explicit + /// CookieName override, the cookie is named with the __Host- prefix so + /// browsers enforce the Secure/no-Domain/Path=/ guarantees the prefix promises. + /// /// A task that represents the asynchronous operation. [Fact] public async Task Build_ConfiguresSecureDashboardCookie() @@ -22,7 +27,7 @@ public sealed class DashboardCookieOptionsTests CookieAuthenticationOptions options = optionsMonitor.Get( DashboardAuthenticationDefaults.AuthenticationScheme); - Assert.Equal(DashboardAuthenticationDefaults.CookieName, options.Cookie.Name); + Assert.Equal(DashboardAuthenticationDefaults.SecureCookieName, options.Cookie.Name); Assert.True(options.Cookie.HttpOnly); Assert.Equal(CookieSecurePolicy.Always, options.Cookie.SecurePolicy); Assert.Equal(SameSiteMode.Strict, options.Cookie.SameSite); @@ -35,11 +40,14 @@ public sealed class DashboardCookieOptionsTests /// /// Verifies that setting MxGateway:Dashboard:RequireHttpsCookie=false /// relaxes the cookie to so - /// the dashboard can be reached over plain HTTP in dev. + /// the dashboard can be reached over plain HTTP in dev, and that the plain + /// is used rather than the + /// __Host- name — a __Host- cookie without a guaranteed Secure flag is + /// silently dropped by browsers. /// /// A task that represents the asynchronous operation. [Fact] - public async Task Build_WithRequireHttpsCookieFalse_UsesSameAsRequest() + public async Task Build_WithRequireHttpsCookieFalse_UsesSameAsRequestAndPlainName() { await using WebApplication app = GatewayApplication.Build( ["--MxGateway:Dashboard:RequireHttpsCookie=false"]); @@ -50,12 +58,14 @@ public sealed class DashboardCookieOptionsTests DashboardAuthenticationDefaults.AuthenticationScheme); Assert.Equal(CookieSecurePolicy.SameAsRequest, options.Cookie.SecurePolicy); + Assert.Equal(DashboardAuthenticationDefaults.CookieName, options.Cookie.Name); } /// - /// Verifies that MxGateway:Dashboard:CookieName overrides the dashboard auth - /// cookie name, so a gateway instance sharing a hostname with another can be given a - /// distinct name (browser cookies are scoped by host+path, not port). + /// Verifies that an explicit MxGateway:Dashboard:CookieName override wins over the + /// secure __Host- default (this build leaves RequireHttpsCookie at its + /// default), so a gateway instance sharing a hostname with another + /// can be given a distinct name (browser cookies are scoped by host+path, not port). /// /// A task that represents the asynchronous operation. [Fact] diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/DashboardEventBroadcasterTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/DashboardEventBroadcasterTests.cs new file mode 100644 index 0000000..7ec3c86 --- /dev/null +++ b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/DashboardEventBroadcasterTests.cs @@ -0,0 +1,171 @@ +using Microsoft.AspNetCore.SignalR; +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; +using ZB.MOM.WW.MxGateway.Contracts.Proto; +using ZB.MOM.WW.MxGateway.Server.Configuration; +using ZB.MOM.WW.MxGateway.Server.Dashboard.Hubs; + +namespace ZB.MOM.WW.MxGateway.Tests.Gateway.Dashboard; + +/// +/// Verifies that honours +/// MxGateway:Dashboard:ShowTagValues (SEC-25): tag values are stripped +/// from the mirrored copy when the flag is off, present when it is on, and the +/// shared source event is never mutated. +/// +public sealed class DashboardEventBroadcasterTests +{ + /// Values are stripped from the mirror when ShowTagValues is off; metadata survives. + [Fact] + public void Publish_WhenShowTagValuesFalse_RedactsValuesButKeepsMetadata() + { + CapturingHubContext hubContext = new(); + DashboardEventBroadcaster broadcaster = Create(hubContext, showTagValues: false); + MxEvent source = BuildEventWithValue(); + + broadcaster.Publish("session-1", source); + + MxEvent sent = Assert.IsType(hubContext.LastArgument); + Assert.Null(sent.Value); + Assert.Null(sent.OnAlarmTransition.CurrentValue); + Assert.Null(sent.OnAlarmTransition.LimitValue); + + // Metadata unrelated to the value survives redaction. + Assert.Equal("session-1", sent.SessionId); + Assert.Equal(7, sent.ServerHandle); + Assert.Equal(11, sent.ItemHandle); + Assert.Equal(192, sent.Quality); + Assert.Equal("Tank01.Level.HiHi", sent.OnAlarmTransition.AlarmFullReference); + } + + /// Redaction applies to a clone, so the shared source event keeps its values. + [Fact] + public void Publish_WhenShowTagValuesFalse_DoesNotMutateSourceEvent() + { + CapturingHubContext hubContext = new(); + DashboardEventBroadcaster broadcaster = Create(hubContext, showTagValues: false); + MxEvent source = BuildEventWithValue(); + + broadcaster.Publish("session-1", source); + + // The redaction must apply to a clone; the shared source keeps its values. + Assert.NotNull(source.Value); + Assert.Equal(42.5, source.Value.DoubleValue); + Assert.NotNull(source.OnAlarmTransition.CurrentValue); + Assert.NotNull(source.OnAlarmTransition.LimitValue); + Assert.NotSame(source, hubContext.LastArgument); + } + + /// Values pass through unredacted when ShowTagValues is on. + [Fact] + public void Publish_WhenShowTagValuesTrue_KeepsValues() + { + CapturingHubContext hubContext = new(); + DashboardEventBroadcaster broadcaster = Create(hubContext, showTagValues: true); + MxEvent source = BuildEventWithValue(); + + broadcaster.Publish("session-1", source); + + MxEvent sent = Assert.IsType(hubContext.LastArgument); + Assert.NotNull(sent.Value); + Assert.Equal(42.5, sent.Value.DoubleValue); + Assert.NotNull(sent.OnAlarmTransition.CurrentValue); + Assert.NotNull(sent.OnAlarmTransition.LimitValue); + } + + private static DashboardEventBroadcaster Create(CapturingHubContext hubContext, bool showTagValues) + { + GatewayOptions gatewayOptions = new() + { + Dashboard = new DashboardOptions { ShowTagValues = showTagValues }, + }; + + return new DashboardEventBroadcaster( + hubContext, + Options.Create(gatewayOptions), + NullLogger.Instance); + } + + private static MxEvent BuildEventWithValue() + { + return new MxEvent + { + Family = MxEventFamily.OnAlarmTransition, + SessionId = "session-1", + ServerHandle = 7, + ItemHandle = 11, + Quality = 192, + Value = new MxValue { DataType = MxDataType.Double, DoubleValue = 42.5 }, + OnAlarmTransition = new OnAlarmTransitionEvent + { + AlarmFullReference = "Tank01.Level.HiHi", + CurrentValue = new MxValue { DataType = MxDataType.Double, DoubleValue = 88.0 }, + LimitValue = new MxValue { DataType = MxDataType.Double, DoubleValue = 90.0 }, + }, + }; + } + + private sealed class CapturingHubContext : IHubContext + { + private readonly CapturingHubClients _clients = new(); + + /// Gets the hub clients. + public IHubClients Clients => _clients; + + /// Gets the group manager. + public IGroupManager Groups { get; } = new NoopGroupManager(); + + /// Gets the first argument of the most recent send call. + public object? LastArgument => _clients.GroupProxy.LastArgument; + } + + private sealed class CapturingHubClients : IHubClients + { + /// Gets the capturing client proxy shared by this fake. + public CapturingClientProxy GroupProxy { get; } = new(); + + public IClientProxy All => GroupProxy; + + public IClientProxy AllExcept(IReadOnlyList excludedConnectionIds) => GroupProxy; + + public IClientProxy Client(string connectionId) => GroupProxy; + + public IClientProxy Clients(IReadOnlyList connectionIds) => GroupProxy; + + public IClientProxy Group(string groupName) => GroupProxy; + + public IClientProxy GroupExcept(string groupName, IReadOnlyList excludedConnectionIds) => GroupProxy; + + public IClientProxy Groups(IReadOnlyList groupNames) => GroupProxy; + + public IClientProxy User(string userId) => GroupProxy; + + public IClientProxy Users(IReadOnlyList userIds) => GroupProxy; + } + + private sealed class CapturingClientProxy : IClientProxy + { + /// Gets the first argument of the most recent send call. + public object? LastArgument { get; private set; } + + /// Records the send call arguments and completes synchronously. + /// The SignalR method name. + /// The method arguments. + /// Token to observe for cancellation. + /// A completed task. + public Task SendCoreAsync(string method, object?[] args, CancellationToken cancellationToken = default) + { + LastArgument = args.Length > 0 ? args[0] : null; + return Task.CompletedTask; + } + } + + private sealed class NoopGroupManager : IGroupManager + { + public Task AddToGroupAsync(string connectionId, string groupName, CancellationToken cancellationToken = default) + => Task.CompletedTask; + + public Task RemoveFromGroupAsync(string connectionId, string groupName, CancellationToken cancellationToken = default) + => Task.CompletedTask; + } +} diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/GatewayEndToEndReconnectReplayTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/GatewayEndToEndReconnectReplayTests.cs new file mode 100644 index 0000000..424bd66 --- /dev/null +++ b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/GatewayEndToEndReconnectReplayTests.cs @@ -0,0 +1,642 @@ +using Google.Protobuf.WellKnownTypes; +using Grpc.Core; +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; +using ZB.MOM.WW.MxGateway.Contracts; +using ZB.MOM.WW.MxGateway.Contracts.Proto; +using ZB.MOM.WW.MxGateway.Server.Configuration; +using ZB.MOM.WW.MxGateway.Server.Grpc; +using ZB.MOM.WW.MxGateway.Server.Metrics; +using ZB.MOM.WW.MxGateway.Server.Security.Authentication; +using ZB.MOM.WW.MxGateway.Server.Security.Authorization; +using ZB.MOM.WW.MxGateway.Server.Sessions; +using ZB.MOM.WW.MxGateway.Server.Workers; +using ZB.MOM.WW.MxGateway.Tests.Gateway.Workers.Fakes; +using ZB.MOM.WW.MxGateway.Tests.TestSupport; + +namespace ZB.MOM.WW.MxGateway.Tests.Gateway; + +/// +/// End-to-end reconnect/replay tests through the real gRPC StreamEvents path via the +/// fake worker harness (TST-01, server half). A single subscriber detaches (cancel + await +/// the stream task so its lease is fully disposed), the session is retained by detach-grace +/// while the distributor pump keeps appending worker events to the replay ring, and a second +/// stream reconnects with . Covers both +/// the no-gap resume (cursor inside the retained window) and the ReplayGap sentinel +/// (cursor predates the oldest retained event after capacity eviction). +/// +/// +/// These tests run in single-subscriber mode (AllowMultipleEventSubscribers=false). +/// Reconnect works because the first stream is FULLY detached before the reconnect attaches: +/// awaiting the first stream task runs EventStreamService's finally block, which +/// disposes the subscriber lease and drops the session's active-subscriber count back to +/// zero, so the reconnect's AttachEventSubscriberWithReplay sees an empty slot rather +/// than an "already active" rejection. The distributor (and its replay ring) is created once +/// per session and survives detach, so events emitted while no subscriber is attached are +/// retained for the reconnecting stream. +/// +public sealed class GatewayEndToEndReconnectReplayTests +{ + private static readonly TimeSpan TestTimeout = TimeSpan.FromSeconds(10); + private const int ServerHandle = 3001; + private const int ItemHandle = 4002; + + /// + /// Reconnecting inside the retained window replays exactly the events newer than the + /// resume cursor — the retained tail of the first batch plus the events emitted while + /// detached — in strictly ascending order, with no duplicates and no ReplayGap + /// sentinel. + /// + /// A task that represents the asynchronous operation. + [Fact] + public async Task StreamEvents_ReconnectInsideRetainedWindow_ReplaysTailNoGap() + { + const int firstBatch = 4; + const int secondBatch = 3; + + GatedEventFakeWorkerProcessLauncher launcher = new(); + // Capacity 16 retains every event emitted here (7 total), so nothing is evicted and a + // resume from a middle cursor never produces a gap. + await using ReconnectReplayGatewayServiceFixture fixture = new(launcher, replayBufferCapacity: 16); + + string sessionId = await OpenSessionAsync(fixture, "reconnect-no-gap"); + + // ---- first connection: receive the first batch, capture its real sequences ---- + using CancellationTokenSource writer1Cts = new(); + RecordingServerStreamWriter writer1 = new(); + Task stream1Task = Task.Run(async () => + await fixture.Service.StreamEvents( + new StreamEventsRequest { SessionId = sessionId }, + writer1, + new TestServerCallContext(cancellationToken: writer1Cts.Token))); + + await fixture.WaitForSubscriberCountAsync(sessionId, n: 1, TestTimeout); + await WireUpAdviseAsync(fixture, sessionId); + + for (int i = 0; i < firstBatch; i++) + { + launcher.AllowNextEvent(); + } + + IReadOnlyList batch1 = await writer1.WaitForMessageCountAsync(firstBatch, TestTimeout); + ulong[] batch1Sequences = batch1.Select(e => e.WorkerSequence).ToArray(); + + // Resume cursor: a middle event of the first batch. Everything strictly newer than this + // must be redelivered to the reconnecting stream. + ulong cursor = batch1Sequences[1]; + int retainedTailFromBatch1 = batch1Sequences.Count(s => s > cursor); + Assert.Equal(2, retainedTailFromBatch1); // sanity: events at index 2 and 3 + + // ---- fully detach writer1 (cancel + await so its lease is disposed) ---- + await DetachAsync(writer1Cts, stream1Task); + + // ---- emit more events while detached; the ring must retain them for the reconnect ---- + for (int i = 0; i < secondBatch; i++) + { + launcher.AllowNextEvent(); + } + + // ---- reconnect with the middle cursor ---- + RecordingServerStreamWriter writer2 = new(); + Task stream2Task = Task.Run(async () => + await fixture.Service.StreamEvents( + new StreamEventsRequest { SessionId = sessionId, AfterWorkerSequence = cursor }, + writer2, + new TestServerCallContext())); + + await fixture.WaitForSubscriberCountAsync(sessionId, n: 1, TestTimeout); + + int expected = retainedTailFromBatch1 + secondBatch; + IReadOnlyList resumed = await writer2.WaitForMessageCountAsync(expected, TestTimeout); + + // ---- tear down before asserting so a hang can't wedge the run ---- + launcher.StopEmitting(); + await CloseAndDrainAsync(fixture, sessionId, stream2Task, launcher); + + // ---- assertions ---- + Assert.Equal(expected, resumed.Count); + + // No sentinel: every message is a real event, none carries a ReplayGap. + Assert.All(resumed, e => Assert.Null(e.ReplayGap)); + + // Every replayed/live event is strictly newer than the cursor. + Assert.All(resumed, e => Assert.True( + e.WorkerSequence > cursor, + $"Event sequence {e.WorkerSequence} is not newer than cursor {cursor}.")); + + // Strictly ascending, no duplicates. + for (int i = 1; i < resumed.Count; i++) + { + Assert.True( + resumed[i].WorkerSequence > resumed[i - 1].WorkerSequence, + $"Sequences must be strictly ascending: {resumed[i - 1].WorkerSequence} then {resumed[i].WorkerSequence}."); + } + + Assert.Equal(resumed.Count, resumed.Select(e => e.WorkerSequence).Distinct().Count()); + + // The retained tail of the first batch (the two events newer than the cursor) is + // replayed first, before the events emitted while detached. + Assert.Equal(batch1Sequences[2], resumed[0].WorkerSequence); + Assert.Equal(batch1Sequences[3], resumed[1].WorkerSequence); + } + + /// + /// Reconnecting with a cursor that predates the oldest retained event (after capacity + /// eviction) yields the ReplayGap sentinel FIRST — family unspecified, no body, no + /// per-item fields, correct requested/oldest sequences — followed by exactly the retained + /// tail, in ascending order. + /// + /// A task that represents the asynchronous operation. + [Fact] + public async Task StreamEvents_ReconnectWithStaleCursor_EmitsReplayGapSentinelFirst() + { + const int capacity = 3; + const int totalEvents = 6; + + GatedEventFakeWorkerProcessLauncher launcher = new(); + // Small capacity forces eviction: with 6 events emitted and a 3-slot ring, the first 3 + // are evicted and only the newest 3 remain replayable. + await using ReconnectReplayGatewayServiceFixture fixture = new(launcher, replayBufferCapacity: capacity); + + string sessionId = await OpenSessionAsync(fixture, "reconnect-gap"); + + using CancellationTokenSource writer1Cts = new(); + RecordingServerStreamWriter writer1 = new(); + Task stream1Task = Task.Run(async () => + await fixture.Service.StreamEvents( + new StreamEventsRequest { SessionId = sessionId }, + writer1, + new TestServerCallContext(cancellationToken: writer1Cts.Token))); + + await fixture.WaitForSubscriberCountAsync(sessionId, n: 1, TestTimeout); + await WireUpAdviseAsync(fixture, sessionId); + + // Emit more events than the ring can hold. Waiting for writer1 to receive all of them + // proves the pump has appended (and evicted) every event, so the ring is settled to its + // final newest-`capacity` contents before we reconnect. + for (int i = 0; i < totalEvents; i++) + { + launcher.AllowNextEvent(); + } + + IReadOnlyList all = await writer1.WaitForMessageCountAsync(totalEvents, TestTimeout); + ulong[] sequences = all.Select(e => e.WorkerSequence).ToArray(); + + // With capacity C and N total events, the newest C are retained; the oldest still + // available is the (N-C+1)th event — index N-C in emission (ascending) order. + ulong oldestAvailable = sequences[totalEvents - capacity]; + + await DetachAsync(writer1Cts, stream1Task); + + // Resume after sequence 1: the real event sequences are envelope-numbered and start well + // above 1 (startup + command-reply envelopes consume the low numbers), so cursor 1 + // predates every event and, with the oldest three already evicted, forces a gap. + const ulong staleCursor = 1; + RecordingServerStreamWriter writer2 = new(); + Task stream2Task = Task.Run(async () => + await fixture.Service.StreamEvents( + new StreamEventsRequest { SessionId = sessionId, AfterWorkerSequence = staleCursor }, + writer2, + new TestServerCallContext())); + + await fixture.WaitForSubscriberCountAsync(sessionId, n: 1, TestTimeout); + + // sentinel + the retained tail (capacity events, all newer than the stale cursor). + int expected = 1 + capacity; + IReadOnlyList resumed = await writer2.WaitForMessageCountAsync(expected, TestTimeout); + + launcher.StopEmitting(); + await CloseAndDrainAsync(fixture, sessionId, stream2Task, launcher); + + // ---- the first message is the ReplayGap sentinel ---- + Assert.Equal(expected, resumed.Count); + + MxEvent sentinel = resumed[0]; + Assert.NotNull(sentinel.ReplayGap); + Assert.Equal(MxEventFamily.Unspecified, sentinel.Family); + Assert.Equal(MxEvent.BodyOneofCase.None, sentinel.BodyCase); + Assert.Equal(sessionId, sentinel.SessionId); + Assert.Equal(staleCursor, sentinel.ReplayGap.RequestedAfterSequence); + Assert.Equal(oldestAvailable, sentinel.ReplayGap.OldestAvailableSequence); + + // ---- the sentinel is followed by the retained tail, ascending, no further sentinels ---- + IReadOnlyList tail = resumed.Skip(1).ToArray(); + Assert.Equal(capacity, tail.Count); + Assert.All(tail, e => Assert.Null(e.ReplayGap)); + Assert.All(tail, e => Assert.True( + e.WorkerSequence >= oldestAvailable, + $"Retained event {e.WorkerSequence} is older than the oldest available {oldestAvailable}.")); + + ulong[] expectedTail = sequences.Skip(totalEvents - capacity).ToArray(); + Assert.Equal(expectedTail, tail.Select(e => e.WorkerSequence).ToArray()); + } + + // ---- shared flow helpers ---- + + private static async Task OpenSessionAsync( + ReconnectReplayGatewayServiceFixture fixture, + string name) + { + OpenSessionReply openReply = await fixture.Service.OpenSession( + new OpenSessionRequest + { + ClientSessionName = name, + ClientCorrelationId = $"open-{name}", + CommandTimeout = Duration.FromTimeSpan(TestTimeout), + }, + new TestServerCallContext()).ConfigureAwait(false); + + Assert.Equal(ProtocolStatusCode.Ok, openReply.ProtocolStatus.Code); + return openReply.SessionId; + } + + private static async Task WireUpAdviseAsync( + ReconnectReplayGatewayServiceFixture fixture, + string sessionId) + { + MxCommandReply registerReply = await fixture.Service.Invoke( + CreateRegisterRequest(sessionId), + new TestServerCallContext()).ConfigureAwait(false); + Assert.Equal(ProtocolStatusCode.Ok, registerReply.ProtocolStatus.Code); + + MxCommandReply addItemReply = await fixture.Service.Invoke( + CreateAddItemRequest(sessionId, registerReply.Register.ServerHandle), + new TestServerCallContext()).ConfigureAwait(false); + Assert.Equal(ProtocolStatusCode.Ok, addItemReply.ProtocolStatus.Code); + + MxCommandReply adviseReply = await fixture.Service.Invoke( + CreateAdviseRequest(sessionId, registerReply.Register.ServerHandle, addItemReply.AddItem.ItemHandle), + new TestServerCallContext()).ConfigureAwait(false); + Assert.Equal(ProtocolStatusCode.Ok, adviseReply.ProtocolStatus.Code); + } + + // Cancels the stream's token and awaits the stream task. Awaiting is load-bearing: it lets + // EventStreamService's finally block dispose the subscriber lease (dropping the session's + // active-subscriber count to zero) BEFORE the reconnect attaches, so the reconnect never + // races a still-registered subscriber. + private static async Task DetachAsync(CancellationTokenSource cts, Task streamTask) + { + await cts.CancelAsync().ConfigureAwait(false); + try + { + await streamTask.WaitAsync(TestTimeout).ConfigureAwait(false); + } + catch (OperationCanceledException) + { + // Expected: the iterator surfaces the cancellation. + } + catch (RpcException rpc) when (rpc.StatusCode == StatusCode.Cancelled) + { + // Also acceptable depending on gRPC exception wrapping. + } + } + + private static async Task CloseAndDrainAsync( + ReconnectReplayGatewayServiceFixture fixture, + string sessionId, + Task streamTask, + GatedEventFakeWorkerProcessLauncher launcher) + { + await fixture.Service.CloseSession( + new CloseSessionRequest { SessionId = sessionId, ClientCorrelationId = "close-reconnect" }, + new TestServerCallContext()).ConfigureAwait(false); + + try + { + await streamTask.WaitAsync(TestTimeout).ConfigureAwait(false); + } + catch (OperationCanceledException) + { + } + + await launcher.WorkerTask.WaitAsync(TestTimeout).ConfigureAwait(false); + } + + // ---- request builders ---- + + private static MxCommandRequest CreateRegisterRequest(string sessionId) => + new() + { + SessionId = sessionId, + ClientCorrelationId = "register-rr", + Command = new MxCommand + { + Kind = MxCommandKind.Register, + Register = new RegisterCommand { ClientName = "reconnect-replay-e2e-client" }, + }, + }; + + private static MxCommandRequest CreateAddItemRequest(string sessionId, int serverHandle) => + new() + { + SessionId = sessionId, + ClientCorrelationId = "add-item-rr", + Command = new MxCommand + { + Kind = MxCommandKind.AddItem, + AddItem = new AddItemCommand + { + ServerHandle = serverHandle, + ItemDefinition = "Galaxy.Tag.Value", + }, + }, + }; + + private static MxCommandRequest CreateAdviseRequest( + string sessionId, + int serverHandle, + int itemHandle) => + new() + { + SessionId = sessionId, + ClientCorrelationId = "advise-rr", + Command = new MxCommand + { + Kind = MxCommandKind.Advise, + Advise = new AdviseCommand { ServerHandle = serverHandle, ItemHandle = itemHandle }, + }, + }; + + private static void ConfigureCommandReply(MxCommandReply reply, MxCommandKind kind) + { + switch (kind) + { + case MxCommandKind.Register: + reply.Register = new RegisterReply { ServerHandle = ServerHandle }; + break; + case MxCommandKind.AddItem: + reply.AddItem = new AddItemReply { ItemHandle = ItemHandle }; + break; + } + } + + // ---- fixture ---- + + /// + /// Gateway service fixture in single-subscriber mode with a configurable replay ring + /// capacity, so each test can retain the whole event history (no gap) or force capacity + /// eviction (gap). + /// + private sealed class ReconnectReplayGatewayServiceFixture : IAsyncDisposable + { + private readonly GatewayMetrics _metrics = new(); + private readonly SessionRegistry _registry = new(); + + /// Initializes a new instance of the class. + /// Fake worker process launcher backing the session manager. + /// Replay ring capacity for the session's event distributor. + public ReconnectReplayGatewayServiceFixture( + IWorkerProcessLauncher launcher, + int replayBufferCapacity) + { + IOptions options = Options.Create(CreateOptions(replayBufferCapacity)); + SessionWorkerClientFactory workerClientFactory = new( + launcher, + options, + _metrics, + NullLoggerFactory.Instance); + SessionManager sessionManager = new( + _registry, + workerClientFactory, + options, + _metrics, + logger: NullLogger.Instance, + dashboardEventBroadcaster: NullDashboardEventBroadcaster.Instance); + MxAccessGrpcMapper mapper = new(); + EventStreamService eventStreamService = new( + sessionManager, + options, + _metrics); + + Service = new MxAccessGatewayService( + sessionManager, + new GatewayRequestIdentityAccessor(), + new AllowAllConstraintEnforcer(), + new MxAccessGrpcRequestValidator(), + mapper, + eventStreamService, + _metrics, + NullLogger.Instance, + new FakeGatewayAlarmService()); + } + + /// Gets the gateway service under test. + public MxAccessGatewayService Service { get; } + + /// + /// Polls for + /// until it reaches , bounded by + /// . Fails the test on timeout. This is the deterministic + /// gate that proves the production code has (re)registered a subscriber before the + /// test drives events or reconnects. + /// + /// Identifier of the session to poll. + /// Target subscriber count to wait for. + /// Maximum time to wait before failing the test. + /// A task that represents the asynchronous operation. + public async Task WaitForSubscriberCountAsync(string sessionId, int n, TimeSpan timeout) + { + using CancellationTokenSource deadlineCts = new(timeout); + + while (true) + { + if (_registry.TryGet(sessionId, out GatewaySession? session) + && session.ActiveEventSubscriberCount >= n) + { + return; + } + + if (deadlineCts.IsCancellationRequested) + { + int actual = _registry.TryGet(sessionId, out GatewaySession? s) + ? s.ActiveEventSubscriberCount + : -1; + Assert.Fail( + $"Timed out waiting for {n} event subscriber(s) on session {sessionId}. " + + $"Actual count after {timeout.TotalSeconds:0.#}s: {actual}."); + } + + await Task.Delay(millisecondsDelay: 5, deadlineCts.Token).ConfigureAwait(false); + } + } + + /// Disposes every session in the registry and releases the fixture's metrics. + /// A task that represents the asynchronous operation. + public async ValueTask DisposeAsync() + { + foreach (GatewaySession session in _registry.Snapshot()) + { + await session.DisposeAsync().ConfigureAwait(false); + } + + _metrics.Dispose(); + } + + private static GatewayOptions CreateOptions(int replayBufferCapacity) => + new() + { + Worker = new WorkerOptions + { + StartupTimeoutSeconds = 5, + ShutdownTimeoutSeconds = 5, + HeartbeatIntervalSeconds = 30, + HeartbeatGraceSeconds = 30, + MaxMessageBytes = WorkerFrameProtocolOptions.DefaultMaxMessageBytes, + }, + Sessions = new SessionOptions + { + DefaultCommandTimeoutSeconds = 5, + MaxSessions = 4, + + // Single-subscriber mode: a fully-detached subscriber (stream task awaited) + // frees the sole slot, so the reconnect attaches cleanly without needing + // multi-subscriber fan-out. Detach-grace keeps the session Ready across the + // detach and the fixture runs no lease-reaper, so the session survives to be + // reconnected. + AllowMultipleEventSubscribers = false, + MaxEventSubscribersPerSession = 8, + }, + Events = new EventOptions + { + QueueCapacity = 32, + ReplayBufferCapacity = replayBufferCapacity, + + // Keep age-eviction effectively off for the duration of a fast test so + // capacity is the only eviction axis under test. + ReplayRetentionSeconds = 300, + }, + }; + } + + // ---- fake worker launcher ---- + + /// + /// Fake worker that emits events one at a time, gated by , so + /// the test drives event timing deterministically. Modeled on the multi-subscriber E2E + /// tests' gated launcher. The worker loop is independent of subscribers, so events emitted + /// while no gRPC stream is attached still flow through the distributor pump into the + /// session's replay ring — exactly the condition the reconnect/replay tests exercise. Call + /// before closing the session so the loop exits cleanly and can + /// process the shutdown envelope. + /// + private sealed class GatedEventFakeWorkerProcessLauncher : IWorkerProcessLauncher + { + public const int ProcessId = 7730; + + private readonly FakeWorkerProcess _process = new(ProcessId); + + // Capacity 64 so AllowNextEvent can be called ahead of time without blocking. + private readonly SemaphoreSlim _emitGate = new(0, 64); + private volatile bool _stopEmitting; + + /// Gets the task representing the fake worker's running background loop. + public Task WorkerTask { get; private set; } = Task.CompletedTask; + + /// Releases the gate so the worker emits one event. + public void AllowNextEvent() => _emitGate.Release(); + + /// + /// Signals the worker to stop waiting for the emit gate and process the shutdown + /// envelope. Must be called before CloseSession. + /// + public void StopEmitting() + { + _stopEmitting = true; + _emitGate.Release(); // unblock a pending gate wait if any + } + + /// + public Task LaunchAsync( + WorkerProcessLaunchRequest request, + CancellationToken cancellationToken = default) + { + WorkerTask = RunWorkerAsync(request, cancellationToken); + + return Task.FromResult(new WorkerProcessHandle( + _process, + new WorkerProcessCommandLine("reconnect-replay-fake-worker.exe", []), + DateTimeOffset.UtcNow)); + } + + private async Task RunWorkerAsync( + WorkerProcessLaunchRequest request, + CancellationToken cancellationToken) + { + await using FakeWorkerHarness harness = await FakeWorkerHarness.ConnectToGatewayPipeAsync( + request.SessionId, + request.Nonce, + request.PipeName, + request.ProtocolVersion, + cancellationToken: cancellationToken).ConfigureAwait(false); + await harness.CompleteStartupAsync(ProcessId, cancellationToken: cancellationToken).ConfigureAwait(false); + + int advisedServerHandle = 0; + int advisedItemHandle = 0; + int emittedCount = 0; + + while (!cancellationToken.IsCancellationRequested) + { + // While subscribed and not stopped, emit gated events using a non-blocking peek + // at the gate so incoming envelopes (including shutdown) are never starved. + while (advisedServerHandle != 0 + && !_stopEmitting + && await _emitGate.WaitAsync(millisecondsTimeout: 0).ConfigureAwait(false)) + { + int index = ++emittedCount; + await harness.EmitEventAsync( + MxEventFamily.OnDataChange, + cancellationToken, + mxEvent => + { + mxEvent.ServerHandle = advisedServerHandle; + mxEvent.ItemHandle = advisedItemHandle; + mxEvent.Quality = 192; + mxEvent.Value = new MxValue + { + DataType = MxDataType.String, + StringValue = $"reconnect-value-{index}", + }; + mxEvent.OnDataChange = new OnDataChangeEvent(); + }).ConfigureAwait(false); + } + + WorkerEnvelope? envelope; + try + { + using CancellationTokenSource readCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); + readCts.CancelAfter(TimeSpan.FromMilliseconds(50)); + envelope = await harness.ReadGatewayEnvelopeAsync(readCts.Token).ConfigureAwait(false); + } + catch (OperationCanceledException) when (!cancellationToken.IsCancellationRequested) + { + // Timed out waiting for an envelope — loop back to check the gate / emit. + continue; + } + + if (envelope.BodyCase == WorkerEnvelope.BodyOneofCase.WorkerShutdown) + { + await harness.SendShutdownAckAsync(cancellationToken: cancellationToken).ConfigureAwait(false); + _process.MarkExited(0); + return; + } + + if (envelope.BodyCase != WorkerEnvelope.BodyOneofCase.WorkerCommand) + { + throw new InvalidOperationException($"Unexpected envelope {envelope.BodyCase}."); + } + + MxCommand command = envelope.WorkerCommand.Command; + await harness.ReplyToCommandAsync( + envelope, + configureReply: reply => ConfigureCommandReply(reply, command.Kind), + cancellationToken: cancellationToken).ConfigureAwait(false); + + if (command.Kind == MxCommandKind.Advise) + { + advisedServerHandle = command.Advise.ServerHandle; + advisedItemHandle = command.Advise.ItemHandle; + } + } + } + } +} diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Grpc/MxAccessGrpcMapperTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Grpc/MxAccessGrpcMapperTests.cs index 92ae3e9..b7249ba 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Grpc/MxAccessGrpcMapperTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Grpc/MxAccessGrpcMapperTests.cs @@ -76,4 +76,35 @@ public sealed class MxAccessGrpcMapperTests Assert.Equal(ProtocolStatusCode.ProtocolViolation, publicReply.ProtocolStatus.Code); } + + /// + /// Verifies MapEvent transfers ownership of the inner MxEvent (GWC-07 / IPC-05): the + /// returned reference is the same instance carried by the WorkerEvent, not a clone. The + /// WorkerEvent is discarded after mapping and the distributor pump is its single consumer, + /// so moving the inner event out is safe and avoids a per-event deep copy. + /// + [Fact] + public void MapEvent_TransfersOwnershipOfInnerEventWithoutCloning() + { + MxEvent innerEvent = new() + { + Family = MxEventFamily.OnDataChange, + RawStatus = "OK", + }; + WorkerEvent workerEvent = new() { Event = innerEvent }; + + MxEvent mapped = new MxAccessGrpcMapper().MapEvent(workerEvent); + + Assert.Same(innerEvent, mapped); + } + + /// Verifies that a worker event with no public payload maps to the unspecified-family sentinel. + [Fact] + public void MapEvent_WhenEventMissing_ReturnsUnspecifiedFamilySentinel() + { + MxEvent mapped = new MxAccessGrpcMapper().MapEvent(new WorkerEvent()); + + Assert.Equal(MxEventFamily.Unspecified, mapped.Family); + Assert.Equal("Worker event did not contain a public event payload.", mapped.RawStatus); + } } diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Sessions/SessionEventDistributorTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Sessions/SessionEventDistributorTests.cs index 1bff529..75a2544 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Sessions/SessionEventDistributorTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Sessions/SessionEventDistributorTests.cs @@ -185,6 +185,90 @@ public sealed class SessionEventDistributorTests Assert.Equal(new ulong[] { 3, 4, 5 }, replay.Select(e => e.WorkerSequence)); } + /// + /// Appending far more events than the capacity wraps the circular buffer's head + /// index past the array boundary multiple times; the retained window stays the + /// newest capacity events, in ascending order, and a replay from before the + /// window reports a gap and returns the whole retained tail. + /// + /// A task that represents the asynchronous operation. + [Fact] + public async Task ReplayBuffer_WrapsRingMultipleTimes_RetainsNewestInAscendingOrder() + { + Channel source = Channel.CreateUnbounded(); + await using SessionEventDistributor distributor = CreateDistributor( + source.Reader, + replayBufferCapacity: 4, + replayRetentionSeconds: 0); + await distributor.StartAsync(CancellationToken.None); + + // 13 events through a capacity-4 ring advances the head index 13 - 4 = 9 slots, + // wrapping the 4-slot array's boundary more than twice. + using IEventSubscriberLease lease = distributor.Register(); + for (ulong sequence = 1; sequence <= 13; sequence++) + { + source.Writer.TryWrite(Event(sequence)); + } + + for (ulong sequence = 1; sequence <= 13; sequence++) + { + MxEvent e = await ReadOneAsync(lease.Reader); + Assert.Equal(sequence, e.WorkerSequence); + } + + // Newest four (10, 11, 12, 13) retained in ascending order despite the wraps. + bool found = distributor.TryGetReplayFrom(0, out IReadOnlyList replay, out bool gap); + + Assert.True(found); + Assert.True(gap); + Assert.Equal(new ulong[] { 10, 11, 12, 13 }, replay.Select(e => e.WorkerSequence)); + + // A replay from inside the wrapped window returns only the newer entries, no gap, + // proving the modular scan reads the logical order and not the physical slots. + bool foundInner = distributor.TryGetReplayFrom(11, out IReadOnlyList inner, out bool innerGap); + + Assert.True(foundInner); + Assert.False(innerGap); + Assert.Equal(new ulong[] { 12, 13 }, inner.Select(e => e.WorkerSequence)); + } + + /// + /// A capacity-1 ring retains only the single newest event; each append overwrites + /// the sole slot, and a replay from before it reports a gap. + /// + /// A task that represents the asynchronous operation. + [Fact] + public async Task ReplayBuffer_Capacity1_RetainsOnlyNewest() + { + Channel source = Channel.CreateUnbounded(); + await using SessionEventDistributor distributor = CreateDistributor( + source.Reader, + replayBufferCapacity: 1, + replayRetentionSeconds: 0); + await distributor.StartAsync(CancellationToken.None); + + using IEventSubscriberLease lease = distributor.Register(); + for (ulong sequence = 1; sequence <= 3; sequence++) + { + source.Writer.TryWrite(Event(sequence)); + _ = await ReadOneAsync(lease.Reader); + } + + // Only sequence 3 is retained; a request from 0 missed 1 and 2 => gap. + bool found = distributor.TryGetReplayFrom(0, out IReadOnlyList replay, out bool gap); + + Assert.True(found); + Assert.True(gap); + Assert.Equal(new ulong[] { 3 }, replay.Select(e => e.WorkerSequence)); + + // A request from exactly the newest retained sequence is caught up: empty, no gap. + bool foundCaughtUp = distributor.TryGetReplayFrom(3, out IReadOnlyList caughtUp, out bool caughtUpGap); + + Assert.True(foundCaughtUp); + Assert.False(caughtUpGap); + Assert.Empty(caughtUp); + } + /// Requesting replay from a sequence still inside the retained window returns only the newer events, with no gap. /// A task that represents the asynchronous operation. [Fact] diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Workers/WorkerFrameProtocolTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Workers/WorkerFrameProtocolTests.cs index ed26f6d..910e77c 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Workers/WorkerFrameProtocolTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Workers/WorkerFrameProtocolTests.cs @@ -29,6 +29,32 @@ public sealed class WorkerFrameProtocolTests Assert.Equal(original, parsed); } + /// Verifies that a large payload near the message cap round-trips through the pooled write/read path. + /// A task that represents the asynchronous operation. + [Fact] + public async Task WriteAndReadAsync_WithLargePayloadNearMax_RoundTripsFrame() + { + // A payload comfortably larger than an ArrayPool bucket boundary so the rented buffer is + // larger than the exact frame length, exercising the length-bounded read and parse. + const int maxMessageBytes = 1024 * 1024; + WorkerFrameProtocolOptions options = + new(SessionId, GatewayContractInfo.WorkerProtocolVersion, maxMessageBytes); + await using MemoryStream stream = new(); + + WorkerEnvelope original = CreateEnvelope(); + original.WorkerHello.WorkerVersion = new string('x', maxMessageBytes - 4096); + Assert.InRange(original.CalculateSize(), 1, maxMessageBytes); + + WorkerFrameWriter writer = new(stream, options); + await writer.WriteAsync(original); + stream.Position = 0; + + WorkerFrameReader reader = new(stream, options); + WorkerEnvelope parsed = await reader.ReadAsync(); + + Assert.Equal(original, parsed); + } + /// Verifies that reading a frame with partial reads reassembles the frame correctly. /// A task that represents the asynchronous operation. [Fact] diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Metrics/GatewayMetricsTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Metrics/GatewayMetricsTests.cs index 2dec8bd..2985e4f 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/Metrics/GatewayMetricsTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/Metrics/GatewayMetricsTests.cs @@ -20,7 +20,9 @@ public sealed class GatewayMetricsTests metrics.EventReceived("session-1", "OnDataChange"); metrics.EventReceived("session-1", "OnDataChange"); metrics.SetWorkerEventQueueDepth(7); - metrics.AdjustGrpcEventStreamQueueDepth(3); + // GWC-15: the gRPC stream queue-depth gauge sums live backlog sources at collection time + // rather than tracking a pushed running total. Register a source reporting 3. + using IDisposable backlogSource = metrics.RegisterEventStreamBacklogSource(static () => 3); metrics.QueueOverflow("session-events"); metrics.Fault("CommandTimeout"); metrics.WorkerKilled("CommandTimeout"); diff --git a/src/ZB.MOM.WW.MxGateway.Worker.Tests/Conversion/MxStatusProxyConverterTests.cs b/src/ZB.MOM.WW.MxGateway.Worker.Tests/Conversion/MxStatusProxyConverterTests.cs index 4f732c3..4224573 100644 --- a/src/ZB.MOM.WW.MxGateway.Worker.Tests/Conversion/MxStatusProxyConverterTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Worker.Tests/Conversion/MxStatusProxyConverterTests.cs @@ -106,6 +106,35 @@ public sealed class MxStatusProxyConverterTests Assert.Contains("success", exception.Message); } + /// + /// Verifies that repeated conversions of the same status type produce + /// identical results. WRK-06 caches the resolved FieldInfo objects per + /// type after the first conversion; the cached path must yield the same + /// message the uncached first call produced. + /// + [Fact] + public void Convert_RepeatedForSameType_ProducesIdenticalResults() + { + FakeMxStatusProxy status = new() + { + success = 0, + category = 3, + detectedBy = 1, + detail = 21, + }; + + // First call populates the per-type FieldInfo cache; the second reuses it. + MxStatusProxy first = _converter.Convert(status); + MxStatusProxy second = _converter.Convert(status); + + Assert.Equal(first, second); + Assert.Equal(MxStatusCategory.CommunicationError, second.Category); + Assert.Equal(MxStatusSource.RespondingLmx, second.DetectedBy); + Assert.Equal(0, second.Success); + Assert.Equal(21, second.Detail); + Assert.Equal("Invalid reference", second.DiagnosticText); + } + public struct FakeMxStatusProxy { public short success; diff --git a/src/ZB.MOM.WW.MxGateway.Worker.Tests/Ipc/WorkerFrameProtocolTests.cs b/src/ZB.MOM.WW.MxGateway.Worker.Tests/Ipc/WorkerFrameProtocolTests.cs index 8921762..c80b447 100644 --- a/src/ZB.MOM.WW.MxGateway.Worker.Tests/Ipc/WorkerFrameProtocolTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Worker.Tests/Ipc/WorkerFrameProtocolTests.cs @@ -373,6 +373,43 @@ public sealed class WorkerFrameProtocolTests Assert.Equal(WorkerEnvelope.BodyOneofCase.WorkerEvent, frame3.BodyCase); } + /// + /// Verifies the writer coalesces the flush across a batch of frames drained together: four frames + /// queued behind an in-progress write drain in a single pass and share one FlushAsync, not four + /// (WRK-12 / IPC-15). Every frame still reaches the wire intact. + /// + /// A task that represents the asynchronous operation. + [Fact] + public async Task WriteAsync_WhenBatchDrainedTogether_FlushesOnce() + { + WorkerFrameProtocolOptions options = CreateOptions(); + using GatedWriteStream stream = new(); + WorkerFrameWriter writer = new(stream, options); + + // A blocked first write occupies the writer and holds the lock while more frames queue behind it. + Task firstWrite = writer.WriteAsync(CreateGatewayHelloEnvelope(), WorkerFrameWritePriority.Control); + await AwaitWithTimeoutAsync(stream.FirstWriteStarted); + + Task eventWrite1 = writer.WriteAsync(CreateEventEnvelope(), WorkerFrameWritePriority.Event); + Task eventWrite2 = writer.WriteAsync(CreateEventEnvelope(), WorkerFrameWritePriority.Event); + Task eventWrite3 = writer.WriteAsync(CreateEventEnvelope(), WorkerFrameWritePriority.Event); + await Task.Delay(50); + + stream.ReleaseFirstWrite(); + await AwaitWithTimeoutAsync(Task.WhenAll(firstWrite, eventWrite1, eventWrite2, eventWrite3)); + + // Four frames written in one drain pass => exactly one flush. + Assert.Equal(1, stream.FlushCount); + + stream.Position = 0; + WorkerFrameReader reader = new(stream, options); + for (int index = 0; index < 4; index++) + { + WorkerEnvelope frame = await reader.ReadAsync(); + Assert.NotEqual(WorkerEnvelope.BodyOneofCase.None, frame.BodyCase); + } + } + /// Verifies a zero negotiated frame maximum keeps the constructor default (IPC-02). [Fact] public void AdoptNegotiatedMaxMessageBytes_WithZero_KeepsDefault() @@ -451,9 +488,12 @@ public sealed class WorkerFrameProtocolTests private readonly TaskCompletionSource _firstWriteStarted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); private int _writeCount; + private int _flushCount; public Task FirstWriteStarted => _firstWriteStarted.Task; + public int FlushCount => Volatile.Read(ref _flushCount); + public void ReleaseFirstWrite() => _release.Release(); public override async Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) @@ -467,6 +507,12 @@ public sealed class WorkerFrameProtocolTests await base.WriteAsync(buffer, offset, count, cancellationToken); } + public override Task FlushAsync(CancellationToken cancellationToken) + { + Interlocked.Increment(ref _flushCount); + return base.FlushAsync(cancellationToken); + } + protected override void Dispose(bool disposing) { if (disposing) diff --git a/src/ZB.MOM.WW.MxGateway.Worker.Tests/MxAccess/MxAccessEventQueueTests.cs b/src/ZB.MOM.WW.MxGateway.Worker.Tests/MxAccess/MxAccessEventQueueTests.cs index 27d684b..84f8e1d 100644 --- a/src/ZB.MOM.WW.MxGateway.Worker.Tests/MxAccess/MxAccessEventQueueTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Worker.Tests/MxAccess/MxAccessEventQueueTests.cs @@ -29,6 +29,26 @@ public sealed class MxAccessEventQueueTests Assert.False(queue.TryDequeue(out _)); } + /// + /// Verifies that Enqueue takes ownership of the passed event instead of + /// cloning it: the dequeued instance is the very reference passed in, and + /// the worker sequence/timestamp are stamped on that same instance + /// (WRK-11). + /// + [Fact] + public void Enqueue_TakesOwnershipOfPassedEventInstance() + { + MxAccessEventQueue queue = new(capacity: 4); + MxEvent original = CreateEvent(MxEventFamily.OnDataChange, itemHandle: 10); + + queue.Enqueue(original); + + Assert.True(queue.TryDequeue(out WorkerEvent? dequeued)); + Assert.Same(original, dequeued?.Event); + Assert.Equal(1UL, original.WorkerSequence); + Assert.NotNull(original.WorkerTimestamp); + } + /// Verifies that Drain removes at most the requested number of events. [Fact] public void Drain_RemovesAtMostRequestedEvents() diff --git a/src/ZB.MOM.WW.MxGateway.Worker.Tests/MxAccess/MxAccessValueCacheTests.cs b/src/ZB.MOM.WW.MxGateway.Worker.Tests/MxAccess/MxAccessValueCacheTests.cs index 7e5cb4f..fded4a2 100644 --- a/src/ZB.MOM.WW.MxGateway.Worker.Tests/MxAccess/MxAccessValueCacheTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Worker.Tests/MxAccess/MxAccessValueCacheTests.cs @@ -46,6 +46,37 @@ public sealed class MxAccessValueCacheTests Assert.Equal(999, other.Value.Int32Value); } + /// + /// Verifies that Set stores an independent deep-copied snapshot: mutating + /// the source event's protobuf sub-messages after caching does not alter + /// the cached value. WRK-11 stopped the event sink cloning before enqueue, + /// so the same MxEvent instance now flows to the outbound queue; the cache + /// must own its own copy so the two never share mutable state. + /// + [Fact] + public void Set_StoresIndependentSnapshot_UnaffectedByLaterEventMutation() + { + MxAccessValueCache cache = new(); + Timestamp sourceTimestamp = Timestamp.FromDateTime(new(2026, 5, 19, 9, 0, 0, DateTimeKind.Utc)); + MxEvent mxEvent = BuildEvent(serverHandle: 7, itemHandle: 21, intValue: 100, quality: 192, sourceTimestamp); + + cache.Set(7, 21, mxEvent); + + // Mutate the event in place after it was cached — as if it kept flowing + // through the (unrelated) outbound path. None of this must reach the cache. + mxEvent.Value.Int32Value = 999; + mxEvent.Quality = 0; + mxEvent.SourceTimestamp = Timestamp.FromDateTime(new(2030, 1, 1, 0, 0, 0, DateTimeKind.Utc)); + mxEvent.Statuses[0].Category = MxStatusCategory.SecurityError; + + Assert.True(cache.TryGet(7, 21, out MxAccessValueCache.CachedValue cached)); + Assert.Equal(100, cached.Value.Int32Value); + Assert.Equal(192, cached.Quality); + Assert.Equal(sourceTimestamp, cached.SourceTimestamp); + Assert.Single(cached.Statuses); + Assert.Equal(MxStatusCategory.Ok, cached.Statuses[0].Category); + } + /// Verifies that TryGet returns false for unknown handles. [Fact] public void TryGet_WithUnknownHandle_ReturnsFalse() diff --git a/src/ZB.MOM.WW.MxGateway.Worker/Conversion/MxStatusProxyConverter.cs b/src/ZB.MOM.WW.MxGateway.Worker/Conversion/MxStatusProxyConverter.cs index 8deff4d..036cd9c 100644 --- a/src/ZB.MOM.WW.MxGateway.Worker/Conversion/MxStatusProxyConverter.cs +++ b/src/ZB.MOM.WW.MxGateway.Worker/Conversion/MxStatusProxyConverter.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Concurrent; using System.Collections.Generic; using System.Globalization; using System.Reflection; @@ -9,6 +10,17 @@ namespace ZB.MOM.WW.MxGateway.Worker.Conversion; /// Converts MXAccess MXSTATUS_PROXY COM objects to protobuf MxStatusProxy messages. public sealed class MxStatusProxyConverter { + /// + /// Per-type cache of the four resolved objects a + /// status conversion needs. The status type is stable (the interop + /// MXSTATUS_PROXY struct in production; a fixed test double in + /// tests), so the expensive + /// metadata scan is resolved once per type and reused. Keyed by + /// so a plain-CLR test double and the real interop + /// struct each get their own entry, keeping the converter interop-agnostic. + /// + private static readonly ConcurrentDictionary FieldCache = new(); + /// Converts a single status object to a protobuf message, reflecting all fields and diagnostics. /// COM status object to convert. /// The converted protobuf status message. @@ -20,10 +32,11 @@ public sealed class MxStatusProxyConverter } Type statusType = status.GetType(); - int success = ReadInt32Field(status, statusType, "success"); - int rawCategory = ReadInt32Field(status, statusType, "category"); - int rawDetectedBy = ReadInt32Field(status, statusType, "detectedBy"); - int detail = ReadInt32Field(status, statusType, "detail"); + StatusFields fields = GetFields(statusType); + int success = ReadInt32Field(status, statusType, fields.Success); + int rawCategory = ReadInt32Field(status, statusType, fields.Category); + int rawDetectedBy = ReadInt32Field(status, statusType, fields.DetectedBy); + int detail = ReadInt32Field(status, statusType, fields.Detail); return new MxStatusProxy { @@ -82,6 +95,43 @@ public sealed class MxStatusProxyConverter private static int ReadInt32Field( object value, + Type valueType, + FieldInfo field) + { + object? fieldValue = field.GetValue(value); + if (fieldValue is null) + { + throw new MxStatusConversionException( + $"Status object field '{field.Name}' on type '{valueType.FullName}' is null."); + } + + return System.Convert.ToInt32(fieldValue, CultureInfo.InvariantCulture); + } + + /// + /// Resolves (and caches) the four objects for the + /// given status type. The first resolution for a type performs the + /// reflection scan; every subsequent conversion of that type reuses the + /// cached entry. A type missing a required field throws the same + /// the per-field lookup used to + /// throw — and, because + /// does not store a value when the factory throws, a bad type keeps + /// failing identically on every call rather than being cached. + /// + /// Runtime type of the status object being converted. + /// The resolved field set for . + private static StatusFields GetFields(Type statusType) + { + return FieldCache.GetOrAdd( + statusType, + type => new StatusFields( + ResolveField(type, "success"), + ResolveField(type, "category"), + ResolveField(type, "detectedBy"), + ResolveField(type, "detail"))); + } + + private static FieldInfo ResolveField( Type valueType, string fieldName) { @@ -92,14 +142,7 @@ public sealed class MxStatusProxyConverter $"Status object type '{valueType.FullName}' does not expose required field '{fieldName}'."); } - object? fieldValue = field.GetValue(value); - if (fieldValue is null) - { - throw new MxStatusConversionException( - $"Status object field '{fieldName}' on type '{valueType.FullName}' is null."); - } - - return System.Convert.ToInt32(fieldValue, CultureInfo.InvariantCulture); + return field; } private static MxStatusCategory MapCategory(int rawCategory) @@ -134,4 +177,32 @@ public sealed class MxStatusProxyConverter _ => MxStatusSource.Unknown, }; } + + /// + /// The four resolved status fields cached per type. Plain readonly struct + /// (not a record) so it compiles under the worker's net48 target, which + /// lacks IsExternalInit. + /// + private readonly struct StatusFields + { + public StatusFields( + FieldInfo success, + FieldInfo category, + FieldInfo detectedBy, + FieldInfo detail) + { + Success = success; + Category = category; + DetectedBy = detectedBy; + Detail = detail; + } + + public FieldInfo Success { get; } + + public FieldInfo Category { get; } + + public FieldInfo DetectedBy { get; } + + public FieldInfo Detail { get; } + } } diff --git a/src/ZB.MOM.WW.MxGateway.Worker/Ipc/WorkerFrameWriter.cs b/src/ZB.MOM.WW.MxGateway.Worker/Ipc/WorkerFrameWriter.cs index 5ae2e58..28734ad 100644 --- a/src/ZB.MOM.WW.MxGateway.Worker/Ipc/WorkerFrameWriter.cs +++ b/src/ZB.MOM.WW.MxGateway.Worker/Ipc/WorkerFrameWriter.cs @@ -112,36 +112,77 @@ public sealed class WorkerFrameWriter // Runs only under _writeLock. Drains control frames before event frames, stamping and writing each. // The stream write itself is not cancellable: a frame is written atomically or fails, never left // half-written on the pipe because a caller gave up waiting. + // + // Flushes are coalesced across the whole drained batch (WRK-12 / IPC-15): each frame is written to + // the stream but not flushed individually; a single FlushAsync runs after the batch, then every + // successfully-written frame is completed. A caller's Completion therefore still signals only after + // its bytes have been written AND flushed, so the "written and flushed" contract is unchanged — but + // a burst of N events now costs one flush syscall instead of N. private async Task DrainQueuedFramesAsync() { + List written = new List(); while (true) { PendingFrame? frame = DequeueNext(); if (frame is null) { - return; + break; } try { await WriteFrameAsync(frame.Envelope).ConfigureAwait(false); - frame.Completion.TrySetResult(true); + written.Add(frame); } catch (WorkerFrameProtocolException exception) when (IsPerFrameRejection(exception)) { // Validation, empty-payload, and oversized-frame errors are specific to this frame and - // do not damage the stream; fail only this frame and keep draining the rest. + // do not damage the stream; fail only this frame and keep draining the rest. Nothing was + // written for it, so it needs no flush. frame.Completion.TrySetException(exception); } catch (Exception exception) { - // A stream write/flush failure means the pipe is broken; fail this frame and every frame - // still queued so no caller awaits forever, then stop draining. + // A stream write failure means the pipe is broken; fail this frame, every frame already + // written this batch but not yet flushed, and every frame still queued so no caller + // awaits forever, then stop draining. frame.Completion.TrySetException(exception); + FailFrames(written, exception); FailAllQueued(exception); return; } } + + if (written.Count == 0) + { + return; + } + + try + { + await _stream.FlushAsync(CancellationToken.None).ConfigureAwait(false); + } + catch (Exception exception) + { + // The batch reached the stream but the flush that guarantees delivery failed: the pipe is + // broken. Fail every frame in the batch (the queue was already drained) so no caller treats + // an unflushed write as delivered. + FailFrames(written, exception); + return; + } + + foreach (PendingFrame frame in written) + { + frame.Completion.TrySetResult(true); + } + } + + private static void FailFrames(List frames, Exception exception) + { + foreach (PendingFrame frame in frames) + { + frame.Completion.TrySetException(exception); + } } private static bool IsPerFrameRejection(WorkerFrameProtocolException exception) @@ -211,14 +252,14 @@ public sealed class WorkerFrameWriter // Serialize once into a single buffer that carries the 4-byte length prefix followed by the // payload, then issue one stream write. This avoids a second serialization pass, a separate - // prefix array, and a separate prefix write. + // prefix array, and a separate prefix write. The flush is deferred to the end of the drained + // batch (see DrainQueuedFramesAsync) so a burst of frames shares one flush. int frameLength = sizeof(uint) + payloadLength; byte[] frame = new byte[frameLength]; WriteUInt32LittleEndian(frame, (uint)payloadLength); envelope.WriteTo(new Span(frame, sizeof(uint), payloadLength)); await _stream.WriteAsync(frame, 0, frameLength, CancellationToken.None).ConfigureAwait(false); - await _stream.FlushAsync(CancellationToken.None).ConfigureAwait(false); } private static void WriteUInt32LittleEndian( diff --git a/src/ZB.MOM.WW.MxGateway.Worker/MxAccess/MxAccessEventQueue.cs b/src/ZB.MOM.WW.MxGateway.Worker/MxAccess/MxAccessEventQueue.cs index 51fe92f..fe09401 100644 --- a/src/ZB.MOM.WW.MxGateway.Worker/MxAccess/MxAccessEventQueue.cs +++ b/src/ZB.MOM.WW.MxGateway.Worker/MxAccess/MxAccessEventQueue.cs @@ -8,6 +8,17 @@ namespace ZB.MOM.WW.MxGateway.Worker.MxAccess; /// /// Thread-safe queue for MxAccess events with capacity overflow and fault tracking. /// +/// +/// Ownership invariant: takes ownership of the +/// passed to it — it stamps the worker sequence and +/// timestamp on that same instance and enqueues it directly, without +/// cloning (WRK-11). Every caller must therefore pass a freshly built +/// that it does not retain, reuse, or mutate after the +/// call returns. All production callers (MxAccessBaseEventSink, +/// MxAccessAlarmEventSink, AlarmCommandHandler) build a new event per +/// Enqueue via the mapper and satisfy this; the value cache stores its own +/// independent snapshot (see ). +/// public sealed class MxAccessEventQueue { /// @@ -110,8 +121,11 @@ public sealed class MxAccessEventQueue /// /// Enqueues an MxAccess event, assigning a sequence number and timestamp. + /// Takes ownership of : the sequence and timestamp + /// are stamped on that same instance and it is enqueued without cloning, so + /// the caller must pass a freshly built event it does not reuse afterwards. /// - /// MXAccess event to enqueue. + /// Freshly built MXAccess event to enqueue; ownership transfers to the queue. public void Enqueue(MxEvent mxEvent) { if (mxEvent is null) @@ -132,13 +146,17 @@ public sealed class MxAccessEventQueue throw new MxAccessEventQueueOverflowException(capacity); } - MxEvent queuedEvent = mxEvent.Clone(); - queuedEvent.WorkerSequence = ++lastEventSequence; - queuedEvent.WorkerTimestamp = Timestamp.FromDateTime(DateTime.UtcNow); + // WRK-11: stamp the sequence/timestamp on the caller's own event and + // enqueue that same instance under the lock instead of cloning. See + // the ownership invariant on the class summary — the caller hands the + // event over exclusively, so a defensive Clone() here is pure + // overhead on the hottest path. + mxEvent.WorkerSequence = ++lastEventSequence; + mxEvent.WorkerTimestamp = Timestamp.FromDateTime(DateTime.UtcNow); WorkerEvent workerEvent = new() { - Event = queuedEvent, + Event = mxEvent, }; events.Enqueue(workerEvent); } diff --git a/src/ZB.MOM.WW.MxGateway.Worker/MxAccess/MxAccessValueCache.cs b/src/ZB.MOM.WW.MxGateway.Worker/MxAccess/MxAccessValueCache.cs index a4ba24d..f82429b 100644 --- a/src/ZB.MOM.WW.MxGateway.Worker/MxAccess/MxAccessValueCache.cs +++ b/src/ZB.MOM.WW.MxGateway.Worker/MxAccess/MxAccessValueCache.cs @@ -40,6 +40,20 @@ public sealed class MxAccessValueCache throw new ArgumentNullException(nameof(mxEvent)); } + // WRK-11: the event sink no longer clones before enqueue, so the passed + // mxEvent is the very instance handed to the outbound queue. Deep-copy + // the value/timestamp/statuses payload we retain here so the cache's + // snapshot stays independent of the enqueued (and later serialized) + // event — the two must never share mutable protobuf sub-messages. + // Value is always set for OnDataChange; SourceTimestamp may be unset when + // the source timestamp could not be parsed, so both are cloned only when + // present. The null-forgiving result matches CachedValue's non-null- + // annotated parameters, which already accepted a runtime-null value or + // timestamp before WRK-11 (the ternary keeps the compiler's null-state + // from poisoning to maybe-null, which a plain null check would do). + MxValue cachedValue = mxEvent.Value is null ? null! : mxEvent.Value.Clone(); + Timestamp cachedTimestamp = mxEvent.SourceTimestamp is null ? null! : mxEvent.SourceTimestamp.Clone(); + long key = CreateItemKey(serverHandle, itemHandle); lock (syncRoot) { @@ -49,10 +63,10 @@ public sealed class MxAccessValueCache entries[key] = new CachedValue( nextVersion, - mxEvent.Value, + cachedValue, mxEvent.Quality, - mxEvent.SourceTimestamp, - mxEvent.Statuses); + cachedTimestamp, + mxEvent.Statuses.Clone()); } }