diff --git a/docs/plans/2026-07-15-universal-discovery-browser-implementation.md.tasks.json b/docs/plans/2026-07-15-universal-discovery-browser-implementation.md.tasks.json index 76fef22c..42c5a951 100644 --- a/docs/plans/2026-07-15-universal-discovery-browser-implementation.md.tasks.json +++ b/docs/plans/2026-07-15-universal-discovery-browser-implementation.md.tasks.json @@ -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]}, diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Browsing/DiscoveryDriverBrowser.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Browsing/DiscoveryDriverBrowser.cs new file mode 100644 index 00000000..9f50bb3e --- /dev/null +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Browsing/DiscoveryDriverBrowser.cs @@ -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; + +/// See docs/plans/2026-07-15-universal-discovery-browser-design.md §4.2. +public sealed class DiscoveryDriverBrowser : IUniversalDriverBrowser +{ + /// Whole-open bound: construct + connect + discover (+ settle). Separate from the + /// 20 s per-call browse timeout, which only covers serving from memory. + public static readonly TimeSpan OpenTimeout = TimeSpan.FromSeconds(60); + + /// Bound on the cleanup ShutdownAsync (R2-01: no unbounded waits — a driver wedged + /// in discovery may be equally wedged in shutdown). + public static readonly TimeSpan ShutdownTimeout = TimeSpan.FromSeconds(10); + + /// Node-count cap the capturing builder enforces before marking the tree truncated. + public const int NodeCap = 50_000; + + /// Per-type declarative config patch guaranteeing browse mode at open time (§5.1). + /// A dictionary entry per config-gated driver — not code. + internal static readonly IReadOnlyDictionary BrowsePatches = + new Dictionary(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 _logger; + private readonly TimeSpan _openTimeout; + private readonly TimeSpan _shutdownTimeout; + + /// Production constructor: standard open + shutdown bounds. + /// Factory used to construct throwaway/capture driver instances. + /// Logger for shutdown-timeout warnings. + public DiscoveryDriverBrowser(IDriverFactory driverFactory, ILogger logger) + : this(driverFactory, logger, OpenTimeout, ShutdownTimeout) { } + + // Test seam: short timeouts. + internal DiscoveryDriverBrowser(IDriverFactory driverFactory, ILogger logger, + TimeSpan openTimeout, TimeSpan shutdownTimeout) + { + _driverFactory = driverFactory; + _logger = logger; + _openTimeout = openTimeout; + _shutdownTimeout = shutdownTimeout; + } + + /// + 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 + } + } + + /// + public async Task OpenAsync(string driverType, string configJson, CancellationToken ct) + { + var tree = await CaptureAsync(driverType, configJson, ct).ConfigureAwait(false); + return new CapturedTreeBrowseSession(tree); + } + + private async Task 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 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(); + } + } +} diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Browsing/IUniversalDriverBrowser.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Browsing/IUniversalDriverBrowser.cs new file mode 100644 index 00000000..ed87b1a4 --- /dev/null +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Browsing/IUniversalDriverBrowser.cs @@ -0,0 +1,28 @@ +namespace ZB.MOM.WW.OtOpcUa.Commons.Browsing; + +/// +/// 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. +/// +public interface IUniversalDriverBrowser +{ + /// Cheap gate (construct + inspect, no connect): can this driver type + config be + /// browsed? Never throws. Drives the picker's Browse-button visibility. + /// The driver type name. + /// The authoring DriverConfig JSON (may be mid-edit / malformed). + /// True when a discovery-capable driver can be constructed for this type + config. + bool CanBrowse(string driverType, string configJson); + + /// 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. + /// The driver type name. + /// The DriverConfig JSON to connect with. + /// Cancellation token for the caller's open request. + /// A browse session over the captured discovery snapshot. + Task OpenAsync(string driverType, string configJson, CancellationToken ct); +} diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/ZB.MOM.WW.OtOpcUa.Commons.csproj b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/ZB.MOM.WW.OtOpcUa.Commons.csproj index 55b52d43..686d3f42 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/ZB.MOM.WW.OtOpcUa.Commons.csproj +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/ZB.MOM.WW.OtOpcUa.Commons.csproj @@ -7,6 +7,7 @@ + @@ -14,4 +15,8 @@ + + + + diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/Browsing/DiscoveryDriverBrowserFakes.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/Browsing/DiscoveryDriverBrowserFakes.cs new file mode 100644 index 00000000..78f45228 --- /dev/null +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/Browsing/DiscoveryDriverBrowserFakes.cs @@ -0,0 +1,118 @@ +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +namespace ZB.MOM.WW.OtOpcUa.Commons.Tests.Browsing; + +/// Shared test doubles for the DiscoveryDriverBrowser suites (Tasks 8–10). +internal sealed class FakeDiscoveryDriver : IDriver, ITagDiscovery +{ + public int InitializeCalls; + public int DiscoverCalls; + public int ShutdownCalls; + + /// Ordered lifecycle trace ("init"/"discover"/"shutdown"). + public List Trace { get; } = []; + + /// Completes once ShutdownAsync has finished (even the fire-and-forget CanBrowse path). + public TaskCompletionSource ShutdownDone { get; } = new(TaskCreationOptions.RunContinuationsAsynchronously); + + public bool SupportsOnlineDiscovery { get; init; } = true; + public DiscoveryRediscoverPolicy RediscoverPolicy { get; init; } = DiscoveryRediscoverPolicy.Once; + + /// Discovery behavior: (builder, 1-based call index) → task. Default streams one leaf. + public Func? OnDiscover { get; init; } + public Func? OnInitialize { get; init; } + public Func? 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; +} + +/// An IDriver that is NOT ITagDiscovery — exercises the not-discovery-capable gate. +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; +} + +/// Configurable IDriverFactory: delegates construction to a supplied func and records what it made. +internal sealed class FakeDriverFactory(Func create) : IDriverFactory +{ + public int CreateCalls; + public List 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 SupportedTypes { get; } = Array.Empty(); + + /// Factory that always returns a fresh default FakeDiscoveryDriver. + public static FakeDriverFactory Of(Func make) => + new((_, _, _) => make()); +} diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/Browsing/DiscoveryDriverBrowserTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/Browsing/DiscoveryDriverBrowserTests.cs new file mode 100644 index 00000000..14dec972 --- /dev/null +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/Browsing/DiscoveryDriverBrowserTests.cs @@ -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.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( + () => 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( + () => 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( + () => browser.OpenAsync("Flat", "{}", CancellationToken.None)); + } + + [Fact] + public async Task OpenAsync_FlagFalse_Throws() + { + var browser = Browser(FakeDriverFactory.Of(() => new FakeDiscoveryDriver { SupportsOnlineDiscovery = false })); + await Should.ThrowAsync( + () => 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 + } +}