diff --git a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/SiteEventLoggerLifetimePinTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/SiteEventLoggerLifetimePinTests.cs new file mode 100644 index 00000000..1f17c8a9 --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/SiteEventLoggerLifetimePinTests.cs @@ -0,0 +1,146 @@ +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using ZB.MOM.WW.ScadaBridge.SiteEventLogging; + +namespace ZB.MOM.WW.ScadaBridge.Host.Tests; + +/// +/// Deferred-work register row 62 — hardening pin, not a bug fix. +/// +/// +/// Six SiteRuntime call sites reach ISiteEventLogger straight off the injected +/// , with no scope in between: +/// +/// +/// SiteRuntime/Actors/AlarmActor.cs — resolves once in the +/// constructor and CACHES it in _siteEventLogger, so it holds the instance for +/// the actor's whole lifetime. +/// SiteRuntime/Actors/ScriptActor.cs (two sites) — per-call +/// resolve. +/// SiteRuntime/Actors/InstanceActor.cs (two sites) — per-call +/// resolve. +/// SiteRuntime/Actors/NativeAlarmActor.cs — per-call +/// resolve. +/// SiteRuntime/Actors/DeploymentManagerActor.cs — per-call +/// resolve. +/// SiteRuntime/Scripts/ScriptRunLauncher.cs — per-run resolve. +/// Not among the five the register row named: it is the run body WP3.1 lifted verbatim +/// out of the deleted ScriptExecutionActor/AlarmExecutionActor, so it inherited the +/// pattern rather than adding it. +/// +/// +/// +/// Every one of those is CORRECT TODAY, and this test is the reason it stays that way. +/// They are correct for exactly one reason: ISiteEventLogger is registered +/// , so resolving it from the root provider yields +/// the same instance a scope would. The day that registration changes lifetime, every one +/// of those call sites silently becomes register row 45 — a captive dependency / +/// resolved-outside-its-scope bug — and nothing else in the tree would notice: +/// +/// +/// ValidateScopes catches a SCOPED service resolved from the +/// root provider — but the actors hold the root provider legitimately, and .NET's +/// check fires at the offending GetService call, i.e. at runtime on a site +/// node, on whatever code path happens to log an event first. Not at boot, and not +/// in CI. +/// ValidateOnBuild walks CONSTRUCTOR graphs. These are +/// imperative GetService calls inside method bodies, invisible to it. +/// CaptiveDependencyGuard catches the AlarmActor field case +/// (a cached resolve) by design, but cannot see the five per-call resolves at +/// all. +/// +/// +/// +/// So the lifetime is a load-bearing invariant with no other enforcement. Pinning it here +/// means a future lifetime change fails loudly, in CI, with a message naming the call +/// sites that have to be fixed alongside it — rather than shipping and being found on the +/// rig months later, which is how row 45 was found. +/// +/// +/// +/// This deliberately asserts the REGISTRATION, not the call sites: the SiteRuntime actor +/// files are correct as written and need no change. The pin is a tripwire on the +/// assumption they rest on. +/// +/// +public class SiteEventLoggerLifetimePinTests +{ + /// + /// The failure message is the whole point of the test — it has to carry the reader + /// from "a lifetime assertion failed" to "here is what breaks and why". + /// + private const string Why = + "ISiteEventLogger MUST be registered Singleton in the site service graph " + + "(deferred-work register row 62).\n" + + "Six SiteRuntime call sites resolve it straight off the root IServiceProvider with " + + "no scope: AlarmActor (cached in a field for the actor's lifetime), ScriptActor (x2), " + + "InstanceActor (x2), NativeAlarmActor, DeploymentManagerActor, and " + + "Scripts/ScriptRunLauncher.\n" + + "Under any non-singleton lifetime every one of those becomes a captive-dependency / " + + "resolved-outside-its-scope bug (register row 45), and NOTHING else catches it: " + + "ValidateOnBuild only walks constructor graphs, ValidateScopes fires at runtime on a " + + "site node rather than at boot, and CaptiveDependencyGuard sees only the AlarmActor " + + "field case.\n" + + "If this lifetime change is intended, fix those six call sites to resolve through an " + + "explicit scope in the same change, then update this pin."; + + /// + /// Builds the real Site service graph the way Program.cs's Site branch does and + /// asserts the lifetime of the registration the SiteRuntime root-resolves depend on. + /// Registration only — the provider is never built, so this costs nothing and touches + /// no SQLite file. + /// + /// The configured site service collection. + private static IServiceCollection BuildSiteServiceGraph() + { + var configuration = new ConfigurationBuilder() + .AddInMemoryCollection(new Dictionary + { + ["ScadaBridge:Node:Role"] = "Site", + ["ScadaBridge:Node:NodeName"] = "node-a", + ["ScadaBridge:Node:NodeHostname"] = "test-site", + ["ScadaBridge:Node:SiteId"] = "TestSite", + ["ScadaBridge:Node:RemotingPort"] = "0", + ["ScadaBridge:Node:GrpcPort"] = "0", + ["LocalDb:Path"] = "/tmp/sb-row62-pin-never-opened.db", + ["AuditLog:SiteWriter:DatabasePath"] = "/tmp/sb-row62-pin-never-opened-audit.db", + ["ScadaBridge:Cluster:SeedNodes:0"] = "akka.tcp://scadabridge@localhost:2551", + ["ScadaBridge:Cluster:SeedNodes:1"] = "akka.tcp://scadabridge@localhost:2552", + }) + .Build(); + + var services = new ServiceCollection(); + SiteServiceRegistration.Configure(services, configuration); + return services; + } + + [Fact] + public void ISiteEventLogger_IsRegisteredSingleton_InTheSiteServiceGraph() + { + var services = BuildSiteServiceGraph(); + + var descriptor = Assert.Single(services, d => d.ServiceType == typeof(ISiteEventLogger)); + + Assert.True(descriptor.Lifetime == ServiceLifetime.Singleton, + $"{Why}\nActual lifetime: {descriptor.Lifetime}."); + } + + [Fact] + public void SiteEventLogger_TheConcreteTypeTheInterfaceForwardsTo_IsAlsoSingleton() + { + // The interface registration is a FACTORY — + // AddSingleton(sp => sp.GetRequiredService()) + // — so a Singleton lifetime on ISiteEventLogger alone does not prove callers get a + // shared instance. Downgrading the concrete SiteEventLogger to Scoped would leave + // the assertion above passing while making the singleton factory resolve a scoped + // service from the root: the exact bug, with the pin still green. Both halves have + // to be pinned for the invariant to actually hold. + var services = BuildSiteServiceGraph(); + + var descriptor = Assert.Single(services, d => d.ServiceType == typeof(SiteEventLogger)); + + Assert.True(descriptor.Lifetime == ServiceLifetime.Singleton, + $"{Why}\nISiteEventLogger forwards to the concrete SiteEventLogger, whose actual " + + $"lifetime is: {descriptor.Lifetime}."); + } +}