feat(options): eager startup validation for site-pipeline options (SiteRuntime, S&F, SiteEventLog) (arch-review 08 §1.5)

Claude-Session: https://claude.ai/code/session_01MtdgwpEeCUn6cUA5f1LMPj
2026-07-10 06:51:05 -04:00
parent af2cfde484
commit 8b775f4ae1
10 changed files with 342 additions and 3 deletions
@@ -1,3 +1,5 @@
+using Microsoft.Extensions.DependencyInjection.Extensions;
+using Microsoft.Extensions.Options;
 using ZB.MOM.WW.ScadaBridge.AuditLog;
 using ZB.MOM.WW.ScadaBridge.ClusterInfrastructure;
 using ZB.MOM.WW.ScadaBridge.Communication;
@@ -121,10 +123,33 @@ public static class SiteServiceRegistration

        // Options binding
        BindSharedOptions(services, config);
-        services.Configure<SiteRuntimeOptions>(config.GetSection("ScadaBridge:SiteRuntime"));
+
+        // Bind + eagerly validate the site-pipeline options. The validators live with their
+        // owner component projects (SiteRuntime / StoreAndForward / SiteEventLogging) per the
+        // "options classes owned by component projects" convention, but these three sections
+        // are bound HERE in the Host (not in a component SCE), so both the ValidateOnStart()
+        // chain and the TryAddEnumerable validator registration happen here. A bad section
+        // fails fast at host build with a clear, key-naming message instead of crashing an
+        // actor / hosted service later with an opaque ArgumentOutOfRangeException.
+        services.AddOptions<SiteRuntimeOptions>()
+            .Bind(config.GetSection("ScadaBridge:SiteRuntime"))
+            .ValidateOnStart();
+        services.TryAddEnumerable(
+            ServiceDescriptor.Singleton<IValidateOptions<SiteRuntimeOptions>, SiteRuntimeOptionsValidator>());
+
        services.Configure<DataConnectionOptions>(config.GetSection("ScadaBridge:DataConnection"));
-        services.Configure<StoreAndForwardOptions>(config.GetSection("ScadaBridge:StoreAndForward"));
-        services.Configure<SiteEventLogOptions>(config.GetSection("ScadaBridge:SiteEventLog"));
+
+        services.AddOptions<StoreAndForwardOptions>()
+            .Bind(config.GetSection("ScadaBridge:StoreAndForward"))
+            .ValidateOnStart();
+        services.TryAddEnumerable(
+            ServiceDescriptor.Singleton<IValidateOptions<StoreAndForwardOptions>, StoreAndForwardOptionsValidator>());
+
+        services.AddOptions<SiteEventLogOptions>()
+            .Bind(config.GetSection("ScadaBridge:SiteEventLog"))
+            .ValidateOnStart();
+        services.TryAddEnumerable(
+            ServiceDescriptor.Singleton<IValidateOptions<SiteEventLogOptions>, SiteEventLogOptionsValidator>());
    }

    /// <summary>Binds shared options sections (Node, Cluster, Database, Communication, etc.) used by both site and central roles.</summary>
@@ -0,0 +1,56 @@
+using ZB.MOM.WW.Configuration;
+
+namespace ZB.MOM.WW.ScadaBridge.SiteEventLogging;
+
+/// <summary>
+/// Validates <see cref="SiteEventLogOptions"/> at startup. Retention/purge values
+/// drive the daily purge PeriodicTimer (a zero/negative period trips
+/// <see cref="ArgumentOutOfRangeException"/>), the paging bounds gate query result
+/// sizes, and the write-queue capacity sizes a bounded channel; a
+/// <c>MaxQueryPageSize</c> below <c>QueryPageSize</c> would silently clamp the
+/// default page below itself. Registered with <c>ValidateOnStart()</c> so a bad
+/// <c>ScadaBridge:SiteEventLog</c> section fails fast at boot with a clear,
+/// key-naming message.
+/// </summary>
+public sealed class SiteEventLogOptionsValidator : OptionsValidatorBase<SiteEventLogOptions>
+{
+    /// <inheritdoc />
+    protected override void Validate(ValidationBuilder builder, SiteEventLogOptions options)
+    {
+        builder.RequireThat(options.RetentionDays > 0,
+            $"ScadaBridge:SiteEventLog:RetentionDays must be greater than 0 " +
+            $"(was {options.RetentionDays}).");
+
+        builder.RequireThat(options.MaxStorageMb > 0,
+            $"ScadaBridge:SiteEventLog:MaxStorageMb must be greater than 0 " +
+            $"(was {options.MaxStorageMb}).");
+
+        builder.RequireThat(!string.IsNullOrWhiteSpace(options.DatabasePath),
+            "ScadaBridge:SiteEventLog:DatabasePath must be a non-empty path; " +
+            "it is the SQLite file backing the site event log.");
+
+        builder.RequireThat(options.QueryPageSize > 0,
+            $"ScadaBridge:SiteEventLog:QueryPageSize must be greater than 0 " +
+            $"(was {options.QueryPageSize}).");
+
+        builder.RequireThat(options.WriteQueueCapacity > 0,
+            $"ScadaBridge:SiteEventLog:WriteQueueCapacity must be greater than 0 " +
+            $"(was {options.WriteQueueCapacity}); it bounds the background write channel.");
+
+        builder.RequireThat(options.PurgeInterval > TimeSpan.Zero,
+            $"ScadaBridge:SiteEventLog:PurgeInterval must be a positive duration " +
+            $"(was {options.PurgeInterval}); it is the purge PeriodicTimer period.");
+
+        // The hard page-size ceiling clamps a caller-supplied PageSize; a ceiling
+        // below the default QueryPageSize would silently clamp the default below
+        // itself. Only meaningful when both are positive (owned above), so stay
+        // silent when either is non-positive rather than double-firing.
+        builder.RequireThat(
+            !(options.QueryPageSize > 0
+              && options.MaxQueryPageSize > 0
+              && options.MaxQueryPageSize < options.QueryPageSize),
+            $"ScadaBridge:SiteEventLog:MaxQueryPageSize ({options.MaxQueryPageSize}) " +
+            $"must be >= QueryPageSize ({options.QueryPageSize}): the ceiling clamps a caller's " +
+            "requested page size, so it cannot be smaller than the default page size.");
+    }
+}
@@ -14,6 +14,7 @@
    <PackageReference Include="Microsoft.Extensions.Hosting.Abstractions" />
    <PackageReference Include="Microsoft.Extensions.Logging.Abstractions" />
    <PackageReference Include="Microsoft.Extensions.Options" />
+    <PackageReference Include="ZB.MOM.WW.Configuration" />
  </ItemGroup>

  <ItemGroup>
@@ -0,0 +1,60 @@
+using ZB.MOM.WW.Configuration;
+
+namespace ZB.MOM.WW.ScadaBridge.SiteRuntime;
+
+/// <summary>
+/// Validates <see cref="SiteRuntimeOptions"/> at startup. These values size the
+/// staggered Instance-Actor startup batches, the dedicated script-execution thread
+/// pool, and the site-wide Akka stream's per-subscriber buffer; a zero or negative
+/// count either creates a degenerate thread scheduler / stream stage or trips an
+/// <see cref="ArgumentOutOfRangeException"/> deep inside actor construction with an
+/// opaque message that never names the offending config key. Registered with
+/// <c>ValidateOnStart()</c> so a bad <c>ScadaBridge:SiteRuntime</c> section fails
+/// fast at boot with a clear, key-naming message.
+/// </summary>
+public sealed class SiteRuntimeOptionsValidator : OptionsValidatorBase<SiteRuntimeOptions>
+{
+    /// <inheritdoc />
+    protected override void Validate(ValidationBuilder builder, SiteRuntimeOptions options)
+    {
+        builder.RequireThat(options.StartupBatchSize > 0,
+            $"ScadaBridge:SiteRuntime:StartupBatchSize must be greater than 0 " +
+            $"(was {options.StartupBatchSize}); it sizes each staggered Instance-Actor startup batch.");
+
+        builder.RequireThat(options.StartupBatchDelayMs >= 0,
+            $"ScadaBridge:SiteRuntime:StartupBatchDelayMs must be >= 0 " +
+            $"(was {options.StartupBatchDelayMs}); it is the inter-batch startup delay.");
+
+        builder.RequireThat(options.MaxScriptCallDepth > 0,
+            $"ScadaBridge:SiteRuntime:MaxScriptCallDepth must be greater than 0 " +
+            $"(was {options.MaxScriptCallDepth}); it bounds recursive CallScript/CallShared depth.");
+
+        builder.RequireThat(options.ScriptExecutionTimeoutSeconds > 0,
+            $"ScadaBridge:SiteRuntime:ScriptExecutionTimeoutSeconds must be greater than 0 " +
+            $"(was {options.ScriptExecutionTimeoutSeconds}).");
+
+        builder.RequireThat(options.StreamBufferSize > 0,
+            $"ScadaBridge:SiteRuntime:StreamBufferSize must be greater than 0 " +
+            $"(was {options.StreamBufferSize}); it is the per-subscriber site-stream buffer size.");
+
+        builder.RequireThat(options.ScriptExecutionThreadCount > 0,
+            $"ScadaBridge:SiteRuntime:ScriptExecutionThreadCount must be greater than 0 " +
+            $"(was {options.ScriptExecutionThreadCount}); it sizes the dedicated script-execution scheduler.");
+
+        builder.RequireThat(options.MirroredAlarmCapPerSource > 0,
+            $"ScadaBridge:SiteRuntime:MirroredAlarmCapPerSource must be greater than 0 " +
+            $"(was {options.MirroredAlarmCapPerSource}).");
+
+        builder.RequireThat(options.NativeAlarmRetryIntervalMs > 0,
+            $"ScadaBridge:SiteRuntime:NativeAlarmRetryIntervalMs must be greater than 0 " +
+            $"(was {options.NativeAlarmRetryIntervalMs}).");
+
+        builder.RequireThat(options.ConfigFetchTimeoutSeconds > 0,
+            $"ScadaBridge:SiteRuntime:ConfigFetchTimeoutSeconds must be greater than 0 " +
+            $"(was {options.ConfigFetchTimeoutSeconds}).");
+
+        builder.RequireThat(options.ConfigFetchRetryCount >= 0,
+            $"ScadaBridge:SiteRuntime:ConfigFetchRetryCount must be >= 0 " +
+            $"(was {options.ConfigFetchRetryCount}).");
+    }
+}
@@ -19,6 +19,7 @@
    <PackageReference Include="Microsoft.Extensions.Http" />
    <PackageReference Include="Microsoft.Extensions.Logging.Abstractions" />
    <PackageReference Include="Microsoft.Extensions.Options" />
+    <PackageReference Include="ZB.MOM.WW.Configuration" />
  </ItemGroup>

  <ItemGroup>
@@ -0,0 +1,35 @@
+using ZB.MOM.WW.Configuration;
+
+namespace ZB.MOM.WW.ScadaBridge.StoreAndForward;
+
+/// <summary>
+/// Validates <see cref="StoreAndForwardOptions"/> at startup. The retry intervals
+/// feed the background sweep timer (a zero/negative period trips
+/// <see cref="ArgumentOutOfRangeException"/> in the timer constructor) and the
+/// SQLite path is opened for the S&amp;F buffer; an empty path yields an opaque
+/// connection failure at first enqueue. Registered with <c>ValidateOnStart()</c>
+/// so a bad <c>ScadaBridge:StoreAndForward</c> section fails fast at boot with a
+/// clear, key-naming message.
+/// </summary>
+public sealed class StoreAndForwardOptionsValidator : OptionsValidatorBase<StoreAndForwardOptions>
+{
+    /// <inheritdoc />
+    protected override void Validate(ValidationBuilder builder, StoreAndForwardOptions options)
+    {
+        builder.RequireThat(!string.IsNullOrWhiteSpace(options.SqliteDbPath),
+            "ScadaBridge:StoreAndForward:SqliteDbPath must be a non-empty path; " +
+            "it is the SQLite file backing the store-and-forward buffer.");
+
+        builder.RequireThat(options.DefaultRetryInterval > TimeSpan.Zero,
+            $"ScadaBridge:StoreAndForward:DefaultRetryInterval must be a positive duration " +
+            $"(was {options.DefaultRetryInterval}); it is the default per-message retry interval.");
+
+        builder.RequireThat(options.RetryTimerInterval > TimeSpan.Zero,
+            $"ScadaBridge:StoreAndForward:RetryTimerInterval must be a positive duration " +
+            $"(was {options.RetryTimerInterval}); it is the background retry-sweep timer period.");
+
+        builder.RequireThat(options.DefaultMaxRetries >= 0,
+            $"ScadaBridge:StoreAndForward:DefaultMaxRetries must be >= 0 " +
+            $"(was {options.DefaultMaxRetries}).");
+    }
+}
@@ -13,6 +13,7 @@
    <PackageReference Include="Microsoft.Extensions.DependencyInjection.Abstractions" />
    <PackageReference Include="Microsoft.Extensions.Logging.Abstractions" />
    <PackageReference Include="Microsoft.Extensions.Options" />
+    <PackageReference Include="ZB.MOM.WW.Configuration" />
  </ItemGroup>

  <ItemGroup>
@@ -0,0 +1,50 @@
+using Microsoft.Extensions.Options;
+
+namespace ZB.MOM.WW.ScadaBridge.SiteEventLogging.Tests;
+
+/// <summary>
+/// <see cref="SiteEventLogOptions"/> retention/purge values drive the daily purge
+/// PeriodicTimer (zero/negative trips its constructor) and the paging bounds gate
+/// query result sizes. These tests assert the
+/// <see cref="SiteEventLogOptionsValidator"/> rejects a bad
+/// <c>ScadaBridge:SiteEventLog</c> section with a clear, key-naming message.
+/// </summary>
+public class SiteEventLogOptionsValidatorTests
+{
+    private static ValidateOptionsResult Validate(SiteEventLogOptions options) =>
+        new SiteEventLogOptionsValidator().Validate(Options.DefaultName, options);
+
+    [Fact]
+    public void DefaultOptions_AreValid()
+    {
+        var result = Validate(new SiteEventLogOptions());
+        Assert.True(result.Succeeded, result.FailureMessage);
+    }
+
+    [Fact]
+    public void ZeroRetentionDays_IsRejected()
+    {
+        var result = Validate(new SiteEventLogOptions { RetentionDays = 0 });
+
+        Assert.True(result.Failed);
+        Assert.Contains("RetentionDays", result.FailureMessage);
+    }
+
+    [Fact]
+    public void ZeroPurgeInterval_IsRejected()
+    {
+        var result = Validate(new SiteEventLogOptions { PurgeInterval = TimeSpan.Zero });
+
+        Assert.True(result.Failed);
+        Assert.Contains("PurgeInterval", result.FailureMessage);
+    }
+
+    [Fact]
+    public void MaxQueryPageSize_BelowQueryPageSize_IsRejected()
+    {
+        var result = Validate(new SiteEventLogOptions { QueryPageSize = 500, MaxQueryPageSize = 100 });
+
+        Assert.True(result.Failed);
+        Assert.Contains("MaxQueryPageSize", result.FailureMessage);
+    }
+}
@@ -0,0 +1,60 @@
+using Microsoft.Extensions.Options;
+
+namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests;
+
+/// <summary>
+/// <see cref="SiteRuntimeOptions"/> values size the staggered Instance-Actor
+/// startup batches, the dedicated script-execution thread scheduler, and the
+/// site-wide stream buffer. A zero/negative count crashes actor construction with
+/// an opaque <see cref="ArgumentOutOfRangeException"/>; these tests assert the
+/// <see cref="SiteRuntimeOptionsValidator"/> rejects a bad
+/// <c>ScadaBridge:SiteRuntime</c> section with a clear, key-naming message.
+/// </summary>
+public class SiteRuntimeOptionsValidatorTests
+{
+    private static ValidateOptionsResult Validate(SiteRuntimeOptions options) =>
+        new SiteRuntimeOptionsValidator().Validate(Options.DefaultName, options);
+
+    [Fact]
+    public void DefaultOptions_AreValid()
+    {
+        var result = Validate(new SiteRuntimeOptions());
+        Assert.True(result.Succeeded, result.FailureMessage);
+    }
+
+    [Fact]
+    public void ZeroStartupBatchSize_IsRejected()
+    {
+        var result = Validate(new SiteRuntimeOptions { StartupBatchSize = 0 });
+
+        Assert.True(result.Failed);
+        Assert.Contains("StartupBatchSize", result.FailureMessage);
+    }
+
+    [Fact]
+    public void ZeroScriptExecutionThreadCount_IsRejected()
+    {
+        var result = Validate(new SiteRuntimeOptions { ScriptExecutionThreadCount = 0 });
+
+        Assert.True(result.Failed);
+        Assert.Contains("ScriptExecutionThreadCount", result.FailureMessage);
+    }
+
+    [Fact]
+    public void ZeroStreamBufferSize_IsRejected()
+    {
+        var result = Validate(new SiteRuntimeOptions { StreamBufferSize = 0 });
+
+        Assert.True(result.Failed);
+        Assert.Contains("StreamBufferSize", result.FailureMessage);
+    }
+
+    [Fact]
+    public void NegativeStartupBatchDelayMs_IsRejected()
+    {
+        var result = Validate(new SiteRuntimeOptions { StartupBatchDelayMs = -1 });
+
+        Assert.True(result.Failed);
+        Assert.Contains("StartupBatchDelayMs", result.FailureMessage);
+    }
+}
@@ -0,0 +1,50 @@
+using Microsoft.Extensions.Options;
+
+namespace ZB.MOM.WW.ScadaBridge.StoreAndForward.Tests;
+
+/// <summary>
+/// <see cref="StoreAndForwardOptions"/> retry intervals feed the background sweep
+/// timer (zero/negative trips the timer constructor) and the SQLite path backs the
+/// buffer. These tests assert the <see cref="StoreAndForwardOptionsValidator"/>
+/// rejects a bad <c>ScadaBridge:StoreAndForward</c> section with a clear,
+/// key-naming message rather than crashing later with an opaque exception.
+/// </summary>
+public class StoreAndForwardOptionsValidatorTests
+{
+    private static ValidateOptionsResult Validate(StoreAndForwardOptions options) =>
+        new StoreAndForwardOptionsValidator().Validate(Options.DefaultName, options);
+
+    [Fact]
+    public void DefaultOptions_AreValid()
+    {
+        var result = Validate(new StoreAndForwardOptions());
+        Assert.True(result.Succeeded, result.FailureMessage);
+    }
+
+    [Fact]
+    public void ZeroDefaultRetryInterval_IsRejected()
+    {
+        var result = Validate(new StoreAndForwardOptions { DefaultRetryInterval = TimeSpan.Zero });
+
+        Assert.True(result.Failed);
+        Assert.Contains("DefaultRetryInterval", result.FailureMessage);
+    }
+
+    [Fact]
+    public void ZeroRetryTimerInterval_IsRejected()
+    {
+        var result = Validate(new StoreAndForwardOptions { RetryTimerInterval = TimeSpan.Zero });
+
+        Assert.True(result.Failed);
+        Assert.Contains("RetryTimerInterval", result.FailureMessage);
+    }
+
+    [Fact]
+    public void EmptySqliteDbPath_IsRejected()
+    {
+        var result = Validate(new StoreAndForwardOptions { SqliteDbPath = "" });
+
+        Assert.True(result.Failed);
+        Assert.Contains("SqliteDbPath", result.FailureMessage);
+    }
+}