test(host): row 62 — pin ISiteEventLogger's Singleton lifetime in the site graph
Six SiteRuntime call sites resolve ISiteEventLogger 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. All correct today, and
correct for exactly one reason — the registration is AddSingleton, so a root
resolve yields what a scope would.
That makes the lifetime a load-bearing invariant with no enforcement anywhere.
If it ever changes, all six silently become register row 45, and nothing in the
tree notices: ValidateOnBuild walks CONSTRUCTOR graphs and these are imperative
GetService calls in method bodies; ValidateScopes would fire, but at runtime on
a site node on whichever path logs an event first, not at boot and not in CI;
CaptiveDependencyGuard catches the AlarmActor field case by design but cannot
see the five per-call resolves at all. This pin is the tripwire, and its failure
message names the six call sites that must be fixed alongside any such change.
Asserts the REGISTRATION, not the call sites — the SiteRuntime actor files are
correct as written and are untouched here.
Two facts, not one, because the interface registration is a FACTORY
(AddSingleton<ISiteEventLogger>(sp => sp.GetRequiredService<SiteEventLogger>())):
a Singleton lifetime on the interface alone does not prove callers share an
instance, since downgrading the concrete SiteEventLogger would make that
singleton factory resolve a scoped service from the root.
Fail-before, both halves, by temporarily flipping the real registration:
- AddScoped<ISiteEventLogger> -> first pin FAILS, "Actual lifetime: Scoped",
with the full row-62 pointer in the message.
- AddScoped<SiteEventLogger> (interface left Singleton) -> the first pin stays
GREEN and only the second catches it. That is the evidence the second fact
is load-bearing rather than redundant.
Registration restored; both green.
Registration-only inspection — the provider is never built, so the pin costs
~30 ms and opens no SQLite file.
Host.Tests: 507/507 green.
This commit is contained in:
Joseph Doherty
@@ -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;
|
||||
|
||||
/// <summary>
|
||||
/// Deferred-work register row 62 — hardening pin, not a bug fix.
|
||||
///
|
||||
/// <para>
|
||||
/// Six SiteRuntime call sites reach <c>ISiteEventLogger</c> straight off the injected
|
||||
/// <see cref="IServiceProvider"/>, with no scope in between:
|
||||
/// </para>
|
||||
/// <list type="bullet">
|
||||
/// <item><description><c>SiteRuntime/Actors/AlarmActor.cs</c> — resolves once in the
|
||||
/// constructor and CACHES it in <c>_siteEventLogger</c>, so it holds the instance for
|
||||
/// the actor's whole lifetime.</description></item>
|
||||
/// <item><description><c>SiteRuntime/Actors/ScriptActor.cs</c> (two sites) — per-call
|
||||
/// resolve.</description></item>
|
||||
/// <item><description><c>SiteRuntime/Actors/InstanceActor.cs</c> (two sites) — per-call
|
||||
/// resolve.</description></item>
|
||||
/// <item><description><c>SiteRuntime/Actors/NativeAlarmActor.cs</c> — per-call
|
||||
/// resolve.</description></item>
|
||||
/// <item><description><c>SiteRuntime/Actors/DeploymentManagerActor.cs</c> — per-call
|
||||
/// resolve.</description></item>
|
||||
/// <item><description><c>SiteRuntime/Scripts/ScriptRunLauncher.cs</c> — 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.</description></item>
|
||||
/// </list>
|
||||
///
|
||||
/// <para>
|
||||
/// Every one of those is CORRECT TODAY, and this test is the reason it stays that way.
|
||||
/// They are correct for exactly one reason: <c>ISiteEventLogger</c> is registered
|
||||
/// <see cref="ServiceLifetime.Singleton"/>, 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:
|
||||
/// </para>
|
||||
/// <list type="bullet">
|
||||
/// <item><description><c>ValidateScopes</c> 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 <c>GetService</c> 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.</description></item>
|
||||
/// <item><description><c>ValidateOnBuild</c> walks CONSTRUCTOR graphs. These are
|
||||
/// imperative <c>GetService</c> calls inside method bodies, invisible to it.</description></item>
|
||||
/// <item><description><c>CaptiveDependencyGuard</c> catches the AlarmActor field case
|
||||
/// (a cached resolve) by design, but cannot see the five per-call resolves at
|
||||
/// all.</description></item>
|
||||
/// </list>
|
||||
///
|
||||
/// <para>
|
||||
/// 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.
|
||||
/// </para>
|
||||
///
|
||||
/// <para>
|
||||
/// 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.
|
||||
/// </para>
|
||||
/// </summary>
|
||||
public class SiteEventLoggerLifetimePinTests
|
||||
{
|
||||
/// <summary>
|
||||
/// 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".
|
||||
/// </summary>
|
||||
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.";
|
||||
|
||||
/// <summary>
|
||||
/// Builds the real Site service graph the way <c>Program.cs</c>'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.
|
||||
/// </summary>
|
||||
/// <returns>The configured site service collection.</returns>
|
||||
private static IServiceCollection BuildSiteServiceGraph()
|
||||
{
|
||||
var configuration = new ConfigurationBuilder()
|
||||
.AddInMemoryCollection(new Dictionary<string, string?>
|
||||
{
|
||||
["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<ISiteEventLogger>(sp => sp.GetRequiredService<SiteEventLogger>())
|
||||
// — 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}.");
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user