fix(options): bind + validate OperationTrackingOptions; register the site IOperationTrackingStore that two comments claimed AddSiteRuntime provides (plan R2-08 T8, arch-review 08r2 NF4)

VERIFY-THEN-FIX: the plan-authoring discovery is CONFIRMED. No code anywhere in
src/ registered IOperationTrackingStore in DI, yet AkkaHostedService and
AuditLog SCE both comment that AddSiteRuntime provides it. Every consumer
resolved it via GetService (null-tolerant) and silently ran degraded:
cached-drain scheduler never armed, PullSiteCalls reconciliation seam never
wired, Tracking.Status degraded to audit-only. This is a FUNCTIONAL FIX riding
the hygiene plan: AddSiteRuntime now registers the store (site-only), so
site-local cached-call tracking runs in its intended mode. OperationTrackingOptions
is now bound + eagerly validated in the Host site-options block.
This commit is contained in:
Joseph Doherty
2026-07-13 10:53:27 -04:00
parent 4a0462e4d0
commit 973e59de84
5 changed files with 111 additions and 0 deletions
@@ -137,6 +137,18 @@ public static class SiteServiceRegistration
services.TryAddEnumerable(
ServiceDescriptor.Singleton<IValidateOptions<SiteRuntimeOptions>, SiteRuntimeOptionsValidator>());
// OperationTrackingOptions for the site-local cached-call tracking store
// (registered by AddSiteRuntime). Bind + eagerly validate here — the store
// opens its SQLite connection string at construction, so a blank connection
// string should fail the host at boot with a key-naming message rather than
// throw opaquely on first resolve (arch-review 08r2 NF4/T8).
services.AddOptions<SiteRuntime.Tracking.OperationTrackingOptions>()
.Bind(config.GetSection("ScadaBridge:OperationTracking"))
.ValidateOnStart();
services.TryAddEnumerable(
ServiceDescriptor.Singleton<IValidateOptions<SiteRuntime.Tracking.OperationTrackingOptions>,
SiteRuntime.Tracking.OperationTrackingOptionsValidator>());
services.Configure<DataConnectionOptions>(config.GetSection("ScadaBridge:DataConnection"));
services.AddOptions<StoreAndForwardOptions>()
@@ -60,6 +60,19 @@ public static class ServiceCollectionExtensions
});
services.AddSingleton<ISiteStreamSubscriber>(sp => sp.GetRequiredService<SiteStreamManager>());
// Site-local cached-operation tracking store — the SQLite source of truth
// that Tracking.Status(TrackedOperationId) reads and the cached-call telemetry
// forwarder writes. Registered HERE (site-only) so the two comments that claim
// "the operational tracking store is registered by AddSiteRuntime" are true
// again (AuditLog SCE + AkkaHostedService). Before this (arch-review 08 round 2
// NF4/T8) it had no DI registration anywhere in src/, so every consumer resolved
// it as null and silently ran degraded (no cached-drain, no PullSiteCalls
// reconciliation, Tracking.Status audit-only). Central roots never call
// AddSiteRuntime, so this stays absent on central — matching the SITE-ONLY
// contract. OperationTrackingOptions is bound + validated by the Host's
// SiteServiceRegistration site-options block.
services.AddSingleton<Commons.Interfaces.IOperationTrackingStore, Tracking.OperationTrackingStore>();
// Site-local repository implementations backed by SQLite
services.AddScoped<IExternalSystemRepository, SiteExternalSystemRepository>();
@@ -0,0 +1,26 @@
using ZB.MOM.WW.Configuration;
namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Tracking;
/// <summary>
/// Validates <see cref="OperationTrackingOptions"/> at host startup (arch-review
/// 08 round 2 NF4). <see cref="OperationTrackingStore"/> opens the SQLite
/// connection string in its constructor, so an empty/blank connection string
/// must fail the host at boot rather than throw opaquely on first resolve. The
/// retention window must be positive — a zero would make the host's terminal-row
/// purge delete every tracking row.
/// </summary>
public sealed class OperationTrackingOptionsValidator : OptionsValidatorBase<OperationTrackingOptions>
{
/// <inheritdoc />
protected override void Validate(ValidationBuilder builder, OperationTrackingOptions options)
{
builder.RequireThat(!string.IsNullOrWhiteSpace(options.ConnectionString),
$"ScadaBridge:OperationTracking:{nameof(OperationTrackingOptions.ConnectionString)} " +
"must be a non-empty SQLite connection string.");
builder.RequireThat(options.RetentionDays > 0,
$"ScadaBridge:OperationTracking:{nameof(OperationTrackingOptions.RetentionDays)} " +
$"({options.RetentionDays}) must be > 0.");
}
}
@@ -339,10 +339,12 @@ public class SiteCompositionRootTests : IDisposable
{
private readonly WebApplication _host;
private readonly string _tempDbPath;
private readonly string _tempTrackingDbPath;
public SiteCompositionRootTests()
{
_tempDbPath = Path.Combine(Path.GetTempPath(), $"scadabridge_test_{Guid.NewGuid()}.db");
_tempTrackingDbPath = Path.Combine(Path.GetTempPath(), $"scadabridge_track_{Guid.NewGuid()}.db");
var builder = WebApplication.CreateBuilder();
builder.Configuration.Sources.Clear();
@@ -355,6 +357,10 @@ public class SiteCompositionRootTests : IDisposable
["ScadaBridge:Node:RemotingPort"] = "0",
["ScadaBridge:Node:GrpcPort"] = "0",
["ScadaBridge:Database:SiteDbPath"] = _tempDbPath,
// Point the site-local operation-tracking SQLite store (the ctor opens/creates
// the file eagerly) at a temp path so resolving IOperationTrackingStore in the
// composition-root test does not litter site-tracking.db in the test cwd.
["ScadaBridge:OperationTracking:ConnectionString"] = $"Data Source={_tempTrackingDbPath}",
// ClusterOptions requires at least one seed node (ClusterOptionsValidator).
["ScadaBridge:Cluster:SeedNodes:0"] = "akka.tcp://scadabridge@localhost:2551",
["ScadaBridge:Cluster:SeedNodes:1"] = "akka.tcp://scadabridge@localhost:2552",
@@ -377,6 +383,20 @@ public class SiteCompositionRootTests : IDisposable
{
(_host as IDisposable)?.Dispose();
try { File.Delete(_tempDbPath); } catch { /* best effort */ }
try { File.Delete(_tempTrackingDbPath); } catch { /* best effort */ }
}
[Fact]
public void Site_Resolves_IOperationTrackingStore()
{
// Site Call Audit's site-local source of truth. AkkaHostedService and
// ScriptRuntimeContext resolve this with GetService and silently degrade
// when absent (audit-only telemetry, no cached-drain, no PullSiteCalls) —
// so its presence on the SITE composition root must be locked here
// (arch-review 08 round 2 NF4/T8: AddSiteRuntime never registered it,
// despite two comments claiming it did).
var store = _host.Services.GetService<ZB.MOM.WW.ScadaBridge.Commons.Interfaces.IOperationTrackingStore>();
Assert.NotNull(store);
}
// --- Singletons ---
@@ -0,0 +1,40 @@
using ZB.MOM.WW.ScadaBridge.SiteRuntime.Tracking;
namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.Tracking;
/// <summary>
/// Eager startup validation for <see cref="OperationTrackingOptions"/>
/// (arch-review 08 round 2 NF4). The store opens the SQLite connection string at
/// construction, so an empty connection string must fail the host at boot; the
/// retention window must be positive or terminal-row purge would delete
/// everything.
/// </summary>
public class OperationTrackingOptionsValidatorTests
{
[Fact]
public void DefaultOptions_AreValid()
{
var validator = new OperationTrackingOptionsValidator();
Assert.True(validator.Validate(null, new OperationTrackingOptions()).Succeeded);
}
[Fact]
public void EmptyConnectionString_Fails()
{
var validator = new OperationTrackingOptionsValidator();
var result = validator.Validate(null, new OperationTrackingOptions { ConnectionString = "" });
Assert.False(result.Succeeded);
Assert.Contains(result.Failures!,
f => f.Contains(nameof(OperationTrackingOptions.ConnectionString), StringComparison.Ordinal));
}
[Fact]
public void ZeroRetentionDays_Fails()
{
var validator = new OperationTrackingOptionsValidator();
var result = validator.Validate(null, new OperationTrackingOptions { RetentionDays = 0 });
Assert.False(result.Succeeded);
Assert.Contains(result.Failures!,
f => f.Contains(nameof(OperationTrackingOptions.RetentionDays), StringComparison.Ordinal));
}
}