From 8d9155682dfe95cb0b271933ab3f06f06c845d64 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 24 Jul 2026 23:43:07 -0400 Subject: [PATCH] fix(mqtt): browse-commit writes a bindable address + a Request-rebirth affordance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two gaps the Task 23/24 review found, both blocking Task 26's live gate. Gap 1 — browse-commit produced a silently dead MQTT tag. RawBrowseCommitMapper had no `Mqtt` case, so a committed leaf fell through to the generic `{"address": …}` key. Neither MqttTagDefinitionFactory entry point reads `address`: the tag deployed clean and reported BadNodeIdUnknown forever, with no signal at commit time. Predates Task 23 (Plain was affected too), but Task 23 built the Sparkplug metric tree precisely so an operator could browse and commit a binding. The address is a DESCRIPTOR, not a reference string, and it cannot be recovered from the browse node id: `{group}/{node}[/{device}]::{metric}` where a metric name legitimately contains `/` (`Node Control/Rebirth`) — the ambiguity MetricSeparator's remarks already name. So the session STATES it, via a new `AttributeInfo.AddressFields` seam, and the mapper reads it. Which keys are emitted is also how the mapper learns Plain vs Sparkplug — the driver type reaching it is just `Mqtt`, and the mode lives on the driver config. Key names are single-sourced in the new `MqttTagConfigKeys` (producer, mapper, factory), with the literals pinned by a test so a symmetric rename cannot silently unbind already-persisted blobs. A leaf with no stated address is refused at commit in words rather than committed dead. Gap 2 — RequestRebirthAsync had no UI. Task 23 shipped it backend-only; Task 26 step 1 assumes the button, and Task 25's runbook notes the picker tree stays empty until a birth lands, so it is the only way to fill it on demand. Added to the /raw browse modal: scope = the clicked tree node (device/metric resolve up to their edge node, group fans out and is refused whole past 32), an explicit two-click confirm naming the resolved scope and its consequence, the outcome or the refusal shown rather than swallowed. Offered only for a Sparkplug session (new `IRebirthCapableBrowseSession.RebirthAvailable` — one session class serves both modes, so a type test alone would draw a button that can only throw) and only to a DriverOperator; the server-side gate remains the boundary. Tests: MQTT 545 → 581, AdminUI 781 → 800. The round-trip tests feed the emitted blob to the REAL factory and were falsified by mutating the emitted key name. Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW --- docs/drivers/Mqtt.md | 38 ++- .../Browsing/BrowseNode.cs | 24 +- .../Browsing/IBrowseSession.cs | 19 ++ .../MqttBrowseSession.cs | 91 +++++- .../MqttTagConfigKeys.cs | 40 +++ .../MqttTagDefinitionFactory.cs | 12 +- .../Browsing/BrowserSessionService.cs | 4 + .../Browsing/IBrowserSessionService.cs | 15 + .../Shared/Drivers/DriverBrowseTree.razor | 12 +- .../Shared/Raw/RawBrowseModal.razor | 285 +++++++++++++++++- .../Uns/RawBrowseCommitMapper.cs | 120 +++++++- .../MqttBrowseSessionTests.cs | 94 ++++++ .../MqttTagDefinitionFactoryTests.cs | 38 +++ .../Browsing/BrowserSessionServiceTests.cs | 24 ++ .../Uns/RawBrowseCommitMapperTests.cs | 199 ++++++++++++ 15 files changed, 988 insertions(+), 27 deletions(-) create mode 100644 src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Contracts/MqttTagConfigKeys.cs diff --git a/docs/drivers/Mqtt.md b/docs/drivers/Mqtt.md index e546f315..f0afcb8f 100644 --- a/docs/drivers/Mqtt.md +++ b/docs/drivers/Mqtt.md @@ -172,11 +172,47 @@ during the window is invisible — so manual entry remains the escape hatch. Bounds: 50 000 nodes (then `⚠ Observation truncated`), 1024-char topics / 256-char segments (then `⚠ Some topics were too long`), 512-byte payload inspection. Sessions are reaped after 2 minutes idle. -`SparkplugB` browse throws `NotSupportedException`. + +**Sparkplug B browse serves a different tree from the same passive window**: observed NBIRTH/DBIRTH +certificates are decoded into `Group → EdgeNode → [Device] → Metric` (a birth is the only Sparkplug +message that names its metrics). Same node cap, same bounds, same "publishes nothing" guarantee. The bespoke browser wins over the universal `DiscoveryDriverBrowser`, which would decline MQTT anyway (`SupportsOnlineDiscovery` is `false`). +### Browse-commit writes the address the factory reads + +A committed leaf's address is a **descriptor**, not a single reference string, so +`RawBrowseCommitMapper` reads it from the `AttributeInfo.AddressFields` the session states — Plain +emits `topic`; Sparkplug emits `groupId` / `edgeNodeId` / `deviceId?` / `metricName`, the names +`MqttTagConfigKeys` single-sources for both the mapper and `MqttTagDefinitionFactory`. + +**It is never parsed out of the browse node id.** A Sparkplug node id is +`{group}/{node}[/{device}]::{metric}` and a metric name legitimately contains `/` +(`Node Control/Rebirth`), so splitting the id cannot recover the tuple. Which keys the session emits +is also how the mapper learns the shape — the driver *type* reaching it is just `Mqtt`, and the mode +lives on the driver config. A leaf whose attribute lookup returned nothing carries no address and is +refused **at commit, in words**, rather than committed as a tag that deploys clean and then reports +`BadNodeIdUnknown` forever. + +### Request rebirth (Sparkplug only) + +Births are never retained, so a healthy but quiet plant shows an **empty** picker tree until one +lands. The `Request rebirth` affordance in the browse modal is the way to fill it on demand: it +publishes a Sparkplug rebirth-request NCMD (`Node Control/Rebirth`) to the selected scope — the **one +and only** browse action that publishes anything. + +- **Scope** is the last node clicked in the tree. A device or metric resolves **up to its owning edge + node** (Sparkplug has no device-scoped rebirth); a group fans out to every observed edge node + beneath it and is refused **whole** past 32, publishing nothing. +- **Two clicks, never one** — `Request rebirth…` arms a confirm panel naming the resolved scope and + its consequence; changing the selection disarms it. +- **Gated on `DriverOperator`**, enforced server-side in `BrowserSessionService.RequestRebirthAsync` + (fail-closed) before the session is touched; the UI gate is defence in depth. A refusal — policy, + over-wide group, unresolvable scope — is **shown**, not swallowed. +- Offered only for a Sparkplug session (`IRebirthCapableBrowseSession.RebirthAvailable`); a Plain + MQTT window publishes nothing, ever. + ## Test Connect `MqttDriverProbe` performs **one CONNECT** (no SUBSCRIBE, no publish) under a `-probe-` client id and diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Browsing/BrowseNode.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Browsing/BrowseNode.cs index c7d05e6e..44f20c91 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Browsing/BrowseNode.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Browsing/BrowseNode.cs @@ -29,9 +29,31 @@ public enum BrowseNodeKind /// AlarmExtension primitive). The picker pre-fills a default native-alarm alarm object /// into the TagConfig when an alarm attribute is selected. Defaults to false so non-alarm-aware /// drivers (e.g. the OPC UA client browser) aren't forced to flow a flag they don't produce. +/// +/// The structured address this leaf binds by, as TagConfig key → value in the +/// driver's own key vocabulary, for a driver whose address is a tuple rather than a single +/// reference string. Null (the default) for every driver whose address IS the +/// — nothing changes for them. +/// +/// It exists because a tuple cannot be recovered from an id string in general. MQTT/Sparkplug +/// is the case in point: a browse node id is +/// {group}/{node}[/{device}]::{metric}, and a metric name may itself contain / +/// (Node Control/Rebirth) — so the session keeps the decomposition it already has and +/// states it here, rather than the AdminUI's browse-commit mapper re-deriving it by +/// splitting the id and hoping. Same discipline as the v3 address space carrying +/// AddressSpaceRealm explicitly instead of parsing it out of a NodeId. +/// +/// +/// The producer states the binding shape too, by which keys it emits — so a consumer +/// never has to infer a driver's sub-mode from the tree's geometry. Consumers must read the +/// keys they know and ignore the rest; this is a description, not a config blob to splat into +/// a persisted TagConfig. +/// +/// public sealed record AttributeInfo( string Name, string DriverDataType, bool IsArray, string SecurityClass, - bool IsAlarm = false); + bool IsAlarm = false, + IReadOnlyDictionary? AddressFields = null); diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Browsing/IBrowseSession.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Browsing/IBrowseSession.cs index 258beeb3..d31ecc40 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Browsing/IBrowseSession.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Browsing/IBrowseSession.cs @@ -61,6 +61,25 @@ public interface IBrowseSession : IAsyncDisposable /// public interface IRebirthCapableBrowseSession : IBrowseSession { + /// + /// Whether this particular session can actually re-announce — the runtime half of the + /// type question, and the one a UI must ask before offering the affordance. + /// + /// + /// One session class may serve several protocol shapes: the MQTT browser opens the same session + /// type for Plain and for Sparkplug B, and a Plain MQTT window publishes nothing, ever — + /// there is no plain-MQTT re-announce to offer. Implementing the interface therefore means "this + /// session type may re-announce"; this property means "this instance will". A UI gating on the + /// type alone would draw a button that can only ever throw. + /// + /// This is not an authorization signal — see the interface remarks. False here hides + /// the affordance; the caller still authorizes before invoking + /// , and the implementation still refuses a call it cannot + /// serve. + /// + /// + bool RebirthAvailable { get; } + /// /// Asks the addressed remote peer(s) to re-announce themselves. /// diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Browser/MqttBrowseSession.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Browser/MqttBrowseSession.cs index be3b909b..cb240b42 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Browser/MqttBrowseSession.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Browser/MqttBrowseSession.cs @@ -213,6 +213,14 @@ internal sealed class MqttBrowseSession : IRebirthCapableBrowseSession /// internal MqttMode Mode { get; } + /// + /// + /// Sparkplug only. A plain-MQTT window has no re-announce action at all — see + /// , which refuses one — so the picker must not draw the + /// affordance for it. + /// + public bool RebirthAvailable => Mode == MqttMode.SparkplugB; + /// public Guid Token { get; } = Guid.NewGuid(); @@ -301,11 +309,26 @@ internal sealed class MqttBrowseSession : IRebirthCapableBrowseSession { if (node.Metric is { } metric) { - attrs = [DescribeMetric(metric)]; + attrs = [DescribeMetric(metric, node.Scope)]; } else if (node.Observed is { } observed) { - attrs = [new AttributeInfo(node.Segment, observed.DataType.ToString(), IsArray: false, observed.Snippet)]; + attrs = + [ + new AttributeInfo( + node.Segment, + observed.DataType.ToString(), + IsArray: false, + observed.Snippet, + // Plain mode's address IS the node id, but it is stated rather than left implicit + // for the same reason the Sparkplug tuple is: the commit mapper reads the address + // off the fields, never off the id, and "which keys are present" is how it learns + // which of the driver's two binding shapes this leaf is. + AddressFields: new Dictionary(StringComparer.Ordinal) + { + [MqttTagConfigKeys.Topic] = node.Path, + }), + ]; } } @@ -317,8 +340,10 @@ internal sealed class MqttBrowseSession : IRebirthCapableBrowseSession /// Projects one observed Sparkplug birth metric into the picker's attribute row. /// /// The metric as its birth certificate declared it. + /// The (group, edgeNode, device?) identity stamped on the metric's node. /// The attribute row. /// + /// /// returns for /// the five v1-unsupported datatypes (Unknown, DataSet, Template, /// PropertySet, PropertySetList), and that null is passed through as the Sparkplug @@ -326,8 +351,18 @@ internal sealed class MqttBrowseSession : IRebirthCapableBrowseSession /// , so the AdminUI's browse-commit mapper falls back to its /// declared default instead of silently binding a metric this driver cannot serve — and the /// descriptor line says so in words. + /// + /// + /// is what makes a browse-committed Sparkplug tag + /// bindable at all. The tag's address is the (group, edgeNode, device?, metric) tuple, + /// and that tuple is not recoverable from the node id: a metric name may contain / + /// (and, off an arbitrary broker, ::), which is exactly why 's + /// remarks say nothing re-parses it. So the decomposition this session already holds — stamped on + /// the node when the birth was decoded — is handed over verbatim, keyed by + /// , the same names MqttTagDefinitionFactory reads. + /// /// - private static AttributeInfo DescribeMetric(SparkplugMetricInfo metric) + private static AttributeInfo DescribeMetric(SparkplugMetricInfo metric, SparkplugScope? scope) { var mapped = metric.DataType?.ToDriverDataType(); var sparkplugName = metric.DataType?.ToString() ?? nameof(SparkplugDataType.Unknown); @@ -340,7 +375,36 @@ internal sealed class MqttBrowseSession : IRebirthCapableBrowseSession metric.Name, mapped?.ToString() ?? sparkplugName, metric.DataType?.IsSparkplugArray() ?? false, - descriptor.ToString()); + descriptor.ToString(), + AddressFields: BuildMetricAddressFields(metric.Name, scope)); + } + + /// + /// Builds the Sparkplug binding tuple a browse-commit writes into the tag's TagConfig. + /// + /// The metric's birth-declared name. + /// The metric node's stamped identity, or . + /// + /// The address fields, or when the node carries no scope (which a + /// Sparkplug metric node never does — every one is created with the scope its birth parsed — + /// so a null here is "the producer cannot state the address", never "the address is empty"). + /// + private static IReadOnlyDictionary? BuildMetricAddressFields( + string metricName, SparkplugScope? scope) + { + if (scope?.EdgeNodeId is not { } edgeNodeId) return null; + + var fields = new Dictionary(StringComparer.Ordinal) + { + [MqttTagConfigKeys.GroupId] = scope.GroupId, + [MqttTagConfigKeys.EdgeNodeId] = edgeNodeId, + [MqttTagConfigKeys.MetricName] = metricName, + }; + + // Omitted, not blank, for a node-level metric: the factory normalises "" to absent anyway, but + // an emitted empty device id would read as "a device whose id is empty" to anything else looking. + if (scope.DeviceId is { } deviceId) fields[MqttTagConfigKeys.DeviceId] = deviceId; + return fields; } /// @@ -489,13 +553,16 @@ internal sealed class MqttBrowseSession : IRebirthCapableBrowseSession return; } - var scope = new SparkplugScope(groupId, edgeNodeId); + // Two scopes, and the difference is load-bearing: the edge node's own scope has no device, and + // stamping the device one on it would make an NBIRTH metric claim a device it does not belong to. + var edgeScope = new SparkplugScope(groupId, edgeNodeId); + var ownerScope = deviceId is null ? edgeScope : edgeScope with { DeviceId = deviceId }; var group = GetOrCreate(_roots, groupId, groupId, BrowseNodeKind.Folder, new SparkplugScope(groupId, null)); if (group is null) return; var edgePath = groupId + "/" + edgeNodeId; - var edge = GetOrCreate(group.Children, edgePath, edgeNodeId, BrowseNodeKind.Folder, scope); + var edge = GetOrCreate(group.Children, edgePath, edgeNodeId, BrowseNodeKind.Folder, edgeScope); if (edge is null) return; var owner = edge; @@ -503,7 +570,7 @@ internal sealed class MqttBrowseSession : IRebirthCapableBrowseSession if (deviceId is not null) { ownerPath = edgePath + "/" + deviceId; - owner = GetOrCreate(edge.Children, ownerPath, deviceId, BrowseNodeKind.Folder, scope); + owner = GetOrCreate(edge.Children, ownerPath, deviceId, BrowseNodeKind.Folder, ownerScope); if (owner is null) return; } @@ -525,7 +592,7 @@ internal sealed class MqttBrowseSession : IRebirthCapableBrowseSession continue; } - var node = GetOrCreate(owner.Children, metricPath, name, BrowseNodeKind.Leaf, scope); + var node = GetOrCreate(owner.Children, metricPath, name, BrowseNodeKind.Leaf, ownerScope); if (node is null) return; // node cap tripped node.RecordMetric(new SparkplugMetricInfo(name, metric.DataType, parsed.Type, metric.Alias)); @@ -956,7 +1023,13 @@ internal sealed class MqttBrowseSession : IRebirthCapableBrowseSession /// The owning edge-node id, or on a group node (which addresses every /// edge node beneath it). /// - private sealed record SparkplugScope(string GroupId, string? EdgeNodeId); + /// + /// The owning device id, or when the node belongs to the edge node + /// itself. Carried for the address (a tag's binding tuple), not for rebirth — Sparkplug + /// has no device-scoped rebirth, so reads only the pair + /// above. + /// + private sealed record SparkplugScope(string GroupId, string? EdgeNodeId, string? DeviceId = null); /// One metric, exactly as a birth certificate declared it. No payload bytes are retained. /// The metric name — the tag's stable binding key across rebirths. diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Contracts/MqttTagConfigKeys.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Contracts/MqttTagConfigKeys.cs new file mode 100644 index 00000000..4a11784b --- /dev/null +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Contracts/MqttTagConfigKeys.cs @@ -0,0 +1,40 @@ +namespace ZB.MOM.WW.OtOpcUa.Driver.Mqtt; + +/// +/// The TagConfig JSON key names that make up an MQTT tag's address, in the two shapes +/// the driver binds by: a single (Plain) or the +/// /// +/// tuple (Sparkplug B). +/// +/// +/// +/// These exist because the address is produced in one place and consumed in another, and the +/// two used to agree only by coincidence: the AdminUI browse-commit mapper writes the blob, +/// reads it, and a key-name drift between them is not a +/// compile error — it is a tag that deploys clean, resolves to nothing, and reports +/// BadNodeIdUnknown at runtime with no authoring-time signal at all. Single-sourcing +/// the names makes that drift impossible rather than merely unlikely. +/// +/// +/// Address keys only. The behavioural keys (payloadFormat, jsonPath, +/// dataType, qos, retainSeed) are deliberately not here — nothing produces +/// them from a browse, so they carry no cross-component drift risk. +/// +/// +public static class MqttTagConfigKeys +{ + /// The concrete MQTT topic a Plain-mode tag subscribes to. + public const string Topic = "topic"; + + /// The Sparkplug group id — the first segment of the tag's binding tuple. + public const string GroupId = "groupId"; + + /// The Sparkplug edge-node id. + public const string EdgeNodeId = "edgeNodeId"; + + /// The Sparkplug device id; absent for a metric published by the edge node itself. + public const string DeviceId = "deviceId"; + + /// The Sparkplug metric's stable name — the tag's binding key across rebirths. + public const string MetricName = "metricName"; +} diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Contracts/MqttTagDefinitionFactory.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Contracts/MqttTagDefinitionFactory.cs index 239d84ab..ecdcf121 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Contracts/MqttTagDefinitionFactory.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Contracts/MqttTagDefinitionFactory.cs @@ -77,7 +77,7 @@ public static class MqttTagDefinitionFactory // topic is the whole point of a Plain-mode tag — without one there is nothing to subscribe // to, so an absent/blank value is a hard reject rather than a defaulted empty subscription. - var topic = ReadString(root, "topic"); + var topic = ReadString(root, MqttTagConfigKeys.Topic); if (string.IsNullOrWhiteSpace(topic)) return false; // Strict enum reads: a present-but-invalid (typo'd) value rejects the whole tag @@ -154,9 +154,9 @@ public static class MqttTagDefinitionFactory // The binding tuple. Without all three of these there is nothing a birth certificate could // ever bind the tag to, so an absent/blank value is a hard reject (→ BadNodeIdUnknown) // rather than a definition that can never resolve and reports nothing about why. - var groupId = ReadString(root, "groupId"); - var edgeNodeId = ReadString(root, "edgeNodeId"); - var metricName = ReadString(root, "metricName"); + var groupId = ReadString(root, MqttTagConfigKeys.GroupId); + var edgeNodeId = ReadString(root, MqttTagConfigKeys.EdgeNodeId); + var metricName = ReadString(root, MqttTagConfigKeys.MetricName); if (string.IsNullOrWhiteSpace(groupId) || string.IsNullOrWhiteSpace(edgeNodeId) || string.IsNullOrWhiteSpace(metricName)) @@ -166,7 +166,7 @@ public static class MqttTagDefinitionFactory // Optional: a node-level metric has no device segment. Blank normalises to absent so // "deviceId":"" and an omitted key describe the same scope rather than two. - var deviceId = ReadString(root, "deviceId"); + var deviceId = ReadString(root, MqttTagConfigKeys.DeviceId); if (string.IsNullOrWhiteSpace(deviceId)) deviceId = null; // Strict, but only when present — see the remarks. @@ -180,7 +180,7 @@ public static class MqttTagDefinitionFactory Name: rawPath, // Plain-shape fields carried through verbatim; the Sparkplug ingest path reads none of // them, and a retyped blob's leftovers must not change what the tag means. - Topic: ReadString(root, "topic"), + Topic: ReadString(root, MqttTagConfigKeys.Topic), PayloadFormat: MqttPayloadFormat.Json, JsonPath: RootJsonPath, DataType: dataType, diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Browsing/BrowserSessionService.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Browsing/BrowserSessionService.cs index da01f21c..4af233fc 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Browsing/BrowserSessionService.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Browsing/BrowserSessionService.cs @@ -140,6 +140,10 @@ public sealed class BrowserSessionService( return published; } + /// + public bool CanRequestRebirth(Guid token) => + registry.TryGet(token, out var session) && session is IRebirthCapableBrowseSession { RebirthAvailable: true }; + /// public async Task CloseAsync(Guid token) { diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Browsing/IBrowserSessionService.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Browsing/IBrowserSessionService.cs index 0aa9dc65..0a70f888 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Browsing/IBrowserSessionService.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Browsing/IBrowserSessionService.cs @@ -71,6 +71,21 @@ public interface IBrowserSessionService /// This session's driver has no re-announce action. Task RequestRebirthAsync(Guid token, string scope, CancellationToken ct); + /// + /// True when the open session behind can actually re-announce — i.e. + /// is worth offering. Cheap (no I/O); never throws; false for + /// an unknown/reaped token. + /// + /// Registry handle for the open browse session. + /// True when the picker should draw a Request-rebirth affordance. + /// + /// A capability probe, not an authorization check — the DriverOperator gate lives on + /// and fails closed there. The picker gates on both: this + /// decides whether the action exists at all (a plain-MQTT window has none), the policy decides + /// whether this operator may fire it. + /// + bool CanRequestRebirth(Guid token); + /// Removes the session from the registry and disposes it. No-op for unknown tokens. /// Registry handle for the browse session to close. /// A task that represents the asynchronous operation. diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/DriverBrowseTree.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/DriverBrowseTree.razor index 5437eff1..bdaec81b 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/DriverBrowseTree.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/DriverBrowseTree.razor @@ -34,6 +34,12 @@ /// Fired when the user clicks a leaf (or any node — caller decides what to do with it). [Parameter] public EventCallback OnNodeSelected { get; set; } + /// The same click as , plus the node's captured ancestor-folder + /// path (root→parent) — which is how a caller learns a node's DEPTH, and therefore what kind of thing + /// it is in a driver whose tree levels mean different things (MQTT/Sparkplug: group / edge node / + /// device). Both callbacks fire; wiring only the first is the existing behaviour. + [Parameter] public EventCallback OnNodeSelectedWithPath { get; set; } + /// When true, leaves render a selection checkbox and clicking a leaf toggles it (WP6 "Browse /// device…" multi-select) instead of single-selecting. Folders still expand/navigate as usual. Default /// false preserves the single-select address-picker behavior. @@ -93,10 +99,12 @@ finally { item.Loading = false; StateHasChanged(); } } - private async Task SelectAsync(TreeItem item) + private async Task SelectAsync(TreeItem item, IReadOnlyList ancestorPath) { _selectedNodeIdLocal = item.Node.NodeId; await OnNodeSelected.InvokeAsync(item.Node); + await OnNodeSelectedWithPath.InvokeAsync( + new ZB.MOM.WW.OtOpcUa.AdminUI.Browsing.BrowseLeafSelection(item.Node, ancestorPath)); } private async Task ToggleLeafAsync(TreeItem item, IReadOnlyList ancestorPath) @@ -142,7 +150,7 @@ } else { - @item.Node.DisplayName } @if (item.Node.Kind == BrowseNodeKind.Leaf) diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Raw/RawBrowseModal.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Raw/RawBrowseModal.razor index a3ba3e70..c0ec5ca4 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Raw/RawBrowseModal.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Raw/RawBrowseModal.razor @@ -11,6 +11,8 @@ SupportsOnlineDiscovery (AbCip/TwinCAT/FOCAS); otherwise browse is unavailable and the modal grays out with a tooltip. *@ @implements IAsyncDisposable +@using Microsoft.AspNetCore.Authorization +@using Microsoft.AspNetCore.Components.Authorization @using ZB.MOM.WW.OtOpcUa.AdminUI.Browsing @using ZB.MOM.WW.OtOpcUa.AdminUI.Components.Shared.Drivers @using ZB.MOM.WW.OtOpcUa.AdminUI.Uns @@ -18,6 +20,8 @@ @inject IBrowserSessionService BrowserService @inject IRawTreeService Svc @inject RawTagCsvExportReader ExportReader +@inject AuthenticationStateProvider AuthState +@inject IAuthorizationService AuthorizationService @if (Visible) { @@ -77,7 +81,89 @@ + SelectedLeafIds="_selectedIds" OnLeafToggled="OnLeafToggledAsync" + OnNodeSelectedWithPath="OnScopeNodeSelected" /> + + @if (_canRebirth && _canOperate) + { + @* Sparkplug only: births are never retained, so a quiet-but-healthy plant + shows an EMPTY tree until one lands. This is the only way to fill it on + demand — and the only browse action that publishes to the live broker, + hence the explicit confirm step. *@ +
+
+
+ Request rebirth + + — asks edge nodes to republish their metric set, so this tree fills + without waiting for a natural birth. + +
+ @if (!_rebirthConfirming) + { + + } +
+ + @if (_rebirthTarget is null) + { +
+ Click a group, edge node, device or metric in the tree above to choose the scope. +
+ } + else + { +
+ Scope: @_rebirthTarget.Kind + @_rebirthTarget.Scope +
+
@_rebirthTarget.Detail
+ } + + @if (_rebirthConfirming && _rebirthTarget is not null) + { +
+
+ This publishes to the live broker: a Sparkplug + rebirth-request command addressed to @_rebirthTarget.Kind + (@_rebirthTarget.Scope). + @if (_rebirthTarget.IsGroup) + { + + Every edge node observed in this group is addressed; a group with + more than @MqttGroupRebirthNodeCap observed edge nodes is refused + outright, and nothing is published. + + } +
+
+ + +
+
+ } + + @if (_rebirthOutcome is not null) + { +
@_rebirthOutcome
+ } + @if (_rebirthError is not null) + { +
@_rebirthError
+ } +
+ }
@_selected.Count tag@(_selected.Count == 1 ? "" : "s") selected. @@ -116,6 +202,11 @@ /// type (the OPC UA Client tree). Driver-typed browsers (AbCip/TwinCAT/FOCAS) report the real type.
 private const string DefaultDataType = "Double"; + /// Mirrors MqttBrowseSession.MaxGroupRebirthNodes for the confirm-step wording. The + /// session owns the rule and refuses whole past it; this is the operator-facing number, and a drift + /// only ever makes the warning less precise, never the refusal wrong. + private const int MqttGroupRebirthNodeCap = 32; + /// Whether the modal is shown. Two-way (@bind-Visible) so the modal can self-close. [Parameter] public bool Visible { get; set; } @@ -154,9 +245,26 @@ private bool _busy; private List _commitErrors = new(); + // Request-rebirth affordance (MQTT/Sparkplug only). _canOperate is defence in depth — the real gate + // is server-side in BrowserSessionService.RequestRebirthAsync, which fails closed. + private bool _canOperate; + private bool _canRebirth; + private RebirthTarget? _rebirthTarget; + private bool _rebirthConfirming; + private bool _rebirthBusy; + private string? _rebirthOutcome; + private string? _rebirthError; + private readonly Dictionary _selected = new(StringComparer.Ordinal); private readonly HashSet _selectedIds = new(StringComparer.Ordinal); + protected override async Task OnInitializedAsync() + { + var auth = await AuthState.GetAuthenticationStateAsync(); + var result = await AuthorizationService.AuthorizeAsync(auth.User, null, AdminUiPolicies.DriverOperator); + _canOperate = result.Succeeded; + } + protected override async Task OnParametersSetAsync() { if (!Visible) @@ -185,6 +293,8 @@ _groupPrefix = null; _canBrowse = false; _disabledReason = null; + ResetRebirthState(); + _canRebirth = false; _resolving = true; StateHasChanged(); @@ -238,7 +348,13 @@ try { var result = await BrowserService.OpenAsync(_driverType, _mergedConfig, default); - if (result.Ok) { _token = result.Token; } + if (result.Ok) + { + _token = result.Token; + // Capability, not authorization: a plain-MQTT window (and every non-MQTT browser) has no + // re-announce action at all, so the affordance must not be drawn for it. + _canRebirth = BrowserService.CanRequestRebirth(_token); + } else { _openError = result.Message; } } finally @@ -263,6 +379,7 @@ // the display name + the default data type. string name = sel.Leaf.DisplayName; string? driverDataType = null; + IReadOnlyDictionary? addressFields = null; try { var attrs = await BrowserService.AttributesAsync(_token, nodeId, default); @@ -270,15 +387,146 @@ { name = string.IsNullOrWhiteSpace(attrs[0].Name) ? sel.Leaf.DisplayName : attrs[0].Name; driverDataType = attrs[0].DriverDataType; + // The STRUCTURED address, for a driver whose binding is a tuple (MQTT/Sparkplug). Never + // reconstructed from the node id: a metric name may contain '/', so the id cannot be + // split back into (group, edgeNode, device?, metric) — see RawBrowseCommitMapper. + addressFields = attrs[0].AddressFields; } } catch { // Attribute lookup is best-effort — a leaf is still selectable with display-name + default type. + // For a tuple-addressed driver that also means "no address", which CommitAsync refuses in words. } _selectedIds.Add(nodeId); - _selected[nodeId] = new SelectedLeaf(nodeId, name, driverDataType, sel.FolderPath); + _selected[nodeId] = new SelectedLeaf(nodeId, name, driverDataType, sel.FolderPath, addressFields); + + // A metric is also a legal rebirth scope (it resolves up to its owning edge node), so a toggle + // moves the scope the same way a folder click does. + SetRebirthTarget(sel.Leaf, sel.FolderPath); + } + + /// Tracks the tree click that defines the Request-rebirth scope (folders and metrics alike). + private void OnScopeNodeSelected(BrowseLeafSelection sel) => SetRebirthTarget(sel.Leaf, sel.FolderPath); + + /// + /// Re-derives the rebirth scope from the clicked node, and — because the scope changed — drops any + /// pending confirmation. A confirm step that survived a selection change would publish to a target + /// the operator is no longer looking at. + /// + private void SetRebirthTarget(BrowseNode node, IReadOnlyList ancestorPath) + { + if (!_canRebirth) { return; } + + var target = DescribeRebirthTarget(node, ancestorPath); + if (_rebirthTarget?.Scope == target.Scope) { return; } + + _rebirthTarget = target; + _rebirthConfirming = false; + _rebirthOutcome = null; + _rebirthError = null; + } + + /// + /// Names what a clicked node means as a rebirth scope. The DEPTH comes from the captured ancestor + /// path, not from splitting the node id — the id is the session's own key and the AdminUI must not + /// reverse-engineer its shape. + /// + /// The clicked browse node; its NodeId is passed to the session verbatim. + /// The node's ancestor display names, root→parent. + /// The described target. + private static RebirthTarget DescribeRebirthTarget(BrowseNode node, IReadOnlyList ancestorPath) + { + var depth = ancestorPath.Count; + var name = node.DisplayName; + + // Sparkplug has no device- or metric-scoped rebirth: an NCMD addresses an EDGE NODE, so a + // deeper selection resolves upward. Say so, rather than letting the operator assume the + // narrower scope they clicked. + if (node.Kind == BrowseNodeKind.Leaf && depth >= 2) + { + return new RebirthTarget( + node.NodeId, + $"edge node {ancestorPath[1]}", + $"Selected the metric '{name}'. Sparkplug addresses rebirth at the edge node, so " + + $"{ancestorPath[1]} republishes its whole metric set.", + IsGroup: false); + } + + return depth switch + { + 0 => new RebirthTarget( + node.NodeId, + $"group {name}", + $"Every edge node observed in group '{name}' is asked to republish.", + IsGroup: true), + 1 => new RebirthTarget( + node.NodeId, + $"edge node {name}", + $"Edge node '{name}' in group '{ancestorPath[0]}' republishes its whole metric set.", + IsGroup: false), + 2 => new RebirthTarget( + node.NodeId, + $"edge node {ancestorPath[1]}", + $"Selected the device '{name}'. Sparkplug addresses rebirth at the edge node, so " + + $"{ancestorPath[1]} republishes its whole metric set.", + IsGroup: false), + _ => new RebirthTarget(node.NodeId, "the selected node", "", IsGroup: false), + }; + } + + /// Arms the confirm step. Publishing is deliberately two clicks — it reaches a live plant broker. + private void BeginRebirth() + { + _rebirthOutcome = null; + _rebirthError = null; + _rebirthConfirming = true; + } + + /// Disarms the confirm step without publishing anything. + private void CancelRebirth() => _rebirthConfirming = false; + + /// + /// The one browse action that publishes. Exceptions are SHOWN, not swallowed: a refusal (an + /// over-wide group scope, a missing DriverOperator grant, an unresolvable scope) is exactly what + /// the operator needs to read, and nothing was published in any of those cases. + /// + private async Task ConfirmRebirthAsync() + { + if (_rebirthTarget is null) { return; } + _rebirthBusy = true; + _rebirthOutcome = null; + _rebirthError = null; + StateHasChanged(); + try + { + var published = await BrowserService.RequestRebirthAsync(_token, _rebirthTarget.Scope, default); + _rebirthOutcome = + $"Rebirth requested for {_rebirthTarget.Kind} — {published} command" + + (published == 1 ? "" : "s") + + " published. Re-expand the tree in a few seconds to see the republished metrics."; + _rebirthConfirming = false; + } + catch (Exception ex) + { + _rebirthError = ex.Message; + } + finally + { + _rebirthBusy = false; + StateHasChanged(); + } + } + + /// Clears every rebirth-panel field (modal re-open, browser close). + private void ResetRebirthState() + { + _rebirthTarget = null; + _rebirthConfirming = false; + _rebirthBusy = false; + _rebirthOutcome = null; + _rebirthError = null; } private async Task CommitAsync() @@ -288,10 +536,21 @@ StateHasChanged(); try { + // Refuse a selection the driver could not bind, IN WORDS — for a tuple-addressed driver + // (MQTT/Sparkplug) a leaf whose attribute lookup returned nothing carries no address, and + // committing it would produce a tag that deploys clean and then reports BadNodeIdUnknown + // forever with nothing to point at. + _commitErrors = _selected.Values + .Select(s => RawBrowseCommitMapper.DescribeUncommittableLeaf(_driverType, s.Name, s.AddressFields)) + .Where(e => e is not null) + .Select(e => e!) + .ToList(); + if (_commitErrors.Count > 0) { return; } + var rows = _selected.Values .Select(s => RawBrowseCommitMapper.MapLeaf( _driverType, s.NodeId, s.Name, s.DriverDataType, DefaultDataType, - _groupPrefix, s.FolderPath, _createGroups)) + _groupPrefix, s.FolderPath, _createGroups, s.AddressFields)) .ToList(); var outcome = await Svc.ImportTagsAsync(_effectiveDeviceId, rows); @@ -314,6 +573,8 @@ { var t = _token; _token = Guid.Empty; + _canRebirth = false; + ResetRebirthState(); Visible = false; _open = false; await VisibleChanged.InvokeAsync(false); @@ -335,5 +596,19 @@ return ValueTask.CompletedTask; } - private sealed record SelectedLeaf(string NodeId, string Name, string? DriverDataType, IReadOnlyList FolderPath); + /// The structured address the browse session stated for this leaf, for a + /// driver whose binding is a tuple rather than the NodeId itself; null for every other driver. + private sealed record SelectedLeaf( + string NodeId, + string Name, + string? DriverDataType, + IReadOnlyList FolderPath, + IReadOnlyDictionary? AddressFields = null); + + /// A described Request-rebirth scope: what gets published, and to what. + /// The browse node id handed to the session verbatim. + /// What the scope resolves to, in operator words (e.g. "edge node EdgeA"). + /// The one-line consequence, spelled out before the confirm step. + /// True for a whole-group fan-out — the bounded, all-or-nothing case. + private sealed record RebirthTarget(string Scope, string Kind, string Detail, bool IsGroup); } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/RawBrowseCommitMapper.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/RawBrowseCommitMapper.cs index 7ee70dc3..47ef905d 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/RawBrowseCommitMapper.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/RawBrowseCommitMapper.cs @@ -3,6 +3,7 @@ using ZB.MOM.WW.OtOpcUa.AdminUI.Uns.TagEditors; using ZB.MOM.WW.OtOpcUa.Commons.Types; using ZB.MOM.WW.OtOpcUa.Configuration.Enums; using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Driver.Mqtt; namespace ZB.MOM.WW.OtOpcUa.AdminUI.Uns; @@ -33,6 +34,9 @@ public static class RawBrowseCommitMapper /// The captured browse-folder nesting above the leaf (root→parent display /// names); mirrored onto nested TagGroups only when is true. /// When true, mirror as nested TagGroups. + /// The leaf's structured address (), + /// for a driver whose binding is a tuple rather than a single reference string; null for every driver + /// whose address is the itself. /// The assembled import row. public static RawTagImportRow MapLeaf( string driverType, @@ -42,10 +46,11 @@ public static class RawBrowseCommitMapper string defaultDataType, string? groupPrefix, IReadOnlyList? folderPath, - bool createGroups) + bool createGroups, + IReadOnlyDictionary? addressFields = null) { var dataType = MapDataType(driverDataType) ?? defaultDataType; - var tagConfig = BuildTagConfig(driverType, fullName); + var tagConfig = BuildTagConfig(driverType, fullName, addressFields); var mirrored = createGroups && folderPath is { Count: > 0 } ? string.Join(RawPaths.Separator, folderPath) : null; @@ -96,13 +101,25 @@ public static class RawBrowseCommitMapper /// address field to and leaving every other field at its model default. Reuses /// the <Driver>TagConfigModel for driver types that have a typed editor; the model-less Galaxy /// driver gets the canonical camelCase attributeRef key directly. + /// + /// MQTT is the one driver whose address is not a single string — a Sparkplug tag binds by a + /// (group, edgeNode, device?, metric) tuple — so it is built from the browse session's stated + /// instead. See . + /// /// /// The owning device's driver type. /// The leaf's driver-side full reference to write into the address field. + /// The leaf's structured address, when the driver binds by a tuple — + /// see . Ignored by every single-reference driver. /// The serialised driver-typed TagConfig JSON. - public static string BuildTagConfig(string driverType, string fullName) + public static string BuildTagConfig( + string driverType, + string fullName, + IReadOnlyDictionary? addressFields = null) { var address = fullName ?? ""; + if (Is(driverType, DriverTypeNames.Mqtt)) + return BuildMqttTagConfig(address, addressFields); if (Is(driverType, DriverTypeNames.OpcUaClient)) return new OpcUaClientTagConfigModel { NodeId = address }.ToJson(); if (Is(driverType, DriverTypeNames.AbCip)) @@ -123,6 +140,103 @@ public static class RawBrowseCommitMapper return WriteSingleKey("address", address); } + /// + /// Builds the TagConfig for a browse-committed MQTT leaf, whose address is a + /// descriptor rather than a single reference string: topic in Plain mode, and the + /// groupId/edgeNodeId/deviceId?/metricName tuple in Sparkplug B mode. + /// + /// The leaf's browse node id — the Plain-mode fallback address only. + /// The structured address the browse session stated. + /// The serialised TagConfig JSON. + /// + /// + /// The address is read from , never parsed out of + /// . A Sparkplug browse node id is + /// {group}/{node}[/{device}]::{metric}, and a metric name legitimately contains / + /// (Node Control/Rebirth) — so splitting the id cannot recover the tuple in general, and a + /// mapper that guessed would silently bind the wrong metric. The session that decoded the birth + /// already holds the decomposition and hands it over; this method only picks the keys it knows. + /// + /// + /// Plain vs Sparkplug is likewise stated, not inferred — the driver type reaching + /// here is just Mqtt, and the mode lives on the driver config, not on the tag. The + /// producing session knows its own mode and says so by which keys it emits: a + /// metricName ⇒ Sparkplug, a topic ⇒ Plain. (The blob deliberately carries no + /// mode key: MqttTagDefinitionFactory takes the shape from the driver's mode and + /// would ignore one, and MqttTagConfigModel re-infers it from these same keys.) + /// + /// + /// Keys are whitelisted, not splatted: these values came off a broker, and everything + /// beyond the address (payload format, JSONPath, QoS, dataType) stays at the driver's own + /// defaults for the operator to author afterwards. A leaf with no stated address at all is + /// rejected before this is reached — see . + /// + /// + private static string BuildMqttTagConfig(string fullName, IReadOnlyDictionary? addressFields) + { + if (TryGetField(addressFields, MqttTagConfigKeys.MetricName) is { } metricName) + { + var o = new JsonObject + { + [MqttTagConfigKeys.GroupId] = TryGetField(addressFields, MqttTagConfigKeys.GroupId) ?? "", + [MqttTagConfigKeys.EdgeNodeId] = TryGetField(addressFields, MqttTagConfigKeys.EdgeNodeId) ?? "", + [MqttTagConfigKeys.MetricName] = metricName, + }; + + // Absent, not blank, for a node-level metric — the two describe the same scope to the + // factory, but only the absent form says "this metric has no device" to a reader. + if (TryGetField(addressFields, MqttTagConfigKeys.DeviceId) is { } deviceId) + o[MqttTagConfigKeys.DeviceId] = deviceId; + + return o.ToJsonString(); + } + + // Plain: the session states the topic; the node id is the same value and is the fallback for a + // session that stated nothing (a shape DescribeUncommittableLeaf refuses upstream). + return WriteSingleKey( + MqttTagConfigKeys.Topic, + TryGetField(addressFields, MqttTagConfigKeys.Topic) ?? fullName); + } + + /// + /// Describes why a selected browse leaf cannot be committed as a working tag, or null when it + /// can. The AdminUI calls this before mapping so an unbindable selection fails at commit, in + /// words. + /// + /// The owning device's driver type. + /// The leaf's browse name, for the message. + /// The structured address the browse session stated, if any. + /// The operator-facing reason, or null when the leaf is committable. + /// + /// Only MQTT can fail this today, and only in one shape: a leaf whose attribute lookup returned + /// nothing, so no address was stated. Committing it anyway is the defect this whole seam exists + /// to close — a Sparkplug tuple cannot be reconstructed from the node id, so the row would deploy + /// clean and then report BadNodeIdUnknown forever with nothing to point at. Every + /// single-reference driver is unaffected: its address IS the node id. + /// + public static string? DescribeUncommittableLeaf( + string driverType, + string browseName, + IReadOnlyDictionary? addressFields) + { + if (!Is(driverType, DriverTypeNames.Mqtt)) return null; + if (TryGetField(addressFields, MqttTagConfigKeys.MetricName) is not null) return null; + if (TryGetField(addressFields, MqttTagConfigKeys.Topic) is not null) return null; + + return $"'{browseName}' could not be committed: the browse session reported no MQTT address for it " + + "(its attribute lookup returned nothing). Re-open the browser and re-select it, or author " + + "the tag manually."; + } + + /// Reads one non-blank address field, or null when it is absent or blank. + /// The stated address fields, or null. + /// The field to read. + /// The trimmed value, or null. + private static string? TryGetField(IReadOnlyDictionary? fields, string key) + => fields is not null && fields.TryGetValue(key, out var v) && !string.IsNullOrWhiteSpace(v) + ? v.Trim() + : null; + /// /// Combines a target-group path prefix with an optional mirrored sub-path, matching the WP5 CSV import /// combine semantics: blank collapses to null, and a present prefix + suffix join with the RawPath diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttBrowseSessionTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttBrowseSessionTests.cs index b2ec5d34..28f128f2 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttBrowseSessionTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttBrowseSessionTests.cs @@ -619,6 +619,100 @@ public sealed class MqttBrowseSessionTests a.SecurityClass.ShouldContain("unsupported"); } + [Fact] + public async Task SparkplugAttributes_StateTheBindingTuple_SoTheCommitNeverParsesTheNodeId() + { + // The address of a Sparkplug tag is (group, edgeNode, device?, metric) — NOT the node id. The + // session already holds the decomposition (it parsed the birth topic), so it states it; the + // AdminUI's browse-commit mapper reads these keys and never splits the id. + await using var s = new MqttBrowseSession(MqttMode.SparkplugB); + s.ObserveBirthForTest("OtOpcUaSim", "EdgeA", "Filler1", "Temperature", SparkplugDataType.Float); + + var a = (await s.AttributesAsync( + "OtOpcUaSim/EdgeA/Filler1::Temperature", TestContext.Current.CancellationToken)).ShouldHaveSingleItem(); + + a.AddressFields.ShouldNotBeNull(); + a.AddressFields![MqttTagConfigKeys.GroupId].ShouldBe("OtOpcUaSim"); + a.AddressFields[MqttTagConfigKeys.EdgeNodeId].ShouldBe("EdgeA"); + a.AddressFields[MqttTagConfigKeys.DeviceId].ShouldBe("Filler1"); + a.AddressFields[MqttTagConfigKeys.MetricName].ShouldBe("Temperature"); + } + + [Fact] + public async Task SparkplugAttributes_NodeLevelMetric_StatesNoDeviceId() + { + // An NBIRTH metric belongs to the edge node itself. An invented/blank device id would be a + // device that does not exist — the same reason the tree hangs it directly under the edge node. + await using var s = new MqttBrowseSession(MqttMode.SparkplugB); + s.ObserveBirthForTest("OtOpcUaSim", "EdgeA", null, "Uptime", SparkplugDataType.Int64); + + var a = (await s.AttributesAsync( + "OtOpcUaSim/EdgeA::Uptime", TestContext.Current.CancellationToken)).ShouldHaveSingleItem(); + + a.AddressFields.ShouldNotBeNull(); + a.AddressFields!.ContainsKey(MqttTagConfigKeys.DeviceId).ShouldBeFalse(); + a.AddressFields[MqttTagConfigKeys.EdgeNodeId].ShouldBe("EdgeA"); + } + + [Fact] + public async Task SparkplugAttributes_MetricNameContainingASlash_IsStatedWhole() + { + // 'Node Control/Rebirth' is a canonical Sparkplug metric name. Its node id is + // 'OtOpcUaSim/EdgeA::Node Control/Rebirth' — un-splittable back into the tuple, which is why the + // tuple travels rather than the id. + await using var s = new MqttBrowseSession(MqttMode.SparkplugB); + s.ObserveBirthForTest("OtOpcUaSim", "EdgeA", null, "Node Control/Rebirth", SparkplugDataType.Boolean); + + var a = (await s.AttributesAsync( + "OtOpcUaSim/EdgeA::Node Control/Rebirth", TestContext.Current.CancellationToken)).ShouldHaveSingleItem(); + + a.AddressFields.ShouldNotBeNull(); + a.AddressFields![MqttTagConfigKeys.MetricName].ShouldBe("Node Control/Rebirth"); + a.AddressFields[MqttTagConfigKeys.EdgeNodeId].ShouldBe("EdgeA"); + } + + [Fact] + public async Task SparkplugAttributes_DeviceAndNodeLevelMetricsOfTheSameName_StateDifferentAddresses() + { + // The two are siblings sharing a label ('EdgeA/Temp' the device folder vs 'EdgeA::Temp' the + // node-level metric). Anything that re-derived the address from the label would collapse them. + await using var s = new MqttBrowseSession(MqttMode.SparkplugB); + s.ObserveBirthForTest("OtOpcUaSim", "EdgeA", null, "Temp", SparkplugDataType.Float); + s.ObserveBirthForTest("OtOpcUaSim", "EdgeA", "Filler1", "Temp", SparkplugDataType.Float); + + var nodeLevel = (await s.AttributesAsync( + "OtOpcUaSim/EdgeA::Temp", TestContext.Current.CancellationToken)).ShouldHaveSingleItem(); + var deviceLevel = (await s.AttributesAsync( + "OtOpcUaSim/EdgeA/Filler1::Temp", TestContext.Current.CancellationToken)).ShouldHaveSingleItem(); + + nodeLevel.AddressFields!.ContainsKey(MqttTagConfigKeys.DeviceId).ShouldBeFalse(); + deviceLevel.AddressFields![MqttTagConfigKeys.DeviceId].ShouldBe("Filler1"); + } + + [Fact] + public async Task PlainAttributes_StateTheTopic_AsTheAddress() + { + await using var s = new MqttBrowseSession(MqttMode.Plain); + s.ObserveTopicForTest("otopcua/fixture/oven/temp", "21.5"); + + var a = (await s.AttributesAsync( + "otopcua/fixture/oven/temp", TestContext.Current.CancellationToken)).ShouldHaveSingleItem(); + + a.AddressFields.ShouldNotBeNull(); + a.AddressFields![MqttTagConfigKeys.Topic].ShouldBe("otopcua/fixture/oven/temp"); + a.AddressFields.ContainsKey(MqttTagConfigKeys.MetricName).ShouldBeFalse(); // Plain has no metric + } + + [Theory] + [InlineData(MqttMode.SparkplugB, true)] + [InlineData(MqttMode.Plain, false)] + public void RebirthAvailable_IsSparkplugOnly(MqttMode mode, bool expected) + { + // The picker draws its Request-rebirth affordance off this. A plain window has no re-announce + // action at all — RequestRebirthAsync refuses one — so offering the button would be a lie. + new MqttBrowseSession(mode).RebirthAvailable.ShouldBe(expected); + } + [Fact] public async Task SparkplugAttributes_ForAFolderNode_AreEmpty() { diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttTagDefinitionFactoryTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttTagDefinitionFactoryTests.cs index 6bd57140..2155beef 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttTagDefinitionFactoryTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttTagDefinitionFactoryTests.cs @@ -221,4 +221,42 @@ public sealed class MqttTagDefinitionFactoryTests [InlineData("[1,2,3]")] // no leading '{' ⇒ not a TagConfig blob at all (mirrors Modbus) public void Inspect_NotATagConfigBlob_ReturnsEmpty(string? reference) => MqttTagDefinitionFactory.Inspect(reference!).ShouldBeEmpty(); + + [Fact] + public void TagConfigKeys_AreTheWireNames_AndAreNotFreeToRename() + { + // MqttTagConfigKeys single-sources the address key names across the producer (the AdminUI + // browse-commit mapper) and the consumer (this factory) — which means a SYMMETRIC rename would + // keep every round-trip test green while silently unbinding every ALREADY-PERSISTED TagConfig + // blob in the config DB. These are wire names; pin them to the literals. + MqttTagConfigKeys.Topic.ShouldBe("topic"); + MqttTagConfigKeys.GroupId.ShouldBe("groupId"); + MqttTagConfigKeys.EdgeNodeId.ShouldBe("edgeNodeId"); + MqttTagConfigKeys.DeviceId.ShouldBe("deviceId"); + MqttTagConfigKeys.MetricName.ShouldBe("metricName"); + } + + [Fact] + public void TagConfigKeys_AreTheKeysTheFactoryActuallyReads() + { + // The pin above is only worth having if the factory really reads THESE keys — a blob written + // entirely from the constants must parse. + var blob = $$""" + {"{{MqttTagConfigKeys.GroupId}}":"G", + "{{MqttTagConfigKeys.EdgeNodeId}}":"E", + "{{MqttTagConfigKeys.DeviceId}}":"D", + "{{MqttTagConfigKeys.MetricName}}":"M"} + """; + + MqttTagDefinitionFactory.FromSparkplugTagConfig(blob, RawPath, out var def).ShouldBeTrue(); + def.GroupId.ShouldBe("G"); + def.EdgeNodeId.ShouldBe("E"); + def.DeviceId.ShouldBe("D"); + def.MetricName.ShouldBe("M"); + + MqttTagDefinitionFactory + .FromTagConfig($$"""{"{{MqttTagConfigKeys.Topic}}":"a/b"}""", RawPath, out var plain) + .ShouldBeTrue(); + plain.Topic.ShouldBe("a/b"); + } } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Browsing/BrowserSessionServiceTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Browsing/BrowserSessionServiceTests.cs index b3bf8553..7c068671 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Browsing/BrowserSessionServiceTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Browsing/BrowserSessionServiceTests.cs @@ -290,6 +290,27 @@ public sealed class BrowserSessionServiceTests () => service.RequestRebirthAsync(Guid.NewGuid(), "Plant1/EdgeA", CancellationToken.None)); } + [Fact] + public void CanRequestRebirth_IsTrue_OnlyForASessionThatAdvertisesTheAction() + { + // The picker draws its Request-rebirth affordance off this. A session type CAN re-announce + // (MqttBrowseSession implements the interface) while a given INSTANCE cannot — a plain-MQTT + // window publishes nothing, ever — so a type test alone would draw a button that only throws. + var registry = new BrowseSessionRegistry(); + var sparkplug = new FakeRebirthCapableBrowseSession(); + var plain = new FakeRebirthCapableBrowseSession { RebirthAvailable = false }; + var plainOldSession = new FakeBrowseSession(); + registry.Register(sparkplug); + registry.Register(plain); + registry.Register(plainOldSession); + var service = NewServiceWithAuthz(registry, authorized: true); + + service.CanRequestRebirth(sparkplug.Token).ShouldBeTrue(); + service.CanRequestRebirth(plain.Token).ShouldBeFalse(); + service.CanRequestRebirth(plainOldSession.Token).ShouldBeFalse(); + service.CanRequestRebirth(Guid.NewGuid()).ShouldBeFalse(); // unknown/reaped token + } + /// A browse session that records the rebirth scopes it was asked for. private sealed class FakeRebirthCapableBrowseSession : IRebirthCapableBrowseSession { @@ -299,6 +320,9 @@ public sealed class BrowserSessionServiceTests public int Published { get; init; } = 1; + /// Whether this fake session advertises the action (a plain-MQTT window does not). + public bool RebirthAvailable { get; init; } = true; + public List Scopes { get; } = []; public Task RequestRebirthAsync(string scope, CancellationToken cancellationToken) diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/RawBrowseCommitMapperTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/RawBrowseCommitMapperTests.cs index f920cb57..e414184b 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/RawBrowseCommitMapperTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/RawBrowseCommitMapperTests.cs @@ -5,6 +5,7 @@ using Xunit; using ZB.MOM.WW.OtOpcUa.AdminUI.Uns; using ZB.MOM.WW.OtOpcUa.Configuration.Enums; using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Driver.Mqtt; namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests.Uns; @@ -96,6 +97,204 @@ public sealed class RawBrowseCommitMapperTests JsonNode.Parse(json)!.AsObject()["address"]!.GetValue().ShouldBe("40001"); } + // ---- BuildTagConfig: MQTT (the address is a DESCRIPTOR, not a single reference string) -------- + + /// The Sparkplug address a browse session states for a metric under a device. + /// The metric name to state. + /// The stated address fields. + private static Dictionary SparkplugFields(string metricName = "Temperature") => new() + { + [MqttTagConfigKeys.GroupId] = "OtOpcUaSim", + [MqttTagConfigKeys.EdgeNodeId] = "EdgeA", + [MqttTagConfigKeys.DeviceId] = "Filler1", + [MqttTagConfigKeys.MetricName] = metricName, + }; + + [Fact] + public void BuildTagConfig_Mqtt_plain_writes_the_topic_key_the_factory_actually_reads() + { + // The generic "address" fallback produced a blob MqttTagDefinitionFactory cannot read at all — + // a browse-committed tag that deploys clean and then reports BadNodeIdUnknown forever. + var json = RawBrowseCommitMapper.BuildTagConfig( + "Mqtt", + "otopcua/fixture/oven/temp", + new Dictionary { [MqttTagConfigKeys.Topic] = "otopcua/fixture/oven/temp" }); + + JsonNode.Parse(json)!.AsObject()["topic"]!.GetValue().ShouldBe("otopcua/fixture/oven/temp"); + } + + [Fact] + public void BuildTagConfig_Mqtt_plain_falls_back_to_the_node_id_when_no_field_was_stated() + { + // A Plain browse node id IS the topic, so the fallback is exact — unlike Sparkplug, where the + // tuple is unrecoverable and DescribeUncommittableLeaf refuses instead. + var json = RawBrowseCommitMapper.BuildTagConfig("Mqtt", "otopcua/fixture/oven/temp"); + + JsonNode.Parse(json)!.AsObject()["topic"]!.GetValue().ShouldBe("otopcua/fixture/oven/temp"); + } + + [Fact] + public void BuildTagConfig_Mqtt_sparkplug_writes_the_binding_tuple_from_the_stated_fields() + { + var json = RawBrowseCommitMapper.BuildTagConfig( + "Mqtt", "OtOpcUaSim/EdgeA/Filler1::Temperature", SparkplugFields()); + + var o = JsonNode.Parse(json)!.AsObject(); + o["groupId"]!.GetValue().ShouldBe("OtOpcUaSim"); + o["edgeNodeId"]!.GetValue().ShouldBe("EdgeA"); + o["deviceId"]!.GetValue().ShouldBe("Filler1"); + o["metricName"]!.GetValue().ShouldBe("Temperature"); + o.ContainsKey("topic").ShouldBeFalse(); // a Sparkplug tag has no per-tag topic + o.ContainsKey("address").ShouldBeFalse(); // and never the generic fallback key + } + + [Fact] + public void BuildTagConfig_Mqtt_sparkplug_node_level_metric_omits_deviceId_entirely() + { + var fields = SparkplugFields(); + fields.Remove(MqttTagConfigKeys.DeviceId); + + var json = RawBrowseCommitMapper.BuildTagConfig("Mqtt", "OtOpcUaSim/EdgeA::Temperature", fields); + + JsonNode.Parse(json)!.AsObject().ContainsKey("deviceId").ShouldBeFalse(); + } + + [Fact] + public void BuildTagConfig_Mqtt_sparkplug_takes_the_metric_name_from_the_fields_not_the_node_id() + { + // THE case the whole seam exists for: a metric name containing '/' makes the node id + // '{group}/{edge}/{device}::{metric}' un-splittable — any parse would bind the wrong metric. + var json = RawBrowseCommitMapper.BuildTagConfig( + "Mqtt", + "OtOpcUaSim/EdgeA/Filler1::Node Control/Rebirth", + SparkplugFields("Node Control/Rebirth")); + + var o = JsonNode.Parse(json)!.AsObject(); + o["metricName"]!.GetValue().ShouldBe("Node Control/Rebirth"); + o["deviceId"]!.GetValue().ShouldBe("Filler1"); + } + + [Fact] + public void BuildTagConfig_Mqtt_sparkplug_survives_a_group_id_containing_the_metric_separator() + { + // A group id is an arbitrary MQTT topic segment and MAY contain '::'. Splitting the node id on + // the first '::' would read the group as 'odd' and the metric as 'group/EdgeA::Temperature'. + var fields = SparkplugFields(); + fields[MqttTagConfigKeys.GroupId] = "odd::group"; + + var json = RawBrowseCommitMapper.BuildTagConfig( + "Mqtt", "odd::group/EdgeA/Filler1::Temperature", fields); + + var o = JsonNode.Parse(json)!.AsObject(); + o["groupId"]!.GetValue().ShouldBe("odd::group"); + o["metricName"]!.GetValue().ShouldBe("Temperature"); + } + + [Fact] + public void BuildTagConfig_Mqtt_sparkplug_blob_deserializes_through_the_REAL_driver_factory() + { + // The round trip that would have caught the defect: the committed blob is fed to the factory the + // deployed driver actually uses. Anything else is a test of the mapper agreeing with itself. + var json = RawBrowseCommitMapper.BuildTagConfig( + "Mqtt", + "OtOpcUaSim/EdgeA/Filler1::Node Control/Rebirth", + SparkplugFields("Node Control/Rebirth")); + + MqttTagDefinitionFactory + .FromSparkplugTagConfig(json, "Plant/Mqtt/dev1/Rebirth", out var def) + .ShouldBeTrue(); + + def.Name.ShouldBe("Plant/Mqtt/dev1/Rebirth"); // v3: identity is the RawPath + def.GroupId.ShouldBe("OtOpcUaSim"); + def.EdgeNodeId.ShouldBe("EdgeA"); + def.DeviceId.ShouldBe("Filler1"); + def.MetricName.ShouldBe("Node Control/Rebirth"); + def.DataTypeAuthored.ShouldBeFalse(); // no dataType key ⇒ take the birth's declared type + } + + [Fact] + public void BuildTagConfig_Mqtt_plain_blob_deserializes_through_the_REAL_driver_factory() + { + var json = RawBrowseCommitMapper.BuildTagConfig( + "Mqtt", + "otopcua/fixture/oven/temp", + new Dictionary { [MqttTagConfigKeys.Topic] = "otopcua/fixture/oven/temp" }); + + MqttTagDefinitionFactory.FromTagConfig(json, "Plant/Mqtt/dev1/Temp", out var def).ShouldBeTrue(); + def.Topic.ShouldBe("otopcua/fixture/oven/temp"); + def.Name.ShouldBe("Plant/Mqtt/dev1/Temp"); + } + + [Fact] + public void The_round_trip_test_is_falsifiable_a_wrong_key_name_is_rejected_by_the_factory() + { + // Control: mutate the emitted key name (what the pre-fix mapper effectively did, writing + // "address") and the factory must REFUSE it. Without this, the round-trip assertions above could + // pass off any blob the factory happened to tolerate. + var wrong = new JsonObject + { + ["group"] = "OtOpcUaSim", // not "groupId" + ["edgeNodeId"] = "EdgeA", + ["metricName"] = "Temperature", + }.ToJsonString(); + + MqttTagDefinitionFactory.FromSparkplugTagConfig(wrong, "Plant/Mqtt/dev1/T", out _).ShouldBeFalse(); + + // And the exact blob the pre-fix generic fallback produced. + var legacy = new JsonObject { ["address"] = "OtOpcUaSim/EdgeA/Filler1::Temperature" }.ToJsonString(); + MqttTagDefinitionFactory.FromSparkplugTagConfig(legacy, "Plant/Mqtt/dev1/T", out _).ShouldBeFalse(); + MqttTagDefinitionFactory.FromTagConfig(legacy, "Plant/Mqtt/dev1/T", out _).ShouldBeFalse(); + } + + [Fact] + public void DescribeUncommittableLeaf_refuses_an_mqtt_leaf_whose_address_was_never_stated() + { + // No stated address ⇒ no bindable tag. Failing here, in words, is the whole point: the old path + // committed it silently and the operator learned about it as a runtime BadNodeIdUnknown. + var error = RawBrowseCommitMapper.DescribeUncommittableLeaf("Mqtt", "Temperature", addressFields: null); + + error.ShouldNotBeNull(); + error!.ShouldContain("Temperature"); + } + + [Theory] + [InlineData(MqttTagConfigKeys.Topic, "otopcua/fixture/oven/temp")] + [InlineData(MqttTagConfigKeys.MetricName, "Temperature")] + public void DescribeUncommittableLeaf_accepts_either_stated_mqtt_shape(string key, string value) + => RawBrowseCommitMapper + .DescribeUncommittableLeaf("Mqtt", "leaf", new Dictionary { [key] = value }) + .ShouldBeNull(); + + [Fact] + public void DescribeUncommittableLeaf_never_blocks_a_single_reference_driver() + { + // Every other driver's address IS the node id — they state no fields and must stay committable. + foreach (var driver in new[] { "OpcUaClient", "AbCip", "TwinCAT", "S7", "GalaxyMxGateway", "Modbus" }) + RawBrowseCommitMapper.DescribeUncommittableLeaf(driver, "leaf", addressFields: null).ShouldBeNull(); + } + + [Fact] + public void MapLeaf_flows_the_stated_address_fields_into_the_committed_row() + { + var row = RawBrowseCommitMapper.MapLeaf( + driverType: "Mqtt", + fullName: "OtOpcUaSim/EdgeA/Filler1::Temperature", + browseName: "Temperature", + driverDataType: "Float32", + defaultDataType: "Double", + groupPrefix: null, + folderPath: new[] { "OtOpcUaSim", "EdgeA", "Filler1" }, + createGroups: false, + addressFields: SparkplugFields()); + + row.Tag.Name.ShouldBe("Temperature"); + row.Tag.DataType.ShouldBe("Float"); + MqttTagDefinitionFactory + .FromSparkplugTagConfig(row.Tag.TagConfig, "Plant/Mqtt/dev1/Temperature", out var def) + .ShouldBeTrue(); + def.MetricName.ShouldBe("Temperature"); + } + // ---- CombineGroupPath ----------------------------------------------------------------------- [Theory]