Path-based NodeIds — decouple client contract from driver address

The pre-refactor design minted OPC UA NodeIds directly from the driver's
FullReference (the native-address string). That had three long-term
problems:

1. OPC UA Part 3 §5.2.2 requires NodeIds to be immutable across a node's
   lifetime. A rename of the underlying device address — Galaxy attribute,
   S7 tag, Modbus register alias — changed the NodeId and broke every
   client that had pinned the previous identifier.
2. Two drivers with coincidentally-matching native addresses (e.g. `temp`
   in Modbus and `temp` in S7 under different Equipment rows) collided on
   the NodeId identifier.
3. TagConfig was being placed verbatim on the wire; for drivers whose
   TagConfig is JSON (every driver shipped today, per the
   CK_Tag_TagConfig_IsJson check constraint), clients saw the raw JSON
   blob as the NodeId string.

Refactor:

* DriverNodeManager.Variable now mints a stable path-based NodeId
  `{driverId}/{folder-path}/{browseName}` and records the driver-side
  FullReference in a new _fullRefByNodeId map. OnReadValue / OnWriteValue
  / ResolveFullRef look the FullReference up via that map instead of
  casting NodeId.Identifier. The old cast path is preserved as a
  fallback so any test fixture that still registers variables with
  FullRef-shaped NodeIds keeps working.

* EquipmentNodeWalker.AddTagVariable now extracts the cross-driver
  `FullName` field from Tag.TagConfig before handing the address to
  DriverAttributeInfo. Every shipped driver stores the wire reference in
  TagConfig[FullName]; falling back to the raw string covers any future
  driver that wants an opaque non-JSON address. ExtractFullName is
  exposed internal for unit coverage.

* scripts/e2e/test-galaxy.ps1 defaults updated to the new path-based
  NodeIds. Verified live against p7-smoke-galaxy on the dev box:
  `ns=2;s=p7-smoke-galaxy/lab-floor/galaxy-line/reactor-1/Source` reads
  return Status=0x00000000 with a real Galaxy byte-array value.

Test suite: 195/195 Core.Tests + 283/283 Server.Tests green. Five new
ExtractFullName / FullName-passthrough tests added.

Task #112 GA-3 — golden-path read verified end-to-end; remaining E2E
script stages still blocked on pre-existing issues (ScriptedAlarm
predicate NRE on empty upstream cache, PowerShell $changeLines.Count
guard), tracked separately.
Task #134 — complete.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Joseph Doherty
2026-04-24 16:57:20 -04:00
parent d11dd0520b
commit 8be82e02c2
4 changed files with 134 additions and 16 deletions

View File

@@ -49,16 +49,18 @@
OtOpcUa server endpoint. Default opc.tcp://localhost:4840. OtOpcUa server endpoint. Default opc.tcp://localhost:4840.
.PARAMETER SourceNodeId .PARAMETER SourceNodeId
NodeId of the driver-sourced Galaxy tag (numeric, writable preferred). NodeId of the driver-sourced Galaxy tag (numeric, writable preferred). NodeIds
Default matches the Phase 7 seed — `ns=2;s=p7-smoke-tag-source`. are path-based per OPC UA Part 3 §5.2.2 — the default matches the Phase 7 seed
walking `p7-smoke-galaxy` (DriverInstanceId) → `lab-floor` → `galaxy-line` →
`reactor-1` → `Source` (Tag.Name).
.PARAMETER VirtualNodeId .PARAMETER VirtualNodeId
NodeId of the VirtualTag computed as Source × 2 (Phase 7 scripting). NodeId of the VirtualTag computed as Source × 2 (Phase 7 scripting). Same
Default matches the Phase 7 seed — `ns=2;s=p7-smoke-vt-derived`. path-based scheme, ending in the VirtualTag.Name (`Doubled`).
.PARAMETER AlarmNodeId .PARAMETER AlarmNodeId
NodeId of the scripted-alarm Condition (fires when Source > 50). NodeId of the scripted-alarm Condition (fires when Source > 50). Same
Default matches the Phase 7 seed — `ns=2;s=p7-smoke-al-overtemp`. path-based scheme, ending in ScriptedAlarm.Name (`OverTemp`).
.PARAMETER AlarmTriggerValue .PARAMETER AlarmTriggerValue
Value written to -SourceNodeId to push it over the alarm threshold. Value written to -SourceNodeId to push it over the alarm threshold.
@@ -90,9 +92,9 @@
param( param(
[string]$OpcUaUrl = "opc.tcp://localhost:4840", [string]$OpcUaUrl = "opc.tcp://localhost:4840",
[string]$SourceNodeId = "ns=2;s=p7-smoke-tag-source", [string]$SourceNodeId = "ns=2;s=p7-smoke-galaxy/lab-floor/galaxy-line/reactor-1/Source",
[string]$VirtualNodeId = "ns=2;s=p7-smoke-vt-derived", [string]$VirtualNodeId = "ns=2;s=p7-smoke-galaxy/lab-floor/galaxy-line/reactor-1/Doubled",
[string]$AlarmNodeId = "ns=2;s=p7-smoke-al-overtemp", [string]$AlarmNodeId = "ns=2;s=p7-smoke-galaxy/lab-floor/galaxy-line/reactor-1/OverTemp",
[string]$AlarmTriggerValue = "75", [string]$AlarmTriggerValue = "75",
[int]$ChangeWaitSec = 10, [int]$ChangeWaitSec = 10,
[int]$AlarmWaitSec = 10, [int]$AlarmWaitSec = 10,

View File

@@ -1,3 +1,4 @@
using System.Text.Json;
using ZB.MOM.WW.OtOpcUa.Configuration.Entities; using ZB.MOM.WW.OtOpcUa.Configuration.Entities;
using ZB.MOM.WW.OtOpcUa.Core.Abstractions; using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
@@ -157,7 +158,7 @@ public static class EquipmentNodeWalker
private static void AddTagVariable(IAddressSpaceBuilder equipmentBuilder, Tag tag) private static void AddTagVariable(IAddressSpaceBuilder equipmentBuilder, Tag tag)
{ {
var attr = new DriverAttributeInfo( var attr = new DriverAttributeInfo(
FullName: tag.TagConfig, FullName: ExtractFullName(tag.TagConfig),
DriverDataType: ParseDriverDataType(tag.DataType), DriverDataType: ParseDriverDataType(tag.DataType),
IsArray: false, IsArray: false,
ArrayDim: null, ArrayDim: null,
@@ -166,6 +167,38 @@ public static class EquipmentNodeWalker
equipmentBuilder.Variable(tag.Name, tag.Name, attr); equipmentBuilder.Variable(tag.Name, tag.Name, attr);
} }
/// <summary>
/// Cross-driver TagConfig convention — the Config DB's <c>CK_Tag_TagConfig_IsJson</c>
/// check constraint requires TagConfig to be a JSON object, and every shipped driver
/// (Galaxy / Modbus / AB CIP / S7 / FOCAS / TwinCAT / ABLegacy) stores the wire-level
/// address in a top-level <c>FullName</c> field. Extracting it here keeps the walker
/// driver-agnostic while giving the driver the plain address string its backend
/// expects at read-time — the raw JSON would otherwise be passed verbatim to
/// <c>IReadable.ReadAsync</c> and the driver would fail to resolve the tag.
/// </summary>
/// <remarks>
/// Falls back to the raw <paramref name="tagConfig"/> if it doesn't parse as JSON or
/// the <c>FullName</c> field is absent. This preserves the pre-refactor behaviour for
/// any legacy row that slipped past the check constraint or any future driver that
/// wants an opaque non-JSON reference.
/// </remarks>
internal static string ExtractFullName(string tagConfig)
{
if (string.IsNullOrWhiteSpace(tagConfig)) return tagConfig;
try
{
using var doc = JsonDocument.Parse(tagConfig);
if (doc.RootElement.ValueKind == JsonValueKind.Object
&& doc.RootElement.TryGetProperty("FullName", out var fullName)
&& fullName.ValueKind == JsonValueKind.String)
{
return fullName.GetString() ?? tagConfig;
}
}
catch (JsonException) { /* fall through */ }
return tagConfig;
}
/// <summary> /// <summary>
/// Parse <see cref="Tag.DataType"/> (stored as the <see cref="DriverDataType"/> enum /// Parse <see cref="Tag.DataType"/> (stored as the <see cref="DriverDataType"/> enum
/// name string, decision #138) into the enum value. Unknown names fall back to /// name string, decision #138) into the enum value. Unknown names fall back to

View File

@@ -50,6 +50,15 @@ public sealed class DriverNodeManager : CustomNodeManager2, IAddressSpaceBuilder
private FolderState? _driverRoot; private FolderState? _driverRoot;
private readonly Dictionary<string, BaseDataVariableState> _variablesByFullRef = new(StringComparer.OrdinalIgnoreCase); private readonly Dictionary<string, BaseDataVariableState> _variablesByFullRef = new(StringComparer.OrdinalIgnoreCase);
// NodeId-identifier (string) → driver FullReference. OPC UA Part 3 §5.2.2 requires NodeIds
// to be immutable across a node's lifetime, which precludes minting them from the driver's
// native address (a backend rename would change the NodeId and break every subscribed
// client). NodeIds are therefore path-based (`{driverId}/{folder-path}/{browseName}`) and
// this map recovers the driver-side FullReference for read/write/history dispatch. The
// fallback in lookups preserves the pre-refactor behaviour for any caller that still
// registered a variable via a FullRef-shaped NodeId.
private readonly Dictionary<string, string> _fullRefByNodeId = new(StringComparer.Ordinal);
// PR 26: SecurityClassification per variable, populated during Variable() registration. // PR 26: SecurityClassification per variable, populated during Variable() registration.
// OnWriteValue looks up the classification here to gate the write by the session's roles. // OnWriteValue looks up the classification here to gate the write by the session's roles.
// Drivers never enforce authz themselves — the classification is discovery-time metadata // Drivers never enforce authz themselves — the classification is discovery-time metadata
@@ -170,12 +179,17 @@ public sealed class DriverNodeManager : CustomNodeManager2, IAddressSpaceBuilder
{ {
lock (Lock) lock (Lock)
{ {
// Path-based NodeId per OPC UA Part 3 §5.2.2 (NodeIds MUST NOT change across the
// node's lifetime). Shape `{driverId}/{folder-path}/{browseName}` is stable across
// driver-side renames of the underlying FullReference + keeps the identifier
// self-describing against the browse tree.
var nodeKey = $"{_currentFolder.NodeId.Identifier}/{browseName}";
var v = new BaseDataVariableState(_currentFolder) var v = new BaseDataVariableState(_currentFolder)
{ {
SymbolicName = browseName, SymbolicName = browseName,
ReferenceTypeId = ReferenceTypeIds.Organizes, ReferenceTypeId = ReferenceTypeIds.Organizes,
TypeDefinitionId = VariableTypeIds.BaseDataVariableType, TypeDefinitionId = VariableTypeIds.BaseDataVariableType,
NodeId = new NodeId(attributeInfo.FullName, NamespaceIndex), NodeId = new NodeId(nodeKey, NamespaceIndex),
BrowseName = new QualifiedName(browseName, NamespaceIndex), BrowseName = new QualifiedName(browseName, NamespaceIndex),
DisplayName = new LocalizedText(displayName), DisplayName = new LocalizedText(displayName),
DataType = MapDataType(attributeInfo.DriverDataType), DataType = MapDataType(attributeInfo.DriverDataType),
@@ -197,6 +211,7 @@ public sealed class DriverNodeManager : CustomNodeManager2, IAddressSpaceBuilder
_securityByFullRef[attributeInfo.FullName] = attributeInfo.SecurityClass; _securityByFullRef[attributeInfo.FullName] = attributeInfo.SecurityClass;
_writeIdempotentByFullRef[attributeInfo.FullName] = attributeInfo.WriteIdempotent; _writeIdempotentByFullRef[attributeInfo.FullName] = attributeInfo.WriteIdempotent;
_sourceByFullRef[attributeInfo.FullName] = attributeInfo.Source; _sourceByFullRef[attributeInfo.FullName] = attributeInfo.Source;
_fullRefByNodeId[nodeKey] = attributeInfo.FullName;
v.OnReadValue = OnReadValue; v.OnReadValue = OnReadValue;
v.OnWriteValue = OnWriteValue; v.OnWriteValue = OnWriteValue;
@@ -228,7 +243,7 @@ public sealed class DriverNodeManager : CustomNodeManager2, IAddressSpaceBuilder
private ServiceResult OnReadValue(ISystemContext context, NodeState node, NumericRange indexRange, private ServiceResult OnReadValue(ISystemContext context, NodeState node, NumericRange indexRange,
QualifiedName dataEncoding, ref object? value, ref StatusCode statusCode, ref DateTime timestamp) QualifiedName dataEncoding, ref object? value, ref StatusCode statusCode, ref DateTime timestamp)
{ {
var fullRef = node.NodeId.Identifier as string ?? ""; var fullRef = NodeIdToFullRef(node.NodeId);
var source = _sourceByFullRef.TryGetValue(fullRef, out var s) ? s : NodeSourceKind.Driver; var source = _sourceByFullRef.TryGetValue(fullRef, out var s) ? s : NodeSourceKind.Driver;
var readable = SelectReadable(source, _readable, _virtualReadable, _scriptedAlarmReadable); var readable = SelectReadable(source, _readable, _virtualReadable, _scriptedAlarmReadable);
@@ -717,7 +732,7 @@ public sealed class DriverNodeManager : CustomNodeManager2, IAddressSpaceBuilder
private ServiceResult OnWriteValue(ISystemContext context, NodeState node, NumericRange indexRange, private ServiceResult OnWriteValue(ISystemContext context, NodeState node, NumericRange indexRange,
QualifiedName dataEncoding, ref object? value, ref StatusCode statusCode, ref DateTime timestamp) QualifiedName dataEncoding, ref object? value, ref StatusCode statusCode, ref DateTime timestamp)
{ {
var fullRef = node.NodeId.Identifier as string; var fullRef = NodeIdToFullRef(node.NodeId);
if (string.IsNullOrEmpty(fullRef)) return StatusCodes.BadNodeIdUnknown; if (string.IsNullOrEmpty(fullRef)) return StatusCodes.BadNodeIdUnknown;
// Per Phase 7 plan decision #6 — virtual tags + scripted alarms reject direct // Per Phase 7 plan decision #6 — virtual tags + scripted alarms reject direct
@@ -1102,7 +1117,23 @@ public sealed class DriverNodeManager : CustomNodeManager2, IAddressSpaceBuilder
} }
} }
private string? ResolveFullRef(NodeHandle handle) => handle.NodeId?.Identifier as string; private string? ResolveFullRef(NodeHandle handle)
{
if (handle.NodeId is null) return null;
return NodeIdToFullRef(handle.NodeId);
}
/// <summary>
/// Recover the driver-side FullReference for a given OPC UA <see cref="NodeId"/>. Looks
/// the identifier up in <see cref="_fullRefByNodeId"/>; when no entry exists (e.g. for
/// legacy test fixtures that still register variables with FullRef-shaped NodeIds) we
/// fall through to the raw identifier string so those code paths keep working.
/// </summary>
private string NodeIdToFullRef(NodeId nodeId)
{
if (nodeId?.Identifier is not string key) return string.Empty;
return _fullRefByNodeId.TryGetValue(key, out var fullRef) ? fullRef : key;
}
// Both the results list AND the parallel errors list must be populated — MasterNodeManager // Both the results list AND the parallel errors list must be populated — MasterNodeManager
// merges them and the merged StatusCode is what the client sees. Leaving errors[i] at its // merges them and the merged StatusCode is what the client sees. Leaving errors[i] at its

View File

@@ -258,6 +258,57 @@ public sealed class EquipmentNodeWalkerTests
v.AttributeInfo.ScriptedAlarmId.ShouldBeNull(); v.AttributeInfo.ScriptedAlarmId.ShouldBeNull();
} }
[Fact]
public void ExtractFullName_unwraps_json_object_with_FullName_field()
{
EquipmentNodeWalker.ExtractFullName(
"{\"FullName\":\"MESReceiver_001.MoveInBatchID\",\"DataType\":\"Int32\"}")
.ShouldBe("MESReceiver_001.MoveInBatchID");
}
[Fact]
public void ExtractFullName_handles_S7_style_extra_fields()
{
EquipmentNodeWalker.ExtractFullName(
"{\"FullName\":\"DB1_DBW0\",\"Address\":\"DB1.DBW0\",\"DataType\":\"Int16\"}")
.ShouldBe("DB1_DBW0");
}
[Fact]
public void ExtractFullName_returns_raw_when_not_json()
{
// Drivers that opt out of JSON TagConfig still work — fallback preserves the literal
// string so the driver's IReadable sees whatever the row author stored.
EquipmentNodeWalker.ExtractFullName("raw-tag-ref").ShouldBe("raw-tag-ref");
}
[Fact]
public void ExtractFullName_returns_raw_when_json_missing_FullName_field()
{
EquipmentNodeWalker.ExtractFullName("{\"Address\":\"DB1.DBW0\"}")
.ShouldBe("{\"Address\":\"DB1.DBW0\"}");
}
[Fact]
public void Driver_tag_FullName_passes_through_from_TagConfig_json()
{
// The walker hands the driver the unwrapped FullName string so IReadable.ReadAsync
// sees the plain address, not the raw TagConfig JSON. Verifies the dispatch contract
// the path-based NodeId refactor relies on.
var eq = Eq("eq-1", "line-1", "oven-3");
var tag = NewTag("t-1", "Temp", "Int32", "plc-01", "eq-1",
tagConfig: "{\"FullName\":\"plc-01/HR200\",\"DataType\":\"Int32\"}");
var content = new EquipmentNamespaceContent(
[Area("area-1", "warsaw")], [Line("line-1", "area-1", "line-a")],
[eq], [tag]);
var rec = new RecordingBuilder("root");
EquipmentNodeWalker.Walk(rec, content);
var v = rec.Children[0].Children[0].Children[0].Variables.Single();
v.AttributeInfo.FullName.ShouldBe("plc-01/HR200");
}
// ----- builders for test seed rows ----- // ----- builders for test seed rows -----
private static UnsArea Area(string id, string name) => new() private static UnsArea Area(string id, string name) => new()
@@ -282,7 +333,8 @@ public sealed class EquipmentNodeWalkerTests
MachineCode = "MC-" + name, MachineCode = "MC-" + name,
}; };
private static Tag NewTag(string tagId, string name, string dataType, string address, string? equipmentId) => new() private static Tag NewTag(string tagId, string name, string dataType, string address,
string? equipmentId, string? tagConfig = null) => new()
{ {
TagRowId = Guid.NewGuid(), TagRowId = Guid.NewGuid(),
GenerationId = 1, GenerationId = 1,
@@ -292,7 +344,7 @@ public sealed class EquipmentNodeWalkerTests
Name = name, Name = name,
DataType = dataType, DataType = dataType,
AccessLevel = ZB.MOM.WW.OtOpcUa.Configuration.Enums.TagAccessLevel.ReadWrite, AccessLevel = ZB.MOM.WW.OtOpcUa.Configuration.Enums.TagAccessLevel.ReadWrite,
TagConfig = address, TagConfig = tagConfig ?? address,
}; };
// ----- recording IAddressSpaceBuilder ----- // ----- recording IAddressSpaceBuilder -----