feat(dcl+ui): rename BrowseOpcUaNode -> ConnectionName-keyed; implement site handler + dialog failure mapping

- BrowseOpcUaNodeCommand: int DataConnectionId -> string ConnectionName (site DataConnectionManagerActor indexes children by name; CentralUI already has the connection name in scope via the dropdown — no extra plumbing across the trust boundary). - IOpcUaBrowseService / OpcUaBrowseService: parameter renamed accordingly. - OpcUaBrowserDialog: collapse the duplicate ConnectionName parameters (display label and routing key are the same string). - Task 10: DataConnectionManagerActor forwards BrowseOpcUaNodeCommand to its child by name (owns ConnectionNotFound); DataConnectionActor adds the receive across all three lifecycle states (Connecting / Connected / Reconnecting) and maps adapter outcomes to BrowseFailureKind (NotBrowsable / ConnectionNotConnected / Timeout / ServerError). - Task 17: SetFailure in OpcUaBrowserDialog implements the full BrowseFailureKind switch with friendly UI messages. - Tests: DataConnectionManagerBrowseHandlerTests covers ConnectionNotFound, NotBrowsable, success, and ConnectionNotConnectedException paths.
2026-05-28 12:09:43 -04:00
parent 6999aedc60
commit d285174597
7 changed files with 313 additions and 13 deletions
@@ -57,7 +57,12 @@

@code {
    [Parameter] public string SiteId { get; set; } = "";
-    [Parameter] public int DataConnectionId { get; set; }
+    /// <summary>
+    /// Name of the site-local data connection. Serves both as the modal-header
+    /// display label AND as the routing key for the browse round-trip — the
+    /// site's <c>DataConnectionManagerActor</c> indexes its children by
+    /// connection name (no id-keyed lookup at the site).
+    /// </summary>
    [Parameter] public string ConnectionName { get; set; } = "";
    [Parameter] public string? InitialNodeId { get; set; }
    [Parameter] public EventCallback<string> OnSelected { get; set; }
@@ -105,7 +110,7 @@
        _rootNodes = new();
        StateHasChanged();

-        var result = await BrowseService.BrowseChildrenAsync(SiteId, DataConnectionId, parentNodeId: null);
+        var result = await BrowseService.BrowseChildrenAsync(SiteId, ConnectionName, parentNodeId: null);
        if (result.Failure is not null)
        {
            SetFailure(result.Failure);
@@ -130,7 +135,7 @@
        {
            node.Loading = true;
            StateHasChanged();
-            var result = await BrowseService.BrowseChildrenAsync(SiteId, DataConnectionId, node.NodeId);
+            var result = await BrowseService.BrowseChildrenAsync(SiteId, ConnectionName, node.NodeId);
            node.Loading = false;

            if (result.Failure is not null)
@@ -155,12 +160,23 @@
        _manualNodeId = node.NodeId;
    }

-    // NOTE: Task 17 will replace this body with the full BrowseFailureKind switch
-    //       that maps each failure kind to a friendly UI message.
+    // Task 17: map each BrowseFailureKind to a friendly UI message. The raw
+    // failure.Message is surfaced verbatim only for ServerError (which carries
+    // the OPC UA SDK's own Bad_* text) and as the default fallback for any
+    // future failure kind added without a UI mapping.
    private void SetFailure(BrowseFailure failure)
    {
        _failure = failure;
-        _failureMessage = failure.Message;
+        _failureMessage = failure.Kind switch
+        {
+            BrowseFailureKind.ConnectionNotFound      => "Connection no longer exists at the site.",
+            BrowseFailureKind.ConnectionNotConnected  => "OPC UA session not connected — retry shortly or use manual entry.",
+            BrowseFailureKind.NotBrowsable            => "This connection does not support browsing.",
+            BrowseFailureKind.Timeout                 => "Browse timed out — the server may be slow. Try again or enter the node id manually.",
+            BrowseFailureKind.ServerError             => $"OPC UA server error: {failure.Message}",
+            _                                          => failure.Message
+        };
+        StateHasChanged();
    }

    private Task RetryRootLoad() => LoadRootAsync();
@@ -21,17 +21,17 @@ public interface IOpcUaBrowseService
 {
    /// <summary>
    /// Enumerates the immediate children of an OPC UA node on the live server
-    /// backing <paramref name="dataConnectionId"/> at <paramref name="siteId"/>.
+    /// backing <paramref name="connectionName"/> at <paramref name="siteId"/>.
    /// Pass <c>null</c> for <paramref name="parentNodeId"/> to browse from the
    /// server root (ObjectsFolder).
    /// </summary>
    /// <param name="siteId">The target site identifier.</param>
-    /// <param name="dataConnectionId">Id of the site-local data connection to browse against.</param>
+    /// <param name="connectionName">Name of the site-local data connection to browse against — the site's <c>DataConnectionManagerActor</c> indexes its children by name.</param>
    /// <param name="parentNodeId">Node to browse, or <c>null</c> to browse from the server root.</param>
    /// <param name="cancellationToken">Cancellation token.</param>
    Task<BrowseOpcUaNodeResult> BrowseChildrenAsync(
        string siteId,
-        int dataConnectionId,
+        string connectionName,
        string? parentNodeId,
        CancellationToken cancellationToken = default);
 }
@@ -37,7 +37,7 @@ public sealed class OpcUaBrowseService : IOpcUaBrowseService
    /// <inheritdoc/>
    public async Task<BrowseOpcUaNodeResult> BrowseChildrenAsync(
        string siteId,
-        int dataConnectionId,
+        string connectionName,
        string? parentNodeId,
        CancellationToken cancellationToken = default)
    {
@@ -56,7 +56,7 @@ public sealed class OpcUaBrowseService : IOpcUaBrowseService
        {
            return await _communication.BrowseOpcUaNodeAsync(
                siteId,
-                new BrowseOpcUaNodeCommand(dataConnectionId, parentNodeId),
+                new BrowseOpcUaNodeCommand(connectionName, parentNodeId),
                cancellationToken);
        }
        catch (TimeoutException ex)
@@ -6,10 +6,17 @@ namespace ZB.MOM.WW.ScadaBridge.Commons.Messages.Management;
 /// Sent from CentralUI to a specific site to enumerate the immediate children
 /// of an OPC UA node on the live server backing the given data connection.
 /// </summary>
-/// <param name="DataConnectionId">Id of the site-local data connection to browse against.</param>
+/// <remarks>
+/// Keyed by <see cref="ConnectionName"/> (not id) because the site-side
+/// <c>DataConnectionManagerActor</c> indexes its children by connection name —
+/// the central UI already has the connection name in scope (dropdown), so a
+/// string carries no extra plumbing across the trust boundary. The central
+/// <c>DataConnections</c> table's id is intentionally not exposed at the site.
+/// </remarks>
+/// <param name="ConnectionName">Name of the site-local data connection to browse against.</param>
 /// <param name="ParentNodeId">Node to browse, or null to browse from the server root (ObjectsFolder).</param>
 public record BrowseOpcUaNodeCommand(
-    int DataConnectionId,
+    string ConnectionName,
    string? ParentNodeId);

 public record BrowseOpcUaNodeResult(
@@ -2,6 +2,7 @@ using Akka.Actor;
 using Akka.Event;
 using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Protocol;
 using ZB.MOM.WW.ScadaBridge.Commons.Messages.DataConnection;
+using ZB.MOM.WW.ScadaBridge.Commons.Messages.Management;
 using ZB.MOM.WW.ScadaBridge.Commons.Types.Enums;
 using ZB.MOM.WW.ScadaBridge.HealthMonitoring;
 using ZB.MOM.WW.ScadaBridge.SiteEventLogging;
@@ -233,6 +234,13 @@ public class DataConnectionActor : UntypedActor, IWithStash, IWithTimers
                // apply it so its state survives into the next ReSubscribeAll.
                HandleSubscribeCompleted(sc);
                break;
+            case BrowseOpcUaNodeCommand browse:
+                // Browse is an interactive design-time query; never stash. The
+                // adapter has no session yet in this state, so reply with a
+                // typed ConnectionNotConnected failure so the dialog can render
+                // an inline banner.
+                HandleBrowse(browse);
+                break;
            case GetHealthReport:
                ReplyWithHealthReport();
                break;
@@ -293,6 +301,9 @@ public class DataConnectionActor : UntypedActor, IWithStash, IWithTimers
            case RetryTagResolution:
                HandleRetryTagResolution();
                break;
+            case BrowseOpcUaNodeCommand browse:
+                HandleBrowse(browse);
+                break;
            case GetHealthReport:
                ReplyWithHealthReport();
                break;
@@ -412,6 +423,12 @@ public class DataConnectionActor : UntypedActor, IWithStash, IWithTimers
                // apply it so its state survives into the next ReSubscribeAll.
                HandleSubscribeCompleted(sc);
                break;
+            case BrowseOpcUaNodeCommand browse:
+                // Browse is design-time and never stashed. While reconnecting
+                // the adapter has no live session, so the adapter call will
+                // throw ConnectionNotConnectedException — mapped by HandleBrowse.
+                HandleBrowse(browse);
+                break;
            case GetHealthReport:
                ReplyWithHealthReport();
                break;
@@ -947,6 +964,72 @@ public class DataConnectionActor : UntypedActor, IWithStash, IWithTimers
        }).PipeTo(sender);
    }

+    // ── OPC UA Tag Browser (interactive design-time query) ──
+
+    /// <summary>
+    /// Handles a <see cref="BrowseOpcUaNodeCommand"/> forwarded by the
+    /// <see cref="DataConnectionManagerActor"/>. The capability check (does
+    /// this adapter support browsing?) and all browse-failure mapping live
+    /// here because the adapter is held by this actor, not the manager.
+    ///
+    /// Failure mapping:
+    /// <list type="bullet">
+    ///   <item><see cref="BrowseFailureKind.NotBrowsable"/> — adapter is not <see cref="IBrowsableDataConnection"/>.</item>
+    ///   <item><see cref="BrowseFailureKind.ConnectionNotConnected"/> — adapter threw <see cref="ConnectionNotConnectedException"/>.</item>
+    ///   <item><see cref="BrowseFailureKind.Timeout"/> — adapter threw <see cref="OperationCanceledException"/>.</item>
+    ///   <item><see cref="BrowseFailureKind.ServerError"/> — any other exception, message carried verbatim.</item>
+    /// </list>
+    ///
+    /// The reply is sent via <c>PipeTo(sender)</c> — the same pattern used by
+    /// <see cref="HandleWrite"/> — so the captured <see cref="Sender"/> is
+    /// safe to use from the continuation (which runs off the actor thread).
+    /// </summary>
+    private void HandleBrowse(BrowseOpcUaNodeCommand command)
+    {
+        var sender = Sender;
+
+        if (_adapter is not IBrowsableDataConnection browsable)
+        {
+            _log.Debug("[{0}] Browse requested but adapter does not implement IBrowsableDataConnection", _connectionName);
+            sender.Tell(new BrowseOpcUaNodeResult(
+                Array.Empty<BrowseNode>(),
+                Truncated: false,
+                new BrowseFailure(
+                    BrowseFailureKind.NotBrowsable,
+                    $"Connection '{_connectionName}' does not support browsing.")));
+            return;
+        }
+
+        _log.Debug("[{0}] Browsing OPC UA children of {1}", _connectionName, command.ParentNodeId ?? "(root)");
+
+        browsable.BrowseChildrenAsync(command.ParentNodeId).ContinueWith(t =>
+        {
+            if (t.IsCompletedSuccessfully)
+            {
+                return new BrowseOpcUaNodeResult(t.Result.Children, t.Result.Truncated, Failure: null);
+            }
+
+            var baseEx = t.Exception?.GetBaseException();
+            return baseEx switch
+            {
+                ConnectionNotConnectedException notConnected => new BrowseOpcUaNodeResult(
+                    Array.Empty<BrowseNode>(),
+                    Truncated: false,
+                    new BrowseFailure(BrowseFailureKind.ConnectionNotConnected, notConnected.Message)),
+                OperationCanceledException => new BrowseOpcUaNodeResult(
+                    Array.Empty<BrowseNode>(),
+                    Truncated: false,
+                    new BrowseFailure(BrowseFailureKind.Timeout, "Browse cancelled.")),
+                _ => new BrowseOpcUaNodeResult(
+                    Array.Empty<BrowseNode>(),
+                    Truncated: false,
+                    new BrowseFailure(
+                        BrowseFailureKind.ServerError,
+                        baseEx?.Message ?? "Unknown browse error.")),
+            };
+        }).PipeTo(sender);
+    }
+
    // ── Tag Resolution Retry (WP-12) ──

    private void HandleRetryTagResolution()
@@ -2,6 +2,7 @@ using Akka.Actor;
 using Akka.Event;
 using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Protocol;
 using ZB.MOM.WW.ScadaBridge.Commons.Messages.DataConnection;
+using ZB.MOM.WW.ScadaBridge.Commons.Messages.Management;
 using ZB.MOM.WW.ScadaBridge.HealthMonitoring;
 using ZB.MOM.WW.ScadaBridge.SiteEventLogging;

@@ -45,6 +46,7 @@ public class DataConnectionManagerActor : ReceiveActor
        Receive<WriteTagRequest>(HandleRouteWrite);
        Receive<RemoveConnectionCommand>(HandleRemoveConnection);
        Receive<GetAllHealthReports>(HandleGetAllHealthReports);
+        Receive<BrowseOpcUaNodeCommand>(HandleBrowse);
    }

    private void HandleCreateConnection(CreateConnectionCommand command)
@@ -111,6 +113,33 @@ public class DataConnectionManagerActor : ReceiveActor
        }
    }

+    /// <summary>
+    /// Routes a <see cref="BrowseOpcUaNodeCommand"/> from the central UI's OPC UA
+    /// Tag Browser to the child <see cref="DataConnectionActor"/> that owns the
+    /// named connection. The manager is the only actor that knows whether a
+    /// connection exists at this site — so it owns the
+    /// <see cref="BrowseFailureKind.ConnectionNotFound"/> failure. Everything
+    /// else (capability check, session state, server errors) lives inside the
+    /// child where the adapter is held.
+    /// </summary>
+    private void HandleBrowse(BrowseOpcUaNodeCommand command)
+    {
+        if (_connectionActors.TryGetValue(command.ConnectionName, out var actor))
+        {
+            actor.Forward(command);
+        }
+        else
+        {
+            _log.Warning("No connection actor for {0} during browse", command.ConnectionName);
+            Sender.Tell(new BrowseOpcUaNodeResult(
+                Array.Empty<BrowseNode>(),
+                Truncated: false,
+                new BrowseFailure(
+                    BrowseFailureKind.ConnectionNotFound,
+                    $"No data connection named '{command.ConnectionName}' at this site.")));
+        }
+    }
+
    private void HandleRemoveConnection(RemoveConnectionCommand command)
    {
        if (_connectionActors.TryGetValue(command.ConnectionName, out var actor))
@@ -0,0 +1,165 @@
+using Akka.Actor;
+using Akka.TestKit.Xunit2;
+using NSubstitute;
+using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Protocol;
+using ZB.MOM.WW.ScadaBridge.Commons.Messages.DataConnection;
+using ZB.MOM.WW.ScadaBridge.Commons.Messages.Management;
+using ZB.MOM.WW.ScadaBridge.Commons.Types.Enums;
+using ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Actors;
+using ZB.MOM.WW.ScadaBridge.HealthMonitoring;
+
+namespace ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests.Actors;
+
+/// <summary>
+/// Task 10 (opcua-tag-browser): the site-side
+/// <see cref="DataConnectionManagerActor"/> + child
+/// <see cref="DataConnectionActor"/> together resolve
+/// <see cref="BrowseOpcUaNodeCommand"/> against the live adapter and surface
+/// every browse outcome as a typed <see cref="BrowseFailure"/>. The split is:
+/// the manager owns <see cref="BrowseFailureKind.ConnectionNotFound"/> (only it
+/// knows the per-site connection set); everything else lives in the child where
+/// the adapter is held — <see cref="BrowseFailureKind.NotBrowsable"/> from the
+/// capability check, <see cref="BrowseFailureKind.ConnectionNotConnected"/> /
+/// <see cref="BrowseFailureKind.Timeout"/> / <see cref="BrowseFailureKind.ServerError"/>
+/// from the adapter call. These tests guard that split.
+/// </summary>
+public class DataConnectionManagerBrowseHandlerTests : TestKit
+{
+    private readonly IDataConnectionFactory _factory;
+    private readonly ISiteHealthCollector _healthCollector;
+    private readonly DataConnectionOptions _options;
+
+    public DataConnectionManagerBrowseHandlerTests()
+        : base(@"akka.loglevel = WARNING")
+    {
+        _factory = Substitute.For<IDataConnectionFactory>();
+        _healthCollector = Substitute.For<ISiteHealthCollector>();
+        _options = new DataConnectionOptions
+        {
+            ReconnectInterval = TimeSpan.FromSeconds(30),
+            TagResolutionRetryInterval = TimeSpan.FromSeconds(30),
+        };
+    }
+
+    [Fact]
+    public void Unknown_connection_name_returns_ConnectionNotFound()
+    {
+        var manager = Sys.ActorOf(Props.Create(() =>
+            new DataConnectionManagerActor(_factory, _options, _healthCollector, null)));
+
+        // No CreateConnectionCommand sent — the manager has zero children, so a
+        // browse against any name must be rejected with ConnectionNotFound
+        // (the manager is the only actor with site-level visibility).
+        manager.Tell(new BrowseOpcUaNodeCommand("unknown-connection", ParentNodeId: null));
+
+        var reply = ExpectMsg<BrowseOpcUaNodeResult>();
+        Assert.NotNull(reply.Failure);
+        Assert.Equal(BrowseFailureKind.ConnectionNotFound, reply.Failure!.Kind);
+        Assert.Empty(reply.Children);
+    }
+
+    [Fact]
+    public void Non_browsable_adapter_returns_NotBrowsable()
+    {
+        // Bare IDataConnection — no IBrowsableDataConnection. The child actor's
+        // capability check must surface this as NotBrowsable.
+        var adapter = Substitute.For<IDataConnection>();
+        adapter.ConnectAsync(Arg.Any<IDictionary<string, string>>(), Arg.Any<CancellationToken>())
+            .Returns(Task.CompletedTask);
+        adapter.Status.Returns(ConnectionHealth.Connected);
+        _factory.Create("OpcUa", Arg.Any<IDictionary<string, string>>()).Returns(adapter);
+
+        var manager = Sys.ActorOf(Props.Create(() =>
+            new DataConnectionManagerActor(_factory, _options, _healthCollector, null)));
+        manager.Tell(new CreateConnectionCommand(
+            "conn-bare", "OpcUa", new Dictionary<string, string>(), null, 3));
+
+        // Give the manager a moment to spawn the child actor. We do not need to
+        // wait for Connected — the browse handler runs in all states.
+        AwaitCondition(
+            () => _factory.ReceivedCalls().Any(c => c.GetMethodInfo().Name == "Create"),
+            TimeSpan.FromSeconds(2));
+
+        manager.Tell(new BrowseOpcUaNodeCommand("conn-bare", ParentNodeId: null));
+
+        var reply = ExpectMsg<BrowseOpcUaNodeResult>(TimeSpan.FromSeconds(3));
+        Assert.NotNull(reply.Failure);
+        Assert.Equal(BrowseFailureKind.NotBrowsable, reply.Failure!.Kind);
+        Assert.Empty(reply.Children);
+    }
+
+    [Fact]
+    public void Success_path_returns_mapped_children()
+    {
+        // Adapter implementing both IDataConnection (so DataConnectionActor can
+        // run its lifecycle) AND IBrowsableDataConnection (so the browse handler
+        // takes the success path).
+        var adapter = Substitute.For<IDataConnection, IBrowsableDataConnection>();
+        ((IDataConnection)adapter).ConnectAsync(Arg.Any<IDictionary<string, string>>(), Arg.Any<CancellationToken>())
+            .Returns(Task.CompletedTask);
+        ((IDataConnection)adapter).Status.Returns(ConnectionHealth.Connected);
+
+        var children = new[]
+        {
+            new BrowseNode("ns=2;s=A", "A", BrowseNodeClass.Variable, HasChildren: false),
+            new BrowseNode("ns=2;s=B", "B", BrowseNodeClass.Object,   HasChildren: true),
+        };
+        ((IBrowsableDataConnection)adapter)
+            .BrowseChildrenAsync(null, Arg.Any<CancellationToken>())
+            .Returns(new BrowseChildrenResult(children, Truncated: false));
+
+        _factory.Create("OpcUa", Arg.Any<IDictionary<string, string>>())
+            .Returns((IDataConnection)adapter);
+
+        var manager = Sys.ActorOf(Props.Create(() =>
+            new DataConnectionManagerActor(_factory, _options, _healthCollector, null)));
+        manager.Tell(new CreateConnectionCommand(
+            "conn-ok", "OpcUa", new Dictionary<string, string>(), null, 3));
+
+        AwaitCondition(
+            () => _factory.ReceivedCalls().Any(c => c.GetMethodInfo().Name == "Create"),
+            TimeSpan.FromSeconds(2));
+
+        manager.Tell(new BrowseOpcUaNodeCommand("conn-ok", ParentNodeId: null));
+
+        var reply = ExpectMsg<BrowseOpcUaNodeResult>(TimeSpan.FromSeconds(3));
+        Assert.Null(reply.Failure);
+        Assert.Equal(2, reply.Children.Count);
+        Assert.Equal("ns=2;s=A", reply.Children[0].NodeId);
+        Assert.Equal("ns=2;s=B", reply.Children[1].NodeId);
+        Assert.False(reply.Truncated);
+    }
+
+    [Fact]
+    public void ConnectionNotConnectedException_maps_to_ConnectionNotConnected()
+    {
+        var adapter = Substitute.For<IDataConnection, IBrowsableDataConnection>();
+        ((IDataConnection)adapter).ConnectAsync(Arg.Any<IDictionary<string, string>>(), Arg.Any<CancellationToken>())
+            .Returns(Task.CompletedTask);
+        ((IDataConnection)adapter).Status.Returns(ConnectionHealth.Connected);
+
+        ((IBrowsableDataConnection)adapter)
+            .BrowseChildrenAsync(Arg.Any<string?>(), Arg.Any<CancellationToken>())
+            .Returns(Task.FromException<BrowseChildrenResult>(
+                new ConnectionNotConnectedException("OPC UA session is not connected.")));
+
+        _factory.Create("OpcUa", Arg.Any<IDictionary<string, string>>())
+            .Returns((IDataConnection)adapter);
+
+        var manager = Sys.ActorOf(Props.Create(() =>
+            new DataConnectionManagerActor(_factory, _options, _healthCollector, null)));
+        manager.Tell(new CreateConnectionCommand(
+            "conn-down", "OpcUa", new Dictionary<string, string>(), null, 3));
+
+        AwaitCondition(
+            () => _factory.ReceivedCalls().Any(c => c.GetMethodInfo().Name == "Create"),
+            TimeSpan.FromSeconds(2));
+
+        manager.Tell(new BrowseOpcUaNodeCommand("conn-down", ParentNodeId: null));
+
+        var reply = ExpectMsg<BrowseOpcUaNodeResult>(TimeSpan.FromSeconds(3));
+        Assert.NotNull(reply.Failure);
+        Assert.Equal(BrowseFailureKind.ConnectionNotConnected, reply.Failure!.Kind);
+        Assert.Empty(reply.Children);
+    }
+}