refactor(#655): delete the dead driver-side history capability
HistoryRead is served server-side by IHistorianDataSource; nothing ever dispatched to a driver's IHistoryProvider. Removes IHistoryProvider, the OpcUaClient history passthrough, DriverCapability.HistoryRead (+ resilience tier rows and AdminUI row), HistoryReadStatusException and its node-manager catches, and the tests that only covered them. Shared result types and HistoryAggregateNotSupportedException stay (moved to Historian/HistoryReadContracts.cs). A stored ResilienceConfig still carrying a HistoryRead key parses (diagnostic only; new test). Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe
This commit is contained in:
+2
-3
@@ -16,9 +16,8 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.IntegrationTests;
|
||||
/// <para>
|
||||
/// <b>Why opc-plc over loopback against our own server</b> — (1) independent
|
||||
/// cert chain + user-token handling catches interop bugs loopback can't;
|
||||
/// (2) built-in alarm ConditionType + history simulation gives
|
||||
/// <see cref="Core.Abstractions.IAlarmSource"/> +
|
||||
/// <see cref="Core.Abstractions.IHistoryProvider"/> coverage without a custom
|
||||
/// (2) built-in alarm ConditionType simulation gives
|
||||
/// <see cref="Core.Abstractions.IAlarmSource"/> coverage without a custom
|
||||
/// driver fake; (3) pinned image tag fixes the test surface in a way our own
|
||||
/// evolving server wouldn't. Follow-up: add <c>open62541/open62541</c> as a
|
||||
/// second image once this lands, for fully-independent-stack interop.
|
||||
|
||||
-28
@@ -92,32 +92,4 @@ public sealed class OpcUaClientSmokeTests(OpcPlcFixture sim)
|
||||
|
||||
await drv.UnsubscribeAsync(handle, TestContext.Current.CancellationToken);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Verifies HistoryReadEvents passthrough issues a well-formed request and returns a
|
||||
/// result without throwing. opc-plc exposes live alarm conditions (--alm) but is NOT a
|
||||
/// historian, so the upstream may return zero historical events or reject the service —
|
||||
/// either way the driver must produce a HistoricalEventsResult, never throw. This proves
|
||||
/// the wire request + unwrap path; a non-empty event list is infra-gated on an upstream
|
||||
/// that historizes events.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public async Task Client_reads_events_returns_result_without_throwing()
|
||||
{
|
||||
if (sim.SkipReason is not null) Assert.Skip(sim.SkipReason);
|
||||
|
||||
var options = OpcPlcProfile.BuildOptions(sim.EndpointUrl);
|
||||
await using var drv = new OpcUaClientDriver(options, driverInstanceId: "opcua-smoke-events");
|
||||
await drv.InitializeAsync("{}", TestContext.Current.CancellationToken);
|
||||
|
||||
var result = await drv.ReadEventsAsync(
|
||||
sourceName: null, // null → upstream Server object (i=2253)
|
||||
startUtc: DateTime.UtcNow.AddHours(-1),
|
||||
endUtc: DateTime.UtcNow,
|
||||
maxEvents: 100,
|
||||
cancellationToken: TestContext.Current.CancellationToken);
|
||||
|
||||
result.ShouldNotBeNull();
|
||||
result.Events.ShouldNotBeNull();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,178 +0,0 @@
|
||||
using Opc.Ua;
|
||||
using Shouldly;
|
||||
using Xunit;
|
||||
using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
|
||||
|
||||
namespace ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests;
|
||||
|
||||
[Trait("Category", "Unit")]
|
||||
public sealed class OpcUaClientHistoryTests
|
||||
{
|
||||
/// <summary>Every <see cref="HistoryAggregateType"/> member (#285: the full Part 13 set, not just five).</summary>
|
||||
public static IEnumerable<object[]> AllAggregates() =>
|
||||
Enum.GetValues<HistoryAggregateType>().Select(a => new object[] { a });
|
||||
|
||||
/// <summary>Every member maps to a DISTINCT aggregate NodeId (a copy/paste mapping would collide).</summary>
|
||||
[Fact]
|
||||
public void MapAggregateToNodeId_is_injective_over_the_whole_enum()
|
||||
{
|
||||
var ids = Enum.GetValues<HistoryAggregateType>().Select(OpcUaClientDriver.MapAggregateToNodeId).ToList();
|
||||
ids.Distinct().Count().ShouldBe(ids.Count);
|
||||
Enum.GetValues<HistoryAggregateType>().Length.ShouldBeGreaterThan(5);
|
||||
}
|
||||
|
||||
/// <summary>Verifies MapAggregateToNodeId returns standard Part 13 aggregate for every enum.</summary>
|
||||
/// <param name="agg">The history aggregate type to test.</param>
|
||||
[Theory]
|
||||
[MemberData(nameof(AllAggregates))]
|
||||
public void MapAggregateToNodeId_returns_standard_Part13_aggregate_for_every_enum(HistoryAggregateType agg)
|
||||
{
|
||||
var nodeId = OpcUaClientDriver.MapAggregateToNodeId(agg);
|
||||
NodeId.IsNull(nodeId).ShouldBeFalse();
|
||||
// Every mapping should resolve to an AggregateFunction_* NodeId (namespace 0, numeric id).
|
||||
nodeId.NamespaceIndex.ShouldBe((ushort)0);
|
||||
}
|
||||
|
||||
/// <summary>Verifies MapAggregateToNodeId rejects invalid enum values.</summary>
|
||||
[Fact]
|
||||
public void MapAggregateToNodeId_rejects_invalid_enum_value()
|
||||
{
|
||||
// Defense-in-depth: a future HistoryAggregateType addition mustn't silently fall through.
|
||||
Should.Throw<ArgumentOutOfRangeException>(() =>
|
||||
OpcUaClientDriver.MapAggregateToNodeId((HistoryAggregateType)99));
|
||||
}
|
||||
|
||||
/// <summary>Verifies ReadRawAsync throws without initialization.</summary>
|
||||
[Fact]
|
||||
public async Task ReadRawAsync_without_initialize_throws_InvalidOperationException()
|
||||
{
|
||||
using var drv = new OpcUaClientDriver(new OpcUaClientDriverOptions(), "opcua-hist-uninit");
|
||||
await Should.ThrowAsync<InvalidOperationException>(async () =>
|
||||
await drv.ReadRawAsync("ns=2;s=Counter",
|
||||
DateTime.UtcNow.AddMinutes(-5), DateTime.UtcNow, 1000,
|
||||
TestContext.Current.CancellationToken));
|
||||
}
|
||||
|
||||
/// <summary>Verifies ReadRawAsync with malformed NodeId returns empty result.</summary>
|
||||
[Fact]
|
||||
public async Task ReadRawAsync_with_malformed_NodeId_returns_empty_result_not_throw()
|
||||
{
|
||||
// Same defensive pattern as ReadAsync / WriteAsync — malformed NodeId short-circuits
|
||||
// to an empty result rather than crashing a batch history call. Needs init via the
|
||||
// throw path first, then we pass "" to trigger the parse-fail branch inside
|
||||
// ExecuteHistoryReadAsync. The init itself fails against 127.0.0.1:1 so we stop there.
|
||||
// Not runnable without init — keep as placeholder for when the in-process fixture
|
||||
// PR lands.
|
||||
await Task.CompletedTask;
|
||||
}
|
||||
|
||||
/// <summary>Verifies ReadProcessedAsync throws without initialization.</summary>
|
||||
[Fact]
|
||||
public async Task ReadProcessedAsync_without_initialize_throws_InvalidOperationException()
|
||||
{
|
||||
using var drv = new OpcUaClientDriver(new OpcUaClientDriverOptions(), "opcua-hist-uninit");
|
||||
await Should.ThrowAsync<InvalidOperationException>(async () =>
|
||||
await drv.ReadProcessedAsync("ns=2;s=Counter",
|
||||
DateTime.UtcNow.AddMinutes(-5), DateTime.UtcNow,
|
||||
TimeSpan.FromSeconds(10), HistoryAggregateType.Average,
|
||||
TestContext.Current.CancellationToken));
|
||||
}
|
||||
|
||||
/// <summary>Verifies ReadAtTimeAsync throws without initialization.</summary>
|
||||
[Fact]
|
||||
public async Task ReadAtTimeAsync_without_initialize_throws_InvalidOperationException()
|
||||
{
|
||||
using var drv = new OpcUaClientDriver(new OpcUaClientDriverOptions(), "opcua-hist-uninit");
|
||||
await Should.ThrowAsync<InvalidOperationException>(async () =>
|
||||
await drv.ReadAtTimeAsync("ns=2;s=Counter",
|
||||
[DateTime.UtcNow.AddMinutes(-5), DateTime.UtcNow],
|
||||
TestContext.Current.CancellationToken));
|
||||
}
|
||||
|
||||
/// <summary>ReadEventsAsync requires an initialized session like the sibling history reads.</summary>
|
||||
[Fact]
|
||||
public async Task ReadEventsAsync_without_initialize_throws_InvalidOperationException()
|
||||
{
|
||||
using var drv = new OpcUaClientDriver(new OpcUaClientDriverOptions(), "opcua-events-uninit");
|
||||
await Should.ThrowAsync<InvalidOperationException>(async () =>
|
||||
await drv.ReadEventsAsync(
|
||||
sourceName: null,
|
||||
startUtc: DateTime.UtcNow.AddMinutes(-5),
|
||||
endUtc: DateTime.UtcNow,
|
||||
maxEvents: 100,
|
||||
cancellationToken: TestContext.Current.CancellationToken));
|
||||
}
|
||||
|
||||
/// <summary>BuildBaseEventFilter emits the six canonical BaseEventType select clauses in order.</summary>
|
||||
[Fact]
|
||||
public void BuildBaseEventFilter_has_six_canonical_BaseEventType_value_clauses()
|
||||
{
|
||||
var filter = OpcUaClientDriver.BuildBaseEventFilter();
|
||||
|
||||
filter.SelectClauses.Count.ShouldBe(6);
|
||||
string[] expected = ["EventId", "SourceName", "Time", "ReceiveTime", "Message", "Severity"];
|
||||
for (var i = 0; i < expected.Length; i++)
|
||||
{
|
||||
var clause = filter.SelectClauses[i];
|
||||
clause.TypeDefinitionId.ShouldBe(ObjectTypeIds.BaseEventType);
|
||||
clause.AttributeId.ShouldBe(Attributes.Value);
|
||||
clause.BrowsePath.Count.ShouldBe(1);
|
||||
clause.BrowsePath[0].Name.ShouldBe(expected[i]);
|
||||
clause.BrowsePath[0].NamespaceIndex.ShouldBe((ushort)0); // BaseEventType fields live in ns 0
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>MapHistoryEvents maps every field by its canonical select-clause index.</summary>
|
||||
[Fact]
|
||||
public void MapHistoryEvents_maps_all_six_fields_by_canonical_index()
|
||||
{
|
||||
var eventTime = new DateTime(2026, 6, 18, 10, 0, 0, DateTimeKind.Utc);
|
||||
var recvTime = eventTime.AddSeconds(1);
|
||||
var he = new HistoryEvent();
|
||||
var fields = new HistoryEventFieldList();
|
||||
fields.EventFields.Add(new Variant(new byte[] { 1, 2, 3 })); // 0 EventId
|
||||
fields.EventFields.Add(new Variant("Pump17")); // 1 SourceName
|
||||
fields.EventFields.Add(new Variant(eventTime)); // 2 Time
|
||||
fields.EventFields.Add(new Variant(recvTime)); // 3 ReceiveTime
|
||||
fields.EventFields.Add(new Variant(new LocalizedText("High temp"))); // 4 Message
|
||||
fields.EventFields.Add(new Variant((ushort)700)); // 5 Severity
|
||||
he.Events.Add(fields);
|
||||
|
||||
var mapped = OpcUaClientDriver.MapHistoryEvents(he);
|
||||
|
||||
mapped.Count.ShouldBe(1);
|
||||
var e = mapped[0];
|
||||
e.EventId.ShouldBe(Convert.ToBase64String(new byte[] { 1, 2, 3 }));
|
||||
e.SourceName.ShouldBe("Pump17");
|
||||
e.EventTimeUtc.ShouldBe(eventTime);
|
||||
e.ReceivedTimeUtc.ShouldBe(recvTime);
|
||||
e.Message.ShouldBe("High temp");
|
||||
e.Severity.ShouldBe((ushort)700);
|
||||
}
|
||||
|
||||
/// <summary>MapHistoryEvents returns empty for a HistoryEvent with no rows.</summary>
|
||||
[Fact]
|
||||
public void MapHistoryEvents_with_no_events_returns_empty()
|
||||
{
|
||||
OpcUaClientDriver.MapHistoryEvents(new HistoryEvent()).ShouldBeEmpty();
|
||||
}
|
||||
|
||||
/// <summary>MapHistoryEvents tolerates a field list shorter than six without throwing.</summary>
|
||||
[Fact]
|
||||
public void MapHistoryEvents_tolerates_short_field_list_without_throwing()
|
||||
{
|
||||
var he = new HistoryEvent();
|
||||
var fields = new HistoryEventFieldList();
|
||||
fields.EventFields.Add(new Variant(new byte[] { 9 })); // only EventId present
|
||||
he.Events.Add(fields);
|
||||
|
||||
var mapped = OpcUaClientDriver.MapHistoryEvents(he);
|
||||
|
||||
mapped.Count.ShouldBe(1);
|
||||
mapped[0].EventId.ShouldBe(Convert.ToBase64String(new byte[] { 9 }));
|
||||
mapped[0].SourceName.ShouldBeNull();
|
||||
mapped[0].EventTimeUtc.ShouldBe(DateTime.MinValue);
|
||||
mapped[0].EventTimeUtc.Kind.ShouldBe(DateTimeKind.Utc); // sentinel is UTC-kinded, not Unspecified
|
||||
mapped[0].Severity.ShouldBe((ushort)0);
|
||||
}
|
||||
}
|
||||
+3
-106
@@ -8,13 +8,13 @@ using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
|
||||
namespace ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests;
|
||||
|
||||
/// <summary>
|
||||
/// Regression coverage for the stale-session concurrency race in the history/read wire
|
||||
/// paths. A background reconnect handler can swap <see cref="OpcUaClientDriver.Session"/>
|
||||
/// Regression coverage for the stale-session concurrency race in the alarm-acknowledge wire
|
||||
/// path. A background reconnect handler can swap <see cref="OpcUaClientDriver.Session"/>
|
||||
/// while a caller is parked on <c>_gate.WaitAsync</c>. If a method captures the session
|
||||
/// reference <i>before</i> the gate and uses it afterwards, it issues the wire call against
|
||||
/// the closed/stale session. The correct idiom — already used by ReadAsync/WriteAsync — is
|
||||
/// a fast-fail guard outside the gate plus an authoritative re-read of <c>Session</c>
|
||||
/// <i>inside</i> the gate. These tests prove the history/read funnels follow it.
|
||||
/// <i>inside</i> the gate. This test proves the acknowledge path follows it.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// The swap is staged so the race is exercised, not raced against:
|
||||
@@ -41,13 +41,6 @@ public sealed class OpcUaClientStaleSessionRaceTests
|
||||
{
|
||||
private static Mock<ISession> NewSessionMock(TaskCompletionSource? prologueReached = null)
|
||||
{
|
||||
var resp = new HistoryReadResponse
|
||||
{
|
||||
ResponseHeader = new ResponseHeader(),
|
||||
Results = new HistoryReadResultCollection(),
|
||||
DiagnosticInfos = new DiagnosticInfoCollection(),
|
||||
};
|
||||
|
||||
var mock = new Mock<ISession>(MockBehavior.Loose);
|
||||
// NodeId parsing inside the driver resolves the reference against the session's
|
||||
// MessageContext. This getter is read in the driver's synchronous prologue (before the
|
||||
@@ -56,14 +49,6 @@ public sealed class OpcUaClientStaleSessionRaceTests
|
||||
.Returns(new ServiceMessageContext(ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.SdkTelemetry.Context))
|
||||
.Callback(() => prologueReached?.TrySetResult());
|
||||
mock.SetupGet(s => s.NamespaceUris).Returns(new NamespaceTable());
|
||||
mock.Setup(s => s.HistoryReadAsync(
|
||||
It.IsAny<RequestHeader>(),
|
||||
It.IsAny<ExtensionObject>(),
|
||||
It.IsAny<TimestampsToReturn>(),
|
||||
It.IsAny<bool>(),
|
||||
It.IsAny<HistoryReadValueIdCollection>(),
|
||||
It.IsAny<CancellationToken>()))
|
||||
.ReturnsAsync(resp);
|
||||
// Acknowledge issues its wire call via CallAsync; stub it so AcknowledgeAsync can run to
|
||||
// completion against whichever session it ends up binding to.
|
||||
mock.Setup(s => s.CallAsync(
|
||||
@@ -79,18 +64,6 @@ public sealed class OpcUaClientStaleSessionRaceTests
|
||||
return mock;
|
||||
}
|
||||
|
||||
private static void VerifyWireHitNewSessionNotOld(Mock<ISession> newSession, Mock<ISession> oldSession)
|
||||
{
|
||||
newSession.Verify(s => s.HistoryReadAsync(
|
||||
It.IsAny<RequestHeader>(), It.IsAny<ExtensionObject>(), It.IsAny<TimestampsToReturn>(),
|
||||
It.IsAny<bool>(), It.IsAny<HistoryReadValueIdCollection>(), It.IsAny<CancellationToken>()),
|
||||
Times.Once);
|
||||
oldSession.Verify(s => s.HistoryReadAsync(
|
||||
It.IsAny<RequestHeader>(), It.IsAny<ExtensionObject>(), It.IsAny<TimestampsToReturn>(),
|
||||
It.IsAny<bool>(), It.IsAny<HistoryReadValueIdCollection>(), It.IsAny<CancellationToken>()),
|
||||
Times.Never);
|
||||
}
|
||||
|
||||
private static void VerifyAckWireHitNewSessionNotOld(Mock<ISession> newSession, Mock<ISession> oldSession)
|
||||
{
|
||||
newSession.Verify(s => s.CallAsync(
|
||||
@@ -101,82 +74,6 @@ public sealed class OpcUaClientStaleSessionRaceTests
|
||||
Times.Never);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// ExecuteHistoryReadAsync (the ReadRaw/ReadProcessed/ReadAtTime funnel) must issue the
|
||||
/// HistoryRead against the session current <i>after</i> the gate, not a reference
|
||||
/// captured before WaitAsync.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public async Task ReadRawAsync_uses_session_swapped_in_across_the_gate_not_the_captured_one()
|
||||
{
|
||||
var ct = TestContext.Current.CancellationToken;
|
||||
// v3: a HistoryRead over a variable is keyed by the tag's RawPath, resolved through the
|
||||
// authored RawTags table to its upstream node id. Author "ns=2;s=Counter" as both the
|
||||
// RawPath and the TagConfig.nodeId so the two-stage resolve reaches the wire.
|
||||
var options = new OpcUaClientDriverOptions
|
||||
{
|
||||
RawTags = [new RawTagEntry("ns=2;s=Counter", """{"nodeId":"ns=2;s=Counter"}""", WriteIdempotent: false)],
|
||||
};
|
||||
using var drv = new OpcUaClientDriver(options, "opcua-stale-raw");
|
||||
drv.SetNamespaceMapForTest(NamespaceMap.FromTable(new NamespaceTable(), options.RawTags));
|
||||
|
||||
var prologueReached = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
|
||||
var oldSession = NewSessionMock(prologueReached);
|
||||
var newSession = NewSessionMock();
|
||||
drv.SetSessionForTest(oldSession.Object);
|
||||
|
||||
// Hold the gate so the method parks on WaitAsync after running its synchronous prologue.
|
||||
await drv.Gate.WaitAsync(ct);
|
||||
|
||||
var call = drv.ReadRawAsync(
|
||||
"ns=2;s=Counter", DateTime.UtcNow.AddMinutes(-5), DateTime.UtcNow, 1000, ct);
|
||||
|
||||
// Wait until the buggy capture-before-gate has grabbed the OLD session (signalled via
|
||||
// its MessageContext read), then stage the reconnect swap and release the gate. The
|
||||
// fixed path reads no pre-gate session member, so the signal never fires and the
|
||||
// bounded fallback elapses — harmless, because the fixed read happens only after the
|
||||
// gate is released and therefore always sees the swapped session.
|
||||
await WaitForPreGateCaptureOrFallbackAsync(prologueReached.Task, ct);
|
||||
drv.SetSessionForTest(newSession.Object);
|
||||
drv.Gate.Release();
|
||||
|
||||
await call;
|
||||
|
||||
VerifyWireHitNewSessionNotOld(newSession, oldSession);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// ReadEventsAsync is a separate HistoryRead site with the same hazard — it must also
|
||||
/// re-read Session inside the gate rather than use the pre-gate capture.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public async Task ReadEventsAsync_uses_session_swapped_in_across_the_gate_not_the_captured_one()
|
||||
{
|
||||
var ct = TestContext.Current.CancellationToken;
|
||||
using var drv = new OpcUaClientDriver(new OpcUaClientDriverOptions(), "opcua-stale-events");
|
||||
|
||||
var prologueReached = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
|
||||
// ReadEventsAsync only parses the source NodeId when sourceName is non-empty, so pass a
|
||||
// concrete source id to force the prologue's MessageContext read (and its signal).
|
||||
var oldSession = NewSessionMock(prologueReached);
|
||||
var newSession = NewSessionMock();
|
||||
drv.SetSessionForTest(oldSession.Object);
|
||||
|
||||
await drv.Gate.WaitAsync(ct);
|
||||
|
||||
var call = drv.ReadEventsAsync(
|
||||
sourceName: "ns=2;s=Pump17", startUtc: DateTime.UtcNow.AddMinutes(-5),
|
||||
endUtc: DateTime.UtcNow, maxEvents: 100, cancellationToken: ct);
|
||||
|
||||
await WaitForPreGateCaptureOrFallbackAsync(prologueReached.Task, ct);
|
||||
drv.SetSessionForTest(newSession.Object);
|
||||
drv.Gate.Release();
|
||||
|
||||
await call;
|
||||
|
||||
VerifyWireHitNewSessionNotOld(newSession, oldSession);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// AcknowledgeAsync issues its Acknowledge method <c>CallAsync</c> against the session
|
||||
/// current after the gate — and parses the namespace-relative ConditionId against that
|
||||
|
||||
Reference in New Issue
Block a user