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 42c5a951..e5a85ce7 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 @@ -10,9 +10,9 @@ {"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": "completed", "blockedBy": [7]}, - {"id": 9, "subject": "Task 9: UntilStable settle loop (FOCAS)", "status": "pending", "blockedBy": [8], "parallelWith": [11]}, + {"id": 9, "subject": "Task 9: UntilStable settle loop (FOCAS)", "status": "completed", "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]}, + {"id": 11, "subject": "Task 11: BrowserSessionService universal fallback + CanBrowse", "status": "completed", "blockedBy": [8], "parallelWith": [9]}, {"id": 12, "subject": "Task 12: DI registration in AddAdminUI", "status": "pending", "blockedBy": [11], "parallelWith": [10]}, {"id": 13, "subject": "Task 13: TagModal -> editor parameter plumbing (DriverType + GetDriverConfigJson)", "status": "completed", "blockedBy": [0], "parallelWith": [1, 2]}, {"id": 14, "subject": "Task 14: AbCip picker body universal browse mode", "status": "pending", "blockedBy": [10, 12, 13, 3], "parallelWith": [15, 16]}, diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Browsing/DiscoveryDriverBrowser.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Browsing/DiscoveryDriverBrowser.cs index 9f50bb3e..5a47d01f 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Browsing/DiscoveryDriverBrowser.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Browsing/DiscoveryDriverBrowser.cs @@ -95,7 +95,7 @@ public sealed class DiscoveryDriverBrowser : IUniversalDriverBrowser openCts.CancelAfter(_openTimeout); await driver.InitializeAsync(patched, openCts.Token).ConfigureAwait(false); - return await DiscoverTreeAsync(disc, openCts.Token).ConfigureAwait(false); + return await DiscoverTreeAsync(disc, driverType, openCts.Token).ConfigureAwait(false); } finally { @@ -103,14 +103,44 @@ public sealed class DiscoveryDriverBrowser : IUniversalDriverBrowser } } - // Task 9 replaces this with the UntilStable settle version. - private static async Task DiscoverTreeAsync(ITagDiscovery disc, CancellationToken ct) + private static readonly TimeSpan SettleInterval = TimeSpan.FromSeconds(1); + + private async Task DiscoverTreeAsync(ITagDiscovery disc, string driverType, CancellationToken ct) { - var builder = new CapturingAddressSpaceBuilder(NodeCap); + if (disc.RediscoverPolicy != DiscoveryRediscoverPolicy.UntilStable) + { + 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); + await disc.DiscoverAsync(builder, ct).ConfigureAwait(false); #pragma warning restore OTOPCUA0001 - return builder.Build(); + return builder.Build(); + } + + // UntilStable (FOCAS): the discovered shape fills in asynchronously after connect — + // re-capture until non-empty and stable across two consecutive passes, bounded by the + // open-timeout. Mirrors the deploy-time contract in DriverInstanceActor. + CapturedTree? previous = null; + while (true) + { + var builder = new CapturingAddressSpaceBuilder(NodeCap); + try + { +#pragma warning disable OTOPCUA0001 // One-shot browse capture, NOT a runtime dispatch path (see above). + await disc.DiscoverAsync(builder, ct).ConfigureAwait(false); +#pragma warning restore OTOPCUA0001 + var current = builder.Build(); + if (current.Count > 0 && previous?.Count == current.Count) return current; + previous = current; + await Task.Delay(SettleInterval, ct).ConfigureAwait(false); + } + catch (OperationCanceledException) when (previous is { Count: > 0 }) + { + _logger.LogWarning( + "UntilStable settle for {DriverType} hit the open-timeout before stabilising; returning the last capture ({Count} nodes)", + driverType, previous.Count); + return previous; // best effort: a fresh-but-unsettled tree beats a failed open + } + } } private async Task BoundedShutdownAsync(IDriver driver, string driverType) 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 60ac40b4..ffb829db 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Browsing/BrowserSessionService.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Browsing/BrowserSessionService.cs @@ -13,7 +13,8 @@ namespace ZB.MOM.WW.OtOpcUa.AdminUI.Browsing; public sealed class BrowserSessionService( IEnumerable browsers, BrowseSessionRegistry registry, - ILogger logger) : IBrowserSessionService + ILogger logger, + IUniversalDriverBrowser universalBrowser) : IBrowserSessionService { /// Upper bound on a single root/expand/attributes call. public static readonly TimeSpan PerCallTimeout = TimeSpan.FromSeconds(20); @@ -25,7 +26,7 @@ public sealed class BrowserSessionService( public async Task OpenAsync(string driverType, string configJson, CancellationToken ct) { if (!_browsersByType.TryGetValue(driverType, out var browser)) - return new(false, $"No browser registered for driver type '{driverType}'.", Guid.Empty); + return await OpenUniversalAsync(driverType, configJson, ct).ConfigureAwait(false); try { var session = await browser.OpenAsync(configJson, ct).ConfigureAwait(false); @@ -40,6 +41,28 @@ public sealed class BrowserSessionService( } } + /// + public bool CanBrowse(string driverType, string configJson) => + _browsersByType.ContainsKey(driverType) || universalBrowser.CanBrowse(driverType, configJson); + + private async Task OpenUniversalAsync(string driverType, string configJson, CancellationToken ct) + { + if (!universalBrowser.CanBrowse(driverType, configJson)) + return new(false, $"No browser registered for driver type '{driverType}'.", Guid.Empty); + try + { + var session = await universalBrowser.OpenAsync(driverType, configJson, ct).ConfigureAwait(false); + registry.Register(session); + return new(true, null, session.Token); + } + catch (Exception ex) + { + logger.LogInformation(ex, + "Universal browse open failed for driverType={DriverType}: {Message}", driverType, ex.Message); + return new(false, ex.Message, Guid.Empty); + } + } + /// public Task> RootAsync(Guid token, CancellationToken ct) => InvokeAsync(token, ct, (s, c) => s.RootAsync(c)); 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 45886b89..9e25fbb5 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Browsing/IBrowserSessionService.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Browsing/IBrowserSessionService.cs @@ -54,6 +54,14 @@ public interface IBrowserSessionService /// Registry handle for the browse session to close. /// A task that represents the asynchronous operation. Task CloseAsync(Guid token); + + /// True when a browse affordance exists for this driver type: a bespoke browser + /// is registered, or the universal browser can serve it. Cheap (no connect); never throws. + /// Drives the picker's Browse-button visibility. + /// The driver type name. + /// The authoring DriverConfig JSON (may be mid-edit). + /// True when the picker should offer a Browse affordance for this driver. + bool CanBrowse(string driverType, string configJson); } /// diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/Browsing/DiscoveryDriverBrowserSettleTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/Browsing/DiscoveryDriverBrowserSettleTests.cs new file mode 100644 index 00000000..2a59f80d --- /dev/null +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/Browsing/DiscoveryDriverBrowserSettleTests.cs @@ -0,0 +1,87 @@ +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 DiscoveryDriverBrowserSettleTests +{ + private static readonly TimeSpan ShortShutdown = TimeSpan.FromMilliseconds(200); + + private static DiscoveryDriverBrowser Browser(IDriverFactory factory, TimeSpan openTimeout) => + new(factory, NullLogger.Instance, openTimeout, ShortShutdown); + + private static void StreamN(IAddressSpaceBuilder builder, int n) + { + for (var i = 0; i < n; i++) + builder.Variable($"T{i}", $"T{i}", + new DriverAttributeInfo($"Dev.T{i}", DriverDataType.Float64, false, null, + SecurityClassification.Operate, false)); + } + + [Fact] + public async Task UntilStable_RerunsUntilNonEmptyAndStable() + { + // Pass 1 streams nothing (FixedTree cache still filling); passes 2+ stream 3 nodes. + var driver = new FakeDiscoveryDriver + { + RediscoverPolicy = DiscoveryRediscoverPolicy.UntilStable, + OnDiscover = (b, idx) => { if (idx >= 2) StreamN(b, 3); return Task.CompletedTask; }, + }; + var browser = Browser(FakeDriverFactory.Of(() => driver), TimeSpan.FromSeconds(30)); + + var session = await browser.OpenAsync("Fake", "{}", CancellationToken.None); + var root = await session.RootAsync(CancellationToken.None); + + root.Count.ShouldBe(3); + driver.DiscoverCalls.ShouldBeGreaterThanOrEqualTo(3); // empty -> 3 -> 3-stable + } + + [Fact] + public async Task UntilStable_NeverStable_ReturnsLastNonEmptyAtTimeout() + { + // Node count grows every pass — never stabilises; on open-timeout return the last capture. + var driver = new FakeDiscoveryDriver + { + RediscoverPolicy = DiscoveryRediscoverPolicy.UntilStable, + OnDiscover = (b, idx) => { StreamN(b, idx); return Task.CompletedTask; }, + }; + var browser = Browser(FakeDriverFactory.Of(() => driver), TimeSpan.FromSeconds(3)); + + var session = await browser.OpenAsync("Fake", "{}", CancellationToken.None); + var root = await session.RootAsync(CancellationToken.None); + + root.ShouldNotBeEmpty(); // last non-empty capture, not an exception + } + + [Fact] + public async Task UntilStable_NeverAnyNodes_FailsAtOpenTimeout() + { + // Every pass empty — no non-empty capture to fall back to, so the timeout surfaces. + var driver = new FakeDiscoveryDriver + { + RediscoverPolicy = DiscoveryRediscoverPolicy.UntilStable, + OnDiscover = (_, _) => Task.CompletedTask, + }; + var browser = Browser(FakeDriverFactory.Of(() => driver), TimeSpan.FromSeconds(2)); + + await Should.ThrowAsync( + () => browser.OpenAsync("Fake", "{}", CancellationToken.None)); + } + + [Fact] + public async Task OncePolicy_SingleDiscoverPass() + { + var driver = new FakeDiscoveryDriver + { + RediscoverPolicy = DiscoveryRediscoverPolicy.Once, + OnDiscover = (b, _) => { StreamN(b, 3); return Task.CompletedTask; }, + }; + var browser = Browser(FakeDriverFactory.Of(() => driver), TimeSpan.FromSeconds(30)); + + await browser.OpenAsync("Fake", "{}", CancellationToken.None); + driver.DiscoverCalls.ShouldBe(1); + } +} 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 d69d8322..1939dfa4 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 @@ -13,7 +13,11 @@ public sealed class BrowserSessionServiceTests { private static BrowserSessionService NewService( BrowseSessionRegistry registry, params IDriverBrowser[] browsers) => - new(browsers, registry, NullLogger.Instance); + new(browsers, registry, NullLogger.Instance, new FakeUniversalDriverBrowser()); + + private static BrowserSessionService NewService( + BrowseSessionRegistry registry, IUniversalDriverBrowser universal, params IDriverBrowser[] browsers) => + new(browsers, registry, NullLogger.Instance, universal); [Fact] public async Task OpenAsync_unknown_driver_type_returns_Ok_false_with_message() @@ -68,6 +72,89 @@ public sealed class BrowserSessionServiceTests result.Message!.ShouldContain("boom"); } + [Fact] + public async Task Open_BespokeBrowserWinsOverUniversal() + { + var registry = new BrowseSessionRegistry(); + var bespokeSession = new FakeBrowseSession(); + var bespoke = new FakeDriverBrowser("AbCip") + { + OpenHandler = (_, _) => Task.FromResult(bespokeSession), + }; + var universal = new FakeUniversalDriverBrowser + { + CanBrowsePredicate = (_, _) => true, // universal would also serve it + OpenHandler = (_, _, _) => Task.FromResult(new FakeBrowseSession()), + }; + var service = NewService(registry, universal, bespoke); + + var result = await service.OpenAsync("AbCip", "{}", CancellationToken.None); + + result.Ok.ShouldBeTrue(); + result.Token.ShouldBe(bespokeSession.Token); // bespoke session, not the universal one + } + + [Fact] + public async Task Open_FallsBackToUniversal_WhenNoBespokeAndCanBrowse() + { + var registry = new BrowseSessionRegistry(); + var universalSession = new FakeBrowseSession(); + var universal = new FakeUniversalDriverBrowser + { + CanBrowsePredicate = (t, _) => t == "AbCip", + OpenHandler = (_, _, _) => Task.FromResult(universalSession), + }; + var service = NewService(registry, universal); // no bespoke browsers + + var result = await service.OpenAsync("AbCip", "{}", CancellationToken.None); + + result.Ok.ShouldBeTrue(); + result.Token.ShouldBe(universalSession.Token); + registry.TryGet(result.Token, out _).ShouldBeTrue(); + } + + [Fact] + public async Task Open_NoBespokeAndCannotBrowse_ReturnsNoBrowserMessage() + { + var registry = new BrowseSessionRegistry(); + var service = NewService(registry, new FakeUniversalDriverBrowser()); // CanBrowse=false + + var result = await service.OpenAsync("Modbus", "{}", CancellationToken.None); + + result.Ok.ShouldBeFalse(); + result.Token.ShouldBe(Guid.Empty); + result.Message!.ShouldContain("Modbus"); + } + + [Fact] + public async Task Open_UniversalThrow_ReturnsOkFalseWithMessage() + { + var registry = new BrowseSessionRegistry(); + var universal = new FakeUniversalDriverBrowser + { + CanBrowsePredicate = (_, _) => true, + OpenHandler = (_, _, _) => throw new InvalidOperationException("universal-boom"), + }; + var service = NewService(registry, universal); + + var result = await service.OpenAsync("AbCip", "{}", CancellationToken.None); + + result.Ok.ShouldBeFalse(); + result.Message!.ShouldContain("universal-boom"); + } + + [Fact] + public void CanBrowse_TrueForBespokeType_TrueForUniversalType_FalseOtherwise() + { + var registry = new BrowseSessionRegistry(); + var universal = new FakeUniversalDriverBrowser { CanBrowsePredicate = (t, _) => t == "AbCip" }; + var service = NewService(registry, universal, new FakeDriverBrowser("Galaxy")); + + service.CanBrowse("Galaxy", "{}").ShouldBeTrue(); // bespoke registered + service.CanBrowse("AbCip", "{}").ShouldBeTrue(); // universal can serve + service.CanBrowse("Modbus", "{}").ShouldBeFalse(); // neither + } + [Fact] public async Task RootAsync_unknown_token_throws_BrowseSessionNotFoundException() { diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Browsing/FakeUniversalDriverBrowser.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Browsing/FakeUniversalDriverBrowser.cs new file mode 100644 index 00000000..9edd82f1 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Browsing/FakeUniversalDriverBrowser.cs @@ -0,0 +1,20 @@ +using ZB.MOM.WW.OtOpcUa.Commons.Browsing; + +namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests.Browsing; + +/// Test double for . CanBrowse is driven by a +/// predicate (default: never); OpenAsync delegates to a handler or returns a fresh session. +internal sealed class FakeUniversalDriverBrowser : IUniversalDriverBrowser +{ + /// CanBrowse gate. Defaults to "no universal affordance for any type". + public Func CanBrowsePredicate { get; init; } = (_, _) => false; + + /// OpenAsync behavior. Defaults to a fresh FakeBrowseSession. + public Func> OpenHandler { get; init; } = + (_, _, _) => Task.FromResult(new FakeBrowseSession()); + + public bool CanBrowse(string driverType, string configJson) => CanBrowsePredicate(driverType, configJson); + + public Task OpenAsync(string driverType, string configJson, CancellationToken ct) => + OpenHandler(driverType, configJson, ct); +}