diff --git a/docs/drivers/Mqtt.md b/docs/drivers/Mqtt.md index f8aef796..5952b4fc 100644 --- a/docs/drivers/Mqtt.md +++ b/docs/drivers/Mqtt.md @@ -117,7 +117,7 @@ driver are pure organisational grouping in the RawPath. "HostId": "", // Host Application identity — spBv1.0/STATE/{HostId} "ActAsPrimaryHost": false, // claim the primary host id (Primary-gated; see below) "RequestRebirthOnGap": true, // a seq gap asks the edge node to re-announce (NCMD) - "BirthObservationWindowSeconds": 15 // how long discovery collects births before calling it stable + "BirthObservationWindowSeconds": 15 // how long the browse picker collects births before calling them stable } } ``` @@ -128,7 +128,7 @@ driver are pure organisational grouping in the RawPath. | `HostId` | `""` | Sparkplug Host Application identity — the `spBv1.0/STATE/{HostId}` topic. Optional for observation; **required** to claim. Set without `ActAsPrimaryHost`, the driver only observes | | `ActAsPrimaryHost` | `false` | Claim the Sparkplug Primary Host Application identity. **Primary-gated** — only the node holding the redundancy Primary role claims it. See [Primary host](#primary-host-actasprimaryhost) | | `RequestRebirthOnGap` | `true` | On a detected sequence-number gap, publish a rebirth-request NCMD instead of running on stale metric state | -| `BirthObservationWindowSeconds` | `15` | Discovery's birth-collection window. Must be ≥ 1 | +| `BirthObservationWindowSeconds` | `15` | The Sparkplug browse picker's birth-collection window, clamped to 1–120 s (R06:M-4). The open budget grows by it; the open lingers while NBIRTH/DBIRTH traffic is still arriving (returning after a 1 s quiet spell — an idle plant costs 1 s); and an operator **Request rebirth** waits, bounded by it and capped at 15 s (below the AdminUI's 20 s per-call timeout), for the births it asked for to arrive and go quiet for 500 ms, so the tree re-rendered afterwards carries them. A caller deadline during that wait still reports the publish. The runtime driver does not read it (its `DiscoverAsync` has no production caller — `SupportsOnlineDiscovery` is false) | `MqttDriverForm` authors all five in Sparkplug mode. The sub-object is **merged, never replaced** (a key a newer driver adds inside it survives an older AdminUI), and in `Plain` mode it is not touched at @@ -249,7 +249,10 @@ One subscription (`spBv1.0/{GroupId}/#`) feeds a per-edge-node state machine [Structured metrics](#structured-metrics-template--dataset--propertyset). - **Unsupported datatypes are skipped with a warning, never guessed**: `PropertySetList`, `Unknown`, extension values, and any `Template`/`DataSet` that cannot be flattened honestly. `Int8`→`Int16` and - `UInt8`→`UInt16` widen; `*Array` variants map to the element type with the array bit set. + `UInt8`→`UInt16` widen; `*Array` variants map to the element type with the array bit set — but + **array metrics are not read in v1** (warned once, never fed), so the browse picker offers them, and any + metric whose type is unsupported, **without an address**: the browse-commit refuses them rather than + committing a tag that waits for initial data forever (R06:M-5). - **Ingest state is reset at every session/authoring seam** — a reconnect or a `ReinitializeAsync` must not carry a previous session's alias table into a new one. @@ -408,9 +411,18 @@ value. `MqttConnection` therefore raises `MqttConnectRejectedException`: code. (`ServerMoved` is permanent, `UseAnotherServer` is temporary — per the MQTT 5 spec.) A reconnect that completes without re-subscribing would be a healthy-looking connection that receives -nothing forever, so `OnReconnectedAsync` **throws** when zero filters are granted, tearing the session -down rather than swallowing it. A SUBACK failure degrades just the affected RawPaths to -`BadCommunicationError`. +nothing forever, so `OnReconnectedAsync` **throws** when none of the previously live filters is granted +(or the SUBSCRIBE gets no verdict), tearing the session down rather than swallowing it. A SUBACK failure +degrades just the affected RawPaths to `BadCommunicationError`. + +**A failed SUBSCRIBE is retried, never forgotten (R06:M-1).** Every topic whose SUBSCRIBE was refused or got +no verdict joins a *pending* set that the next `SubscribeAsync` and every reconnect re-request; a grant moves +it to the live set, and a redeploy that drops its tags drops it. Pending topics do not count toward the +reconnect's total-failure verdict, so one ACL-denied topic cannot reconnect-loop the driver. A plain-mode +`SubscribeAsync` whose SUBSCRIBE got **no verdict at all** (transport throw, SUBSCRIBE deadline) degrades its +refs and then **throws**, so `DriverInstanceActor` holds no handle and its health-tick reconcile re-subscribes; +a SUBACK *refusal* still returns a handle (the broker answered). A subscribe issued with no transport attached +records its topics as pending, never as established (M-11). There are **two** kinds of reconnect and only one of them fires that callback. A drop the connection's own supervisor recovers keeps the same `MqttConnection`, so `Reconnected` → `OnReconnectedAsync` @@ -479,6 +491,11 @@ with their real type; a birth type the encoder cannot represent (any `*Array`, D matters and is deliberately not configurable: a retained command would be replayed by the broker to the device on *every* future subscribe — including the device's own reconnect — so one stale write would be re-applied forever. +**A batch publishes concurrently, one topic at a time (R06:M-8).** Writes to *different* topics in one +`WriteAsync` go out together, at most 8 in flight, so a stalled broker costs a batch about one publish deadline +rather than one per request; writes to the *same* topic stay sequential in request order, so the last one wins. +A PUBACKed (QoS 1) publish advances `LastSuccessfulRead` like an inbound message; a QoS 0 Sparkplug command does not, because its completion only means the bytes left the local socket. + **Plain QoS 1 is at-least-once.** Know the consequences before pointing a Plain `writeTopic` at a non-idempotent device topic: diff --git a/docs/drivers/Sql.md b/docs/drivers/Sql.md index 7b7ad3ae..120d41c1 100644 --- a/docs/drivers/Sql.md +++ b/docs/drivers/Sql.md @@ -166,6 +166,24 @@ emits a `SqlQueryPlan` per distinct group key. Grouping is deterministic (groups order of their first member, members keep their input order, parameters keep first-appearance order), so an unchanged configuration produces byte-identical SQL. +**Every key-value and where-pair query is row-capped (R06:S-1).** A key-value plan reads at most twice as +many rows as it has distinct keys and asks for one more (`TOP (2n+1)` / `LIMIT 2n+1` / `FETCH FIRST 2n+1 ROWS +ONLY`); a where-pair wide-row plan asks for two. The reader stops there even if a provider ignores the limit. + +- **A duplicated key is refused per key.** When a key comes back more than once, only the tags on THAT key + publish `BadConfigurationError`; every other key in the group reads normally, whatever else is present or + absent. (Before this, a duplicate was last-row-wins Good.) +- **A gross excess fails the group.** Receiving the extra row past the cap means a history/events table or a + non-unique where column, which cannot be attributed because the rest is unread, so every member of the group + publishes `BadConfigurationError`. For a where-pair any second row is this case. +- Each case logs a per-plan rate-limited warning naming the table and the selector (or an example key). + `topByTimestamp` keeps its single-row limit. +- **Upgrade note:** a deployment whose key-value source carries stray duplicate rows, or whose where column is + not unique, will see those tags go Bad where they used to read an arbitrary row at Good. + +Every such refusal, like the other refusals (absent row, NULL cell, uncoercible cell), is the database +answering: it advances `LastSuccessfulRead` and restores `Healthy` after a comms fault. + ⚠️ **Group keys are case-sensitive on identifiers.** Two tags authored `dbo.T` and `DBO.T` plan as **two** groups — one extra round-trip, not a correctness problem, but worth knowing if a group count looks higher than expected. diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Browser/MqttBrowseSession.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Browser/MqttBrowseSession.cs index a231124b..1b6b691c 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Browser/MqttBrowseSession.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Browser/MqttBrowseSession.cs @@ -149,6 +149,16 @@ internal sealed class MqttBrowseSession : IRebirthCapableBrowseSession /// private static readonly UTF8Encoding Utf8Strict = new(encoderShouldEmitUTF8Identifier: false, throwOnInvalidBytes: true); + /// + /// Ceiling on the rebirth request's wait, below the AdminUI's 20 s BrowserSessionService.PerCallTimeout + /// (review minor 4): a configured window above it could never take effect there, and a rebirth aimed at an + /// offline edge node must not hold the operator's button for the whole window. + /// + internal static readonly TimeSpan MaxRebirthWait = TimeSpan.FromSeconds(15); + + /// The quiet spell that ends a rebirth request's wait once its births have started arriving. + private static readonly TimeSpan RebirthQuietInterval = TimeSpan.FromMilliseconds(500); + /// Bound on the courtesy DISCONNECT issued at dispose — teardown never hangs on a dead broker. private static readonly TimeSpan DisconnectTimeout = TimeSpan.FromSeconds(5); @@ -174,6 +184,7 @@ internal sealed class MqttBrowseSession : IRebirthCapableBrowseSession private readonly ConcurrentQueue _publishedTopics = new(); private int _disposed; + private long _birthCount; private long _lastUsedTicksUtc = DateTime.UtcNow.Ticks; private int _nodeCount; private int _oversized; @@ -195,9 +206,16 @@ internal sealed class MqttBrowseSession : IRebirthCapableBrowseSession /// The connected MQTT client; disconnected and disposed by . /// Optional logger; never receives credentials or payload bodies. /// Cap on recorded nodes; defaults to . - internal MqttBrowseSession(MqttMode mode, IMqttClient? client, ILogger? logger, int nodeCap = DefaultNodeCap) + /// + /// The Sparkplug birth-observation window: how long waits for the + /// births it asked for. (the test default) returns as soon as the NCMDs + /// are published. + /// + internal MqttBrowseSession( + MqttMode mode, IMqttClient? client, ILogger? logger, int nodeCap = DefaultNodeCap, TimeSpan birthWindow = default) { Mode = mode; + BirthWindow = birthWindow; _client = client; _logger = logger; _nodeCap = nodeCap; @@ -211,6 +229,59 @@ internal sealed class MqttBrowseSession : IRebirthCapableBrowseSession /// internal int PublishCountForTest => Volatile.Read(ref _publishCount); + /// The Sparkplug birth-observation window this session was opened with (R06:M-4). + internal TimeSpan BirthWindow { get; } + + /// How long a rebirth request waits for its births: the window, capped at . + internal TimeSpan RebirthWait => BirthWindow < MaxRebirthWait ? BirthWindow : MaxRebirthWait; + + /// How many NBIRTH/DBIRTH messages this session has observed — the settle loop's signal. + internal long BirthCount => Interlocked.Read(ref _birthCount); + + /// + /// Waits until birth traffic has gone quiet for one interval, bounded by + /// — the UntilStable cadence of the birth-observation window. + /// + /// Reads the running birth count. + /// The count before the wait began. + /// + /// When true, quiet only counts once at least one birth has arrived past + /// — the shape a rebirth request needs, since its answer takes a broker round-trip to arrive. + /// + /// The quiet spell that ends the wait. + /// The hard ceiling; non-positive returns at once. + /// Cancels the wait. + /// A task that completes when the births have settled or the window has elapsed. + internal static async Task SettleBirthsAsync( + Func birthCount, + long baseline, + bool awaitFirstBirth, + TimeSpan quiet, + TimeSpan window, + CancellationToken cancellationToken) + { + if (window <= TimeSpan.Zero) return; + + var clock = System.Diagnostics.Stopwatch.StartNew(); + var last = baseline; + var sawBirth = !awaitFirstBirth; + while (clock.Elapsed < window) + { + var step = window - clock.Elapsed; + await Task.Delay(step < quiet ? step : quiet, cancellationToken).ConfigureAwait(false); + + var now = birthCount(); + if (now != last) + { + last = now; + sawBirth = true; + continue; + } + + if (sawBirth) return; + } + } + /// The topics this session published, in order — the discriminating view of the above. internal IReadOnlyList PublishedTopicsForTest => [.. _publishedTopics]; @@ -380,16 +451,25 @@ internal sealed class MqttBrowseSession : IRebirthCapableBrowseSession var mapped = metric.DataType?.ToDriverDataType(); var sparkplugName = metric.DataType?.ToString() ?? nameof(SparkplugDataType.Unknown); + var isArray = metric.DataType?.IsSparkplugArray() ?? false; + var descriptor = new StringBuilder(metric.BirthType.ToString()).Append(" · ").Append(sparkplugName); if (metric.Alias is { } alias) descriptor.Append(" · alias ").Append(alias); if (mapped is null) descriptor.Append(" · unsupported in v1 — not bindable"); + else if (isArray) descriptor.Append(" · array metrics are not read in v1 — not bindable"); + + // Unbindable = the ingestor refuses it: an unmapped type, or ANY array variant (SparkplugIngestor + // gates on the array bit and never feeds the tag). Such a leaf states NO address, so the + // browse-commit refuses it at commit instead of writing a tag that sits at + // BadWaitingForInitialData forever (R06:M-5). Labelling it alone was not enough — it still bound. + var bindable = mapped is not null && !isArray; return new AttributeInfo( metric.Name, mapped?.ToString() ?? sparkplugName, - metric.DataType?.IsSparkplugArray() ?? false, + isArray, descriptor.ToString(), - AddressFields: BuildMetricAddressFields(metric.Name, scope)); + AddressFields: bindable ? BuildMetricAddressFields(metric.Name, scope) : null); } /// @@ -557,6 +637,8 @@ internal sealed class MqttBrowseSession : IRebirthCapableBrowseSession if (!SparkplugTopic.TryParse(topic, out var parsed)) return; if (parsed.Type is not (SparkplugMessageType.NBIRTH or SparkplugMessageType.DBIRTH)) return; + Interlocked.Increment(ref _birthCount); + // The bound has to be on the whole message: unlike a plain payload, a birth must be parsed // entire to yield its metric list, and the parse runs on the shared dispatcher thread. if (payload.Length > MaxBirthPayloadBytes) @@ -898,6 +980,7 @@ internal sealed class MqttBrowseSession : IRebirthCapableBrowseSession ArgumentException.ThrowIfNullOrWhiteSpace(scope); var targets = ResolveRebirthTargets(scope); + var baseline = BirthCount; foreach (var (groupId, edgeNodeId) in targets) { @@ -908,6 +991,20 @@ internal sealed class MqttBrowseSession : IRebirthCapableBrowseSession .ConfigureAwait(false); } + // The birth-observation window (R06:M-4): wait — bounded — for the births just asked for to arrive + // and settle, so the tree the picker re-renders after this call already carries them. A courtesy, + // never a failure: the NCMDs are out, so a caller deadline here still reports the publish. + try + { + await SettleBirthsAsync( + () => BirthCount, baseline, awaitFirstBirth: true, RebirthQuietInterval, RebirthWait, cancellationToken) + .ConfigureAwait(false); + } + catch (OperationCanceledException) + { + // Published; the caller simply stopped waiting for the answer. + } + Touch(); return targets.Count; } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Browser/MqttDriverBrowser.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Browser/MqttDriverBrowser.cs index 3ef6b659..8be8affc 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Browser/MqttDriverBrowser.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Browser/MqttDriverBrowser.cs @@ -39,6 +39,15 @@ public sealed class MqttDriverBrowser : IDriverBrowser /// Ceiling on the open budget — the picker must never hang on an unreachable broker. internal const int MaxOpenBudgetSeconds = 30; + /// Ceiling on the Sparkplug birth-observation window — an authored hour must not park a picker. + internal const int MaxBirthWindowSeconds = 120; + + /// + /// How long the open waits for a quiet spell in birth traffic before handing the session back. An + /// idle plant (births are never retained) costs exactly this, once. + /// + internal static readonly TimeSpan BirthQuietInterval = TimeSpan.FromSeconds(1); + /// Marks the transient browse identity in the broker's client-id/session logs. internal const string BrowseClientIdPrefix = "-browse-"; @@ -77,7 +86,8 @@ public sealed class MqttDriverBrowser : IDriverBrowser openCts.CancelAfter(OpenBudget(opts)); var client = new MqttClientFactory().CreateMqttClient(); - var session = new MqttBrowseSession(opts.Mode, client, _logger); + var birthWindow = opts.Mode == MqttMode.SparkplugB ? BirthWindow(opts) : TimeSpan.Zero; + var session = new MqttBrowseSession(opts.Mode, client, _logger, birthWindow: birthWindow); try { // Runs on MQTTnet's dispatcher: record and return, nothing else. @@ -100,6 +110,29 @@ public sealed class MqttDriverBrowser : IDriverBrowser .Build(); await client.SubscribeAsync(subscribeOptions, openCts.Token).ConfigureAwait(false); + if (opts.Mode == MqttMode.SparkplugB) + { + // The birth-observation window (R06:M-4): births already in flight — an edge node mid-rebirth, + // a primary-host flap — are collected until the traffic goes quiet, bounded by the window, so + // the first RootAsync is not a snapshot taken in the middle of a burst. Passive: nothing is + // published to provoke a birth. An idle plant costs one quiet interval. + try + { + await MqttBrowseSession.SettleBirthsAsync( + () => session.BirthCount, + baseline: session.BirthCount, + awaitFirstBirth: false, + BirthQuietInterval, + birthWindow, + openCts.Token) + .ConfigureAwait(false); + } + catch (OperationCanceledException) when (!cancellationToken.IsCancellationRequested) + { + // The open budget ran out mid-settle: what has been observed is still a usable session. + } + } + _logger.LogInformation( "AdminUI MQTT browse session opened against {Host}:{Port} in {Mode} mode observing " + "'{Filter}' (read-only).", @@ -179,12 +212,30 @@ public sealed class MqttDriverBrowser : IDriverBrowser options with { CleanSession = true }; /// - /// Whole-open bound (connect + subscribe), clamped to - /// – around the config's - /// own connect deadline. + /// Whole-open bound: connect + subscribe, clamped to + /// – around the config's own + /// connect deadline — plus, in Sparkplug mode, the the open may spend + /// collecting births. /// /// The browse configuration. - /// The clamped open budget. - internal static TimeSpan OpenBudget(MqttDriverOptions options) => - TimeSpan.FromSeconds(Math.Clamp(options.ConnectTimeoutSeconds, MinOpenBudgetSeconds, MaxOpenBudgetSeconds)); + /// The open budget. + internal static TimeSpan OpenBudget(MqttDriverOptions options) + { + var connect = TimeSpan.FromSeconds( + Math.Clamp(options.ConnectTimeoutSeconds, MinOpenBudgetSeconds, MaxOpenBudgetSeconds)); + return options.Mode == MqttMode.SparkplugB ? connect + BirthWindow(options) : connect; + } + + /// + /// The authored sparkplug.birthObservationWindowSeconds, clamped to 1– + /// s: how long the browse surface collects NBIRTH/DBIRTH traffic before calling the observed metric set + /// stable (on open, and after an operator rebirth request). + /// + /// The browse configuration. + /// The clamped window. + internal static TimeSpan BirthWindow(MqttDriverOptions options) => + TimeSpan.FromSeconds(Math.Clamp( + options.Sparkplug?.BirthObservationWindowSeconds ?? new MqttSparkplugOptions().BirthObservationWindowSeconds, + 1, + MaxBirthWindowSeconds)); } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/LastValueCache.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/LastValueCache.cs index 7df9dd0e..5ba310bf 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/LastValueCache.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/LastValueCache.cs @@ -45,6 +45,30 @@ public sealed class LastValueCache _values[rawPath] = snapshot; } + /// How many RawPaths currently hold an entry. Diagnostics and the bounded-growth tests. + internal int Count => _values.Count; + + /// + /// Drops every entry whose RawPath rejects — called by each ingest path's + /// Register with "is this still an authored tag", so the cache is bounded by the CURRENT + /// authored set rather than by the union of every RawPath (and every non-authored reference ever + /// subscribed) across the driver's lifetime (R06:M-2). + /// + /// Returns for a RawPath whose entry must survive. + /// How many entries were dropped. + public int RetainOnly(Func keep) + { + ArgumentNullException.ThrowIfNull(keep); + var dropped = 0; + foreach (var rawPath in _values.Keys) + { + if (!keep(rawPath) && _values.TryRemove(rawPath, out _)) + dropped++; + } + + return dropped; + } + /// /// Read the last observed value for . Never throws: a /// null/empty or a RawPath never observed both return a diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttDriver.cs index b8e92621..c265dce1 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttDriver.cs @@ -104,6 +104,12 @@ public sealed class MqttDriver /// private const string DiscoveryFolderName = "Mqtt"; + /// + /// Ceiling on PUBLISHes one write batch keeps in flight (R06:M-8). Bounded so a large batch cannot + /// flood a broker's inflight window; different topics share it, one topic's writes stay sequential. + /// + internal const int MaxConcurrentPublishes = 8; + /// How often the host-connectivity poll samples . private static readonly TimeSpan HostProbeInterval = TimeSpan.FromSeconds(1); @@ -208,6 +214,18 @@ public sealed class MqttDriver private SparkplugPrimaryHost? _primaryHost; private DriverHealth _health = new(DriverState.Unknown, null, null); + + /// UTC ticks of the last successful write PUBLISH; 0 = none. Answer evidence for health. + private long _lastPublishUtcTicks; + + /// Guards the generation bump against the generation-checked health write. + private readonly object _healthLock = new(); + + /// + /// Bumped by every lifecycle operation (initialize inside the gate; shutdown and dispose before + /// waiting for it). The stamp that keeps a superseded operation from publishing state. + /// + private long _lifecycleGeneration; private HostState _hostState = HostState.Unknown; private DateTime _hostStateChangedUtc = DateTime.UtcNow; private int _disposed; @@ -289,6 +307,9 @@ public sealed class MqttDriver /// internal Func? BeforeConnectHookForTests { get; set; } + /// Test seam: observes every an initialize builds. Null in production. + internal Action? ConnectionCreatedForTests { get; set; } + // ---- IDriver: lifecycle ---- /// @@ -387,6 +408,11 @@ public sealed class MqttDriver // watchdog's reconnect) is measured against, reporting a driver that was delivering a moment ago // as one that has never delivered. var lastMessage = GetHealth().LastSuccessfulRead; + + // Taken BEFORE the gate wait: a wedged initialize holding the gate is superseded by this call, so + // when it finally completes it disposes the session it built instead of publishing it behind the + // ungated teardown below (R06:M-10), and its late health write is refused. + var generation = BeginLifecycleOperation(); if (await _lifecycleGate.WaitAsync(TimeSpan.FromSeconds(_options.ConnectTimeoutSeconds), cancellationToken) .ConfigureAwait(false)) { @@ -408,8 +434,10 @@ public sealed class MqttDriver await TeardownAsync().ConfigureAwait(false); } - WriteHealth(new DriverHealth(DriverState.Unknown, lastMessage, null)); - TransitionHost(HostState.Unknown); + if (WriteHealthIfCurrent(generation, new DriverHealth(DriverState.Unknown, lastMessage, null))) + { + TransitionHost(HostState.Unknown); + } } /// @@ -427,7 +455,7 @@ public sealed class MqttDriver return stored; } - var lastMessage = connection.LastMessageUtc ?? stored.LastSuccessfulRead; + var lastMessage = Latest(connection.LastMessageUtc ?? stored.LastSuccessfulRead, LastPublishUtc); return connection.State switch { MqttConnectionState.Connected => new DriverHealth(DriverState.Healthy, lastMessage, stored.LastError), @@ -689,9 +717,12 @@ public sealed class MqttDriver { ArgumentNullException.ThrowIfNull(writes); - var results = new WriteResult[writes.Count]; + var results = new WriteResult?[writes.Count]; var connection = _connection; + // Pass 1 — everything that needs no broker: resolve, encode, refuse. What survives is grouped by + // destination topic in REQUEST order. + var byTopic = new Dictionary>(StringComparer.Ordinal); for (var i = 0; i < writes.Count; i++) { var w = writes[i]; @@ -719,22 +750,72 @@ public sealed class MqttDriver continue; } + if (!byTopic.TryGetValue(encoded.Topic, out var queue)) + { + byTopic[encoded.Topic] = queue = []; + } + + queue.Add((i, encoded, w.FullReference)); + } + + // Pass 2 — publish (R06:M-8). Different topics go out concurrently, at most MaxConcurrentPublishes + // in flight, so a stalled broker costs a batch ~one publish deadline instead of one per request. + // Writes to the SAME topic stay strictly sequential in request order: the device applies them in + // arrival order, and the last one must win. + if (byTopic.Count > 0) + { + using var slots = new SemaphoreSlim(MaxConcurrentPublishes, MaxConcurrentPublishes); + await Task.WhenAll(byTopic.Values.Select(queue => PublishQueueAsync(connection!, queue, slots, results, cancellationToken))) + .ConfigureAwait(false); + } + + // Every slot is filled — a caller cancellation leaves the unattempted ones BadCommunicationError + // rather than throwing, so the batch shape holds. + var filled = new WriteResult[writes.Count]; + for (var i = 0; i < filled.Length; i++) + { + filled[i] = results[i] ?? new WriteResult(StatusBadCommunicationError); + } + + return filled; + } + + /// Publishes one topic's writes in request order, each under a shared concurrency slot. + private async Task PublishQueueAsync( + MqttConnection connection, + List<(int Index, MqttEncodedWrite Encoded, string Reference)> queue, + SemaphoreSlim slots, + WriteResult?[] results, + CancellationToken cancellationToken) + { + foreach (var (index, encoded, reference) in queue) + { + try + { + await slots.WaitAsync(cancellationToken).ConfigureAwait(false); + } + catch (OperationCanceledException) + { + return; // caller-cancelled: the remaining slots are filled by the caller + } + try { await connection .PublishAsync(encoded.Topic, encoded.Payload, encoded.Qos, retain: false, cancellationToken) .ConfigureAwait(false); - results[i] = new WriteResult(0u); + results[index] = new WriteResult(0u); + + // A PUBACK is the broker answering — evidence the session works, so it feeds + // LastSuccessfulRead alongside inbound messages. A completed QoS 0 publish is NOT: it only + // means the bytes reached the local socket buffer, and frequent Sparkplug commands would + // otherwise keep the watchdog's clock fresh over a half-open session (review minor 2). + if (encoded.Qos >= 1) Interlocked.Exchange(ref _lastPublishUtcTicks, DateTime.UtcNow.Ticks); } catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) { - // Caller-cancelled: fill the remainder rather than throwing, so the batch shape holds. - for (var j = i; j < writes.Count; j++) - { - results[j] = new WriteResult(StatusBadCommunicationError); - } - - return results; + results[index] = new WriteResult(StatusBadCommunicationError); + return; } catch (Exception ex) { @@ -742,13 +823,15 @@ public sealed class MqttDriver ex, "MQTT driver '{DriverId}': write to '{Reference}' could not be published on '{Topic}'.", _driverInstanceId, - w.FullReference, + reference, encoded.Topic); - results[i] = new WriteResult(StatusBadCommunicationError); + results[index] = new WriteResult(StatusBadCommunicationError); + } + finally + { + slots.Release(); } } - - return results; } /// Maps an encode refusal to the status an OPC UA client sees. @@ -911,6 +994,9 @@ public sealed class MqttDriver return; } + // Supersedes any in-flight lifecycle operation, exactly as ShutdownAsync does. + BeginLifecycleOperation(); + if (await _lifecycleGate.WaitAsync(TimeSpan.FromSeconds(_options.ConnectTimeoutSeconds)) .ConfigureAwait(false)) { @@ -940,7 +1026,12 @@ public sealed class MqttDriver /// The gate-held body of . private async Task InitializeCoreAsync(string driverConfigJson, CancellationToken cancellationToken) { - WriteHealth(new DriverHealth(DriverState.Initializing, ReadHealth().LastSuccessfulRead, null)); + // Taken INSIDE the gate (the caller holds it), so a second initialize queued behind this one does + // not supersede it; only a ShutdownAsync / DisposeAsync — which take theirs before waiting — can. + var generation = BeginLifecycleOperation(); + WriteHealthIfCurrent(generation, new DriverHealth(DriverState.Initializing, ReadHealth().LastSuccessfulRead, null)); + MqttConnection? built = null; + var published = false; try { // Tear down whatever this driver already holds BEFORE building a new session. Both fields @@ -965,6 +1056,8 @@ public sealed class MqttDriver RegisterAuthoredTags(_options); var connection = new MqttConnection(_options, _driverInstanceId, _logger); + built = connection; + ConnectionCreatedForTests?.Invoke(connection); // MUST precede the connect: this wires message delivery AND the reconnect re-subscribe. // The handler is passed through unwrapped on purpose — its throw-on-total-failure is what @@ -992,10 +1085,13 @@ public sealed class MqttDriver // disposing what we just built, never leaking it. if (Volatile.Read(ref _disposed) != 0) { - await connection.DisposeAsync().ConfigureAwait(false); throw new ObjectDisposedException(nameof(MqttDriver)); } + // The same race against a ShutdownAsync whose bounded gate wait expired: it has already torn + // down ungated, so publishing now would leave a live session no later shutdown can see. + ThrowIfSuperseded(generation); + // Sparkplug subscribes ONCE, here, rather than on the OPC UA server's first subscribe: the // driver must see birth certificates whether or not anything is subscribed yet, because a // birth is where every alias and datatype comes from. This also issues the late-join @@ -1008,28 +1104,47 @@ public sealed class MqttDriver // MqttConnection's own remarks describe having already fixed once. if (_sparkplug is { } sparkplug) { - try - { - await sparkplug.EstablishAsync(cancellationToken).ConfigureAwait(false); - } - catch - { - await connection.DisposeAsync().ConfigureAwait(false); - throw; - } + await sparkplug.EstablishAsync(cancellationToken).ConfigureAwait(false); } + ThrowIfSuperseded(generation); _connection = connection; + published = true; + + if (WriteHealthIfCurrent(generation, new DriverHealth(DriverState.Healthy, connection.LastMessageUtc, null))) + { + TransitionHost(HostState.Running); + } - WriteHealth(new DriverHealth(DriverState.Healthy, connection.LastMessageUtc, null)); - TransitionHost(HostState.Running); StartHostProbe(); await StartPrimaryHostAsync(cancellationToken).ConfigureAwait(false); + + // Re-checked after publishing: a shutdown whose gate wait expired between the check above and + // here captured whatever it found and left these fields for us. Intended consequence (review + // minor 8): a perfectly healthy init that is just completing when a ShutdownAsync arrives also + // releases itself and throws OperationCanceledException — the queued shutdown would tear it down + // anyway, and the actor serialises init/shutdown, so only an abandoned init can meet this. We still hold the gate, so no + // newer initialize can have published anything — everything in them is ours to release. + if (!IsCurrentLifecycle(generation)) + { + await TeardownAsync().ConfigureAwait(false); + ThrowIfSuperseded(generation); + } } catch (Exception ex) { - WriteHealth(new DriverHealth(DriverState.Faulted, ReadHealth().LastSuccessfulRead, ex.Message)); - TransitionHost(HostState.Faulted); + // R06:M-3: a connection built but never published is disposed here — a refused connect, a + // failed Sparkplug establish, a supersede. The published one belongs to TeardownAsync. + if (!published && built is not null) + { + await built.DisposeAsync().ConfigureAwait(false); + } + + if (WriteHealthIfCurrent(generation, new DriverHealth(DriverState.Faulted, ReadHealth().LastSuccessfulRead, ex.Message))) + { + TransitionHost(HostState.Faulted); + } + throw; } } @@ -1380,24 +1495,28 @@ public sealed class MqttDriver /// Shared teardown for , and the session rebuild. private async Task TeardownAsync() { + // Every field is CAPTURED before anything is disposed. A shutdown that timed out on the gate runs + // this ungated; capturing up front means it only ever releases what existed when it started, and + // never reaches into fields a later initialize publishes while the slow disposals below run. + var primaryHost = Interlocked.Exchange(ref _primaryHost, null); + var probe = Interlocked.Exchange(ref _hostProbeCts, null); + var connection = Interlocked.Exchange(ref _connection, null); + // Stand the host claim down FIRST, while the broker is still reachable and before the ingest // session goes. Its explicit retained OFFLINE is what makes the transition immediate for edge // nodes; leaving it to the Will costs them up to 1.5 keep-alive periods of believing a host // that has already stopped is still online. - var primaryHost = Interlocked.Exchange(ref _primaryHost, null); if (primaryHost is not null) { await primaryHost.DisposeAsync().ConfigureAwait(false); } - var probe = Interlocked.Exchange(ref _hostProbeCts, null); if (probe is not null) { await probe.CancelAsync().ConfigureAwait(false); probe.Dispose(); } - var connection = Interlocked.Exchange(ref _connection, null); if (connection is not null) { await connection.DisposeAsync().ConfigureAwait(false); @@ -1476,9 +1595,66 @@ public sealed class MqttDriver OnHostStatusChanged?.Invoke(this, new HostStatusChangedEventArgs(HostName, previous, next)); } + /// The last successful write publish, or null. + private DateTime? LastPublishUtc + { + get + { + var ticks = Interlocked.Read(ref _lastPublishUtcTicks); + return ticks == 0 ? null : new DateTime(ticks, DateTimeKind.Utc); + } + } + + /// The later of two optional instants. + private static DateTime? Latest(DateTime? a, DateTime? b) + => a is null ? b : b is null ? a : (a > b ? a : b); + /// Barrier-protected read of the multi-threaded health field. private DriverHealth ReadHealth() => Volatile.Read(ref _health); - /// Barrier-protected publish of a new health snapshot. - private void WriteHealth(DriverHealth value) => Volatile.Write(ref _health, value); + /// + /// Starts a lifecycle operation and returns its generation. Every later operation supersedes it: + /// its health / host-state writes are refused and an initialize stops before publishing a session. + /// + private long BeginLifecycleOperation() + { + lock (_healthLock) + { + return ++_lifecycleGeneration; + } + } + + /// Whether is still the newest lifecycle operation. + private bool IsCurrentLifecycle(long generation) => Interlocked.Read(ref _lifecycleGeneration) == generation; + + /// Throws when a newer lifecycle operation has superseded . + private void ThrowIfSuperseded(long generation) + { + if (!IsCurrentLifecycle(generation)) + { + throw new OperationCanceledException( + $"MQTT driver '{_driverInstanceId}': this initialize was superseded by a later shutdown/dispose; " + + "the session it built has been released."); + } + } + + /// + /// Publishes a health snapshot only while is the newest lifecycle + /// operation — the check and the write are atomic against , so a + /// superseded initialize or shutdown can never overwrite a newer one's state. + /// + /// when the write was applied. + private bool WriteHealthIfCurrent(long generation, DriverHealth value) + { + lock (_healthLock) + { + if (_lifecycleGeneration != generation) + { + return false; + } + + Volatile.Write(ref _health, value); + return true; + } + } } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttSubscriptionManager.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttSubscriptionManager.cs index 92c45051..2a80209d 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttSubscriptionManager.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttSubscriptionManager.cs @@ -48,7 +48,8 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Mqtt; /// accepted by the parser (warned at deploy, see MqttTagDefinitionFactory.Inspect), so /// matching uses MQTTnet's own rather than string /// equality — the same comparer a broker applies. Concrete topics stay a single dictionary -/// lookup; the linear wildcard scan runs only when a wildcard tag actually exists. +/// lookup; wildcard filters live in a level trie (TopicFilterTrie) whose candidates the comparer +/// confirms, so a thousand wildcard tags cost a walk over the topic's levels, not a thousand compares. /// /// /// Sparkplug seam. P1 ingests plain topics only. Sparkplug B decodes a @@ -74,6 +75,13 @@ public sealed class MqttSubscriptionManager /// public const int DefaultQos = 1; + /// + /// Throw-on-invalid UTF-8, shared (R06:M-7): it is stateless and thread-safe, and allocating one per + /// Raw/Scalar delivery was per-message garbage on the dispatcher thread. Same choice + /// MqttBrowseSession makes. + /// + private static readonly UTF8Encoding Utf8Strict = new(encoderShouldEmitUTF8Identifier: false, throwOnInvalidBytes: true); + // OPC UA status codes (values verified against Opc.Ua.StatusCodes, not recalled). private const uint StatusGood = 0x00000000u; @@ -113,6 +121,14 @@ public sealed class MqttSubscriptionManager /// Topics already SUBSCRIBEd on the live session — the dedupe set and the re-subscribe set. private readonly ConcurrentDictionary _subscribedTopics = new(StringComparer.Ordinal); + /// + /// Topics a subscribe asked for whose SUBSCRIBE failed (refused, or no verdict at all). Retried by + /// every later and by ; moved to + /// when granted. Without it a failed topic left every retry set and + /// stayed dark behind a Healthy connection until a redeploy. + /// + private readonly ConcurrentDictionary _pendingTopics = new(StringComparer.Ordinal); + /// RawPaths whose extraction failure has already been logged loudly once. private readonly ConcurrentDictionary _warnedRefs = new(StringComparer.Ordinal); @@ -212,8 +228,8 @@ public sealed class MqttSubscriptionManager /// The manager does not own the connection's lifetime and never detaches — the driver disposes /// both together. Call once per connection, and before the first /// : a subscribe issued with no transport attached records its - /// intent (and warns) but sends nothing to a broker, and would only be established if the - /// connection later dropped and recovered. + /// topics as pending (and warns) but sends nothing to a broker; the next subscribe or + /// reconnect on an attached session retries them. /// /// /// Binding to a session forgets the previous session's established filters — the same @@ -317,6 +333,11 @@ public sealed class MqttSubscriptionManager _authored = table; _warnedRefs.Clear(); + // Bound the shared last-value cache by the CURRENT authored set (R06:M-2): a dropped tag's value + // and every non-authored reference's BadNodeIdUnknown marker would otherwise live forever. A + // still-authored tag keeps its value, so a redeploy never blanks what has been observed. + Values.RetainOnly(table.ByRawPath.ContainsKey); + // Drop state belonging to tags this deploy removed. Without this, a topic whose last tag was // deleted stays in _subscribedTopics and is dutifully re-SUBSCRIBEd on every future reconnect, // forever — and its RawPaths keep a handle mapping that can never fire again. @@ -328,6 +349,14 @@ public sealed class MqttSubscriptionManager } } + foreach (var topic in _pendingTopics.Keys) + { + if (!table.ByFilter.ContainsKey(topic)) + { + _pendingTopics.TryRemove(topic, out _); + } + } + foreach (var rawPath in _handleByRawPath.Keys) { if (!table.ByRawPath.ContainsKey(rawPath)) @@ -356,12 +385,13 @@ public sealed class MqttSubscriptionManager /// opening SUBSCRIBE itself; only re-establishes it later. /// /// - /// Never throws, never hangs. A reference that is not an authored tag is recorded as - /// BadNodeIdUnknown; a SUBACK rejection, transport failure or subscribe deadline - /// degrades exactly the affected references to BadCommunicationError. Either way a - /// handle is returned — the OPC UA server's subscribe call must not fail because one topic - /// is ACL-denied. Recovery for a transient failure comes from the reconnect path; - /// a persistent broker-side refusal is visible as a Bad status on the affected nodes. + /// Never hangs; throws only when the broker gave no verdict. A reference that is not an + /// authored tag is recorded as BadNodeIdUnknown. A SUBACK rejection degrades exactly the + /// affected references to BadCommunicationError and still returns a handle — the OPC UA + /// server's subscribe must not fail because one topic is ACL-denied. A transport failure or + /// subscribe deadline degrades the same way and then throws, so the host holds no handle + /// and its reconcile re-subscribes. Every failed topic also joins a pending set that the next + /// subscribe and every reconnect retry. /// /// /// The RawPath references to subscribe. @@ -414,12 +444,56 @@ public sealed class MqttSubscriptionManager // One SUBSCRIBE per distinct topic, and only for topics not already live: re-SUBSCRIBEing an // established filter is legal but pointless, and it re-triggers the retained seed. - var pending = defs.Where(d => !_subscribedTopics.ContainsKey(d.Topic)).ToList(); - await EstablishAsync(BuildFilters(pending, _defaultQos), cancellationToken).ConfigureAwait(false); + // Topics an EARLIER subscribe failed to establish ride along: a redeploy's bulk re-subscribe is then a + // retry of a refusal the broker may since have lifted, instead of a no-op that leaves them dark. + var authored = _authored; + var pending = defs.Where(d => !_subscribedTopics.ContainsKey(d.Topic)) + .Concat(PendingDefinitions(authored)) + .ToList(); + var outcome = await EstablishAsync(BuildFilters(pending, _defaultQos), cancellationToken).ConfigureAwait(false); + if (outcome.TransportFailure is { } failure) + { + // The refs are already Bad and pending either way. But a cancel of the CALLER's token (a pipeline + // timeout, host shutdown) is a cancellation, not a broker fault: surfacing it as + // InvalidOperationException made the resilience breaker count it as a failure (review minor 3). + var callerCancelled = cancellationToken.IsCancellationRequested; + // No verdict at all (transport throw / SUBSCRIBE deadline): THROW, so the caller holds no handle + // and DriverInstanceActor's health-tick reconcile re-subscribes. Returning a handle here was the + // silent-death case — the actor thought the subscription live and never retried. The refs already + // carry BadCommunicationError (raised under this handle above), so the throw costs no visibility. + _refsByHandle.TryRemove(handle, out _); + foreach (var def in defs) + { + _handleByRawPath.TryRemove(new KeyValuePair(def.Name, handle)); + } + + if (callerCancelled) + { + throw new OperationCanceledException( + $"MQTT driver '{_driverId}': SUBSCRIBE for {pending.Count} tag(s) was cancelled by the caller; " + + "the topics stay pending and will be retried.", + failure, + cancellationToken); + } + + throw new InvalidOperationException( + $"MQTT driver '{_driverId}': SUBSCRIBE for {pending.Count} tag(s) got no broker verdict " + + $"({failure.GetType().Name}); the subscription will be retried.", + failure); + } + + // A SUBACK refusal (an ACL denial, say) does NOT throw: the broker answered, the refused topics are + // Bad + pending, and failing the whole subscribe would take every other topic in it dark too. return handle; } + /// The authored definitions behind every still-authored pending topic. + private IEnumerable PendingDefinitions(AuthoredTable authored) + => _pendingTopics.Keys + .Select(topic => authored.ByFilter.TryGetValue(topic, out var defs) ? defs : []) + .SelectMany(defs => defs); + /// /// Stops attributing notifications to . References another live /// subscription also covers keep publishing. @@ -498,8 +572,13 @@ public sealed class MqttSubscriptionManager .SelectMany(defs => defs) .ToList(); - var filters = BuildFilters(live, _defaultQos); - if (filters.Count == 0) + // Pending topics (a failed earlier SUBSCRIBE) are retried here too — a reconnect re-subscribing + // only the granted set is how a failed topic stayed dark forever. They do NOT count toward the + // total-failure verdict below: re-refusing a topic that was never live loses nothing, and throwing + // for it would turn one ACL-denied topic into a reconnect loop for the whole driver. + var pendingDefs = PendingDefinitions(authored).ToList(); + var filters = BuildFilters(live.Concat(pendingDefs), _defaultQos); + if (live.Count == 0 && pendingDefs.Count == 0) { // Silence here is how the wildcard defect hid: no exception, no log, no SUBSCRIBE. Live // topics resolving to zero filters re-establishes nothing, which is the total-failure case @@ -519,13 +598,17 @@ public sealed class MqttSubscriptionManager return; } - var granted = await EstablishAsync(filters, cancellationToken).ConfigureAwait(false); - if (granted == 0) + var outcome = await EstablishAsync(filters, cancellationToken).ConfigureAwait(false); + var liveTopics = live.Select(d => d.Topic).Distinct(StringComparer.Ordinal).ToList(); + var liveLost = outcome.TransportFailure is not null + || (subscribed.Count > 0 && (liveTopics.Count == 0 || !liveTopics.Any(outcome.Granted.Contains))); + if (liveLost) { throw new InvalidOperationException( $"MQTT driver '{_driverId}': no subscription could be re-established after reconnect " + $"({filters.Count} filter(s) requested); tearing the session down rather than serving a " - + "connection that receives nothing."); + + "connection that receives nothing.", + outcome.TransportFailure); } } @@ -582,13 +665,14 @@ public sealed class MqttSubscriptionManager } // A published topic name may not itself contain a wildcard, so a wildcard-authored - // definition can never also be an exact-index hit: no tag is delivered twice. - foreach (var def in authored.Wildcards) + // definition can never also be an exact-index hit: no tag is delivered twice. The trie walks + // the topic's levels (R06:M-6 — this used to be one comparer call per wildcard tag, per + // message, on the shared dispatcher thread) and every candidate it yields is confirmed by + // MQTTnet's own comparer inside the visitor, so the semantics are the broker's. + if (authored.WildcardTrie.Count > 0) { - if (MqttTopicFilterComparer.Compare(topic, def.Topic) == MqttTopicFilterCompareResult.IsMatch) - { - Deliver(def, payload, retained, now, oversized, ref json); - } + var visitor = new WildcardDelivery(this, topic, payload, retained, now, oversized, ref json); + authored.WildcardTrie.Match(topic, ref visitor); } } finally @@ -633,35 +717,35 @@ public sealed class MqttSubscriptionManager /// /// The filters to establish; an empty list is a no-op. /// Cancellation for the round-trip. - /// How many filters the broker granted. - private async Task EstablishAsync( + /// The granted topics, plus the transport failure when the broker gave no verdict at all. + private async Task EstablishAsync( IReadOnlyList filters, CancellationToken cancellationToken) { if (filters.Count == 0) { - return 0; + return EstablishOutcome.Nothing; } var transport = _transport; if (transport is null) { - // No transport: the intent is recorded so the reconnect path establishes it, but NOTHING - // reaches a broker now. Loud, because outside tests this means the composition order is - // wrong — AttachTo must run before the OPC UA server's first subscribe, or these topics - // stay unestablished until the connection happens to drop and recover. + // No transport: NOTHING reaches a broker, so nothing is recorded as subscribed — recording it + // was a silent "subscribed, receives nothing" if the AttachTo-before-subscribe ordering ever + // changed. The topics wait in the pending set, which the next subscribe and every reconnect + // retry. Loud, because outside tests this means the composition order is wrong. foreach (var filter in filters) { - _subscribedTopics[filter.Topic] = 0; + _pendingTopics[filter.Topic] = 0; } _logger?.LogWarning( - "MQTT driver '{DriverId}': {FilterCount} subscription(s) recorded with no connection attached; " - + "nothing was sent to a broker. Call AttachTo before subscribing.", + "MQTT driver '{DriverId}': {FilterCount} subscription(s) requested with no connection attached; " + + "nothing was sent to a broker and they stay pending. Call AttachTo before subscribing.", _driverId, filters.Count); - return filters.Count; + return EstablishOutcome.Nothing; } IReadOnlyList outcomes; @@ -681,16 +765,17 @@ public sealed class MqttSubscriptionManager DegradeTopic(filter.Topic, ex.GetType().Name); } - return 0; + return new EstablishOutcome(new HashSet(StringComparer.Ordinal), ex); } - var granted = 0; + var granted = new HashSet(StringComparer.Ordinal); foreach (var outcome in outcomes) { if (outcome.Granted) { _subscribedTopics[outcome.Topic] = 0; - granted++; + _pendingTopics.TryRemove(outcome.Topic, out _); + granted.Add(outcome.Topic); } else { @@ -703,7 +788,16 @@ public sealed class MqttSubscriptionManager } } - return granted; + return new EstablishOutcome(granted, null); + } + + /// One SUBSCRIBE round-trip's verdict. + /// The filters the broker granted. + /// Set when the call produced no verdict at all (throw / deadline). + private sealed record EstablishOutcome(IReadOnlySet Granted, Exception? TransportFailure) + { + public static readonly EstablishOutcome Nothing = + new(new HashSet(StringComparer.Ordinal), null); } /// Marks every authored reference on as unreachable. @@ -722,6 +816,7 @@ public sealed class MqttSubscriptionManager if (!_authored.ByFilter.TryGetValue(topic, out var defs)) { + _pendingTopics.TryRemove(topic, out _); _logger?.LogWarning( "MQTT driver '{DriverId}': cannot degrade tags for unestablished filter '{Topic}' ({Reason}) — " + "no authored tag resolves to it.", @@ -731,6 +826,9 @@ public sealed class MqttSubscriptionManager return; } + // Still authored, so it must be retried — see _pendingTopics. + _pendingTopics[topic] = 0; + var now = DateTime.UtcNow; var snapshot = new DataValueSnapshot(null, StatusBadCommunicationError, null, now); foreach (var def in defs) @@ -1371,8 +1469,7 @@ public sealed class MqttSubscriptionManager { try { - text = new UTF8Encoding(encoderShouldEmitUTF8Identifier: false, throwOnInvalidBytes: true) - .GetString(payload); + text = Utf8Strict.GetString(payload); return true; } catch (DecoderFallbackException) @@ -1400,6 +1497,47 @@ public sealed class MqttSubscriptionManager return $"mqtt:{_mode}:{id}:[{named}{overflow}]"; } + /// + /// Delivers each wildcard-trie candidate whose filter the MQTTnet comparer confirms. A ref struct + /// so the payload span and the fan-out's shared JSON parse ride along without a closure or a list. + /// + private ref struct WildcardDelivery : ITopicFilterVisitor + { + private readonly MqttSubscriptionManager _manager; + private readonly string _topic; + private readonly ReadOnlySpan _payload; + private readonly bool _retained; + private readonly DateTime _now; + private readonly bool _oversized; + private ref PayloadJson _json; + + public WildcardDelivery( + MqttSubscriptionManager manager, + string topic, + ReadOnlySpan payload, + bool retained, + DateTime now, + bool oversized, + ref PayloadJson json) + { + _manager = manager; + _topic = topic; + _payload = payload; + _retained = retained; + _now = now; + _oversized = oversized; + _json = ref json; + } + + public void Visit(MqttTagDefinition def) + { + if (MqttTopicFilterComparer.Compare(_topic, def.Topic) == MqttTopicFilterCompareResult.IsMatch) + { + _manager.Deliver(def, _payload, _retained, _now, _oversized, ref _json); + } + } + } + /// Opaque subscription identity; the diagnostic id names the mode and its topics. /// The human-readable identity surfaced for diagnostics. private sealed record MqttSubscriptionHandle(string DiagnosticId) : ISubscriptionHandle; @@ -1411,7 +1549,8 @@ public sealed class MqttSubscriptionManager /// /// Two topic indexes, and the difference is load-bearing. Delivery matches an /// incoming published topic, which never carries a wildcard, so it uses - /// (concrete only) plus a comparer scan of . + /// (concrete only) plus a walk whose candidates the + /// comparer confirms. /// Every path that instead reconstructs state from a subscribed filter string — /// reconnect re-subscribe and per-topic degradation — must use , which is /// keyed by the tag's authored topic including wildcard patterns, because that is what @@ -1425,12 +1564,12 @@ public sealed class MqttSubscriptionManager /// included. Keyed the same way _subscribedTopics and SUBACK outcomes are. /// /// Concrete topic → its definitions; the delivery fast path. - /// Definitions whose authored topic carries an MQTT wildcard. + /// Definitions whose authored topic carries an MQTT wildcard, by filter level. private sealed record AuthoredTable( FrozenDictionary ByRawPath, FrozenDictionary ByFilter, FrozenDictionary ByExactTopic, - MqttTagDefinition[] Wildcards) + TopicFilterTrie WildcardTrie) { public static readonly AuthoredTable Empty = Build(new Dictionary(StringComparer.Ordinal)); @@ -1480,7 +1619,7 @@ public sealed class MqttSubscriptionManager byRawPath.ToFrozenDictionary(StringComparer.Ordinal), byFilter.ToFrozenDictionary(kv => kv.Key, kv => kv.Value.ToArray(), StringComparer.Ordinal), concrete.ToFrozenDictionary(kv => kv.Key, kv => kv.Value.ToArray(), StringComparer.Ordinal), - [.. wildcards]); + TopicFilterTrie.Build(wildcards.Select(d => (d.Topic, d)))); } } } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/Sparkplug/SparkplugIngestor.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/Sparkplug/SparkplugIngestor.cs index cd368eae..5d63e1b7 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/Sparkplug/SparkplugIngestor.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/Sparkplug/SparkplugIngestor.cs @@ -416,6 +416,11 @@ public sealed class SparkplugIngestor _authored = table; _warned.Clear(); + // Bound the shared last-value cache by the CURRENT authored set (R06:M-2): a dropped tag's value + // and every non-authored reference's BadNodeIdUnknown marker would otherwise live forever. A + // still-authored tag keeps its value, so a redeploy never blanks what has been observed. + Values.RetainOnly(table.ByRawPath.ContainsKey); + // Drop state belonging to edge nodes this deploy removed. Without this a tracker (and its // rebirth debounce slot) for a node nobody reads survives every future redeploy. foreach (var key in _trackers.Keys) diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/TopicFilterTrie.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/TopicFilterTrie.cs new file mode 100644 index 00000000..976523f7 --- /dev/null +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/TopicFilterTrie.cs @@ -0,0 +1,152 @@ +using System.Collections.Frozen; + +namespace ZB.MOM.WW.OtOpcUa.Driver.Mqtt; + +/// Receives each candidate a match produces. +/// The value type stored against each filter. +internal interface ITopicFilterVisitor +{ + /// Called once per value whose filter may match the topic. + /// The candidate. + void Visit(T value); +} + +/// +/// An immutable trie over MQTT topic FILTERS (+ / # wildcards), so matching one published +/// topic costs a walk over its levels instead of a comparer call per wildcard-authored tag (R06:M-6: +/// the linear scan ran on MQTTnet's shared dispatcher thread for every inbound message). +/// +/// +/// +/// A candidate generator, not the authority. The trie implements the plain MQTT 3.1.1 §4.7 +/// level rules — + is exactly one level (an empty one included), # is the parent level +/// and everything under it — and deliberately nothing subtler (the $-topic exclusion, shared +/// subscriptions). It can therefore only ever return a SUPERSET of the true matches, and every +/// caller confirms each candidate with MQTTnet's own MqttTopicFilterComparer, the comparer a +/// broker applies. Semantics stay byte-identical to the scan it replaces; only the work changes. +/// A differential test pins both halves against that comparer. +/// +/// +/// Allocation-free per match. Levels are walked as spans and looked up through the +/// dictionary's span alternate lookup; candidates are pushed to a caller-supplied visitor (a +/// ref struct in production) rather than collected into a list. +/// +/// +/// The value type stored against each filter. +internal sealed class TopicFilterTrie +{ + private readonly Node _root; + + private TopicFilterTrie(Node root, int count) + { + _root = root; + Count = count; + } + + /// An empty trie. + public static TopicFilterTrie Empty { get; } = Build([]); + + /// How many values were inserted. + public int Count { get; } + + /// Builds a trie from (filter, value) pairs. + /// The filters and their values. + /// The immutable trie. + public static TopicFilterTrie Build(IEnumerable<(string Filter, T Value)> entries) + { + var root = new Builder(); + var count = 0; + foreach (var (filter, value) in entries) + { + var node = root; + var levels = filter.Split('/'); + var terminated = false; + for (var i = 0; i < levels.Length; i++) + { + var level = levels[i]; + if (level == "#") + { + // '#' must be the last level; anything after it is a malformed filter the comparer + // will refuse anyway, so treating '#' as terminal here can only over-generate. + node.Hash.Add(value); + terminated = true; + break; + } + + node = level == "+" + ? node.Plus ??= new Builder() + : node.Children.TryGetValue(level, out var child) ? child : node.Children[level] = new Builder(); + } + + if (!terminated) node.Exact.Add(value); + count++; + } + + return new TopicFilterTrie(root.Freeze(), count); + } + + /// Pushes every value whose filter may match to . + /// The visitor; a ref struct is allowed. + /// A concrete published topic (never a filter). + /// Receives each candidate — a superset of the true matches; confirm each one. + public void Match(string topic, ref TVisitor visitor) + where TVisitor : ITopicFilterVisitor, allows ref struct + { + if (Count == 0 || topic is null) return; + Walk(_root, topic.AsSpan(), exhausted: false, ref visitor); + } + + private static void Walk(Node node, ReadOnlySpan rest, bool exhausted, ref TVisitor visitor) + where TVisitor : ITopicFilterVisitor, allows ref struct + { + // '#' here covers this level's parent-match AND every deeper level. + foreach (var value in node.Hash) visitor.Visit(value); + + if (exhausted) + { + foreach (var value in node.Exact) visitor.Visit(value); + return; + } + + var slash = rest.IndexOf('/'); + var level = slash < 0 ? rest : rest[..slash]; + var next = slash < 0 ? ReadOnlySpan.Empty : rest[(slash + 1)..]; + var nextExhausted = slash < 0; + + if (node.Children.Count > 0 + && node.Children.GetAlternateLookup>().TryGetValue(level, out var child)) + { + Walk(child, next, nextExhausted, ref visitor); + } + + if (node.Plus is { } plus) Walk(plus, next, nextExhausted, ref visitor); + } + + private sealed class Node(FrozenDictionary children, Node? plus, T[] exact, T[] hash) + { + public FrozenDictionary Children { get; } = children; + + public Node? Plus { get; } = plus; + + public T[] Exact { get; } = exact; + + public T[] Hash { get; } = hash; + } + + private sealed class Builder + { + public Dictionary Children { get; } = new(StringComparer.Ordinal); + + public Builder? Plus { get; set; } + + public List Exact { get; } = []; + + public List Hash { get; } = []; + + public Node Freeze() => new( + Children.ToFrozenDictionary(kv => kv.Key, kv => kv.Value.Freeze(), StringComparer.Ordinal), + Plus?.Freeze(), + [.. Exact], + [.. Hash]); + } +} diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Browser/SqlBrowseSession.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Browser/SqlBrowseSession.cs index f25aaff7..ceabf3a7 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Browser/SqlBrowseSession.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Browser/SqlBrowseSession.cs @@ -48,6 +48,12 @@ internal sealed class SqlBrowseSession : IBrowseSession /// Column alias carrying the SQL type family name. private const string DataTypeColumn = "DATA_TYPE"; + /// + /// Column alias carrying YES/NO nullability. Every shipped dialect projects it (Oracle + /// normalises its Y/N); it used to be read by nothing (R06:S-5). + /// + private const string IsNullableColumn = "IS_NULLABLE"; + /// The TABLE_TYPE value marking a view rather than a base table. private const string ViewTableType = "VIEW"; @@ -158,7 +164,7 @@ internal sealed class SqlBrowseSession : IBrowseSession reference.Schema, reference.Table!, cancellationToken).ConfigureAwait(false); // Computed from the columns ALREADY READ for this call — the catalog is not queried a second time. - var timestampColumn = PreferredTimestampColumn(columns); + var timestampColumn = PreferredTimestampColumn([.. columns.Select(c => (c.Name, c.DataType))]); // Ordinal: a column NodeId is only ever minted from this same catalog output, so its case is the // catalog's own. A case-insensitive match would pick the wrong column on a case-sensitive collation @@ -191,7 +197,18 @@ internal sealed class SqlBrowseSession : IBrowseSession Name: column.Name, DriverDataType: _dialect.MapColumnType(column.DataType).ToString(), IsArray: false, - SecurityClass: ReadOnlySecurityClass, + // ⚠ SecurityClass is (ab)used as the picker's descriptor line ("{type} · {SecurityClass}"), + // exactly as the MQTT browser does. Every consumer today is display-only; a future one that + // parses it as SecurityClassification must not read the Sql value (review m-6 — a dedicated + // descriptor field on AttributeInfo is the Commons follow-up). Display only. Nullability is + // what tells an operator a cell can come back NULL, which publishes Uncertain (or Bad under + // nullIsBad), before they commit the tag. + SecurityClass: column.IsNullable switch + { + true => ReadOnlySecurityClass + " · nullable", + false => ReadOnlySecurityClass + " · NOT NULL", + null => ReadOnlySecurityClass, + }, IsAlarm: false, AddressFields: addressFields), ]; @@ -356,7 +373,7 @@ internal sealed class SqlBrowseSession : IBrowseSession /// than an error: the catalog answering "no rows" is indistinguishable from a genuinely column-less /// relation, and neither is a reason to fail an operator's click. /// - private Task> ReadColumnsAsync( + private Task> ReadColumnsAsync( string schema, string table, CancellationToken ct) => QueryAsync( _dialect.ListColumnsSql, @@ -367,9 +384,30 @@ internal sealed class SqlBrowseSession : IBrowseSession }, static reader => ( Name: ReadString(reader, ColumnNameColumn), - DataType: ReadOptionalString(reader, DataTypeColumn) ?? string.Empty), + DataType: ReadOptionalString(reader, DataTypeColumn) ?? string.Empty, + IsNullable: ReadNullability(reader)), ct); + /// + /// Reads IS_NULLABLE as a tri-state: when a (custom) dialect does not + /// project it or the value is neither YES nor NO — "unknown" is said by omission, never + /// guessed. + /// + private static bool? ReadNullability(DbDataReader reader) + { + for (var i = 0; i < reader.FieldCount; i++) + { + if (!string.Equals(reader.GetName(i), IsNullableColumn, StringComparison.OrdinalIgnoreCase)) continue; + if (reader.IsDBNull(i)) return null; + var text = reader.GetValue(i)?.ToString()?.Trim(); + if (string.Equals(text, "YES", StringComparison.OrdinalIgnoreCase)) return true; + if (string.Equals(text, "NO", StringComparison.OrdinalIgnoreCase)) return false; + return null; + } + + return null; + } + /// /// Runs one catalog query under the gate and projects every row. The disposed check happens /// inside the gate wait so a dispose racing an in-flight call cannot be missed. diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Dialect/ISqlDialect.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Dialect/ISqlDialect.cs index c144ec62..e1069372 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Dialect/ISqlDialect.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Dialect/ISqlDialect.cs @@ -177,6 +177,31 @@ public interface ISqlDialect /// string SingleRowLimitSuffix { get; } + /// + /// The position's fragment for an -row limit — + /// T-SQL's "TOP (n) "; empty for a dialect that limits at the end. Same trailing-space rule. + /// + /// + /// Added for the key-value / where-pair row cap (R06:S-1): those plans used to carry no limit, so an + /// ambiguous selector against a history table materialised every matching row on every poll. The + /// planner asks for N + 1 rows — one more than the plan can use — so ambiguity stays observable + /// (the reader sees the extra row and publishes a configuration error) without unbounded reads. + /// + /// The row limit; at least 1. + /// The fragment, possibly empty. + /// is below 1. + string RowLimitPrefix(int rows); + + /// + /// The position's fragment for an -row limit — + /// " LIMIT n" / " FETCH FIRST n ROWS ONLY"; empty for a dialect that limits after + /// SELECT. Same leading-space rule. See . + /// + /// The row limit; at least 1. + /// The fragment, possibly empty. + /// is below 1. + string RowLimitSuffix(int rows); + /// Catalog query listing schemas — the browser's RootAsync level. Takes no parameters. string ListSchemasSql { get; } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Dialect/OracleDialect.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Dialect/OracleDialect.cs index 2dc72a83..1789959f 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Dialect/OracleDialect.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Dialect/OracleDialect.cs @@ -124,6 +124,21 @@ public sealed class OracleDialect : ISqlDialect // statement shape, which this seam deliberately leaves with the planner. public string SingleRowLimitSuffix => " FETCH FIRST 1 ROWS ONLY"; + /// + public string RowLimitPrefix(int rows) + { + ArgumentOutOfRangeException.ThrowIfLessThan(rows, 1); + return string.Empty; + } + + /// + // The same 12c+ row-limiting clause as SingleRowLimitSuffix; valid after a WHERE with no ORDER BY. + public string RowLimitSuffix(int rows) + { + ArgumentOutOfRangeException.ThrowIfLessThan(rows, 1); + return " FETCH FIRST " + rows.ToString(System.Globalization.CultureInfo.InvariantCulture) + " ROWS ONLY"; + } + /// // Schemas visible to the connecting principal, from ALL_USERS (Oracle equates a schema with a // user), aliased to the TABLE_SCHEMA column name the catalog readers project by. diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Dialect/PostgresDialect.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Dialect/PostgresDialect.cs index b332d912..6809b328 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Dialect/PostgresDialect.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Dialect/PostgresDialect.cs @@ -86,6 +86,20 @@ public sealed class PostgresDialect : ISqlDialect /// public string SingleRowLimitSuffix => " LIMIT 1"; + /// + public string RowLimitPrefix(int rows) + { + ArgumentOutOfRangeException.ThrowIfLessThan(rows, 1); + return string.Empty; + } + + /// + public string RowLimitSuffix(int rows) + { + ArgumentOutOfRangeException.ThrowIfLessThan(rows, 1); + return " LIMIT " + rows.ToString(System.Globalization.CultureInfo.InvariantCulture); + } + // The schemas holding relations this connection can actually see, system schemas excluded. // // Sourced from information_schema.tables, not information_schema.schemata - a deliberate diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Dialect/SqlServerDialect.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Dialect/SqlServerDialect.cs index 9db9848c..61057407 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Dialect/SqlServerDialect.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Dialect/SqlServerDialect.cs @@ -87,6 +87,22 @@ public sealed class SqlServerDialect : ISqlDialect /// public string SingleRowLimitSuffix => string.Empty; + /// + // Parenthesised: TOP (n) is the documented modern form, and TOP n without parentheses is the + // backwards-compatibility spelling. + public string RowLimitPrefix(int rows) + { + ArgumentOutOfRangeException.ThrowIfLessThan(rows, 1); + return "TOP (" + rows.ToString(System.Globalization.CultureInfo.InvariantCulture) + ") "; + } + + /// + public string RowLimitSuffix(int rows) + { + ArgumentOutOfRangeException.ThrowIfLessThan(rows, 1); + return string.Empty; + } + /// public string ListSchemasSql => "SELECT DISTINCT TABLE_SCHEMA FROM INFORMATION_SCHEMA.TABLES ORDER BY TABLE_SCHEMA"; diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlDriver.cs index 59ae4956..ac10102c 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlDriver.cs @@ -28,7 +28,7 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Sql; /// manufacture an exception to force backoff. /// public sealed class SqlDriver - : IDriver, ITagDiscovery, IReadable, ISubscribable, IHostConnectivityProbe, ISkippedTagReporter, IAsyncDisposable + : IDriver, ITagDiscovery, IReadable, IBatchSubscribable, IHostConnectivityProbe, ISkippedTagReporter, IAsyncDisposable { /// Authored tags this driver could not build into its poll table. private readonly SkippedTagLedger _skippedTags = new(); @@ -156,6 +156,16 @@ public sealed class SqlDriver private DriverHealth _health = new(DriverState.Unknown, null, null); + /// Guards the lifecycle-generation bump against the generation-checked lifecycle health write. + private readonly object _healthLock = new(); + + /// + /// Bumped by every and . A lifecycle write from + /// a superseded call, and a poll that STARTED under an older generation (a stale poll outliving the + /// engine's 5 s teardown wait, R06:S-9), can no longer overwrite the newer state. + /// + private long _lifecycleGeneration; + /// /// The exception instance has already turned into a health verdict, held only /// until — which the poll engine calls with that same object — has @@ -173,6 +183,9 @@ public sealed class SqlDriver /// Occurs when a subscribed tag's value or quality changes. public event EventHandler? OnDataChange; + /// Occurs once per poll cycle that produced at least one change, carrying every change of that cycle. + public event EventHandler? OnDataChangeBatch; + /// Occurs when the database endpoint transitions between reachable and unreachable. public event EventHandler? OnHostStatusChanged; @@ -230,6 +243,9 @@ public sealed class SqlDriver OnDataChange?.Invoke(this, new DataChangeEventArgs(handle, tagRef, snapshot)), onError: HandlePollError, backoffCap: PollBackoffCap, + // R02:P1 — one batch per poll burst, so DriverInstanceActor forwards one message per cycle instead + // of one per tag (it subscribes exactly one of the two events). + onBatch: (handle, items) => OnDataChangeBatch?.Invoke(this, new DataChangeBatchEventArgs(handle, items)), // The engine owns no Meter (Core.Abstractions takes no metrics dependency), so this // closure IS the production consumer of its overrun signal. onOverrun: _ => OtOpcUaTelemetry.PollOverruns.Add(1, @@ -261,6 +277,10 @@ public sealed class SqlDriver _factory = _explicitFactory ?? dialect.Factory; _connectionString = connectionString; Endpoint = DescribeEndpoint(connectionString); + + // The reader being replaced owns a SemaphoreSlim (R06:S-3). Disposing it is safe even with a stale + // poll still inside it: the gate goes only when the last in-flight group releases it. + var previous = _reader; _reader = new SqlPollReader( _factory, connectionString, @@ -271,6 +291,7 @@ public sealed class SqlDriver nullIsBad: options.NullIsBad, resolve: rawPath => _resolver.TryResolve(rawPath, out var definition) ? definition : null, logger: _logger); + previous?.Dispose(); } /// True when the supplied DriverConfig JSON carries a real body. The bootstrapper always passes @@ -347,20 +368,39 @@ public sealed class SqlDriver // identifier. public async Task InitializeAsync(string driverConfigJson, CancellationToken cancellationToken) { - WriteHealth(new DriverHealth(DriverState.Initializing, null, null)); + var generation = BeginLifecycle(); + // LastSuccessfulRead is PRESERVED across the lifecycle states — it is the watchdog's evidence clock. + WriteHealthIfCurrent(generation, new DriverHealth(DriverState.Initializing, ReadHealth().LastSuccessfulRead, null)); // adopt a changed config BEFORE the tag table is built, so the table and the connection it // will be polled over come from the same revision. ApplyBinding takes the options, dialect and // connection string as one unit; adopting options alone would poll a NEW tag set through the OLD // database, which is why this driver was excluded from the first pass. A null rebinder (direct // construction) or an empty/placeholder document keeps the constructor-supplied trio. - if (_rebind is not null && HasConfigBody(driverConfigJson)) + try { - var binding = _rebind(driverConfigJson); - ApplyBinding(binding.Options, binding.Dialect, binding.ConnectionString); - } + if (_rebind is not null && HasConfigBody(driverConfigJson)) + { + var binding = _rebind(driverConfigJson); + ApplyBinding(binding.Options, binding.Dialect, binding.ConnectionString); + } - BuildTagTable(); + BuildTagTable(); + } + catch (Exception ex) + { + // Review m-4: an unresolvable connectionStringRef or a malformed config threw here AFTER Initializing + // was written, and no poll may end Initializing — so the driver latched it with no LastError. Fault + // it like the liveness stage does: exception TYPE only on the operator surface (the factory's own + // messages are already sanitized, but this method cannot rely on that), full exception to the log. + var message = + $"Sql driver '{_driverInstanceId}' could not adopt its configuration: {ex.GetType().Name}. " + + "Check the connectionStringRef and the driver config JSON."; + _logger.LogError(ex, "Sql driver {DriverInstanceId} configuration could not be adopted.", _driverInstanceId); + if (WriteHealthIfCurrent(generation, new DriverHealth(DriverState.Faulted, ReadHealth().LastSuccessfulRead, message))) + TransitionHostTo(HostState.Stopped); + throw; + } try { @@ -369,7 +409,7 @@ public sealed class SqlDriver catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) { // The caller tore the initialization down; not a database verdict. - WriteHealth(new DriverHealth(DriverState.Unknown, null, null)); + WriteHealthIfCurrent(generation, new DriverHealth(DriverState.Unknown, ReadHealth().LastSuccessfulRead, null)); throw; } catch (Exception ex) @@ -385,8 +425,8 @@ public sealed class SqlDriver $"Check the database is reachable and the connection string is valid."; _logger.LogError(ex, "Sql driver {DriverInstanceId} liveness check against {Endpoint} failed.", _driverInstanceId, Endpoint); - WriteHealth(new DriverHealth(DriverState.Faulted, null, message)); - TransitionHostTo(HostState.Stopped); + if (WriteHealthIfCurrent(generation, new DriverHealth(DriverState.Faulted, ReadHealth().LastSuccessfulRead, message))) + TransitionHostTo(HostState.Stopped); throw new InvalidOperationException(message, ex); } @@ -399,7 +439,7 @@ public sealed class SqlDriver } catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) { - WriteHealth(new DriverHealth(DriverState.Unknown, null, null)); + WriteHealthIfCurrent(generation, new DriverHealth(DriverState.Unknown, ReadHealth().LastSuccessfulRead, null)); throw; } catch (Exception ex) @@ -415,13 +455,13 @@ public sealed class SqlDriver _logger.LogError(ex, "Sql driver {DriverInstanceId} catalog validation against {Endpoint} failed.", _driverInstanceId, Endpoint); - WriteHealth(new DriverHealth(DriverState.Faulted, null, message)); - TransitionHostTo(HostState.Stopped); + if (WriteHealthIfCurrent(generation, new DriverHealth(DriverState.Faulted, ReadHealth().LastSuccessfulRead, message))) + TransitionHostTo(HostState.Stopped); throw new InvalidOperationException(message, ex); } - WriteHealth(new DriverHealth(DriverState.Healthy, DateTime.UtcNow, null)); - TransitionHostTo(HostState.Running); + if (WriteHealthIfCurrent(generation, new DriverHealth(DriverState.Healthy, DateTime.UtcNow, null))) + TransitionHostTo(HostState.Running); _logger.LogInformation( "Sql driver {DriverInstanceId} initialized against {Endpoint} with {TagCount} authored tag(s).", _driverInstanceId, Endpoint, Tags.Count); @@ -437,9 +477,10 @@ public sealed class SqlDriver /// public async Task ShutdownAsync(CancellationToken cancellationToken) { + var generation = BeginLifecycle(); var lastRead = ReadHealth().LastSuccessfulRead; await TeardownAsync().ConfigureAwait(false); - WriteHealth(new DriverHealth(DriverState.Unknown, lastRead, null)); + WriteHealthIfCurrent(generation, new DriverHealth(DriverState.Unknown, lastRead, null)); } /// @@ -512,11 +553,37 @@ public sealed class SqlDriver // unreachable" and degrade a perfectly healthy driver. ArgumentNullException.ThrowIfNull(fullReferences); + // The generation this read started under: if a Shutdown/Initialize lands while it is in flight, its + // verdict describes a superseded configuration and must not touch the newer health (R06:S-9). + var generation = Interlocked.Read(ref _lifecycleGeneration); + return await ReadThroughAsync(_reader, fullReferences, generation, cancellationToken).ConfigureAwait(false); + } + + /// + /// 's body against a specific reader — the one the caller captured. Internal so a + /// test can drive the "reader replaced between capture and use" race deterministically. + /// + internal async Task> ReadThroughAsync( + SqlPollReader reader, IReadOnlyList fullReferences, long generation, CancellationToken cancellationToken) + { try { - var snapshots = await _reader.ReadAsync(fullReferences, cancellationToken).ConfigureAwait(false); - ObservePollOutcome(snapshots); - return snapshots; + var result = await reader.ReadDetailedAsync(fullReferences, cancellationToken).ConfigureAwait(false); + ObservePollOutcome(result.Snapshots, result.AnsweredRefusalGroups, generation); + return result.Snapshots; + } + catch (SqlPollReaderDisposedException ex) when (!cancellationToken.IsCancellationRequested) + { + // A rebind swapped the reader under this poll (review m-1: a redeploy re-initialises without + // teardown, so the engine keeps polling). Retry ONCE on the current reader; if there is no newer + // one (the driver itself was disposed), hand the engine an exception it is told not to classify — + // it used to Degrade the driver as "poll failed: OperationCanceledException". + var current = Volatile.Read(ref _reader); + if (!ReferenceEquals(current, reader)) + return await ReadThroughAsync(current, fullReferences, generation, cancellationToken).ConfigureAwait(false); + + Volatile.Write(ref _classifiedByRead, ex); + throw; } catch (OperationCanceledException) { @@ -530,8 +597,12 @@ public sealed class SqlDriver _driverInstanceId, Endpoint); // Type name only — never ex.Message (see InitializeAsync for why); the full exception is already // on the log sink via the LogWarning exception parameter above. - Degrade($"Sql driver '{_driverInstanceId}' could not reach {Endpoint}: {ex.GetType().Name}."); - TransitionHostTo(HostState.Stopped); + if (IsCurrentLifecycle(generation)) + { + Degrade($"Sql driver '{_driverInstanceId}' could not reach {Endpoint}: {ex.GetType().Name}."); + TransitionHostTo(HostState.Stopped); + } + // Mark THIS exception object as already classified before it goes back to the poll engine, which // routes it to HandlePollError next. The mark is what makes the deferral there exact — see that // method for why inferring it from the exception's type was not. @@ -603,9 +674,21 @@ public sealed class SqlDriver /// cadence rather than backed off from. /// /// The snapshots the reader returned for one poll. - internal void ObservePollOutcome(IReadOnlyList snapshots) + /// + /// Groups the database answered but the driver refused (an over-cap result set, R06:S-1). Their + /// members carry , which alone is not answer evidence + /// (the planner mints it before any I/O), so the count is what makes them count as the database + /// answering. + /// + /// + /// The lifecycle generation the poll started under; a superseded one is ignored outright. Null (tests) + /// means "current". + /// + internal void ObservePollOutcome( + IReadOnlyList snapshots, int answeredRefusals = 0, long? generation = null) { if (snapshots.Count == 0) return; + if (generation is { } started && !IsCurrentLifecycle(started)) return; var good = 0; var unreachable = 0; @@ -620,6 +703,8 @@ public sealed class SqlDriver if (IsAnswerEvidence(snapshot.StatusCode)) answered++; } + answered += answeredRefusals; + if (unreachable > 0) { Degrade( @@ -655,17 +740,12 @@ public sealed class SqlDriver if (answered == 0) return; // authoring-class Bad only — not a statement about the database. - if (good == 0) - { - // The database answered, just not with a value: an absent row, a NULL cell, a cell that would - // not coerce. Those are data/authoring facts, so the health STATE is deliberately left exactly - // as it was — but they are still proof the transport works, and the evidence clock the - // comms-loss watchdog reads must say so. - TouchLastSuccessfulRead(); - TransitionHostTo(HostState.Running); - return; - } + // Good values, OR a database that answered with refusals only — an absent row, a NULL cell, an + // uncoercible cell, an over-cap result set. A refusal is the database answering: it advances the + // evidence clock AND restores Healthy after a comms fault, exactly as a Good read does. Leaving the + // state as it was latched a recovered database Degraded behind every poll that read only refusals. + // // Route through the Faulted guard, not a raw WriteHealth: a poll still in flight when a concurrent // ReinitializeAsync has recorded Faulted (the engine's ~5 s teardown wait is shorter than the 15 s // default operationTimeout, so a stale poll can still complete afterwards) must not flip Faulted back @@ -792,7 +872,11 @@ public sealed class SqlDriver /// /// Builds the snapshot to publish from the current one. private void UpdateHealthUnlessFaulted(Func project) - => UpdateHealth(current => current.State == DriverState.Faulted ? null : project(current)); + // Initializing too: only the initialize's own final write may end it — a poll landing mid-init + // (a re-init without shutdown keeps the engine running) used to flip Initializing → Healthy. + => UpdateHealth(current => current.State is DriverState.Faulted or DriverState.Initializing + ? null + : project(current)); /// /// The one read-modify-write of the health field, as a compare-and-swap retry loop. @@ -823,13 +907,31 @@ public sealed class SqlDriver /// Barrier-protected read of the multi-threaded health field. private DriverHealth ReadHealth() => Volatile.Read(ref _health); + /// Starts a lifecycle operation and returns its generation; every later one supersedes it. + private long BeginLifecycle() + { + lock (_healthLock) return ++_lifecycleGeneration; + } + + /// Whether is still the newest lifecycle operation. + private bool IsCurrentLifecycle(long generation) => Interlocked.Read(ref _lifecycleGeneration) == generation; + + /// The current lifecycle generation. Test observation of the S-9 guard. + internal long LifecycleGenerationForTests => Interlocked.Read(ref _lifecycleGeneration); + /// - /// Unconditional publish of a new health snapshot, for the lifecycle transitions that are - /// last-writer-wins by definition (Initialize / Shutdown). Everything driven by a poll goes through - /// instead, because it has to read the current snapshot first. + /// A lifecycle write that applies only while is current — atomic against + /// , so a superseded initialize/shutdown never overwrites a newer one. /// - /// The health snapshot to publish. - private void WriteHealth(DriverHealth value) => Volatile.Write(ref _health, value); + private bool WriteHealthIfCurrent(long generation, DriverHealth value) + { + lock (_healthLock) + { + if (_lifecycleGeneration != generation) return false; + Volatile.Write(ref _health, value); + return true; + } + } /// Records a host-state change and raises — on transitions only. private void TransitionHostTo(HostState newState) @@ -867,10 +969,15 @@ public sealed class SqlDriver table[entry.RawPath] = definition; else { - blobSkips.Add(new SkippedTag(entry.RawPath, "TagConfig did not map to a definition")); + // The reason is the DEPLOY GATE's own explanation (R06:S-4) — SqlTagDefinitionFactory.Inspect + // asks this same parser and then says why — so /hosts names e.g. the deferred Query model + // instead of a generic "did not map" for a tag the gate already warned about. + var reason = SqlTagDefinitionFactory.Inspect(entry.TagConfig).FirstOrDefault() + ?? "TagConfig did not map to a definition"; + blobSkips.Add(new SkippedTag(entry.RawPath, reason)); _logger.LogWarning( - "Sql tag config did not map to a definition; skipping. Driver={DriverInstanceId} RawPath={RawPath}", - _driverInstanceId, entry.RawPath); + "Sql tag config did not map to a definition; skipping. Driver={DriverInstanceId} RawPath={RawPath} Reason={Reason}", + _driverInstanceId, entry.RawPath, reason); } } catch (Exception ex) @@ -1150,7 +1257,20 @@ public sealed class SqlDriver /// await using does not leak the poll loops. /// /// A task that represents the asynchronous operation. - public async ValueTask DisposeAsync() => await TeardownAsync().ConfigureAwait(false); + public async ValueTask DisposeAsync() + { + await TeardownAsync().ConfigureAwait(false); + + // Only on the FINAL dispose: a Shutdown → Initialize cycle on the same instance keeps polling + // through this reader when the init does not rebind (no rebinder / placeholder config). + _reader.Dispose(); + } + + /// Re-runs on the current binding — a real reader swap, for the m-1 race test. + internal void RebindForTests() => ApplyBinding(_options, _dialect, _connectionString); + + /// The live poll reader. Test observation of the S-3 lifecycle only. + internal SqlPollReader ReaderForTests => _reader; /// /// Shared teardown: stops every polled subscription and drops the authored table. Idempotent — safe diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlDriverFactoryExtensions.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlDriverFactoryExtensions.cs index a53e4390..d4f46fa8 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlDriverFactoryExtensions.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlDriverFactoryExtensions.cs @@ -194,8 +194,14 @@ public static class SqlDriverFactoryExtensions } catch (JsonException ex) { + // The parser's message AND the exception carrying it are withheld (R06:S-8), matching + // SqlDriverProbe and SqlDriverBrowser: System.Text.Json quotes the offending token, and a pasted + // connection-string literal is exactly the malformed text that lands here. This message becomes + // InitializeFailed -> health LastError -> /hosts. Line/byte position carry no content, so they stay. throw new InvalidOperationException( - $"Sql driver config for '{driverInstanceId}' is not valid JSON: {ex.Message}", ex); + $"Sql driver config for '{driverInstanceId}' is not valid JSON (line " + + $"{(ex.LineNumber is { } line ? line + 1 : (object)"?")}, byte {ex.BytePositionInLine?.ToString(System.Globalization.CultureInfo.InvariantCulture) ?? "?"}). The parser's message is withheld because the malformed text may " + + "carry a connection string."); } } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlGroupPlanner.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlGroupPlanner.cs index 9d1142b8..915eb89c 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlGroupPlanner.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlGroupPlanner.cs @@ -230,15 +230,23 @@ public static class SqlGroupPlanner values.Add(keyValue); } + // Row cap (R06:S-1): one row per distinct key is all this plan can use. It reads up to TWICE that, so a + // handful of stray duplicates stays inside the cap and is attributed PER KEY by the reader (only the + // duplicated key's tags go BadConfigurationError) — and asks for one row more than that, so only a + // gross excess (a history/events table, unattributable by construction because the rest is unread) + // fails the whole group. Memory stays O(keys) either way. + var rowCap = values.Count * 2; var sql = new StringBuilder("SELECT ") + .Append(dialect.RowLimitPrefix(rowCap + 1)) .Append(QuoteList(selected, dialect)) .Append(" FROM ").Append(QuoteQualifiedName(first.Table, dialect)) .Append(" WHERE ").Append(dialect.QuoteIdentifier(keyColumn)) .Append(" IN (").AppendJoin(", ", markers).Append(')') + .Append(dialect.RowLimitSuffix(rowCap + 1)) .ToString(); return new SqlQueryPlan( - SqlTagModel.KeyValue, groupKey, sql, markers, bindNames, values, members, selected); + SqlTagModel.KeyValue, groupKey, sql, markers, bindNames, values, members, selected, rowCap); } /// @@ -270,14 +278,18 @@ public static class SqlGroupPlanner // A where-pair wins over topByTimestamp; the parser already guarantees exactly one is populated. if (!Blank(first.RowSelectorColumn) && first.RowSelectorValue is not null) { + // Capped at TWO rows (R06:S-1): the selector must identify one row, and the second is the proof it + // does not. A TOP 1 alone would hide that ambiguity (see SqlPollReader.SelectWideRow); no cap at all + // materialised every matching row of an events table on every poll. var marker = dialect.ParameterMarker(RowSelectorBaseName); var sql = string.Concat( - "SELECT ", columns, " FROM ", table, - " WHERE ", dialect.QuoteIdentifier(first.RowSelectorColumn!), " = ", marker); + "SELECT ", dialect.RowLimitPrefix(2), columns, " FROM ", table, + " WHERE ", dialect.QuoteIdentifier(first.RowSelectorColumn!), " = ", marker, + dialect.RowLimitSuffix(2)); return new SqlQueryPlan( SqlTagModel.WideRow, groupKey, sql, [marker], [dialect.ParameterBindName(RowSelectorBaseName)], - [first.RowSelectorValue], members, selected); + [first.RowSelectorValue], members, selected, RowCap: 1); } if (!Blank(first.RowSelectorTopByTimestamp)) diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlPollReader.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlPollReader.cs index 04789c2d..0d3a96c5 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlPollReader.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlPollReader.cs @@ -1,5 +1,7 @@ +using System.Collections.Concurrent; using System.Data.Common; using System.Diagnostics; +using System.Runtime.CompilerServices; using System.Globalization; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; @@ -36,7 +38,7 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Sql; /// no subscription state and no connection between polls, so the driver shell can wrap it in /// and own those concerns. /// -public sealed class SqlPollReader +public sealed class SqlPollReader : IDisposable { /// Minimum spacing between "your source violates the query contract" warnings, per reader. private static readonly TimeSpan ContractWarningInterval = TimeSpan.FromMinutes(1); @@ -52,13 +54,41 @@ public sealed class SqlPollReader /// /// Caps concurrently executing group queries — and therefore concurrently held connections - /// (design §8.4). Never disposed: a only needs disposal once its - /// AvailableWaitHandle has been touched, and not disposing it also removes the hazard of a - /// timed-out-but-still-running group releasing into a disposed semaphore. + /// (design §8.4). Disposed by only once every group that ever acquired it has + /// released it (), so a timed-out-but-still-running group can never release into a + /// disposed semaphore (R06:S-3 — it used to be never disposed, one per reinitialize). /// private readonly SemaphoreSlim _gate; - private long _lastContractWarningTicks; + /// + /// Owner reference (1) plus one per in-flight group. drops the owner's; whoever + /// takes it to zero disposes . Zero is terminal — no group can start after it. + /// + private int _refs = 1; + + private int _disposed; + + + /// + /// Per-key last-warned instant for the per-poll warnings (R06:S-6): a frozen group's BadTimeout and a + /// failing group's query error used to log every poll, per group, indefinitely — 20 groups at 1 s was + /// 20 lines a second carrying SQL text. Keyed by the plan's SQL text, so bounded by the plan count. + /// + private readonly ConcurrentDictionary _lastWarnedTicks = new(StringComparer.Ordinal); + + /// Tags whose uncoercible cell has been warned about — once per tag per reader (the refusal rule). + private readonly ConcurrentDictionary _coerceWarned = new(StringComparer.Ordinal); + + /// + /// The resolved definitions, slot map and plans for each reference list a caller polls (R06:S-7). A + /// poll loop passes the SAME list instance every cycle, so it is planned once instead of every poll. + /// Weakly keyed: an unsubscribed list (and its entry) is collected with it. Invalidation is structural — + /// each entry remembers the definition INSTANCES it was built from, and any reference resolving to a + /// different instance (a catalog-gate swap) rebuilds it. + /// + private readonly ConditionalWeakTable, PollShape> _shapes = new(); + + private int _plansBuilt; /// Initializes a new instance of the class. /// @@ -154,26 +184,87 @@ public sealed class SqlPollReader /// The database could not be reached (opening a connection failed). public async Task> ReadAsync( IReadOnlyList fullReferences, CancellationToken cancellationToken) + => (await ReadDetailedAsync(fullReferences, cancellationToken).ConfigureAwait(false)).Snapshots; + + /// + /// plus what the snapshots alone cannot say: how many groups the database + /// ANSWERED with more rows than the plan can use (R06:S-1). Those groups publish + /// — the same code an un-plannable definition gets + /// before any I/O — so the driver needs this count to treat them as the answer evidence they are. + /// + /// The RawPaths to read. + /// The caller's token; see . + /// The snapshots and the answered-refusal count. + public async Task ReadDetailedAsync( + IReadOnlyList fullReferences, CancellationToken cancellationToken) { ArgumentNullException.ThrowIfNull(fullReferences); var results = new DataValueSnapshot[fullReferences.Count]; - if (results.Length == 0) return results; + if (results.Length == 0) return new SqlPollResult(results, 0); var readAt = DateTime.UtcNow; + var shape = GetShape(fullReferences); - // Resolve first. A miss is this tag's own problem (BadNodeIdUnknown) and must not reach the planner, - // whose members are the sole thing the slice-back walks. + // A miss is this tag's own problem (BadNodeIdUnknown) and never reaches the planner, whose members + // are the sole thing the slice-back walks. + for (var i = 0; i < shape.Resolved.Length; i++) + { + if (shape.Resolved[i] is null) + results[i] = new DataValueSnapshot(null, SqlStatusCodes.BadNodeIdUnknown, null, readAt); + } + + var answeredRefusals = shape.Definitions.Count > 0 + ? await ReadGroupsAsync(shape, results, readAt, cancellationToken).ConfigureAwait(false) + : 0; + + // Fail-safe. Nothing above should leave a hole, but "N in, N out" is a contract the caller indexes + // blind — a null here would be a NullReferenceException in the publish fan-out, far from its cause. + for (var i = 0; i < results.Length; i++) + results[i] ??= new DataValueSnapshot(null, SqlStatusCodes.BadInternalError, null, readAt); + + return new SqlPollResult(results, answeredRefusals); + } + + /// How many times a reference list has been (re)planned. Test observation of the S-7 cache. + internal int PlansBuiltForTests => Volatile.Read(ref _plansBuilt); + + /// + /// Returns the cached shape for , rebuilding it when any reference now + /// resolves to a different definition instance than the one it was built from. + /// + private PollShape GetShape(IReadOnlyList fullReferences) + { + if (_shapes.TryGetValue(fullReferences, out var cached) && cached.Resolved.Length == fullReferences.Count) + { + var current = true; + for (var i = 0; i < fullReferences.Count && current; i++) + { + var definition = fullReferences[i] is { } reference ? _resolve(reference) : null; + current = ReferenceEquals(definition, cached.Resolved[i]); + } + + if (current) return cached; + } + + var shape = BuildShape(fullReferences); + _shapes.AddOrUpdate(fullReferences, shape); + return shape; + } + + /// Resolves, slots and plans one reference list. + private PollShape BuildShape(IReadOnlyList fullReferences) + { + Interlocked.Increment(ref _plansBuilt); + + var resolved = new SqlTagDefinition?[fullReferences.Count]; var definitions = new List(fullReferences.Count); var slots = new Dictionary>(); for (var i = 0; i < fullReferences.Count; i++) { var definition = fullReferences[i] is { } reference ? _resolve(reference) : null; - if (definition is null) - { - results[i] = new DataValueSnapshot(null, SqlStatusCodes.BadNodeIdUnknown, null, readAt); - continue; - } + resolved[i] = definition; + if (definition is null) continue; definitions.Add(definition); if (!slots.TryGetValue(definition, out var positions)) @@ -181,45 +272,54 @@ public sealed class SqlPollReader positions.Add(i); } - if (definitions.Count > 0) - await ReadGroupsAsync(definitions, slots, results, readAt, cancellationToken).ConfigureAwait(false); + if (definitions.Count == 0) return new PollShape(resolved, definitions, slots, [], PlanningFailed: false); - // Fail-safe. Nothing above should leave a hole, but "N in, N out" is a contract the caller indexes - // blind — a null here would be a NullReferenceException in the publish fan-out, far from its cause. - for (var i = 0; i < results.Length; i++) - results[i] ??= new DataValueSnapshot(null, SqlStatusCodes.BadInternalError, null, readAt); - - return results; - } - - /// Plans the resolved definitions, runs every group concurrently, and slices the results back. - private async Task ReadGroupsAsync( - List definitions, - Dictionary> slots, - DataValueSnapshot[] results, - DateTime readAt, - CancellationToken cancellationToken) - { - IReadOnlyList plans; try { - plans = SqlGroupPlanner.Plan(definitions, _dialect); + return new PollShape(resolved, definitions, slots, SqlGroupPlanner.Plan(definitions, _dialect), PlanningFailed: false); } catch (Exception ex) when (ex is ArgumentException or NotSupportedException) { // A definition with a broken invariant — SqlEquipmentTagParser rejects all of these up front, so // reaching here means a definition was built past the parser. It is an authoring fault, not a // database fault: Bad-code the planned tags rather than throwing and triggering connection backoff. + // Logged once per (re)plan, which the cache makes once per shape rather than once per poll. _logger.LogError(ex, "Sql poll planning failed for {Count} tag(s); publishing BadConfigurationError.", definitions.Count); - foreach (var definition in definitions) + return new PollShape(resolved, definitions, slots, [], PlanningFailed: true); + } + } + + /// One reference list's resolved definitions, slot map and plans — immutable once built. + private sealed record PollShape( + SqlTagDefinition?[] Resolved, + List Definitions, + Dictionary> Slots, + IReadOnlyList Plans, + bool PlanningFailed); + + /// + /// Runs every planned group concurrently and slices the results back. Returns how many groups came + /// back with an answered refusal (over the row cap, or a duplicated key). + /// + private async Task ReadGroupsAsync( + PollShape shape, + DataValueSnapshot[] results, + DateTime readAt, + CancellationToken cancellationToken) + { + var slots = shape.Slots; + var plans = shape.Plans; + if (shape.PlanningFailed) + { + foreach (var definition in shape.Definitions) { if (!slots.TryGetValue(definition, out var positions)) continue; var snapshot = new DataValueSnapshot(null, SqlStatusCodes.BadConfigurationError, null, readAt); foreach (var position in positions) results[position] = snapshot; } - return; + return 0; } // Groups run concurrently; _gate — not the task count — is what bounds open connections. Each group @@ -233,8 +333,18 @@ public sealed class SqlPollReader // leaves a sibling group running against a connection nobody is watching. var outcomes = await Task.WhenAll(groups).ConfigureAwait(false); + // Surfaced AFTER WhenAll, as one typed exception: a group throwing it would turn WhenAll into a plain + // TaskCanceledException and the caller could no longer tell "superseded reader" from a caller cancel. + if (outcomes.Any(o => ReferenceEquals(o, GroupResult.ReaderDisposed))) throw new SqlPollReaderDisposedException(); + + var answeredRefusals = 0; for (var g = 0; g < plans.Count; g++) - Slice(plans[g], outcomes[g], slots, results, readAt); + { + var refusedSome = Slice(plans[g], outcomes[g], slots, results, readAt); + if (refusedSome || ReferenceEquals(outcomes[g], GroupResult.RowCapExceeded)) answeredRefusals++; + } + + return answeredRefusals; } /// @@ -273,6 +383,22 @@ public sealed class SqlPollReader /// inside the deadline, so a poll behind a wedged group still returns on time — as BadTimeout. /// private async Task RunGroupAsync(SqlQueryPlan plan, CancellationToken cancellationToken) + { + // A reader replaced by a reinitialize (and disposed) can still be holding a stale poll: it runs no + // new group. Cancellation is the honest answer — the poll that asked has been superseded. + if (!TryAddRef()) return GroupResult.ReaderDisposed; + + try + { + return await RunGroupCoreAsync(plan, cancellationToken).ConfigureAwait(false); + } + finally + { + ReleaseRef(); + } + } + + private async Task RunGroupCoreAsync(SqlQueryPlan plan, CancellationToken cancellationToken) { var clock = Stopwatch.StartNew(); @@ -288,6 +414,9 @@ public sealed class SqlPollReader if (budget <= TimeSpan.Zero) return GroupResult.TimedOut; deadline.CancelAfter(budget); + // The work holds its OWN reference: it can outlive this method (a timed-out group is abandoned + // still running) and its finally releases the slot into _gate, which must therefore still exist. + AddRefForWork(); work = Task.Run( async () => { @@ -299,6 +428,7 @@ public sealed class SqlPollReader { _gate.Release(); deadline.Dispose(); + ReleaseRef(); } }, CancellationToken.None); @@ -334,6 +464,41 @@ public sealed class SqlPollReader } } + /// + /// Releases the concurrency gate once no group still holds it. Idempotent; safe while groups are in + /// flight (the last one out disposes it). A disposed reader starts no new group. + /// + public void Dispose() + { + if (Interlocked.Exchange(ref _disposed, 1) == 0) ReleaseRef(); + } + + /// True once has actually been disposed. Test observation only. + internal bool GateDisposedForTests => Volatile.Read(ref _refs) == 0; + + /// Takes a reference unless the reader is already fully released. + private bool TryAddRef() + { + while (true) + { + // A disposed reader starts no new group even while older ones are still in flight (review m-2): + // the gate would survive until they finish, but a replaced reader must not take new work. + if (Volatile.Read(ref _disposed) != 0) return false; + var current = Volatile.Read(ref _refs); + if (current == 0) return false; + if (Interlocked.CompareExchange(ref _refs, current + 1, current) == current) return true; + } + } + + /// A further reference for work started under an already-held one (so never from zero). + private void AddRefForWork() => Interlocked.Increment(ref _refs); + + /// Drops a reference; the one that reaches zero disposes the gate. + private void ReleaseRef() + { + if (Interlocked.Decrement(ref _refs) == 0) _gate.Dispose(); + } + /// How much of this group's operationTimeout budget is left; never negative. private TimeSpan Remaining(Stopwatch clock) { @@ -346,11 +511,14 @@ public sealed class SqlPollReader { // The table is named for the same reason the contract warnings name it: under a partial outage this // line repeats every poll, and "which source is frozen" is the only question the operator has. - _logger.LogWarning( - "Sql poll group ({Model}, '{Table}', {Members} tag(s)) exceeded the {Timeout} ms operation " + - "timeout; publishing BadTimeout.", - plan.Model, plan.Members[0].Table, plan.Members.Count, - (int)_operationTimeout.TotalMilliseconds); + if (ShouldWarn("timeout:" + plan.SqlText)) + { + _logger.LogWarning( + "Sql poll group ({Model}, '{Table}', {Members} tag(s)) exceeded the {Timeout} ms operation " + + "timeout; publishing BadTimeout. Repeats for this group are suppressed for {Interval} s.", + plan.Model, plan.Members[0].Table, plan.Members.Count, + (int)_operationTimeout.TotalMilliseconds, (int)ContractWarningInterval.TotalSeconds); + } AbandonedTask.Detach(work); return GroupResult.TimedOut; @@ -412,9 +580,13 @@ public sealed class SqlPollReader { // The connection opened, so the server is up — this query is what failed (an identifier that // is not in the catalog, a lock, a permission). That is this group's problem alone. - _logger.LogWarning( - ex, "Sql poll group ({Model}, {Members} tag(s)) failed: {Sql}", - plan.Model, plan.Members.Count, plan.SqlText); + if (ShouldWarn("failed:" + plan.SqlText)) + { + _logger.LogWarning( + ex, "Sql poll group ({Model}, {Members} tag(s)) failed: {Sql}. Repeats for this group are " + + "suppressed for {Interval} s.", + plan.Model, plan.Members.Count, plan.SqlText, (int)ContractWarningInterval.TotalSeconds); + } return GroupResult.Failed; } } @@ -442,8 +614,11 @@ public sealed class SqlPollReader columnIndex[selected[c]] = c; } + // The client-side half of the row cap (R06:S-1): the statement already asks for RowCap + 1 rows, but a + // provider that cannot spell a limit must not turn that into an unbounded read either. + var limit = plan.RowCap is { } cap ? cap + 1 : int.MaxValue; var rows = new List(); - while (await reader.ReadAsync(cancellationToken).ConfigureAwait(false)) + while (rows.Count < limit && await reader.ReadAsync(cancellationToken).ConfigureAwait(false)) { var row = new object?[selected.Count]; for (var c = 0; c < selected.Count; c++) @@ -451,6 +626,27 @@ public sealed class SqlPollReader rows.Add(row); } + if (plan.RowCap is { } rowCap && rows.Count > rowCap) + { + // Per plan, not reader-wide: with two ambiguous groups the reader-wide limiter could name only the + // first table forever, and with health still Healthy this line is the operator's only pointer. + if (ShouldWarn("rowcap:" + plan.SqlText)) + { + var first = plan.Members[0]; + var selector = plan.Model == SqlTagModel.KeyValue + ? string.Concat(first.KeyColumn, " IN (", (rowCap / 2).ToString(CultureInfo.InvariantCulture), " key(s))") + : string.Concat(first.RowSelectorColumn, " = ", first.RowSelectorValue); + _logger.LogWarning( + "Sql poll source '{Table}' returned more rows than the {Model} selector '{Selector}' can attribute " + + "(at most {Cap}) in one poll, so the selector is ambiguous — a history/events table, or a " + + "where column that is not unique. Its {Members} tag(s) publish BadConfigurationError and " + + "nothing past row {Limit} is read. Point the tag at a current-values table or a latest-row view.", + first.Table, plan.Model, selector, rowCap, plan.Members.Count, rowCap + 1); + } + + return GroupResult.RowCapExceeded; + } + return GroupResult.Ok(rows, columnIndex, typeNames); } @@ -470,8 +666,11 @@ public sealed class SqlPollReader } } - /// Maps one group's outcome onto every slot its members occupy in the caller's list. - private void Slice( + /// + /// Maps one group's outcome onto every slot its members occupy in the caller's list. Returns whether any + /// member was refused on an ANSWERED result (a duplicated key) — answer evidence the driver must count. + /// + private bool Slice( SqlQueryPlan plan, GroupResult outcome, Dictionary> slots, @@ -481,9 +680,11 @@ public sealed class SqlPollReader // KeyValue indexes rows by the key column; WideRow's members all read the one selected row, so the // where-column is not even in the result set (design §5.3). Both selections happen ONCE per group — // they are group-wide facts, and doing them per member would report a contract violation N times. + HashSet? duplicatedKeys = null; var byKey = outcome.Succeeded && plan.Model == SqlTagModel.KeyValue - ? IndexByKey(plan, outcome) + ? IndexByKey(plan, outcome, out duplicatedKeys) : null; + var refusedSome = false; var wideRow = outcome.Succeeded && plan.Model == SqlTagModel.WideRow ? SelectWideRow(plan, outcome) : null; @@ -493,26 +694,45 @@ public sealed class SqlPollReader // Members may repeat (two tags on one keyValue, or the same ref twice) — every occurrence is fed. if (!slots.TryGetValue(member, out var positions)) continue; - var snapshot = outcome.Succeeded - ? MapMember(plan, member, outcome, byKey, wideRow, readAt) - : new DataValueSnapshot(null, outcome.FailureStatus, null, readAt); + DataValueSnapshot snapshot; + if (!outcome.Succeeded) + { + snapshot = new DataValueSnapshot(null, outcome.FailureStatus, null, readAt); + } + else if (duplicatedKeys is not null && member.KeyValue is { } key && duplicatedKeys.Contains(key)) + { + // More than one row for THIS tag's key: its selector is ambiguous — the same rule a where-pair + // follows — so it is a configuration error for this key's tags only. Every other key in the + // group is unaffected, whatever else is present or absent (R06:S-1 review I-1: it used to be + // last-row-wins Good, or the whole group Bad, depending on an unrelated key's absence). + snapshot = new DataValueSnapshot(null, SqlStatusCodes.BadConfigurationError, null, readAt); + refusedSome = true; + } + else + { + snapshot = MapMember(plan, member, outcome, byKey, wideRow, readAt); + } foreach (var position in positions) results[position] = snapshot; } + + return refusedSome; } /// - /// Builds the key → row index a key-value plan slices through. Later rows win, so a source that - /// breaks the one-row-per-key contract (design §3.6) still yields a deterministic value; the - /// violation is reported through a rate-limited warning rather than silently. + /// Builds the key → row index a key-value plan slices through, and the set of keys that came back more + /// than once — a source breaking the one-row-per-key contract (design §3.6). Those keys' tags publish + /// ; the violation is also reported through a + /// per-plan rate-limited warning. /// - private Dictionary IndexByKey(SqlQueryPlan plan, GroupResult outcome) + private Dictionary IndexByKey( + SqlQueryPlan plan, GroupResult outcome, out HashSet? duplicatedKeys) { + duplicatedKeys = null; var index = new Dictionary(StringComparer.Ordinal); if (plan.KeyColumn is null || !outcome.ColumnIndex.TryGetValue(plan.KeyColumn, out var keyAt)) return index; - var duplicated = 0; foreach (var row in outcome.Rows) { // A NULL key cell indexes nothing: Convert.ToString would fold it to "", which would then be @@ -523,20 +743,16 @@ public sealed class SqlPollReader // (an INTEGER station id, say) — the provider coerces on the way in, so match on the way out. var key = Convert.ToString(keyCell, CultureInfo.InvariantCulture); if (key is null) continue; - if (!index.TryAdd(key, row)) - { - index[key] = row; - duplicated++; - } + if (!index.TryAdd(key, row)) (duplicatedKeys ??= new HashSet(StringComparer.Ordinal)).Add(key); } - if (duplicated > 0 && ShouldWarnAboutContract()) + if (duplicatedKeys is not null && ShouldWarn("dupkey:" + plan.SqlText)) { _logger.LogWarning( - "Sql poll source '{Table}' returned more than one row for {Count} key(s) in one poll; the " + - "last row wins. The key-value model requires one row per key — point the tag at a " + - "current-values table or an operator-authored latest-per-key view.", - plan.Members[0].Table, duplicated); + "Sql poll source '{Table}' returned more than one row for {Count} key(s) in one poll (e.g. " + + "'{Example}'); those keys' tags publish BadConfigurationError. The key-value model requires one " + + "row per key — point the tag at a current-values table or an operator-authored latest-per-key view.", + plan.Members[0].Table, duplicatedKeys.Count, duplicatedKeys.First()); } return index; @@ -549,15 +765,14 @@ public sealed class SqlPollReader /// emits no ORDER BY, so across polls the "last" row is whatever the server returned last, /// i.e. storage order. That is why more than one row is a reported contract violation and not merely /// a tie to break — the model's premise (design §5.3) is that the selector identifies one row. - /// Why the where-pair form is deliberately NOT given the dialect's single-row limit. A - /// TOP 1 with no ORDER BY is not deterministic either — it returns whichever row the - /// plan happens to produce first, so it would trade one arbitrary row for a differently arbitrary - /// one. Worse, it would destroy the only evidence this method has that the selector is ambiguous - /// (Rows.Count > 1), converting a loud misconfiguration into a permanently silent one — - /// precisely the failure mode this class's summary calls its top risk. The topByTimestamp - /// form keeps its row limit because there the ORDER BY at least fixes which rows can - /// be first. The accepted cost is that an ambiguous selector materialises every matching row for one - /// poll; the warning is what gets it fixed. + /// Why the where-pair form is capped at TWO rows, not one (R06:S-1). A TOP 1 with no + /// ORDER BY is not deterministic — it returns whichever row the plan produces first — and it would + /// destroy the only evidence that the selector is ambiguous, converting a loud misconfiguration into a + /// permanently silent one. No limit at all materialised every matching row of an events table on every + /// poll. Two rows keeps the evidence and bounds the read: a second row makes the group publish + /// BadConfigurationError in MaterialiseAsync, so a where-pair group never reaches this + /// method with more than one row. The multi-row warning below is now reached only by a plan with no + /// cap. /// ⚠ The topByTimestamp answer is only defined when the ordering column is unique /// ORDER BY ts DESC plus a one-row limit over a column with ties returns an /// arbitrary row among the tied ones, and — unlike the where-pair form — this method can never @@ -572,7 +787,7 @@ public sealed class SqlPollReader private object?[]? SelectWideRow(SqlQueryPlan plan, GroupResult outcome) { if (outcome.Rows.Count == 0) return null; - if (outcome.Rows.Count > 1 && ShouldWarnAboutContract()) + if (outcome.Rows.Count > 1 && ShouldWarn("multirow:" + plan.SqlText)) { var first = plan.Members[0]; var selector = string.IsNullOrWhiteSpace(first.RowSelectorColumn) @@ -642,9 +857,14 @@ public sealed class SqlPollReader var target = member.DeclaredType ?? _dialect.MapColumnType(outcome.TypeNames[valueAt]); if (!TryCoerce(cell, target, out var value)) { - _logger.LogWarning( - "Sql tag '{Tag}' could not coerce a {Actual} cell to its {Declared} type.", - member.Name, cell.GetType().Name, target); + // A per-tag refusal: warned ONCE per tag (per reader), never per poll (R06:S-6). + if (_coerceWarned.TryAdd(member.Name, 0)) + { + _logger.LogWarning( + "Sql tag '{Tag}' could not coerce a {Actual} cell to its {Declared} type; it publishes " + + "BadTypeMismatch (warned once per tag).", + member.Name, cell.GetType().Name, target); + } return new DataValueSnapshot(null, SqlStatusCodes.BadTypeMismatch, sourceTimestamp, readAt); } @@ -711,13 +931,21 @@ public sealed class SqlPollReader Convert.ToDateTime(cell, CultureInfo.InvariantCulture), DateTimeKind.Utc), }; - /// Rate-limits the source-contract warning so a misconfigured table cannot flood the log. - private bool ShouldWarnAboutContract() + /// Rate-limits one per-poll warning by key; true at most once per . + private bool ShouldWarn(string key) { var now = Stopwatch.GetTimestamp(); - var last = Interlocked.Read(ref _lastContractWarningTicks); - if (last != 0 && Stopwatch.GetElapsedTime(last, now) < ContractWarningInterval) return false; - return Interlocked.CompareExchange(ref _lastContractWarningTicks, now, last) == last; + while (true) + { + if (!_lastWarnedTicks.TryGetValue(key, out var last)) + { + if (_lastWarnedTicks.TryAdd(key, now)) return true; + continue; + } + + if (Stopwatch.GetElapsedTime(last, now) < ContractWarningInterval) return false; + if (_lastWarnedTicks.TryUpdate(key, now, last)) return true; + } } /// One group's outcome: either a materialised result set, or the status its members inherit. @@ -747,6 +975,17 @@ public sealed class SqlPollReader public static GroupResult Failed { get; } = new(SqlStatusCodes.BadCommunicationError, NoRows, NoColumns, []); + /// The reader was disposed before this group could start; surfaced as a typed exception. + public static GroupResult ReaderDisposed { get; } = + new(SqlStatusCodes.BadInternalError, NoRows, NoColumns, []); + + /// + /// The database answered with more rows than the plan can use — an ambiguous selector (R06:S-1). + /// A configuration fact, not a database fault; the driver counts it as answer evidence. + /// + public static GroupResult RowCapExceeded { get; } = + new(SqlStatusCodes.BadConfigurationError, NoRows, NoColumns, []); + /// The status every member inherits when is false. public uint FailureStatus { get; } @@ -775,6 +1014,21 @@ public sealed class SqlPollReader } } +/// +/// A poll reached a after it was disposed — a rebind replaced it, or the driver +/// was disposed. Not a database verdict: the driver retries on the current reader, and never degrades for it. +/// +public sealed class SqlPollReaderDisposedException() + : OperationCanceledException("The Sql poll reader was disposed (replaced by a rebind, or the driver was disposed)."); + +/// One poll's snapshots plus the answer evidence they cannot carry themselves. +/// One snapshot per requested reference, at the same index. +/// +/// Groups the database ANSWERED but whose result the reader refused in whole or part — an over-cap result +/// set, or a duplicated key. Those members carry . +/// +public sealed record SqlPollResult(IReadOnlyList Snapshots, int AnsweredRefusalGroups); + /// /// The OPC UA status codes the Sql driver publishes, and the quality-class predicates its tests read /// them back with. diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlQueryPlan.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlQueryPlan.cs index 8e20c989..ec32c5d1 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlQueryPlan.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlQueryPlan.cs @@ -55,6 +55,15 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Sql; /// The bare (unquoted) column names in 's SELECT list, in order and distinct — the /// reader's map from result-set ordinal to source column. /// +/// +/// The most rows the reader will attribute before refusing the whole group — twice the distinct key count +/// (key-value: headroom so stray duplicates are attributed per key) or 1 (a where-pair wide row: one +/// selector, so any second row is the group's ambiguity); when the statement +/// already limits itself (topByTimestamp). asks for RowCap + 1, and the +/// reader stops there: receiving the extra row means a history/events table or a non-unique where column, +/// and the group publishes BadConfigurationError instead of materialising every matching row on +/// every poll (R06:S-1). +/// public sealed record SqlQueryPlan( SqlTagModel Model, object GroupKey, @@ -63,7 +72,8 @@ public sealed record SqlQueryPlan( IReadOnlyList ParameterBindNames, IReadOnlyList Parameters, IReadOnlyList Members, - IReadOnlyList SelectedColumns) + IReadOnlyList SelectedColumns, + int? RowCap = null) { // A plan is documented as safe to cache and reuse across polls, so it must not stay aliased to the // planner's mutable working lists — an IReadOnlyList holding a List is downcastable, and a diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MiniBroker.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MiniBroker.cs index 7950a240..5cb47e49 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MiniBroker.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MiniBroker.cs @@ -5,7 +5,8 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests; /// /// The smallest broker that can complete an MQTT 3.1.1 handshake: accept TCP, answer CONNECT with -/// a CONNACK, answer PINGREQ with PINGRESP, and nothing else. That is enough for MQTTnet to report +/// a CONNACK, answer PINGREQ with PINGRESP, SUBSCRIBE with a SUBACK (unless told not to), and record +/// PUBLISHes (PUBACKing QoS 1 only when told to). It never forwards a PUBLISH to a subscriber. That is enough for MQTTnet to report /// a real connection — which is what makes a real drop () drive a real /// DisconnectedAsync and a real reconnect, rather than a test seam standing in for one. /// @@ -59,6 +60,144 @@ internal sealed class MiniBroker : IDisposable /// public byte ConnAckReturnCode { get; set; } = ReturnCodeAccepted; + /// + /// When true (the default) a SUBSCRIBE is answered with a SUBACK granting each filter's requested + /// QoS. False is a broker that accepts SUBSCRIBE and never answers — the subscribe-deadline shape. + /// + public bool AnswerSubscribe { get; set; } = true; + + /// + /// Builds the SUBACK for one SUBSCRIBE packet read whole into , or null when + /// the packet is not parseable from one read (the fake handles one packet per read, like CONNECT). + /// + private static byte[]? TryBuildSubAck(byte[] buffer, int read) + { + // Fixed header: type byte, then the variable-length "remaining length". + var at = 1; + var remaining = 0; + var multiplier = 1; + byte digit; + do + { + if (at >= read) return null; + digit = buffer[at++]; + remaining += (digit & 0x7F) * multiplier; + multiplier *= 128; + } + while ((digit & 0x80) != 0); + + var end = at + remaining; + if (end > read || remaining < 2) return null; + + var packetIdHigh = buffer[at]; + var packetIdLow = buffer[at + 1]; + at += 2; + + var grants = new List(); + while (at + 2 <= end) + { + var length = (buffer[at] << 8) | buffer[at + 1]; + at += 2 + length; + if (at >= end) return null; + grants.Add((byte)(buffer[at] & 0x03)); + at++; + } + + var ack = new List { 0x90, (byte)(2 + grants.Count), packetIdHigh, packetIdLow }; + ack.AddRange(grants); + return [.. ack]; + } + + /// + /// When true, a QoS 1 PUBLISH is answered with a PUBACK. False (the default) is a broker that takes + /// the PUBLISH and never acknowledges it — the stalled-broker shape a write deadline exists for. + /// + public bool AckPublish { get; set; } + + /// Every PUBLISH received, in arrival order, as (topic, payload). + public System.Collections.Concurrent.ConcurrentQueue<(string Topic, byte[] Payload)> Published { get; } = new(); + + /// Handles one complete packet; returns false when the connection must close. + private async Task HandlePacketAsync(NetworkStream stream, byte[] packet) + { + var packetType = packet[0] & 0xF0; + if (packetType == 0x10) + { + await stream.WriteAsync(new byte[] { 0x20, 0x02, 0x00, ConnAckReturnCode }, _cts.Token); + + // A real broker closes the socket straight after refusing, which is what drives + // MQTTnet's DisconnectedAsync. Mirroring it keeps the fake honest. + return ConnAckReturnCode == ReturnCodeAccepted; + } + + if (packetType == 0xC0) + { + await stream.WriteAsync(PingResp, _cts.Token); + } + else if (packetType == 0x80 && AnswerSubscribe && TryBuildSubAck(packet, packet.Length) is { } subAck) + { + await stream.WriteAsync(subAck, _cts.Token); + } + else if (packetType == 0x30) + { + var qos = (packet[0] >> 1) & 0x03; + var at = HeaderLength(packet); + var topicLength = (packet[at] << 8) | packet[at + 1]; + var topic = System.Text.Encoding.UTF8.GetString(packet, at + 2, topicLength); + at += 2 + topicLength; + byte idHigh = 0, idLow = 0; + if (qos > 0) + { + idHigh = packet[at]; + idLow = packet[at + 1]; + at += 2; + } + + Published.Enqueue((topic, packet[at..])); + if (qos == 1 && AckPublish) + { + await stream.WriteAsync(new byte[] { 0x40, 0x02, idHigh, idLow }, _cts.Token); + } + } + + return true; + } + + /// Length of a packet's fixed header (type byte + variable-length remaining length). + private static int HeaderLength(byte[] packet) + { + var at = 1; + while ((packet[at] & 0x80) != 0) at++; + return at + 1; + } + + /// Removes one complete packet from the front of , if one is there. + private static bool TryTakePacket(List pending, out byte[] packet) + { + packet = []; + if (pending.Count < 2) return false; + + var at = 1; + var remaining = 0; + var multiplier = 1; + byte digit; + do + { + if (at >= pending.Count) return false; + digit = pending[at++]; + remaining += (digit & 0x7F) * multiplier; + multiplier *= 128; + } + while ((digit & 0x80) != 0); + + var total = at + remaining; + if (pending.Count < total) return false; + + packet = pending.GetRange(0, total).ToArray(); + pending.RemoveRange(0, total); + return true; + } + /// Total TCP connections accepted since construction — counts reconnect attempts. public int AcceptedCount => Volatile.Read(ref _acceptedCount); @@ -130,7 +269,8 @@ internal sealed class MiniBroker : IDisposable private async Task ServeAsync(TcpClient client) { - var buffer = new byte[1024]; + var buffer = new byte[4096]; + var pending = new List(); try { var stream = client.GetStream(); @@ -147,21 +287,22 @@ internal sealed class MiniBroker : IDisposable continue; } - var packetType = buffer[0] & 0xF0; - if (packetType == 0x10) + // Frame properly: several packets can share one TCP read (concurrent publishes do), and + // one packet can span two. + pending.AddRange(buffer.AsSpan(0, read).ToArray()); + var close = false; + while (TryTakePacket(pending, out var packet)) { - await stream.WriteAsync(new byte[] { 0x20, 0x02, 0x00, ConnAckReturnCode }, _cts.Token); - - // A real broker closes the socket straight after refusing, which is what drives - // MQTTnet's DisconnectedAsync. Mirroring it keeps the fake honest. - if (ConnAckReturnCode != ReturnCodeAccepted) + if (!await HandlePacketAsync(stream, packet)) { + close = true; break; } } - else if (packetType == 0xC0) + + if (close) { - await stream.WriteAsync(PingResp, _cts.Token); + break; } } } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttBirthObservationWindowTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttBirthObservationWindowTests.cs new file mode 100644 index 00000000..3132d3b2 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttBirthObservationWindowTests.cs @@ -0,0 +1,160 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Browser; +using SparkplugDataType = Org.Eclipse.Tahu.Protobuf.DataType; + +namespace ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests; + +/// +/// R06:M-4. sparkplug.birthObservationWindowSeconds was authored, range-checked and +/// persisted by the AdminUI and consumed by nothing. It now bounds how long the Sparkplug browse surface +/// collects NBIRTH/DBIRTH traffic before calling the observed set stable: the open budget grows by it, +/// the open lingers while births are still arriving, and an operator rebirth request waits (bounded) for +/// the births it asked for, so the tree the picker re-renders already carries them. +/// +public sealed class MqttBirthObservationWindowTests +{ + private static MqttDriverOptions Sparkplug(int windowSeconds, int connectTimeoutSeconds = 10) => new() + { + Mode = MqttMode.SparkplugB, + ConnectTimeoutSeconds = connectTimeoutSeconds, + Sparkplug = new MqttSparkplugOptions { GroupId = "Plant1", BirthObservationWindowSeconds = windowSeconds }, + }; + + [Fact] + public void OpenBudget_Sparkplug_AddsTheBirthObservationWindow() + { + MqttDriverBrowser.OpenBudget(Sparkplug(windowSeconds: 15)).ShouldBe(TimeSpan.FromSeconds(10 + 15)); + MqttDriverBrowser.OpenBudget(Sparkplug(windowSeconds: 40)).ShouldBe(TimeSpan.FromSeconds(10 + 40)); + } + + [Fact] + public void OpenBudget_Plain_IgnoresTheSparkplugWindow() + { + var plain = Sparkplug(windowSeconds: 40) with { Mode = MqttMode.Plain }; + MqttDriverBrowser.OpenBudget(plain).ShouldBe(TimeSpan.FromSeconds(10)); + } + + [Theory] + [InlineData(0, 1)] + [InlineData(-5, 1)] + [InlineData(15, 15)] + [InlineData(100_000, 120)] + public void BirthWindow_IsClamped(int configured, int expectedSeconds) + => MqttDriverBrowser.BirthWindow(Sparkplug(configured)).ShouldBe(TimeSpan.FromSeconds(expectedSeconds)); + + /// + /// Review minor 4: the rebirth wait is capped below the AdminUI's 20 s per-call timeout, so a long + /// configured window cannot hold the button past it (and a short one still governs). + /// + [Theory] + [InlineData(120, 15)] + [InlineData(15, 15)] + [InlineData(5, 5)] + public async Task RebirthWait_IsTheWindowCappedAt15Seconds(int windowSeconds, int expectedSeconds) + { + await using var s = new MqttBrowseSession( + MqttMode.SparkplugB, client: null, logger: null, birthWindow: TimeSpan.FromSeconds(windowSeconds)); + s.RebirthWait.ShouldBe(TimeSpan.FromSeconds(expectedSeconds)); + } + + /// An idle plant costs one quiet interval, not the whole window. + [Fact] + public async Task Settle_WithNoBirths_ReturnsAfterOneQuietInterval() + { + var reads = 0; + await MqttBrowseSession.SettleBirthsAsync( + () => { reads++; return 0; }, + baseline: 0, + awaitFirstBirth: false, + quiet: TimeSpan.FromMilliseconds(20), + window: TimeSpan.FromMinutes(5), + TestContext.Current.CancellationToken) + .WaitAsync(TimeSpan.FromSeconds(10), TestContext.Current.CancellationToken); + + reads.ShouldBe(1); + } + + /// + /// While births keep arriving the settle keeps collecting; it returns on the first quiet interval + /// after the last one. Counted in reads, not wall-clock, so it cannot flake. + /// + [Fact] + public async Task Settle_KeepsCollectingWhileBirthsArrive_ThenReturnsOnQuiet() + { + var reads = 0; + long births = 0; + await MqttBrowseSession.SettleBirthsAsync( + () => + { + reads++; + if (reads <= 5) births++; // five intervals with fresh births, then silence + return births; + }, + baseline: 0, + awaitFirstBirth: false, + quiet: TimeSpan.FromMilliseconds(20), + window: TimeSpan.FromMinutes(5), + TestContext.Current.CancellationToken) + .WaitAsync(TimeSpan.FromSeconds(10), TestContext.Current.CancellationToken); + + reads.ShouldBe(6); + } + + /// The window is a hard ceiling: a birth flood that never goes quiet still ends. + [Trait("Category", "SlowUnit")] + [Fact] + public async Task Settle_NeverQuiet_EndsAtTheWindow() + { + long births = 0; + await MqttBrowseSession.SettleBirthsAsync( + () => ++births, + baseline: 0, + awaitFirstBirth: false, + quiet: TimeSpan.FromMilliseconds(20), + window: TimeSpan.FromSeconds(1), + TestContext.Current.CancellationToken) + .WaitAsync(TimeSpan.FromSeconds(10), TestContext.Current.CancellationToken); + + births.ShouldBeGreaterThan(1); + } + + /// + /// A rebirth request waits for the birth it asked for. The fixed version cannot complete + /// synchronously (it awaits the settle), the unfixed one completes before the call returns — so the + /// IsCompleted check is deterministic, not a timing race. The birth then releases it. + /// + [Fact] + public async Task RequestRebirth_WaitsForTheRequestedBirths_ThenReturns() + { + await using var s = new MqttBrowseSession( + MqttMode.SparkplugB, client: null, logger: null, birthWindow: TimeSpan.FromMinutes(5)); + + var request = s.RequestRebirthAsync("Plant1/EdgeA", TestContext.Current.CancellationToken); + request.IsCompleted.ShouldBeFalse("the request must wait for the births it asked for"); + + s.ObserveBirthForTest("Plant1", "EdgeA", null, "Uptime", SparkplugDataType.Int64); + + (await request.WaitAsync(TimeSpan.FromSeconds(10), TestContext.Current.CancellationToken)).ShouldBe(1); + s.PublishCountForTest.ShouldBe(1); + } + + /// + /// The wait is a courtesy, never a failure: the NCMD was already published, so a caller deadline + /// during the settle returns the published count instead of reporting an error for a request that + /// succeeded. + /// + [Fact] + public async Task RequestRebirth_CallerCancelsDuringTheSettle_StillReportsThePublish() + { + await using var s = new MqttBrowseSession( + MqttMode.SparkplugB, client: null, logger: null, birthWindow: TimeSpan.FromMinutes(5)); + using var cts = CancellationTokenSource.CreateLinkedTokenSource(TestContext.Current.CancellationToken); + + var request = s.RequestRebirthAsync("Plant1/EdgeA", cts.Token); + request.IsCompleted.ShouldBeFalse(); + await cts.CancelAsync(); + + (await request.WaitAsync(TimeSpan.FromSeconds(10), TestContext.Current.CancellationToken)).ShouldBe(1); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttBrowseSessionTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttBrowseSessionTests.cs index bbc14bc1..fd624dde 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttBrowseSessionTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttBrowseSessionTests.cs @@ -779,6 +779,13 @@ public sealed class MqttBrowseSessionTests "Plant1/EdgeA::Profile", TestContext.Current.CancellationToken)).ShouldHaveSingleItem(); a.DriverDataType.ShouldBe(nameof(Core.Abstractions.DriverDataType.Int32)); // element type a.IsArray.ShouldBeTrue(); + + // R06:M-5: the ingestor refuses array metrics ("not supported in v1"), so the browser must not + // offer one as bindable — no stated address means the browse-commit refuses it in words, instead + // of committing a tag that sits at BadWaitingForInitialData forever. The scalar control is + // SparkplugAttributes_StateTheBindingTuple_SoTheCommitNeverParsesTheNodeId. + a.AddressFields.ShouldBeNull(); + a.SecurityClass.ShouldContain("not bindable"); } [Fact] @@ -796,6 +803,7 @@ public sealed class MqttBrowseSessionTests Enum.TryParse(a.DriverDataType, ignoreCase: true, out _) .ShouldBeFalse(); // not a guessed type — unmappable, and visibly so a.SecurityClass.ShouldContain("unsupported"); + a.AddressFields.ShouldBeNull(); // R06:M-5 — labelled AND unbindable, not just labelled } [Fact] diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttDriverSessionLifecycleTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttDriverSessionLifecycleTests.cs index ca4e667e..8abf72aa 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttDriverSessionLifecycleTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttDriverSessionLifecycleTests.cs @@ -13,9 +13,9 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests; /// ( / ). /// /// -/// Plain mode deliberately: it is the only mode whose initialize needs no SUBSCRIBE, and -/// answers CONNECT and PINGREQ and nothing else — so a Sparkplug driver -/// would park on its group SUBSCRIBE until the deadline and prove nothing about connections. +/// Plain mode deliberately: it is the only mode whose initialize needs no SUBSCRIBE. +/// answers CONNECT, SUBSCRIBE (unless told not to) and PINGREQ, and never +/// PUBLISHes, so ingest is driven through Subscriptions.HandleMessage. /// [Trait("Category", "Unit")] public sealed class MqttDriverSessionLifecycleTests @@ -122,7 +122,7 @@ public sealed class MqttDriverSessionLifecycleTests /// /// /// Ingest is fed through Subscriptions.HandleMessage, the method MQTTnet's dispatcher - /// calls, because answers CONNECT and PINGREQ and nothing else: it + /// calls, because answers CONNECT, SUBSCRIBE and PINGREQ only: it /// can drop a session (which is what this test needs from it) but cannot PUBLISH. /// /// @@ -174,6 +174,266 @@ public sealed class MqttDriverSessionLifecycleTests recovered.Value.ShouldBe(21.5); } + /// + /// R06:M-1, end to end. A SUBSCRIBE the broker never answers used to come back from + /// as a live handle while the topic left every retry set, so + /// DriverInstanceActor never re-subscribed and the tag stayed dark behind a Healthy connection. + /// It must throw (the actor's reconcile then retries), and the retry against a broker that answers + /// must establish and deliver — the positive control. + /// + [Trait("Category", "SlowUnit")] + [Fact] + public async Task ASubscribeTheBrokerNeverAnswers_Throws_AndTheRetryEstablishes() + { + const string rawPath = "Plant/Mqtt/dev1/Temp"; + const string topic = "factory/oven/temp"; + + using var broker = new MiniBroker { AnswerSubscribe = false }; + var options = BrokerOptions(broker.Port) with + { + RawTags = + [ + new RawTagEntry( + rawPath, + $$"""{"topic":"{{topic}}","payloadFormat":"Scalar","dataType":"Float64"}""", + WriteIdempotent: false), + ], + }; + await using var driver = new MqttDriver(options, "d", null); + await driver.InitializeAsync(Json(options), TestContext.Current.CancellationToken); + + await Should.ThrowAsync(() => + driver.SubscribeAsync([rawPath], TimeSpan.Zero, TestContext.Current.CancellationToken)); + driver.GetHealth().State.ShouldBe(DriverState.Healthy, "the connection itself is fine — that was the trap"); + + broker.AnswerSubscribe = true; + var handle = await driver.SubscribeAsync([rawPath], TimeSpan.Zero, TestContext.Current.CancellationToken); + + var raised = new List(); + driver.OnDataChange += (_, e) => { lock (raised) raised.Add(e); }; + driver.Subscriptions.HandleMessage(topic, "7"u8.ToArray(), retained: false); + lock (raised) + { + raised.ShouldHaveSingleItem().SubscriptionHandle.ShouldBe(handle); + raised[0].Snapshot.StatusCode.ShouldBe(0u); + } + } + + /// + /// R06:M-3. A refused initial connect used to rethrow without disposing the + /// (and its IMqttClient) it had just built. Control: a + /// successful initialize leaves its connection live, so the seam is not reporting "disposed" for + /// everything. + /// + [Fact] + public async Task InitializeAsync_RefusedConnect_DisposesTheConnectionItBuilt() + { + using var broker = new MiniBroker { ConnAckReturnCode = MiniBroker.ReturnCodeNotAuthorized }; + var options = BrokerOptions(broker.Port); + await using var driver = new MqttDriver(options, "d", null); + var built = new List(); + driver.ConnectionCreatedForTests = c => built.Add(c); + + await Should.ThrowAsync(() => driver.InitializeAsync(Json(options), TestContext.Current.CancellationToken)); + + built.ShouldHaveSingleItem().State.ShouldBe(MqttConnectionState.Disposed); + + broker.ConnAckReturnCode = MiniBroker.ReturnCodeAccepted; + await driver.InitializeAsync(Json(options), TestContext.Current.CancellationToken); + built.Count.ShouldBe(2); + built[1].State.ShouldBe(MqttConnectionState.Connected); + } + + /// + /// R06:M-10 + the lifecycle-generation rule. A shutdown that times out on the lifecycle gate + /// tears down ungated; the wedged initialize that held the gate must then NOT publish its freshly + /// connected session behind that teardown, and its late health write must not overwrite the + /// shutdown's. The positive controls: the wedged init really did reach the broker (AcceptedCount), + /// and a later initialize connects and reports Healthy. + /// + [Trait("Category", "SlowUnit")] + [Fact] + public async Task AWedgedInit_CompletingAfterATimedOutShutdown_PublishesNothing() + { + using var broker = new MiniBroker(); + var options = BrokerOptions(broker.Port) with { ConnectTimeoutSeconds = 1 }; + await using var driver = new MqttDriver(options, "d", null); + + var entered = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var release = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + driver.BeforeConnectHookForTests = async _ => + { + entered.TrySetResult(); + await release.Task; + }; + + var initialize = Task.Run(() => driver.InitializeAsync(Json(options), TestContext.Current.CancellationToken)); + await entered.Task.WaitAsync(TimeSpan.FromSeconds(10), TestContext.Current.CancellationToken); + + // Times out on the gate (1 s) and tears down ungated. + await driver.ShutdownAsync(TestContext.Current.CancellationToken); + driver.GetHealth().State.ShouldBe(DriverState.Unknown); + + driver.BeforeConnectHookForTests = null; + release.SetResult(); + await Should.ThrowAsync(() => initialize); + + broker.AcceptedCount.ShouldBe(1, "the wedged initialize did connect — the control for the absence below"); + await WaitForAsync(() => broker.LiveConnections == 0, "the superseded session to be torn down"); + driver.GetHealth().State.ShouldBe(DriverState.Unknown, "a superseded init must not overwrite newer health"); + driver.GetHostStatuses().ShouldHaveSingleItem().State.ShouldBe(HostState.Unknown); + + await driver.InitializeAsync(Json(options), TestContext.Current.CancellationToken); + await WaitForAsync(() => broker.LiveConnections == 1, "the fresh session"); + driver.GetHealth().State.ShouldBe(DriverState.Healthy); + } + + private static RawTagEntry WritableTag(string rawPath, string writeTopic) => new( + rawPath, + $$"""{"topic":"{{writeTopic}}/state","payloadFormat":"Scalar","dataType":"Float64","writeTopic":"{{writeTopic}}"}""", + WriteIdempotent: false); + + /// + /// R06:M-8. A write batch used to publish strictly serially, one awaited QoS 1 round-trip per + /// request — against a stalled broker, N writes × the publish deadline. Different topics now publish + /// concurrently: all four PUBLISHes reach a broker that never PUBACKs well inside ONE 30 s deadline + /// (serial code would have sent exactly one). Cancelling the batch then fills every slot, so the + /// batch shape holds. + /// + [Fact] + public async Task AWriteBatch_PublishesDifferentTopicsConcurrently() + { + using var broker = new MiniBroker(); // never PUBACKs + var options = BrokerOptions(broker.Port) with + { + ConnectTimeoutSeconds = 30, + RawTags = [.. Enumerable.Range(0, 4).Select(i => WritableTag($"Plant/Mqtt/dev1/Sp{i}", $"cmd/sp{i}"))], + }; + await using var driver = new MqttDriver(options, "d", null); + await driver.InitializeAsync(Json(options), TestContext.Current.CancellationToken); + + using var cts = CancellationTokenSource.CreateLinkedTokenSource(TestContext.Current.CancellationToken); + var write = driver.WriteAsync( + [.. Enumerable.Range(0, 4).Select(i => new WriteRequest($"Plant/Mqtt/dev1/Sp{i}", (double)i))], + cts.Token); + + await WaitForAsync(() => broker.Published.Count == 4, "all four PUBLISHes to be in flight at once"); + write.IsCompleted.ShouldBeFalse("no PUBACK has arrived, so nothing can have completed"); + + await cts.CancelAsync(); + var results = await write.WaitAsync(TimeSpan.FromSeconds(10), TestContext.Current.CancellationToken); + results.Count.ShouldBe(4); + results.ShouldAllBe(r => r.StatusCode == 0x80050000u); // BadCommunicationError, every slot filled + } + + /// + /// Writes to the SAME topic in one batch stay in request order — last write wins at the device — + /// while the batch as a whole is concurrent. The broker PUBACKs here, so every slot is Good — and + /// those acks advance LastSuccessfulRead (writes feed health). + /// + [Fact] + public async Task AWriteBatch_KeepsSameTopicWritesInRequestOrder() + { + using var broker = new MiniBroker { AckPublish = true }; + var options = BrokerOptions(broker.Port) with + { + RawTags = [WritableTag("Plant/Mqtt/dev1/Sp", "cmd/sp"), WritableTag("Plant/Mqtt/dev1/Other", "cmd/other")], + }; + await using var driver = new MqttDriver(options, "d", null); + await driver.InitializeAsync(Json(options), TestContext.Current.CancellationToken); + driver.GetHealth().LastSuccessfulRead.ShouldBeNull("nothing has answered yet"); + + var results = await driver.WriteAsync( + [ + new WriteRequest("Plant/Mqtt/dev1/Sp", 1.0), + new WriteRequest("Plant/Mqtt/dev1/Other", 9.0), + new WriteRequest("Plant/Mqtt/dev1/Sp", 2.0), + new WriteRequest("Plant/Mqtt/dev1/Sp", 3.0), + ], + TestContext.Current.CancellationToken); + + results.ShouldAllBe(r => r.StatusCode == 0u); + broker.Published.Where(p => p.Topic == "cmd/sp") + .Select(p => System.Text.Encoding.UTF8.GetString(p.Payload)) + .ShouldBe(["1", "2", "3"]); + + // A PUBACK is the broker answering, so writes feed the evidence clock the comms-loss watchdog reads. + driver.GetHealth().LastSuccessfulRead.ShouldNotBeNull(); + } + + /// + /// Review minor 6: the bound itself. Ten topics against a broker that never PUBACKs: exactly + /// PUBLISHes go out and the count then holds — a + /// regression to unbounded concurrency sends all ten. The stability check re-reads the count across + /// several condition polls rather than sleeping once. + /// + [Fact] + public async Task AWriteBatch_NeverHasMoreThanTheBoundInFlight() + { + using var broker = new MiniBroker(); // never PUBACKs + var options = BrokerOptions(broker.Port) with + { + ConnectTimeoutSeconds = 30, + RawTags = [.. Enumerable.Range(0, 10).Select(i => WritableTag($"Plant/Mqtt/dev1/Sp{i}", $"cmd/sp{i}"))], + }; + await using var driver = new MqttDriver(options, "d", null); + await driver.InitializeAsync(Json(options), TestContext.Current.CancellationToken); + + using var cts = CancellationTokenSource.CreateLinkedTokenSource(TestContext.Current.CancellationToken); + var write = driver.WriteAsync( + [.. Enumerable.Range(0, 10).Select(i => new WriteRequest($"Plant/Mqtt/dev1/Sp{i}", (double)i))], + cts.Token); + + await WaitForAsync(() => broker.Published.Count >= MqttDriver.MaxConcurrentPublishes, "the bound to fill"); + for (var i = 0; i < 10; i++) + { + await Task.Delay(50, TestContext.Current.CancellationToken); + broker.Published.Count.ShouldBe(MqttDriver.MaxConcurrentPublishes, "a slot freed with no PUBACK received"); + } + + await cts.CancelAsync(); + var results = await write.WaitAsync(TimeSpan.FromSeconds(10), TestContext.Current.CancellationToken); + results.Count.ShouldBe(10); + results.ShouldAllBe(r => r.StatusCode == 0x80050000u); + } + + /// + /// Review minor 2: a QoS 0 publish (a Sparkplug command) completing only means the bytes reached the + /// local socket buffer; the broker has not answered, so it must not advance LastSuccessfulRead. + /// The positive control is , where a QoS 1 + /// PUBACK does advance it. + /// + [Fact] + public async Task ASparkplugQos0Command_DoesNotAdvanceLastSuccessfulRead() + { + using var broker = new MiniBroker(); + var options = BrokerOptions(broker.Port) with + { + Mode = MqttMode.SparkplugB, + Sparkplug = new MqttSparkplugOptions { GroupId = "G", RequestRebirthOnGap = false }, + RawTags = + [ + new RawTagEntry( + "Plant/Mqtt/spb/Setpoint", + """{"groupId":"G","edgeNodeId":"E","metricName":"Setpoint","dataType":"Float64"}""", + WriteIdempotent: false), + ], + }; + await using var driver = new MqttDriver(options, "d", null); + await driver.InitializeAsync(Json(options), TestContext.Current.CancellationToken); + var before = driver.GetHealth().LastSuccessfulRead; + var publishedBefore = broker.Published.Count; + + var results = await driver.WriteAsync( + [new WriteRequest("Plant/Mqtt/spb/Setpoint", 4.5)], TestContext.Current.CancellationToken); + + results.ShouldHaveSingleItem().StatusCode.ShouldBe(0u); + await WaitForAsync( + () => broker.Published.Skip(publishedBefore).Any(p => p.Topic == "spBv1.0/G/NCMD/E"), + "the NCMD to reach the broker — the control that the write really went out"); + driver.GetHealth().LastSuccessfulRead.ShouldBe(before); + } + private static async Task WaitForAsync(Func condition, string what) { var deadline = DateTime.UtcNow + TimeSpan.FromSeconds(10); diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttSubscriptionManagerTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttSubscriptionManagerTests.cs index 558d4477..3757c169 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttSubscriptionManagerTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttSubscriptionManagerTests.cs @@ -538,17 +538,224 @@ public sealed class MqttSubscriptionManagerTests mgr.Values.Read(OvenTempPath).StatusCode.ShouldBe(BadCommunicationError); } + /// + /// A SUBSCRIBE that never got a verdict (transport throw / deadline) degrades its refs AND throws, + /// so DriverInstanceActor holds no handle and its health-tick reconcile re-subscribes. It + /// used to return a live-looking handle with the topic dropped from every retry set — dark behind a + /// Healthy connection until a redeploy. + /// [Fact] - public async Task SubscribeAsync_TransportThrows_DegradesRefsToBad_DoesNotThrow() + public async Task SubscribeAsync_TransportThrows_DegradesRefsToBad_AndThrows() { var transport = new FakeTransport { Throw = new InvalidOperationException("broker gone") }; var mgr = NewManager(transport); mgr.Register([Tag(OvenTempPath, $$"""{"topic":"{{OvenTopic}}","payloadFormat":"Scalar","dataType":"Float64"}""")]); + var raised = new List(); + mgr.OnDataChange += (_, e) => raised.Add(e); - await Should.NotThrowAsync(() => + await Should.ThrowAsync(() => mgr.SubscribeAsync([OvenTempPath], TimeSpan.FromSeconds(1), TestContext.Current.CancellationToken)); mgr.Values.Read(OvenTempPath).StatusCode.ShouldBe(BadCommunicationError); + // The Bad verdict still reached the subscriber before the throw. + raised.ShouldContain(e => e.FullReference == OvenTempPath && e.Snapshot.StatusCode == BadCommunicationError); + } + + /// + /// Review minor 3: a cancel of the CALLER's token (a pipeline timeout, host shutdown) is a cancellation, + /// not a broker fault — it must surface as OperationCanceledException, or the resilience breaker counts + /// it as a failure. The refs are still Bad and pending, so the next reconnect retries them (the positive + /// control). The uncancelled no-verdict case throws InvalidOperationException + /// (). + /// + [Fact] + public async Task SubscribeAsync_CallerCancelled_ThrowsOperationCanceled_AndTheTopicStaysPending() + { + var transport = new FakeTransport { Throw = new OperationCanceledException() }; + var mgr = NewManager(transport); + mgr.Register([Tag(OvenTempPath, $$"""{"topic":"{{OvenTopic}}","payloadFormat":"Scalar","dataType":"Float64"}""")]); + using var cts = new CancellationTokenSource(); + await cts.CancelAsync(); + + var ex = await Should.ThrowAsync(() => + mgr.SubscribeAsync([OvenTempPath], TimeSpan.FromSeconds(1), cts.Token)); + ex.ShouldNotBeOfType(); + mgr.Values.Read(OvenTempPath).StatusCode.ShouldBe(BadCommunicationError); + + transport.Throw = null; + await mgr.OnReconnectedAsync(TestContext.Current.CancellationToken); + transport.Calls.Last().Select(f => f.Topic).ShouldBe([OvenTopic]); + } + + /// + /// The reconcile path: once the actor re-subscribes after the throw, the topic IS re-SUBSCRIBEd and + /// delivery resumes on the new handle — the positive control for the throw above. + /// + [Fact] + public async Task SubscribeAsync_AfterATransportThrow_TheRetrySubscribesAndDelivers() + { + var transport = new FakeTransport { Throw = new InvalidOperationException("broker gone") }; + var mgr = NewManager(transport); + mgr.Register([Tag(OvenTempPath, $$"""{"topic":"{{OvenTopic}}","payloadFormat":"Scalar","dataType":"Float64"}""")]); + await Should.ThrowAsync(() => + mgr.SubscribeAsync([OvenTempPath], TimeSpan.FromSeconds(1), TestContext.Current.CancellationToken)); + + transport.Throw = null; + var handle = await mgr.SubscribeAsync([OvenTempPath], TimeSpan.FromSeconds(1), TestContext.Current.CancellationToken); + var raised = new List(); + mgr.OnDataChange += (_, e) => raised.Add(e); + mgr.HandleMessage(OvenTopic, "21.5"u8.ToArray(), retained: false); + + transport.Calls.Count.ShouldBe(2); + transport.Calls[1].Select(f => f.Topic).ShouldBe([OvenTopic]); + raised.ShouldHaveSingleItem().SubscriptionHandle.ShouldBe(handle); + raised[0].Snapshot.Value.ShouldBe(21.5); + } + + /// + /// A topic whose first SUBSCRIBE was refused stays in a pending set the reconnect re-subscribe + /// covers. The reconnect used to re-SUBSCRIBE only the granted set, so it was never retried. + /// + [Fact] + public async Task OnReconnectedAsync_RetriesATopicWhoseFirstSubscribeWasRefused() + { + var transport = new FakeTransport { Reject = _ => true }; + var mgr = NewManager(transport); + mgr.Register([Tag(OvenTempPath, $$"""{"topic":"{{OvenTopic}}","payloadFormat":"Scalar","dataType":"Float64"}""")]); + var handle = await mgr.SubscribeAsync([OvenTempPath], TimeSpan.FromSeconds(1), TestContext.Current.CancellationToken); + + transport.Reject = _ => false; + await mgr.OnReconnectedAsync(TestContext.Current.CancellationToken); + + transport.Calls.Count.ShouldBe(2); + transport.Calls[1].Select(f => f.Topic).ShouldBe([OvenTopic]); + + var raised = new List(); + mgr.OnDataChange += (_, e) => raised.Add(e); + mgr.HandleMessage(OvenTopic, "3"u8.ToArray(), retained: false); + raised.ShouldHaveSingleItem().SubscriptionHandle.ShouldBe(handle); + raised[0].Snapshot.StatusCode.ShouldBe(Good); + } + + /// + /// A persistently ACL-denied topic must not turn every reconnect into a teardown loop: a pending + /// topic refused again is not "re-established nothing" — nothing that was live was lost. Both a + /// mixed set (the granted topic still re-subscribes) and a pending-only set are covered. + /// + [Fact] + public async Task OnReconnectedAsync_PendingTopicsRefusedAgain_DoesNotThrow() + { + var transport = new FakeTransport { Reject = t => t == "f/oven/press" }; + var mgr = NewManager(transport); + mgr.Register( + [ + Tag(OvenTempPath, $$"""{"topic":"{{OvenTopic}}","payloadFormat":"Scalar","dataType":"Float64"}"""), + Tag(OvenPressPath, """{"topic":"f/oven/press","payloadFormat":"Scalar","dataType":"Float64"}"""), + ]); + await mgr.SubscribeAsync([OvenTempPath, OvenPressPath], TimeSpan.FromSeconds(1), TestContext.Current.CancellationToken); + + await Should.NotThrowAsync(() => mgr.OnReconnectedAsync(TestContext.Current.CancellationToken)); + transport.Calls[1].Select(f => f.Topic).OrderBy(t => t, StringComparer.Ordinal) + .ShouldBe(["f/oven/press", OvenTopic]); + + var lone = new FakeTransport { Reject = _ => true }; + var mgr2 = NewManager(lone); + mgr2.Register([Tag(OvenPressPath, """{"topic":"f/oven/press","payloadFormat":"Scalar","dataType":"Float64"}""")]); + await mgr2.SubscribeAsync([OvenPressPath], TimeSpan.FromSeconds(1), TestContext.Current.CancellationToken); + + await Should.NotThrowAsync(() => mgr2.OnReconnectedAsync(TestContext.Current.CancellationToken)); + lone.Calls.Count.ShouldBe(2); // it WAS retried + } + + /// + /// A later subscribe on the same session (a redeploy's bulk re-subscribe) also retries the pending + /// set, so a broker ACL fixed while connected recovers without waiting for a reconnect. + /// + [Fact] + public async Task SubscribeAsync_RetriesPendingTopicsFromAnEarlierRefusal() + { + var transport = new FakeTransport { Reject = t => t == "f/oven/press" }; + var mgr = NewManager(transport); + mgr.Register( + [ + Tag(OvenTempPath, $$"""{"topic":"{{OvenTopic}}","payloadFormat":"Scalar","dataType":"Float64"}"""), + Tag(OvenPressPath, """{"topic":"f/oven/press","payloadFormat":"Scalar","dataType":"Float64"}"""), + ]); + await mgr.SubscribeAsync([OvenPressPath], TimeSpan.FromSeconds(1), TestContext.Current.CancellationToken); + + transport.Reject = _ => false; + await mgr.SubscribeAsync([OvenTempPath], TimeSpan.FromSeconds(1), TestContext.Current.CancellationToken); + + transport.Calls[1].Select(f => f.Topic).OrderBy(t => t, StringComparer.Ordinal) + .ShouldBe(["f/oven/press", OvenTopic]); + } + + /// + /// R06:M-2. Register prunes the cache to the authored set — an unauthored reference's + /// BadNodeIdUnknown marker and a dropped tag's value go. Control: a still-authored tag keeps its + /// value across the re-register (a redeploy must not blank observed values). + /// + [Fact] + public async Task Register_PrunesCacheEntriesForReferencesNoLongerAuthored() + { + var mgr = NewManager(new FakeTransport()); + mgr.Register( + [ + Tag(OvenTempPath, $$"""{"topic":"{{OvenTopic}}","payloadFormat":"Scalar","dataType":"Float64"}"""), + Tag(OvenPressPath, """{"topic":"f/oven/press","payloadFormat":"Scalar","dataType":"Float64"}"""), + ]); + await mgr.SubscribeAsync([OvenTempPath, OvenPressPath, "Plant/Mqtt/broker1/Nope"], TimeSpan.Zero, TestContext.Current.CancellationToken); + mgr.HandleMessage(OvenTopic, "1"u8.ToArray(), retained: false); + mgr.HandleMessage("f/oven/press", "2"u8.ToArray(), retained: false); + mgr.Values.Read("Plant/Mqtt/broker1/Nope").StatusCode.ShouldBe(BadNodeIdUnknown); + + mgr.Register([Tag(OvenTempPath, $$"""{"topic":"{{OvenTopic}}","payloadFormat":"Scalar","dataType":"Float64"}""")]); + + mgr.Values.Read(OvenTempPath).Value.ShouldBe(1.0); + mgr.Values.Read(OvenPressPath).StatusCode.ShouldBe(0x80320000u); + mgr.Values.Read("Plant/Mqtt/broker1/Nope").StatusCode.ShouldBe(0x80320000u); + mgr.Values.Count.ShouldBe(1); + } + + /// + /// R06:M-11. A subscribe issued before any transport is attached reaches no broker, so it must + /// not be recorded as established. It used to be — "subscribed, receives nothing" — and the first + /// reconnect then had nothing to re-establish. Now it stays pending, and the first reconnect on the + /// attached session attempts it: the Bad verdict from the unconnected session is the proof. + /// + [Fact] + public async Task SubscribeAsync_WithNoTransport_StaysPending_AndTheFirstReconnectAttemptsIt() + { + var mgr = NewManager(transport: null); + mgr.Register([Tag(OvenTempPath, $$"""{"topic":"{{OvenTopic}}","payloadFormat":"Scalar","dataType":"Float64"}""")]); + await mgr.SubscribeAsync([OvenTempPath], TimeSpan.FromSeconds(1), TestContext.Current.CancellationToken); + mgr.Values.Read(OvenTempPath).StatusCode.ShouldBe(0x80320000u); // BadWaitingForInitialData + + await using var attached = new MqttConnection( + new MqttDriverOptions { Host = "127.0.0.1", Port = 1, UseTls = false }, "mqtt-1"); + mgr.AttachTo(attached); + + // The unconnected session gives no verdict, so the reconnect throws — and it ATTEMPTED the topic. + await Should.ThrowAsync(() => mgr.OnReconnectedAsync(TestContext.Current.CancellationToken)); + mgr.Values.Read(OvenTempPath).StatusCode.ShouldBe(BadCommunicationError); + } + + /// + /// Register drops a pending topic whose tags the redeploy removed — the same housekeeping the + /// granted set gets — so a deleted tag's denied topic is not retried forever. + /// + [Fact] + public async Task Register_DroppingATag_RemovesItsTopicFromThePendingSet() + { + var transport = new FakeTransport { Reject = _ => true }; + var mgr = NewManager(transport); + mgr.Register([Tag(OvenPressPath, """{"topic":"f/oven/press","payloadFormat":"Scalar","dataType":"Float64"}""")]); + await mgr.SubscribeAsync([OvenPressPath], TimeSpan.FromSeconds(1), TestContext.Current.CancellationToken); + + mgr.Register([Tag(OvenTempPath, $$"""{"topic":"{{OvenTopic}}","payloadFormat":"Scalar","dataType":"Float64"}""")]); + await mgr.OnReconnectedAsync(TestContext.Current.CancellationToken); + + transport.Calls.Count.ShouldBe(1); // nothing left to retry } // --------------------------------------------------------------------------------- @@ -939,7 +1146,10 @@ public sealed class MqttSubscriptionManagerTests new MqttDriverOptions { Host = "127.0.0.1", Port = 1, UseTls = false }, "mqtt-1"); mgr.AttachTo(rebound); - await mgr.SubscribeAsync([OvenTempPath], TimeSpan.FromSeconds(1), TestContext.Current.CancellationToken); + // The unconnected session gives no verdict, so the subscribe now THROWS (M-1) — the throw is a + // second proof the SUBSCRIBE was attempted against the new session. + await Should.ThrowAsync(() => + mgr.SubscribeAsync([OvenTempPath], TimeSpan.FromSeconds(1), TestContext.Current.CancellationToken)); mgr.Values.Read(OvenTempPath).StatusCode.ShouldBe( BadCommunicationError, diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/SparkplugIngestorTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/SparkplugIngestorTests.cs index 1bd3b830..43c44103 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/SparkplugIngestorTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/SparkplugIngestorTests.cs @@ -852,6 +852,28 @@ public sealed class SparkplugIngestorTests ing.TryResolve(TempPath, out _).ShouldBeFalse(); } + /// + /// R06:M-2. Register prunes the shared cache to the authored set: an unauthored reference's + /// BadNodeIdUnknown marker and a dropped tag's value go, so the cache cannot grow with every RawPath + /// and stray reference ever seen. Control: a still-authored tag's value survives the re-register. + /// + [Fact] + public async Task Register_PrunesCacheEntriesForReferencesNoLongerAuthored() + { + var ing = NewIngestor(); + await ing.SubscribeAsync(["Plant/Mqtt/spb/NotAThing"], TimeSpan.Zero, TestContext.Current.CancellationToken); + var kept = new DataValueSnapshot(1.0f, 0u, null, DateTime.UtcNow); + ing.Values.Update(TempPath, kept); + ing.Values.Update(PressPath, kept); + + ing.Register([Tag(TempPath, Blob(Group, Node, Device, "Temperature", "Float32"))]); + + ing.Values.Read(TempPath).ShouldBe(kept); + ing.Values.Read(PressPath).StatusCode.ShouldBe(0x80320000u); // dropped tag: no stale value + ing.Values.Read("Plant/Mqtt/spb/NotAThing").StatusCode.ShouldBe(0x80320000u); + ing.Values.Count.ShouldBe(1); + } + [Fact] public async Task SubscribeAsync_UnauthoredReference_ReportsBadNodeIdUnknown() { diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/TopicFilterTrieTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/TopicFilterTrieTests.cs new file mode 100644 index 00000000..191b4596 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/TopicFilterTrieTests.cs @@ -0,0 +1,117 @@ +using MQTTnet; +using Shouldly; +using Xunit; + +namespace ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests; + +/// +/// R06:M-6. The wildcard trie replaced a linear MqttTopicFilterComparer scan on the +/// dispatcher thread. Its contract is "a superset of the true matches, confirmed by the comparer", so it +/// is pinned differentially against that comparer — the oracle a broker uses — over a seeded random +/// space that deliberately includes empty levels, $-topics and wildcard-only filters. +/// +public sealed class TopicFilterTrieTests +{ + private static readonly string[] Levels = ["a", "b", "c", "", "$SYS", "x y"]; + + private sealed class Collect : ITopicFilterVisitor + { + public HashSet Seen { get; } = []; + + public void Visit(int value) => Seen.Add(value).ShouldBeTrue("a filter must be visited at most once per match"); + } + + [Fact] + public void Match_IsASupersetOfTheComparer_AndConfirmingYieldsExactlyTheComparersAnswer() + { + var random = new Random(744); + for (var round = 0; round < 200; round++) + { + var filters = Enumerable.Range(0, random.Next(1, 25)).Select(_ => RandomFilter(random)).ToList(); + var trie = TopicFilterTrie.Build(filters.Select((f, i) => (f, i))); + + for (var t = 0; t < 50; t++) + { + var topic = RandomTopic(random); + var expected = filters + .Select((f, i) => (f, i)) + .Where(p => MqttTopicFilterComparer.Compare(topic, p.f) == MqttTopicFilterCompareResult.IsMatch) + .Select(p => p.i) + .ToHashSet(); + + var collect = new Collect(); + trie.Match(topic, ref collect); + + collect.Seen.IsSupersetOf(expected).ShouldBeTrue($"topic '{topic}' missed a match; filters: {string.Join(" | ", filters)}"); + collect.Seen + .Where(i => MqttTopicFilterComparer.Compare(topic, filters[i]) == MqttTopicFilterCompareResult.IsMatch) + .ToHashSet() + .SetEquals(expected) + .ShouldBeTrue(); + } + } + } + + [Theory] + [InlineData("a/#", "a", true)] // '#' includes the parent level + [InlineData("a/#", "a/b/c", true)] + [InlineData("a/+", "a/", true)] // '+' matches an empty level + [InlineData("a/+", "a", false)] + [InlineData("+/+", "a/b", true)] + [InlineData("#", "anything/at/all", true)] + [InlineData("a/b", "a/b", true)] + [InlineData("a/b", "a/c", false)] + public void Match_FindsTheCanonicalCases(string filter, string topic, bool expected) + { + var trie = TopicFilterTrie.Build([(filter, 1)]); + var collect = new Collect(); + trie.Match(topic, ref collect); + collect.Seen.Contains(1).ShouldBe(expected); + } + + /// + /// Review minor 7: partial-level wildcards and $share/… filters are outside the trie's level rules. + /// Whatever it yields, confirmation by the broker's comparer must produce no match — pinned so a later + /// "smarter" trie cannot start delivering them. + /// + [Theory] + [InlineData("a+", "ab")] + [InlineData("a/b#", "a/bc")] + [InlineData("a/b#", "a/b")] + [InlineData("$share/g/a/b", "a/b")] + [InlineData("$share/g/#", "a/b")] + public void Match_partialLevelWildcardsAndSharedFilters_neverConfirm(string filter, string topic) + { + var trie = TopicFilterTrie.Build([(filter, 1)]); + var collect = new Collect(); + trie.Match(topic, ref collect); + + collect.Seen.Where(_ => MqttTopicFilterComparer.Compare(topic, filter) == MqttTopicFilterCompareResult.IsMatch) + .ShouldBeEmpty(); + } + + [Fact] + public void Empty_MatchesNothing() + { + var collect = new Collect(); + TopicFilterTrie.Empty.Match("a/b", ref collect); + collect.Seen.ShouldBeEmpty(); + } + + private static string RandomFilter(Random random) + { + var count = random.Next(1, 5); + var parts = new List(count); + for (var i = 0; i < count; i++) + { + var roll = random.Next(10); + if (roll < 2 && i == count - 1) { parts.Add("#"); break; } + parts.Add(roll < 5 ? "+" : Levels[random.Next(Levels.Length)]); + } + + return string.Join('/', parts); + } + + private static string RandomTopic(Random random) => + string.Join('/', Enumerable.Range(0, random.Next(1, 6)).Select(_ => Levels[random.Next(Levels.Length)])); +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Browser.Tests/OracleFlavouredSqliteDialect.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Browser.Tests/OracleFlavouredSqliteDialect.cs index a6868aa3..0987f4b8 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Browser.Tests/OracleFlavouredSqliteDialect.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Browser.Tests/OracleFlavouredSqliteDialect.cs @@ -38,6 +38,10 @@ public sealed class OracleFlavouredSqliteDialect : ISqlDialect /// public string SingleRowLimitSuffix => _inner.SingleRowLimitSuffix; + public string RowLimitPrefix(int rows) => _inner.RowLimitPrefix(rows); + + public string RowLimitSuffix(int rows) => _inner.RowLimitSuffix(rows); + /// public string ListSchemasSql => _inner.ListSchemasSql; diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Browser.Tests/SqlBrowseSessionTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Browser.Tests/SqlBrowseSessionTests.cs index 6be390f9..0bd53ad5 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Browser.Tests/SqlBrowseSessionTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Browser.Tests/SqlBrowseSessionTests.cs @@ -78,7 +78,8 @@ public sealed class SqlBrowseSessionTests attribute.Name.ShouldBe(SqliteBrowseFixture.RealColumn); attribute.DriverDataType.ShouldBe(nameof(DriverDataType.Float64)); attribute.IsArray.ShouldBeFalse(); - attribute.SecurityClass.ShouldBe("ViewOnly"); + // R06:S-5: every dialect projects IS_NULLABLE and the picker now says so. The column is declared NULL. + attribute.SecurityClass.ShouldBe("ViewOnly · nullable"); attribute.IsAlarm.ShouldBeFalse(); } @@ -92,7 +93,10 @@ public sealed class SqlBrowseSessionTests SqliteDialect.MainSchema, SqliteBrowseFixture.KeyValueTable, SqliteBrowseFixture.TextColumn); var attributes = await session.AttributesAsync(nodeId, CancellationToken.None); - attributes.ShouldHaveSingleItem().DriverDataType.ShouldBe(nameof(DriverDataType.String)); + var attribute = attributes.ShouldHaveSingleItem(); + attribute.DriverDataType.ShouldBe(nameof(DriverDataType.String)); + // The NOT NULL control for the nullable case above — the descriptor tracks the catalog, not a constant. + attribute.SecurityClass.ShouldBe("ViewOnly · NOT NULL"); } /// diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.IntegrationTests/SqlServerReadTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.IntegrationTests/SqlServerReadTests.cs index ebf2d212..73339481 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.IntegrationTests/SqlServerReadTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.IntegrationTests/SqlServerReadTests.cs @@ -58,6 +58,127 @@ public sealed class SqlServerReadTests driver.GetHostStatuses().ShouldHaveSingleItem().State.ShouldBe(HostState.Running); } + // ---- R06:S-1: the row cap, on real T-SQL ---- + + /// + /// A where-pair selector pointed at an events table (40 rows for one station) used to pull every + /// matching row on every poll. The plan now emits SELECT TOP (2) … — proven valid T-SQL here — + /// and the second row makes the tag publish BadConfigurationError while the driver stays + /// Healthy (the server answered). The positive control: a station with exactly one row reads Good + /// through the same capped statement. + /// + [Fact] + public async Task ReadAsync_realSqlServer_anAmbiguousSelectorIsRowCappedToAConfigurationError() + { + SkipUnlessLive(); + const string table = "RowCapEvents"; + + await using (var arrange = await _sql.OpenInspectionConnectionAsync()) + { + await using var command = arrange.CreateCommand(); + command.CommandText = string.Create(CultureInfo.InvariantCulture, $""" + IF OBJECT_ID(N'{SqlPollServerFixture.Schema}.{table}') IS NOT NULL DROP TABLE {SqlPollServerFixture.Schema}.{table}; + CREATE TABLE {SqlPollServerFixture.Schema}.{table} ( + {SqlPollServerFixture.WideRowSelectorColumn} int NOT NULL, + oven_temp float NULL, + {SqlPollServerFixture.TimestampColumn} datetime2 NOT NULL); + INSERT INTO {SqlPollServerFixture.Schema}.{table} VALUES (1, 21.5, '2026-07-24T10:00:00'); + DECLARE @i int = 0; + WHILE @i < 40 + BEGIN + INSERT INTO {SqlPollServerFixture.Schema}.{table} VALUES (7, @i, DATEADD(second, @i, '2026-07-24T10:00:00')); + SET @i += 1; + END + """); + await command.ExecuteNonQueryAsync(TestContext.Current.CancellationToken); + } + + try + { + await using var driver = await StartAsync( + WideRowWhere("Sql/Events/Ambiguous", "oven_temp", "7", table), + WideRowWhere("Sql/Events/Clean", "oven_temp", "1", table)); + + var snapshots = await driver.ReadAsync( + ["Sql/Events/Ambiguous", "Sql/Events/Clean"], TestContext.Current.CancellationToken); + + snapshots[0].StatusCode.ShouldBe(SqlStatusCodes.BadConfigurationError); + snapshots[1].StatusCode.ShouldBe(SqlStatusCodes.Good); + Convert.ToDouble(snapshots[1].Value).ShouldBe(21.5); + driver.GetHealth().State.ShouldBe(DriverState.Healthy); + } + finally + { + await using var cleanup = await _sql.OpenInspectionConnectionAsync(); + await using var drop = cleanup.CreateCommand(); + drop.CommandText = $"DROP TABLE IF EXISTS {SqlPollServerFixture.Schema}.{table}"; + await drop.ExecuteNonQueryAsync(CancellationToken.None); + } + } + + /// + /// Review I-1, live: one stray duplicate row in a key-value table fails ONLY that key's tag; the other + /// key in the same group (one capped SELECT TOP (5) … IN (@k0, @k1)) reads Good — the positive + /// control — and the driver stays Healthy. + /// + [Fact] + public async Task ReadAsync_realSqlServer_aDuplicatedKeyFailsOnlyItsOwnTag() + { + SkipUnlessLive(); + const string table = "RowCapCurrent"; + var qualified = $"{SqlPollServerFixture.Schema}.{table}"; + + await using (var arrange = await _sql.OpenInspectionConnectionAsync()) + { + await using var command = arrange.CreateCommand(); + command.CommandText = $""" + IF OBJECT_ID(N'{qualified}') IS NOT NULL DROP TABLE {qualified}; + CREATE TABLE {qualified} ( + {SqlPollServerFixture.KeyColumn} nvarchar(64) NOT NULL, + {SqlPollServerFixture.ValueColumn} float NULL, + {SqlPollServerFixture.TimestampColumn} datetime2 NOT NULL); + INSERT INTO {qualified} VALUES + (N'Pump7', 1.0, '2026-07-24T10:00:00'), + (N'Pump7', 2.0, '2026-07-24T10:00:01'), + (N'Pump8', 8.5, '2026-07-24T10:00:00'); + """; + await command.ExecuteNonQueryAsync(TestContext.Current.CancellationToken); + } + + try + { + await using var driver = await StartAsync( + KeyValueOn("Sql/Current/Pump7", "Pump7", table), + KeyValueOn("Sql/Current/Pump8", "Pump8", table)); + + var snapshots = await driver.ReadAsync( + ["Sql/Current/Pump7", "Sql/Current/Pump8"], TestContext.Current.CancellationToken); + + snapshots[0].StatusCode.ShouldBe(SqlStatusCodes.BadConfigurationError); + snapshots[1].StatusCode.ShouldBe(SqlStatusCodes.Good); + Convert.ToDouble(snapshots[1].Value).ShouldBe(8.5); + driver.GetHealth().State.ShouldBe(DriverState.Healthy); + } + finally + { + await using var cleanup = await _sql.OpenInspectionConnectionAsync(); + await using var drop = cleanup.CreateCommand(); + drop.CommandText = $"DROP TABLE IF EXISTS {qualified}"; + await drop.ExecuteNonQueryAsync(CancellationToken.None); + } + } + + private static RawTagEntry KeyValueOn(string rawPath, string key, string table) => Tag(rawPath, new JsonObject + { + ["driver"] = "Sql", + ["model"] = "KeyValue", + ["table"] = SqlPollServerFixture.Qualified(table), + ["keyColumn"] = SqlPollServerFixture.KeyColumn, + ["keyValue"] = key, + ["valueColumn"] = SqlPollServerFixture.ValueColumn, + ["timestampColumn"] = SqlPollServerFixture.TimestampColumn, + }); + // ---- key-value model ---- /// The plan's headline case: a seeded key-value row round-trips as a Good snapshot. diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/ColonMarkerDialect.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/ColonMarkerDialect.cs index f514b1be..939a267b 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/ColonMarkerDialect.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/ColonMarkerDialect.cs @@ -77,6 +77,10 @@ public sealed class ColonMarkerDialect : ISqlDialect /// public string SingleRowLimitSuffix => Inner.SingleRowLimitSuffix; + public string RowLimitPrefix(int rows) => Inner.RowLimitPrefix(rows); + + public string RowLimitSuffix(int rows) => Inner.RowLimitSuffix(rows); + /// public string ListSchemasSql => Colonize(Inner.ListSchemasSql); diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/OracleDialectTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/OracleDialectTests.cs index a0050cf7..70629ae4 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/OracleDialectTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/OracleDialectTests.cs @@ -210,6 +210,15 @@ public class OracleDialectTests public void LivenessSql_selectsFromDual() => new OracleDialect().LivenessSql.ShouldBe("SELECT 1 FROM DUAL"); + [Fact] + public void RowLimit_isFetchFirstN_inTheSuffixPosition() + { + var d = new OracleDialect(); + d.RowLimitPrefix(3).ShouldBe(""); + d.RowLimitSuffix(3).ShouldBe(" FETCH FIRST 3 ROWS ONLY"); + Should.Throw(() => d.RowLimitSuffix(0)); + } + [Fact] public void SingleRowLimit_isFetchFirst_inTheSuffixPosition_withItsOwnLeadingSpace() { diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/PostgresDialectTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/PostgresDialectTests.cs index 2f56b7d1..de681931 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/PostgresDialectTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/PostgresDialectTests.cs @@ -226,6 +226,15 @@ public class PostgresDialectTests d.ListColumnsSql.ShouldContain("\"IS_NULLABLE\""); } + [Fact] + public void RowLimit_isLimitN_inTheSuffixPosition() + { + var d = new PostgresDialect(); + d.RowLimitPrefix(3).ShouldBe(""); + d.RowLimitSuffix(3).ShouldBe(" LIMIT 3"); + Should.Throw(() => d.RowLimitSuffix(0)); + } + [Fact] public void SingleRowLimit_isLimit1_inTheSuffixPosition_withItsOwnLeadingSpace() { diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlCatalogDefaultSchemaTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlCatalogDefaultSchemaTests.cs index 0c72ca4a..2f8a1f47 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlCatalogDefaultSchemaTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlCatalogDefaultSchemaTests.cs @@ -113,6 +113,10 @@ public sealed class SqlCatalogDefaultSchemaTests public string SingleRowLimitSuffix => Inner.SingleRowLimitSuffix; + public string RowLimitPrefix(int rows) => Inner.RowLimitPrefix(rows); + + public string RowLimitSuffix(int rows) => Inner.RowLimitSuffix(rows); + public string ListSchemasSql => twoSchemas ? Inner.ListSchemasSql + " UNION ALL SELECT 'other' AS TABLE_SCHEMA" : Inner.ListSchemasSql; diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlCatalogGateDriverTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlCatalogGateDriverTests.cs index f0e5d373..017c0bd9 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlCatalogGateDriverTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlCatalogGateDriverTests.cs @@ -374,6 +374,10 @@ public sealed class SqlCatalogGateDriverTests public string SingleRowLimitSuffix => Inner.SingleRowLimitSuffix; + public string RowLimitPrefix(int rows) => Inner.RowLimitPrefix(rows); + + public string RowLimitSuffix(int rows) => Inner.RowLimitSuffix(rows); + public string ListSchemasSql { get; } = listSchemasSql; public string DefaultSchemaSql => Inner.DefaultSchemaSql; diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlDriverFactoryTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlDriverFactoryTests.cs index 65d6a281..9d1265de 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlDriverFactoryTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlDriverFactoryTests.cs @@ -25,6 +25,28 @@ public sealed class SqlDriverFactoryTests { // ---- the plan's headline legs ---- + /// + /// R06:S-8. A malformed config used to quote System.Text.Json's message — which echoes the + /// offending token — into the exception that becomes InitializeFailed → health + /// LastError → /hosts. A pasted connection string is exactly the malformed text that + /// reaches here, so the parser text (and the inner exception carrying it) is withheld, as the probe and + /// browser already do. The message still names the instance and says what is wrong. + /// + [Fact] + public void CreateInstance_malformedJson_withholdsTheParserText() + { + const string secret = "hunter2-do-not-log"; + var ex = Should.Throw(() => + SqlDriverFactoryExtensions.CreateInstance( + "d1", $$"""{"provider":"SqlServer","connectionStringRef": Password={{secret}} }""", null)); + + ex.Message.ShouldContain("d1"); + ex.Message.ShouldContain("not valid JSON"); + ex.Message.ShouldNotContain(secret); + ex.Message.ShouldNotContain("Password"); + ex.InnerException.ShouldBeNull(); + } + [Fact] public void CreateInstance_missingConnectionStringRef_throwsActionable() => Should.Throw(() => diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlDriverTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlDriverTests.cs index 492cc05d..ed93c7a5 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlDriverTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlDriverTests.cs @@ -272,14 +272,15 @@ public sealed class SqlDriverTests // ---- the comms-loss watchdog's evidence clock (Driver.Sql-006) ---- [Fact] - public async Task ReadAsync_whenTheDatabaseAnswersWithNoRow_advancesTheEvidenceClock_withoutChangingHealth() + public async Task ReadAsync_whenTheDatabaseAnswersWithNoRow_advancesTheEvidenceClock_andRestoresHealthy() { // Driver.Sql-006. LastSuccessfulRead is what DriverInstanceActor's comms-loss watchdog reads: a // non-Healthy driver whose clock has been frozen for 90 s is force-reconnected (Gitea #527), on the // premise that the clock is the honest evidence the transport works. BadNoData IS that evidence — // the query ran and the row is absent — so a driver whose key set was deleted must not be - // reconnected every 90 s against a database answering every poll on time. The health STATE stays - // exactly as it was: an absent row is a data fact, not a connectivity one. + // reconnected every 90 s against a database answering every poll on time. And since the database + // answered, the driver is Healthy again (lane rule: a refusal means the device answered — it used to + // leave the Degraded latched behind every refusal-only poll). using var fixture = new SqlitePollFixture(); await using var driver = NewDriver(fixture, KvEntry("Missing", SqlitePollFixture.AbsentKey)); await driver.InitializeAsync(ConfigJson, CancellationToken.None); @@ -295,7 +296,7 @@ public sealed class SqlDriverTests snapshots[0].StatusCode.ShouldBe(SqlStatusCodes.BadNoData); var health = driver.GetHealth(); health.LastSuccessfulRead.ShouldNotBeNull().ShouldBeGreaterThan(before); // RED pre-fix: frozen - health.State.ShouldBe(DriverState.Degraded); // classification untouched + health.State.ShouldBe(DriverState.Healthy); // the database answered } [Fact] @@ -356,6 +357,191 @@ public sealed class SqlDriverTests driver.GetHealth().LastSuccessfulRead.ShouldBe(before); } + /// + /// R06:S-1. An over-cap result set publishes BadConfigurationError — the code the planner also + /// mints before any I/O, which on its own is NOT answer evidence (theory above). The reader's + /// answered-refusal count is what makes it count: the database answered, so the clock advances and a + /// comms-Degraded driver is Healthy again. Control: the same snapshot with no count leaves both alone. + /// + [Fact] + public async Task ObservePollOutcome_aRowCapRefusal_isAnswerEvidence_andRestoresHealthy() + { + using var fixture = new SqlitePollFixture(); + await using var driver = NewDriver(fixture, KvEntry("Speed", SqlitePollFixture.PresentKey)); + await driver.InitializeAsync(ConfigJson, CancellationToken.None); + var refused = new DataValueSnapshot(null, SqlStatusCodes.BadConfigurationError, null, DateTime.UtcNow); + + driver.ObservePollOutcome([new DataValueSnapshot(null, SqlStatusCodes.BadTimeout, null, DateTime.UtcNow)]); + driver.GetHealth().State.ShouldBe(DriverState.Degraded); + var before = driver.GetHealth().LastSuccessfulRead.ShouldNotBeNull(); + await Task.Delay(20, TestContext.Current.CancellationToken); + + driver.ObservePollOutcome([refused]); // no count: planner-minted, no evidence + driver.GetHealth().State.ShouldBe(DriverState.Degraded); + driver.GetHealth().LastSuccessfulRead.ShouldBe(before); + + driver.ObservePollOutcome([refused], answeredRefusals: 1); // the database answered, over-cap + driver.GetHealth().State.ShouldBe(DriverState.Healthy); + driver.GetHealth().LastSuccessfulRead.ShouldNotBeNull().ShouldBeGreaterThan(before); + } + + /// + /// R06:S-9 / lifecycle generations. A poll that started before a Shutdown → Initialize cycle + /// (it can outlive the engine's 5 s teardown wait) must not write its verdict onto the NEW + /// configuration's health. Control: the same BadTimeout verdict under the current generation degrades. + /// + [Fact] + public async Task ObservePollOutcome_fromASupersededGeneration_isIgnored() + { + using var fixture = new SqlitePollFixture(); + await using var driver = NewDriver(fixture, KvEntry("Speed", SqlitePollFixture.PresentKey)); + await driver.InitializeAsync(ConfigJson, CancellationToken.None); + var stale = driver.LifecycleGenerationForTests; + + await driver.ShutdownAsync(CancellationToken.None); + await driver.InitializeAsync(ConfigJson, CancellationToken.None); + driver.GetHealth().State.ShouldBe(DriverState.Healthy); + var timedOut = new DataValueSnapshot(null, SqlStatusCodes.BadTimeout, null, DateTime.UtcNow); + + driver.ObservePollOutcome([timedOut], generation: stale); + driver.GetHealth().State.ShouldBe(DriverState.Healthy, "a superseded poll must not degrade the new init"); + + driver.ObservePollOutcome([timedOut], generation: driver.LifecycleGenerationForTests); + driver.GetHealth().State.ShouldBe(DriverState.Degraded); + } + + /// + /// Only the initialize's own final write ends Initializing: a Good poll landing mid-init used to + /// flip it to Healthy before liveness had even answered. Control: once the init completes it is Healthy. + /// + [Fact] + public async Task APollLandingMidInitialize_cannotEndInitializing() + { + using var fixture = new SqlitePollFixture(); + var factory = new GatedFactory(); + await using var driver = NewDriver(fixture, fixture.ConnectionString, CapturingLogger.Null, factory, + KvEntry("Speed", SqlitePollFixture.PresentKey)); + + factory.Arm(); + var init = driver.InitializeAsync(ConfigJson, CancellationToken.None); + (await factory.Entered.Task.WaitAsync(TimeSpan.FromSeconds(10), TestContext.Current.CancellationToken)).ShouldBeTrue(); + driver.GetHealth().State.ShouldBe(DriverState.Initializing); + + driver.ObservePollOutcome([new DataValueSnapshot(42.0, SqlStatusCodes.Good, DateTime.UtcNow, DateTime.UtcNow)]); + driver.GetHealth().State.ShouldBe(DriverState.Initializing); + + factory.Release(); + await init.WaitAsync(TimeSpan.FromSeconds(10), TestContext.Current.CancellationToken); + driver.GetHealth().State.ShouldBe(DriverState.Healthy); + } + + /// A SQLite factory whose next connection creation blocks until released — parks an initialize. + private sealed class GatedFactory : DbProviderFactory + { + private readonly ManualResetEventSlim _release = new(initialState: true); + private volatile bool _armed; + + public TaskCompletionSource Entered { get; } = new(TaskCreationOptions.RunContinuationsAsynchronously); + + public void Arm() + { + _release.Reset(); + _armed = true; + } + + public void Release() => _release.Set(); + + public override DbConnection CreateConnection() + { + if (_armed) + { + _armed = false; + Entered.TrySetResult(true); + _release.Wait(TimeSpan.FromSeconds(30)); + } + + return Microsoft.Data.Sqlite.SqliteFactory.Instance.CreateConnection()!; + } + } + + /// + /// R06:S-4. A skipped tag's ledger reason (what /hosts shows) is the deploy gate's own explanation + /// — for a Query tag, that the model is deferred — instead of a generic "did not map". Control: a + /// well-formed tag is not skipped at all. + /// + [Fact] + public async Task ADeferredQueryTag_isSkipped_withTheGatesReason() + { + using var fixture = new SqlitePollFixture(); + await using var driver = NewDriver(fixture, + KvEntry("Speed", SqlitePollFixture.PresentKey), + new RawTagEntry("Q", """{"driver":"Sql","model":"Query","table":"T","query":"SELECT 1"}""", WriteIdempotent: false)); + await driver.InitializeAsync(ConfigJson, CancellationToken.None); + + var report = ((ISkippedTagReporter)driver).GetSkippedTags().ShouldNotBeNull(); + var skipped = report.Sample.ShouldHaveSingleItem(); + skipped.RawPath.ShouldBe("Q"); + skipped.Reason.ShouldContain("Query"); + } + + /// + /// Review m-1. A redeploy re-initialises without teardown, so a poll can capture the reader just + /// before a rebind disposes it. That poll used to throw an OperationCanceledException the engine then + /// classified as a poll failure (Degrade). It now retries once on the current reader — and reads Good, + /// the positive control — with health untouched. + /// + [Fact] + public async Task APollOnAReplacedReader_retriesOnTheCurrentOne() + { + using var fixture = new SqlitePollFixture(); + await using var driver = NewDriver(fixture, KvEntry("Speed", SqlitePollFixture.PresentKey)); + await driver.InitializeAsync(ConfigJson, CancellationToken.None); + var captured = driver.ReaderForTests; + + driver.RebindForTests(); + captured.GateDisposedForTests.ShouldBeTrue(); + + var snapshots = await driver.ReadThroughAsync( + captured, ["Speed"], driver.LifecycleGenerationForTests, CancellationToken.None); + + snapshots[0].Value.ShouldBe(SqlitePollFixture.PresentValue); + driver.GetHealth().State.ShouldBe(DriverState.Healthy); + } + + /// + /// With no newer reader (the driver's own reader was disposed) the disposed-reader exception still + /// propagates, but the engine's error callback must not degrade for it. Control: any other poll error + /// still degrades. + /// + [Fact] + public async Task ADisposedReaderError_isNotAPollFailure() + { + using var fixture = new SqlitePollFixture(); + await using var driver = NewDriver(fixture, KvEntry("Speed", SqlitePollFixture.PresentKey)); + await driver.InitializeAsync(ConfigJson, CancellationToken.None); + driver.ReaderForTests.Dispose(); + + // Awaited directly (as the poll engine does): an OperationCanceledException-derived throw ends the task + // Canceled, and only a direct await rethrows the original instance — which HandlePollError matches on. + Exception? caught = null; + try + { + await driver.ReadThroughAsync( + driver.ReaderForTests, ["Speed"], driver.LifecycleGenerationForTests, CancellationToken.None); + } + catch (Exception e) + { + caught = e; + } + + caught.ShouldBeOfType(); + driver.HandlePollError(caught!); + driver.GetHealth().State.ShouldBe(DriverState.Healthy); + + driver.HandlePollError(new InvalidOperationException("contract violation")); + driver.GetHealth().State.ShouldBe(DriverState.Degraded); + } + // ---- Driver.Sql-010: the health field is read-modify-written from several poll loops ---- [Fact] @@ -445,6 +631,31 @@ public sealed class SqlDriverTests Volatile.Read(ref changes).ShouldBe(afterUnsubscribe); } + /// + /// R02:P1. The driver runs PollGroupEngine, so it raises one batch per poll burst: + /// DriverInstanceActor then forwards one message per cycle instead of one per tag. Both tags of + /// the first (force-raised) poll arrive in the SAME batch, on the subscription's handle. + /// + [Fact] + public async Task Subscribe_raisesOneBatchPerPollBurst() + { + using var fixture = new SqlitePollFixture(); + await using var driver = NewDriver(fixture, + KvEntry("Speed", SqlitePollFixture.PresentKey), KvEntry("Pressure", SqlitePollFixture.SecondPresentKey)); + await driver.InitializeAsync(ConfigJson, CancellationToken.None); + + var batchable = driver.ShouldBeAssignableTo(); + var first = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + batchable!.OnDataChangeBatch += (_, args) => first.TrySetResult(args); + + var handle = await driver.SubscribeAsync(["Speed", "Pressure"], TimeSpan.FromMilliseconds(100), CancellationToken.None); + + var batch = await first.Task.WaitAsync(TimeSpan.FromSeconds(10), TestContext.Current.CancellationToken); + batch.SubscriptionHandle.ShouldBe(handle); + batch.Items.Select(i => i.FullReference).OrderBy(r => r, StringComparer.Ordinal).ShouldBe(["Pressure", "Speed"]); + batch.Items.Single(i => i.FullReference == "Speed").Snapshot.Value.ShouldBe(SqlitePollFixture.PresentValue); + } + // ---- disposal ---- [Fact] diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlEquipmentTagParserTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlEquipmentTagParserTests.cs index 4734ee30..3433a4af 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlEquipmentTagParserTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlEquipmentTagParserTests.cs @@ -232,6 +232,21 @@ public class SqlEquipmentTagParserTests SqlEquipmentTagParser.TryParse(json, "raw", out _).ShouldBeFalse(); } + /// + /// R06:S-4. The deploy gate (EquipmentTagConfigInspector) reaches this through the parser: + /// a Query tag draws a warning naming the deferral (a rejection under TagConfigValidation=Error). + /// Control: a served key-value blob draws nothing. + /// + [Fact] + public void Inspect_theDeferredQueryModel_isNamedAtTheDeployGate() + { + SqlTagDefinitionFactory.Inspect("""{"driver":"Sql","model":"Query","table":"T","query":"SELECT 1"}""") + .ShouldHaveSingleItem().ShouldContain("Query"); + SqlTagDefinitionFactory.Inspect( + """{"driver":"Sql","model":"KeyValue","table":"T","keyColumn":"k","keyValue":"v","valueColumn":"c"}""") + .ShouldBeEmpty(); + } + [Theory] [InlineData("""{"driver":"Sql","model":"KeyValue","keyColumn":"k","keyValue":"v","valueColumn":"c"}""")] [InlineData("""{"driver":"Sql","model":"KeyValue","table":"","keyColumn":"k","keyValue":"v","valueColumn":"c"}""")] diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlGroupPlannerChunkingTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlGroupPlannerChunkingTests.cs index d8bc6c8a..dd6920c2 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlGroupPlannerChunkingTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlGroupPlannerChunkingTests.cs @@ -150,7 +150,7 @@ public sealed class SqlGroupPlannerChunkingTests plan.ParameterNames.ShouldBe( Enumerable.Range(0, plan.Parameters.Count) .Select(i => "@k" + i.ToString(CultureInfo.InvariantCulture))); - plan.SqlText.ShouldEndWith("IN (" + string.Join(", ", plan.ParameterNames) + ")"); + plan.SqlText.ShouldContain("IN (" + string.Join(", ", plan.ParameterNames) + ")"); } } @@ -297,6 +297,10 @@ public sealed class SqlGroupPlannerChunkingTests public string SingleRowLimitSuffix => Inner.SingleRowLimitSuffix; + public string RowLimitPrefix(int rows) => Inner.RowLimitPrefix(rows); + + public string RowLimitSuffix(int rows) => Inner.RowLimitSuffix(rows); + public string ListSchemasSql => Inner.ListSchemasSql; public string DefaultSchemaSql => Inner.DefaultSchemaSql; diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlGroupPlannerTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlGroupPlannerTests.cs index 344f4447..ef7661b9 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlGroupPlannerTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlGroupPlannerTests.cs @@ -29,6 +29,39 @@ public class SqlGroupPlannerTests RowSelectorColumn: selectorColumn, RowSelectorValue: selectorValue, RowSelectorTopByTimestamp: topByTimestamp); + // ---- R06:S-1: the row cap ---- + + /// + /// A key-value plan reads up to twice its distinct keys (so stray duplicates are attributed per key) and + /// asks for one row more; a where-pair wide-row plan asks for two. Enough to SEE an ambiguous selector, + /// never enough to materialise a history table every poll. + /// The topByTimestamp form keeps its single-row limit and needs no cap. + /// + [Fact] + public void Plans_carryARowCap_andAskForOneRowMore() + { + var plans = SqlGroupPlanner.Plan( + [ + KvTag("A", "dbo.TagValues", "tag_name", "k0", "num_value", null), + KvTag("B", "dbo.TagValues", "tag_name", "k1", "num_value", null), + KvTag("C", "dbo.TagValues", "tag_name", "k1", "num_value", null), // same key: one row serves both + WideTag("W", "dbo.LatestStatus", "oven_temp", selectorColumn: "station_id", selectorValue: "7"), + WideTag("T", "dbo.Readings", "oven_temp", topByTimestamp: "sample_ts"), + ], new SqlServerDialect()).ToList(); + + var kv = plans.Single(p => p.Model == SqlTagModel.KeyValue); + kv.RowCap.ShouldBe(4); // 2 distinct keys, with 2x headroom for per-key attribution + kv.SqlText.ShouldStartWith("SELECT TOP (5) "); + + var wherePair = plans.Single(p => p.Parameters.Count == 1 && p.Model == SqlTagModel.WideRow); + wherePair.RowCap.ShouldBe(1); + wherePair.SqlText.ShouldStartWith("SELECT TOP (2) "); + + var top = plans.Single(p => p.Parameters.Count == 0); + top.RowCap.ShouldBeNull(); + top.SqlText.ShouldStartWith("SELECT TOP 1 "); + } + // ---- the plan's golden case ---- [Fact] @@ -58,7 +91,7 @@ public class SqlGroupPlannerTests new SqlServerDialect()); plans[0].SqlText.ShouldBe( - "SELECT [tag_name], [num_value], [sample_ts] FROM [dbo].[TagValues] WHERE [tag_name] IN (@k0)"); + "SELECT TOP (3) [tag_name], [num_value], [sample_ts] FROM [dbo].[TagValues] WHERE [tag_name] IN (@k0)"); plans[0].ParameterNames.ShouldBe(new[] { "@k0" }); plans[0].SelectedColumns.ShouldBe(new[] { "tag_name", "num_value", "sample_ts" }); plans[0].Model.ShouldBe(SqlTagModel.KeyValue); @@ -75,7 +108,7 @@ public class SqlGroupPlannerTests new SqlServerDialect()); plans[0].SqlText.ShouldBe( - "SELECT [tag_name], [num_value] FROM [dbo].[TagValues] WHERE [tag_name] IN (@k0)"); + "SELECT TOP (3) [tag_name], [num_value] FROM [dbo].[TagValues] WHERE [tag_name] IN (@k0)"); plans[0].TimestampColumn.ShouldBeNull(); } @@ -181,7 +214,7 @@ public class SqlGroupPlannerTests var plan = plans[0]; plan.ParameterNames.Count.ShouldBe(plan.Parameters.Count); - plan.SqlText.ShouldEndWith("IN (" + string.Join(", ", plan.ParameterNames) + ")"); + plan.SqlText.ShouldEndWith("IN (" + string.Join(", ", plan.ParameterNames) + ")"); // T-SQL caps at the front plan.Parameters.ShouldBe(new object[] { "k0", "k1", "k2" }); } @@ -201,7 +234,7 @@ public class SqlGroupPlannerTests KvTag("B", "TagValues", "tag_name", "Line1.Temp", "num_value", null), ], new ColonMarkerDialect()).ToList(); - plans[0].SqlText.ShouldEndWith("IN (:k0, :k1)"); + plans[0].SqlText.ShouldEndWith("IN (:k0, :k1) LIMIT 5"); plans[0].SqlText.ShouldNotContain("@"); plans[0].ParameterNames.ShouldBe(new[] { ":k0", ":k1" }); } @@ -213,7 +246,7 @@ public class SqlGroupPlannerTests [WideTag("A", "LatestStatus", "oven_temp", selectorColumn: "station_id", selectorValue: "7")], new ColonMarkerDialect()).ToList(); - plans[0].SqlText.ShouldEndWith("= :w"); + plans[0].SqlText.ShouldEndWith("= :w LIMIT 2"); plans[0].SqlText.ShouldNotContain("@"); plans[0].ParameterNames.ShouldBe(new[] { ":w" }); } @@ -277,7 +310,7 @@ public class SqlGroupPlannerTests plans.Count.ShouldBe(1); plans[0].SqlText.ShouldBe( - "SELECT [oven_temp], [pressure] FROM [dbo].[LatestStatus] WHERE [station_id] = @w"); + "SELECT TOP (2) [oven_temp], [pressure] FROM [dbo].[LatestStatus] WHERE [station_id] = @w"); plans[0].ParameterNames.ShouldBe(new[] { "@w" }); plans[0].Parameters.ShouldBe(new object[] { "7" }); // the whereValue is bound, never inlined plans[0].SelectedColumns.ShouldBe(new[] { "oven_temp", "pressure" }); @@ -313,7 +346,7 @@ public class SqlGroupPlannerTests plans.Count.ShouldBe(1); plans[0].SelectedColumns.ShouldBe(new[] { "oven_temp", "pressure", "sample_ts" }); plans[0].SqlText.ShouldBe( - "SELECT [oven_temp], [pressure], [sample_ts] FROM [dbo].[LatestStatus] WHERE [station_id] = @w"); + "SELECT TOP (2) [oven_temp], [pressure], [sample_ts] FROM [dbo].[LatestStatus] WHERE [station_id] = @w"); } [Fact] @@ -424,7 +457,7 @@ public class SqlGroupPlannerTests // A column name is an identifier — it cannot be parameterized, so the guarantee is that it is // bracket-quoted with ] doubled, leaving one (nonexistent) identifier rather than executable SQL. plans[0].SqlText.ShouldBe( - "SELECT [tag_name], [v]] ; DROP TABLE Users --] FROM [dbo].[T] WHERE [tag_name] IN (@k0)"); + "SELECT TOP (3) [tag_name], [v]] ; DROP TABLE Users --] FROM [dbo].[T] WHERE [tag_name] IN (@k0)"); } // ---- contract edges ---- diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlPollReaderTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlPollReaderTests.cs index da658517..19d28944 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlPollReaderTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlPollReaderTests.cs @@ -187,7 +187,7 @@ public sealed class SqlPollReaderTests } [Fact] - public async Task ReadAsync_wideRow_whenTheSelectorMatchesManyRows_warnsInsteadOfSilentlyPickingOne() + public async Task ReadAsync_wideRow_whenTheSelectorMatchesManyRows_publishesAConfigurationError_andWarns() { using var fixture = new SqlitePollFixture(); // A second row for the SAME station. The where-pair form emits no ORDER BY and no row limit, so @@ -201,8 +201,11 @@ public sealed class SqlPollReaderTests var snapshots = await reader.ReadAsync(["Oven"], CancellationToken.None); - // Still a value — the reader degrades to "last row wins", exactly as the key-value model does. - snapshots[0].StatusCode.ShouldBe(SqlStatusCodes.Good); + // R06:S-1: the where-pair plan now asks for TWO rows (one more than it can use). Getting the second is + // proof the selector is ambiguous, so the group publishes a configuration error rather than a + // storage-order-dependent "last row" value — and nothing past that second row is ever read. + snapshots[0].StatusCode.ShouldBe(SqlStatusCodes.BadConfigurationError); + snapshots[0].Value.ShouldBeNull(); var warning = logger.Entries.ShouldHaveSingleItem(); warning.Level.ShouldBe(LogLevel.Warning); @@ -269,8 +272,48 @@ public sealed class SqlPollReaderTests snapshots[0].SourceTimestampUtc!.Value.ShouldBe(snapshots[0].ServerTimestampUtc); } + /// + /// R06:S-1 review I-1. One stray duplicate row is attributed to ITS key: that key's tags publish + /// and every other key in the group reads normally. + /// It used to be last-row-wins Good, or the whole group Bad, depending on whether an UNRELATED key was + /// absent. Both variants are pinned: every other key present, and another key absent. The other keys' + /// Good / BadNoData are the positive controls. + /// + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task ReadAsync_aDuplicatedKey_failsOnlyThatKeysTags_whateverElseIsAbsent(bool withAnAbsentKey) + { + using var fixture = new SqlitePollFixture(); + var logger = new RecordingLogger(); + fixture.Execute( + $"INSERT INTO {SqlitePollFixture.KeyValueTable} " + + $"({SqlitePollFixture.KeyColumn}, {SqlitePollFixture.ValueColumn}, {SqlitePollFixture.TimestampColumn}) " + + $"VALUES ('{SqlitePollFixture.PresentKey}', 99.0, '2026-07-24T10:00:09Z')"); + var tags = new List + { + KvTag("Speed", SqlitePollFixture.PresentKey), + KvTag("Speed2", SqlitePollFixture.PresentKey), // a second tag on the duplicated key + KvTag("Pressure", SqlitePollFixture.SecondPresentKey), + }; + if (withAnAbsentKey) tags.Add(KvTag("Gone", SqlitePollFixture.AbsentKey)); + var reader = Reader(fixture, logger, [.. tags]); + + var snapshots = await reader.ReadAsync([.. tags.Select(t => t.Name)], CancellationToken.None); + + snapshots[0].StatusCode.ShouldBe(SqlStatusCodes.BadConfigurationError); + snapshots[1].StatusCode.ShouldBe(SqlStatusCodes.BadConfigurationError); + snapshots[2].StatusCode.ShouldBe(SqlStatusCodes.Good); + snapshots[2].Value.ShouldBe(SqlitePollFixture.SecondPresentValue); + if (withAnAbsentKey) snapshots[3].StatusCode.ShouldBe(SqlStatusCodes.BadNoData); + + var warning = logger.Entries.ShouldHaveSingleItem(); + warning.Message.ShouldContain(SqlitePollFixture.PresentKey); + } + + /// The driver counts a duplicated-key refusal as the database answering, like an over-cap one. [Fact] - public async Task ReadAsync_whenASourceViolatesOneRowPerKey_takesTheLastRowDeterministically() + public async Task ReadDetailedAsync_aDuplicatedKey_isReportedAsAnAnsweredRefusal() { using var fixture = new SqlitePollFixture(); fixture.Execute( @@ -278,12 +321,86 @@ public sealed class SqlPollReaderTests $"({SqlitePollFixture.KeyColumn}, {SqlitePollFixture.ValueColumn}, {SqlitePollFixture.TimestampColumn}) " + $"VALUES ('{SqlitePollFixture.PresentKey}', 99.0, '2026-07-24T10:00:09Z')"); var reader = Reader(fixture, KvTag("Speed", SqlitePollFixture.PresentKey)); + var clean = Reader(fixture, KvTag("Pressure", SqlitePollFixture.SecondPresentKey)); - var snapshots = await reader.ReadAsync(["Speed"], CancellationToken.None); + (await reader.ReadDetailedAsync(["Speed"], CancellationToken.None)).AnsweredRefusalGroups.ShouldBe(1); + (await clean.ReadDetailedAsync(["Pressure"], CancellationToken.None)).AnsweredRefusalGroups.ShouldBe(0); + } - // Design §3.6: the contract is one row per key; when a source breaks it the reader is at least - // deterministic — last occurrence in reader order — rather than silently arbitrary. - snapshots[0].Value.ShouldBe(99.0); + /// + /// R06:S-1. A key-value tag pointed at a history table (many rows per key) used to pull every + /// matching row into memory on every poll. The plan now asks for one row more than it has keys; a full + /// result set proves the selector is ambiguous, so every member publishes a configuration error and a + /// (rate-limited) warning names the table. The positive control: the same tag against a clean source + /// reads Good. + /// + [Fact] + public async Task ReadAsync_keyValueRowCapExceeded_publishesAConfigurationError_forEveryMember() + { + using var fixture = new SqlitePollFixture(); + var logger = new RecordingLogger(); + var clean = Reader(fixture, logger, KvTag("Speed", SqlitePollFixture.PresentKey)); + (await clean.ReadAsync(["Speed"], CancellationToken.None))[0].StatusCode.ShouldBe(SqlStatusCodes.Good); + logger.Entries.ShouldBeEmpty(); + + for (var i = 0; i < 50; i++) + { + fixture.Execute(string.Create(CultureInfo.InvariantCulture, + $"INSERT INTO {SqlitePollFixture.KeyValueTable} " + + $"({SqlitePollFixture.KeyColumn}, {SqlitePollFixture.ValueColumn}, {SqlitePollFixture.TimestampColumn}) " + + $"VALUES ('{SqlitePollFixture.PresentKey}', {i}, '2026-07-24T11:00:00Z')")); + } + + var reader = Reader(fixture, logger, KvTag("Speed", SqlitePollFixture.PresentKey), KvTag("Speed2", SqlitePollFixture.PresentKey)); + var snapshots = await reader.ReadAsync(["Speed", "Speed2"], CancellationToken.None); + + snapshots.ShouldAllBe(x => x.StatusCode == SqlStatusCodes.BadConfigurationError); + var warning = logger.Entries.ShouldHaveSingleItem(); + warning.Level.ShouldBe(LogLevel.Warning); + warning.Message.ShouldContain(SqlitePollFixture.KeyValueTable); + } + + /// + /// The client-side half of the cap: a dialect whose limit fragments are empty (a provider that cannot + /// spell one) must still stop reading at cap + 1 and report the same configuration error. + /// + [Fact] + public async Task ReadAsync_aDialectThatEmitsNoLimit_isStillCappedClientSide() + { + using var fixture = new SqlitePollFixture(); + AddSecondRowForPresentStation(fixture); + var reader = new SqlPollReader(fixture.Factory, fixture.ConnectionString, new NoLimitDialect(), + commandTimeout: CommandTimeout, operationTimeout: OperationTimeout, + maxConcurrentGroups: 4, nullIsBad: false, + resolve: Resolver(WideTag("Oven", "oven_temp", selectorValue: SqlitePollFixture.PresentStation))); + + var snapshots = await reader.ReadAsync(["Oven"], CancellationToken.None); + + snapshots[0].StatusCode.ShouldBe(SqlStatusCodes.BadConfigurationError); + } + + /// SQLite with the row-limit fragments blanked — the "provider cannot spell a limit" shape. + private sealed class NoLimitDialect : ISqlDialect + { + private readonly SqliteDialect _inner = new(); + public SqlProvider Provider => _inner.Provider; + public System.Data.Common.DbProviderFactory Factory => _inner.Factory; + public int MaxBindParameters => _inner.MaxBindParameters; + public string LivenessSql => _inner.LivenessSql; + public string SingleRowLimitPrefix => _inner.SingleRowLimitPrefix; + public string SingleRowLimitSuffix => _inner.SingleRowLimitSuffix; + public string RowLimitPrefix(int rows) => string.Empty; + public string RowLimitSuffix(int rows) => string.Empty; + public string ListSchemasSql => _inner.ListSchemasSql; + public string DefaultSchemaSql => _inner.DefaultSchemaSql; + public string ListTablesSql => _inner.ListTablesSql; + public string ListColumnsSql => _inner.ListColumnsSql; + public string QuoteIdentifier(string ident) => _inner.QuoteIdentifier(ident); + public string ParameterMarker(string baseName) => _inner.ParameterMarker(baseName); + public string ParameterBindName(string baseName) => _inner.ParameterBindName(baseName); + public void ConfigureCommand(System.Data.Common.DbCommand command) => _inner.ConfigureCommand(command); + public void ConfigureParameter(System.Data.Common.DbParameter parameter, object? value) => ((ISqlDialect)_inner).ConfigureParameter(parameter, value); + public DriverDataType MapColumnType(string sqlDataType) => _inner.MapColumnType(sqlDataType); } // ---- the frozen-peer contract ---- @@ -522,6 +639,180 @@ public sealed class SqlPollReaderTests dialect.Factory, " ", dialect, CommandTimeout, OperationTimeout, 4, false, _ => null)); } + // ---- R06:S-7: plans are cached per subscription reference list ---- + + /// + /// A poll loop hands the reader the SAME reference list every cycle; re-planning it (and rebuilding the + /// slot dictionary keyed by a 12-field record) every poll was pure per-poll garbage. Planned once, + /// and the cached plan still reads correct values every time — the positive control. + /// + [Fact] + public async Task ReadAsync_theSameReferenceList_isPlannedOnce() + { + using var fixture = new SqlitePollFixture(); + var reader = Reader(fixture, + KvTag("Speed", SqlitePollFixture.PresentKey), KvTag("Pressure", SqlitePollFixture.SecondPresentKey)); + IReadOnlyList refs = ["Speed", "Pressure"]; + + for (var i = 0; i < 3; i++) + { + var snapshots = await reader.ReadAsync(refs, CancellationToken.None); + snapshots[0].Value.ShouldBe(SqlitePollFixture.PresentValue); + snapshots[1].Value.ShouldBe(SqlitePollFixture.SecondPresentValue); + } + + reader.PlansBuiltForTests.ShouldBe(1); + } + + /// + /// The cache is invalidated by the definitions themselves: a catalog-gate swap (or any re-resolve) + /// hands back different definition instances, and the next poll re-plans against them. + /// + [Fact] + public async Task ReadAsync_whenAReferenceResolvesToADifferentDefinition_rePlans() + { + using var fixture = new SqlitePollFixture(); + var table = new Dictionary(StringComparer.Ordinal) + { + ["Speed"] = KvTag("Speed", SqlitePollFixture.PresentKey), + }; + var reader = new SqlPollReader(fixture.Factory, fixture.ConnectionString, new SqliteDialect(), + commandTimeout: CommandTimeout, operationTimeout: OperationTimeout, + maxConcurrentGroups: 4, nullIsBad: false, resolve: r => table.GetValueOrDefault(r)); + IReadOnlyList refs = ["Speed"]; + + (await reader.ReadAsync(refs, CancellationToken.None))[0].Value.ShouldBe(SqlitePollFixture.PresentValue); + table["Speed"] = KvTag("Speed", SqlitePollFixture.SecondPresentKey); // re-pointed by a new resolve + + (await reader.ReadAsync(refs, CancellationToken.None))[0].Value.ShouldBe(SqlitePollFixture.SecondPresentValue); + reader.PlansBuiltForTests.ShouldBe(2); + } + + // ---- R06:S-6: per-poll warnings are rate-limited ---- + + /// + /// A dropped table polled every second used to log a warning — with the full SQL text — on every + /// poll, per group, forever. One per minute per plan now; every poll still publishes the Bad status. + /// The first poll's warning is the positive control. + /// + [Fact] + public async Task ReadAsync_aPersistentlyFailingGroup_warnsOncePerInterval() + { + using var fixture = new SqlitePollFixture(); + var logger = new RecordingLogger(); + var reader = Reader(fixture, logger, + new SqlTagDefinition("Gone", SqlTagModel.KeyValue, "NoSuchTable", + KeyColumn: SqlitePollFixture.KeyColumn, KeyValue: "k", ValueColumn: SqlitePollFixture.ValueColumn)); + + for (var i = 0; i < 3; i++) + { + (await reader.ReadAsync(["Gone"], CancellationToken.None))[0] + .StatusCode.ShouldBe(SqlStatusCodes.BadCommunicationError); + } + + logger.Entries.Count(e => e.Level == LogLevel.Warning).ShouldBe(1); + } + + /// A cell that will not coerce is a per-tag refusal: one warning per tag, not one per poll. + [Fact] + public async Task ReadAsync_anUncoercibleCell_warnsOncePerTag() + { + using var fixture = new SqlitePollFixture(); + var logger = new RecordingLogger(); + var reader = Reader(fixture, logger, + KvTag("Speed", SqlitePollFixture.PresentKey, declaredType: DriverDataType.DateTime)); + + for (var i = 0; i < 3; i++) + { + (await reader.ReadAsync(["Speed"], CancellationToken.None))[0] + .StatusCode.ShouldBe(SqlStatusCodes.BadTypeMismatch); + } + + logger.Entries.Count(e => e.Level == LogLevel.Warning).ShouldBe(1); + } + + /// A frozen group's BadTimeout warning is rate-limited too; both polls still time out. + [Fact] + public async Task ReadAsync_aRepeatedlyTimedOutGroup_warnsOncePerInterval() + { + using var fixture = new SqlitePollFixture(); + var logger = new RecordingLogger(); + var reader = new SqlPollReader(fixture.Factory, fixture.ConnectionString, new SqliteDialect(), + commandTimeout: TimeSpan.FromSeconds(30), operationTimeout: TimeSpan.FromMilliseconds(500), + maxConcurrentGroups: 4, nullIsBad: false, + resolve: Resolver(KvTag("Speed", SqlitePollFixture.PresentKey)), logger: logger); + + var locker = fixture.OpenNewConnection(); + Execute(locker, "BEGIN EXCLUSIVE"); + try + { + (await reader.ReadAsync(["Speed"], CancellationToken.None))[0].StatusCode.ShouldBe(SqlStatusCodes.BadTimeout); + (await reader.ReadAsync(["Speed"], CancellationToken.None))[0].StatusCode.ShouldBe(SqlStatusCodes.BadTimeout); + } + finally + { + Execute(locker, "ROLLBACK"); + locker.Dispose(); + } + + logger.Entries.Count(e => e.Level == LogLevel.Warning && e.Message.Contains("operation timeout")) + .ShouldBe(1); + } + + // ---- R06:S-3: the concurrency gate is released ---- + + /// An idle reader releases its gate on Dispose, and a disposed reader starts no new group. + [Fact] + public async Task Dispose_releasesTheGate_andADisposedReaderRunsNoNewGroup() + { + using var fixture = new SqlitePollFixture(); + var reader = Reader(fixture, KvTag("Speed", SqlitePollFixture.PresentKey)); + (await reader.ReadAsync(["Speed"], CancellationToken.None))[0].StatusCode.ShouldBe(SqlStatusCodes.Good); + + reader.Dispose(); + reader.Dispose(); // idempotent + + reader.GateDisposedForTests.ShouldBeTrue(); + await Should.ThrowAsync(() => reader.ReadAsync(["Speed"], CancellationToken.None)); + } + + /// + /// Disposing while a timed-out group is still wedged inside the provider must NOT dispose the gate + /// under it — the zombie releases its slot into the gate when it finally finishes, which is exactly + /// the hazard that kept the gate undisposed before. The gate goes once the zombie is out. + /// + [Fact] + public async Task Dispose_withAWedgedGroupInFlight_waitsForItBeforeReleasingTheGate() + { + using var fixture = new SqlitePollFixture(); + var factory = new TrackingFactory(); + var reader = new SqlPollReader( + factory, fixture.ConnectionString, new SqliteDialect(), + commandTimeout: TimeSpan.FromSeconds(30), operationTimeout: TimeSpan.FromMilliseconds(500), + maxConcurrentGroups: 1, nullIsBad: false, + resolve: Resolver(KvTag("Speed", SqlitePollFixture.PresentKey))); + + var locker = fixture.OpenNewConnection(); + Execute(locker, "BEGIN EXCLUSIVE"); + try + { + (await reader.ReadAsync(["Speed"], CancellationToken.None))[0].StatusCode.ShouldBe(SqlStatusCodes.BadTimeout); + factory.Live.ShouldBe(1); // the zombie is still inside the provider — the control + + reader.Dispose(); + reader.GateDisposedForTests.ShouldBeFalse("a group still holds the gate"); + } + finally + { + Execute(locker, "ROLLBACK"); + locker.Dispose(); + } + + (await WaitUntilAsync(() => reader.GateDisposedForTests, TimeSpan.FromSeconds(30))) + .ShouldBeTrue("the last group out must release the gate"); + factory.Live.ShouldBe(0); + } + [Fact] public async Task ReadAsync_rejectsANullReferenceList() => await Should.ThrowAsync(async () => diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlReinitConfigAdoptionTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlReinitConfigAdoptionTests.cs index 1181de1a..12e2fc22 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlReinitConfigAdoptionTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlReinitConfigAdoptionTests.cs @@ -81,6 +81,11 @@ public sealed class SqlReinitConfigAdoptionTests "old-server", customMessage: "a reinitialize that could not resolve its new connection must leave the previous binding whole"); + + // Review m-4: the failure is a fault with a reason, never a latched Initializing with no LastError. + var health = driver.GetHealth(); + health.State.ShouldBe(Core.Abstractions.DriverState.Faulted); + health.LastError.ShouldNotBeNull().ShouldContain("could not adopt its configuration"); } /// An empty/placeholder document keeps the constructor-supplied binding, so the many lifecycle @@ -101,6 +106,43 @@ public sealed class SqlReinitConfigAdoptionTests driver.Endpoint.ShouldContain("old-server"); } + /// + /// R06:S-3. Every rebind builds a new ; the replaced one (and its + /// SemaphoreSlim) used to be dropped undisposed, one per reinitialize. The control: the reader + /// that replaced it is live. + /// + [Fact] + public async Task A_rebind_disposes_the_reader_it_replaces() + { + using var _ = new ScopedEnvironmentVariable( + SqlConnectionStringResolver.EnvironmentVariablePrefix + RefA, + "Server=old-server;Database=OldDb;Integrated Security=true"); + using var __ = new ScopedEnvironmentVariable( + SqlConnectionStringResolver.EnvironmentVariablePrefix + RefB, + "Server=new-server;Database=NewDb;Integrated Security=true"); + + var driver = SqlDriverFactoryExtensions.CreateInstance( + "sql-1", $$"""{"provider":"SqlServer","connectionStringRef":"{{RefA}}"}""", loggerFactory: null); + var original = driver.ReaderForTests; + + try + { + await driver.ReinitializeAsync( + $$"""{"provider":"SqlServer","connectionStringRef":"{{RefB}}"}""", CancellationToken.None); + } + catch + { + // Liveness failure against a non-existent server is expected; the rebind ran first. + } + + driver.ReaderForTests.ShouldNotBeSameAs(original); + original.GateDisposedForTests.ShouldBeTrue(); + driver.ReaderForTests.GateDisposedForTests.ShouldBeFalse(); + + await driver.DisposeAsync(); + driver.ReaderForTests.GateDisposedForTests.ShouldBeTrue("the final dispose releases the live reader too"); + } + /// Sets an environment variable for the duration of a test and restores it after. The resolver /// reads process-global state, so a leaked variable would race every sibling test. private sealed class ScopedEnvironmentVariable : IDisposable diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlServerDialectTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlServerDialectTests.cs index 901382f1..912a8ee6 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlServerDialectTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlServerDialectTests.cs @@ -13,6 +13,15 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests; /// public class SqlServerDialectTests { + [Fact] + public void RowLimit_isTopN_inThePrefixPosition() + { + var d = new SqlServerDialect(); + d.RowLimitPrefix(3).ShouldBe("TOP (3) "); + d.RowLimitSuffix(3).ShouldBe(""); + Should.Throw(() => d.RowLimitPrefix(0)); + } + [Fact] public void QuoteIdentifier_bracketsAndEscapesEmbeddedBrackets() { diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqliteDialect.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqliteDialect.cs index 62b9f3f4..20dded60 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqliteDialect.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqliteDialect.cs @@ -87,6 +87,20 @@ public sealed class SqliteDialect : ISqlDialect /// public string SingleRowLimitSuffix => " LIMIT 1"; + /// + public string RowLimitPrefix(int rows) + { + ArgumentOutOfRangeException.ThrowIfLessThan(rows, 1); + return string.Empty; + } + + /// + public string RowLimitSuffix(int rows) + { + ArgumentOutOfRangeException.ThrowIfLessThan(rows, 1); + return " LIMIT " + rows.ToString(System.Globalization.CultureInfo.InvariantCulture); + } + /// /// SQLite has no schema namespace, so the schema level is the single literal /// — enough to give the browser a real root to expand.