fix(options): canonicalize Communication/DataConnection config sections to ScadaBridge:*; drop the duplicate Host bindings that were masking the drift (plan R2-08 T9, arch-review 08r2 NF8)
This commit is contained in:
Joseph Doherty
@@ -4,7 +4,7 @@ namespace ZB.MOM.WW.ScadaBridge.Communication;
|
||||
|
||||
/// <summary>
|
||||
/// Validates <see cref="CommunicationOptions"/> at startup (ValidateOnStart) so a
|
||||
/// malformed "Communication" appsettings section fails fast at boot with a
|
||||
/// malformed "ScadaBridge:Communication" appsettings section fails fast at boot with a
|
||||
/// key-naming message instead of surfacing later at first Ask/gRPC use. Every
|
||||
/// timeout feeds a per-pattern Ask deadline or a gRPC keepalive/stream-lifetime
|
||||
/// setting, and a zero/negative value there produces an opaque runtime failure
|
||||
@@ -16,74 +16,74 @@ public sealed class CommunicationOptionsValidator : OptionsValidatorBase<Communi
|
||||
protected override void Validate(ValidationBuilder builder, CommunicationOptions options)
|
||||
{
|
||||
builder.RequireThat(options.DeploymentTimeout > TimeSpan.Zero,
|
||||
$"Communication:DeploymentTimeout must be a positive duration (was {options.DeploymentTimeout}).");
|
||||
$"ScadaBridge:Communication:DeploymentTimeout must be a positive duration (was {options.DeploymentTimeout}).");
|
||||
|
||||
builder.RequireThat(options.LifecycleTimeout > TimeSpan.Zero,
|
||||
$"Communication:LifecycleTimeout must be a positive duration (was {options.LifecycleTimeout}).");
|
||||
$"ScadaBridge:Communication:LifecycleTimeout must be a positive duration (was {options.LifecycleTimeout}).");
|
||||
|
||||
builder.RequireThat(options.ArtifactDeploymentTimeout > TimeSpan.Zero,
|
||||
$"Communication:ArtifactDeploymentTimeout must be a positive duration (was {options.ArtifactDeploymentTimeout}).");
|
||||
$"ScadaBridge:Communication:ArtifactDeploymentTimeout must be a positive duration (was {options.ArtifactDeploymentTimeout}).");
|
||||
|
||||
builder.RequireThat(options.QueryTimeout > TimeSpan.Zero,
|
||||
$"Communication:QueryTimeout must be a positive duration (was {options.QueryTimeout}).");
|
||||
$"ScadaBridge:Communication:QueryTimeout must be a positive duration (was {options.QueryTimeout}).");
|
||||
|
||||
builder.RequireThat(options.IntegrationTimeout > TimeSpan.Zero,
|
||||
$"Communication:IntegrationTimeout must be a positive duration (was {options.IntegrationTimeout}).");
|
||||
$"ScadaBridge:Communication:IntegrationTimeout must be a positive duration (was {options.IntegrationTimeout}).");
|
||||
|
||||
builder.RequireThat(options.DebugViewTimeout > TimeSpan.Zero,
|
||||
$"Communication:DebugViewTimeout must be a positive duration (was {options.DebugViewTimeout}).");
|
||||
$"ScadaBridge:Communication:DebugViewTimeout must be a positive duration (was {options.DebugViewTimeout}).");
|
||||
|
||||
builder.RequireThat(options.HealthReportTimeout > TimeSpan.Zero,
|
||||
$"Communication:HealthReportTimeout must be a positive duration (was {options.HealthReportTimeout}).");
|
||||
$"ScadaBridge:Communication:HealthReportTimeout must be a positive duration (was {options.HealthReportTimeout}).");
|
||||
|
||||
builder.RequireThat(options.NotificationForwardTimeout > TimeSpan.Zero,
|
||||
$"Communication:NotificationForwardTimeout must be a positive duration (was {options.NotificationForwardTimeout}).");
|
||||
$"ScadaBridge:Communication:NotificationForwardTimeout must be a positive duration (was {options.NotificationForwardTimeout}).");
|
||||
|
||||
builder.RequireThat(options.GrpcKeepAlivePingDelay > TimeSpan.Zero,
|
||||
$"Communication:GrpcKeepAlivePingDelay must be a positive duration (was {options.GrpcKeepAlivePingDelay}).");
|
||||
$"ScadaBridge:Communication:GrpcKeepAlivePingDelay must be a positive duration (was {options.GrpcKeepAlivePingDelay}).");
|
||||
|
||||
builder.RequireThat(options.GrpcKeepAlivePingTimeout > TimeSpan.Zero,
|
||||
$"Communication:GrpcKeepAlivePingTimeout must be a positive duration (was {options.GrpcKeepAlivePingTimeout}).");
|
||||
$"ScadaBridge:Communication:GrpcKeepAlivePingTimeout must be a positive duration (was {options.GrpcKeepAlivePingTimeout}).");
|
||||
|
||||
builder.RequireThat(options.GrpcMaxStreamLifetime > TimeSpan.Zero,
|
||||
$"Communication:GrpcMaxStreamLifetime must be a positive duration (was {options.GrpcMaxStreamLifetime}).");
|
||||
$"ScadaBridge:Communication:GrpcMaxStreamLifetime must be a positive duration (was {options.GrpcMaxStreamLifetime}).");
|
||||
|
||||
builder.RequireThat(options.TransportHeartbeatInterval > TimeSpan.Zero,
|
||||
$"Communication:TransportHeartbeatInterval must be a positive duration (was {options.TransportHeartbeatInterval}).");
|
||||
$"ScadaBridge:Communication:TransportHeartbeatInterval must be a positive duration (was {options.TransportHeartbeatInterval}).");
|
||||
|
||||
builder.RequireThat(options.ApplicationHeartbeatInterval > TimeSpan.Zero,
|
||||
$"Communication:ApplicationHeartbeatInterval must be a positive duration (was {options.ApplicationHeartbeatInterval}).");
|
||||
$"ScadaBridge:Communication:ApplicationHeartbeatInterval must be a positive duration (was {options.ApplicationHeartbeatInterval}).");
|
||||
|
||||
builder.RequireThat(options.TransportFailureThreshold > TimeSpan.Zero,
|
||||
$"Communication:TransportFailureThreshold must be a positive duration (was {options.TransportFailureThreshold}).");
|
||||
$"ScadaBridge:Communication:TransportFailureThreshold must be a positive duration (was {options.TransportFailureThreshold}).");
|
||||
|
||||
builder.RequireThat(options.PendingDeploymentTtl > TimeSpan.Zero,
|
||||
$"Communication:PendingDeploymentTtl must be a positive duration (was {options.PendingDeploymentTtl}).");
|
||||
$"ScadaBridge:Communication:PendingDeploymentTtl must be a positive duration (was {options.PendingDeploymentTtl}).");
|
||||
|
||||
builder.RequireThat(options.PendingDeploymentPurgeInterval > TimeSpan.Zero,
|
||||
$"Communication:PendingDeploymentPurgeInterval must be a positive duration (was {options.PendingDeploymentPurgeInterval}).");
|
||||
$"ScadaBridge:Communication:PendingDeploymentPurgeInterval must be a positive duration (was {options.PendingDeploymentPurgeInterval}).");
|
||||
|
||||
builder.RequireThat(options.GrpcMaxConcurrentStreams > 0,
|
||||
$"Communication:GrpcMaxConcurrentStreams must be positive (was {options.GrpcMaxConcurrentStreams}).");
|
||||
$"ScadaBridge:Communication:GrpcMaxConcurrentStreams must be positive (was {options.GrpcMaxConcurrentStreams}).");
|
||||
|
||||
// ── Aggregated live alarm cache (plan #10, Task 6) ───────────────────────
|
||||
// Linger drives a Timer dueTime; TimeSpan.Zero is valid (stop the aggregator
|
||||
// immediately when the last viewer leaves), only a negative value is invalid.
|
||||
builder.RequireThat(options.LiveAlarmCacheLinger >= TimeSpan.Zero,
|
||||
$"Communication:LiveAlarmCacheLinger must be zero or a positive duration (was {options.LiveAlarmCacheLinger}).");
|
||||
$"ScadaBridge:Communication:LiveAlarmCacheLinger must be zero or a positive duration (was {options.LiveAlarmCacheLinger}).");
|
||||
|
||||
// Reconcile interval is a periodic timer cadence AND the start-retry cadence; a
|
||||
// zero/negative value would spin or never fire.
|
||||
builder.RequireThat(options.LiveAlarmCacheReconcileInterval > TimeSpan.Zero,
|
||||
$"Communication:LiveAlarmCacheReconcileInterval must be a positive duration (was {options.LiveAlarmCacheReconcileInterval}).");
|
||||
$"ScadaBridge:Communication:LiveAlarmCacheReconcileInterval must be a positive duration (was {options.LiveAlarmCacheReconcileInterval}).");
|
||||
|
||||
// Seed fan-out concurrency gates a SemaphoreSlim; at least 1, capped so a
|
||||
// fat-fingered config can't open an unbounded burst of cross-cluster Asks.
|
||||
builder.RequireThat(options.LiveAlarmCacheSeedConcurrency is >= 1 and <= 64,
|
||||
$"Communication:LiveAlarmCacheSeedConcurrency must be between 1 and 64 (was {options.LiveAlarmCacheSeedConcurrency}).");
|
||||
$"ScadaBridge:Communication:LiveAlarmCacheSeedConcurrency must be between 1 and 64 (was {options.LiveAlarmCacheSeedConcurrency}).");
|
||||
|
||||
// Per-site viewer cap must admit at least one viewer, else the page could never go live.
|
||||
builder.RequireThat(options.LiveAlarmCacheMaxSubscribersPerSite >= 1,
|
||||
$"Communication:LiveAlarmCacheMaxSubscribersPerSite must be at least 1 (was {options.LiveAlarmCacheMaxSubscribersPerSite}).");
|
||||
$"ScadaBridge:Communication:LiveAlarmCacheMaxSubscribersPerSite must be at least 1 (was {options.LiveAlarmCacheMaxSubscribersPerSite}).");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -12,8 +12,12 @@ public static class ServiceCollectionExtensions
|
||||
/// <returns>The same <see cref="IServiceCollection"/> to allow chaining.</returns>
|
||||
public static IServiceCollection AddCommunication(this IServiceCollection services)
|
||||
{
|
||||
// Canonical section is ScadaBridge:Communication — the section every real
|
||||
// appsettings actually writes (arch-review 08 round 2 NF8). The Host used to
|
||||
// supply the real values via a duplicate Configure<> binding; that duplicate is
|
||||
// gone, so AddCommunication must bind the canonical section itself.
|
||||
services.AddOptions<CommunicationOptions>()
|
||||
.BindConfiguration("Communication")
|
||||
.BindConfiguration("ScadaBridge:Communication")
|
||||
.ValidateOnStart();
|
||||
services.TryAddEnumerable(
|
||||
ServiceDescriptor.Singleton<IValidateOptions<CommunicationOptions>, CommunicationOptionsValidator>());
|
||||
|
||||
@@ -4,7 +4,7 @@ namespace ZB.MOM.WW.ScadaBridge.DataConnectionLayer;
|
||||
|
||||
/// <summary>
|
||||
/// Validates <see cref="DataConnectionOptions"/> at startup (ValidateOnStart) so a
|
||||
/// malformed "DataConnectionLayer" appsettings section fails fast at boot with a
|
||||
/// malformed "ScadaBridge:DataConnection" appsettings section fails fast at boot with a
|
||||
/// key-naming message instead of surfacing later at runtime. The intervals drive
|
||||
/// the reconnect/tag-resolution timers and the seed-read retry loop; a
|
||||
/// zero/negative duration (or a non-positive <c>SeedReadMaxAttempts</c>) would
|
||||
@@ -16,24 +16,24 @@ public sealed class DataConnectionOptionsValidator : OptionsValidatorBase<DataCo
|
||||
protected override void Validate(ValidationBuilder builder, DataConnectionOptions options)
|
||||
{
|
||||
builder.RequireThat(options.ReconnectInterval > TimeSpan.Zero,
|
||||
$"DataConnectionLayer:ReconnectInterval must be a positive duration (was {options.ReconnectInterval}).");
|
||||
$"ScadaBridge:DataConnection:ReconnectInterval must be a positive duration (was {options.ReconnectInterval}).");
|
||||
|
||||
builder.RequireThat(options.TagResolutionRetryInterval > TimeSpan.Zero,
|
||||
$"DataConnectionLayer:TagResolutionRetryInterval must be a positive duration (was {options.TagResolutionRetryInterval}).");
|
||||
$"ScadaBridge:DataConnection:TagResolutionRetryInterval must be a positive duration (was {options.TagResolutionRetryInterval}).");
|
||||
|
||||
builder.RequireThat(options.WriteTimeout > TimeSpan.Zero,
|
||||
$"DataConnectionLayer:WriteTimeout must be a positive duration (was {options.WriteTimeout}).");
|
||||
$"ScadaBridge:DataConnection:WriteTimeout must be a positive duration (was {options.WriteTimeout}).");
|
||||
|
||||
builder.RequireThat(options.SeedReadTimeout > TimeSpan.Zero,
|
||||
$"DataConnectionLayer:SeedReadTimeout must be a positive duration (was {options.SeedReadTimeout}).");
|
||||
$"ScadaBridge:DataConnection:SeedReadTimeout must be a positive duration (was {options.SeedReadTimeout}).");
|
||||
|
||||
builder.RequireThat(options.StableConnectionThreshold > TimeSpan.Zero,
|
||||
$"DataConnectionLayer:StableConnectionThreshold must be a positive duration (was {options.StableConnectionThreshold}).");
|
||||
$"ScadaBridge:DataConnection:StableConnectionThreshold must be a positive duration (was {options.StableConnectionThreshold}).");
|
||||
|
||||
builder.RequireThat(options.SeedReadRetryDelay > TimeSpan.Zero,
|
||||
$"DataConnectionLayer:SeedReadRetryDelay must be a positive duration (was {options.SeedReadRetryDelay}).");
|
||||
$"ScadaBridge:DataConnection:SeedReadRetryDelay must be a positive duration (was {options.SeedReadRetryDelay}).");
|
||||
|
||||
builder.RequireThat(options.SeedReadMaxAttempts > 0,
|
||||
$"DataConnectionLayer:SeedReadMaxAttempts must be positive (was {options.SeedReadMaxAttempts}).");
|
||||
$"ScadaBridge:DataConnection:SeedReadMaxAttempts must be positive (was {options.SeedReadMaxAttempts}).");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,8 +1,8 @@
|
||||
namespace ZB.MOM.WW.ScadaBridge.DataConnectionLayer;
|
||||
|
||||
/// <summary>
|
||||
/// Deployment-wide MxGateway defaults, bound from the "MxGateway" section of
|
||||
/// appsettings.json. Per-endpoint behavior lives on MxGatewayEndpointConfig.
|
||||
/// Deployment-wide MxGateway defaults, bound from the "ScadaBridge:MxGateway" section
|
||||
/// of appsettings.json. Per-endpoint behavior lives on MxGatewayEndpointConfig.
|
||||
/// </summary>
|
||||
public class MxGatewayGlobalOptions
|
||||
{
|
||||
|
||||
@@ -1,8 +1,8 @@
|
||||
namespace ZB.MOM.WW.ScadaBridge.DataConnectionLayer;
|
||||
|
||||
/// <summary>
|
||||
/// Deployment-wide OPC UA application identity. Bound from the "OpcUa" section
|
||||
/// of appsettings.json. Per-endpoint behavior lives on OpcUaEndpointConfig.
|
||||
/// Deployment-wide OPC UA application identity. Bound from the "ScadaBridge:OpcUa"
|
||||
/// section of appsettings.json. Per-endpoint behavior lives on OpcUaEndpointConfig.
|
||||
/// Empty paths fall back to a default under Path.GetTempPath() so dev runs
|
||||
/// work without explicit configuration.
|
||||
/// </summary>
|
||||
|
||||
@@ -14,20 +14,26 @@ public static class ServiceCollectionExtensions
|
||||
/// <returns>The same <paramref name="services"/> instance for chaining.</returns>
|
||||
public static IServiceCollection AddDataConnectionLayer(this IServiceCollection services)
|
||||
{
|
||||
// Canonical section is ScadaBridge:DataConnection — the section every real
|
||||
// appsettings actually writes (arch-review 08 round 2 NF8). The Host used to
|
||||
// supply the real values via a duplicate Configure<> binding; that duplicate is
|
||||
// gone, so AddDataConnectionLayer must bind the canonical section itself.
|
||||
services.AddOptions<DataConnectionOptions>()
|
||||
.BindConfiguration("DataConnectionLayer")
|
||||
.BindConfiguration("ScadaBridge:DataConnection")
|
||||
.ValidateOnStart();
|
||||
services.TryAddEnumerable(
|
||||
ServiceDescriptor.Singleton<IValidateOptions<DataConnectionOptions>, DataConnectionOptionsValidator>());
|
||||
|
||||
// OpcUaGlobalOptions / MxGatewayGlobalOptions carry only string identity/path
|
||||
// fields (no numeric or duration knobs), so there is nothing to range-validate
|
||||
// — no dedicated validator is registered for them.
|
||||
// — no dedicated validator is registered for them. Canonical sections are
|
||||
// ScadaBridge:OpcUa / ScadaBridge:MxGateway (NF8): the bare "OpcUa"/"MxGateway"
|
||||
// sections exist in no appsettings, so binding them read nothing.
|
||||
services.AddOptions<OpcUaGlobalOptions>()
|
||||
.BindConfiguration("OpcUa");
|
||||
.BindConfiguration("ScadaBridge:OpcUa");
|
||||
|
||||
services.AddOptions<MxGatewayGlobalOptions>()
|
||||
.BindConfiguration("MxGateway");
|
||||
.BindConfiguration("ScadaBridge:MxGateway");
|
||||
|
||||
// Register the factory for protocol extensibility
|
||||
services.AddSingleton<IDataConnectionFactory, DataConnectionFactory>();
|
||||
|
||||
@@ -149,7 +149,9 @@ public static class SiteServiceRegistration
|
||||
ServiceDescriptor.Singleton<IValidateOptions<SiteRuntime.Tracking.OperationTrackingOptions>,
|
||||
SiteRuntime.Tracking.OperationTrackingOptionsValidator>());
|
||||
|
||||
services.Configure<DataConnectionOptions>(config.GetSection("ScadaBridge:DataConnection"));
|
||||
// NF8: DataConnectionOptions is now bound by AddDataConnectionLayer() itself
|
||||
// (canonical ScadaBridge:DataConnection section) — the duplicate Host binding
|
||||
// that used to mask the SCE's wrong section name is deleted.
|
||||
|
||||
services.AddOptions<StoreAndForwardOptions>()
|
||||
.Bind(config.GetSection("ScadaBridge:StoreAndForward"))
|
||||
@@ -184,12 +186,17 @@ public static class SiteServiceRegistration
|
||||
services.AddOptions<DatabaseOptions>().Bind(config.GetSection("ScadaBridge:Database")).ValidateOnStart();
|
||||
services.TryAddEnumerable(
|
||||
ServiceDescriptor.Singleton<IValidateOptions<DatabaseOptions>, DatabaseOptionsValidator>());
|
||||
services.Configure<CommunicationOptions>(config.GetSection("ScadaBridge:Communication"));
|
||||
// NF8: CommunicationOptions is now bound by AddCommunication() itself (canonical
|
||||
// ScadaBridge:Communication section) — the duplicate Host binding that used to mask
|
||||
// the SCE's wrong section name is deleted.
|
||||
// Bind + eagerly validate: HealthMonitoringOptionsValidator is registered (TryAddEnumerable)
|
||||
// by the HealthMonitoring module, so chaining ValidateOnStart() here makes a bad
|
||||
// ScadaBridge:HealthMonitoring section fail fast at host build instead of lazily on first resolve.
|
||||
services.AddOptions<HealthMonitoringOptions>().Bind(config.GetSection("ScadaBridge:HealthMonitoring")).ValidateOnStart();
|
||||
services.Configure<NotificationOptions>(config.GetSection("ScadaBridge:Notification"));
|
||||
// NF8: NotificationOptions is bound by AddNotificationService() (central-only,
|
||||
// canonical ScadaBridge:Notification section). Sites don't deliver notifications
|
||||
// and have no IOptions<NotificationOptions> consumer, so the redundant Host
|
||||
// duplicate that bound it on both paths is deleted.
|
||||
// Bind + eagerly validate: an unrecognised MinimumLevel would silently fall back
|
||||
// to Information; fail fast at boot so the operator's intended floor is honoured
|
||||
// (arch-review 08r2 NF4).
|
||||
|
||||
@@ -0,0 +1,30 @@
|
||||
using Microsoft.Extensions.Configuration;
|
||||
using Microsoft.Extensions.DependencyInjection;
|
||||
using Microsoft.Extensions.Options;
|
||||
|
||||
namespace ZB.MOM.WW.ScadaBridge.Communication.Tests;
|
||||
|
||||
/// <summary>
|
||||
/// Locks the config section <see cref="ServiceCollectionExtensions.AddCommunication"/>
|
||||
/// binds (arch-review 08 round 2 NF8). Every real appsettings nests this section under
|
||||
/// <c>ScadaBridge:</c>; binding a root-level <c>Communication</c> section reads config
|
||||
/// that no deployment writes — the drift the Host duplicate was silently masking.
|
||||
/// AddCommunication alone must bind the canonical <c>ScadaBridge:Communication</c> section.
|
||||
/// </summary>
|
||||
public class CommunicationOptionsBindingTests
|
||||
{
|
||||
[Fact]
|
||||
public void AddCommunication_BindsTheScadaBridgeCommunicationSection()
|
||||
{
|
||||
var config = new ConfigurationBuilder().AddInMemoryCollection(new Dictionary<string, string?>
|
||||
{
|
||||
["ScadaBridge:Communication:DeploymentTimeout"] = "00:01:23",
|
||||
}).Build();
|
||||
var services = new ServiceCollection();
|
||||
services.AddSingleton<IConfiguration>(config);
|
||||
services.AddCommunication();
|
||||
using var sp = services.BuildServiceProvider();
|
||||
Assert.Equal(TimeSpan.FromSeconds(83),
|
||||
sp.GetRequiredService<IOptions<CommunicationOptions>>().Value.DeploymentTimeout);
|
||||
}
|
||||
}
|
||||
+30
@@ -0,0 +1,30 @@
|
||||
using Microsoft.Extensions.Configuration;
|
||||
using Microsoft.Extensions.DependencyInjection;
|
||||
using Microsoft.Extensions.Options;
|
||||
|
||||
namespace ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests;
|
||||
|
||||
/// <summary>
|
||||
/// Locks the config section <see cref="ServiceCollectionExtensions.AddDataConnectionLayer"/>
|
||||
/// binds (arch-review 08 round 2 NF8). Every real appsettings nests this section under
|
||||
/// <c>ScadaBridge:</c>; binding a root-level <c>DataConnectionLayer</c> section reads config
|
||||
/// that no deployment writes — the drift the Host duplicate was silently masking.
|
||||
/// AddDataConnectionLayer alone must bind the canonical <c>ScadaBridge:DataConnection</c> section.
|
||||
/// </summary>
|
||||
public class DataConnectionOptionsBindingTests
|
||||
{
|
||||
[Fact]
|
||||
public void AddDataConnectionLayer_BindsTheScadaBridgeDataConnectionSection()
|
||||
{
|
||||
var config = new ConfigurationBuilder().AddInMemoryCollection(new Dictionary<string, string?>
|
||||
{
|
||||
["ScadaBridge:DataConnection:ReconnectInterval"] = "00:00:42",
|
||||
}).Build();
|
||||
var services = new ServiceCollection();
|
||||
services.AddSingleton<IConfiguration>(config);
|
||||
services.AddDataConnectionLayer();
|
||||
using var sp = services.BuildServiceProvider();
|
||||
Assert.Equal(TimeSpan.FromSeconds(42),
|
||||
sp.GetRequiredService<IOptions<DataConnectionOptions>>().Value.ReconnectInterval);
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user