diff --git a/src/ZB.MOM.WW.ScadaBridge.Host/Program.cs b/src/ZB.MOM.WW.ScadaBridge.Host/Program.cs index 69fb8252..f6b3cc0f 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Host/Program.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Host/Program.cs @@ -94,8 +94,12 @@ try builder.Services.AddTransport(); // Script-artifact invalidation bus: in-process, per-node pub/sub // for ScriptArtifactsChanged. The Transport bundle importer publishes - // post-commit; the Inbound API compiled-handler cache subscribes as the - // consumer (wired in plan 06). Central-only — it lives beside the importer. + // post-commit; the Inbound API's ScriptArtifactChangeSubscriber (a hosted + // service registered by AddInboundAPI below) consumes ApiMethod notifications + // and invalidates the compiled-handler cache (wired in plan R2-06 T1; the + // per-request revision check remains the correctness fallback). + // SharedScript/Template notifications currently have NO consumer — nothing at + // central caches compiled artifacts of those kinds. Central-only. builder.Services.AddSingleton< ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Scripting.IScriptArtifactChangeBus, ZB.MOM.WW.ScadaBridge.Host.Services.InProcessScriptArtifactChangeBus>(); diff --git a/src/ZB.MOM.WW.ScadaBridge.InboundAPI/ScriptArtifactChangeSubscriber.cs b/src/ZB.MOM.WW.ScadaBridge.InboundAPI/ScriptArtifactChangeSubscriber.cs new file mode 100644 index 00000000..dff2f3cf --- /dev/null +++ b/src/ZB.MOM.WW.ScadaBridge.InboundAPI/ScriptArtifactChangeSubscriber.cs @@ -0,0 +1,87 @@ +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Logging; +using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Scripting; +using ZB.MOM.WW.ScadaBridge.Commons.Messages.Scripting; + +namespace ZB.MOM.WW.ScadaBridge.InboundAPI; + +/// +/// N1 (arch-review 06 round 2): the Inbound API consumer of the script-artifact +/// invalidation contract (docs/plans/2026-07-08-script-artifact-invalidation-contract.md, +/// handoff item (a)). Subscribes to on host +/// start and, for every ApiMethod notification (e.g. a Transport bundle +/// import overwriting method scripts, published post-commit), drops the compiled +/// handler AND the known-bad record via +/// , so the next request +/// recompiles from the fresh row without waiting for the per-request +/// revision-check self-heal. +/// +/// Fast-path only: the revision check in ExecuteAsync remains the +/// correctness fallback — this bus is in-process/per-node and at-least-once +/// (contract §4), so a missed or duplicate notification must always be harmless. +/// The bus is optional (site roles and plain test hosts register none) — the +/// subscriber then no-ops. SharedScript/Template notifications are ignored: this +/// executor caches only ApiMethod handlers. +/// +/// +public sealed class ScriptArtifactChangeSubscriber : IHostedService +{ + private readonly InboundScriptExecutor _executor; + private readonly ILogger _logger; + private readonly IScriptArtifactChangeBus? _bus; + private IDisposable? _subscription; + + /// Initializes the subscriber. + /// The compiled-handler cache to invalidate. + /// Logger instance. + /// The change bus, or null when the host registers none (site roles, tests). + public ScriptArtifactChangeSubscriber( + InboundScriptExecutor executor, + ILogger logger, + IScriptArtifactChangeBus? bus = null) + { + _executor = executor; + _logger = logger; + _bus = bus; + } + + /// + public Task StartAsync(CancellationToken cancellationToken) + { + if (_bus is null) + { + _logger.LogDebug( + "No IScriptArtifactChangeBus registered; inbound handler-cache freshness relies on the per-request revision check alone."); + return Task.CompletedTask; + } + + _subscription = _bus.Subscribe(OnChanged); + return Task.CompletedTask; + } + + /// + public Task StopAsync(CancellationToken cancellationToken) + { + _subscription?.Dispose(); + _subscription = null; + return Task.CompletedTask; + } + + private void OnChanged(ScriptArtifactsChanged notification) + { + if (!string.Equals( + notification.ArtifactKind, ScriptArtifactKinds.ApiMethod, StringComparison.Ordinal)) + { + return; // only ApiMethod handlers are cached by this executor + } + + foreach (var name in notification.Names) + { + _executor.InvalidateMethod(name); + } + + _logger.LogInformation( + "Invalidated {Count} inbound API method handler(s) after a {Source} script-artifact change.", + notification.Names.Count, notification.Source); + } +} diff --git a/src/ZB.MOM.WW.ScadaBridge.InboundAPI/ServiceCollectionExtensions.cs b/src/ZB.MOM.WW.ScadaBridge.InboundAPI/ServiceCollectionExtensions.cs index 12df28ac..bf694c50 100644 --- a/src/ZB.MOM.WW.ScadaBridge.InboundAPI/ServiceCollectionExtensions.cs +++ b/src/ZB.MOM.WW.ScadaBridge.InboundAPI/ServiceCollectionExtensions.cs @@ -27,6 +27,12 @@ public static class ServiceCollectionExtensions // body size cap and active-node gating for POST /api/{methodName}. services.AddSingleton(); + // N1: the ApiMethod consumer of the script-artifact invalidation bus. + // The bus parameter is optional — hosts without a registered + // IScriptArtifactChangeBus (site roles, plain test hosts) start it as a no-op + // and the executor's per-request revision check alone keeps handlers fresh. + services.AddHostedService(); + return services; } } diff --git a/tests/ZB.MOM.WW.ScadaBridge.InboundAPI.Tests/ScriptArtifactChangeSubscriberTests.cs b/tests/ZB.MOM.WW.ScadaBridge.InboundAPI.Tests/ScriptArtifactChangeSubscriberTests.cs new file mode 100644 index 00000000..37b3076f --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.InboundAPI.Tests/ScriptArtifactChangeSubscriberTests.cs @@ -0,0 +1,133 @@ +using Microsoft.Extensions.Logging.Abstractions; +using NSubstitute; +using ZB.MOM.WW.ScadaBridge.Commons.Entities.InboundApi; +using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Scripting; +using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services; +using ZB.MOM.WW.ScadaBridge.Commons.Messages.Scripting; + +namespace ZB.MOM.WW.ScadaBridge.InboundAPI.Tests; + +/// +/// Arch-review R2 N1: the Inbound API must be a REAL subscriber of +/// IScriptArtifactChangeBus — a bundle-import publish must invalidate a cached +/// handler immediately, without waiting for the per-request revision-check +/// self-heal (which remains the correctness fallback per the invalidation +/// contract's normative semantics). +/// +public class ScriptArtifactChangeSubscriberTests +{ + /// Minimal in-test bus: records subscriptions, delivers synchronously. + private sealed class RecordingBus : IScriptArtifactChangeBus + { + private readonly List> _handlers = new(); + public int SubscriberCount => _handlers.Count; + + public void Publish(ScriptArtifactsChanged notification) + { + foreach (var handler in _handlers.ToArray()) handler(notification); + } + + public IDisposable Subscribe(Action handler) + { + _handlers.Add(handler); + return new Token(() => _handlers.Remove(handler)); + } + + private sealed class Token(Action dispose) : IDisposable + { + public void Dispose() => dispose(); + } + } + + private readonly InboundScriptExecutor _executor = new( + NullLogger.Instance, Substitute.For()); + private readonly RecordingBus _bus = new(); + private readonly RouteHelper _route = new( + Substitute.For(), Substitute.For()); + + private ScriptArtifactChangeSubscriber CreateSubscriber(IScriptArtifactChangeBus? bus) => + new(_executor, NullLogger.Instance, bus); + + private Task Run(ApiMethod m) => _executor.ExecuteAsync( + m, new Dictionary(), _route, TimeSpan.FromSeconds(10)); + + private static ScriptArtifactsChanged ApiMethodChanged(params string[] names) => + new(ScriptArtifactKinds.ApiMethod, names, "BundleImport", DateTimeOffset.UtcNow); + + [Fact] + public async Task ApiMethodPublish_InvalidatesCachedHandler_WithoutRevisionCheckSelfHeal() + { + var subscriber = CreateSubscriber(_bus); + await subscriber.StartAsync(CancellationToken.None); + + // Pin a SCRIPT-LESS handler: the RegisterHandler seam is exempt from the + // per-request revision check, so the ONLY mechanism that can stop it + // serving is an explicit InvalidateMethod — exactly what the bus must trigger. + _executor.RegisterHandler("m", async _ => { await Task.CompletedTask; return 99; }); + var row = new ApiMethod("m", "return 1;") { Id = 1, TimeoutSeconds = 10 }; + + var before = await Run(row); + Assert.Contains("99", before.ResultJson); // pinned handler serves; no self-heal fires + + _bus.Publish(ApiMethodChanged("m")); + + var after = await Run(row); + Assert.True(after.Success); + Assert.Contains("1", after.ResultJson); // recompiled from the fresh row — invalidation, not self-heal + } + + [Fact] + public async Task ApiMethodPublish_PurgesKnownBadRecord() + { + var subscriber = CreateSubscriber(_bus); + await subscriber.StartAsync(CancellationToken.None); + + var broken = new ApiMethod("bad", "%%% not C# %%%") { Id = 2, TimeoutSeconds = 10 }; + Assert.False(_executor.CompileAndRegister(broken)); + Assert.Equal(1, _executor.KnownBadMethodCount); + + _bus.Publish(ApiMethodChanged("bad")); + + Assert.Equal(0, _executor.KnownBadMethodCount); + } + + [Fact] + public async Task NonApiMethodKinds_AreIgnored() + { + var subscriber = CreateSubscriber(_bus); + await subscriber.StartAsync(CancellationToken.None); + + _executor.RegisterHandler("m", async _ => { await Task.CompletedTask; return 99; }); + var row = new ApiMethod("m", "return 1;") { Id = 3, TimeoutSeconds = 10 }; + + _bus.Publish(new ScriptArtifactsChanged( + ScriptArtifactKinds.SharedScript, new[] { "m" }, "BundleImport", DateTimeOffset.UtcNow)); + _bus.Publish(new ScriptArtifactsChanged( + ScriptArtifactKinds.Template, new[] { "m" }, "BundleImport", DateTimeOffset.UtcNow)); + + var result = await Run(row); + Assert.Contains("99", result.ResultJson); // pinned handler untouched + } + + [Fact] + public async Task StopAsync_Unsubscribes() + { + var subscriber = CreateSubscriber(_bus); + await subscriber.StartAsync(CancellationToken.None); + Assert.Equal(1, _bus.SubscriberCount); + + await subscriber.StopAsync(CancellationToken.None); + + Assert.Equal(0, _bus.SubscriberCount); + } + + [Fact] + public async Task NullBus_NoOps() + { + // Site roles / plain test hosts have no bus registered — the hosted + // service must start and stop cleanly (the revision check alone applies). + var subscriber = CreateSubscriber(bus: null); + await subscriber.StartAsync(CancellationToken.None); + await subscriber.StopAsync(CancellationToken.None); + } +}