docs(plan): OpcUaClient ReadEventsAsync event-history passthrough design

Driver-internal IHistoryProvider.ReadEventsAsync passthrough to the upstream
OPC UA server's HistoryReadEvents service. No interface change: the driver
builds a fixed canonical 6-clause EventFilter and maps the upstream HistoryEvent
onto the existing HistoricalEvent record (the server re-projects only those 6
BaseEventType fields, so richer clauses are discarded anyway).
This commit is contained in:
Joseph Doherty
2026-06-18 05:55:58 -04:00
parent bd791e797a
commit 400bef4769
@@ -0,0 +1,154 @@
# OpcUaClient `ReadEventsAsync` event-history passthrough — Design
**Date:** 2026-06-18
**Branch:** `feat/opcuaclient-read-events` (off master)
**Backlog item:** `stillpending.md` §A / `project_stillpending_backlog` item 2 — "OpcUaClient `ReadEventsAsync` event-history passthrough."
## Goal
Implement the optional `IHistoryProvider.ReadEventsAsync` on `OpcUaClientDriver` so the
driver forwards OPC UA **HistoryReadEvents** to its upstream server — completing the
driver's history-provider surface (Raw / Processed / AtTime already passthrough; Events was
the lone unimplemented method, inheriting the throwing default-interface body).
## Key finding (scope correction)
The backlog framed this as "needs an `IHistoryProvider.ReadEventsAsync` filter-spec/EventFilter
param extension." That is **overcautious and not required.** The interface returns a **fixed
6-field `HistoricalEvent`** record (`EventId, SourceName, EventTimeUtc, ReceivedTimeUtc,
Message, Severity`), and the OtOpcUa server's `OtOpcUaNodeManager.HistoryReadEvents` override
**re-projects only those 6 `BaseEventType` fields** (`ProjectEventField`) — a richer
SelectClause set would be discarded server-side. So the driver builds its **own canonical
`EventFilter` internally** and maps the upstream result onto the existing `HistoricalEvent`
record, exactly as `WonderwareHistorianClient.ReadEventsAsync` already does.
**Decision (user-approved):** driver-internal implementation, **NO** `IHistoryProvider` /
Core.Abstractions / Commons contract change.
## Architecture
`OpcUaClientDriver` already implements `IHistoryProvider` and forwards Raw/Processed/AtTime
through a shared wire path (`ExecuteHistoryReadAsync``Session.HistoryReadAsync`). Events
needs a sibling path because the request details type differs (`ReadEventDetails` carrying an
`EventFilter`) and the response payload differs (`HistoryEvent` whose `Events` are
`HistoryEventFieldList` arrays, ordered to match the SelectClauses the driver sent) and the
return type differs (`HistoricalEventsResult`, not `HistoryReadResult`).
The two duplicated history seams stay distinct and unchanged:
- **`IHistoryProvider`** = a **driver capability** (OpcUaClient → upstream server). This is
what we extend.
- **`IHistorianDataSource`** = the **server-side** HistoryRead backend (single registered
Wonderware/Null source the OtOpcUa node-manager dispatches to). **Untouched.** Note the
driver's `IHistoryProvider` is not currently bridged into the OtOpcUa server's HistoryRead
service — wiring a driver as a server history backend is a separate, larger concern and is
out of scope. This item completes the driver capability surface.
## Components (all in `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs`)
1. **`internal static EventFilter BuildBaseEventFilter()`** — 6 `SimpleAttributeOperand`
SelectClauses on `ObjectTypeIds.BaseEventType` (`AttributeId = Attributes.Value`), in the
fixed canonical order the mapper relies on:
`EventId, SourceName, Time, ReceiveTime, Message, Severity` (via `BrowseNames.*`).
2. **`internal static IReadOnlyList<HistoricalEvent> MapHistoryEvents(HistoryEvent he)`** —
maps each `HistoryEventFieldList.EventFields[i]` by index → `HistoricalEvent`, with
defensive per-field coercion:
- `[0] EventId` — upstream `byte[]` (ByteString) → base64 string (stable/unique; the field
is opaque "driver-specific format" per the record's XML doc). String passthrough if the
upstream already returned a string.
- `[1] SourceName` — string (null-safe).
- `[2] Time``EventTimeUtc` (DateTime).
- `[3] ReceiveTime``ReceivedTimeUtc` (DateTime).
- `[4] Message``LocalizedText``.Text` (or string).
- `[5] Severity``ushort` (safe convert).
- Field-lists shorter than 6 degrade gracefully (missing fields → null/default), never throw.
- Empty `he.Events` → empty list.
3. **`public Task<HistoricalEventsResult> ReadEventsAsync(string? sourceName, DateTime startUtc,
DateTime endUtc, int maxEvents, CancellationToken ct)`** (replaces the inherited throwing
default):
- Resolve the upstream notifier NodeId: `sourceName` null/empty → `ObjectIds.Server`
(i=2253, the standard server-wide event notifier); else `TryParseNodeId(session,
sourceName, out nodeId)` — parse-fail → empty result (mirrors the raw path's malformed-
NodeId short-circuit). `sourceName` is the upstream notifier NodeId, mirroring how
`fullReference` is the upstream NodeId for raw reads.
- Build `ReadEventDetails { StartTime = startUtc, EndTime = endUtc,
NumValuesPerNode = maxEvents <= 0 ? 0u : (uint)maxEvents, Filter = BuildBaseEventFilter() }`
(the `maxEvents <= 0` → 0 "no cap" sentinel honored per the interface's documented
convention; OPC UA treats `NumValuesPerNode = 0` as unbounded).
- Call `session.HistoryReadAsync(ReadEventDetails)` under `_gate`; unwrap
`r.HistoryData?.Body is HistoryEvent he` → `MapHistoryEvents(he)`; preserve the upstream
continuation point (`r.ContinuationPoint`).
- Remove the stale "out of scope for this PR" comment at `OpcUaClientDriver.cs:1678-1682`.
## Data flow
```
driver.ReadEventsAsync(sourceName, start, end, maxEvents)
→ BuildBaseEventFilter() (canonical 6-clause EventFilter)
→ Session.HistoryReadAsync(ReadEventDetails) (upstream HistoryReadEvents)
→ upstream HistoryEvent { Events: HistoryEventFieldList[] }
→ MapHistoryEvents() (index-mapped → HistoricalEvent[])
→ HistoricalEventsResult(events, continuationPoint)
```
## Error handling
- No session → `InvalidOperationException` (via `RequireSession()`, same as the sibling reads).
- Unparseable `sourceName` → empty result (mirrors raw's malformed-NodeId short-circuit).
- Upstream rejects events (e.g. opc-plc is not a historian) → the upstream Bad status surfaces
through the SDK; empty `Events` returns an empty list, never a throw. Cascading-quality rule:
no status translation.
## Testing (NO bUnit; xUnit + Shouldly)
Mirrors the existing history-test split: the **pure cores** are `internal static` and unit-
tested offline (like `MapAggregateToNodeId`); the thin wire glue is covered by the without-init
unit test + the skip-gated integration smoke (the driver exposes no in-process session-injection
seam, so the unwrap glue is integration/live-proven — same as today's Raw/Processed path).
- **Pure unit (offline, `OpcUaClientHistoryTests`):**
- `BuildBaseEventFilter` → 6 clauses, canonical order, correct `BrowseNames` + `Value` attr +
`BaseEventType` type-definition.
- `MapHistoryEvents` → full 6-field map; EventId base64; short/empty field-list resilience;
empty `Events` → empty list.
- **Contract flip:** replace `ReadEventsAsync_throws_NotSupportedException_as_documented` with
`ReadEventsAsync_without_initialize_throws_InvalidOperationException` (mirrors the sibling
without-init tests).
- **Integration smoke (skip-gated, `OpcUaClientSmokeTests`, opc-plc):** connect +
`ReadEventsAsync` against the Server node returns a `HistoricalEventsResult` gracefully (no
throw), proving the wire request is well-formed and handled.
The unit + integration test projects already suppress the `UnwrappedCapabilityCallAnalyzer`
rule for direct driver-capability calls (the existing smoke tests call `drv.ReadAsync` directly)
— the new tests follow the same suppression; no new analyzer concern (the driver's own method is
an implementation, not a caller).
## Live verification (honest)
Live `/run` drives the skip-gated integration smoke against opc-plc (`--alm`): proves the driver
issues HistoryReadEvents and handles the upstream verdict. A **non-empty** event list is
**infra-gated** — opc-plc generates live alarm conditions for subscriptions but is **not** a
historian, so it does not serve historical events; a non-empty proof needs an upstream OPC UA
server that historizes events. Stated plainly (same class of caveat as the recent
S7-Timer/Counter and TwinCAT-live items).
## Hard rules honored
- NO Commons / proto / EF / `IHistoryProvider` contract change.
- NO bUnit. NO `git add .` (stage by explicit path). Never stage `sql_login.txt`,
`src/Server/.../pki/`, `pending.md`, `stillpending.md`, `docker-dev/docker-compose.yml`.
- No force-push, no `--no-verify`.
- Driver-internal only (`Driver.OpcUaClient` + its two test projects + docs).
- Finish = merge to master + push.
## Slices (for the implementation plan)
- **T1** — pure cores: `BuildBaseEventFilter` + `MapHistoryEvents` (+ field-coercion helpers) as
`internal static`, with full offline unit tests (TDD).
- **T2** — wire-in: `public ReadEventsAsync` using the pure cores + `Session.HistoryReadAsync`;
remove the stale "out of scope" comment; replace the NotSupported contract test with the
without-init test.
- **T3** — integration smoke (skip-gated) + docs (driver comment region, `docs/Historian.md`
driver-passthrough note, the backlog line) + build + tests + live `/run` + finish (merge+push).