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 e5a85ce7..ce5d48fd 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 @@ -11,9 +11,9 @@ {"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": "completed", "blockedBy": [8], "parallelWith": [11]}, - {"id": 10, "subject": "Task 10: Capture coalescing + MaxConcurrentCaptures cap", "status": "pending", "blockedBy": [9], "parallelWith": [12]}, + {"id": 10, "subject": "Task 10: Capture coalescing + MaxConcurrentCaptures cap", "status": "completed", "blockedBy": [9], "parallelWith": [12]}, {"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": 12, "subject": "Task 12: DI registration in AddAdminUI", "status": "completed", "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]}, {"id": 15, "subject": "Task 15: TwinCAT picker body universal browse mode", "status": "pending", "blockedBy": [10, 12, 13, 4], "parallelWith": [14, 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 5a47d01f..6c12e808 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Browsing/DiscoveryDriverBrowser.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Browsing/DiscoveryDriverBrowser.cs @@ -1,3 +1,4 @@ +using System.Collections.Concurrent; using System.Text.Json.Nodes; using Microsoft.Extensions.Logging; using ZB.MOM.WW.OtOpcUa.Core.Abstractions; @@ -18,6 +19,9 @@ public sealed class DiscoveryDriverBrowser : IUniversalDriverBrowser /// Node-count cap the capturing builder enforces before marking the tree truncated. public const int NodeCap = 50_000; + /// Global cap on simultaneous captures (§4.2). Coalesced awaiters don't hold a slot. + public const int MaxConcurrentCaptures = 4; + /// 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 = @@ -33,6 +37,8 @@ public sealed class DiscoveryDriverBrowser : IUniversalDriverBrowser private readonly ILogger _logger; private readonly TimeSpan _openTimeout; private readonly TimeSpan _shutdownTimeout; + private readonly ConcurrentDictionary>> _inflight = new(); + private readonly SemaphoreSlim _captureSlots = new(MaxConcurrentCaptures, MaxConcurrentCaptures); /// Production constructor: standard open + shutdown bounds. /// Factory used to construct throwaway/capture driver instances. @@ -75,10 +81,41 @@ public sealed class DiscoveryDriverBrowser : IUniversalDriverBrowser /// public async Task OpenAsync(string driverType, string configJson, CancellationToken ct) { - var tree = await CaptureAsync(driverType, configJson, ct).ConfigureAwait(false); + var key = CaptureKey(driverType, configJson); + // Lazy: exactly one RunCaptureAsync per key even under GetOrAdd races. + var capture = _inflight.GetOrAdd(key, + _ => new Lazy>(() => RunCaptureAsync(key, driverType, configJson))); + // The shared capture runs independent of any single caller (its own OpenTimeout bounds it); + // each caller — including a queued one — observes ITS OWN ct via WaitAsync. + var tree = await capture.Value.WaitAsync(ct).ConfigureAwait(false); return new CapturedTreeBrowseSession(tree); } + private async Task RunCaptureAsync(string key, string driverType, string configJson) + { + try + { + await _captureSlots.WaitAsync().ConfigureAwait(false); // FIFO-ish queue for a slot + try + { + return await CaptureAsync(driverType, configJson, CancellationToken.None).ConfigureAwait(false); + } + finally + { + _captureSlots.Release(); + } + } + finally + { + _inflight.TryRemove(key, out _); // in-flight only — no result caching (Refresh = fresh capture) + } + } + + internal static string CaptureKey(string driverType, string configJson) => + driverType.ToLowerInvariant() + ":" + + Convert.ToHexString(System.Security.Cryptography.SHA256.HashData( + System.Text.Encoding.UTF8.GetBytes(configJson ?? ""))); + private async Task CaptureAsync(string driverType, string configJson, CancellationToken ct) { var patched = PatchForBrowse(driverType, configJson); diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/EndpointRouteBuilderExtensions.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/EndpointRouteBuilderExtensions.cs index d8a1eb98..5f2578c6 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/EndpointRouteBuilderExtensions.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/EndpointRouteBuilderExtensions.cs @@ -3,9 +3,11 @@ using Microsoft.AspNetCore.Components; using Microsoft.AspNetCore.Components.Web; using Microsoft.AspNetCore.Routing; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; using ZB.MOM.WW.OtOpcUa.AdminUI.Hubs; using ZB.MOM.WW.OtOpcUa.AdminUI.Uns; using ZB.MOM.WW.OtOpcUa.Commons.Browsing; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; using ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Browser; using ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Browser; @@ -48,6 +50,12 @@ public static class EndpointRouteBuilderExtensions services.AddScoped(); services.AddSingleton(); services.AddSingleton(); + // Universal Discover-backed fallback browser (Wave 0). IDriverFactory is bound by the + // fused Host's DriverFactoryBootstrap; a standalone AdminUI has none → NullDriverFactory + // → CanBrowse false → pickers gracefully stay manual-entry (design §6). + services.AddSingleton(sp => new DiscoveryDriverBrowser( + sp.GetService() ?? NullDriverFactory.Instance, + sp.GetRequiredService>())); // Roslyn-backed Monaco script-editor analysis (diagnostics/completions/hover/...). services.AddScoped(); diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/Browsing/DiscoveryDriverBrowserConcurrencyTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/Browsing/DiscoveryDriverBrowserConcurrencyTests.cs new file mode 100644 index 00000000..610585a6 --- /dev/null +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/Browsing/DiscoveryDriverBrowserConcurrencyTests.cs @@ -0,0 +1,177 @@ +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 DiscoveryDriverBrowserConcurrencyTests +{ + private static readonly TimeSpan LongOpen = TimeSpan.FromSeconds(30); + private static readonly TimeSpan ShortShutdown = TimeSpan.FromMilliseconds(200); + private static readonly TimeSpan Wait = TimeSpan.FromSeconds(5); + + private static DiscoveryDriverBrowser Browser(IDriverFactory factory) => + new(factory, NullLogger.Instance, LongOpen, ShortShutdown); + + /// A driver gate: signals when Initialize is entered, blocks until released. + private sealed class Gate + { + public readonly SemaphoreSlim Entered = new(0); + public readonly TaskCompletionSource Release = new(TaskCreationOptions.RunContinuationsAsynchronously); + } + + private static FakeDiscoveryDriver Gated(Gate gate) => new() + { + OnInitialize = async ct => + { + gate.Entered.Release(); + await gate.Release.Task.WaitAsync(ct); + }, + }; + + [Fact] + public async Task SameKey_TwoConcurrentOpens_OneCapture_DistinctTokens() + { + var gate = new Gate(); + var factory = FakeDriverFactory.Of(() => Gated(gate)); + var browser = Browser(factory); + + var open1 = browser.OpenAsync("Fake", """{"h":"1"}""", CancellationToken.None); + var open2 = browser.OpenAsync("Fake", """{"h":"1"}""", CancellationToken.None); + + await gate.Entered.WaitAsync(Wait, TestContext.Current.CancellationToken); // one driver entered Initialize + factory.CreateCalls.ShouldBe(1); // coalesced — only one driver constructed + gate.Release.SetResult(); + + var s1 = await open1; + var s2 = await open2; + s1.Token.ShouldNotBe(s2.Token); // distinct sessions... + (await s1.RootAsync(CancellationToken.None)).Count + .ShouldBe((await s2.RootAsync(CancellationToken.None)).Count); // ...over the same tree + ((FakeDiscoveryDriver)factory.Created[0]).InitializeCalls.ShouldBe(1); + } + + [Fact] + public async Task SameKey_DisposingOneSession_LeavesOtherBrowsable() + { + var gate = new Gate(); + var browser = Browser(FakeDriverFactory.Of(() => Gated(gate))); + + var open1 = browser.OpenAsync("Fake", """{"h":"1"}""", CancellationToken.None); + var open2 = browser.OpenAsync("Fake", """{"h":"1"}""", CancellationToken.None); + await gate.Entered.WaitAsync(Wait, TestContext.Current.CancellationToken); + gate.Release.SetResult(); + + var s1 = await open1; + var s2 = await open2; + await s1.DisposeAsync(); + (await s2.RootAsync(CancellationToken.None)).ShouldNotBeEmpty(); // shared immutable tree survives + } + + [Fact] + public async Task SameKey_FailedCapture_FailsBothAwaiters() + { + var factory = FakeDriverFactory.Of(() => new FakeDiscoveryDriver + { + OnDiscover = (_, _) => throw new InvalidOperationException("shared-boom"), + }); + var browser = Browser(factory); + + var open1 = browser.OpenAsync("Fake", """{"h":"1"}""", CancellationToken.None); + var open2 = browser.OpenAsync("Fake", """{"h":"1"}""", CancellationToken.None); + + (await Should.ThrowAsync(() => open1)).Message.ShouldBe("shared-boom"); + (await Should.ThrowAsync(() => open2)).Message.ShouldBe("shared-boom"); + } + + [Fact] + public async Task SameKey_OpenAfterCompletion_RunsFreshCapture() + { + var factory = FakeDriverFactory.Of(() => new FakeDiscoveryDriver()); // ungated, completes fast + var browser = Browser(factory); + + await browser.OpenAsync("Fake", """{"h":"1"}""", CancellationToken.None); + await browser.OpenAsync("Fake", """{"h":"1"}""", CancellationToken.None); + + factory.CreateCalls.ShouldBe(2); // in-flight-only map: second open is a fresh capture (Refresh path) + } + + [Fact] + public async Task DistinctKeys_CapLimitsInflightCaptures() + { + const int cap = DiscoveryDriverBrowser.MaxConcurrentCaptures; // 4 + const int total = cap + 2; // 6 + var release = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var entered = 0; + var factory = new FakeDriverFactory((_, _, _) => new FakeDiscoveryDriver + { + OnInitialize = async ct => { Interlocked.Increment(ref entered); await release.Task.WaitAsync(ct); }, + }); + var browser = Browser(factory); + + var opens = Enumerable.Range(0, total) + .Select(i => browser.OpenAsync("Fake", $$"""{"i":{{i}}}""", CancellationToken.None)) + .ToArray(); + + // Give the semaphore time to admit exactly `cap` captures into Initialize. + await Task.Delay(TimeSpan.FromMilliseconds(500), TestContext.Current.CancellationToken); + Volatile.Read(ref entered).ShouldBe(cap); // at most 4 in flight; the other 2 queue on the semaphore + + release.SetResult(); + var sessions = await Task.WhenAll(opens); + sessions.Length.ShouldBe(total); + factory.CreateCalls.ShouldBe(total); // all eventually run once a slot frees + } + + [Fact] + public async Task QueuedOpen_ObservesCallerCancellation() + { + const int cap = DiscoveryDriverBrowser.MaxConcurrentCaptures; // 4 + var release = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var factory = new FakeDriverFactory((_, _, _) => new FakeDiscoveryDriver + { + OnInitialize = async ct => { await release.Task.WaitAsync(ct); }, + }); + var browser = Browser(factory); + + // Fill all slots with distinct-key captures. + var inflight = Enumerable.Range(0, cap) + .Select(i => browser.OpenAsync("Fake", $$"""{"i":{{i}}}""", CancellationToken.None)) + .ToArray(); + await Task.Delay(TimeSpan.FromMilliseconds(300), TestContext.Current.CancellationToken); + + // A queued open with a cancelable token — it waits for a slot, then the caller cancels. + using var cts = new CancellationTokenSource(); + var queued = browser.OpenAsync("Fake", """{"i":"queued"}""", cts.Token); + cts.Cancel(); + await Should.ThrowAsync(() => queued); + + // The in-flight captures are unaffected. + release.SetResult(); + var done = await Task.WhenAll(inflight); + done.Length.ShouldBe(cap); + } + + [Fact] + public async Task CoalescedAwaiter_CancellationDoesNotCancelSharedCapture() + { + var gate = new Gate(); + var factory = FakeDriverFactory.Of(() => Gated(gate)); + var browser = Browser(factory); + + using var cts1 = new CancellationTokenSource(); + var open1 = browser.OpenAsync("Fake", """{"h":"1"}""", cts1.Token); + var open2 = browser.OpenAsync("Fake", """{"h":"1"}""", CancellationToken.None); + + await gate.Entered.WaitAsync(Wait, TestContext.Current.CancellationToken); + cts1.Cancel(); // first caller abandons + await Should.ThrowAsync(() => open1); + + gate.Release.SetResult(); // shared capture proceeds regardless + var s2 = await open2; + (await s2.RootAsync(CancellationToken.None)).ShouldNotBeEmpty(); + ((FakeDiscoveryDriver)factory.Created[0]).InitializeCalls.ShouldBe(1); + } +}