test(historian): multi-node HistoryRead isolation + single-lookup ServeNode + comment fix
Fix A: add Raw_multi_node_per_node_error_isolation test — two historized variables (eqA/good→A.PV, eqB/bad→B.PV) in one Raw batch; per-tagname fake throws for B.PV, returns a sample for A.PV; asserts errors[0]=Good+sample, errors[1]=Bad, HistoryData[1]=null (no cross-slot leak), no exception escapes. Fix B: collapse double ConcurrentDictionary lookup in ServeNode — TryGetHistorizedTagname now captures `out var tagname` on the guard; the resolved tagname is threaded into the read callback as a second parameter (Func<IHistorianDataSource, string, Task<HistorianRead>>), removing the redundant ResolveTagname helper (deleted) and the tiny race window between the check and the second lookup. All three call-sites (Raw/Processed/AtTime) updated. Fix C: rewrite the IsReadModified comment at NodeManagerHistoryReadTests.cs:102 — the SDK's ReadRawModifiedDetails.Initialize() sets m_isReadModified=true (generated ctor body in Opc.Ua.DataTypes.cs), so the default IS true; the test must explicitly clear it to false for a plain raw read. Previous comment said the same thing but imprecisely; now cites the SDK mechanism (Initialize() call in the public ctor).
This commit is contained in:
Joseph Doherty
@@ -1050,8 +1050,8 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
|
||||
continue;
|
||||
}
|
||||
|
||||
ServeNode(handle, results, errors, source => source.ReadRawAsync(
|
||||
ResolveTagname(handle),
|
||||
ServeNode(handle, results, errors, (source, tagname) => source.ReadRawAsync(
|
||||
tagname,
|
||||
details.StartTime,
|
||||
details.EndTime,
|
||||
details.NumValuesPerNode,
|
||||
@@ -1088,8 +1088,8 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
|
||||
continue;
|
||||
}
|
||||
|
||||
ServeNode(handle, results, errors, source => source.ReadProcessedAsync(
|
||||
ResolveTagname(handle),
|
||||
ServeNode(handle, results, errors, (source, tagname) => source.ReadProcessedAsync(
|
||||
tagname,
|
||||
details.StartTime,
|
||||
details.EndTime,
|
||||
// OPC UA ProcessingInterval is a Duration in milliseconds.
|
||||
@@ -1117,8 +1117,8 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
|
||||
var timestamps = details.ReqTimes?.ToList() ?? new List<DateTime>();
|
||||
foreach (var handle in nodesToProcess)
|
||||
{
|
||||
ServeNode(handle, results, errors, source => source.ReadAtTimeAsync(
|
||||
ResolveTagname(handle),
|
||||
ServeNode(handle, results, errors, (source, tagname) => source.ReadAtTimeAsync(
|
||||
tagname,
|
||||
timestamps,
|
||||
CancellationToken.None));
|
||||
}
|
||||
@@ -1126,25 +1126,28 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
|
||||
|
||||
/// <summary>
|
||||
/// Block-bridge to the historian source for one node handle and project the result onto the
|
||||
/// service-level results/errors slots. Resolves the node's registered historian tagname first
|
||||
/// (a node we don't recognise as historized — which shouldn't reach us, since the base only
|
||||
/// hands us nodes with the HistoryRead access bit — maps to <c>BadHistoryOperationUnsupported</c>).
|
||||
/// The <paramref name="read"/> callback is invoked only AFTER the tagname is confirmed present;
|
||||
/// it is wrapped in try/catch so a backend throw / timeout becomes a Bad status for THIS node
|
||||
/// without throwing out of the batch.
|
||||
/// service-level results/errors slots. Resolves the node's registered historian tagname first —
|
||||
/// a single <see cref="TryGetHistorizedTagname"/> lookup; the resolved tagname is passed directly
|
||||
/// to <paramref name="read"/>, removing any risk of a second concurrent lookup on the same key.
|
||||
/// A node we don't recognise as historized maps to <c>BadHistoryOperationUnsupported</c>
|
||||
/// (shouldn't normally reach us, since the base only hands us nodes with the HistoryRead access
|
||||
/// bit, but we guard explicitly). The <paramref name="read"/> callback receives the resolved
|
||||
/// tagname and is wrapped in try/catch so a backend throw / timeout becomes a Bad status for
|
||||
/// THIS node without throwing out of the batch.
|
||||
/// </summary>
|
||||
/// <param name="handle">The pre-filtered node handle to serve; <c>handle.Index</c> indexes results/errors.</param>
|
||||
/// <param name="results">The service-level results list to fill at <c>handle.Index</c>.</param>
|
||||
/// <param name="errors">The service-level errors list to fill at <c>handle.Index</c>.</param>
|
||||
/// <param name="read">Invokes the resolved data-source read; only called once the tagname is known.</param>
|
||||
/// <param name="read">Invokes the resolved data-source read with the resolved tagname; only called
|
||||
/// once the tagname is confirmed present.</param>
|
||||
private void ServeNode(
|
||||
NodeHandle handle,
|
||||
IList<SdkHistoryReadResult> results,
|
||||
IList<ServiceResult> errors,
|
||||
Func<IHistorianDataSource, Task<HistorianRead>> read)
|
||||
Func<IHistorianDataSource, string, Task<HistorianRead>> read)
|
||||
{
|
||||
var idString = handle.NodeId.Identifier?.ToString();
|
||||
if (idString is null || !TryGetHistorizedTagname(idString, out _))
|
||||
if (idString is null || !TryGetHistorizedTagname(idString, out var tagname))
|
||||
{
|
||||
// Not a historized node we own a tagname for — unsupported. (The base pre-seeds this same
|
||||
// status, but set it explicitly so the contract is local + obvious.)
|
||||
@@ -1156,7 +1159,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
|
||||
{
|
||||
// HistoryRead is NOT invoked under the node-manager Lock (unlike OnWriteValue), so blocking
|
||||
// on the async source here is safe and won't freeze the address space.
|
||||
var sourceResult = read(HistorianDataSource).GetAwaiter().GetResult();
|
||||
var sourceResult = read(HistorianDataSource, tagname!).GetAwaiter().GetResult();
|
||||
var historyData = ToHistoryData(sourceResult);
|
||||
|
||||
results[handle.Index] = new SdkHistoryReadResult
|
||||
@@ -1183,15 +1186,6 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>Resolve a handle's registered historian tagname. The caller (<see cref="ServeNode"/>)
|
||||
/// has already confirmed the node is historized before invoking the read callback, so this is a
|
||||
/// guaranteed hit; the null-coalesce is a defensive fallback to the bare NodeId string.</summary>
|
||||
private string ResolveTagname(NodeHandle handle)
|
||||
{
|
||||
var idString = handle.NodeId.Identifier?.ToString() ?? string.Empty;
|
||||
return TryGetHistorizedTagname(idString, out var tagname) ? tagname! : idString;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Map an OPC UA Part 13 standard-aggregate function NodeId to our
|
||||
/// <see cref="HistoryAggregateType"/>. Returns <c>null</c> for any aggregate we don't serve so
|
||||
|
||||
Reference in New Issue
Block a user