merge: captive-dependency guard (register row 50) — scope validation in Development + actor field scan; fixes two Site-role unconstructible registrations
This commit is contained in:
Joseph Doherty
@@ -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
|
||||
|
||||
@@ -248,15 +248,38 @@ public static class ServiceCollectionExtensions
|
||||
// provider as a singleton on both site and central paths.
|
||||
sp.GetRequiredService<INodeIdentityProvider>()));
|
||||
|
||||
// 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;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Registers the Audit Log's KPI sample source. CENTRAL-ONLY — call from the Central
|
||||
/// composition root only, alongside <c>AddKpiHistory</c>.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// <para>
|
||||
/// 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 <c>KpiSample</c> history table. Scoped to match its <c>IAuditLogRepository</c>
|
||||
/// 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.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// Split out of <see cref="AddAuditLog"/> by deferred register row 50. Both the KPI
|
||||
/// recorder and <c>IAuditLogRepository</c> are central-only, so on a site node this
|
||||
/// registration was unconstructible — enough on its own to fail <c>ValidateOnBuild</c> for
|
||||
/// the entire site graph and so to keep scope validation switched off there. The site half
|
||||
/// of <see cref="AddAuditLog"/> (the SQLite writer chain) is unaffected and stays shared.
|
||||
/// </para>
|
||||
/// </remarks>
|
||||
/// <param name="services">The DI service collection to register services into.</param>
|
||||
/// <returns>The same <see cref="IServiceCollection"/> to allow chaining.</returns>
|
||||
public static IServiceCollection AddAuditLogCentralKpi(this IServiceCollection services)
|
||||
{
|
||||
ArgumentNullException.ThrowIfNull(services);
|
||||
|
||||
services.TryAddEnumerable(
|
||||
ServiceDescriptor.Scoped<IKpiSampleSource, AuditLogKpiSampleSource>());
|
||||
|
||||
|
||||
@@ -42,9 +42,33 @@ public static class ServiceCollectionExtensions
|
||||
services.AddSingleton<SiteAlarmLiveCacheService>();
|
||||
services.AddSingleton<ISiteAlarmLiveCache>(sp => sp.GetRequiredService<SiteAlarmLiveCacheService>());
|
||||
|
||||
return services;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Registers the communication services that only a CENTRAL node can construct.
|
||||
/// Call alongside <see cref="AddCommunication"/> from the Central composition root only.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// <para>
|
||||
/// <see cref="ReconcileService"/> is scoped and takes <c>IDeploymentManagerRepository</c> —
|
||||
/// an EF repository registered by <c>AddConfigurationDatabase</c>, which only central calls.
|
||||
/// It used to live in the shared <see cref="AddCommunication"/> 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 <c>ValidateOnBuild</c> 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.
|
||||
/// </para>
|
||||
/// </remarks>
|
||||
/// <param name="services">The DI service collection to register services into.</param>
|
||||
/// <returns>The same <see cref="IServiceCollection"/> to allow chaining.</returns>
|
||||
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<ReconcileService>();
|
||||
|
||||
return services;
|
||||
|
||||
@@ -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();
|
||||
|
||||
|
||||
@@ -0,0 +1,73 @@
|
||||
using Microsoft.Extensions.DependencyInjection;
|
||||
|
||||
namespace ZB.MOM.WW.ScadaBridge.Host;
|
||||
|
||||
/// <summary>
|
||||
/// 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
|
||||
/// <c>Program.cs</c>.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// <para>
|
||||
/// Deferred register row 50. Row 45 was a live captive dependency — the Notification
|
||||
/// Outbox actor cached <c>INotificationDeliveryAdapter</c> instances resolved from an
|
||||
/// actor-lifetime <c>IServiceScope</c>, so up to <c>MaxParallelDeliveries</c> concurrent
|
||||
/// deliveries shared one EF <c>DbContext</c> and central-b logged EF's
|
||||
/// "second operation started on this context instance" continuously (fixed in 9e769735).
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// <b>What this catches and what it does not.</b> <see cref="ServiceProviderOptions.ValidateScopes"/>
|
||||
/// fails a resolve where a SINGLETON directly injects a SCOPED service, and forbids resolving a
|
||||
/// scoped service from the root provider. <see cref="ServiceProviderOptions.ValidateOnBuild"/>
|
||||
/// 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
|
||||
/// <see cref="IServiceProvider"/> and call <c>GetRequiredService</c> by hand, which is opaque to
|
||||
/// static graph validation — that half of row 50 is covered by the reflection guard test
|
||||
/// (<c>CaptiveDependencyGuard</c> in the Host test suite). The two mitigations are
|
||||
/// complementary, and neither subsumes the other.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// <b>Why this is written out rather than left to the framework default.</b>
|
||||
/// <c>WebApplication.CreateBuilder</c> already sets both flags to
|
||||
/// <c>IsDevelopment()</c>, 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 <c>CreateBuilder</c> (or onto a bare <c>HostBuilder</c>) would drop it with
|
||||
/// no diagnostic; and (b) this host has THREE environment-ish signals —
|
||||
/// <c>SCADABRIDGE_CONFIG</c> (role config: Central/Site), <c>DOTNET_ENVIRONMENT</c> (role
|
||||
/// fallback), and <c>ASPNETCORE_ENVIRONMENT</c> (the actual <see cref="IHostEnvironment"/>) —
|
||||
/// so "is this node in Development" is genuinely non-obvious at the call site.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// <b>Production is deliberately excluded.</b> <c>ValidateScopes</c> costs a check on every
|
||||
/// resolve and <c>ValidateOnBuild</c> 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.
|
||||
/// </para>
|
||||
/// </remarks>
|
||||
internal static class ServiceProviderValidation
|
||||
{
|
||||
/// <summary>
|
||||
/// Enables scope + build-time validation when the host environment is Development.
|
||||
/// Wired via <c>builder.Host.UseDefaultServiceProvider(ServiceProviderValidation.Configure)</c>.
|
||||
/// </summary>
|
||||
/// <param name="context">The host builder context, read for its hosting environment.</param>
|
||||
/// <param name="options">The service-provider options to configure.</param>
|
||||
internal static void Configure(HostBuilderContext context, ServiceProviderOptions options)
|
||||
{
|
||||
Apply(options, context.HostingEnvironment.IsDevelopment());
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// The pure decision, split out so a test can assert both the on and off cases without a host.
|
||||
/// </summary>
|
||||
/// <param name="options">The service-provider options to configure.</param>
|
||||
/// <param name="isDevelopment">Whether the host environment is Development.</param>
|
||||
internal static void Apply(ServiceProviderOptions options, bool isDevelopment)
|
||||
{
|
||||
options.ValidateScopes = isDevelopment;
|
||||
options.ValidateOnBuild = isDevelopment;
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,287 @@
|
||||
using System.Reflection;
|
||||
using Akka.Actor;
|
||||
using Microsoft.EntityFrameworkCore;
|
||||
using Microsoft.Extensions.DependencyInjection;
|
||||
|
||||
namespace ZB.MOM.WW.ScadaBridge.Host.Tests;
|
||||
|
||||
/// <summary>
|
||||
/// Deferred register row 50, mitigation 2: a reflection guard that no Akka actor holds
|
||||
/// captive scope state — a scoped service, or an <see cref="IServiceScope"/>, cached for
|
||||
/// the actor's lifetime.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// <para>
|
||||
/// <b>Why this exists separately from scope validation.</b> Register row 45 was exactly this
|
||||
/// bug: <c>NotificationOutboxActor</c> cached delivery adapters resolved from an
|
||||
/// actor-lifetime <c>IServiceScope</c>, so <c>MaxParallelDeliveries</c> concurrent deliveries
|
||||
/// shared one EF <c>DbContext</c> and EF threw "a second operation was started on this
|
||||
/// context instance" continuously on the rig (fixed 9e769735). .NET's own
|
||||
/// <c>ValidateScopes</c> could never have caught it: the actors take the ROOT
|
||||
/// <see cref="IServiceProvider"/> and call <c>GetRequiredService</c> by hand, which the
|
||||
/// container's static graph analysis cannot see through. This guard closes that half.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// <b>What is allowed.</b> Holding <see cref="IServiceProvider"/> or
|
||||
/// <see cref="IServiceScopeFactory"/> 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.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// <b>How "scoped" is decided.</b> 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: <see cref="IServiceScope"/> itself, and any <see cref="DbContext"/>.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// Auto-property backing fields need no special handling: the compiler emits them as private
|
||||
/// instance fields, so <c>GetFields(NonPublic | Instance)</c> already returns them.
|
||||
/// </para>
|
||||
/// </remarks>
|
||||
internal static class CaptiveDependencyGuard
|
||||
{
|
||||
/// <summary>One offending field on one actor type.</summary>
|
||||
/// <param name="ActorType">The actor class declaring the field.</param>
|
||||
/// <param name="FieldName">The field's name (a compiler-generated backing field keeps its mangled name).</param>
|
||||
/// <param name="FieldType">The field's declared type.</param>
|
||||
/// <param name="Reason">Why the field was flagged.</param>
|
||||
internal sealed record Violation(Type ActorType, string FieldName, Type FieldType, string Reason)
|
||||
{
|
||||
/// <inheritdoc/>
|
||||
public override string ToString() =>
|
||||
$"{ActorType.FullName}.{FieldName} : {FieldType.FullName} — {Reason}";
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// 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.
|
||||
/// </summary>
|
||||
internal static readonly IReadOnlyList<Type> AllowedProviderTypes =
|
||||
[
|
||||
typeof(IServiceProvider),
|
||||
typeof(IServiceScopeFactory),
|
||||
];
|
||||
|
||||
/// <summary>
|
||||
/// Fields that hold a scoped-typed value but are provably safe, keyed
|
||||
/// <c>"Namespace.ActorType.fieldName"</c>. Every entry carries its safety argument —
|
||||
/// nothing is exempted silently.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// <para>
|
||||
/// Both entries below are the same shape: a nullable repository field assigned ONLY by an
|
||||
/// explicitly test-mode constructor, so the production wiring
|
||||
/// (<c>AkkaHostedService</c> constructs both actors through their <see cref="IServiceProvider"/>
|
||||
/// 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 <c>SiteCallAuditActor</c> additionally wraps
|
||||
/// the injected instance in its own <c>SerializedRepository</c> — i.e. the test path
|
||||
/// already carries a serialization guard for precisely the concurrency hazard row 45 hit.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// 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.
|
||||
/// </para>
|
||||
/// </remarks>
|
||||
internal static readonly IReadOnlySet<string> Allowlist = new HashSet<string>(StringComparer.Ordinal)
|
||||
{
|
||||
"ZB.MOM.WW.ScadaBridge.AuditLog.Central.AuditLogIngestActor._injectedRepository",
|
||||
"ZB.MOM.WW.ScadaBridge.SiteCallAudit.SiteCallAuditActor._injectedRepository",
|
||||
};
|
||||
|
||||
/// <summary>
|
||||
/// Scans the given actor types for instance fields holding captive scope state.
|
||||
/// </summary>
|
||||
/// <param name="actorTypes">Actor classes to inspect.</param>
|
||||
/// <param name="scopedServiceTypes">
|
||||
/// Service and implementation types registered with <see cref="ServiceLifetime.Scoped"/>
|
||||
/// in the composition root being checked.
|
||||
/// </param>
|
||||
/// <param name="applyAllowlist">
|
||||
/// 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.
|
||||
/// </param>
|
||||
/// <returns>Every violation found, in declaration order.</returns>
|
||||
internal static IReadOnlyList<Violation> Scan(
|
||||
IEnumerable<Type> actorTypes,
|
||||
IReadOnlySet<Type> scopedServiceTypes,
|
||||
bool applyAllowlist = true)
|
||||
{
|
||||
ArgumentNullException.ThrowIfNull(actorTypes);
|
||||
ArgumentNullException.ThrowIfNull(scopedServiceTypes);
|
||||
|
||||
var violations = new List<Violation>();
|
||||
|
||||
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;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// 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
|
||||
/// <see cref="ActorBase"/> internals are excluded without naming them.
|
||||
/// </summary>
|
||||
/// <param name="actorType">The actor class to walk.</param>
|
||||
/// <returns>The instance fields, most-derived first.</returns>
|
||||
private static IEnumerable<FieldInfo> 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;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>Whether a type belongs to a first-party ScadaBridge assembly.</summary>
|
||||
/// <param name="type">The type to test.</param>
|
||||
/// <returns>True when the declaring assembly is a ScadaBridge assembly.</returns>
|
||||
private static bool IsScadaBridgeType(Type type) =>
|
||||
type.Assembly.GetName().Name?.StartsWith(AssemblyPrefix, StringComparison.Ordinal) == true;
|
||||
|
||||
private const string AssemblyPrefix = "ZB.MOM.WW.ScadaBridge";
|
||||
|
||||
/// <summary>
|
||||
/// 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.
|
||||
/// </summary>
|
||||
/// <returns>The production actor types, ordered by full name.</returns>
|
||||
internal static IReadOnlyList<Type> 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();
|
||||
|
||||
/// <summary>
|
||||
/// Transitively loads and returns the shipped ScadaBridge assemblies.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// Referenced assemblies load lazily, so enumerating <c>AppDomain.CurrentDomain</c> 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.
|
||||
/// </remarks>
|
||||
/// <returns>The distinct production ScadaBridge assemblies.</returns>
|
||||
private static IReadOnlyList<Assembly> ProductionAssemblies()
|
||||
{
|
||||
var seen = new Dictionary<string, Assembly>(StringComparer.Ordinal);
|
||||
var pending = new Stack<Assembly>();
|
||||
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();
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Collects the service and implementation types registered with
|
||||
/// <see cref="ServiceLifetime.Scoped"/> in a composition root.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// Both sides of a descriptor are collected: an actor can just as easily cache the
|
||||
/// concrete <c>EmailNotificationDeliveryAdapter</c> as the <c>INotificationDeliveryAdapter</c>
|
||||
/// it is registered as, and row 45 in fact cached the interface.
|
||||
/// </remarks>
|
||||
/// <param name="services">The composition root's service collection.</param>
|
||||
/// <returns>The scoped service and implementation types.</returns>
|
||||
internal static IReadOnlySet<Type> ScopedTypes(IEnumerable<ServiceDescriptor> services)
|
||||
{
|
||||
ArgumentNullException.ThrowIfNull(services);
|
||||
|
||||
var scoped = new HashSet<Type>();
|
||||
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;
|
||||
}
|
||||
}
|
||||
@@ -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;
|
||||
|
||||
/// <summary>
|
||||
/// 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.
|
||||
/// </summary>
|
||||
public class CaptiveDependencyGuardSelfTests
|
||||
{
|
||||
/// <summary>A stand-in for any service the container registers <c>Scoped</c>.</summary>
|
||||
private interface IPretendScopedService;
|
||||
|
||||
/// <summary>
|
||||
/// POSITIVE CONTROL — the pre-row-45 shape, preserved permanently. An actor that caches a
|
||||
/// scoped-typed service for its lifetime is exactly what <c>NotificationOutboxActor</c>
|
||||
/// looked like before 9e769735, and the guard must report it.
|
||||
/// </summary>
|
||||
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
|
||||
}
|
||||
|
||||
/// <summary>POSITIVE CONTROL — caching the scope itself, the other half of row 45.</summary>
|
||||
private sealed class CachesScopeActor : ReceiveActor
|
||||
{
|
||||
#pragma warning disable CS0169, IDE0051, CA1823
|
||||
private readonly IServiceScope? _scope;
|
||||
#pragma warning restore CS0169, IDE0051, CA1823
|
||||
}
|
||||
|
||||
/// <summary>POSITIVE CONTROL — an EF context held for the actor's lifetime.</summary>
|
||||
private sealed class CachesDbContextActor : ReceiveActor
|
||||
{
|
||||
#pragma warning disable CS0169, IDE0051, CA1823
|
||||
private readonly ScadaBridgeDbContext? _db;
|
||||
#pragma warning restore CS0169, IDE0051, CA1823
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// 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.
|
||||
/// </summary>
|
||||
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
|
||||
}
|
||||
|
||||
/// <summary>POSITIVE CONTROL — an auto-property backing field must be caught too.</summary>
|
||||
private sealed class ScopedAutoPropertyActor : ReceiveActor
|
||||
{
|
||||
private IPretendScopedService? Captive { get; init; }
|
||||
}
|
||||
|
||||
private static readonly IReadOnlySet<Type> PretendScopedSet =
|
||||
new HashSet<Type> { 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<Type>());
|
||||
|
||||
var violation = Assert.Single(violations);
|
||||
Assert.Equal("_scope", violation.FieldName);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Guard_Flags_ActorCachingADbContext()
|
||||
{
|
||||
var violations = CaptiveDependencyGuard.Scan(
|
||||
[typeof(CachesDbContextActor)], new HashSet<Type>());
|
||||
|
||||
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);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// 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.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void ScopedTypes_CollectsServiceAndImplementationTypes()
|
||||
{
|
||||
var services = new ServiceCollection();
|
||||
services.AddScoped<IPretendScopedService, PretendScopedService>();
|
||||
services.AddSingleton<PretendSingleton>();
|
||||
|
||||
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;
|
||||
|
||||
/// <summary>
|
||||
/// 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.
|
||||
/// </summary>
|
||||
[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");
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// 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.
|
||||
/// </summary>
|
||||
[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.");
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// A minimal descriptor set naming the two repository types the allowlisted fields hold,
|
||||
/// so <see cref="Allowlist_EntriesAllStillMatchARealFlaggedField"/> does not need to boot a
|
||||
/// host. Both are registered <c>Scoped</c> by <c>AddConfigurationDatabase</c>; the
|
||||
/// role-scan tests below verify that against the real container.
|
||||
/// </summary>
|
||||
/// <returns>The probe descriptors.</returns>
|
||||
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;
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Runs the guard against every shipped actor using the CENTRAL composition root's real
|
||||
/// scoped registrations.
|
||||
/// </summary>
|
||||
[Collection(HostBootCollection.Name)]
|
||||
public class CentralCaptiveDependencyGuardTests : IDisposable
|
||||
{
|
||||
private readonly WebApplicationFactory<Program> _factory;
|
||||
private readonly string? _previousEnv;
|
||||
private readonly CentralDbTestEnvironment _dbEnv;
|
||||
private readonly List<ServiceDescriptor> _descriptors = [];
|
||||
|
||||
public CentralCaptiveDependencyGuardTests()
|
||||
{
|
||||
_previousEnv = Environment.GetEnvironmentVariable("DOTNET_ENVIRONMENT");
|
||||
Environment.SetEnvironmentVariable("DOTNET_ENVIRONMENT", "Central");
|
||||
_dbEnv = new CentralDbTestEnvironment();
|
||||
|
||||
_factory = new WebApplicationFactory<Program>()
|
||||
.WithWebHostBuilder(builder =>
|
||||
{
|
||||
builder.ConfigureAppConfiguration((_, config) =>
|
||||
{
|
||||
config.AddInMemoryCollection(new Dictionary<string, string?>
|
||||
{
|
||||
["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<ScadaBridgeDbContext>) ||
|
||||
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<ScadaBridgeDbContext>(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));
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// 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.
|
||||
/// </summary>
|
||||
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<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",
|
||||
["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));
|
||||
}
|
||||
}
|
||||
@@ -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;
|
||||
|
||||
/// <summary>
|
||||
/// 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.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// The build-and-validate form is the load-bearing one. Asserting that
|
||||
/// <c>ValidateScopes</c> is set proves only that a flag was written; asserting that the
|
||||
/// full Central and Site graphs BUILD with <c>ValidateOnBuild</c> 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.
|
||||
/// </remarks>
|
||||
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);
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Builds the REAL Central composition root (the whole <c>Program.cs</c> Central branch)
|
||||
/// with scope + build validation forced on, and asserts it builds clean.
|
||||
/// </summary>
|
||||
[Collection(HostBootCollection.Name)]
|
||||
public class CentralServiceProviderValidationTests : IDisposable
|
||||
{
|
||||
private readonly string? _previousEnv;
|
||||
private readonly CentralDbTestEnvironment _dbEnv;
|
||||
private WebApplicationFactory<Program>? _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();
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// A captive dependency anywhere in the Central graph — a singleton whose constructor
|
||||
/// takes a scoped service — makes <c>ValidateOnBuild</c> throw an
|
||||
/// <see cref="AggregateException"/> 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).
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void Central_CompositionRoot_BuildsWithScopeValidationEnabled()
|
||||
{
|
||||
_factory = new WebApplicationFactory<Program>()
|
||||
.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<string, string?>
|
||||
{
|
||||
["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<ScadaBridgeDbContext>) ||
|
||||
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<ScadaBridgeDbContext>(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);
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// The Site half. This is the branch that actually gains coverage from the Program.cs
|
||||
/// change: the docker rig sets <c>ASPNETCORE_ENVIRONMENT=Development</c> on the central
|
||||
/// containers but not on the site containers, so a site node has always built its
|
||||
/// provider unvalidated.
|
||||
/// </summary>
|
||||
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<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",
|
||||
["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<ZB.MOM.WW.ScadaBridge.Communication.Grpc.SiteStreamGrpcServer>();
|
||||
builder.Services.AddSingleton<ZB.MOM.WW.ScadaBridge.Communication.Grpc.SiteCommandGrpcService>();
|
||||
|
||||
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);
|
||||
}
|
||||
}
|
||||
@@ -88,14 +88,14 @@ public class SiteHealthEndpointTests : IDisposable
|
||||
var factory = new WebApplicationFactory<Program>()
|
||||
.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
|
||||
|
||||
Reference in New Issue
Block a user