diff --git a/docs/requirements/Component-Host.md b/docs/requirements/Component-Host.md index 911ceef7..25fee106 100644 --- a/docs/requirements/Component-Host.md +++ b/docs/requirements/Component-Host.md @@ -181,6 +181,18 @@ Each component library must expose its services to the Host via a consistent set The Host's `Program.cs` calls these extension methods; the component libraries own the registration logic. This keeps the Host thin and each component self-contained. (The ManagementService component no longer registers the ManagementActor with ClusterClientReceptionist — that registration was removed 2026-07-22, and the whole receptionist surface went in Phase 4.) +### REQ-HOST-11: Dependency-Injection Lifetime Discipline + +Akka actors are constructed by the actor system, not by the DI container, so the container cannot enforce lifetimes on them. Three rules therefore apply, and each has a mechanical guard. + +**1. An actor may hold the root `IServiceProvider` (or an `IServiceScopeFactory`) for its lifetime — never a scope or anything resolved from one.** Every operation that needs a scoped service opens its own scope (`CreateScope` / `CreateAsyncScope`, disposed in the same handler) and resolves inside it. Caching the resolved instance in a field is a **captive dependency**: the actor keeps one EF `DbContext` alive for its whole lifetime and any concurrency the actor allows then collides on it. This is not hypothetical — it shipped as register row 45, where `NotificationOutboxActor` cached delivery adapters from an actor-lifetime scope and `MaxParallelDeliveries` concurrent deliveries drove EF's "a second operation was started on this context instance" continuously on the rig. + +*Guard:* `CaptiveDependencyGuard` (Host test suite) reflects over every actor class in the shipped assemblies and fails on any instance field — auto-property backing fields included — typed as `IServiceScope`, as a `DbContext`, or as a service the real composition roots register `Scoped`. The scoped set is derived from the actual registrations of the built Central and Site containers rather than hand-listed, so changing a registration's lifetime automatically re-scopes the check. `IServiceProvider` / `IServiceScopeFactory` fields are explicitly permitted. Exemptions live in a small allowlist inside the guard, each carrying its safety argument, and a self-test fails any allowlist entry that has stopped matching a real field. + +**2. Scope validation is on in Development, for both roles.** `Program.cs` applies `ValidateScopes` + `ValidateOnBuild` via `ServiceProviderValidation` in both the Central and Site branches. This is the container-visible half of the same concern — it catches a singleton whose *constructor* takes a scoped service, which is the ordinary form of the bug in non-actor code. It cannot see rule 1's hand-rolled `GetRequiredService` calls, and rule 1's guard cannot see constructor injection; neither subsumes the other. Production runs unvalidated deliberately: a validation failure is fail-closed, and a defect must not take a production node down at boot when Development and CI give the same signal. + +**3. A registration must be constructible on every role that registers it.** A service whose dependencies exist only on the other role belongs in a role-scoped extension method (`AddCentralCommunication`, `AddAuditLogCentralKpi`), not in a shared `AddXxx()` on the argument that "only the central actor ever resolves it". An unconstructible descriptor is not inert: it fails `ValidateOnBuild` for the *entire* graph, which is what previously kept rule 2 switched off on the Site branch and would have masked any genuine defect the validator found there. + --- ## Component Registration Matrix diff --git a/src/ZB.MOM.WW.ScadaBridge.AuditLog/ServiceCollectionExtensions.cs b/src/ZB.MOM.WW.ScadaBridge.AuditLog/ServiceCollectionExtensions.cs index e60f4331..3446bc1f 100644 --- a/src/ZB.MOM.WW.ScadaBridge.AuditLog/ServiceCollectionExtensions.cs +++ b/src/ZB.MOM.WW.ScadaBridge.AuditLog/ServiceCollectionExtensions.cs @@ -248,15 +248,38 @@ public static class ServiceCollectionExtensions // provider as a singleton on both site and central paths. sp.GetRequiredService())); - // KPI History & Trends: the AuditLog source the central KPI - // recorder enumerates each sampling pass to snapshot Global-scope Audit - // Log KPIs (volume / error rate / backlog) into the KpiSample history - // table. Scoped to match its IAuditLogRepository dependency — a SCOPED - // EF Core service; the recorder opens a fresh scope per sampling pass. - // TryAdd-Enumerable so multiple sources can register the same interface - // (NotificationOutbox + SiteCallAudit + SiteHealth add their own) without - // any one composition root clobbering another, and re-registration is a - // no-op if AddAuditLog were ever called twice. + return services; + } + + /// + /// Registers the Audit Log's KPI sample source. CENTRAL-ONLY — call from the Central + /// composition root only, alongside AddKpiHistory. + /// + /// + /// + /// KPI History & Trends: the AuditLog source the central KPI recorder enumerates each + /// sampling pass to snapshot Global-scope Audit Log KPIs (volume / error rate / backlog) + /// into the KpiSample history table. Scoped to match its IAuditLogRepository + /// dependency — a SCOPED EF Core service; the recorder opens a fresh scope per sampling + /// pass. TryAdd-Enumerable so multiple sources can register the same interface + /// (NotificationOutbox + SiteCallAudit + SiteHealth add their own) without any one + /// composition root clobbering another, and re-registration is a no-op if this were ever + /// called twice. + /// + /// + /// Split out of by deferred register row 50. Both the KPI + /// recorder and IAuditLogRepository are central-only, so on a site node this + /// registration was unconstructible — enough on its own to fail ValidateOnBuild for + /// the entire site graph and so to keep scope validation switched off there. The site half + /// of (the SQLite writer chain) is unaffected and stays shared. + /// + /// + /// The DI service collection to register services into. + /// The same to allow chaining. + public static IServiceCollection AddAuditLogCentralKpi(this IServiceCollection services) + { + ArgumentNullException.ThrowIfNull(services); + services.TryAddEnumerable( ServiceDescriptor.Scoped()); diff --git a/src/ZB.MOM.WW.ScadaBridge.Communication/ServiceCollectionExtensions.cs b/src/ZB.MOM.WW.ScadaBridge.Communication/ServiceCollectionExtensions.cs index 0dc5ce22..d3297ba2 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Communication/ServiceCollectionExtensions.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Communication/ServiceCollectionExtensions.cs @@ -42,9 +42,33 @@ public static class ServiceCollectionExtensions services.AddSingleton(); services.AddSingleton(sp => sp.GetRequiredService()); + return services; + } + + /// + /// Registers the communication services that only a CENTRAL node can construct. + /// Call alongside from the Central composition root only. + /// + /// + /// + /// is scoped and takes IDeploymentManagerRepository — + /// an EF repository registered by AddConfigurationDatabase, which only central calls. + /// It used to live in the shared on the argument that it was + /// "harmless on site hosts: only the central actor ever resolves it". That is true of the + /// runtime behaviour but not of the container: the registration was UNCONSTRUCTIBLE on a + /// site node, so ValidateOnBuild failed the whole site graph and the site branch + /// could not have scope validation turned on at all (deferred register row 50). A + /// registration nothing can ever build is not harmless — it is the thing that stops the + /// validator from being usable, and it hides whatever real defect the validator would + /// otherwise have found. + /// + /// + /// The DI service collection to register services into. + /// The same to allow chaining. + public static IServiceCollection AddCentralCommunication(this IServiceCollection services) + { // Startup reconciliation handler — scoped (holds scoped repositories), resolved - // per-request by CentralCommunicationActor inside a DI scope. Harmless on site - // hosts: only the central actor ever resolves it. + // per-request by CentralCommunicationActor inside a DI scope. services.AddScoped(); return services; diff --git a/src/ZB.MOM.WW.ScadaBridge.Host/Program.cs b/src/ZB.MOM.WW.ScadaBridge.Host/Program.cs index 38d5fa01..30c3091f 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Host/Program.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Host/Program.cs @@ -166,6 +166,10 @@ try var builder = WebApplication.CreateBuilder(args); builder.Configuration.AddConfiguration(configuration); + // Captive-dependency validation (deferred register row 50). See the identical + // block in the Site branch below and ServiceProviderValidation for the full rationale. + builder.Host.UseDefaultServiceProvider(ServiceProviderValidation.Configure); + // Serilog builder.Host.UseSerilog(); @@ -218,6 +222,11 @@ try // Shared components builder.Services.AddClusterInfrastructure(); builder.Services.AddCommunication(); + // Central-only communication services (ReconcileService — scoped, holds the + // central-only IDeploymentManagerRepository). Split out of AddCommunication by + // deferred register row 50: registering it on a site node produced an + // unconstructible descriptor that failed ValidateOnBuild for the whole site graph. + builder.Services.AddCentralCommunication(); // Per-site gRPC preshared keys. Central-only: it is the side that dials sites, and // the only side whose key set is dynamic (sites come from the configuration @@ -301,6 +310,11 @@ try // the KpiHistoryRecorderActor cluster singleton (started in // AkkaHostedService). Observability/best-effort: NOT readiness-gated. builder.Services.AddKpiHistory(builder.Configuration); + // The Audit Log's IKpiSampleSource, enumerated by the recorder registered just + // above. Central-only (its IAuditLogRepository is central-only); split out of + // AddAuditLog by deferred register row 50 for the same reason as + // AddCentralCommunication above. + builder.Services.AddAuditLogCentralKpi(); builder.Services.AddTemplateEngine(); builder.Services.AddDeploymentManager(); // Host is the composition root and owns config-coupled wiring: register the @@ -683,6 +697,12 @@ try var builder = WebApplication.CreateBuilder(args); builder.Configuration.AddConfiguration(configuration); + // Captive-dependency validation (deferred register row 50) — the SITE half of the + // pair. This branch is the one that actually gains coverage: the docker rig sets + // ASPNETCORE_ENVIRONMENT=Development on the central containers but NOT on the site + // containers, so a site node has always built its provider unvalidated. + builder.Host.UseDefaultServiceProvider(ServiceProviderValidation.Configure); + // Serilog builder.Host.UseSerilog(); diff --git a/src/ZB.MOM.WW.ScadaBridge.Host/ServiceProviderValidation.cs b/src/ZB.MOM.WW.ScadaBridge.Host/ServiceProviderValidation.cs new file mode 100644 index 00000000..a2a08fe6 --- /dev/null +++ b/src/ZB.MOM.WW.ScadaBridge.Host/ServiceProviderValidation.cs @@ -0,0 +1,73 @@ +using Microsoft.Extensions.DependencyInjection; + +namespace ZB.MOM.WW.ScadaBridge.Host; + +/// +/// The composition root's captive-dependency tripwire: turns .NET's built-in DI +/// scope validation on in Development, for BOTH the Central and Site branches of +/// Program.cs. +/// +/// +/// +/// Deferred register row 50. Row 45 was a live captive dependency — the Notification +/// Outbox actor cached INotificationDeliveryAdapter instances resolved from an +/// actor-lifetime IServiceScope, so up to MaxParallelDeliveries concurrent +/// deliveries shared one EF DbContext and central-b logged EF's +/// "second operation started on this context instance" continuously (fixed in 9e769735). +/// +/// +/// What this catches and what it does not. +/// fails a resolve where a SINGLETON directly injects a SCOPED service, and forbids resolving a +/// scoped service from the root provider. +/// moves the constructor-resolvability half of that check to build time, so a graph defect +/// surfaces as one aggregated boot failure instead of a first-request 500 on a page nobody +/// opened yet. Neither one can see row 45 itself: the actors take the ROOT +/// and call GetRequiredService by hand, which is opaque to +/// static graph validation — that half of row 50 is covered by the reflection guard test +/// (CaptiveDependencyGuard in the Host test suite). The two mitigations are +/// complementary, and neither subsumes the other. +/// +/// +/// Why this is written out rather than left to the framework default. +/// WebApplication.CreateBuilder already sets both flags to +/// IsDevelopment(), so on paper this is a no-op. It is stated explicitly because +/// (a) it is a silent default — nothing in the repo said the property was relied on, and a +/// future move off CreateBuilder (or onto a bare HostBuilder) would drop it with +/// no diagnostic; and (b) this host has THREE environment-ish signals — +/// SCADABRIDGE_CONFIG (role config: Central/Site), DOTNET_ENVIRONMENT (role +/// fallback), and ASPNETCORE_ENVIRONMENT (the actual ) — +/// so "is this node in Development" is genuinely non-obvious at the call site. +/// +/// +/// Production is deliberately excluded. ValidateScopes costs a check on every +/// resolve and ValidateOnBuild costs a full graph walk at boot; more to the point, a +/// validation failure is fail-closed, and a defect that only a rarely-resolved registration +/// would have exposed must not take a production node down at boot when the equivalent signal +/// is available in Development and in CI. The Host test suite forces both flags on for each +/// role regardless of environment, so CI validates what a Production node will not. +/// +/// +internal static class ServiceProviderValidation +{ + /// + /// Enables scope + build-time validation when the host environment is Development. + /// Wired via builder.Host.UseDefaultServiceProvider(ServiceProviderValidation.Configure). + /// + /// The host builder context, read for its hosting environment. + /// The service-provider options to configure. + internal static void Configure(HostBuilderContext context, ServiceProviderOptions options) + { + Apply(options, context.HostingEnvironment.IsDevelopment()); + } + + /// + /// The pure decision, split out so a test can assert both the on and off cases without a host. + /// + /// The service-provider options to configure. + /// Whether the host environment is Development. + internal static void Apply(ServiceProviderOptions options, bool isDevelopment) + { + options.ValidateScopes = isDevelopment; + options.ValidateOnBuild = isDevelopment; + } +} diff --git a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/CaptiveDependencyGuard.cs b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/CaptiveDependencyGuard.cs new file mode 100644 index 00000000..8e323481 --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/CaptiveDependencyGuard.cs @@ -0,0 +1,287 @@ +using System.Reflection; +using Akka.Actor; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.DependencyInjection; + +namespace ZB.MOM.WW.ScadaBridge.Host.Tests; + +/// +/// Deferred register row 50, mitigation 2: a reflection guard that no Akka actor holds +/// captive scope state — a scoped service, or an , cached for +/// the actor's lifetime. +/// +/// +/// +/// Why this exists separately from scope validation. Register row 45 was exactly this +/// bug: NotificationOutboxActor cached delivery adapters resolved from an +/// actor-lifetime IServiceScope, so MaxParallelDeliveries concurrent deliveries +/// shared one EF DbContext and EF threw "a second operation was started on this +/// context instance" continuously on the rig (fixed 9e769735). .NET's own +/// ValidateScopes could never have caught it: the actors take the ROOT +/// and call GetRequiredService by hand, which the +/// container's static graph analysis cannot see through. This guard closes that half. +/// +/// +/// What is allowed. Holding or +/// for the actor's lifetime is the CORRECT pattern and is +/// deliberately not flagged — every actor here does that and opens a fresh scope per +/// operation. What is forbidden is holding the SCOPE, or anything resolved from one, past +/// the operation that opened it. +/// +/// +/// How "scoped" is decided. The caller supplies the scoped set derived from the actual +/// DI registrations of a real composition root, so the guard tracks the container rather than +/// a hand-maintained list — re-registering a service from singleton to scoped automatically +/// brings any actor field of that type into scope for the check. Two types are flagged +/// unconditionally on top of that set, because they are wrong in an actor field whatever the +/// container says: itself, and any . +/// +/// +/// Auto-property backing fields need no special handling: the compiler emits them as private +/// instance fields, so GetFields(NonPublic | Instance) already returns them. +/// +/// +internal static class CaptiveDependencyGuard +{ + /// One offending field on one actor type. + /// The actor class declaring the field. + /// The field's name (a compiler-generated backing field keeps its mangled name). + /// The field's declared type. + /// Why the field was flagged. + internal sealed record Violation(Type ActorType, string FieldName, Type FieldType, string Reason) + { + /// + public override string ToString() => + $"{ActorType.FullName}.{FieldName} : {FieldType.FullName} — {Reason}"; + } + + /// + /// Field types that are always correct to hold for an actor's lifetime: they ARE the + /// per-operation-scope pattern rather than a violation of it. + /// + internal static readonly IReadOnlyList AllowedProviderTypes = + [ + typeof(IServiceProvider), + typeof(IServiceScopeFactory), + ]; + + /// + /// Fields that hold a scoped-typed value but are provably safe, keyed + /// "Namespace.ActorType.fieldName". Every entry carries its safety argument — + /// nothing is exempted silently. + /// + /// + /// + /// Both entries below are the same shape: a nullable repository field assigned ONLY by an + /// explicitly test-mode constructor, so the production wiring + /// (AkkaHostedService constructs both actors through their + /// overload) leaves the field null and every handler takes the create-a-scope branch. The + /// test fixtures that do use those constructors are MSSQL-backed TestKit fixtures that + /// drive the actor one message at a time, and SiteCallAuditActor additionally wraps + /// the injected instance in its own SerializedRepository — i.e. the test path + /// already carries a serialization guard for precisely the concurrency hazard row 45 hit. + /// + /// + /// They are allowlisted rather than removed because deleting a test-mode constructor is a + /// larger change than this row justifies, and rather than left unflagged because the shape + /// is genuinely one constructor call away from being row 45 again: if a production call + /// site ever switched to the repository overload, the actor would hold a scoped EF-backed + /// repository for its whole lifetime. That residual risk is recorded, not dismissed. + /// + /// + internal static readonly IReadOnlySet Allowlist = new HashSet(StringComparer.Ordinal) + { + "ZB.MOM.WW.ScadaBridge.AuditLog.Central.AuditLogIngestActor._injectedRepository", + "ZB.MOM.WW.ScadaBridge.SiteCallAudit.SiteCallAuditActor._injectedRepository", + }; + + /// + /// Scans the given actor types for instance fields holding captive scope state. + /// + /// Actor classes to inspect. + /// + /// Service and implementation types registered with + /// in the composition root being checked. + /// + /// + /// False makes the scan report allowlisted fields too — used by the guard's own self-tests + /// so the allowlist cannot quietly grow to cover a real regression. + /// + /// Every violation found, in declaration order. + internal static IReadOnlyList Scan( + IEnumerable actorTypes, + IReadOnlySet scopedServiceTypes, + bool applyAllowlist = true) + { + ArgumentNullException.ThrowIfNull(actorTypes); + ArgumentNullException.ThrowIfNull(scopedServiceTypes); + + var violations = new List(); + + foreach (var actorType in actorTypes) + { + foreach (var field in InstanceFields(actorType)) + { + var fieldType = field.FieldType; + + if (AllowedProviderTypes.Contains(fieldType)) + { + // The correct pattern — create a scope per operation. + continue; + } + + string? reason = null; + if (fieldType == typeof(IServiceScope) || fieldType == typeof(AsyncServiceScope)) + { + reason = "holds a DI scope for the actor's lifetime; open a scope per operation instead"; + } + else if (typeof(DbContext).IsAssignableFrom(fieldType)) + { + reason = "holds an EF DbContext for the actor's lifetime; resolve it inside a per-operation scope"; + } + else if (scopedServiceTypes.Contains(fieldType)) + { + reason = "holds a service registered Scoped; resolve it inside a per-operation scope"; + } + + if (reason is null) + { + continue; + } + + if (applyAllowlist && Allowlist.Contains($"{actorType.FullName}.{field.Name}")) + { + continue; + } + + violations.Add(new Violation(actorType, field.Name, fieldType, reason)); + } + } + + return violations; + } + + /// + /// Every instance field declared by the actor class and by its ScadaBridge-owned base + /// classes. The walk stops at the first non-ScadaBridge base, which is how Akka's own + /// internals are excluded without naming them. + /// + /// The actor class to walk. + /// The instance fields, most-derived first. + private static IEnumerable InstanceFields(Type actorType) + { + for (var t = actorType; t is not null && IsScadaBridgeType(t); t = t.BaseType) + { + foreach (var field in t.GetFields( + BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.DeclaredOnly)) + { + yield return field; + } + } + } + + /// Whether a type belongs to a first-party ScadaBridge assembly. + /// The type to test. + /// True when the declaring assembly is a ScadaBridge assembly. + private static bool IsScadaBridgeType(Type type) => + type.Assembly.GetName().Name?.StartsWith(AssemblyPrefix, StringComparison.Ordinal) == true; + + private const string AssemblyPrefix = "ZB.MOM.WW.ScadaBridge"; + + /// + /// Every concrete Akka actor class in the shipped ScadaBridge assemblies. Test assemblies + /// are excluded deliberately — the guard's own deliberately-violating fixtures live there, + /// and a test-only actor is not part of any production composition root. + /// + /// The production actor types, ordered by full name. + internal static IReadOnlyList ProductionActorTypes() => + ProductionAssemblies() + .SelectMany(a => a.GetTypes()) + .Where(t => typeof(ActorBase).IsAssignableFrom(t) && t is { IsAbstract: false, IsInterface: false }) + .OrderBy(t => t.FullName, StringComparer.Ordinal) + .ToList(); + + /// + /// Transitively loads and returns the shipped ScadaBridge assemblies. + /// + /// + /// Referenced assemblies load lazily, so enumerating AppDomain.CurrentDomain alone + /// would silently scan whichever subset the test run happened to touch — a guard that + /// quietly checks nothing. The reference graph is walked from the Host assembly instead, + /// and the caller asserts a floor on the actor count so a regression here fails loudly. + /// + /// The distinct production ScadaBridge assemblies. + private static IReadOnlyList ProductionAssemblies() + { + var seen = new Dictionary(StringComparer.Ordinal); + var pending = new Stack(); + pending.Push(typeof(Program).Assembly); + + while (pending.Count > 0) + { + var assembly = pending.Pop(); + var name = assembly.GetName().Name; + if (name is null + || !name.StartsWith(AssemblyPrefix, StringComparison.Ordinal) + || name.EndsWith(".Tests", StringComparison.Ordinal) + || !seen.TryAdd(name, assembly)) + { + continue; + } + + foreach (var reference in assembly.GetReferencedAssemblies()) + { + if (reference.Name?.StartsWith(AssemblyPrefix, StringComparison.Ordinal) != true + || seen.ContainsKey(reference.Name)) + { + continue; + } + + try + { + pending.Push(Assembly.Load(reference)); + } + catch (Exception ex) when (ex is FileNotFoundException or BadImageFormatException) + { + // A ScadaBridge assembly that is referenced but not deployed alongside the + // test binaries cannot contribute actors to this run either. + } + } + } + + return seen.Values.ToList(); + } + + /// + /// Collects the service and implementation types registered with + /// in a composition root. + /// + /// + /// Both sides of a descriptor are collected: an actor can just as easily cache the + /// concrete EmailNotificationDeliveryAdapter as the INotificationDeliveryAdapter + /// it is registered as, and row 45 in fact cached the interface. + /// + /// The composition root's service collection. + /// The scoped service and implementation types. + internal static IReadOnlySet ScopedTypes(IEnumerable services) + { + ArgumentNullException.ThrowIfNull(services); + + var scoped = new HashSet(); + foreach (var descriptor in services.Where(d => d.Lifetime == ServiceLifetime.Scoped)) + { + scoped.Add(descriptor.ServiceType); + + // ImplementationType throws on a keyed descriptor and vice versa, so branch. + var implementationType = descriptor.IsKeyedService + ? descriptor.KeyedImplementationType + : descriptor.ImplementationType; + if (implementationType is not null) + { + scoped.Add(implementationType); + } + } + + return scoped; + } +} diff --git a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/CaptiveDependencyGuardTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/CaptiveDependencyGuardTests.cs new file mode 100644 index 00000000..1c8419dc --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/CaptiveDependencyGuardTests.cs @@ -0,0 +1,383 @@ +using Akka.Actor; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Mvc.Testing; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using ZB.MOM.WW.ScadaBridge.ConfigurationDatabase; + +namespace ZB.MOM.WW.ScadaBridge.Host.Tests; + +/// +/// The guard's own self-tests. These are what make the production scans below trustworthy: +/// a reflection guard that silently matches nothing passes just as green as one that works, +/// so the positive controls are permanent fixtures, not scaffolding. +/// +public class CaptiveDependencyGuardSelfTests +{ + /// A stand-in for any service the container registers Scoped. + private interface IPretendScopedService; + + /// + /// POSITIVE CONTROL — the pre-row-45 shape, preserved permanently. An actor that caches a + /// scoped-typed service for its lifetime is exactly what NotificationOutboxActor + /// looked like before 9e769735, and the guard must report it. + /// + private sealed class CachesScopedServiceActor : ReceiveActor + { +#pragma warning disable CS0169, IDE0051, CA1823 // the unused field IS the fixture + private readonly IPretendScopedService? _captive; +#pragma warning restore CS0169, IDE0051, CA1823 + } + + /// POSITIVE CONTROL — caching the scope itself, the other half of row 45. + private sealed class CachesScopeActor : ReceiveActor + { +#pragma warning disable CS0169, IDE0051, CA1823 + private readonly IServiceScope? _scope; +#pragma warning restore CS0169, IDE0051, CA1823 + } + + /// POSITIVE CONTROL — an EF context held for the actor's lifetime. + private sealed class CachesDbContextActor : ReceiveActor + { +#pragma warning disable CS0169, IDE0051, CA1823 + private readonly ScadaBridgeDbContext? _db; +#pragma warning restore CS0169, IDE0051, CA1823 + } + + /// + /// NEGATIVE CONTROL — the correct pattern. Holding the root provider and/or the scope + /// factory for the actor's lifetime must NOT be flagged; every production actor does this. + /// + private sealed class CreatesScopePerOperationActor : ReceiveActor + { +#pragma warning disable CS0169, IDE0051, CA1823 + private readonly IServiceProvider? _services; + private readonly IServiceScopeFactory? _scopeFactory; +#pragma warning restore CS0169, IDE0051, CA1823 + } + + /// POSITIVE CONTROL — an auto-property backing field must be caught too. + private sealed class ScopedAutoPropertyActor : ReceiveActor + { + private IPretendScopedService? Captive { get; init; } + } + + private static readonly IReadOnlySet PretendScopedSet = + new HashSet { typeof(IPretendScopedService) }; + + [Fact] + public void Guard_Flags_ActorCachingAScopedService() + { + var violations = CaptiveDependencyGuard.Scan( + [typeof(CachesScopedServiceActor)], PretendScopedSet); + + var violation = Assert.Single(violations); + Assert.Equal("_captive", violation.FieldName); + Assert.Equal(typeof(IPretendScopedService), violation.FieldType); + } + + [Fact] + public void Guard_Flags_ActorCachingAServiceScope_EvenWhenTheScopedSetIsEmpty() + { + // An IServiceScope field is wrong whatever the container says, so this must not + // depend on the derived scoped set having caught anything. + var violations = CaptiveDependencyGuard.Scan( + [typeof(CachesScopeActor)], new HashSet()); + + var violation = Assert.Single(violations); + Assert.Equal("_scope", violation.FieldName); + } + + [Fact] + public void Guard_Flags_ActorCachingADbContext() + { + var violations = CaptiveDependencyGuard.Scan( + [typeof(CachesDbContextActor)], new HashSet()); + + var violation = Assert.Single(violations); + Assert.Equal(typeof(ScadaBridgeDbContext), violation.FieldType); + } + + [Fact] + public void Guard_Flags_ScopedAutoPropertyBackingField() + { + var violations = CaptiveDependencyGuard.Scan( + [typeof(ScopedAutoPropertyActor)], PretendScopedSet); + + var violation = Assert.Single(violations); + Assert.Contains("Captive", violation.FieldName, StringComparison.Ordinal); + } + + [Fact] + public void Guard_Allows_RootProviderAndScopeFactoryFields() + { + var violations = CaptiveDependencyGuard.Scan( + [typeof(CreatesScopePerOperationActor)], PretendScopedSet); + + Assert.Empty(violations); + } + + /// + /// The scoped set must be derived from real registrations rather than hand-listed, and it + /// must pick up BOTH sides of a descriptor — row 45 cached the interface, but caching the + /// concrete adapter would have been the same bug. + /// + [Fact] + public void ScopedTypes_CollectsServiceAndImplementationTypes() + { + var services = new ServiceCollection(); + services.AddScoped(); + services.AddSingleton(); + + var scoped = CaptiveDependencyGuard.ScopedTypes(services); + + Assert.Contains(typeof(IPretendScopedService), scoped); + Assert.Contains(typeof(PretendScopedService), scoped); + Assert.DoesNotContain(typeof(PretendSingleton), scoped); + } + + private sealed class PretendScopedService : IPretendScopedService; + + private sealed class PretendSingleton; + + /// + /// The assembly walk must actually find the production actors. Without this floor a + /// broken loader would make every scan below vacuously green — the classic way a + /// reflection guard rots into a no-op. + /// + [Fact] + public void ProductionActorTypes_FindsTheShippedActors() + { + var actorTypes = CaptiveDependencyGuard.ProductionActorTypes(); + + // 28 actor classes ship today; the floor is deliberately loose so adding or + // consolidating an actor does not fail this, while a loader that finds nothing does. + Assert.True( + actorTypes.Count >= 20, + $"Expected the ScadaBridge assembly walk to find at least 20 actors, found {actorTypes.Count}. " + + "A guard that scans no types passes vacuously."); + + // Spot-check one actor from a site assembly and one from a central assembly, so a + // walk that only reached the Host assembly's own actors cannot satisfy the floor. + Assert.Contains(actorTypes, t => t.Name == "InstanceActor"); + Assert.Contains(actorTypes, t => t.Name == "NotificationOutboxActor"); + } + + /// + /// The allowlist is an escape hatch, so it is itself pinned: every entry must name a + /// field that still exists and would still be flagged. An entry that has stopped matching + /// is either a stale exemption or — worse — a typo silently covering nothing. + /// + [Fact] + public void Allowlist_EntriesAllStillMatchARealFlaggedField() + { + var actorTypes = CaptiveDependencyGuard.ProductionActorTypes(); + + // Scan with the allowlist disabled against a scoped set wide enough to include the + // repositories the allowlisted fields hold. + var everyScopedType = CaptiveDependencyGuard.ScopedTypes(BuildCentralishScopedProbe()); + var unfiltered = CaptiveDependencyGuard.Scan(actorTypes, everyScopedType, applyAllowlist: false); + + var flagged = unfiltered + .Select(v => $"{v.ActorType.FullName}.{v.FieldName}") + .ToHashSet(StringComparer.Ordinal); + + foreach (var entry in CaptiveDependencyGuard.Allowlist) + { + Assert.True( + flagged.Contains(entry), + $"Allowlist entry '{entry}' no longer matches a flagged field. Remove it — a stale " + + "exemption is indistinguishable from an exemption that never worked."); + } + } + + /// + /// A minimal descriptor set naming the two repository types the allowlisted fields hold, + /// so does not need to boot a + /// host. Both are registered Scoped by AddConfigurationDatabase; the + /// role-scan tests below verify that against the real container. + /// + /// The probe descriptors. + private static IServiceCollection BuildCentralishScopedProbe() + { + var services = new ServiceCollection(); + services.AddScoped< + ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories.IAuditLogRepository>( + _ => throw new NotSupportedException("registration shape only")); + services.AddScoped< + ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories.ISiteCallAuditRepository>( + _ => throw new NotSupportedException("registration shape only")); + return services; + } +} + +/// +/// Runs the guard against every shipped actor using the CENTRAL composition root's real +/// scoped registrations. +/// +[Collection(HostBootCollection.Name)] +public class CentralCaptiveDependencyGuardTests : IDisposable +{ + private readonly WebApplicationFactory _factory; + private readonly string? _previousEnv; + private readonly CentralDbTestEnvironment _dbEnv; + private readonly List _descriptors = []; + + public CentralCaptiveDependencyGuardTests() + { + _previousEnv = Environment.GetEnvironmentVariable("DOTNET_ENVIRONMENT"); + Environment.SetEnvironmentVariable("DOTNET_ENVIRONMENT", "Central"); + _dbEnv = new CentralDbTestEnvironment(); + + _factory = new WebApplicationFactory() + .WithWebHostBuilder(builder => + { + builder.ConfigureAppConfiguration((_, config) => + { + config.AddInMemoryCollection(new Dictionary + { + ["ScadaBridge:Node:NodeName"] = "central-a", + ["ScadaBridge:Node:NodeHostname"] = "localhost", + ["ScadaBridge:Node:RemotingPort"] = "0", + ["ScadaBridge:Cluster:SeedNodes:0"] = "akka.tcp://scadabridge@localhost:2551", + ["ScadaBridge:Cluster:SeedNodes:1"] = "akka.tcp://scadabridge@localhost:2552", + ["ScadaBridge:Database:SkipMigrations"] = "true", + ["ScadaBridge:Security:JwtSigningKey"] = "test-signing-key-must-be-at-least-32-chars-long!", + ["ScadaBridge:Security:Ldap:Server"] = "localhost", + ["ScadaBridge:Security:Ldap:Port"] = "3893", + ["ScadaBridge:Security:Ldap:Transport"] = "None", + ["ScadaBridge:Security:Ldap:AllowInsecure"] = "true", + ["ScadaBridge:Security:Ldap:SearchBase"] = "dc=zb,dc=local", + ["ScadaBridge:Security:Ldap:ServiceAccountDn"] = "cn=admin,dc=zb,dc=local", + ["ScadaBridge:InboundApi:ApiKeyPepper"] = "test-inbound-api-key-pepper-at-least-32-chars!", + }); + }); + builder.UseSetting("ScadaBridge:Node:Role", "Central"); + builder.UseSetting("ScadaBridge:Database:SkipMigrations", "true"); + builder.ConfigureServices(services => + { + var descriptorsToRemove = services + .Where(d => + d.ServiceType == typeof(DbContextOptions) || + d.ServiceType == typeof(DbContextOptions) || + d.ServiceType == typeof(ScadaBridgeDbContext) || + d.ServiceType.FullName?.Contains("EntityFrameworkCore") == true) + .ToList(); + foreach (var d in descriptorsToRemove) + services.Remove(d); + + services.AddDbContext(options => + options.UseInMemoryDatabase($"CaptiveGuard_{Guid.NewGuid()}")); + + AkkaHostedServiceRemover.RemoveAkkaHostedServiceOnly(services); + + // ConfigureServices runs after Program.cs has added everything, so this + // snapshot IS the Central composition root's registration set. + _descriptors.AddRange(services); + }); + }); + + _ = _factory.Server; + } + + public void Dispose() + { + try + { + _factory.Dispose(); + } + finally + { + Environment.SetEnvironmentVariable("DOTNET_ENVIRONMENT", _previousEnv); + _dbEnv.Dispose(); + } + } + + [Fact] + public void Central_NoActorHoldsCaptiveScopedState() + { + var scopedTypes = CaptiveDependencyGuard.ScopedTypes(_descriptors); + Assert.NotEmpty(scopedTypes); + + var violations = CaptiveDependencyGuard.Scan( + CaptiveDependencyGuard.ProductionActorTypes(), scopedTypes); + + Assert.True( + violations.Count == 0, + "Actors holding captive scope state (register row 45's bug class):\n " + + string.Join("\n ", violations)); + } +} + +/// +/// The same scan against the SITE composition root's real scoped registrations. Both roles +/// are checked because the scoped set differs between them — a type that is a singleton on +/// central can be scoped on a site, and only the role that registers it scoped can catch an +/// actor caching it. +/// +public class SiteCaptiveDependencyGuardTests : IDisposable +{ + private readonly string _tempDbPath; + private readonly string _tempTrackingDbPath; + private readonly string _tempLocalDbPath; + private readonly string _tempAuditDbPath; + private readonly IServiceCollection _services; + + public SiteCaptiveDependencyGuardTests() + { + _tempDbPath = Path.Combine(Path.GetTempPath(), $"sb_guard_{Guid.NewGuid()}.db"); + _tempTrackingDbPath = Path.Combine(Path.GetTempPath(), $"sb_guard_track_{Guid.NewGuid()}.db"); + _tempLocalDbPath = Path.Combine(Path.GetTempPath(), $"sb_guard_localdb_{Guid.NewGuid()}.db"); + _tempAuditDbPath = Path.Combine(Path.GetTempPath(), $"sb_guard_audit_{Guid.NewGuid()}.db"); + + var builder = WebApplication.CreateBuilder(); + builder.Configuration.Sources.Clear(); + builder.Configuration.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", + ["ScadaBridge:Database:SiteDbPath"] = _tempDbPath, + ["ScadaBridge:OperationTracking:ConnectionString"] = $"Data Source={_tempTrackingDbPath}", + ["LocalDb:Path"] = _tempLocalDbPath, + ["AuditLog:SiteWriter:DatabasePath"] = _tempAuditDbPath, + ["ScadaBridge:Cluster:SeedNodes:0"] = "akka.tcp://scadabridge@localhost:2551", + ["ScadaBridge:Cluster:SeedNodes:1"] = "akka.tcp://scadabridge@localhost:2552", + }); + + builder.Services.AddGrpc(); + SiteServiceRegistration.Configure(builder.Services, builder.Configuration); + + // Registration only — the provider is never built here, so nothing resolves and no + // site database file is opened. + _services = builder.Services; + } + + public void Dispose() + { + foreach (var path in new[] { _tempDbPath, _tempTrackingDbPath, _tempLocalDbPath, _tempAuditDbPath }) + { + try { File.Delete(path); } catch { /* best effort */ } + } + } + + [Fact] + public void Site_NoActorHoldsCaptiveScopedState() + { + var scopedTypes = CaptiveDependencyGuard.ScopedTypes(_services); + Assert.NotEmpty(scopedTypes); + + var violations = CaptiveDependencyGuard.Scan( + CaptiveDependencyGuard.ProductionActorTypes(), scopedTypes); + + Assert.True( + violations.Count == 0, + "Actors holding captive scope state (register row 45's bug class):\n " + + string.Join("\n ", violations)); + } +} diff --git a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/ServiceProviderValidationTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/ServiceProviderValidationTests.cs new file mode 100644 index 00000000..c16f2958 --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/ServiceProviderValidationTests.cs @@ -0,0 +1,232 @@ +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Mvc.Testing; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; +using ZB.MOM.WW.ScadaBridge.ConfigurationDatabase; + +namespace ZB.MOM.WW.ScadaBridge.Host.Tests; + +/// +/// Deferred register row 50, mitigation 1: .NET's built-in DI scope validation is on in +/// Development for BOTH composition roots, and both roots actually survive it. +/// +/// +/// The build-and-validate form is the load-bearing one. Asserting that +/// ValidateScopes is set proves only that a flag was written; asserting that the +/// full Central and Site graphs BUILD with ValidateOnBuild on is what proves there +/// is no latent singleton-captures-scoped violation to be surfaced — and it keeps +/// proving it as registrations are added, on every CI run, without anyone having to boot +/// a node in Development. +/// +/// Both roles force the flags on explicitly rather than relying on the environment, so +/// the check runs identically in CI (where the environment is whatever the runner +/// inherits) and so a future change to the environment plumbing cannot silently turn the +/// coverage off — the exact failure mode this row is about. +/// +public class ServiceProviderValidationOptionsTests +{ + [Fact] + public void Apply_InDevelopment_EnablesBothValidations() + { + var options = new ServiceProviderOptions(); + + ServiceProviderValidation.Apply(options, isDevelopment: true); + + Assert.True(options.ValidateScopes); + Assert.True(options.ValidateOnBuild); + } + + [Fact] + public void Apply_OutsideDevelopment_LeavesBothValidationsOff() + { + // Production pays neither the per-resolve scope check nor the boot-time graph + // walk, and — more importantly — a validation failure cannot fail a production + // node closed at boot. See ServiceProviderValidation's remarks. + var options = new ServiceProviderOptions { ValidateScopes = true, ValidateOnBuild = true }; + + ServiceProviderValidation.Apply(options, isDevelopment: false); + + Assert.False(options.ValidateScopes); + Assert.False(options.ValidateOnBuild); + } +} + +/// +/// Builds the REAL Central composition root (the whole Program.cs Central branch) +/// with scope + build validation forced on, and asserts it builds clean. +/// +[Collection(HostBootCollection.Name)] +public class CentralServiceProviderValidationTests : IDisposable +{ + private readonly string? _previousEnv; + private readonly CentralDbTestEnvironment _dbEnv; + private WebApplicationFactory? _factory; + + public CentralServiceProviderValidationTests() + { + _previousEnv = Environment.GetEnvironmentVariable("DOTNET_ENVIRONMENT"); + Environment.SetEnvironmentVariable("DOTNET_ENVIRONMENT", "Central"); + _dbEnv = new CentralDbTestEnvironment(); + } + + public void Dispose() + { + try + { + _factory?.Dispose(); + } + finally + { + Environment.SetEnvironmentVariable("DOTNET_ENVIRONMENT", _previousEnv); + _dbEnv.Dispose(); + } + } + + /// + /// A captive dependency anywhere in the Central graph — a singleton whose constructor + /// takes a scoped service — makes ValidateOnBuild throw an + /// here listing every offender, so this test names + /// the violation rather than leaving it to be found by a rig log line months later + /// (which is how row 45 was found). + /// + [Fact] + public void Central_CompositionRoot_BuildsWithScopeValidationEnabled() + { + _factory = new WebApplicationFactory() + .WithWebHostBuilder(builder => + { + // Force the flags on regardless of the runner's environment. This is the + // same configuration Program.cs applies in Development, exercised here as + // a hard assertion. + builder.UseDefaultServiceProvider(options => + { + options.ValidateScopes = true; + options.ValidateOnBuild = true; + }); + + builder.ConfigureAppConfiguration((_, config) => + { + config.AddInMemoryCollection(new Dictionary + { + ["ScadaBridge:Node:NodeName"] = "central-a", + ["ScadaBridge:Node:NodeHostname"] = "localhost", + ["ScadaBridge:Node:RemotingPort"] = "0", + ["ScadaBridge:Cluster:SeedNodes:0"] = "akka.tcp://scadabridge@localhost:2551", + ["ScadaBridge:Cluster:SeedNodes:1"] = "akka.tcp://scadabridge@localhost:2552", + ["ScadaBridge:Database:SkipMigrations"] = "true", + ["ScadaBridge:Security:JwtSigningKey"] = "test-signing-key-must-be-at-least-32-chars-long!", + ["ScadaBridge:Security:Ldap:Server"] = "localhost", + ["ScadaBridge:Security:Ldap:Port"] = "3893", + ["ScadaBridge:Security:Ldap:Transport"] = "None", + ["ScadaBridge:Security:Ldap:AllowInsecure"] = "true", + ["ScadaBridge:Security:Ldap:SearchBase"] = "dc=zb,dc=local", + ["ScadaBridge:Security:Ldap:ServiceAccountDn"] = "cn=admin,dc=zb,dc=local", + ["ScadaBridge:InboundApi:ApiKeyPepper"] = "test-inbound-api-key-pepper-at-least-32-chars!", + }); + }); + builder.UseSetting("ScadaBridge:Node:Role", "Central"); + builder.UseSetting("ScadaBridge:Database:SkipMigrations", "true"); + builder.ConfigureServices(services => + { + var descriptorsToRemove = services + .Where(d => + d.ServiceType == typeof(DbContextOptions) || + d.ServiceType == typeof(DbContextOptions) || + d.ServiceType == typeof(ScadaBridgeDbContext) || + d.ServiceType.FullName?.Contains("EntityFrameworkCore") == true) + .ToList(); + foreach (var d in descriptorsToRemove) + services.Remove(d); + + services.AddDbContext(options => + options.UseInMemoryDatabase($"ScopeValidation_{Guid.NewGuid()}")); + + AkkaHostedServiceRemover.RemoveAkkaHostedServiceOnly(services); + }); + }); + + // Building the server is the assertion: ValidateOnBuild throws from + // BuildServiceProvider, and ValidateScopes throws on the first offending resolve. + var services = _factory.Services; + Assert.NotNull(services); + } +} + +/// +/// The Site half. This is the branch that actually gains coverage from the Program.cs +/// change: the docker rig sets ASPNETCORE_ENVIRONMENT=Development on the central +/// containers but not on the site containers, so a site node has always built its +/// provider unvalidated. +/// +public class SiteServiceProviderValidationTests : IDisposable +{ + private readonly string _tempDbPath; + private readonly string _tempTrackingDbPath; + private readonly string _tempLocalDbPath; + private readonly string _tempAuditDbPath; + private WebApplication? _host; + + public SiteServiceProviderValidationTests() + { + _tempDbPath = Path.Combine(Path.GetTempPath(), $"sb_scopeval_{Guid.NewGuid()}.db"); + _tempTrackingDbPath = Path.Combine(Path.GetTempPath(), $"sb_scopeval_track_{Guid.NewGuid()}.db"); + _tempLocalDbPath = Path.Combine(Path.GetTempPath(), $"sb_scopeval_localdb_{Guid.NewGuid()}.db"); + _tempAuditDbPath = Path.Combine(Path.GetTempPath(), $"sb_scopeval_audit_{Guid.NewGuid()}.db"); + } + + public void Dispose() + { + (_host as IDisposable)?.Dispose(); + foreach (var path in new[] { _tempDbPath, _tempTrackingDbPath, _tempLocalDbPath, _tempAuditDbPath }) + { + try { File.Delete(path); } catch { /* best effort */ } + } + } + + [Fact] + public void Site_CompositionRoot_BuildsWithScopeValidationEnabled() + { + var builder = WebApplication.CreateBuilder(); + builder.Configuration.Sources.Clear(); + builder.Configuration.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", + ["ScadaBridge:Database:SiteDbPath"] = _tempDbPath, + ["ScadaBridge:OperationTracking:ConnectionString"] = $"Data Source={_tempTrackingDbPath}", + ["LocalDb:Path"] = _tempLocalDbPath, + ["AuditLog:SiteWriter:DatabasePath"] = _tempAuditDbPath, + ["ScadaBridge:Cluster:SeedNodes:0"] = "akka.tcp://scadabridge@localhost:2551", + ["ScadaBridge:Cluster:SeedNodes:1"] = "akka.tcp://scadabridge@localhost:2552", + }); + + // The production Program.cs Site branch applies this via + // ServiceProviderValidation.Configure; forced on here so the check does not depend + // on the test runner's environment. + builder.Host.UseDefaultServiceProvider(options => + { + options.ValidateScopes = true; + options.ValidateOnBuild = true; + }); + + // Mirrors the Program.cs site section. + builder.Services.AddGrpc(); + builder.Services.AddSingleton(); + builder.Services.AddSingleton(); + + SiteServiceRegistration.Configure(builder.Services, builder.Configuration); + AkkaHostedServiceRemover.RemoveAkkaHostedServiceOnly(builder.Services); + + // Build is the assertion — ValidateOnBuild throws here if anything in the site + // graph is a captive dependency. + _host = builder.Build(); + Assert.NotNull(_host.Services); + } +} diff --git a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/SiteHealthEndpointTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/SiteHealthEndpointTests.cs index 6f0718d7..d1088666 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/SiteHealthEndpointTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/SiteHealthEndpointTests.cs @@ -88,14 +88,14 @@ public class SiteHealthEndpointTests : IDisposable var factory = new WebApplicationFactory() .WithWebHostBuilder(builder => { - // WebApplicationFactory defaults the environment to Development, which turns on - // ValidateOnBuild/ValidateScopes. The site graph carries scoped registrations whose - // dependencies are central-only (ReconcileService → IDeploymentManagerRepository, - // AuditLogKpiSampleSource → IAuditLogRepository) and are never resolved on a site - // node, so eager validation fails on a composition that runs fine in production. - // That is a pre-existing property of the site container, not of the health wiring - // under test — run this fixture the way a site node actually runs. - builder.UseEnvironment("Production"); + // The environment is left at WebApplicationFactory's Development default on + // purpose, so this fixture boots the site branch with ValidateOnBuild/ + // ValidateScopes ON. It used to pin Production because the site graph carried two + // central-only scoped registrations it could not construct (ReconcileService → + // IDeploymentManagerRepository, AuditLogKpiSampleSource → IAuditLogRepository); + // deferred register row 50 moved both to central-only extensions + // (AddCentralCommunication / AddAuditLogCentralKpi), so the site container now + // validates clean and there is no reason to run it unvalidated. // ConfigureTestServices runs after the app's own registrations, so this drops the // hosted service that would form a real cluster while leaving the AkkaHostedService