From f85f036b09b81875d12b24af69dcf6773f86e84a Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 13 Jul 2026 10:39:03 -0400 Subject: [PATCH] =?UTF-8?q?fix(options):=20eager=20startup=20validation=20?= =?UTF-8?q?for=20AuditLog=20central=20sub-options=20=E2=80=94=20PartitionM?= =?UTF-8?q?aintenance,=20Purge,=20Reconciliation=20(plan=20R2-08=20T6,=20a?= =?UTF-8?q?rch-review=2008r2=20NF4)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- ...LogPartitionMaintenanceOptionsValidator.cs | 25 +++++++++ .../Central/AuditLogPurgeOptionsValidator.cs | 31 +++++++++++ ...SiteAuditReconciliationOptionsValidator.cs | 31 +++++++++++ .../ServiceCollectionExtensions.cs | 12 ++--- ...rtitionMaintenanceOptionsValidatorTests.cs | 41 ++++++++++++++ .../AuditLogPurgeOptionsValidatorTests.cs | 53 ++++++++++++++++++ ...uditReconciliationOptionsValidatorTests.cs | 54 +++++++++++++++++++ 7 files changed, 241 insertions(+), 6 deletions(-) create mode 100644 src/ZB.MOM.WW.ScadaBridge.AuditLog/Central/AuditLogPartitionMaintenanceOptionsValidator.cs create mode 100644 src/ZB.MOM.WW.ScadaBridge.AuditLog/Central/AuditLogPurgeOptionsValidator.cs create mode 100644 src/ZB.MOM.WW.ScadaBridge.AuditLog/Central/SiteAuditReconciliationOptionsValidator.cs create mode 100644 tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/Central/AuditLogPartitionMaintenanceOptionsValidatorTests.cs create mode 100644 tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/Central/AuditLogPurgeOptionsValidatorTests.cs create mode 100644 tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/Central/SiteAuditReconciliationOptionsValidatorTests.cs diff --git a/src/ZB.MOM.WW.ScadaBridge.AuditLog/Central/AuditLogPartitionMaintenanceOptionsValidator.cs b/src/ZB.MOM.WW.ScadaBridge.AuditLog/Central/AuditLogPartitionMaintenanceOptionsValidator.cs new file mode 100644 index 00000000..0ad5373a --- /dev/null +++ b/src/ZB.MOM.WW.ScadaBridge.AuditLog/Central/AuditLogPartitionMaintenanceOptionsValidator.cs @@ -0,0 +1,25 @@ +using ZB.MOM.WW.Configuration; + +namespace ZB.MOM.WW.ScadaBridge.AuditLog.Central; + +/// +/// Validates at host startup +/// (arch-review 08 round 2 NF4). A zero tick interval spins the maintenance +/// hosted service; a zero look-ahead leaves pf_AuditLog_Month without a +/// future monthly boundary, risking inserts into the unbounded tail partition. +/// +public sealed class AuditLogPartitionMaintenanceOptionsValidator + : OptionsValidatorBase +{ + /// + protected override void Validate(ValidationBuilder builder, AuditLogPartitionMaintenanceOptions options) + { + builder.RequireThat(options.IntervalSeconds > 0, + $"AuditLog:PartitionMaintenance:{nameof(AuditLogPartitionMaintenanceOptions.IntervalSeconds)} " + + $"({options.IntervalSeconds}) must be > 0."); + + builder.RequireThat(options.LookaheadMonths > 0, + $"AuditLog:PartitionMaintenance:{nameof(AuditLogPartitionMaintenanceOptions.LookaheadMonths)} " + + $"({options.LookaheadMonths}) must be > 0."); + } +} diff --git a/src/ZB.MOM.WW.ScadaBridge.AuditLog/Central/AuditLogPurgeOptionsValidator.cs b/src/ZB.MOM.WW.ScadaBridge.AuditLog/Central/AuditLogPurgeOptionsValidator.cs new file mode 100644 index 00000000..da5bb659 --- /dev/null +++ b/src/ZB.MOM.WW.ScadaBridge.AuditLog/Central/AuditLogPurgeOptionsValidator.cs @@ -0,0 +1,31 @@ +using ZB.MOM.WW.Configuration; + +namespace ZB.MOM.WW.ScadaBridge.AuditLog.Central; + +/// +/// Validates at host startup (arch-review 08 +/// round 2 NF4). The class self-clamps at resolve time, but a plainly-wrong +/// config (zero cadence, zero batch, zero maintenance-command timeout) should +/// still fail the boot so the operator learns immediately. +/// is test-only (per the +/// class remarks) and is deliberately not validated. +/// +public sealed class AuditLogPurgeOptionsValidator : OptionsValidatorBase +{ + /// + protected override void Validate(ValidationBuilder builder, AuditLogPurgeOptions options) + { + builder.RequireThat(options.IntervalHours > 0, + $"AuditLog:Purge:{nameof(AuditLogPurgeOptions.IntervalHours)} " + + $"({options.IntervalHours}) must be > 0."); + + // Backing field for the documented "ChannelPurgeBatchSize" config key. + builder.RequireThat(options.ChannelPurgeBatchSizeConfigured > 0, + $"AuditLog:Purge:{nameof(AuditLogPurgeOptions.ChannelPurgeBatchSizeConfigured)} " + + $"(config key ChannelPurgeBatchSize = {options.ChannelPurgeBatchSizeConfigured}) must be > 0."); + + builder.RequireThat(options.MaintenanceCommandTimeoutMinutes > 0, + $"AuditLog:Purge:{nameof(AuditLogPurgeOptions.MaintenanceCommandTimeoutMinutes)} " + + $"({options.MaintenanceCommandTimeoutMinutes}) must be > 0."); + } +} diff --git a/src/ZB.MOM.WW.ScadaBridge.AuditLog/Central/SiteAuditReconciliationOptionsValidator.cs b/src/ZB.MOM.WW.ScadaBridge.AuditLog/Central/SiteAuditReconciliationOptionsValidator.cs new file mode 100644 index 00000000..63bee720 --- /dev/null +++ b/src/ZB.MOM.WW.ScadaBridge.AuditLog/Central/SiteAuditReconciliationOptionsValidator.cs @@ -0,0 +1,31 @@ +using ZB.MOM.WW.Configuration; + +namespace ZB.MOM.WW.ScadaBridge.AuditLog.Central; + +/// +/// Validates at host startup +/// (arch-review 08 round 2 NF4). A zero tick interval spins the reconciliation +/// singleton, a zero batch pulls nothing per RPC, and a zero stalled-cycle +/// threshold would fire the stalled signal on the first non-draining cycle. +/// +/// is test-only (per the class remarks) and is deliberately not validated. +/// +public sealed class SiteAuditReconciliationOptionsValidator + : OptionsValidatorBase +{ + /// + protected override void Validate(ValidationBuilder builder, SiteAuditReconciliationOptions options) + { + builder.RequireThat(options.ReconciliationIntervalSeconds > 0, + $"AuditLog:Reconciliation:{nameof(SiteAuditReconciliationOptions.ReconciliationIntervalSeconds)} " + + $"({options.ReconciliationIntervalSeconds}) must be > 0."); + + builder.RequireThat(options.BatchSize > 0, + $"AuditLog:Reconciliation:{nameof(SiteAuditReconciliationOptions.BatchSize)} " + + $"({options.BatchSize}) must be > 0."); + + builder.RequireThat(options.StalledAfterNonDrainingCycles > 0, + $"AuditLog:Reconciliation:{nameof(SiteAuditReconciliationOptions.StalledAfterNonDrainingCycles)} " + + $"({options.StalledAfterNonDrainingCycles}) must be > 0."); + } +} diff --git a/src/ZB.MOM.WW.ScadaBridge.AuditLog/ServiceCollectionExtensions.cs b/src/ZB.MOM.WW.ScadaBridge.AuditLog/ServiceCollectionExtensions.cs index f1d68167..54497c4a 100644 --- a/src/ZB.MOM.WW.ScadaBridge.AuditLog/ServiceCollectionExtensions.cs +++ b/src/ZB.MOM.WW.ScadaBridge.AuditLog/ServiceCollectionExtensions.cs @@ -368,8 +368,8 @@ public static class ServiceCollectionExtensions ArgumentNullException.ThrowIfNull(services); ArgumentNullException.ThrowIfNull(config); - services.AddOptions() - .Bind(config.GetSection(PartitionMaintenanceSectionName)); + services.AddValidatedOptions(config, PartitionMaintenanceSectionName); services.AddHostedService(); // I1 (review): bind the two central-singleton options HERE rather than in @@ -385,10 +385,10 @@ public static class ServiceCollectionExtensions // 5 min reconciliation tick); production exposes IntervalHours / // ReconciliationIntervalSeconds only — the test-only *Override knobs are // not intended to be set from config (see the options classes' remarks). - services.AddOptions() - .Bind(config.GetSection(PurgeSectionName)); - services.AddOptions() - .Bind(config.GetSection(ReconciliationSectionName)); + services.AddValidatedOptions( + config, PurgeSectionName); + services.AddValidatedOptions( + config, ReconciliationSectionName); // Central health snapshot — a single object // that owns the CentralAuditWriteFailures + AuditRedactionFailure diff --git a/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/Central/AuditLogPartitionMaintenanceOptionsValidatorTests.cs b/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/Central/AuditLogPartitionMaintenanceOptionsValidatorTests.cs new file mode 100644 index 00000000..2b45be34 --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/Central/AuditLogPartitionMaintenanceOptionsValidatorTests.cs @@ -0,0 +1,41 @@ +using ZB.MOM.WW.ScadaBridge.AuditLog.Central; + +namespace ZB.MOM.WW.ScadaBridge.AuditLog.Tests.Central; + +/// +/// Eager startup validation for +/// (arch-review 08 round 2 NF4). A zero tick interval spins the maintenance +/// hosted service; a zero look-ahead leaves the partition function without a +/// future monthly boundary, risking inserts into the unbounded tail partition. +/// +public class AuditLogPartitionMaintenanceOptionsValidatorTests +{ + [Fact] + public void DefaultOptions_AreValid() + { + var validator = new AuditLogPartitionMaintenanceOptionsValidator(); + Assert.True(validator.Validate(null, new AuditLogPartitionMaintenanceOptions()).Succeeded); + } + + [Fact] + public void ZeroIntervalSeconds_Fails() + { + var validator = new AuditLogPartitionMaintenanceOptionsValidator(); + var result = validator.Validate(null, + new AuditLogPartitionMaintenanceOptions { IntervalSeconds = 0 }); + Assert.False(result.Succeeded); + Assert.Contains(result.Failures!, + f => f.Contains(nameof(AuditLogPartitionMaintenanceOptions.IntervalSeconds), StringComparison.Ordinal)); + } + + [Fact] + public void ZeroLookaheadMonths_Fails() + { + var validator = new AuditLogPartitionMaintenanceOptionsValidator(); + var result = validator.Validate(null, + new AuditLogPartitionMaintenanceOptions { LookaheadMonths = 0 }); + Assert.False(result.Succeeded); + Assert.Contains(result.Failures!, + f => f.Contains(nameof(AuditLogPartitionMaintenanceOptions.LookaheadMonths), StringComparison.Ordinal)); + } +} diff --git a/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/Central/AuditLogPurgeOptionsValidatorTests.cs b/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/Central/AuditLogPurgeOptionsValidatorTests.cs new file mode 100644 index 00000000..d2c05964 --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/Central/AuditLogPurgeOptionsValidatorTests.cs @@ -0,0 +1,53 @@ +using ZB.MOM.WW.ScadaBridge.AuditLog.Central; + +namespace ZB.MOM.WW.ScadaBridge.AuditLog.Tests.Central; + +/// +/// Eager startup validation for +/// (arch-review 08 round 2 NF4). The class self-clamps at resolve time, but a +/// plainly-wrong config (zero cadence, zero batch, zero command timeout) should +/// still fail the boot so the operator learns immediately. +/// is test-only and left +/// unvalidated. +/// +public class AuditLogPurgeOptionsValidatorTests +{ + [Fact] + public void DefaultOptions_AreValid() + { + var validator = new AuditLogPurgeOptionsValidator(); + Assert.True(validator.Validate(null, new AuditLogPurgeOptions()).Succeeded); + } + + [Fact] + public void ZeroIntervalHours_Fails() + { + var validator = new AuditLogPurgeOptionsValidator(); + var result = validator.Validate(null, new AuditLogPurgeOptions { IntervalHours = 0 }); + Assert.False(result.Succeeded); + Assert.Contains(result.Failures!, + f => f.Contains(nameof(AuditLogPurgeOptions.IntervalHours), StringComparison.Ordinal)); + } + + [Fact] + public void ZeroChannelPurgeBatchSize_Fails() + { + var validator = new AuditLogPurgeOptionsValidator(); + var result = validator.Validate(null, + new AuditLogPurgeOptions { ChannelPurgeBatchSizeConfigured = 0 }); + Assert.False(result.Succeeded); + Assert.Contains(result.Failures!, + f => f.Contains(nameof(AuditLogPurgeOptions.ChannelPurgeBatchSizeConfigured), StringComparison.Ordinal)); + } + + [Fact] + public void ZeroMaintenanceCommandTimeout_Fails() + { + var validator = new AuditLogPurgeOptionsValidator(); + var result = validator.Validate(null, + new AuditLogPurgeOptions { MaintenanceCommandTimeoutMinutes = 0 }); + Assert.False(result.Succeeded); + Assert.Contains(result.Failures!, + f => f.Contains(nameof(AuditLogPurgeOptions.MaintenanceCommandTimeoutMinutes), StringComparison.Ordinal)); + } +} diff --git a/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/Central/SiteAuditReconciliationOptionsValidatorTests.cs b/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/Central/SiteAuditReconciliationOptionsValidatorTests.cs new file mode 100644 index 00000000..4d333045 --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/Central/SiteAuditReconciliationOptionsValidatorTests.cs @@ -0,0 +1,54 @@ +using ZB.MOM.WW.ScadaBridge.AuditLog.Central; + +namespace ZB.MOM.WW.ScadaBridge.AuditLog.Tests.Central; + +/// +/// Eager startup validation for +/// (arch-review 08 round 2 NF4). A zero tick interval spins the reconciliation +/// singleton, a zero batch pulls nothing, and a zero stalled-cycle threshold +/// would fire the stalled signal on the first non-draining cycle. +/// +/// is test-only and left unvalidated. +/// +public class SiteAuditReconciliationOptionsValidatorTests +{ + [Fact] + public void DefaultOptions_AreValid() + { + var validator = new SiteAuditReconciliationOptionsValidator(); + Assert.True(validator.Validate(null, new SiteAuditReconciliationOptions()).Succeeded); + } + + [Fact] + public void ZeroReconciliationInterval_Fails() + { + var validator = new SiteAuditReconciliationOptionsValidator(); + var result = validator.Validate(null, + new SiteAuditReconciliationOptions { ReconciliationIntervalSeconds = 0 }); + Assert.False(result.Succeeded); + Assert.Contains(result.Failures!, + f => f.Contains(nameof(SiteAuditReconciliationOptions.ReconciliationIntervalSeconds), StringComparison.Ordinal)); + } + + [Fact] + public void ZeroBatchSize_Fails() + { + var validator = new SiteAuditReconciliationOptionsValidator(); + var result = validator.Validate(null, + new SiteAuditReconciliationOptions { BatchSize = 0 }); + Assert.False(result.Succeeded); + Assert.Contains(result.Failures!, + f => f.Contains(nameof(SiteAuditReconciliationOptions.BatchSize), StringComparison.Ordinal)); + } + + [Fact] + public void ZeroStalledCycles_Fails() + { + var validator = new SiteAuditReconciliationOptionsValidator(); + var result = validator.Validate(null, + new SiteAuditReconciliationOptions { StalledAfterNonDrainingCycles = 0 }); + Assert.False(result.Succeeded); + Assert.Contains(result.Failures!, + f => f.Contains(nameof(SiteAuditReconciliationOptions.StalledAfterNonDrainingCycles), StringComparison.Ordinal)); + } +}