From 3543a7f295958b4abbba5152b5b788c92a98597d Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 22 Oct 2026 06:29:05 -0400 Subject: [PATCH] fix(dcl): feed ScadaBridge:MxGateway:ClientNamePrefix into the MxGateway client-name fallback (review 2026-10-21 02/U1, 08/O2) --- .../Component-DataConnectionLayer.md | 2 +- .../Adapters/MxGatewayDataConnection.cs | 14 ++++++++-- .../DataConnectionFactory.cs | 10 +++++-- .../MxGatewayGlobalOptions.cs | 6 ++++- .../Adapters/MxGatewayDataConnectionTests.cs | 19 ++++++++++++++ .../GlobalOptionsValidationTests.cs | 26 +++++++++++++++++++ 6 files changed, 71 insertions(+), 6 deletions(-) diff --git a/docs/requirements/Component-DataConnectionLayer.md b/docs/requirements/Component-DataConnectionLayer.md index 857cffa6..6760dcd1 100644 --- a/docs/requirements/Component-DataConnectionLayer.md +++ b/docs/requirements/Component-DataConnectionLayer.md @@ -189,7 +189,7 @@ All settings are parsed from the data connection's configuration JSON dictionari |-----|------|---------|-------------| | `Endpoint` | string | `http://localhost:5000` | Gateway base URL | | `ApiKey` | string | — | Sent to the gateway as `authorization: Bearer ` | -| `ClientName` | string | `scadabridge` (when blank) | MXAccess client registration name | +| `ClientName` | string | `ScadaBridge:MxGateway:ClientNamePrefix` (when blank; that option defaults to `scadabridge`) | MXAccess client registration name. The site-wide `ClientNamePrefix` is the fallback for every connection that leaves this blank — e.g. to tell sites apart on a shared gateway; it must not be blank (validated at startup). | | `WriteUserId` | int | `0` | MXAccess user id applied to every write-back (0 = no user context) | | `UseTls` | bool | `false` | Use TLS to a secured gateway | | `CaFile` | string | — | Path to the CA certificate (TLS only) | diff --git a/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Adapters/MxGatewayDataConnection.cs b/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Adapters/MxGatewayDataConnection.cs index 76811576..356f3a5c 100644 --- a/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Adapters/MxGatewayDataConnection.cs +++ b/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Adapters/MxGatewayDataConnection.cs @@ -29,6 +29,7 @@ public class MxGatewayDataConnection private readonly ILogger _logger; private readonly ISecretResolver? _secretResolver; private readonly int _supervisoryAdviseParallelism; + private readonly string _clientNamePrefix; private IMxGatewayClient? _client; private ConnectionHealth _status = ConnectionHealth.Disconnected; private CancellationTokenSource? _eventLoopCts; @@ -79,16 +80,25 @@ public class MxGatewayDataConnection /// Maximum in-flight supervisory advise commands on the bulk-subscribe path when the /// endpoint has no write-user context (DataConnectionOptions.MxSupervisoryAdviseParallelism). /// + /// + /// Gateway client registration name used when the connection's own ClientName is blank + /// (ScadaBridge:MxGateway:ClientNamePrefix, default scadabridge). Null or blank + /// keeps the scadabridge default. + /// public MxGatewayDataConnection( IMxGatewayClientFactory clientFactory, ILogger logger, ISecretResolver? secretResolver = null, - int supervisoryAdviseParallelism = 16) + int supervisoryAdviseParallelism = 16, + string? clientNamePrefix = null) { _clientFactory = clientFactory; _logger = logger; _secretResolver = secretResolver; _supervisoryAdviseParallelism = Math.Max(1, supervisoryAdviseParallelism); + _clientNamePrefix = string.IsNullOrWhiteSpace(clientNamePrefix) + ? new MxGatewayGlobalOptions().ClientNamePrefix + : clientNamePrefix.Trim(); } /// @@ -177,7 +187,7 @@ public class MxGatewayDataConnection await _client.ConnectAsync(new MxGatewayConnectionOptions( cfg.Endpoint, apiKey, - string.IsNullOrWhiteSpace(cfg.ClientName) ? "scadabridge" : cfg.ClientName, + string.IsNullOrWhiteSpace(cfg.ClientName) ? _clientNamePrefix : cfg.ClientName, cfg.WriteUserId, cfg.UseTls, string.IsNullOrWhiteSpace(cfg.CaFile) ? null : cfg.CaFile, diff --git a/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/DataConnectionFactory.cs b/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/DataConnectionFactory.cs index 44e033bc..ad16e437 100644 --- a/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/DataConnectionFactory.cs +++ b/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/DataConnectionFactory.cs @@ -35,11 +35,13 @@ public class DataConnectionFactory : IDataConnectionFactory /// construction without it keeps literal-key behaviour. /// /// Optional data connection options, such as the MxGateway supervisory-advise parallelism; defaults apply when null. + /// Optional MxGateway global options (the client-name fallback); defaults apply when null. public DataConnectionFactory( ILoggerFactory loggerFactory, IOptions opcUaGlobalOptions, ISecretResolver? secretResolver = null, - IOptions? dataConnectionOptions = null) + IOptions? dataConnectionOptions = null, + IOptions? mxGatewayGlobalOptions = null) { _loggerFactory = loggerFactory; _secretResolver = secretResolver; @@ -48,6 +50,9 @@ public class DataConnectionFactory : IDataConnectionFactory // DataConnectionOptions is consumed by the actor, which is constructed elsewhere. var supervisoryAdviseParallelism = (dataConnectionOptions?.Value ?? new DataConnectionOptions()).MxSupervisoryAdviseParallelism; + // The client registration name a connection with a blank ClientName falls back to + // (review 2026-10-21 02/U1 — the option was bound and validated but never read). + var mxClientNamePrefix = (mxGatewayGlobalOptions?.Value ?? new MxGatewayGlobalOptions()).ClientNamePrefix; // Register built-in protocols. // Pass the ILoggerFactory into RealOpcUaClientFactory so @@ -64,7 +69,8 @@ public class DataConnectionFactory : IDataConnectionFactory new RealMxGatewayClientFactory(_loggerFactory), _loggerFactory.CreateLogger(), _secretResolver, - supervisoryAdviseParallelism)); + supervisoryAdviseParallelism, + mxClientNamePrefix)); } /// diff --git a/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/MxGatewayGlobalOptions.cs b/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/MxGatewayGlobalOptions.cs index 464f6782..2e6be841 100644 --- a/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/MxGatewayGlobalOptions.cs +++ b/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/MxGatewayGlobalOptions.cs @@ -6,6 +6,10 @@ namespace ZB.MOM.WW.ScadaBridge.DataConnectionLayer; /// public class MxGatewayGlobalOptions { - /// Prefix used to derive a per-connection client registration name when the connection's ClientName is blank. + /// + /// Gateway client registration name used by any MxGateway connection whose own ClientName is + /// blank (e.g. set per site to tell sites apart on a shared gateway). The default keeps the + /// historical scadabridge name. + /// public string ClientNamePrefix { get; set; } = "scadabridge"; } diff --git a/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/Adapters/MxGatewayDataConnectionTests.cs b/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/Adapters/MxGatewayDataConnectionTests.cs index e1a8f040..d172f537 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/Adapters/MxGatewayDataConnectionTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/Adapters/MxGatewayDataConnectionTests.cs @@ -59,6 +59,25 @@ public class MxGatewayDataConnectionTests Assert.Equal("scadabridge", fake.ConnectedWith!.ClientName); } + /// + /// Review 2026-10-21 02/U1: a blank per-connection ClientName falls back to the configured + /// ScadaBridge:MxGateway:ClientNamePrefix rather than a hard-coded literal. + /// + /// A task that represents the asynchronous operation. + [Fact] + public async Task ConnectAsync_blank_client_name_uses_configured_prefix() + { + var fake = new FakeMxGatewayClient(); + var adapter = new MxGatewayDataConnection( + fake, NullLogger.Instance, clientNamePrefix: "plant7-scadabridge"); + var details = Details(); + details["ClientName"] = " "; + + await adapter.ConnectAsync(details); + + Assert.Equal("plant7-scadabridge", fake.ConnectedWith!.ClientName); + } + /// Verifies that the Disconnected event fires exactly once when the event loop faults. /// A task that represents the asynchronous operation. [Fact] diff --git a/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/GlobalOptionsValidationTests.cs b/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/GlobalOptionsValidationTests.cs index 6b455baf..97214ed2 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/GlobalOptionsValidationTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/GlobalOptionsValidationTests.cs @@ -1,6 +1,9 @@ using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Options; +using ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Adapters; namespace ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests; @@ -32,4 +35,27 @@ public class GlobalOptionsValidationTests using var provider = services.BuildServiceProvider(); provider.GetRequiredService().Validate(); } + + /// + /// Review 2026-10-21 02/U1: the bound ScadaBridge:MxGateway:ClientNamePrefix actually + /// reaches the MxGateway adapters the DI-built factory creates. + /// + [Fact] + public void ClientNamePrefix_FlowsIntoTheMxGatewayAdapter() + { + var services = new ServiceCollection(); + services.AddSingleton(new ConfigurationBuilder().AddInMemoryCollection( + new Dictionary { ["ScadaBridge:MxGateway:ClientNamePrefix"] = "plant7" }).Build()); + services.AddSingleton(NullLoggerFactory.Instance); + services.AddDataConnectionLayer(); + using var provider = services.BuildServiceProvider(); + + var adapter = provider.GetRequiredService() + .Create("MxGateway", new Dictionary()); + + var prefix = typeof(MxGatewayDataConnection) + .GetField("_clientNamePrefix", System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic)! + .GetValue(adapter); + Assert.Equal("plant7", prefix); + } }