merge(agent-d): low-tail sweep rows 36/37/58/59/60/64/65 — retention pin + lazy-loading pin + serial-loop remark + row-65 prune tick on BundleSessionEvictionService
This commit is contained in:
@@ -138,7 +138,7 @@ The two levels interact in a specific order, and it is load-bearing (register ro
|
||||
|
||||
- **It fails OPEN.** A store error logs a Warning naming the client and the cap, and *permits* the attempt. This is a best-effort rate cap layered over a surface that is already authenticated and Administrator-gated, so the availability of legitimate imports outranks the cap — a database blip must not make the import wizard unusable. Falling back to the old process-local window instead was considered and rejected: it would still block a legitimate operator's 11th import during an outage, which is precisely the outcome fail-open exists to prevent. The Warning is the operator's signal that the fleet cap is not currently being enforced.
|
||||
- **Accuracy is deliberately approximate.** The count-then-insert pair is not atomic across the two nodes, so genuinely simultaneous attempts can overshoot the cap by up to the number in flight. A conditional single-statement insert would close that at the cost of provider-specific raw SQL; the overshoot is bounded and self-corrects within the window.
|
||||
- **Rows are pruned opportunistically, not by a background service.** Transport owns no central scheduler, so the prune rides the unlock path: claimed by CAS at most once per 15 minutes per process, bounded to 1000 rows per pass, retaining `2 ×` the window so a prune racing a count can never delete a row the count still needs, and wrapped in a try/catch that logs and swallows — retention maintenance must never fail the operator's actual unlock. This is the same shape `DeploymentService` uses for terminal deployment records.
|
||||
- **Rows are pruned by two drivers sharing one cadence claim (register row 65).** The prune is claimed by CAS at most once per 15 minutes per process, bounded to 1000 rows per pass, retaining `2 ×` the window so a prune racing a count can never delete a row the count still needs, and wrapped in a try/catch that logs and swallows — retention maintenance must never fail the operator's actual unlock. This is the same shape `DeploymentService` uses for terminal deployment records. It originally rode the **unlock path only**, on the reasoning that Transport owned no central scheduler; the consequence was that a cluster which stopped importing entirely left its last rows in place indefinitely, since only a node actively serving unlocks ever pruned. Transport has since gained exactly such a scheduler — `BundleSessionEvictionService`, the central-only 1-minute session sweep — so it now also ticks `IBundleUnlockAttemptWindow.PruneAsync`, and the table converges whether or not anyone is importing. Because both drivers take the **same** claim, ticking on the 1-minute sweep costs at most one bounded DELETE per quarter hour and the two can never prune concurrently. `PruneAsync` is contractually non-throwing (unlike `TryRegisterAttemptAsync`, which throws and lets the limiter own the fail-open decision) — a background sweep has no caller to report to and must survive a database blip.
|
||||
|
||||
Behaviour the operator sees is otherwise unchanged: same config key, same semantics ("attempts, not failures"), same ordering relative to the per-bundle lockout, same `<unattributed>` bucket.
|
||||
|
||||
|
||||
@@ -338,7 +338,7 @@ Imports are all-or-nothing per bundle. A bundle either applies fully or not at a
|
||||
- **Passphrase never persisted.** It lives only inside the export/import service call path and is discarded after use. No environment variable, no log line.
|
||||
- **Failed-unlock rate limit:** per-bundle 3-strike lockout (keyed by `ContentHash`, server-side); per-IP-per-hour cap (`MaxUnlockAttemptsPerIpPerHour`, default 10, configurable) to deter brute force against a stolen bundle. Each failed attempt produces a `BundleImportUnlockFailed` audit row. The two are evaluated in order — bundle lockout, then per-IP throttle, then decrypt — so hammering an already-locked bundle spends no hourly budget, and a throttle denial (`BundleUnlockThrottledException`, fail-closed with a curated message on both the UI and CLI surfaces) burns **no** per-bundle strike, since it never reaches the decryptor. Both typed exceptions live in `Commons/Types/Transport/` — `ManagementService` references Commons but not Transport, so a type declared beside the importer cannot be curated by `MapFault` and reaches the CLI as a generic internal-error message (register rows 44 and 51). Successful unlocks count against the hourly budget too: it caps attempts, not failures. The client key is the caller's remote IP — from `CircuitClientAddressProvider` (snapshotted at Blazor circuit establishment, since `HttpContext` is unavailable mid-circuit) for the Central UI, and from `ManagementEnvelope.ClientIp` for the CLI; a caller with no attributable address shares a single stricter bucket rather than bypassing the throttle. Per-client isolation is only real behind a ForwardedHeaders-trusted proxy, the same caveat the login throttle carries in `Component-Security.md`. (Register row 44: the limiter existed, was configured and unit-tested, but was never invoked until this was wired.)
|
||||
|
||||
**The per-IP window is fleet-wide and restart-durable (register row 52).** It is persisted in the central MS SQL `BundleUnlockAttempt` table — the HA store both central nodes already share — rather than in each node's process memory, so the documented cap is a fleet cap and not N × the cap, and a node restart no longer clears it. `BundleUnlockRateLimiter` still owns the policy; `SqlBundleUnlockAttemptWindow` is the store behind the `IBundleUnlockAttemptWindow` seam, and `IBundleUnlockAttemptRepository` (Commons interface, ConfigurationDatabase implementation) is the persistence. Three deliberate trades: **the throttle fails OPEN** — a store error logs a Warning and permits the attempt, because this is a best-effort rate cap over an already-authenticated Administrator-gated surface and the availability of legitimate imports outranks the cap; **accuracy is approximate** — count-then-insert is not atomic across nodes, so simultaneous attempts can overshoot by the number in flight, which is bounded and self-corrects within the window; and **rows are pruned opportunistically on the unlock path** (CAS-claimed at most every 15 minutes per process, bounded to 1000 rows, retaining twice the window, best-effort) rather than by a new background service, since Transport owns no central scheduler.
|
||||
**The per-IP window is fleet-wide and restart-durable (register row 52).** It is persisted in the central MS SQL `BundleUnlockAttempt` table — the HA store both central nodes already share — rather than in each node's process memory, so the documented cap is a fleet cap and not N × the cap, and a node restart no longer clears it. `BundleUnlockRateLimiter` still owns the policy; `SqlBundleUnlockAttemptWindow` is the store behind the `IBundleUnlockAttemptWindow` seam, and `IBundleUnlockAttemptRepository` (Commons interface, ConfigurationDatabase implementation) is the persistence. Three deliberate trades: **the throttle fails OPEN** — a store error logs a Warning and permits the attempt, because this is a best-effort rate cap over an already-authenticated Administrator-gated surface and the availability of legitimate imports outranks the cap; **accuracy is approximate** — count-then-insert is not atomic across nodes, so simultaneous attempts can overshoot by the number in flight, which is bounded and self-corrects within the window; and **rows are pruned best-effort** (CAS-claimed at most every 15 minutes per process, bounded to 1000 rows, retaining twice the window). That prune originally rode the unlock path alone, since Transport owned no central scheduler — which meant a cluster that stopped importing entirely left its last rows in place indefinitely. **Register row 65 closed that**: Transport's central-only `BundleSessionEvictionService` sweep now also ticks `IBundleUnlockAttemptWindow.PruneAsync`, sharing the one 15-minute cadence claim with the unlock path, so the table converges with no import traffic at all. `PruneAsync` never throws (the fail-open policy stays with the limiter on the attempt path).
|
||||
- **Bundle size cap** on upload (default 100 MB, configurable) to bound memory.
|
||||
- **In-transit:** existing HTTPS to the Central UI; no new channel.
|
||||
- **Audit trail is the chain of custody.** Every export, every import (including aborted ones at validation), and every unlock failure is audit-logged with source env, content hash, encrypted yes/no, and artifact summary.
|
||||
|
||||
@@ -291,7 +291,22 @@ public class ArtifactDeploymentService
|
||||
site.Id, globals, cancellationToken, deploymentId);
|
||||
}
|
||||
|
||||
// Deploy to each site in parallel with per-site timeout
|
||||
// Deploy to each site in parallel with per-site timeout.
|
||||
//
|
||||
// Deferred-work register row 60 — what makes this fan-out safe, stated precisely
|
||||
// because the invariant is easy to break from a distance:
|
||||
// * `globals` is NOT EF state. FetchGlobalArtifactsAsync projects the entities
|
||||
// into pure Commons message DTOs and drops the entities, so the snapshot the
|
||||
// lambda closes over cannot touch a DbContext at all.
|
||||
// * `site` IS a tracked EF entity, and it is the one piece of EF state that
|
||||
// genuinely crosses into the parallel phase. Only already-materialised scalar
|
||||
// properties are read below (Id, SiteIdentifier, Name) and NO navigation
|
||||
// property is traversed — with lazy loading off (pinned by
|
||||
// ConfigurationDatabase.Tests.AddConfigurationDatabase_DoesNotEnableLazyLoading)
|
||||
// those are plain field reads that never re-enter the shared context.
|
||||
// So: do not traverse a navigation property on `site` in here, and do not move
|
||||
// per-site command building (which DOES query) inside this lambda — either one
|
||||
// puts concurrent load on one non-thread-safe DbContext.
|
||||
var tasks = sites.Select(async site =>
|
||||
{
|
||||
try
|
||||
|
||||
@@ -522,6 +522,20 @@ public class SiteCallAuditActor : ReceiveActor
|
||||
/// pass (the failing site's cursor is left at its previous value so the next
|
||||
/// tick retries the same window).
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// <b>SERIAL BY DESIGN — do NOT fan this loop out (register row 59).</b> The
|
||||
/// <paramref name="repository"/> is a single EF Core <c>DbContext</c>-backed instance
|
||||
/// resolved once per tick by <see cref="OnReconciliationTickAsync"/>, and a DbContext
|
||||
/// is not thread-safe. The `await`-in-`foreach` below is the ONLY thing keeping this
|
||||
/// tick's scope — which spans every site x up to
|
||||
/// <see cref="MaxReconciliationPagesPerTick"/> pages of upserts — to one concurrent
|
||||
/// user. Replacing it with <c>Task.WhenAll</c> to overlap the per-site network pulls
|
||||
/// would reintroduce register row 45 (concurrent use of one scoped DbContext), which
|
||||
/// surfaces as sporadic
|
||||
/// "A second operation was started on this context instance" failures rather than a
|
||||
/// clean break. If the serial pass ever becomes too slow, the fix is a scope PER SITE
|
||||
/// (resolve a repository inside the loop body), not a shared scope under fan-out.
|
||||
/// </remarks>
|
||||
private async Task ReconcileSitesAsync(
|
||||
IReadOnlyList<SiteEntry> sites, IPullSiteCallsClient client, ISiteCallAuditRepository repository)
|
||||
{
|
||||
|
||||
@@ -14,12 +14,22 @@ namespace ZB.MOM.WW.ScadaBridge.Transport.Import;
|
||||
/// design contract is "bundles are not retained server-side after ApplyAsync
|
||||
/// commits". This service keeps abandoned / failed sessions from pinning that
|
||||
/// plaintext for the full 30-minute TTL when no other traffic flows.
|
||||
/// <para>
|
||||
/// <b>Register row 65: it also ticks the bundle-unlock attempt prune.</b> That retention
|
||||
/// sweep used to ride the unlock path alone, on the reasoning that Transport owned no
|
||||
/// central scheduler — this service is that scheduler, so a cluster that stops importing
|
||||
/// entirely no longer leaves its last <c>BundleUnlockAttempt</c> rows in place
|
||||
/// indefinitely. The window keeps its own 15-minute cadence claim, so ticking it on the
|
||||
/// 1-minute session sweep costs at most one bounded DELETE per quarter hour, and the two
|
||||
/// drivers can never prune concurrently.
|
||||
/// </para>
|
||||
/// </summary>
|
||||
internal sealed class BundleSessionEvictionService : BackgroundService
|
||||
{
|
||||
private static readonly TimeSpan SweepInterval = TimeSpan.FromMinutes(1);
|
||||
|
||||
private readonly IBundleSessionStore _sessionStore;
|
||||
private readonly IBundleUnlockAttemptWindow _unlockAttemptWindow;
|
||||
private readonly ILogger<BundleSessionEvictionService> _logger;
|
||||
|
||||
/// <summary>
|
||||
@@ -35,12 +45,15 @@ internal sealed class BundleSessionEvictionService : BackgroundService
|
||||
/// Initializes a new instance of <see cref="BundleSessionEvictionService"/>.
|
||||
/// </summary>
|
||||
/// <param name="sessionStore">The bundle session store to sweep for expired sessions.</param>
|
||||
/// <param name="unlockAttemptWindow">Shared unlock-attempt window whose retention prune this sweep also ticks (register row 65).</param>
|
||||
/// <param name="logger">Logger for sweep diagnostics.</param>
|
||||
public BundleSessionEvictionService(
|
||||
IBundleSessionStore sessionStore,
|
||||
IBundleUnlockAttemptWindow unlockAttemptWindow,
|
||||
ILogger<BundleSessionEvictionService> logger)
|
||||
{
|
||||
_sessionStore = sessionStore ?? throw new ArgumentNullException(nameof(sessionStore));
|
||||
_unlockAttemptWindow = unlockAttemptWindow ?? throw new ArgumentNullException(nameof(unlockAttemptWindow));
|
||||
_logger = logger ?? throw new ArgumentNullException(nameof(logger));
|
||||
_openSessionCountProvider = () => _sessionStore.OpenSessionCount;
|
||||
}
|
||||
@@ -95,6 +108,26 @@ internal sealed class BundleSessionEvictionService : BackgroundService
|
||||
{
|
||||
_logger.LogWarning(ex, "Bundle session sweep failed; will retry on next interval.");
|
||||
}
|
||||
|
||||
// Register row 65: retention tick for the fleet-wide unlock-attempt window.
|
||||
// PruneAsync is contractually non-throwing and self-limits to its own
|
||||
// 15-minute cadence, so this is a cheap no-op on most sweeps. Kept separate
|
||||
// from the session sweep's try so neither maintenance duty can mask the other.
|
||||
try
|
||||
{
|
||||
await _unlockAttemptWindow
|
||||
.PruneAsync(BundleUnlockRateLimiter.DefaultWindow, stoppingToken)
|
||||
.ConfigureAwait(false);
|
||||
}
|
||||
catch (OperationCanceledException) when (stoppingToken.IsCancellationRequested)
|
||||
{
|
||||
return;
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
_logger.LogWarning(
|
||||
ex, "Bundle-unlock attempt prune tick failed; will retry on next interval.");
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -25,4 +25,23 @@ public interface IBundleUnlockAttemptWindow
|
||||
/// <returns><c>true</c> when the attempt was recorded (within budget); <c>false</c> when the bucket is exhausted.</returns>
|
||||
Task<bool> TryRegisterAttemptAsync(
|
||||
string clientKey, int maxAttemptsPerWindow, TimeSpan window, CancellationToken ct = default);
|
||||
|
||||
/// <summary>
|
||||
/// Register row 65: drives one retention sweep of aged-out attempt rows independently
|
||||
/// of the unlock path. The prune used to ride <see cref="TryRegisterAttemptAsync"/>
|
||||
/// only, so a cluster that stopped importing entirely left its last rows in place
|
||||
/// indefinitely; <c>BundleSessionEvictionService</c> — Transport's periodic
|
||||
/// central-only sweep — now also ticks this, which closes that.
|
||||
/// <para>
|
||||
/// Unlike <see cref="TryRegisterAttemptAsync"/>, implementations must NOT throw on
|
||||
/// store failure: this is pure retention maintenance with no caller to report to, and
|
||||
/// the sweep that drives it must never be taken down by a database blip. Implementations
|
||||
/// are also expected to keep their own cadence claim, so calling this more often than
|
||||
/// the intended prune interval is harmless.
|
||||
/// </para>
|
||||
/// </summary>
|
||||
/// <param name="window">Trailing window the attempts are counted over (one hour in production); rows are retained for a multiple of it.</param>
|
||||
/// <param name="ct">Cancellation token.</param>
|
||||
/// <returns>A task that completes when the prune attempt has finished (or been skipped for cadence).</returns>
|
||||
Task PruneAsync(TimeSpan window, CancellationToken ct = default);
|
||||
}
|
||||
|
||||
@@ -27,11 +27,12 @@ namespace ZB.MOM.WW.ScadaBridge.Transport.Import;
|
||||
public sealed class SqlBundleUnlockAttemptWindow : IBundleUnlockAttemptWindow
|
||||
{
|
||||
/// <summary>
|
||||
/// Minimum wall-clock gap between opportunistic prunes. The prune runs on the unlock
|
||||
/// path (Transport owns no central scheduler, and the design constraint was "no new
|
||||
/// background service"), so it is claimed at most this often per process — the same
|
||||
/// rate-limited opportunistic-sweep shape <c>DeploymentService</c> uses for terminal
|
||||
/// deployment records.
|
||||
/// Minimum wall-clock gap between prunes, claimed at most this often per process — the
|
||||
/// same rate-limited sweep shape <c>DeploymentService</c> uses for terminal deployment
|
||||
/// records. Both prune drivers share the one claim: the opportunistic sweep on the
|
||||
/// unlock path, and (register row 65) <c>BundleSessionEvictionService</c>'s periodic
|
||||
/// tick, which is what keeps the table converging on a cluster that has stopped
|
||||
/// importing altogether.
|
||||
/// </summary>
|
||||
internal static readonly TimeSpan PruneMinInterval = TimeSpan.FromMinutes(15);
|
||||
|
||||
@@ -116,6 +117,39 @@ public sealed class SqlBundleUnlockAttemptWindow : IBundleUnlockAttemptWindow
|
||||
return true;
|
||||
}
|
||||
|
||||
/// <inheritdoc />
|
||||
public async Task PruneAsync(TimeSpan window, CancellationToken ct = default)
|
||||
{
|
||||
if (window <= TimeSpan.Zero)
|
||||
{
|
||||
throw new ArgumentOutOfRangeException(nameof(window), "Window must be positive.");
|
||||
}
|
||||
|
||||
// Register row 65: the same rate-limited, bounded, best-effort sweep the unlock
|
||||
// path runs — deliberately sharing the ONE prune claim, so a background tick and a
|
||||
// concurrent unlock cannot both prune, and the effective cadence stays
|
||||
// PruneMinInterval no matter how often this is called.
|
||||
try
|
||||
{
|
||||
using var scope = _scopeFactory.CreateScope();
|
||||
var repo = scope.ServiceProvider.GetRequiredService<IBundleUnlockAttemptRepository>();
|
||||
await TryPruneAsync(repo, _timeProvider.GetUtcNow().UtcDateTime, window, ct)
|
||||
.ConfigureAwait(false);
|
||||
}
|
||||
catch (OperationCanceledException) when (ct.IsCancellationRequested)
|
||||
{
|
||||
throw;
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
// The interface contract is "must not throw": there is no caller to report to,
|
||||
// and the periodic sweep driving this must survive a database blip. Resolving
|
||||
// the scope/repository is outside TryPruneAsync's own try, so it is caught here.
|
||||
_logger.LogWarning(
|
||||
ex, "Scheduled bundle-unlock attempt prune failed; will retry on the next sweep.");
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Opportunistic, rate-limited, bounded prune of aged-out attempt rows. Best-effort:
|
||||
/// any failure is logged and swallowed, because retention maintenance must never fail
|
||||
|
||||
@@ -112,6 +112,118 @@ public class DeploymentsPushUpdateTests : BunitContext
|
||||
Arg.Any<DeploymentListFilter>(), Arg.Any<int>(), Arg.Any<int>(), Arg.Any<CancellationToken>());
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Deferred-work register row 58: the notifier is a process SINGLETON that retains
|
||||
/// delegates targeting circuit-scoped components, so an un-detached handler leaks the
|
||||
/// whole component graph (and its circuit) for the process lifetime.
|
||||
/// <para>
|
||||
/// The sibling <see cref="Deployments_Dispose_UnsubscribesFromNotifier"/> pins the
|
||||
/// observable behaviour, but it is satisfied by the <c>_disposed</c> guard ALONE —
|
||||
/// delete the <c>-=</c> from <c>Dispose</c> and it still passes while the leak is
|
||||
/// wide open. This test pins the retention property structurally instead: the
|
||||
/// notifier's invocation list must actually shrink back to empty.
|
||||
/// </para>
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void Deployments_Dispose_ActuallyDetachesTheDelegate_NotJustGuardsIt()
|
||||
{
|
||||
RegisterServices();
|
||||
var cut = Render<DeploymentsPage>();
|
||||
|
||||
Assert.Equal(1, SubscriberCount(_notifier));
|
||||
|
||||
cut.Instance.Dispose();
|
||||
|
||||
Assert.Equal(0, SubscriberCount(_notifier));
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Reads the compiler-generated backing field of the field-like
|
||||
/// <c>StatusChanged</c> event and counts its invocation list. Reflection is the only
|
||||
/// way to observe retention — the notifier deliberately exposes no subscriber count,
|
||||
/// and a behavioural probe cannot distinguish "detached" from "attached but guarded".
|
||||
/// </summary>
|
||||
private static int SubscriberCount(DeploymentStatusNotifier notifier)
|
||||
{
|
||||
var field = typeof(DeploymentStatusNotifier).GetField(
|
||||
"StatusChanged", BindingFlags.Instance | BindingFlags.NonPublic);
|
||||
Assert.NotNull(field);
|
||||
|
||||
var handlers = (Delegate?)field!.GetValue(notifier);
|
||||
return handlers?.GetInvocationList().Length ?? 0;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Deferred-work register row 58's TRIGGER, pinned: "a second page subscribes, or any
|
||||
/// subscriber lands without a matching <c>Dispose</c> unsubscribe". Row 58 is
|
||||
/// acceptable only while every subscriber detaches itself; a new page that subscribes
|
||||
/// and forgets is a silent circuit leak that no existing test would catch, because
|
||||
/// every other pin is written against the one known subscriber.
|
||||
/// <para>
|
||||
/// A source scan rather than reflection: the subscription is a <c>+=</c> operator
|
||||
/// inside a method body, which is not visible on the type's reflected surface.
|
||||
/// Mirrors the source-scan convention in <c>Commons.Tests/ArchitecturalConstraintTests</c>.
|
||||
/// </para>
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void EveryCentralUiSubscriberToTheNotifier_UnsubscribesAndIsDisposable()
|
||||
{
|
||||
var sourceDir = GetCentralUiSourceDirectory();
|
||||
var sourceFiles = Directory
|
||||
.GetFiles(sourceDir, "*.razor", SearchOption.AllDirectories)
|
||||
.Concat(Directory.GetFiles(sourceDir, "*.cs", SearchOption.AllDirectories))
|
||||
.Where(f => !f.Contains($"{Path.DirectorySeparatorChar}obj{Path.DirectorySeparatorChar}",
|
||||
StringComparison.Ordinal)
|
||||
&& !f.Contains($"{Path.DirectorySeparatorChar}bin{Path.DirectorySeparatorChar}",
|
||||
StringComparison.Ordinal));
|
||||
|
||||
var subscribers = new List<string>();
|
||||
foreach (var file in sourceFiles)
|
||||
{
|
||||
var content = File.ReadAllText(file);
|
||||
if (!content.Contains("StatusChanged +=", StringComparison.Ordinal))
|
||||
{
|
||||
continue;
|
||||
}
|
||||
|
||||
subscribers.Add(file);
|
||||
|
||||
Assert.True(
|
||||
content.Contains("StatusChanged -=", StringComparison.Ordinal),
|
||||
$"{Path.GetFileName(file)} subscribes to IDeploymentStatusNotifier.StatusChanged but never "
|
||||
+ "unsubscribes. The notifier is a singleton, so the delegate pins this circuit-scoped "
|
||||
+ "component for the process lifetime (deferred-work register row 58).");
|
||||
|
||||
Assert.True(
|
||||
content.Contains("IDisposable", StringComparison.Ordinal)
|
||||
|| content.Contains("IAsyncDisposable", StringComparison.Ordinal),
|
||||
$"{Path.GetFileName(file)} subscribes to IDeploymentStatusNotifier.StatusChanged but does not "
|
||||
+ "implement IDisposable/IAsyncDisposable, so its unsubscribe can never run.");
|
||||
}
|
||||
|
||||
// Guard the guard: if the scan silently stops matching (file moved, API renamed)
|
||||
// the loop above would vacuously pass and the trigger would go unwatched.
|
||||
Assert.NotEmpty(subscribers);
|
||||
}
|
||||
|
||||
/// <summary>Walks up from the test output directory to the CentralUI project source root.</summary>
|
||||
private static string GetCentralUiSourceDirectory()
|
||||
{
|
||||
var dir = new DirectoryInfo(AppContext.BaseDirectory);
|
||||
while (dir != null)
|
||||
{
|
||||
var candidate = Path.Combine(dir.FullName, "src", "ZB.MOM.WW.ScadaBridge.CentralUI");
|
||||
if (Directory.Exists(candidate))
|
||||
{
|
||||
return candidate;
|
||||
}
|
||||
|
||||
dir = dir.Parent;
|
||||
}
|
||||
|
||||
throw new InvalidOperationException("Could not locate the ZB.MOM.WW.ScadaBridge.CentralUI source directory.");
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Regression test for CentralUI-022. The notifier is a process singleton:
|
||||
/// it can read its subscriber list and begin invoking
|
||||
|
||||
+71
@@ -48,6 +48,77 @@ public class ServiceCollectionExtensionsTests
|
||||
#pragma warning restore EF1001
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Deferred-work register row 60: <c>ArtifactDeploymentService</c> builds its per-site
|
||||
/// deploy commands serially ("DbContext is not thread-safe") and then fans the SEND out
|
||||
/// with <c>Task.WhenAll</c>, with EF-materialised <c>Site</c> entities captured by the
|
||||
/// parallel lambda. That is safe for exactly one reason: with lazy loading off, reading
|
||||
/// a property on a materialised entity is a plain field read that never re-enters the
|
||||
/// DbContext. Turn lazy-loading proxies on and every one of those reads becomes a
|
||||
/// database round-trip on the shared context from N threads at once — the row's named
|
||||
/// trigger, and a failure mode that surfaces as sporadic "A second operation was
|
||||
/// started on this context instance" rather than a clean break.
|
||||
/// <para>
|
||||
/// Pinned here rather than at the call site because the hazard is a DI/configuration
|
||||
/// change made somewhere else entirely, by someone who will never read
|
||||
/// <c>ArtifactDeploymentService</c>.
|
||||
/// </para>
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void AddConfigurationDatabase_DoesNotEnableLazyLoading()
|
||||
{
|
||||
var services = new ServiceCollection();
|
||||
services.AddConfigurationDatabase("Server=unused;Database=unused;");
|
||||
using var sp = services.BuildServiceProvider();
|
||||
|
||||
var options = sp.GetRequiredService<DbContextOptions<ScadaBridgeDbContext>>();
|
||||
|
||||
Assert.DoesNotContain(
|
||||
options.Extensions,
|
||||
e => e.GetType().Name.Contains("Proxies", StringComparison.Ordinal));
|
||||
|
||||
// Belt and braces, and the structurally stronger half: UseLazyLoadingProxies() is
|
||||
// not even a compilable call without the Proxies package, so asserting the package
|
||||
// is absent from the whole repo closes the hazard at the source rather than at one
|
||||
// registration. (Mirrors Csproj_ShouldNotHaveForbiddenPackageReferences in
|
||||
// Commons.Tests/ArchitecturalConstraintTests.)
|
||||
var repoRoot = GetRepositoryRoot();
|
||||
var projectFiles = Directory
|
||||
.GetFiles(repoRoot, "*.csproj", SearchOption.AllDirectories)
|
||||
.Concat(Directory.GetFiles(repoRoot, "Directory.Packages.props", SearchOption.AllDirectories))
|
||||
.Where(f => !f.Contains($"{Path.DirectorySeparatorChar}obj{Path.DirectorySeparatorChar}",
|
||||
StringComparison.Ordinal)
|
||||
&& !f.Contains($"{Path.DirectorySeparatorChar}bin{Path.DirectorySeparatorChar}",
|
||||
StringComparison.Ordinal))
|
||||
.ToList();
|
||||
|
||||
Assert.NotEmpty(projectFiles);
|
||||
foreach (var file in projectFiles)
|
||||
{
|
||||
Assert.DoesNotContain(
|
||||
"EntityFrameworkCore.Proxies",
|
||||
File.ReadAllText(file),
|
||||
StringComparison.Ordinal);
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>Walks up from the test output directory to the repository root.</summary>
|
||||
private static string GetRepositoryRoot()
|
||||
{
|
||||
var dir = new DirectoryInfo(AppContext.BaseDirectory);
|
||||
while (dir != null)
|
||||
{
|
||||
if (File.Exists(Path.Combine(dir.FullName, "ZB.MOM.WW.ScadaBridge.slnx")))
|
||||
{
|
||||
return dir.FullName;
|
||||
}
|
||||
|
||||
dir = dir.Parent;
|
||||
}
|
||||
|
||||
throw new InvalidOperationException("Could not locate the repository root (ZB.MOM.WW.ScadaBridge.slnx).");
|
||||
}
|
||||
|
||||
// The no-arg overload is [Obsolete(error: true)], so it cannot be referenced directly
|
||||
// from source — that is the compile-time guard. Invoke it via reflection to verify the
|
||||
// runtime defence-in-depth behaviour.
|
||||
|
||||
+64
@@ -252,6 +252,70 @@ public sealed class FleetWideUnlockThrottleTests : IDisposable
|
||||
Assert.Equal(2, await ctx.BundleUnlockAttempts.AsNoTracking().CountAsync());
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task ScheduledPrune_ClearsAgedRows_WithNoUnlockTrafficAtAll()
|
||||
{
|
||||
// Register row 65: the prune used to ride the unlock path ONLY, so a cluster that
|
||||
// stopped importing entirely left its last rows in place indefinitely. The
|
||||
// periodic sweep now drives PruneAsync directly — no second unlock attempt is made
|
||||
// anywhere in this test, which is the whole point.
|
||||
var clock = new TestClock(new DateTimeOffset(2026, 6, 1, 12, 0, 0, TimeSpan.Zero));
|
||||
var window = new SqlBundleUnlockAttemptWindow(
|
||||
_provider.GetRequiredService<IServiceScopeFactory>(), clock);
|
||||
|
||||
Assert.True(await window.TryRegisterAttemptAsync(
|
||||
"10.0.0.1", 10, BundleUnlockRateLimiter.DefaultWindow));
|
||||
|
||||
// Past both the 2-hour retention horizon and the 15-minute prune cadence.
|
||||
clock.Advance(TimeSpan.FromHours(3));
|
||||
await window.PruneAsync(BundleUnlockRateLimiter.DefaultWindow);
|
||||
|
||||
using var scope = _provider.CreateScope();
|
||||
var ctx = scope.ServiceProvider.GetRequiredService<ScadaBridgeDbContext>();
|
||||
Assert.Empty(await ctx.BundleUnlockAttempts.AsNoTracking().ToListAsync());
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task ScheduledPrune_RespectsTheRetentionHorizonAndTheCadenceClaim()
|
||||
{
|
||||
// Two negative controls in one: a row inside the retention horizon survives, and
|
||||
// the shared cadence claim means a burst of ticks cannot prune more often than
|
||||
// PruneMinInterval (so ticking on the 1-minute session sweep is cheap).
|
||||
var clock = new TestClock(new DateTimeOffset(2026, 6, 1, 12, 0, 0, TimeSpan.Zero));
|
||||
var window = new SqlBundleUnlockAttemptWindow(
|
||||
_provider.GetRequiredService<IServiceScopeFactory>(), clock);
|
||||
|
||||
Assert.True(await window.TryRegisterAttemptAsync(
|
||||
"10.0.0.1", 10, BundleUnlockRateLimiter.DefaultWindow));
|
||||
|
||||
// Recent row, past the cadence: pruning runs but must delete nothing.
|
||||
clock.Advance(TimeSpan.FromMinutes(30));
|
||||
for (var i = 0; i < 5; i++)
|
||||
{
|
||||
await window.PruneAsync(BundleUnlockRateLimiter.DefaultWindow);
|
||||
}
|
||||
|
||||
using var scope = _provider.CreateScope();
|
||||
var ctx = scope.ServiceProvider.GetRequiredService<ScadaBridgeDbContext>();
|
||||
Assert.Equal(1, await ctx.BundleUnlockAttempts.AsNoTracking().CountAsync());
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task ScheduledPrune_DoesNotThrow_WhenTheStoreIsUnavailable()
|
||||
{
|
||||
// The sweep that drives this has no caller to report to, so PruneAsync's contract
|
||||
// is "never throw" — a database blip must not kill the background service loop.
|
||||
var services = new ServiceCollection();
|
||||
services.AddScoped<IBundleUnlockAttemptRepository>(
|
||||
_ => throw new InvalidOperationException("database unavailable"));
|
||||
using var broken = services.BuildServiceProvider();
|
||||
|
||||
var window = new SqlBundleUnlockAttemptWindow(
|
||||
broken.GetRequiredService<IServiceScopeFactory>(), TimeProvider.System);
|
||||
|
||||
await window.PruneAsync(BundleUnlockRateLimiter.DefaultWindow);
|
||||
}
|
||||
|
||||
/// <summary>Minimal mutable-clock <see cref="TimeProvider"/> for deterministic windows.</summary>
|
||||
private sealed class TestClock : TimeProvider
|
||||
{
|
||||
|
||||
+21
-3
@@ -219,7 +219,7 @@ public sealed class BundleSessionObservabilityTests
|
||||
// against this test's own store (register row 74) — see ReadGaugeDelta.
|
||||
var clock = new TestClock(DateTimeOffset.UtcNow);
|
||||
var store = new BundleSessionStore(Options.Create(new TransportOptions()), clock, NullLogger<BundleSessionStore>.Instance);
|
||||
var service = new BundleSessionEvictionService(store, NullLogger<BundleSessionEvictionService>.Instance);
|
||||
var service = new BundleSessionEvictionService(store, InertUnlockWindow.Instance, NullLogger<BundleSessionEvictionService>.Instance);
|
||||
|
||||
await service.StartAsync(CancellationToken.None);
|
||||
try
|
||||
@@ -259,8 +259,8 @@ public sealed class BundleSessionObservabilityTests
|
||||
var clock = new TestClock(DateTimeOffset.UtcNow);
|
||||
var oldStore = new BundleSessionStore(Options.Create(new TransportOptions()), clock, NullLogger<BundleSessionStore>.Instance);
|
||||
var newStore = new BundleSessionStore(Options.Create(new TransportOptions()), clock, NullLogger<BundleSessionStore>.Instance);
|
||||
var oldService = new BundleSessionEvictionService(oldStore, NullLogger<BundleSessionEvictionService>.Instance);
|
||||
var newService = new BundleSessionEvictionService(newStore, NullLogger<BundleSessionEvictionService>.Instance);
|
||||
var oldService = new BundleSessionEvictionService(oldStore, InertUnlockWindow.Instance, NullLogger<BundleSessionEvictionService>.Instance);
|
||||
var newService = new BundleSessionEvictionService(newStore, InertUnlockWindow.Instance, NullLogger<BundleSessionEvictionService>.Instance);
|
||||
|
||||
await oldService.StartAsync(CancellationToken.None);
|
||||
await newService.StartAsync(CancellationToken.None);
|
||||
@@ -423,6 +423,24 @@ public sealed class BundleSessionObservabilityTests
|
||||
public void Advance(TimeSpan by) => _now = _now.Add(by);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Do-nothing <see cref="IBundleUnlockAttemptWindow"/> for the eviction-service tests.
|
||||
/// The service also ticks the unlock-attempt retention prune (register row 65), but
|
||||
/// these tests exercise only its gauge lifecycle — never <c>ExecuteAsync</c> — so the
|
||||
/// window is present purely to satisfy the constructor. The prune tick itself is
|
||||
/// covered by <c>FleetWideUnlockThrottleTests</c> against the real SQL-backed window.
|
||||
/// </summary>
|
||||
private sealed class InertUnlockWindow : IBundleUnlockAttemptWindow
|
||||
{
|
||||
public static readonly InertUnlockWindow Instance = new();
|
||||
|
||||
public Task<bool> TryRegisterAttemptAsync(
|
||||
string clientKey, int maxAttemptsPerWindow, TimeSpan window, CancellationToken ct = default)
|
||||
=> Task.FromResult(true);
|
||||
|
||||
public Task PruneAsync(TimeSpan window, CancellationToken ct = default) => Task.CompletedTask;
|
||||
}
|
||||
|
||||
/// <summary>Captures formatted log entries so the cap-rejection Warning can be asserted.</summary>
|
||||
private sealed class CapturingLogger : ILogger<BundleSessionStore>
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user