From dabdc2e2966b392ea839b1550653503d33d9fb07 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 17 Jul 2026 00:13:47 -0400 Subject: [PATCH] feat(dcl): bind OPC UA nodes by namespace URI, not baked index MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A stored ns= reference is only meaningful against one server's namespace table. A server that adds, removes or reorders a namespace silently re-points every binding: best case BadNodeIdUnknown, worst case the index now names a different namespace holding a colliding identifier and the binding resolves to the wrong node with Good quality. Nothing could detect that, because ScadaBridge stored no namespace URI anywhere. OtOpcUa v3.0 makes this concrete: v2's sole custom namespace and v3's raw namespace both sit at index 2, so v2-era bindings resolve against v3 without error while meaning something else entirely. Adds OpcUaNodeReference as the single translation seam between stored references and the wire. Resolve() accepts both ns=;s= (existing bindings, which keep working unchanged) and the durable nsu=;s=, mapping the URI to the live index at use time; it is wired into every parse site — subscribe, read, write, alarm-subscribe and browse. An unpublished URI now throws naming the URI rather than binding to whatever occupies that index, and a svr= reference to another server is rejected instead of being resolved against the wrong address space. The browser emits ToDurable(), so what the picker shows is what gets stored and newly-authored bindings are index-proof from the start. That also closes a round-trip gap: browse previously emitted ExpandedNodeId.ToString(), which for a URI- or server-index-carrying reference produced a string NodeId.Parse could not read back — the same method already resolved it correctly 57 lines later. Bindings stored before this change keep their ns= form and keep working; they are only as durable as the server's namespace order. Re-authoring against the picker is what makes them durable, and that re-bind still needs a live v3 rig. Refs: Gitea #14 --- .../Component-DataConnectionLayer.md | 4 + .../Adapters/OpcUaNodeReference.cs | 118 +++++++++++++ .../Adapters/RealOpcUaClient.cs | 18 +- .../Adapters/OpcUaNodeReferenceTests.cs | 165 ++++++++++++++++++ 4 files changed, 299 insertions(+), 6 deletions(-) create mode 100644 src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Adapters/OpcUaNodeReference.cs create mode 100644 tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/Adapters/OpcUaNodeReferenceTests.cs diff --git a/docs/requirements/Component-DataConnectionLayer.md b/docs/requirements/Component-DataConnectionLayer.md index 9b2af993..c5abdc13 100644 --- a/docs/requirements/Component-DataConnectionLayer.md +++ b/docs/requirements/Component-DataConnectionLayer.md @@ -199,6 +199,10 @@ DCL is a clean data pipe on the hot path. Browse is an **opt-in capability** for - `DataConnectionManagerActor` handles `BrowseNodeCommand` (fields: `ConnectionName`, `ParentNodeId`, and an optional opaque `ContinuationToken` for paging) and replies with `BrowseNodeResult` (children + `Truncated` + an optional continuation token + structured `BrowseFailure?`). The Central UI facade is `IBrowseService`/`BrowseService`, backing the `NodeBrowserDialog` tag picker. - **Browse type-info**: each child `BrowseNode` carries optional best-effort type metadata — `DataType` (friendly name), `ValueRank` (scalar/array), and `Writable`. `DataType` is populated by **both** OPC UA and MxGateway: `RealOpcUaClient` batch-reads built-in data-type node ids and maps them to friendly names for **Variable** nodes; `MxGatewayDataConnection` surfaces each attribute leaf's `GalaxyAttribute.data_type_name` (free-form Galaxy type text) as `DataType`. `ValueRank` and `Writable` are OPC-UA-only; MxGateway leaves them unset. - Node ids are opaque protocol-specific strings: OPC UA uses NodeIds; MxGateway uses Galaxy gobject ids for navigable objects and full tag references for selectable attribute leaves. +- **OPC UA namespace binding (Gitea #14)**: a NodeId's `ns=` is meaningful only against one server's namespace table, so a server that adds/removes/reorders a namespace silently re-points every stored binding — best case `BadNodeIdUnknown`, worst case the index names a different namespace holding a colliding identifier and the binding resolves to the **wrong node with Good quality**. The namespace **URI** is the stable identity. `OpcUaNodeReference` is therefore the single translation seam between stored references and the wire: + - `Resolve(reference, session.NamespaceUris)` accepts **both** `ns=;s=` (existing bindings, unchanged) and the durable `nsu=;s=`, mapping the URI to the live index at use time. It is used at every parse site — subscribe, read, write, alarm-subscribe, browse. A URI absent from the server's `NamespaceArray` throws with a message naming the URI, rather than binding to whatever sits at that index; a `svr=` reference to another server is rejected outright. + - `ToDurable(expandedNodeId, session.NamespaceUris)` is what the browser emits, so **what the picker shows is what gets stored** and newly-authored bindings are index-proof from the start. Namespace 0 is exempt (spec-fixed) and keeps its short form. It also closes a round-trip gap: the browse previously emitted `ExpandedNodeId.ToString()`, which for a URI- or server-index-carrying reference produced a string `NodeId.Parse` could not read back. + - Bindings stored before this change keep their `ns=` form and keep working; they are only as durable as the server's namespace order. Re-authoring against a picker (which now emits `nsu=`) is what makes them durable — see `docs/plans/` and Gitea #14 for the OtOpcUa v3.0 cutover, where v2's sole custom namespace and v3's `raw` namespace both sit at index 2, so v2-era bindings resolve against v3 without error while meaning something else. - Browse runs against the live session; no caching at DCL. - **Frame-size guard**: the reply crosses the site→central Akka frame (default 128 KB) on a temp Ask actor; an oversized reply is silently discarded by remoting, hanging the picker. The child handler caps each `BrowseNodeResult` to a byte budget (~100 KB) before replying, OR-ing the adapter's own truncation signal into `Truncated`. This is protocol-agnostic (every adapter's reply funnels through it). Per-protocol upstream caps narrow the window first: OPC UA requests at most 500 references per node (continuation point → `Truncated`); MxGateway relies on the gateway's `BrowseChildren` page cap. - **`BrowseNext` paging** (OPC UA): a `Truncated` level no longer forces manual node-id entry. When the OPC UA browse is truncated, the adapter returns the session's continuation point as the reply's opaque `ContinuationToken`; a follow-up `BrowseNodeCommand` carrying that token calls `Session.BrowseNext` to fetch the next page (the picker exposes this as **"Load more"**). Continuation points are session-bound and can expire — `BadContinuationPointInvalid` is caught and the browse restarts from a fresh first page rather than failing. Manual node-id entry remains as a fallback when the site or its session is offline. diff --git a/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Adapters/OpcUaNodeReference.cs b/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Adapters/OpcUaNodeReference.cs new file mode 100644 index 00000000..44221186 --- /dev/null +++ b/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Adapters/OpcUaNodeReference.cs @@ -0,0 +1,118 @@ +using Opc.Ua; + +namespace ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Adapters; + +/// +/// Translates the node-reference strings ScadaBridge stores in configuration +/// (TemplateAttribute.DataSourceReference and friends) to session-local +/// s, and back again for the node browser. +/// +/// +/// +/// Why this exists. A stored ns=2;s=Foo reference hard-codes a namespace +/// index, and an index is only meaningful relative to one server's namespace +/// table. A server that adds, removes or reorders a namespace silently re-points every +/// stored binding: best case the read fails BadNodeIdUnknown, worst case the +/// index now names a different namespace whose table happens to contain a colliding +/// identifier and the binding resolves to the wrong node with Good quality. The +/// namespace URI is the stable identity — the index is just its position today. +/// +/// +/// The durable form is therefore nsu=<namespace-uri>;s=<identifier>, +/// resolved against the live namespace table at use time. accepts +/// both forms, so existing ns= bindings keep working unchanged; +/// is what the browser emits so newly-authored bindings are index-proof from the start. +/// +/// +/// Namespace 0 is exempt: the OPC UA spec fixes it to http://opcfoundation.org/UA/, +/// so standard NodeIds like i=85 are already stable and stay in their short form. +/// +/// +public static class OpcUaNodeReference +{ + /// + /// Resolves a stored node reference — either ns=<index>;… or the durable + /// nsu=<uri>;… form — to a valid for this session. + /// + /// The stored reference string. + /// The connected session's namespace table. + /// A session-local . + /// + /// The reference is unparseable, names a namespace URI this server does not publish, + /// or points at a different server via svr=. + /// + public static NodeId Resolve(string reference, NamespaceTable namespaceUris) + { + ArgumentException.ThrowIfNullOrEmpty(reference); + ArgumentNullException.ThrowIfNull(namespaceUris); + + ExpandedNodeId expanded; + try + { + expanded = ExpandedNodeId.Parse(reference); + } + catch (Exception ex) + { + throw new ArgumentException( + $"'{reference}' is not a valid OPC UA node reference. Expected 'ns=;s=' " + + $"or the namespace-URI-qualified 'nsu=;s='.", + nameof(reference), ex); + } + + // svr= addresses a node on a *different* server via this server's server table. + // Nothing downstream follows that indirection, so fail loudly rather than hand + // the SDK a reference it would resolve against the wrong address space. + if (expanded.ServerIndex != 0) + { + throw new ArgumentException( + $"'{reference}' targets another server (svr={expanded.ServerIndex}); " + + "ScadaBridge binds only to nodes on the connected server.", + nameof(reference)); + } + + // Maps nsu= -> the live index; leaves an ns= reference's index untouched. + var nodeId = ExpandedNodeId.ToNodeId(expanded, namespaceUris); + if (NodeId.IsNull(nodeId)) + { + throw new ArgumentException( + $"'{reference}' names namespace URI '{expanded.NamespaceUri}', which this server " + + "does not publish in its NamespaceArray. The binding likely predates a server " + + "upgrade that changed its namespaces.", + nameof(reference)); + } + + return nodeId; + } + + /// + /// Renders a browse result as the durable, index-proof reference to store. + /// + /// The a browse returned. + /// The connected session's namespace table. + /// + /// nsu=<uri>;s=<id> for server-specific namespaces; the plain short form for + /// namespace 0. Falls back to the raw string when the namespace cannot be named. + /// + public static string ToDurable(ExpandedNodeId nodeId, NamespaceTable namespaceUris) + { + ArgumentNullException.ThrowIfNull(nodeId); + ArgumentNullException.ThrowIfNull(namespaceUris); + + // A browse can hand back a reference carrying a NamespaceUri or a server index + // rather than a plain index. Resolving to session-local first collapses those + // shapes; without it the raw ToString() can emit a string Resolve cannot re-read. + var local = ExpandedNodeId.ToNodeId(nodeId, namespaceUris); + if (NodeId.IsNull(local)) + return nodeId.ToString() ?? string.Empty; + + // ns=0 is spec-fixed, so its short form is already durable. + if (local.NamespaceIndex == 0) + return local.ToString() ?? string.Empty; + + var uri = namespaceUris.GetString(local.NamespaceIndex); + if (string.IsNullOrEmpty(uri)) + return local.ToString() ?? string.Empty; + + return new ExpandedNodeId(local, uri).ToString() ?? string.Empty; + } +} diff --git a/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Adapters/RealOpcUaClient.cs b/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Adapters/RealOpcUaClient.cs index b886ac0d..344cb774 100644 --- a/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Adapters/RealOpcUaClient.cs +++ b/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Adapters/RealOpcUaClient.cs @@ -411,7 +411,7 @@ public class RealOpcUaClient : IOpcUaClient var monitoredItem = new MonitoredItem(_subscription.DefaultItem) { DisplayName = nodeId, - StartNodeId = nodeId, + StartNodeId = OpcUaNodeReference.Resolve(nodeId, _session.NamespaceUris), AttributeId = Attributes.Value, SamplingInterval = _options.SamplingIntervalMs, QueueSize = (uint)_options.QueueSize, @@ -475,7 +475,9 @@ public class RealOpcUaClient : IOpcUaClient _alarmInRefresh[handle] = false; _alarmLastState[handle] = new Dictionary(StringComparer.Ordinal); - var startNode = string.IsNullOrEmpty(sourceNodeId) ? ObjectIds.Server : NodeId.Parse(sourceNodeId); + var startNode = string.IsNullOrEmpty(sourceNodeId) + ? ObjectIds.Server + : OpcUaNodeReference.Resolve(sourceNodeId, _session.NamespaceUris); var item = new MonitoredItem(_subscription.DefaultItem) { DisplayName = $"alarm:{sourceNodeId ?? "Server"}", @@ -829,7 +831,7 @@ public class RealOpcUaClient : IOpcUaClient var readValue = new ReadValueId { - NodeId = nodeId, + NodeId = OpcUaNodeReference.Resolve(nodeId, _session.NamespaceUris), AttributeId = Attributes.Value }; @@ -848,7 +850,7 @@ public class RealOpcUaClient : IOpcUaClient var writeValue = new WriteValue { - NodeId = nodeId, + NodeId = OpcUaNodeReference.Resolve(nodeId, _session.NamespaceUris), AttributeId = Attributes.Value, Value = new DataValue(new Variant(value)) }; @@ -960,7 +962,7 @@ public class RealOpcUaClient : IOpcUaClient // an absolute NodeId expression. var nodeToBrowse = string.IsNullOrEmpty(parentNodeId) ? ObjectIds.ObjectsFolder - : NodeId.Parse(parentNodeId); + : OpcUaNodeReference.Resolve(parentNodeId, session.NamespaceUris); // No token → fresh browse of the node. A non-empty token → continue a // prior browse via BrowseNext, falling back to a fresh browse if the @@ -1066,7 +1068,11 @@ public class RealOpcUaClient : IOpcUaClient foreach (var r in refs) { children.Add(new Commons.Interfaces.Protocol.BrowseNode( - NodeId: r.NodeId.ToString(), + // Durable nsu= form, not r.NodeId.ToString(): the raw ExpandedNodeId + // string can carry a namespace URI or server index that Resolve cannot + // read back, and a bare ns= index rots the moment the server's + // namespace table changes. What the picker shows is what gets stored. + NodeId: OpcUaNodeReference.ToDurable(r.NodeId, session.NamespaceUris), DisplayName: r.DisplayName?.Text ?? r.BrowseName?.Name ?? "(unnamed)", NodeClass: MapNodeClass(r.NodeClass), HasChildren: r.NodeClass == NodeClass.Object)); diff --git a/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/Adapters/OpcUaNodeReferenceTests.cs b/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/Adapters/OpcUaNodeReferenceTests.cs new file mode 100644 index 00000000..ca0f264f --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/Adapters/OpcUaNodeReferenceTests.cs @@ -0,0 +1,165 @@ +using Opc.Ua; +using ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Adapters; + +namespace ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests.Adapters; + +/// +/// Gitea #14: stored bindings hard-code a namespace index, which is only +/// meaningful against one server's namespace table. These tests pin the durable +/// nsu=<uri> form and the back-compat of the existing ns= form. +/// +public class OpcUaNodeReferenceTests +{ + // Mirrors an OtOpcUa v3 server: 0=UA (spec), 1=ApplicationUri, 2=raw, 3=uns. + private const string RawUri = "https://zb.com/otopcua/raw"; + private const string UnsUri = "https://zb.com/otopcua/uns"; + + private static NamespaceTable V3Table() + { + var t = new NamespaceTable(); // index 0 = http://opcfoundation.org/UA/ + t.Append("urn:localhost:OtOpcUa"); // 1 + t.Append(RawUri); // 2 + t.Append(UnsUri); // 3 + return t; + } + + // The v2 server: a single custom namespace, which also landed at index 2. + private static NamespaceTable V2Table() + { + var t = new NamespaceTable(); + t.Append("urn:localhost:OtOpcUa"); // 1 + t.Append("https://zb.com/otopcua/ns"); // 2 + return t; + } + + [Fact] + public void Resolve_IndexForm_StillWorks() + { + // Back-compat: every binding in the field today is this shape. + var nodeId = OpcUaNodeReference.Resolve("ns=2;s=pymodbus/plc/HR200", V3Table()); + + Assert.Equal(2, nodeId.NamespaceIndex); + Assert.Equal("pymodbus/plc/HR200", nodeId.Identifier); + } + + [Fact] + public void Resolve_UriForm_MapsToLiveIndex() + { + var nodeId = OpcUaNodeReference.Resolve($"nsu={RawUri};s=pymodbus/plc/HR200", V3Table()); + + Assert.Equal(2, nodeId.NamespaceIndex); + Assert.Equal("pymodbus/plc/HR200", nodeId.Identifier); + } + + [Fact] + public void Resolve_UriForm_TracksIndexAcrossServers() + { + // The point of the durable form: same reference, a server that orders its + // namespaces differently, still the right namespace. + var reordered = new NamespaceTable(); + reordered.Append("urn:localhost:OtOpcUa"); // 1 + reordered.Append("urn:something:else"); // 2 <- pushed raw along + reordered.Append(RawUri); // 3 + + var v3 = OpcUaNodeReference.Resolve($"nsu={RawUri};s=Tag", V3Table()); + var moved = OpcUaNodeReference.Resolve($"nsu={RawUri};s=Tag", reordered); + + Assert.Equal(2, v3.NamespaceIndex); + Assert.Equal(3, moved.NamespaceIndex); // followed the URI, not the index + } + + [Fact] + public void Resolve_IndexForm_SilentlyBindsToTheWrongNamespace_AcrossServerUpgrade() + { + // This is the #14 hazard, pinned as a characterisation test rather than a wish: + // v2's sole custom namespace and v3's raw namespace BOTH sit at index 2, so a + // v2-era ns=2 binding resolves happily against v3 while meaning something else + // entirely. Nothing in the index form can detect that — which is the whole + // argument for re-authoring bindings in the nsu= form. + const string v2Binding = "ns=2;s=Line1/Pump/Speed"; + + var againstV2 = OpcUaNodeReference.Resolve(v2Binding, V2Table()); + var againstV3 = OpcUaNodeReference.Resolve(v2Binding, V3Table()); + + Assert.Equal(againstV2.NamespaceIndex, againstV3.NamespaceIndex); + Assert.Equal("https://zb.com/otopcua/ns", V2Table().GetString(againstV2.NamespaceIndex)); + Assert.Equal(RawUri, V3Table().GetString(againstV3.NamespaceIndex)); // different namespace, no error + + // The durable form fails loudly instead. + var ex = Assert.Throws( + () => OpcUaNodeReference.Resolve($"nsu=https://zb.com/otopcua/ns;s=Line1/Pump/Speed", V3Table())); + Assert.Contains("does not publish", ex.Message); + } + + [Fact] + public void Resolve_UnknownNamespaceUri_Throws() + { + var ex = Assert.Throws( + () => OpcUaNodeReference.Resolve("nsu=https://example.invalid/nope;s=Tag", V3Table())); + + Assert.Contains("NamespaceArray", ex.Message); + } + + [Fact] + public void Resolve_ForeignServerReference_Throws() + { + var ex = Assert.Throws( + () => OpcUaNodeReference.Resolve($"svr=1;nsu={RawUri};s=Tag", V3Table())); + + Assert.Contains("another server", ex.Message); + } + + [Theory] + [InlineData("")] + [InlineData(" ;;; not a node id")] + public void Resolve_Unparseable_Throws(string reference) + { + Assert.ThrowsAny(() => OpcUaNodeReference.Resolve(reference, V3Table())); + } + + [Fact] + public void ToDurable_ServerNamespace_EmitsUriForm() + { + var browsed = new ExpandedNodeId("pymodbus/plc/HR200", 2, null, 0); + + var durable = OpcUaNodeReference.ToDurable(browsed, V3Table()); + + Assert.Equal($"nsu={RawUri};s=pymodbus/plc/HR200", durable); + } + + [Fact] + public void ToDurable_StandardNamespace_StaysShort() + { + // ns=0 is spec-fixed; qualifying it would be noise. + var durable = OpcUaNodeReference.ToDurable(new ExpandedNodeId(85u, 0), V3Table()); + + Assert.Equal("i=85", durable); + } + + [Fact] + public void ToDurable_RoundTripsThroughResolve() + { + // The bug this closes: browse emitted ExpandedNodeId.ToString(), which for a + // URI-carrying reference produced a string NodeId.Parse could not read back. + var table = V3Table(); + var browsed = new ExpandedNodeId("Area/Line/Equip/Speed", 0, UnsUri, 0); + + var durable = OpcUaNodeReference.ToDurable(browsed, table); + var resolved = OpcUaNodeReference.Resolve(durable, table); + + Assert.Equal(3, resolved.NamespaceIndex); + Assert.Equal("Area/Line/Equip/Speed", resolved.Identifier); + } + + [Fact] + public void ToDurable_UriNotInTable_FallsBackRatherThanThrowing() + { + // Browsing is a discovery path — a namespace we cannot name should degrade to + // the raw string, not abort the browse. + var browsed = new ExpandedNodeId("Tag", 0, "https://example.invalid/nope", 0); + + var durable = OpcUaNodeReference.ToDurable(browsed, V3Table()); + + Assert.Contains("Tag", durable); + } +}