feat(browse): Wave-0 Batch D — IUniversalDriverBrowser + DiscoveryDriverBrowser core

Task 8: IUniversalDriverBrowser + DiscoveryDriverBrowser open path — construct(+PatchForBrowse) -> connect -> one-shot capture -> bounded (10s) shutdown; CanBrowse gate (catch-throwing TryCreate, defensive teardown of the throwaway instance); deep-merge browse patch. Commons gains Microsoft.Extensions.Logging.Abstractions (ILogger doesn't flow transitively) + InternalsVisibleTo(Commons.Tests). OTOPCUA0001 suppressed on the deliberate one-shot browse-capture DiscoverAsync (not a runtime dispatch path). 12 unit tests green.
2026-07-15 17:27:04 -04:00
parent 15da8d4f0d
commit fa339a5565
6 changed files with 445 additions and 1 deletions
@@ -9,7 +9,7 @@
    {"id": 5, "subject": "Task 5: FOCAS opt-in SupportsOnlineDiscovery=true", "status": "completed", "blockedBy": [1], "parallelWith": [3, 4]},
    {"id": 6, "subject": "Task 6: CapturingAddressSpaceBuilder + captured-tree model", "status": "completed", "blockedBy": [1, 2]},
    {"id": 7, "subject": "Task 7: CapturedTreeBrowseSession", "status": "completed", "blockedBy": [6]},
-    {"id": 8, "subject": "Task 8: IUniversalDriverBrowser + DiscoveryDriverBrowser core open path", "status": "pending", "blockedBy": [7]},
+    {"id": 8, "subject": "Task 8: IUniversalDriverBrowser + DiscoveryDriverBrowser core open path", "status": "completed", "blockedBy": [7]},
    {"id": 9, "subject": "Task 9: UntilStable settle loop (FOCAS)", "status": "pending", "blockedBy": [8], "parallelWith": [11]},
    {"id": 10, "subject": "Task 10: Capture coalescing + MaxConcurrentCaptures cap", "status": "pending", "blockedBy": [9], "parallelWith": [12]},
    {"id": 11, "subject": "Task 11: BrowserSessionService universal fallback + CanBrowse", "status": "pending", "blockedBy": [8], "parallelWith": [9]},
@@ -0,0 +1,148 @@
+using System.Text.Json.Nodes;
+using Microsoft.Extensions.Logging;
+using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
+
+namespace ZB.MOM.WW.OtOpcUa.Commons.Browsing;
+
+/// <summary>See docs/plans/2026-07-15-universal-discovery-browser-design.md §4.2.</summary>
+public sealed class DiscoveryDriverBrowser : IUniversalDriverBrowser
+{
+    /// <summary>Whole-open bound: construct + connect + discover (+ settle). Separate from the
+    /// 20 s per-call browse timeout, which only covers serving from memory.</summary>
+    public static readonly TimeSpan OpenTimeout = TimeSpan.FromSeconds(60);
+
+    /// <summary>Bound on the cleanup ShutdownAsync (R2-01: no unbounded waits — a driver wedged
+    /// in discovery may be equally wedged in shutdown).</summary>
+    public static readonly TimeSpan ShutdownTimeout = TimeSpan.FromSeconds(10);
+
+    /// <summary>Node-count cap the capturing builder enforces before marking the tree truncated.</summary>
+    public const int NodeCap = 50_000;
+
+    /// <summary>Per-type declarative config patch guaranteeing browse mode at open time (§5.1).
+    /// A dictionary entry per config-gated driver — not code.</summary>
+    internal static readonly IReadOnlyDictionary<string, string> BrowsePatches =
+        new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase)
+        {
+            ["AbCip"]   = """{ "EnableControllerBrowse": true }""",
+            ["TwinCAT"] = """{ "EnableControllerBrowse": true }""",
+            ["FOCAS"]   = """{ "FixedTree": { "Enabled": true } }""",
+            // OpcUaClient/BACnet/MTConnect: no patch — their discovery is unconditional.
+        };
+
+    private readonly IDriverFactory _driverFactory;
+    private readonly ILogger<DiscoveryDriverBrowser> _logger;
+    private readonly TimeSpan _openTimeout;
+    private readonly TimeSpan _shutdownTimeout;
+
+    /// <summary>Production constructor: standard open + shutdown bounds.</summary>
+    /// <param name="driverFactory">Factory used to construct throwaway/capture driver instances.</param>
+    /// <param name="logger">Logger for shutdown-timeout warnings.</param>
+    public DiscoveryDriverBrowser(IDriverFactory driverFactory, ILogger<DiscoveryDriverBrowser> logger)
+        : this(driverFactory, logger, OpenTimeout, ShutdownTimeout) { }
+
+    // Test seam: short timeouts.
+    internal DiscoveryDriverBrowser(IDriverFactory driverFactory, ILogger<DiscoveryDriverBrowser> logger,
+        TimeSpan openTimeout, TimeSpan shutdownTimeout)
+    {
+        _driverFactory = driverFactory;
+        _logger = logger;
+        _openTimeout = openTimeout;
+        _shutdownTimeout = shutdownTimeout;
+    }
+
+    /// <inheritdoc />
+    public bool CanBrowse(string driverType, string configJson)
+    {
+        IDriver? probe = null;
+        try
+        {
+            probe = _driverFactory.TryCreate(driverType, $"browse-can-{Guid.NewGuid():N}",
+                PatchForBrowse(driverType, configJson));
+            return probe is ITagDiscovery { SupportsOnlineDiscovery: true };
+        }
+        catch
+        {
+            // TryCreate parses configJson and can throw on mid-authoring JSON. UI affordance
+            // gate — must never throw into the page (§4.2).
+            return false;
+        }
+        finally
+        {
+            if (probe is not null) _ = BoundedShutdownAsync(probe, driverType); // best-effort, never leak
+        }
+    }
+
+    /// <inheritdoc />
+    public async Task<IBrowseSession> OpenAsync(string driverType, string configJson, CancellationToken ct)
+    {
+        var tree = await CaptureAsync(driverType, configJson, ct).ConfigureAwait(false);
+        return new CapturedTreeBrowseSession(tree);
+    }
+
+    private async Task<CapturedTree> CaptureAsync(string driverType, string configJson, CancellationToken ct)
+    {
+        var patched = PatchForBrowse(driverType, configJson);
+        var driver = _driverFactory.TryCreate(driverType, $"browse-{Guid.NewGuid():N}", patched)
+            ?? throw new InvalidOperationException(
+                $"Driver type '{driverType}' is not available in this host.");
+        try
+        {
+            if (driver is not ITagDiscovery disc || !disc.SupportsOnlineDiscovery)
+                throw new InvalidOperationException(
+                    $"Driver type '{driverType}' has no online discovery — author tags manually.");
+
+            using var openCts = CancellationTokenSource.CreateLinkedTokenSource(ct);
+            openCts.CancelAfter(_openTimeout);
+
+            await driver.InitializeAsync(patched, openCts.Token).ConfigureAwait(false);
+            return await DiscoverTreeAsync(disc, openCts.Token).ConfigureAwait(false);
+        }
+        finally
+        {
+            await BoundedShutdownAsync(driver, driverType).ConfigureAwait(false);
+        }
+    }
+
+    // Task 9 replaces this with the UntilStable settle version.
+    private static async Task<CapturedTree> DiscoverTreeAsync(ITagDiscovery disc, CancellationToken ct)
+    {
+        var builder = new CapturingAddressSpaceBuilder(NodeCap);
+#pragma warning disable OTOPCUA0001 // One-shot browse capture, NOT a runtime dispatch path: the driver is a throwaway instance we construct/connect/discover/discard for the picker. CapabilityInvoker (retry/breaker/telemetry) governs live polling, not this admin-side capture.
+        await disc.DiscoverAsync(builder, ct).ConfigureAwait(false);
+#pragma warning restore OTOPCUA0001
+        return builder.Build();
+    }
+
+    private async Task BoundedShutdownAsync(IDriver driver, string driverType)
+    {
+        try
+        {
+            using var cts = new CancellationTokenSource(_shutdownTimeout);
+            // WaitAsync: a token-ignoring wedged driver is ABANDONED (dropped), never awaited forever.
+            await driver.ShutdownAsync(cts.Token).WaitAsync(cts.Token).ConfigureAwait(false);
+        }
+        catch (Exception ex)
+        {
+            _logger.LogWarning(ex,
+                "Browse-capture shutdown for {DriverType} failed/timed out; instance abandoned", driverType);
+        }
+    }
+
+    internal static string PatchForBrowse(string driverType, string configJson)
+    {
+        if (!BrowsePatches.TryGetValue(driverType, out var patch)) return configJson;
+        var config = JsonNode.Parse(string.IsNullOrWhiteSpace(configJson) ? "{}" : configJson)
+            ?.AsObject() ?? new JsonObject();
+        Merge(config, JsonNode.Parse(patch)!.AsObject());
+        return config.ToJsonString();
+    }
+
+    private static void Merge(JsonObject target, JsonObject patch)
+    {
+        foreach (var (key, value) in patch)
+        {
+            if (value is JsonObject po && target[key] is JsonObject to) Merge(to, po);
+            else target[key] = value?.DeepClone();
+        }
+    }
+}
@@ -0,0 +1,28 @@
+namespace ZB.MOM.WW.OtOpcUa.Commons.Browsing;
+
+/// <summary>
+///     Universal fallback browser: captures any discovery-capable driver's DiscoverAsync
+///     output and serves it as a browse session. Resolved by BrowserSessionService only
+///     when no bespoke IDriverBrowser matches the driver type (bespoke-first). A separate
+///     interface — NOT an IDriverBrowser — because BrowserSessionService indexes those
+///     ToDictionary-by-DriverType and the universal browser has no single type.
+/// </summary>
+public interface IUniversalDriverBrowser
+{
+    /// <summary>Cheap gate (construct + inspect, no connect): can this driver type + config be
+    /// browsed? Never throws. Drives the picker's Browse-button visibility.</summary>
+    /// <param name="driverType">The driver type name.</param>
+    /// <param name="configJson">The authoring DriverConfig JSON (may be mid-edit / malformed).</param>
+    /// <returns>True when a discovery-capable driver can be constructed for this type + config.</returns>
+    bool CanBrowse(string driverType, string configJson);
+
+    /// <summary>Construct the driver (with the per-type browse patch), connect, capture one
+    /// full discovery pass (with UntilStable settle), shut the driver down (bounded), and
+    /// return a session over the captured snapshot. Throws on failure — the caller
+    /// (BrowserSessionService) converts to BrowseOpenResult.</summary>
+    /// <param name="driverType">The driver type name.</param>
+    /// <param name="configJson">The DriverConfig JSON to connect with.</param>
+    /// <param name="ct">Cancellation token for the caller's open request.</param>
+    /// <returns>A browse session over the captured discovery snapshot.</returns>
+    Task<IBrowseSession> OpenAsync(string driverType, string configJson, CancellationToken ct);
+}
@@ -7,6 +7,7 @@

    <ItemGroup>
        <PackageReference Include="Akka"/>
+        <PackageReference Include="Microsoft.Extensions.Logging.Abstractions"/>
        <PackageReference Include="ZB.MOM.WW.Audit"/>
    </ItemGroup>

@@ -14,4 +15,8 @@
        <ProjectReference Include="..\ZB.MOM.WW.OtOpcUa.Core.Abstractions\ZB.MOM.WW.OtOpcUa.Core.Abstractions.csproj"/>
    </ItemGroup>

+    <ItemGroup>
+        <InternalsVisibleTo Include="ZB.MOM.WW.OtOpcUa.Commons.Tests"/>
+    </ItemGroup>
+
 </Project>
@@ -0,0 +1,118 @@
+using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
+
+namespace ZB.MOM.WW.OtOpcUa.Commons.Tests.Browsing;
+
+/// <summary>Shared test doubles for the DiscoveryDriverBrowser suites (Tasks 8–10).</summary>
+internal sealed class FakeDiscoveryDriver : IDriver, ITagDiscovery
+{
+    public int InitializeCalls;
+    public int DiscoverCalls;
+    public int ShutdownCalls;
+
+    /// <summary>Ordered lifecycle trace ("init"/"discover"/"shutdown").</summary>
+    public List<string> Trace { get; } = [];
+
+    /// <summary>Completes once ShutdownAsync has finished (even the fire-and-forget CanBrowse path).</summary>
+    public TaskCompletionSource ShutdownDone { get; } = new(TaskCreationOptions.RunContinuationsAsynchronously);
+
+    public bool SupportsOnlineDiscovery { get; init; } = true;
+    public DiscoveryRediscoverPolicy RediscoverPolicy { get; init; } = DiscoveryRediscoverPolicy.Once;
+
+    /// <summary>Discovery behavior: (builder, 1-based call index) → task. Default streams one leaf.</summary>
+    public Func<IAddressSpaceBuilder, int, Task>? OnDiscover { get; init; }
+    public Func<CancellationToken, Task>? OnInitialize { get; init; }
+    public Func<CancellationToken, Task>? OnShutdown { get; init; }
+
+    public string DriverInstanceId { get; }
+    public string DriverType { get; }
+    public string? LastConfigJson { get; private set; }
+
+    public FakeDiscoveryDriver(string driverType = "Fake", string id = "fake-1")
+    {
+        DriverType = driverType;
+        DriverInstanceId = id;
+    }
+
+    public async Task InitializeAsync(string driverConfigJson, CancellationToken cancellationToken)
+    {
+        LastConfigJson = driverConfigJson;
+        Interlocked.Increment(ref InitializeCalls);
+        lock (Trace) Trace.Add("init");
+        if (OnInitialize is not null) await OnInitialize(cancellationToken).ConfigureAwait(false);
+    }
+
+    public async Task DiscoverAsync(IAddressSpaceBuilder builder, CancellationToken cancellationToken)
+    {
+        var idx = Interlocked.Increment(ref DiscoverCalls);
+        lock (Trace) Trace.Add("discover");
+        if (OnDiscover is not null)
+        {
+            await OnDiscover(builder, idx).ConfigureAwait(false);
+            return;
+        }
+        // Default: stream one leaf so the captured tree is non-empty.
+        builder.Variable("Tag1", "Tag1",
+            new DriverAttributeInfo("Dev.Tag1", DriverDataType.Float64, false, null,
+                SecurityClassification.Operate, false));
+    }
+
+    public async Task ShutdownAsync(CancellationToken cancellationToken)
+    {
+        Interlocked.Increment(ref ShutdownCalls);
+        lock (Trace) Trace.Add("shutdown");
+        try
+        {
+            if (OnShutdown is not null) await OnShutdown(cancellationToken).ConfigureAwait(false);
+        }
+        finally
+        {
+            ShutdownDone.TrySetResult();
+        }
+    }
+
+    public Task ReinitializeAsync(string driverConfigJson, CancellationToken cancellationToken) => Task.CompletedTask;
+    public DriverHealth GetHealth() => new(DriverState.Healthy, null, null);
+    public long GetMemoryFootprint() => 0;
+    public Task FlushOptionalCachesAsync(CancellationToken cancellationToken) => Task.CompletedTask;
+}
+
+/// <summary>An IDriver that is NOT ITagDiscovery — exercises the not-discovery-capable gate.</summary>
+internal sealed class FakeNonDiscoveryDriver(string driverType = "Flat", string id = "flat-1") : IDriver
+{
+    public int ShutdownCalls;
+    public TaskCompletionSource ShutdownDone { get; } = new(TaskCreationOptions.RunContinuationsAsynchronously);
+    public string DriverInstanceId { get; } = id;
+    public string DriverType { get; } = driverType;
+    public Task InitializeAsync(string driverConfigJson, CancellationToken cancellationToken) => Task.CompletedTask;
+    public Task ReinitializeAsync(string driverConfigJson, CancellationToken cancellationToken) => Task.CompletedTask;
+    public Task ShutdownAsync(CancellationToken cancellationToken)
+    {
+        Interlocked.Increment(ref ShutdownCalls);
+        ShutdownDone.TrySetResult();
+        return Task.CompletedTask;
+    }
+    public DriverHealth GetHealth() => new(DriverState.Healthy, null, null);
+    public long GetMemoryFootprint() => 0;
+    public Task FlushOptionalCachesAsync(CancellationToken cancellationToken) => Task.CompletedTask;
+}
+
+/// <summary>Configurable IDriverFactory: delegates construction to a supplied func and records what it made.</summary>
+internal sealed class FakeDriverFactory(Func<string, string, string, IDriver?> create) : IDriverFactory
+{
+    public int CreateCalls;
+    public List<IDriver> Created { get; } = [];
+
+    public IDriver? TryCreate(string driverType, string driverInstanceId, string driverConfigJson)
+    {
+        Interlocked.Increment(ref CreateCalls);
+        var d = create(driverType, driverInstanceId, driverConfigJson);
+        if (d is not null) lock (Created) Created.Add(d);
+        return d;
+    }
+
+    public IReadOnlyCollection<string> SupportedTypes { get; } = Array.Empty<string>();
+
+    /// <summary>Factory that always returns a fresh default FakeDiscoveryDriver.</summary>
+    public static FakeDriverFactory Of(Func<FakeDiscoveryDriver> make) =>
+        new((_, _, _) => make());
+}
@@ -0,0 +1,145 @@
+using Microsoft.Extensions.Logging.Abstractions;
+using Shouldly;
+using ZB.MOM.WW.OtOpcUa.Commons.Browsing;
+using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
+using Xunit;
+
+namespace ZB.MOM.WW.OtOpcUa.Commons.Tests.Browsing;
+
+public class DiscoveryDriverBrowserTests
+{
+    private static readonly TimeSpan ShortOpen = TimeSpan.FromSeconds(5);
+    private static readonly TimeSpan ShortShutdown = TimeSpan.FromMilliseconds(200);
+
+    private static DiscoveryDriverBrowser Browser(IDriverFactory factory) =>
+        new(factory, NullLogger<DiscoveryDriverBrowser>.Instance, ShortOpen, ShortShutdown);
+
+    [Fact]
+    public async Task OpenAsync_RunsInitializeDiscoverShutdown_InOrder()
+    {
+        var driver = new FakeDiscoveryDriver();
+        var browser = Browser(FakeDriverFactory.Of(() => driver));
+
+        var session = await browser.OpenAsync("Fake", "{}", CancellationToken.None);
+
+        session.ShouldNotBeNull();
+        driver.Trace.ShouldBe(new[] { "init", "discover", "shutdown" });
+        driver.InitializeCalls.ShouldBe(1);
+        driver.ShutdownCalls.ShouldBe(1);
+    }
+
+    [Fact]
+    public async Task OpenAsync_ShutdownRuns_EvenWhenDiscoverThrows()
+    {
+        var driver = new FakeDiscoveryDriver
+        {
+            OnDiscover = (_, _) => throw new InvalidOperationException("boom-discovery"),
+        };
+        var browser = Browser(FakeDriverFactory.Of(() => driver));
+
+        var ex = await Should.ThrowAsync<InvalidOperationException>(
+            () => browser.OpenAsync("Fake", "{}", CancellationToken.None));
+        ex.Message.ShouldBe("boom-discovery"); // discovery error surfaces, not a shutdown error
+        driver.ShutdownCalls.ShouldBe(1);       // shutdown still ran
+    }
+
+    [Fact]
+    public async Task OpenAsync_UnknownType_ThrowsCleanMessage()
+    {
+        var browser = Browser(new FakeDriverFactory((_, _, _) => null));
+        var ex = await Should.ThrowAsync<InvalidOperationException>(
+            () => browser.OpenAsync("Ghost", "{}", CancellationToken.None));
+        ex.Message.ShouldContain("Ghost");
+    }
+
+    [Fact]
+    public async Task OpenAsync_NotDiscoveryCapable_Throws()
+    {
+        var browser = Browser(new FakeDriverFactory((_, _, _) => new FakeNonDiscoveryDriver()));
+        await Should.ThrowAsync<InvalidOperationException>(
+            () => browser.OpenAsync("Flat", "{}", CancellationToken.None));
+    }
+
+    [Fact]
+    public async Task OpenAsync_FlagFalse_Throws()
+    {
+        var browser = Browser(FakeDriverFactory.Of(() => new FakeDiscoveryDriver { SupportsOnlineDiscovery = false }));
+        await Should.ThrowAsync<InvalidOperationException>(
+            () => browser.OpenAsync("Fake", "{}", CancellationToken.None));
+    }
+
+    [Fact]
+    public void PatchForBrowse_MergesAbCipPatch()
+    {
+        var patched = DiscoveryDriverBrowser.PatchForBrowse("AbCip", """{ "Host": "1.2.3.4" }""");
+        var obj = System.Text.Json.Nodes.JsonNode.Parse(patched)!.AsObject();
+        ((string?)obj["Host"]).ShouldBe("1.2.3.4");
+        ((bool?)obj["EnableControllerBrowse"]).ShouldBe(true);
+
+        // Unknown type: unchanged.
+        DiscoveryDriverBrowser.PatchForBrowse("Modbus", """{ "X": 1 }""").ShouldBe("""{ "X": 1 }""");
+    }
+
+    [Fact]
+    public void PatchForBrowse_DeepMergesFocasFixedTree()
+    {
+        var patched = DiscoveryDriverBrowser.PatchForBrowse("FOCAS", """{ "FixedTree": { "Series": "30i" } }""");
+        var fixedTree = System.Text.Json.Nodes.JsonNode.Parse(patched)!["FixedTree"]!.AsObject();
+        ((string?)fixedTree["Series"]).ShouldBe("30i");   // kept
+        ((bool?)fixedTree["Enabled"]).ShouldBe(true);      // added
+    }
+
+    [Fact]
+    public void CanBrowse_TrueForDiscoveryCapable()
+    {
+        var browser = Browser(FakeDriverFactory.Of(() => new FakeDiscoveryDriver()));
+        browser.CanBrowse("Fake", "{}").ShouldBeTrue();
+    }
+
+    [Fact]
+    public void CanBrowse_FalseWhenTryCreateReturnsNull()
+    {
+        var browser = Browser(new FakeDriverFactory((_, _, _) => null));
+        browser.CanBrowse("Ghost", "{}").ShouldBeFalse();
+    }
+
+    [Fact]
+    public void CanBrowse_FalseWhenTryCreateThrows_NoExceptionEscapes()
+    {
+        var browser = Browser(new FakeDriverFactory((_, _, _) => throw new System.Text.Json.JsonException("bad json")));
+        Should.NotThrow(() => browser.CanBrowse("Fake", "{ half-typed").ShouldBeFalse());
+    }
+
+    [Fact]
+    public async Task CanBrowse_ShutsDownThrowawayInstance()
+    {
+        var driver = new FakeDiscoveryDriver { SupportsOnlineDiscovery = false };
+        var factory = FakeDriverFactory.Of(() => driver);
+        var browser = Browser(factory);
+
+        browser.CanBrowse("Fake", "{}").ShouldBeFalse();
+        // Teardown is fire-and-forget — await its completion.
+        await driver.ShutdownDone.Task.WaitAsync(TimeSpan.FromSeconds(2), TestContext.Current.CancellationToken);
+        driver.ShutdownCalls.ShouldBe(1);
+    }
+
+    [Fact]
+    public async Task BoundedShutdown_AbandonsWedgedDriver()
+    {
+        // Shutdown ignores its token and would never complete; the browser must abandon it
+        // and still return the successfully-captured session within the shutdown bound.
+        var driver = new FakeDiscoveryDriver
+        {
+            OnShutdown = _ => Task.Delay(Timeout.Infinite, CancellationToken.None),
+        };
+        var browser = Browser(FakeDriverFactory.Of(() => driver));
+
+        var sw = System.Diagnostics.Stopwatch.StartNew();
+        var session = await browser.OpenAsync("Fake", "{}", CancellationToken.None);
+        sw.Stop();
+
+        session.ShouldNotBeNull();                       // discovery succeeded → usable session
+        driver.ShutdownCalls.ShouldBe(1);                // shutdown was attempted
+        sw.Elapsed.ShouldBeLessThan(ShortOpen);          // returned on the shutdown bound, not the open timeout
+    }
+}