Files
lmxopcua/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorRediscoverySignalTests.cs
T
Joseph Doherty 39db75083d feat(drivers): every driver reports the authored tags it skipped (Gitea #541)
A driver builds its RawPath -> definition table at init and tolerates a
per-tag miss: an unmappable TagConfig, an unparseable address, a
capability-matrix rejection. That tolerance is right -- one bad tag must not
take down the other few thousand. What was missing is any statement of the
fact afterwards. A skip is not an error and happens before any read, so the
driver stays Healthy, errors/5 min stays 0, Last error stays empty, and Last
read advances live off the tags that did map. On the wonder Makino in #540
that reading held while focas-z-34184 ignored 20 of its 22 tags.

New optional capability ISkippedTagReporter, backed by a shared
SkippedTagLedger so the reset/commit/cap rules are one implementation rather
than one per driver. The report -- authored count, exact skipped count, and a
capped sample of RawPath + reason -- rides DriverHealthChanged to a new Tags
column on /hosts, over the Phase 5 telemetry stream. No table: a driver-only
node has no ConfigDb to write to, the lesson from #521.

The issue named eight drivers. It is twelve -- every driver in
DriverTypeNames.All has an authored-tag skip path. Calculation (no scriptId),
MTConnect (no readable DataItem id), Galaxy (no attributeRef) and OpcUaClient
(no nodeId) were unlisted, and OpcUaClient's was the quietest of the twelve:
NamespaceMap drops such an entry with a bare `continue` and not even a log
line. SkippedTagReporterFleetTests pins the invariant reflectively over every
IDriver in src/Drivers/, so a thirteenth cannot re-introduce the silence.

Sql drops a tag at TWO points and its report is the union of both: a blob
that does not map, AND an identifier the 8.1 catalog gate rejects against the
live catalog. The second is what schema drift produces -- a renamed column, a
dropped view -- months after a deployment sealed green, and reporting only
the first would have that driver claim "0 skipped" while polling half its
tags, which is worse than reporting nothing because the column would then be
actively reassuring.

Deliberately additive to driver state, never a change of it. Latching
non-Healthy for a config problem would satisfy both halves of the #527
comms-loss conjunction -- 90 s without a successful read AND non-Healthy --
and a driver whose tags were ALL skipped has no successful reads by
construction, so it would reconnect-loop forever against a healthy device.

Three details that are load-bearing rather than incidental:

- The skip digest is part of the publish fingerprint. A skipped tag leaves
  every other fingerprint component identical to a clean driver's, so without
  it PublishHealthSnapshot's dedup swallows exactly the publish carrying the
  signal -- the trap that already bit the rediscovery and host-status
  channels. The digest covers the sample, not just the counts: a redeploy
  that fixes one tag and breaks another leaves both counts unchanged.
- The count is exact; only the sample is capped, and the tooltip says so when
  it is showing fewer than it counted. A truncated list that looked complete
  would have an operator fix twenty tags and believe the job done.
- The ledger stages and commits, so an init that throws part-way leaves the
  previous report standing. A partial snapshot would read as "we now skip
  fewer tags", the opposite of the truth.

The proto field needs no companion presence flag, unlike host_statuses: a
singular message field carries real presence in proto3, and a has_ flag would
collide with protoc's generated HasSkippedTags.

Verified red-first three ways: dropping SkippedTagDigest from the fingerprint
fails the two dedup tests, removing the seam from one driver fails the fleet
guard, and dropping the catalog-gate leg fails the Sql union test. Full
solution builds with zero new warnings against 4e22f024.
2026-08-05 19:28:46 -04:00

247 lines
11 KiB
C#

using Akka.Actor;
using Shouldly;
using Xunit;
using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
using ZB.MOM.WW.OtOpcUa.Runtime.Drivers;
using ZB.MOM.WW.OtOpcUa.Runtime.Tests.Harness;
namespace ZB.MOM.WW.OtOpcUa.Runtime.Tests.Drivers;
/// <summary>
/// Covers the <see cref="IRediscoverable"/> consumer (Gitea #518). Nine drivers raise
/// <see cref="IRediscoverable.OnRediscoveryNeeded"/> when they observe that a remote's tag set may have
/// changed — a Galaxy redeploy, a TwinCAT symbol-version bump, an MTConnect agent restart, a Sparkplug
/// rebirth. Before this wiring NOTHING in <c>src/</c> subscribed, so every one of those raises went into
/// the void.
/// <para><b>The signal is advisory.</b> It does not rebuild the served address space: v3 authors raw tags
/// explicitly through the <c>/raw</c> browse-commit flow, so a runtime graft would create nodes no
/// operator approved. It rides the driver-health snapshot to the AdminUI as a re-browse prompt.</para>
/// </summary>
[Trait("Category", "Unit")]
public sealed class DriverInstanceActorRediscoverySignalTests : RuntimeActorTestBase
{
/// <summary>
/// The load-bearing case: a driver raises rediscovery while otherwise perfectly healthy, and the
/// signal reaches the health publisher carrying the driver's reason.
/// </summary>
[Fact]
public void Rediscovery_raise_reaches_the_health_publisher_with_its_reason()
{
var driver = new RediscoverableStubDriver();
var publisher = new RecordingHealthPublisher();
var parent = CreateTestProbe();
parent.IgnoreMessages(_ => true);
var actor = parent.ChildActorOf(DriverInstanceActor.Props(driver, healthPublisher: publisher));
actor.Tell(new DriverInstanceActor.InitializeRequested("{}"));
AwaitAssert(() => publisher.Published.Count.ShouldBeGreaterThan(0), TimeSpan.FromSeconds(3));
driver.RaiseRediscovery("deploy-time-changed");
AwaitAssert(
() =>
{
var withSignal = publisher.Published.LastOrDefault(p => p.RediscoveryNeededUtc is not null);
withSignal.ShouldNotBeNull();
withSignal!.RediscoveryReason.ShouldBe("deploy-time-changed");
},
TimeSpan.FromSeconds(3));
}
/// <summary>
/// <b>Regression guard for the dedup trap.</b> <c>PublishHealthSnapshot</c> suppresses a publish whose
/// (state, lastSuccessfulRead, lastError, errorCount) tuple repeats. A rediscovery raise on an
/// otherwise-unchanged Healthy driver leaves all four identical, so unless the rediscovery timestamp
/// is part of the fingerprint the dedup swallows the very publish carrying the signal and the operator
/// never sees the prompt.
/// <para>Positive control: the assertion is that the count STRICTLY INCREASES across the raise. An
/// "eventually non-null" assertion alone would pass on the warm-up publish and prove nothing. Revert
/// <c>_rediscoveryNeededUtc</c> out of the fingerprint tuple and this test must fail.</para>
/// </summary>
[Fact]
public void Rediscovery_raise_is_not_swallowed_by_the_unchanged_health_dedup()
{
var driver = new RediscoverableStubDriver();
var publisher = new RecordingHealthPublisher();
var parent = CreateTestProbe();
parent.IgnoreMessages(_ => true);
var actor = parent.ChildActorOf(DriverInstanceActor.Props(driver, healthPublisher: publisher));
actor.Tell(new DriverInstanceActor.InitializeRequested("{}"));
AwaitAssert(() => publisher.Published.Count.ShouldBeGreaterThan(0), TimeSpan.FromSeconds(3));
// Let the actor settle so nothing else is in flight, then capture the baseline. Everything about
// the driver's health is now stable — only the rediscovery flag will change.
ExpectNoMsg(TimeSpan.FromMilliseconds(200));
var before = publisher.Published.Count;
driver.RaiseRediscovery("symbol-version-changed");
AwaitAssert(
() => publisher.Published.Count.ShouldBeGreaterThan(before),
TimeSpan.FromSeconds(3));
publisher.Published[^1].RediscoveryNeededUtc.ShouldNotBeNull();
}
/// <summary>
/// The signal is sticky: a later health publish still carries it. The remote's tag set stayed
/// changed, and only an operator re-browsing resolves that — which this actor cannot observe. A
/// flag that cleared itself on the next heartbeat would be a prompt the operator could miss entirely.
/// </summary>
[Fact]
public void Rediscovery_flag_persists_on_subsequent_health_publishes()
{
var driver = new RediscoverableStubDriver();
var publisher = new RecordingHealthPublisher();
var parent = CreateTestProbe();
parent.IgnoreMessages(_ => true);
var actor = parent.ChildActorOf(DriverInstanceActor.Props(driver, healthPublisher: publisher));
actor.Tell(new DriverInstanceActor.InitializeRequested("{}"));
AwaitAssert(() => publisher.Published.Count.ShouldBeGreaterThan(0), TimeSpan.FromSeconds(3));
driver.RaiseRediscovery("agent-instance-changed");
AwaitAssert(
() => publisher.Published.Any(p => p.RediscoveryNeededUtc is not null),
TimeSpan.FromSeconds(3));
// Force a further publish by changing something else about the driver's health.
driver.SetLastError("transient read failure");
AwaitAssert(
() => publisher.Published.Any(p => p.LastError == "transient read failure"),
TimeSpan.FromSeconds(3));
var latest = publisher.Published[^1];
latest.RediscoveryNeededUtc.ShouldNotBeNull();
latest.RediscoveryReason.ShouldBe("agent-instance-changed");
}
/// <summary>
/// <b>Leak guard.</b> The <see cref="IDriver"/> instance can OUTLIVE the actor — the host stops a
/// child and respawns another around the same driver object — so a missing <c>-=</c> in
/// <c>PostStop</c> accumulates one handler per respawn, each holding a dead <c>Self</c>. Asserts the
/// driver's invocation list is empty again after the actor stops.
/// </summary>
[Fact]
public void Stopping_the_actor_detaches_the_rediscovery_handler()
{
var driver = new RediscoverableStubDriver();
var parent = CreateTestProbe();
parent.IgnoreMessages(_ => true);
var actor = parent.ChildActorOf(DriverInstanceActor.Props(driver));
actor.Tell(new DriverInstanceActor.InitializeRequested("{}"));
AwaitAssert(() => driver.SubscriberCount.ShouldBe(1), TimeSpan.FromSeconds(3));
Watch(actor);
actor.Tell(PoisonPill.Instance);
ExpectTerminated(actor, TimeSpan.FromSeconds(3));
driver.SubscriberCount.ShouldBe(0);
}
/// <summary>A driver that does NOT implement <see cref="IRediscoverable"/> must publish a null flag —
/// the field is absent, not defaulted to "needs re-browse".</summary>
[Fact]
public void A_non_rediscoverable_driver_never_sets_the_flag()
{
var publisher = new RecordingHealthPublisher();
var parent = CreateTestProbe();
parent.IgnoreMessages(_ => true);
var actor = parent.ChildActorOf(DriverInstanceActor.Props(new StubDriver(), healthPublisher: publisher));
actor.Tell(new DriverInstanceActor.InitializeRequested("{}"));
AwaitAssert(() => publisher.Published.Count.ShouldBeGreaterThan(0), TimeSpan.FromSeconds(3));
publisher.Published.ShouldAllBe(p => p.RediscoveryNeededUtc == null);
}
/// <summary>Captures every health publish so a test can assert on the rediscovery fields.</summary>
private sealed record HealthPublish(
string ClusterId,
string DriverInstanceId,
DriverHealth Health,
int ErrorCount5Min,
DateTime? RediscoveryNeededUtc,
string? RediscoveryReason)
{
public string? LastError => Health.LastError;
}
private sealed class RecordingHealthPublisher : IDriverHealthPublisher
{
private readonly List<HealthPublish> _published = [];
/// <summary>Thread-safe snapshot — <c>Publish</c> is called from the actor thread while the test
/// asserts from its own.</summary>
public IReadOnlyList<HealthPublish> Published
{
get { lock (_published) return _published.ToArray(); }
}
/// <inheritdoc />
public void Publish(
string clusterId,
string driverInstanceId,
DriverHealth health,
int errorCount5Min,
DateTime? rediscoveryNeededUtc = null,
string? rediscoveryReason = null,
IReadOnlyList<HostConnectivityStatus>? hostStatuses = null,
SkippedTagReport? skippedTags = null)
{
lock (_published)
{
_published.Add(new HealthPublish(
clusterId, driverInstanceId, health, errorCount5Min, rediscoveryNeededUtc, rediscoveryReason));
}
}
}
/// <summary>
/// A stub driver exposing <see cref="IRediscoverable"/>, a hook to raise it, and the live subscriber
/// count (for the detach guard).
/// <para><b>Implemented from scratch rather than derived from the shared <c>StubDriver</c> on
/// purpose.</b> That one returns <c>GetHealth() =&gt; new(Healthy, DateTime.UtcNow, null)</c>, so its
/// health fingerprint differs on EVERY call and the dedup under test never engages — which would make
/// <see cref="Rediscovery_raise_is_not_swallowed_by_the_unchanged_health_dedup"/> pass vacuously
/// whether or not the fix is present. This stub's health is STABLE until a test changes it.</para>
/// </summary>
private sealed class RediscoverableStubDriver : IDriver, IRediscoverable
{
private static readonly DateTime FixedLastRead = new(2026, 7, 27, 12, 0, 0, DateTimeKind.Utc);
private volatile string? _lastError;
/// <inheritdoc />
public event EventHandler<RediscoveryEventArgs>? OnRediscoveryNeeded;
/// <inheritdoc />
public string DriverInstanceId => "rediscoverable-stub-1";
/// <inheritdoc />
public string DriverType => "Stub";
/// <summary>Number of live subscribers on <see cref="OnRediscoveryNeeded"/>.</summary>
public int SubscriberCount => OnRediscoveryNeeded?.GetInvocationList().Length ?? 0;
/// <summary>Raises the event exactly as a real driver's watcher does.</summary>
public void RaiseRediscovery(string reason)
=> OnRediscoveryNeeded?.Invoke(this, new RediscoveryEventArgs(reason, ScopeHint: null));
/// <summary>Changes the reported health so a test can force a publish that is NOT the rediscovery one.</summary>
public void SetLastError(string error) => _lastError = error;
/// <inheritdoc />
public Task InitializeAsync(string driverConfigJson, CancellationToken cancellationToken) => Task.CompletedTask;
/// <inheritdoc />
public Task ReinitializeAsync(string driverConfigJson, CancellationToken cancellationToken) => Task.CompletedTask;
/// <inheritdoc />
public Task ShutdownAsync(CancellationToken cancellationToken) => Task.CompletedTask;
/// <inheritdoc />
public DriverHealth GetHealth() => new(DriverState.Healthy, FixedLastRead, _lastError);
}
}