fix(host,auditlog): close Host.Tests env-var race + harden hosted-service shutdown (#15) #19
@@ -41,7 +41,7 @@ Repos cloned as sibling directories under `~/Desktop/` — referenced for contex
|
||||
- `~/Desktop/MxAccessGateway` — MxAccess Gateway (`https://gitea.dohertylan.com/dohertj2/mxaccessgw`).
|
||||
- `~/Desktop/OtOpcUa` — OtOpcUa (`https://gitea.dohertylan.com/dohertj2/lmxopcua`).
|
||||
|
||||
Other peers in the `scadaproj` family (see `scadaproj/CLAUDE.md` for details): `~/Desktop/HistorianGateway` (`historiangw`) — AVEVA Historian gRPC sidecar, OtOpcUa's sole historian backend; its SDK source `~/Desktop/histsdk` (`histsdk`); and `~/Desktop/scadaproj` (`scadaproj`) itself — shared `ZB.MOM.WW.*` libs + the dev/test GLAuth on `10.100.0.35:3893`.
|
||||
Other peers in the `scadaproj` family (see `scadaproj/CLAUDE.md` for details): `~/Desktop/HistorianGateway` (`historiangw`) — AVEVA Historian gRPC sidecar, OtOpcUa's sole historian backend; its historian SDK (`AVEVA.Historian.Client`) is now owned inside that repo at `HistorianGateway/histsdk/` (the separate `~/Desktop/histsdk` repo was folded in with history); and `~/Desktop/scadaproj` (`scadaproj`) itself — shared `ZB.MOM.WW.*` libs + the dev/test GLAuth on `10.100.0.35:3893`.
|
||||
|
||||
## Document Conventions
|
||||
|
||||
|
||||
@@ -0,0 +1,116 @@
|
||||
# Documentation Analysis Report
|
||||
|
||||
Files Scanned: 793
|
||||
Files With Issues: 10
|
||||
Total Issues: 11
|
||||
|
||||
## Issues
|
||||
|
||||
FILE: /Users/dohertj2/Desktop/ScadaBridge/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/BrowserTime.cs
|
||||
LINE: 26
|
||||
CATEGORY: TaskReferenceInComment
|
||||
SEVERITY: Warning
|
||||
MEMBER: Comment
|
||||
SIGNATURE: UTC-5
|
||||
MESSAGE: Comment contains 'UTC-5', which looks like a task/issue tracking identifier; tracking IDs should not appear in code documentation.
|
||||
|
||||
---
|
||||
|
||||
FILE: /Users/dohertj2/Desktop/ScadaBridge/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Design/TransportImport.razor.cs
|
||||
LINE: 225
|
||||
CATEGORY: TaskReferenceInComment
|
||||
SEVERITY: Warning
|
||||
MEMBER: Comment
|
||||
SIGNATURE: Step-2
|
||||
MESSAGE: Comment contains 'Step-2', which looks like a task/issue tracking identifier; tracking IDs should not appear in code documentation.
|
||||
|
||||
---
|
||||
|
||||
FILE: /Users/dohertj2/Desktop/ScadaBridge/src/ZB.MOM.WW.ScadaBridge.CentralUI/Services/ConnectionHealthQueryService.cs
|
||||
LINE: 18
|
||||
CATEGORY: TaskReferenceInComment
|
||||
SEVERITY: Warning
|
||||
MEMBER: Comment
|
||||
SIGNATURE: PLC-1
|
||||
MESSAGE: Comment contains 'PLC-1', which looks like a task/issue tracking identifier; tracking IDs should not appear in code documentation.
|
||||
|
||||
---
|
||||
|
||||
FILE: /Users/dohertj2/Desktop/ScadaBridge/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/AuditExportHelpers.cs
|
||||
LINE: 156
|
||||
CATEGORY: TaskReferenceInComment
|
||||
SEVERITY: Warning
|
||||
MEMBER: Comment
|
||||
SIGNATURE: non-403
|
||||
MESSAGE: Comment contains 'non-403', which looks like a task/issue tracking identifier; tracking IDs should not appear in code documentation.
|
||||
|
||||
---
|
||||
|
||||
FILE: /Users/dohertj2/Desktop/ScadaBridge/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/BundleCommands.cs
|
||||
LINE: 407
|
||||
CATEGORY: TaskReferenceInComment
|
||||
SEVERITY: Warning
|
||||
MEMBER: Comment
|
||||
SIGNATURE: non-403
|
||||
MESSAGE: Comment contains 'non-403', which looks like a task/issue tracking identifier; tracking IDs should not appear in code documentation.
|
||||
|
||||
---
|
||||
|
||||
FILE: /Users/dohertj2/Desktop/ScadaBridge/src/ZB.MOM.WW.ScadaBridge.Communication/Grpc/SiteStreamGrpcServer.cs
|
||||
LINE: 43
|
||||
CATEGORY: TaskReferenceInComment
|
||||
SEVERITY: Warning
|
||||
MEMBER: Comment
|
||||
SIGNATURE: sub-100
|
||||
MESSAGE: Comment contains 'sub-100', which looks like a task/issue tracking identifier; tracking IDs should not appear in code documentation.
|
||||
|
||||
---
|
||||
|
||||
FILE: /Users/dohertj2/Desktop/ScadaBridge/src/ZB.MOM.WW.ScadaBridge.HealthMonitoring/SiteHealthState.cs
|
||||
LINE: 29
|
||||
CATEGORY: TaskReferenceInComment
|
||||
SEVERITY: Warning
|
||||
MEMBER: Comment
|
||||
SIGNATURE: year-0001
|
||||
MESSAGE: Comment contains 'year-0001', which looks like a task/issue tracking identifier; tracking IDs should not appear in code documentation.
|
||||
|
||||
---
|
||||
|
||||
FILE: /Users/dohertj2/Desktop/ScadaBridge/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs
|
||||
LINE: 2034
|
||||
CATEGORY: TrackingReferenceInComment
|
||||
SEVERITY: Warning
|
||||
MEMBER: Comment
|
||||
SIGNATURE: M365
|
||||
MESSAGE: Comment contains 'M365', which looks like a project tracking reference; bookkeeping references should not appear in code documentation.
|
||||
|
||||
---
|
||||
|
||||
FILE: /Users/dohertj2/Desktop/ScadaBridge/src/ZB.MOM.WW.ScadaBridge.NotificationOutbox/Delivery/EmailNotificationDeliveryAdapter.cs
|
||||
LINE: 206
|
||||
CATEGORY: TrackingReferenceInComment
|
||||
SEVERITY: Warning
|
||||
MEMBER: Comment
|
||||
SIGNATURE: M365
|
||||
MESSAGE: Comment contains 'M365', which looks like a project tracking reference; bookkeeping references should not appear in code documentation.
|
||||
|
||||
---
|
||||
|
||||
FILE: /Users/dohertj2/Desktop/ScadaBridge/src/ZB.MOM.WW.ScadaBridge.NotificationService/MailKitSmtpClientWrapper.cs
|
||||
LINE: 9
|
||||
CATEGORY: TrackingReferenceInComment
|
||||
SEVERITY: Warning
|
||||
MEMBER: Comment
|
||||
SIGNATURE: M365
|
||||
MESSAGE: Comment contains 'M365', which looks like a project tracking reference; bookkeeping references should not appear in code documentation.
|
||||
|
||||
---
|
||||
|
||||
FILE: /Users/dohertj2/Desktop/ScadaBridge/src/ZB.MOM.WW.ScadaBridge.NotificationService/MailKitSmtpClientWrapper.cs
|
||||
LINE: 113
|
||||
CATEGORY: TrackingReferenceInComment
|
||||
SEVERITY: Warning
|
||||
MEMBER: Comment
|
||||
SIGNATURE: M365
|
||||
MESSAGE: Comment contains 'M365', which looks like a project tracking reference; bookkeeping references should not appear in code documentation.
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
+44
@@ -0,0 +1,44 @@
|
||||
# Remaining Deferred Work
|
||||
|
||||
Source: `docs/plans/2026-07-08-deferred-work-register.md` (snapshot 2026-07-10).
|
||||
Everything in the register's "Fix-now" table is already landed via the archreview
|
||||
plans; what's left are the intentional deferrals below.
|
||||
|
||||
## Product / roadmap-locked (revisit needs a decision or a trigger event)
|
||||
|
||||
| # | Item | Why deferred | Revisit trigger |
|
||||
|---|------|--------------|-----------------|
|
||||
| 8 | Hash-chain tamper evidence (CLI `verify-chain` is a no-op stub) | v1.x by locked decision — append-only DB roles are the current control | A compliance requirement for cryptographic tamper evidence |
|
||||
| 9 | Parquet audit archival (endpoint returns `501`) | v1.x — the `501` + CLI messaging are honest, not broken | AuditLog partition volume nears the retention ceiling |
|
||||
| 11 | Central-persisted OPC UA cert-trust audit | Broadcast-to-both-nodes already covers HA | A governance/audit requirement for trust decisions |
|
||||
| 19 | Bundle signing / cluster-to-cluster pull / differential bundles | v1 manifest hash + AES-GCM held sufficient | A non-repudiation requirement across orgs |
|
||||
| 17 | Unified notifications + site-calls outbox page | Explicit M9 decision to keep two pages | Operator confusion reports |
|
||||
| 18 | Folder drag-drop | Permanently closed — menu reorder shipped instead | — (closed) |
|
||||
|
||||
## Scale / YAGNI (deferred until load justifies it)
|
||||
|
||||
| # | Item | Why deferred | Revisit trigger | Status |
|
||||
|---|------|--------------|-----------------|--------|
|
||||
| 10 | Aggregated live alarm stream for Alarm Summary | Snapshot fan-out is acceptable at current instance counts | Latency complaints or >~50 instances/site | ✅ **SHIPPED + MERGED to main 2026-07-10** (`8c888f13`, plan `docs/plans/2026-07-10-aggregated-live-alarm-stream-plan.md`, T1–T8). Transient per-site in-memory live cache (`ISiteAlarmLiveCache`/`SiteAlarmAggregatorActor`) seeded by snapshot fan-out + additive `SubscribeSite` alarm-only gRPC stream; live-cache-driven Alarm Summary with 15s poll fallback; `[PERM]` no-central-store honored (code-reviewer-confirmed); validated options + telemetry; end-to-end trace. Register row moved to Resolved. |
|
||||
| 22 | KPI history hourly rollups | 90-day retention already bounds the table | `KpiSample` query latency on dashboards | ✅ **SHIPPED + MERGED to main 2026-07-10** (`8c888f13`, plan `docs/plans/2026-07-10-kpi-history-hourly-rollups-plan.md`, T1–T8). Separate `KpiRollupHourly` table (migration `20260710153953`), recorder hourly fold w/ failover-safe lookback re-fold + idempotent upsert, per-metric gauge-vs-rate aggregation, range-threshold query routing (`RollupThresholdHours` 168h), longer rollup retention (365 ≥ 90) + dual purge, one-shot backfill, and 30 d/90 d window buttons. Register row moved to Resolved. |
|
||||
|
||||
## Low-priority polish (near-complete, small remainder)
|
||||
|
||||
| # | Item | Why deferred | Revisit trigger |
|
||||
|---|------|--------------|-----------------|
|
||||
| 12 | Native-alarm-source-override CSV import — Central UI upload button only | CLI + Management API + parser shipped 2026-07-10; the Blazor upload affordance is the only piece left, and it's pure polish (needs a live Blazor smoke) | First request to bulk-import native sources from the UI instead of the CLI |
|
||||
|
||||
## New deferrals from review 08 (engineering debt, no defect)
|
||||
|
||||
| Item | Why deferred | Revisit trigger |
|
||||
|------|--------------|-----------------|
|
||||
| Communication → HealthMonitoring layering inversion | Moving the interface + `SiteHealthState` to Commons ripples across 5 projects for a cosmetic inversion | Next breaking change to `ICentralHealthAggregator` |
|
||||
| Reference docs for ScriptAnalysis, KpiHistory, DelmiaNotifier | Full StyleGuide-conformant docs are substantial; README claim was scoped instead (PLAN-08 T10) | Next doc-writing session touching those components |
|
||||
| Test-coverage backfill: SiteCallAudit.Tests, DeploymentManager.Tests | No defect identified; coverage partly lives in ManagementService/Host/Integration suites | First regression escaping either component |
|
||||
| Failover-timing + broader perf envelope (S&F drain rate, per-subscriber backpressure) | Needs the PLAN-01 two-node rig; placeholder harness already shipped (PLAN-08 T8) | PLAN-01 rig landing |
|
||||
|
||||
---
|
||||
|
||||
Summary: 12 open deferrals (13th, folder drag-drop #18, is permanently closed).
|
||||
None are currently actionable without a triggering event or product decision —
|
||||
except row #12's UI upload button, whose CLI/API/parser core already shipped.
|
||||
@@ -86,8 +86,16 @@ public sealed class AuditLogPartitionMaintenanceService : IHostedService, IDispo
|
||||
// Linked CTS lets StopAsync's cancellation AND the host's shutdown
|
||||
// token both terminate the loop; either side firing aborts the
|
||||
// pending Task.Delay.
|
||||
_cts = CancellationTokenSource.CreateLinkedTokenSource(ct);
|
||||
_loop = Task.Run(() => RunLoopAsync(_cts.Token));
|
||||
var cts = CancellationTokenSource.CreateLinkedTokenSource(ct);
|
||||
_cts = cts;
|
||||
|
||||
// Read Token here, on the caller's thread, rather than inside the lambda:
|
||||
// the lambda runs whenever the thread pool gets to it, so a Dispose landing
|
||||
// first would make the deferred _cts.Token read throw ObjectDisposedException
|
||||
// and fault the loop task — which StopAsync hands to the host to await. The
|
||||
// token struct stays usable once captured.
|
||||
var token = cts.Token;
|
||||
_loop = Task.Run(() => RunLoopAsync(token), CancellationToken.None);
|
||||
return Task.CompletedTask;
|
||||
}
|
||||
|
||||
@@ -148,15 +156,43 @@ public sealed class AuditLogPartitionMaintenanceService : IHostedService, IDispo
|
||||
/// <returns>The background loop task, or a completed task if the loop was never started.</returns>
|
||||
public Task StopAsync(CancellationToken ct)
|
||||
{
|
||||
_cts?.Cancel();
|
||||
try
|
||||
{
|
||||
_cts?.Cancel();
|
||||
}
|
||||
catch (ObjectDisposedException)
|
||||
{
|
||||
// Stop-after-Dispose is a legal ordering — WebApplicationFactory's
|
||||
// teardown disposes the container before driving StopAsync. Dispose
|
||||
// already cancelled the loop, so there is nothing left to signal, and
|
||||
// letting this escape would abort the host's whole shutdown sequence.
|
||||
}
|
||||
|
||||
return _loop ?? Task.CompletedTask;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Disposes the internal <see cref="CancellationTokenSource"/> used to stop the maintenance loop.
|
||||
/// Cancels and disposes the internal <see cref="CancellationTokenSource"/> used to
|
||||
/// stop the maintenance loop.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// Cancels before disposing so the loop is always signalled, even when the host
|
||||
/// disposes the container without having driven <see cref="StopAsync"/> first.
|
||||
/// It also keeps the loop's pending <c>Task.Delay(interval, token)</c> safe: an
|
||||
/// already-cancelled token makes Delay complete as cancelled rather than register
|
||||
/// a callback against a dead source and throw.
|
||||
/// </remarks>
|
||||
public void Dispose()
|
||||
{
|
||||
try
|
||||
{
|
||||
_cts?.Cancel();
|
||||
}
|
||||
catch (ObjectDisposedException)
|
||||
{
|
||||
// Already disposed — Dispose is idempotent.
|
||||
}
|
||||
|
||||
_cts?.Dispose();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -102,30 +102,46 @@ public sealed class SiteAuditBacklogReporter : IHostedService, IDisposable
|
||||
// Linked CTS lets StopAsync's cancellation AND the host's shutdown
|
||||
// token both terminate the loop; either side firing aborts the
|
||||
// pending Task.Delay.
|
||||
_cts = CancellationTokenSource.CreateLinkedTokenSource(ct);
|
||||
_loop = Task.Run(() => RunLoopAsync(_cts.Token));
|
||||
var cts = CancellationTokenSource.CreateLinkedTokenSource(ct);
|
||||
_cts = cts;
|
||||
|
||||
// Read Token on the caller's thread, not inside the lambda: the lambda runs
|
||||
// whenever the thread pool gets to it, so a Dispose landing first would make
|
||||
// the deferred _cts.Token read throw and fault the loop task the host awaits.
|
||||
var token = cts.Token;
|
||||
_loop = Task.Run(() => RunLoopAsync(token), CancellationToken.None);
|
||||
return Task.CompletedTask;
|
||||
}
|
||||
|
||||
private async Task RunLoopAsync(CancellationToken ct)
|
||||
{
|
||||
// First tick runs immediately so the very first health report after
|
||||
// process start carries a real backlog snapshot — without this the
|
||||
// dashboard would show null for the first 30 s after a deploy.
|
||||
await SafeProbeAsync(ct).ConfigureAwait(false);
|
||||
|
||||
while (!ct.IsCancellationRequested)
|
||||
try
|
||||
{
|
||||
try
|
||||
{
|
||||
await Task.Delay(_refreshInterval, ct).ConfigureAwait(false);
|
||||
}
|
||||
catch (OperationCanceledException)
|
||||
{
|
||||
break;
|
||||
}
|
||||
|
||||
// First tick runs immediately so the very first health report after
|
||||
// process start carries a real backlog snapshot — without this the
|
||||
// dashboard would show null for the first 30 s after a deploy.
|
||||
await SafeProbeAsync(ct).ConfigureAwait(false);
|
||||
|
||||
while (!ct.IsCancellationRequested)
|
||||
{
|
||||
try
|
||||
{
|
||||
await Task.Delay(_refreshInterval, ct).ConfigureAwait(false);
|
||||
}
|
||||
catch (OperationCanceledException)
|
||||
{
|
||||
break;
|
||||
}
|
||||
|
||||
await SafeProbeAsync(ct).ConfigureAwait(false);
|
||||
}
|
||||
}
|
||||
catch (OperationCanceledException)
|
||||
{
|
||||
// Shutdown landed mid-probe: SafeProbeAsync rethrows OCE by design so the probe
|
||||
// aborts promptly, but the loop task must complete CLEANLY — StopAsync hands
|
||||
// _loop straight to the host, and a canceled task there is shutdown-log noise
|
||||
// (arch-review 04 round 2, R7). Cancellation here IS the clean exit.
|
||||
}
|
||||
}
|
||||
|
||||
@@ -155,13 +171,35 @@ public sealed class SiteAuditBacklogReporter : IHostedService, IDisposable
|
||||
/// <returns>A task that represents the asynchronous operation.</returns>
|
||||
public Task StopAsync(CancellationToken ct)
|
||||
{
|
||||
_cts?.Cancel();
|
||||
try
|
||||
{
|
||||
_cts?.Cancel();
|
||||
}
|
||||
catch (ObjectDisposedException)
|
||||
{
|
||||
// Stop-after-Dispose is a legal ordering; Dispose already cancelled the
|
||||
// loop. Letting this escape would abort the host's shutdown sequence.
|
||||
}
|
||||
|
||||
return _loop ?? Task.CompletedTask;
|
||||
}
|
||||
|
||||
/// <summary>Releases the internal <see cref="CancellationTokenSource"/> used to stop the polling loop.</summary>
|
||||
public void Dispose()
|
||||
{
|
||||
// Cancel before disposing so the loop is always signalled even when the host
|
||||
// disposes the container without having driven StopAsync first, and so the
|
||||
// loop's pending Task.Delay(interval, token) sees an already-cancelled token
|
||||
// rather than registering against a dead source.
|
||||
try
|
||||
{
|
||||
_cts?.Cancel();
|
||||
}
|
||||
catch (ObjectDisposedException)
|
||||
{
|
||||
// Already disposed — Dispose is idempotent.
|
||||
}
|
||||
|
||||
_cts?.Dispose();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -54,8 +54,14 @@ public sealed class SiteAuditRetentionService : IHostedService, IDisposable
|
||||
public Task StartAsync(CancellationToken ct)
|
||||
{
|
||||
// Linked CTS so both StopAsync and the host shutdown token abort the loop.
|
||||
_cts = CancellationTokenSource.CreateLinkedTokenSource(ct);
|
||||
_loop = Task.Run(() => RunLoopAsync(_cts.Token));
|
||||
var cts = CancellationTokenSource.CreateLinkedTokenSource(ct);
|
||||
_cts = cts;
|
||||
|
||||
// Read Token on the caller's thread, not inside the lambda: the lambda runs
|
||||
// whenever the thread pool gets to it, so a Dispose landing first would make
|
||||
// the deferred _cts.Token read throw and fault the loop task the host awaits.
|
||||
var token = cts.Token;
|
||||
_loop = Task.Run(() => RunLoopAsync(token), CancellationToken.None);
|
||||
return Task.CompletedTask;
|
||||
}
|
||||
|
||||
@@ -131,13 +137,35 @@ public sealed class SiteAuditRetentionService : IHostedService, IDisposable
|
||||
/// <returns>A task that represents the asynchronous operation.</returns>
|
||||
public Task StopAsync(CancellationToken ct)
|
||||
{
|
||||
_cts?.Cancel();
|
||||
try
|
||||
{
|
||||
_cts?.Cancel();
|
||||
}
|
||||
catch (ObjectDisposedException)
|
||||
{
|
||||
// Stop-after-Dispose is a legal ordering; Dispose already cancelled the
|
||||
// loop. Letting this escape would abort the host's shutdown sequence.
|
||||
}
|
||||
|
||||
return _loop ?? Task.CompletedTask;
|
||||
}
|
||||
|
||||
/// <summary>Releases the internal <see cref="CancellationTokenSource"/>.</summary>
|
||||
public void Dispose()
|
||||
{
|
||||
// Cancel before disposing so the loop is always signalled even when the host
|
||||
// disposes the container without having driven StopAsync first, and so the
|
||||
// loop's pending Task.Delay(interval, token) sees an already-cancelled token
|
||||
// rather than registering against a dead source.
|
||||
try
|
||||
{
|
||||
_cts?.Cancel();
|
||||
}
|
||||
catch (ObjectDisposedException)
|
||||
{
|
||||
// Already disposed — Dispose is idempotent.
|
||||
}
|
||||
|
||||
_cts?.Dispose();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -85,8 +85,14 @@ public sealed class SiteEventLogFailureCountReporter : IHostedService, IDisposab
|
||||
// Linked CTS lets StopAsync's cancellation AND the host's shutdown
|
||||
// token both terminate the loop; either side firing aborts the
|
||||
// pending Task.Delay.
|
||||
_cts = CancellationTokenSource.CreateLinkedTokenSource(ct);
|
||||
_loop = Task.Run(() => RunLoopAsync(_cts.Token));
|
||||
var cts = CancellationTokenSource.CreateLinkedTokenSource(ct);
|
||||
_cts = cts;
|
||||
|
||||
// Read Token on the caller's thread, not inside the lambda: the lambda runs
|
||||
// whenever the thread pool gets to it, so a Dispose landing first would make
|
||||
// the deferred _cts.Token read throw and fault the loop task the host awaits.
|
||||
var token = cts.Token;
|
||||
_loop = Task.Run(() => RunLoopAsync(token), CancellationToken.None);
|
||||
return Task.CompletedTask;
|
||||
}
|
||||
|
||||
@@ -134,13 +140,35 @@ public sealed class SiteEventLogFailureCountReporter : IHostedService, IDisposab
|
||||
/// <returns>A task that represents the asynchronous operation.</returns>
|
||||
public Task StopAsync(CancellationToken ct)
|
||||
{
|
||||
_cts?.Cancel();
|
||||
try
|
||||
{
|
||||
_cts?.Cancel();
|
||||
}
|
||||
catch (ObjectDisposedException)
|
||||
{
|
||||
// Stop-after-Dispose is a legal ordering; Dispose already cancelled the
|
||||
// loop. Letting this escape would abort the host's shutdown sequence.
|
||||
}
|
||||
|
||||
return _loop ?? Task.CompletedTask;
|
||||
}
|
||||
|
||||
/// <summary>Releases the internal <see cref="CancellationTokenSource"/> used to stop the polling loop.</summary>
|
||||
public void Dispose()
|
||||
{
|
||||
// Cancel before disposing so the loop is always signalled even when the host
|
||||
// disposes the container without having driven StopAsync first, and so the
|
||||
// loop's pending Task.Delay(interval, token) sees an already-cancelled token
|
||||
// rather than registering against a dead source.
|
||||
try
|
||||
{
|
||||
_cts?.Cancel();
|
||||
}
|
||||
catch (ObjectDisposedException)
|
||||
{
|
||||
// Already disposed — Dispose is idempotent.
|
||||
}
|
||||
|
||||
_cts?.Dispose();
|
||||
}
|
||||
}
|
||||
|
||||
+36
@@ -151,4 +151,40 @@ public class AuditLogPartitionMaintenanceServiceTests
|
||||
Assert.Equal(LogLevel.Error, errorEntry.Level);
|
||||
Assert.Equal(1, maintenance.EnsureCallCount);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task StopAsync_AfterDispose_DoesNotThrow()
|
||||
{
|
||||
// Regression (Gitea #15 follow-up): Dispose tears down the CTS that
|
||||
// StopAsync cancels, and the host does not guarantee that every
|
||||
// IHostedService.StopAsync is driven before the DI container is
|
||||
// disposed — WebApplicationFactory's teardown reaches Dispose first and
|
||||
// then calls StopAsync. Cancel() on a disposed CTS throws
|
||||
// ObjectDisposedException, and letting it escape aborts the host's whole
|
||||
// shutdown sequence (in Host.Tests it stranded the fixture's env-var
|
||||
// restore, contaminating every later test in the run). Stop-after-Dispose
|
||||
// must therefore be a no-op, not a throw.
|
||||
var opts = Options.Create(new AuditLogPartitionMaintenanceOptions
|
||||
{
|
||||
IntervalSeconds = 60,
|
||||
LookaheadMonths = 1,
|
||||
});
|
||||
var maintenance = new RecordingMaintenance();
|
||||
var sp = BuildProvider(maintenance);
|
||||
|
||||
var svc = new AuditLogPartitionMaintenanceService(
|
||||
sp.GetRequiredService<IServiceScopeFactory>(),
|
||||
opts,
|
||||
NullLogger<AuditLogPartitionMaintenanceService>.Instance);
|
||||
|
||||
await svc.StartAsync(CancellationToken.None);
|
||||
|
||||
svc.Dispose();
|
||||
|
||||
// The assertion is the absence of ObjectDisposedException.
|
||||
await svc.StopAsync(CancellationToken.None);
|
||||
|
||||
// Dispose is also idempotent — the host may reach it twice on the same path.
|
||||
svc.Dispose();
|
||||
}
|
||||
}
|
||||
|
||||
+55
@@ -1,8 +1,10 @@
|
||||
using System.Diagnostics;
|
||||
using Microsoft.Extensions.Logging.Abstractions;
|
||||
using Microsoft.Extensions.Options;
|
||||
using NSubstitute;
|
||||
using ZB.MOM.WW.ScadaBridge.AuditLog.Site;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Types;
|
||||
using ZB.MOM.WW.ScadaBridge.HealthMonitoring;
|
||||
|
||||
namespace ZB.MOM.WW.ScadaBridge.AuditLog.Tests.Site;
|
||||
@@ -23,6 +25,59 @@ public class SiteAuditBacklogReporterCadenceTests
|
||||
explicitInterval,
|
||||
options);
|
||||
|
||||
[Fact]
|
||||
public async Task StopAsync_WhileProbeInFlight_LoopCompletesCleanly()
|
||||
{
|
||||
// SafeProbeAsync rethrows OperationCanceledException by design so a shutdown
|
||||
// aborts the probe promptly — but RunLoopAsync must absorb it, because StopAsync
|
||||
// hands _loop straight to the host and a canceled task there throws out of
|
||||
// Host.StopAsync. Mirrors the guard SiteAuditRetentionService already carries
|
||||
// (arch-review 04 round 2, R7).
|
||||
var probeStarted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
|
||||
var queue = Substitute.For<ISiteAuditQueue>();
|
||||
queue.GetBacklogStatsAsync(Arg.Any<CancellationToken>())
|
||||
.Returns(ci => BlockUntilCancelledAsync(probeStarted, ci.Arg<CancellationToken>()));
|
||||
|
||||
var reporter = new SiteAuditBacklogReporter(
|
||||
queue,
|
||||
Substitute.For<ISiteHealthCollector>(),
|
||||
NullLogger<SiteAuditBacklogReporter>.Instance,
|
||||
TimeSpan.FromHours(1),
|
||||
null);
|
||||
|
||||
await reporter.StartAsync(CancellationToken.None);
|
||||
await probeStarted.Task; // the immediate first probe is now in flight
|
||||
|
||||
// The assertion is that awaiting the loop task does not throw.
|
||||
await reporter.StopAsync(CancellationToken.None);
|
||||
reporter.Dispose();
|
||||
}
|
||||
|
||||
private static async Task<SiteAuditBacklogSnapshot> BlockUntilCancelledAsync(
|
||||
TaskCompletionSource started, CancellationToken ct)
|
||||
{
|
||||
started.TrySetResult();
|
||||
await Task.Delay(Timeout.Infinite, ct);
|
||||
throw new UnreachableException("the delay above always throws on cancellation");
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task StopAsync_AfterDispose_DoesNotThrow()
|
||||
{
|
||||
// Regression (Gitea #15 follow-up): Dispose tears down the CTS StopAsync
|
||||
// cancels, and the host does not guarantee StopAsync is driven before the DI
|
||||
// container is disposed. Cancel() on a disposed CTS throws, and letting that
|
||||
// escape an IHostedService aborts the host's whole shutdown sequence.
|
||||
var reporter = Create(Options.Create(new SqliteAuditWriterOptions()), TimeSpan.FromHours(1));
|
||||
|
||||
await reporter.StartAsync(CancellationToken.None);
|
||||
reporter.Dispose();
|
||||
|
||||
// The assertion is the absence of ObjectDisposedException.
|
||||
await reporter.StopAsync(CancellationToken.None);
|
||||
reporter.Dispose();
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Cadence_ComesFromOptions_WhenConfigured()
|
||||
{
|
||||
|
||||
@@ -19,6 +19,30 @@ namespace ZB.MOM.WW.ScadaBridge.AuditLog.Tests.Site;
|
||||
/// </summary>
|
||||
public class SiteAuditRetentionServiceTests
|
||||
{
|
||||
[Fact]
|
||||
public async Task StopAsync_AfterDispose_DoesNotThrow()
|
||||
{
|
||||
// Regression (Gitea #15 follow-up): Dispose tears down the CTS StopAsync
|
||||
// cancels, and the host does not guarantee StopAsync is driven before the DI
|
||||
// container is disposed. Cancel() on a disposed CTS throws, and letting that
|
||||
// escape an IHostedService aborts the host's whole shutdown sequence.
|
||||
var queue = new RecordingSiteAuditQueue();
|
||||
var options = Options.Create(new SiteAuditRetentionOptions
|
||||
{
|
||||
RetentionDays = 7,
|
||||
PurgeIntervalOverride = TimeSpan.FromMilliseconds(50),
|
||||
InitialDelay = TimeSpan.Zero,
|
||||
});
|
||||
var svc = new SiteAuditRetentionService(queue, options, NullLogger<SiteAuditRetentionService>.Instance);
|
||||
|
||||
await svc.StartAsync(CancellationToken.None);
|
||||
svc.Dispose();
|
||||
|
||||
// The assertion is the absence of ObjectDisposedException.
|
||||
await svc.StopAsync(CancellationToken.None);
|
||||
svc.Dispose();
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Tick_Purges_With_RetentionCutoff()
|
||||
{
|
||||
|
||||
+21
@@ -10,6 +10,27 @@ namespace ZB.MOM.WW.ScadaBridge.HealthMonitoring.Tests;
|
||||
/// </summary>
|
||||
public class SiteEventLogFailureCountReporterTests
|
||||
{
|
||||
[Fact]
|
||||
public async Task StopAsync_AfterDispose_DoesNotThrow()
|
||||
{
|
||||
// Regression (Gitea #15 follow-up): Dispose tears down the CTS StopAsync
|
||||
// cancels, and the host does not guarantee StopAsync is driven before the DI
|
||||
// container is disposed. Cancel() on a disposed CTS throws, and letting that
|
||||
// escape an IHostedService aborts the host's whole shutdown sequence.
|
||||
var reporter = new SiteEventLogFailureCountReporter(
|
||||
failedWriteCountProvider: () => 0L,
|
||||
collector: new SiteHealthCollector(),
|
||||
logger: NullLogger<SiteEventLogFailureCountReporter>.Instance,
|
||||
refreshInterval: TimeSpan.FromHours(1));
|
||||
|
||||
await reporter.StartAsync(CancellationToken.None);
|
||||
reporter.Dispose();
|
||||
|
||||
// The assertion is the absence of ObjectDisposedException.
|
||||
await reporter.StopAsync(CancellationToken.None);
|
||||
reporter.Dispose();
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task StartAsync_ImmediatelyProbes_FailedWriteCount()
|
||||
{
|
||||
|
||||
@@ -10,14 +10,11 @@ using ZB.MOM.WW.ScadaBridge.Host.Actors;
|
||||
|
||||
namespace ZB.MOM.WW.ScadaBridge.Host.Tests;
|
||||
|
||||
[CollectionDefinition("ActorSystem")]
|
||||
public class ActorSystemCollection : ICollectionFixture<object> { }
|
||||
|
||||
/// <summary>
|
||||
/// Verifies that all expected Central-role actors are created at the correct paths
|
||||
/// when AkkaHostedService starts.
|
||||
/// </summary>
|
||||
[Collection("ActorSystem")]
|
||||
[Collection(HostBootCollection.Name)]
|
||||
public class CentralActorPathTests : IAsyncLifetime
|
||||
{
|
||||
private WebApplicationFactory<Program>? _factory;
|
||||
@@ -93,9 +90,20 @@ public class CentralActorPathTests : IAsyncLifetime
|
||||
|
||||
public async Task DisposeAsync()
|
||||
{
|
||||
_factory?.Dispose();
|
||||
Environment.SetEnvironmentVariable("DOTNET_ENVIRONMENT", _previousEnv);
|
||||
_dbEnv?.Dispose();
|
||||
// try/finally: a throwing host teardown must not strand the process-wide
|
||||
// environment variables. Without it, one failed _factory.Dispose() leaks
|
||||
// DOTNET_ENVIRONMENT=Central and the CentralDbTestEnvironment vars into
|
||||
// every later test in the run.
|
||||
try
|
||||
{
|
||||
_factory?.Dispose();
|
||||
}
|
||||
finally
|
||||
{
|
||||
Environment.SetEnvironmentVariable("DOTNET_ENVIRONMENT", _previousEnv);
|
||||
_dbEnv?.Dispose();
|
||||
}
|
||||
|
||||
await Task.CompletedTask;
|
||||
}
|
||||
|
||||
@@ -149,7 +157,7 @@ public class CentralActorPathTests : IAsyncLifetime
|
||||
/// Verifies that all expected Site-role actors are created at the correct paths
|
||||
/// when AkkaHostedService starts.
|
||||
/// </summary>
|
||||
[Collection("ActorSystem")]
|
||||
[Collection(HostBootCollection.Name)]
|
||||
public class SiteActorPathTests : IAsyncLifetime
|
||||
{
|
||||
private IHost? _host;
|
||||
|
||||
@@ -86,6 +86,7 @@ public class AkkaHostedServiceAuditWiringHoconTests
|
||||
/// <summary>
|
||||
/// Verifies Audit Log (#23) services land in the Central composition root.
|
||||
/// </summary>
|
||||
[Collection(HostBootCollection.Name)]
|
||||
public class CentralAuditWiringTests : IDisposable
|
||||
{
|
||||
private readonly WebApplicationFactory<Program> _factory;
|
||||
@@ -156,9 +157,17 @@ public class CentralAuditWiringTests : IDisposable
|
||||
|
||||
public void Dispose()
|
||||
{
|
||||
_factory.Dispose();
|
||||
Environment.SetEnvironmentVariable("DOTNET_ENVIRONMENT", _previousEnv);
|
||||
_dbEnv.Dispose();
|
||||
// try/finally: a throwing host teardown must not strand the process-wide
|
||||
// environment variables for the rest of the run.
|
||||
try
|
||||
{
|
||||
_factory.Dispose();
|
||||
}
|
||||
finally
|
||||
{
|
||||
Environment.SetEnvironmentVariable("DOTNET_ENVIRONMENT", _previousEnv);
|
||||
_dbEnv.Dispose();
|
||||
}
|
||||
}
|
||||
|
||||
[Fact]
|
||||
|
||||
@@ -26,6 +26,14 @@ namespace ZB.MOM.WW.ScadaBridge.Host.Tests;
|
||||
/// the tokens entirely — so tests that boot the real <c>Program</c> pipeline do
|
||||
/// not need a seeded secrets store. All vars are restored on Dispose so tests
|
||||
/// stay isolated.
|
||||
///
|
||||
/// Every var below is <b>process-wide</b>, and the reader — <c>Program</c>'s
|
||||
/// <c>AddEnvironmentVariables()</c> — runs at an unpredictable point during host boot.
|
||||
/// Two live instances therefore cannot be allowed to overlap: one's Dispose would
|
||||
/// restore/clear a var while the other's boot is still reading it, and the fail-closed
|
||||
/// secrets expander turns that into a <c>SecretNotFoundException</c>. Serialization via
|
||||
/// <see cref="HostBootCollection"/> is what enforces the invariant; the overlap check in
|
||||
/// the constructor makes a regression fail deterministically instead of intermittently.
|
||||
/// </summary>
|
||||
internal sealed class CentralDbTestEnvironment : IDisposable
|
||||
{
|
||||
@@ -64,8 +72,25 @@ internal sealed class CentralDbTestEnvironment : IDisposable
|
||||
private readonly string? _previousLdapPassword;
|
||||
private readonly string? _previousJwtSigningKey;
|
||||
|
||||
/// <summary>
|
||||
/// Number of live instances. Only ever 0 or 1 while every consuming fixture is a
|
||||
/// member of <see cref="HostBootCollection"/>; see the overlap check below.
|
||||
/// </summary>
|
||||
private static int _liveCount;
|
||||
|
||||
public CentralDbTestEnvironment()
|
||||
{
|
||||
if (Interlocked.Increment(ref _liveCount) != 1)
|
||||
{
|
||||
Interlocked.Decrement(ref _liveCount);
|
||||
throw new InvalidOperationException(
|
||||
$"Two {nameof(CentralDbTestEnvironment)} instances are live at once, so one fixture's " +
|
||||
"teardown can clear the process-wide environment variables another fixture's host boot " +
|
||||
$"is reading. Add the offending test class to the \"{HostBootCollection.Name}\" collection " +
|
||||
$"([Collection({nameof(HostBootCollection)}.Name)]) so xUnit runs it sequentially with the " +
|
||||
"other host-boot fixtures.");
|
||||
}
|
||||
|
||||
_previousConfig = Environment.GetEnvironmentVariable(ConfigKey);
|
||||
Environment.SetEnvironmentVariable(ConfigKey, ConfigurationDb);
|
||||
|
||||
@@ -89,5 +114,7 @@ internal sealed class CentralDbTestEnvironment : IDisposable
|
||||
Environment.SetEnvironmentVariable(PepperKey, _previousPepper);
|
||||
Environment.SetEnvironmentVariable(LdapPasswordKey, _previousLdapPassword);
|
||||
Environment.SetEnvironmentVariable(JwtSigningKeyKey, _previousJwtSigningKey);
|
||||
|
||||
Interlocked.Decrement(ref _liveCount);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -80,6 +80,7 @@ internal static class AkkaHostedServiceRemover
|
||||
/// Verifies every expected DI service resolves from the Central composition root.
|
||||
/// Uses WebApplicationFactory to exercise the real Program.cs pipeline.
|
||||
/// </summary>
|
||||
[Collection(HostBootCollection.Name)]
|
||||
public class CentralCompositionRootTests : IDisposable
|
||||
{
|
||||
private readonly WebApplicationFactory<Program> _factory;
|
||||
@@ -159,9 +160,17 @@ public class CentralCompositionRootTests : IDisposable
|
||||
|
||||
public void Dispose()
|
||||
{
|
||||
_factory.Dispose();
|
||||
Environment.SetEnvironmentVariable("DOTNET_ENVIRONMENT", _previousEnv);
|
||||
_dbEnv.Dispose();
|
||||
// try/finally: a throwing host teardown must not strand the process-wide
|
||||
// environment variables for the rest of the run.
|
||||
try
|
||||
{
|
||||
_factory.Dispose();
|
||||
}
|
||||
finally
|
||||
{
|
||||
Environment.SetEnvironmentVariable("DOTNET_ENVIRONMENT", _previousEnv);
|
||||
_dbEnv.Dispose();
|
||||
}
|
||||
}
|
||||
|
||||
// --- Singletons ---
|
||||
|
||||
@@ -17,6 +17,7 @@ namespace ZB.MOM.WW.ScadaBridge.Host.Tests;
|
||||
/// active-node) rather than by check-name predicates. These are pure route/tag assertions
|
||||
/// — they require no database, LDAP, or formed Akka cluster.
|
||||
/// </summary>
|
||||
[Collection(HostBootCollection.Name)]
|
||||
public class HealthCheckTests : IDisposable
|
||||
{
|
||||
private readonly List<IDisposable> _disposables = new();
|
||||
|
||||
@@ -0,0 +1,25 @@
|
||||
namespace ZB.MOM.WW.ScadaBridge.Host.Tests;
|
||||
|
||||
/// <summary>
|
||||
/// Serializes every fixture that boots a real host while depending on process-wide
|
||||
/// state — the <c>DOTNET_ENVIRONMENT</c> role selector and the environment variables
|
||||
/// <see cref="CentralDbTestEnvironment"/> sets.
|
||||
///
|
||||
/// <c>Program</c>'s configuration builder calls <c>AddEnvironmentVariables()</c> at an
|
||||
/// unpredictable point during boot, so a sibling fixture's teardown (which restores or
|
||||
/// clears those vars) can otherwise be observed mid-boot as a transient null. Since the
|
||||
/// secrets adoption (G-4) three of those keys are <c>${secret:...}</c> references that
|
||||
/// fail closed, which turns that race into a hard <c>SecretNotFoundException</c>.
|
||||
/// xUnit runs classes in a shared collection sequentially, which removes the overlap.
|
||||
///
|
||||
/// Site-role fixtures are deliberately not members: they call
|
||||
/// <c>Configuration.Sources.Clear()</c>, dropping the environment-variable provider, so
|
||||
/// they neither read nor write the shared state and stay free to run in parallel.
|
||||
///
|
||||
/// Supersedes the narrower "ActorSystem" collection, whose members are folded in here.
|
||||
/// </summary>
|
||||
[CollectionDefinition(HostBootCollection.Name)]
|
||||
public class HostBootCollection
|
||||
{
|
||||
public const string Name = "HostBoot";
|
||||
}
|
||||
@@ -9,6 +9,7 @@ using ZB.MOM.WW.ScadaBridge.Host;
|
||||
|
||||
namespace ZB.MOM.WW.ScadaBridge.Host.Tests;
|
||||
|
||||
[Collection(HostBootCollection.Name)]
|
||||
public class HostStartupTests : IDisposable
|
||||
{
|
||||
private readonly List<IDisposable> _disposables = new();
|
||||
|
||||
@@ -13,6 +13,7 @@ namespace ZB.MOM.WW.ScadaBridge.Host.Tests;
|
||||
/// cluster state. This is a pure route assertion — it requires no database, LDAP, or formed
|
||||
/// Akka cluster. The Central-role factory bootstrap mirrors <see cref="HealthCheckTests"/>.
|
||||
/// </summary>
|
||||
[Collection(HostBootCollection.Name)]
|
||||
public class MetricsEndpointTests : IDisposable
|
||||
{
|
||||
private readonly List<IDisposable> _disposables = new();
|
||||
|
||||
Reference in New Issue
Block a user