From 9668a4e84abf86be272ce1db3a104e1dc9ddce54 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 1 Jun 2026 22:49:41 -0400 Subject: [PATCH] refactor: ScadaBridge module options registration -> AddValidatedOptions; clarify De Morgan predicates --- .../Configuration/AuditLogOptionsValidator.cs | 4 ++++ .../ServiceCollectionExtensions.cs | 12 +++++++----- .../HealthMonitoringOptionsValidator.cs | 6 ++++++ 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/src/ZB.MOM.WW.ScadaBridge.AuditLog/Configuration/AuditLogOptionsValidator.cs b/src/ZB.MOM.WW.ScadaBridge.AuditLog/Configuration/AuditLogOptionsValidator.cs index f07e813c..91863980 100644 --- a/src/ZB.MOM.WW.ScadaBridge.AuditLog/Configuration/AuditLogOptionsValidator.cs +++ b/src/ZB.MOM.WW.ScadaBridge.AuditLog/Configuration/AuditLogOptionsValidator.cs @@ -39,11 +39,15 @@ public sealed class AuditLogOptionsValidator : OptionsValidatorBase= {nameof(AuditLogOptions.DefaultCapBytes)} ({options.DefaultCapBytes}); " + "the error-row cap is intended to capture more detail than the happy-path summary."); + // Valid when RetentionDays is within [Min, Max] inclusive. The De Morgan'd + // guard !(below Min OR above Max) is equivalent to (>= Min AND <= Max). builder.RequireThat( !(options.RetentionDays < MinRetentionDays || options.RetentionDays > MaxRetentionDays), $"AuditLog:{nameof(AuditLogOptions.RetentionDays)} ({options.RetentionDays}) " + $"must be in [{MinRetentionDays}, {MaxRetentionDays}] days."); + // Valid when InboundMaxBytes is within [Min, Max] inclusive. The De Morgan'd + // guard !(below Min OR above Max) is equivalent to (>= Min AND <= Max). builder.RequireThat( !(options.InboundMaxBytes < MinInboundMaxBytes || options.InboundMaxBytes > MaxInboundMaxBytes), $"AuditLog:{nameof(AuditLogOptions.InboundMaxBytes)} ({options.InboundMaxBytes}) " + diff --git a/src/ZB.MOM.WW.ScadaBridge.AuditLog/ServiceCollectionExtensions.cs b/src/ZB.MOM.WW.ScadaBridge.AuditLog/ServiceCollectionExtensions.cs index 81101d09..f065bbbd 100644 --- a/src/ZB.MOM.WW.ScadaBridge.AuditLog/ServiceCollectionExtensions.cs +++ b/src/ZB.MOM.WW.ScadaBridge.AuditLog/ServiceCollectionExtensions.cs @@ -3,7 +3,7 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.DependencyInjection.Extensions; using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; -using Microsoft.Extensions.Options; +using ZB.MOM.WW.Configuration; using ZB.MOM.WW.ScadaBridge.AuditLog.Central; using ZB.MOM.WW.ScadaBridge.AuditLog.Configuration; using ZB.MOM.WW.ScadaBridge.AuditLog.Payload; @@ -62,10 +62,12 @@ public static class ServiceCollectionExtensions ArgumentNullException.ThrowIfNull(config); // M1: top-level AuditLogOptions + validator (redaction policy, payload caps, etc.). - services.AddOptions() - .Bind(config.GetSection(ConfigSectionName)) - .ValidateOnStart(); - services.AddSingleton, AuditLogOptionsValidator>(); + // Collapsed onto the shared ZB.MOM.WW.Configuration helper: it binds the + // "AuditLog" section, registers the validator, and enables ValidateOnStart in + // one call. Same section path as before; AddAuditLog is call-once per + // collection, and the helper's TryAddEnumerable is idempotent for the + // validator (a strict improvement over the previous AddSingleton). + services.AddValidatedOptions(config, ConfigSectionName); // M5 Bundle A: payload filter — truncates oversized RequestSummary / // ResponseSummary / ErrorDetail / Extra fields between event diff --git a/src/ZB.MOM.WW.ScadaBridge.HealthMonitoring/HealthMonitoringOptionsValidator.cs b/src/ZB.MOM.WW.ScadaBridge.HealthMonitoring/HealthMonitoringOptionsValidator.cs index f9a27a49..3fdf3bb1 100644 --- a/src/ZB.MOM.WW.ScadaBridge.HealthMonitoring/HealthMonitoringOptionsValidator.cs +++ b/src/ZB.MOM.WW.ScadaBridge.HealthMonitoring/HealthMonitoringOptionsValidator.cs @@ -35,6 +35,12 @@ public sealed class HealthMonitoringOptionsValidator : OptionsValidatorBase= OfflineTimeout (both already + // required to be positive above). The De Morgan'd guard !(both positive + // AND Central < Offline) is true unless BOTH timeouts are positive and + // Central is strictly smaller — so it stays silent when either field is + // non-positive, leaving that failure to the dedicated positive-duration + // checks above rather than double-firing here. builder.RequireThat( !(options.OfflineTimeout > TimeSpan.Zero && options.CentralOfflineTimeout > TimeSpan.Zero