diff --git a/ZB.MOM.WW.OtOpcUa.slnx b/ZB.MOM.WW.OtOpcUa.slnx index 4e577d11..c5a50698 100644 --- a/ZB.MOM.WW.OtOpcUa.slnx +++ b/ZB.MOM.WW.OtOpcUa.slnx @@ -49,6 +49,7 @@ + diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Browser/MqttDriverBrowser.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Browser/MqttDriverBrowser.cs index 911a87aa..df8335e0 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Browser/MqttDriverBrowser.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Browser/MqttDriverBrowser.cs @@ -23,12 +23,12 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Browser; /// sanctioned publish, and it is never reached by OpenAsync or by any browse call. /// /// -/// Layering note. Unlike every other bespoke browser here, this project references the -/// runtime .Driver project (to reuse MqttConnection.BuildClientOptions rather -/// than fork the TLS / CA-pin path). That is a known, deliberate exception with a documented -/// cost and a documented clean fix — see the comment on that ProjectReference in -/// ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Browser.csproj before copying this project as a -/// template. +/// Layering note. This project depends on the shared leaves only — .Contracts for +/// the Sparkplug wire schema and codec, .Transport for +/// — and NOT on the runtime .Driver project, which +/// it used to reference under a documented exception (Gitea #512). Reuse of the TLS / CA-pin path +/// is what that exception bought, and it is preserved: there is still exactly one copy, now in +/// .Transport. Do not fork it. Pinned by MqttBrowserLayeringTests. /// /// public sealed class MqttDriverBrowser : IDriverBrowser @@ -73,7 +73,7 @@ public sealed class MqttDriverBrowser : IDriverBrowser var filter = BuildRootFilter(opts); var suffix = BuildBrowseClientIdSuffix(); - var clientOptions = MqttConnection.BuildClientOptions(ToBrowseOptions(opts), suffix, _logger); + var clientOptions = MqttClientOptionsFactory.BuildClientOptions(ToBrowseOptions(opts), suffix, _logger); using var openCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); openCts.CancelAfter(OpenBudget(opts)); diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Browser/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Browser.csproj b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Browser/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Browser.csproj index e32fa07f..44cb9c98 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Browser/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Browser.csproj +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Browser/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Browser.csproj @@ -14,34 +14,36 @@ - + diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/Sparkplug/RebirthRequester.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Contracts/Sparkplug/RebirthRequester.cs similarity index 100% rename from src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/Sparkplug/RebirthRequester.cs rename to src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Contracts/Sparkplug/RebirthRequester.cs diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/Sparkplug/SparkplugCodec.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Contracts/Sparkplug/SparkplugCodec.cs similarity index 100% rename from src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/Sparkplug/SparkplugCodec.cs rename to src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Contracts/Sparkplug/SparkplugCodec.cs diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/Sparkplug/SparkplugTopic.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Contracts/Sparkplug/SparkplugTopic.cs similarity index 100% rename from src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/Sparkplug/SparkplugTopic.cs rename to src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Contracts/Sparkplug/SparkplugTopic.cs diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Transport/MqttClientOptionsFactory.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Transport/MqttClientOptionsFactory.cs new file mode 100644 index 00000000..4e4e9278 --- /dev/null +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Transport/MqttClientOptionsFactory.cs @@ -0,0 +1,244 @@ +using System.Net.Security; +using System.Security.Cryptography; +using System.Security.Cryptography.X509Certificates; +using Microsoft.Extensions.Logging; +using MQTTnet; + +namespace ZB.MOM.WW.OtOpcUa.Driver.Mqtt; + +/// +/// Assembles the MQTTnet client options for every broker connection this repo opens — the runtime +/// driver's, the browse session's and the Test-connect probe's alike. +/// +/// +/// +/// Why this is its own type in its own assembly (Gitea #512). It owns the security posture of +/// an MQTT connection: whether TLS is on, whether an untrusted certificate is tolerated, and — when a +/// PEM CA is pinned — the custom chain validator that replaces the OS trust anchor. Both the runtime +/// driver and the browse session must connect with identical semantics, and the failure mode +/// of a second copy is not a crash but a silent divergence: browse quietly not honouring a pinned CA +/// while the driver does. Previously this lived on MqttConnection and .Browser took a +/// documented layering exception to reach it; the exception is gone, and this is the one copy. +/// +/// +/// Pure. No I/O, no network, nothing read from disk — so it is safe to call on a +/// config-validation path. A configured is read +/// lazily by the returned validation callback at handshake time, not here. +/// +/// +public static class MqttClientOptionsFactory +{ + /// + /// Assembles the MQTTnet client options from . Pure — it performs + /// no I/O and touches no network, so it is safe to call on a config-validation path. A + /// configured is read lazily by the + /// returned certificate-validation callback at handshake time, not here. + /// + /// Broker connection settings. + /// + /// Appended to , so a browse/probe session can share + /// the configured identity without colliding with the driver's own client id. When both are + /// empty MQTTnet generates a random client id. + /// + /// Optional logger for the deferred CA-pin load; never receives credentials. + public static MqttClientOptions BuildClientOptions( + MqttDriverOptions options, + string? clientIdSuffix, + ILogger? logger = null) + { + ArgumentNullException.ThrowIfNull(options); + + var builder = new MqttClientOptionsBuilder() + .WithTcpServer(options.Host, options.Port) + .WithProtocolVersion(MapProtocolVersion(options.ProtocolVersion)) + .WithCleanSession(options.CleanSession) + .WithKeepAlivePeriod(TimeSpan.FromSeconds(options.KeepAliveSeconds)) + .WithTimeout(TimeSpan.FromSeconds(options.ConnectTimeoutSeconds)); + + var clientId = string.Concat(options.ClientId ?? string.Empty, clientIdSuffix ?? string.Empty); + if (!string.IsNullOrWhiteSpace(clientId)) + { + builder = builder.WithClientId(clientId); + } + + // An empty username means "connect anonymously"; MQTTnet would otherwise send an empty + // CONNECT username, which some brokers treat as a failed auth rather than as anonymous. + if (!string.IsNullOrEmpty(options.Username)) + { + builder = builder.WithCredentials(options.Username, options.Password ?? string.Empty); + } + + builder = builder.WithTlsOptions(tls => ConfigureTls(tls, options, logger)); + + return builder.Build(); + } + + private static void ConfigureTls(MqttClientTlsOptionsBuilder tls, MqttDriverOptions options, ILogger? logger) + { + if (!options.UseTls) + { + tls.UseTls(false); + return; + } + + tls.UseTls(true) + // Explicit SNI / hostname-verification target. Left unset, name validation depends on + // how MQTTnet infers the host from the endpoint — pin it to the configured host. + .WithTargetHost(options.Host); + + if (options.AllowUntrustedServerCertificate) + { + // Dev / on-prem escape hatch, off unless explicitly enabled — mirrors the + // ServerHistorian TLS knobs. This is the ONLY branch that accepts a bad certificate. + tls.WithAllowUntrustedCertificates(true) + .WithIgnoreCertificateChainErrors(true) + .WithCertificateValidationHandler(static _ => true); + return; + } + + if (string.IsNullOrWhiteSpace(options.CaCertificatePath)) + { + // No pin: MQTTnet's default handler validates against the OS trust store. + return; + } + + tls.WithCertificateValidationHandler(CreatePinnedCaValidator(options.CaCertificatePath, logger)); + } + + /// + /// Builds a certificate-validation callback that pins the broker chain to the PEM CA at + /// . The file is loaded lazily on first validation + /// (i.e. during the TLS handshake) so that this — and therefore + /// — stays free of I/O. Any failure to load or to chain + /// up to the pinned CA rejects the certificate: fail closed. + /// + private static Func CreatePinnedCaValidator( + string caCertificatePath, + ILogger? logger) + { + var trustedRoots = new Lazy( + () => LoadPemCa(caCertificatePath, logger), + LazyThreadSafetyMode.ExecutionAndPublication); + + return args => ValidateAgainstPinnedCa(args, trustedRoots.Value, caCertificatePath, logger); + } + + private static X509Certificate2Collection? LoadPemCa(string caCertificatePath, ILogger? logger) + { + try + { + var collection = new X509Certificate2Collection(); + collection.ImportFromPemFile(caCertificatePath); + if (collection.Count != 0) + { + return collection; + } + + logger?.LogError("MQTT CA pin '{CaCertificatePath}' contains no certificates; broker TLS will be rejected.", caCertificatePath); + } + catch (Exception ex) + { + logger?.LogError(ex, "MQTT CA pin '{CaCertificatePath}' could not be loaded; broker TLS will be rejected.", caCertificatePath); + } + + return null; + } + + private static bool ValidateAgainstPinnedCa( + MqttClientCertificateValidationEventArgs args, + X509Certificate2Collection? trustedRoots, + string caCertificatePath, + ILogger? logger) + { + if (trustedRoots is null || trustedRoots.Count == 0) + { + // Unreadable pin ⇒ no trust anchor ⇒ refuse. Never silently degrade to the OS store. + return false; + } + + // The pin replaces the trust anchor only. Anything else the platform flagged — a hostname + // mismatch, an absent certificate — remains fatal. + if ((args.SslPolicyErrors & ~SslPolicyErrors.RemoteCertificateChainErrors) != SslPolicyErrors.None) + { + logger?.LogError("MQTT broker certificate rejected: {SslPolicyErrors}.", args.SslPolicyErrors); + return false; + } + + if (args.Certificate is null) + { + return false; + } + + var presented = args.Certificate as X509Certificate2; + X509Certificate2? owned = null; + if (presented is null) + { + owned = X509CertificateLoader.LoadCertificate(args.Certificate.Export(X509ContentType.Cert)); + presented = owned; + } + + try + { + using var chain = new X509Chain(); + chain.ChainPolicy.TrustMode = X509ChainTrustMode.CustomRootTrust; + chain.ChainPolicy.CustomTrustStore.AddRange(trustedRoots); + + // Revocation is deliberately not checked: the pin exists precisely for self-issued + // dev / on-prem CAs, which typically publish no CRL or OCSP responder, so checking + // would fail every such chain. The pin itself is the revocation story — remove the CA. + chain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck; + + // Seed the intermediates the broker presented during the handshake. A leaf signed by + // an intermediate that chains up to the pinned root is a common broker topology, and + // that intermediate arrives on the wire rather than being installed locally — without + // it in the candidate pool a legitimate trust path simply cannot be built. + // CustomRootTrust still means only `trustedRoots` may terminate the chain, so a rogue + // self-signed cert smuggled in here cannot become an anchor. + // Both sources are read: platforms differ in whether the peer-supplied intermediates + // land in the incoming chain's elements, its ExtraStore, or both. + if (args.Chain is not null) + { + foreach (var element in args.Chain.ChainElements) + { + chain.ChainPolicy.ExtraStore.Add(element.Certificate); + } + + chain.ChainPolicy.ExtraStore.AddRange(args.Chain.ChainPolicy.ExtraStore); + } + + if (chain.Build(presented)) + { + return true; + } + + logger?.LogError( + "MQTT broker certificate does not chain to the pinned CA '{CaCertificatePath}': {ChainStatus}.", + caCertificatePath, + string.Join(", ", chain.ChainStatus.Select(s => s.Status))); + return false; + } + catch (CryptographicException ex) + { + // X509Chain.Build throws on a malformed certificate or an unusable policy. An exception + // escaping a TLS validation callback surfaces as an opaque handshake crash, so classify + // it here and refuse: an unverifiable certificate is a rejected certificate. + logger?.LogError( + ex, + "MQTT broker certificate could not be validated against the pinned CA '{CaCertificatePath}'; rejecting.", + caCertificatePath); + return false; + } + finally + { + owned?.Dispose(); + } + } + + private static MQTTnet.Formatter.MqttProtocolVersion MapProtocolVersion(MqttProtocolVersion version) + => version switch + { + MqttProtocolVersion.V311 => MQTTnet.Formatter.MqttProtocolVersion.V311, + MqttProtocolVersion.V500 => MQTTnet.Formatter.MqttProtocolVersion.V500, + _ => throw new ArgumentOutOfRangeException(nameof(version), version, "Unsupported MQTT protocol version."), + }; +} diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Transport/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Transport.csproj b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Transport/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Transport.csproj new file mode 100644 index 00000000..9d2b4e39 --- /dev/null +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Transport/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Transport.csproj @@ -0,0 +1,40 @@ + + + + net10.0 + enable + enable + latest + true + ZB.MOM.WW.OtOpcUa.Driver.Mqtt + ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Transport + + + + + + + + + + + + + diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttConnection.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttConnection.cs index 1733fefc..6582de2e 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttConnection.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttConnection.cs @@ -568,7 +568,7 @@ public sealed class MqttConnection : IAsyncDisposable, IMqttSubscribeTransport, MqttClientOptions clientOptions; try { - clientOptions = BuildClientOptions(_options, clientIdSuffix: null, _logger); + clientOptions = MqttClientOptionsFactory.BuildClientOptions(_options, clientIdSuffix: null, _logger); } catch (Exception ex) { @@ -1345,217 +1345,4 @@ public sealed class MqttConnection : IAsyncDisposable, IMqttSubscribeTransport, } } - /// - /// Assembles the MQTTnet client options from . Pure — it performs - /// no I/O and touches no network, so it is safe to call on a config-validation path. A - /// configured is read lazily by the - /// returned certificate-validation callback at handshake time, not here. - /// - /// Broker connection settings. - /// - /// Appended to , so a browse/probe session can share - /// the configured identity without colliding with the driver's own client id. When both are - /// empty MQTTnet generates a random client id. - /// - /// Optional logger for the deferred CA-pin load; never receives credentials. - public static MqttClientOptions BuildClientOptions( - MqttDriverOptions options, - string? clientIdSuffix, - ILogger? logger = null) - { - ArgumentNullException.ThrowIfNull(options); - - var builder = new MqttClientOptionsBuilder() - .WithTcpServer(options.Host, options.Port) - .WithProtocolVersion(MapProtocolVersion(options.ProtocolVersion)) - .WithCleanSession(options.CleanSession) - .WithKeepAlivePeriod(TimeSpan.FromSeconds(options.KeepAliveSeconds)) - .WithTimeout(TimeSpan.FromSeconds(options.ConnectTimeoutSeconds)); - - var clientId = string.Concat(options.ClientId ?? string.Empty, clientIdSuffix ?? string.Empty); - if (!string.IsNullOrWhiteSpace(clientId)) - { - builder = builder.WithClientId(clientId); - } - - // An empty username means "connect anonymously"; MQTTnet would otherwise send an empty - // CONNECT username, which some brokers treat as a failed auth rather than as anonymous. - if (!string.IsNullOrEmpty(options.Username)) - { - builder = builder.WithCredentials(options.Username, options.Password ?? string.Empty); - } - - builder = builder.WithTlsOptions(tls => ConfigureTls(tls, options, logger)); - - return builder.Build(); - } - - private static void ConfigureTls(MqttClientTlsOptionsBuilder tls, MqttDriverOptions options, ILogger? logger) - { - if (!options.UseTls) - { - tls.UseTls(false); - return; - } - - tls.UseTls(true) - // Explicit SNI / hostname-verification target. Left unset, name validation depends on - // how MQTTnet infers the host from the endpoint — pin it to the configured host. - .WithTargetHost(options.Host); - - if (options.AllowUntrustedServerCertificate) - { - // Dev / on-prem escape hatch, off unless explicitly enabled — mirrors the - // ServerHistorian TLS knobs. This is the ONLY branch that accepts a bad certificate. - tls.WithAllowUntrustedCertificates(true) - .WithIgnoreCertificateChainErrors(true) - .WithCertificateValidationHandler(static _ => true); - return; - } - - if (string.IsNullOrWhiteSpace(options.CaCertificatePath)) - { - // No pin: MQTTnet's default handler validates against the OS trust store. - return; - } - - tls.WithCertificateValidationHandler(CreatePinnedCaValidator(options.CaCertificatePath, logger)); - } - - /// - /// Builds a certificate-validation callback that pins the broker chain to the PEM CA at - /// . The file is loaded lazily on first validation - /// (i.e. during the TLS handshake) so that this — and therefore - /// — stays free of I/O. Any failure to load or to chain - /// up to the pinned CA rejects the certificate: fail closed. - /// - private static Func CreatePinnedCaValidator( - string caCertificatePath, - ILogger? logger) - { - var trustedRoots = new Lazy( - () => LoadPemCa(caCertificatePath, logger), - LazyThreadSafetyMode.ExecutionAndPublication); - - return args => ValidateAgainstPinnedCa(args, trustedRoots.Value, caCertificatePath, logger); - } - - private static X509Certificate2Collection? LoadPemCa(string caCertificatePath, ILogger? logger) - { - try - { - var collection = new X509Certificate2Collection(); - collection.ImportFromPemFile(caCertificatePath); - if (collection.Count != 0) - { - return collection; - } - - logger?.LogError("MQTT CA pin '{CaCertificatePath}' contains no certificates; broker TLS will be rejected.", caCertificatePath); - } - catch (Exception ex) - { - logger?.LogError(ex, "MQTT CA pin '{CaCertificatePath}' could not be loaded; broker TLS will be rejected.", caCertificatePath); - } - - return null; - } - - private static bool ValidateAgainstPinnedCa( - MqttClientCertificateValidationEventArgs args, - X509Certificate2Collection? trustedRoots, - string caCertificatePath, - ILogger? logger) - { - if (trustedRoots is null || trustedRoots.Count == 0) - { - // Unreadable pin ⇒ no trust anchor ⇒ refuse. Never silently degrade to the OS store. - return false; - } - - // The pin replaces the trust anchor only. Anything else the platform flagged — a hostname - // mismatch, an absent certificate — remains fatal. - if ((args.SslPolicyErrors & ~SslPolicyErrors.RemoteCertificateChainErrors) != SslPolicyErrors.None) - { - logger?.LogError("MQTT broker certificate rejected: {SslPolicyErrors}.", args.SslPolicyErrors); - return false; - } - - if (args.Certificate is null) - { - return false; - } - - var presented = args.Certificate as X509Certificate2; - X509Certificate2? owned = null; - if (presented is null) - { - owned = X509CertificateLoader.LoadCertificate(args.Certificate.Export(X509ContentType.Cert)); - presented = owned; - } - - try - { - using var chain = new X509Chain(); - chain.ChainPolicy.TrustMode = X509ChainTrustMode.CustomRootTrust; - chain.ChainPolicy.CustomTrustStore.AddRange(trustedRoots); - - // Revocation is deliberately not checked: the pin exists precisely for self-issued - // dev / on-prem CAs, which typically publish no CRL or OCSP responder, so checking - // would fail every such chain. The pin itself is the revocation story — remove the CA. - chain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck; - - // Seed the intermediates the broker presented during the handshake. A leaf signed by - // an intermediate that chains up to the pinned root is a common broker topology, and - // that intermediate arrives on the wire rather than being installed locally — without - // it in the candidate pool a legitimate trust path simply cannot be built. - // CustomRootTrust still means only `trustedRoots` may terminate the chain, so a rogue - // self-signed cert smuggled in here cannot become an anchor. - // Both sources are read: platforms differ in whether the peer-supplied intermediates - // land in the incoming chain's elements, its ExtraStore, or both. - if (args.Chain is not null) - { - foreach (var element in args.Chain.ChainElements) - { - chain.ChainPolicy.ExtraStore.Add(element.Certificate); - } - - chain.ChainPolicy.ExtraStore.AddRange(args.Chain.ChainPolicy.ExtraStore); - } - - if (chain.Build(presented)) - { - return true; - } - - logger?.LogError( - "MQTT broker certificate does not chain to the pinned CA '{CaCertificatePath}': {ChainStatus}.", - caCertificatePath, - string.Join(", ", chain.ChainStatus.Select(s => s.Status))); - return false; - } - catch (CryptographicException ex) - { - // X509Chain.Build throws on a malformed certificate or an unusable policy. An exception - // escaping a TLS validation callback surfaces as an opaque handshake crash, so classify - // it here and refuse: an unverifiable certificate is a rejected certificate. - logger?.LogError( - ex, - "MQTT broker certificate could not be validated against the pinned CA '{CaCertificatePath}'; rejecting.", - caCertificatePath); - return false; - } - finally - { - owned?.Dispose(); - } - } - - private static MQTTnet.Formatter.MqttProtocolVersion MapProtocolVersion(MqttProtocolVersion version) - => version switch - { - MqttProtocolVersion.V311 => MQTTnet.Formatter.MqttProtocolVersion.V311, - MqttProtocolVersion.V500 => MQTTnet.Formatter.MqttProtocolVersion.V500, - _ => throw new ArgumentOutOfRangeException(nameof(version), version, "Unsupported MQTT protocol version."), - }; } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttDriver.cs index a6814efb..3fd63a76 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttDriver.cs @@ -939,7 +939,7 @@ public sealed class MqttDriver // ONLINE half without the Will is strictly worse than shipping neither: a crashed node // would leave a retained ONLINE asserting forever that a dead host is alive, which is the // exact state the mechanism exists to prevent. The Will belongs in - // MqttConnection.BuildClientOptions; until it is there this flag stays inert and said so. + // MqttClientOptionsFactory.BuildClientOptions; until it is there this flag stays inert and said so. _logger?.LogWarning( "MQTT driver '{DriverId}': Sparkplug actAsPrimaryHost is set but primary-host STATE " + "publishing is not implemented — this driver observes STATE and never publishes one. " diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttDriverProbe.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttDriverProbe.cs index d14886a4..e71666da 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttDriverProbe.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttDriverProbe.cs @@ -17,7 +17,7 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Mqtt; /// /// /// -/// Reuses for the connect (TLS, +/// Reuses for the connect (TLS, /// CA-pin, credentials, protocol-version mapping) rather than hand-rolling a second /// connect path — a duplicate would be a security divergence. A distinct client-id suffix /// (see ) keeps a probe from colliding with the driver's @@ -101,7 +101,7 @@ public sealed class MqttDriverProbe : IDriverProbe MqttClientOptions clientOptions; try { - clientOptions = MqttConnection.BuildClientOptions( + clientOptions = MqttClientOptionsFactory.BuildClientOptions( probeOptions, ProbeClientIdPrefix + Guid.NewGuid().ToString("N")[..8], NullLogger.Instance); diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/Sparkplug/SparkplugIngestor.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/Sparkplug/SparkplugIngestor.cs index cb14d71f..a5c294ef 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/Sparkplug/SparkplugIngestor.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/Sparkplug/SparkplugIngestor.cs @@ -1038,7 +1038,7 @@ public sealed class SparkplugIngestor /// while their host is offline. Shipping the ONLINE half without the Will is strictly worse /// than shipping neither: an OtOpcUa node that crashes would leave a retained ONLINE asserting /// forever that a dead host is alive, which is precisely the state the mechanism exists to - /// prevent. The Will has to be set inside MqttConnection.BuildClientOptions — a + /// prevent. The Will has to be set inside MqttClientOptionsFactory.BuildClientOptions — a /// different file, a different task — so this task implements the observation half and /// warns when the flag is set. See the design doc §7/§8. /// diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.csproj b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.csproj index 55ffef0b..79edc634 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.csproj +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.csproj @@ -14,6 +14,7 @@ + diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Browsing/SparkplugRebirthScope.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Browsing/SparkplugRebirthScope.cs new file mode 100644 index 00000000..10c9f965 --- /dev/null +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Browsing/SparkplugRebirthScope.cs @@ -0,0 +1,80 @@ +namespace ZB.MOM.WW.OtOpcUa.AdminUI.Browsing; + +/// +/// Validates an operator-typed Sparkplug rebirth scope for the raw browse modal (Gitea #514). +/// +/// +/// +/// Why a typed scope exists at all. The Sparkplug browse tree is birth-driven, and +/// births are never retained. On a quiet plant — every edge node birthed long before the browse +/// window opened, and is now publishing only NDATA/DDATA — the tree comes up empty. The +/// Request-rebirth scope was derived solely from a tree selection, so the one action that would +/// populate the tree could not be armed: the action needed the tree, and the tree needed the action. +/// +/// +/// This is advisory, not the gate. MqttBrowseSession.ResolveRebirthTargets re-resolves +/// the scope and refuses an unusable one before any message leaves, and the +/// DriverOperator policy is enforced server-side in BrowserSessionService. This exists +/// so the modal does not let an operator arm a two-click confirm that cannot succeed. Being the +/// looser of the two is therefore safe; being the stricter one would block a scope the session +/// accepts, which is the failure mode the repo's authoring surfaces avoid by convention. +/// +/// +/// A group-only scope is rejected on purpose. The session resolves a bare group by enumerating +/// the edge nodes it has observed — of which an empty tree has none — so it could not help the +/// case this input exists for, and on a populated tree it would fan one click across the whole group. +/// The tree-selection path still offers a group scope, with its own confirm-time cap and warning. +/// +/// +public static class SparkplugRebirthScope +{ + /// MQTT wildcards. A rebirth addresses one edge node exactly; a wildcard cannot name one. + private static readonly char[] Wildcards = ['+', '#']; + + /// + /// Describes why is not a usable {group}/{edgeNode} rebirth scope. + /// + /// The operator-typed scope, untrimmed; is treated as blank. + /// The operator-facing error text, or when the scope is usable. + public static string? Describe(string? raw) + { + var scope = (raw ?? "").Trim(); + if (scope.Length == 0) { return "Enter the edge node as 'group/edgeNode'."; } + + if (scope.IndexOfAny(Wildcards) >= 0) + { + return "An MQTT wildcard (+ or #) cannot address an edge node; name one exactly."; + } + + var segments = scope.Split('/'); + if (segments.Length != 2 || Array.Exists(segments, string.IsNullOrWhiteSpace)) + { + return "Use exactly 'group/edgeNode' — a group on its own would ask every edge node in it to " + + "republish, which this window cannot enumerate while the tree is empty."; + } + + return null; + } + + /// True when is a usable rebirth scope. + /// The operator-typed scope. + /// when finds no fault. + public static bool IsUsable(string? raw) => Describe(raw) is null; + + /// + /// Splits a scope already accepted by into its two segments. + /// + /// A scope for which is . + /// The trimmed (group, edgeNode) pair. + /// The scope is not usable. + public static (string GroupId, string EdgeNodeId) Split(string? raw) + { + if (Describe(raw) is { } error) + { + throw new ArgumentException($"'{raw}' is not a usable rebirth scope: {error}", nameof(raw)); + } + + var segments = raw!.Trim().Split('/'); + return (segments[0].Trim(), segments[1].Trim()); + } +} diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Raw/RawBrowseModal.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Raw/RawBrowseModal.razor index 144a112f..1e57c8c3 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Raw/RawBrowseModal.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Raw/RawBrowseModal.razor @@ -126,9 +126,34 @@ @if (_rebirthTarget is null) { + @* Gitea #514: the tree is BIRTH-driven, and births are never retained. On a + quiet plant — every edge node birthed long before this window opened and is + now publishing only NDATA/DDATA — the tree is empty, so a tree-selected + scope can never be armed and the one action that would fill the tree is + unreachable. Naming an edge node directly breaks that cycle. Deliberately + an EDGE NODE rather than a group broadcast: a group rebirth is a + self-inflicted broker storm on a large plant. *@
- Click a group, edge node, device or metric in the tree above to choose the scope. + Click a group, edge node, device or metric in the tree above to choose the + scope — or, if the tree is empty because every edge node birthed + before this window opened, name one directly:
+
+ Edge node + + +
+ @if (!string.IsNullOrWhiteSpace(_manualScope) && ManualScopeError is { } manualErr) + { +
@manualErr
+ } } else { @@ -179,6 +204,17 @@ } } + else if (_rebirthExpired && _canOperate) + { + @* Gitea #510: the affordance is retired rather than left to fail on click. It is + drawn from a capability probed at browse-open, and sessions are reaped for + idleness — so a modal left open past the reap used to offer an action whose + token no longer resolved. Say what happened instead of silently vanishing. *@ +
+ This browse session expired while idle, so a rebirth can no longer be requested + through it. Close and re-open the browser to request one. +
+ }
@_selected.Count tag@(_selected.Count == 1 ? "" : "s") selected. @@ -276,6 +312,18 @@ private string? _rebirthOutcome; private string? _rebirthError; + /// + /// Set when a re-probe finds the session's rebirth capability gone (#510), so the retired + /// affordance can be explained rather than silently disappearing. + /// + private bool _rebirthExpired; + + /// + /// Operator-typed {group}/{edgeNode} rebirth scope, for the birth-driven-tree deadlock + /// (#514). Seeded with the driver's configured group so only the edge node has to be typed. + /// + private string _manualScope = ""; + private readonly Dictionary _selected = new(StringComparer.Ordinal); private readonly HashSet _selectedIds = new(StringComparer.Ordinal); @@ -316,6 +364,8 @@ _disabledReason = null; ResetRebirthState(); _canRebirth = false; + _rebirthExpired = false; + _manualScope = ""; _resolving = true; StateHasChanged(); @@ -373,8 +423,16 @@ { _token = result.Token; // Capability, not authorization: a plain-MQTT window (and every non-MQTT browser) has no - // re-announce action at all, so the affordance must not be drawn for it. + // re-announce action at all, so the affordance must not be drawn for it. Re-probed per + // click as well — see RebirthStillAvailable (#510). _canRebirth = BrowserService.CanRequestRebirth(_token); + _rebirthExpired = false; + // Only the edge node should have to be typed (#514): the group is the driver's entire + // subscription scope, so it is already in the config this browser was opened with. + if (_canRebirth && _manualScope.Length == 0 && TryReadConfiguredGroupId() is { } gid) + { + _manualScope = gid + "/"; + } } else { _openError = result.Message; } } @@ -502,9 +560,64 @@ { _rebirthOutcome = null; _rebirthError = null; + // #510 — the capability was probed once at browse-open; re-probe before offering to act on it. + if (!RebirthStillAvailable()) { return; } _rebirthConfirming = true; } + /// + /// Re-probes the open session's rebirth capability, retiring the affordance when it has gone + /// (Gitea #510). + /// + /// when a rebirth can still be requested through this session. + /// + /// CanRequestRebirth is a cheap, non-throwing, no-I/O probe that is false for a reaped or + /// unknown token, so re-running it per click costs nothing. It is a capability probe, not + /// an authorization check — the DriverOperator gate stays server-side on + /// RequestRebirthAsync and fails closed there, and _canOperate remains the + /// defence-in-depth half of the render condition. + /// + private bool RebirthStillAvailable() + { + if (BrowserService.CanRequestRebirth(_token)) { return true; } + + _canRebirth = false; + _rebirthExpired = true; + ResetRebirthState(); + return false; + } + + /// + /// Describes why is not a usable edge-node scope, or null when + /// it is. Mirrors the session's own {group}/{edgeNode} rule (Gitea #514). + /// + /// + /// Advisory only: MqttBrowseSession.ResolveRebirthTargets re-validates and refuses an + /// unresolvable scope before publishing anything, so this exists to keep the operator from + /// arming a confirm step that cannot succeed — not as the gate. A group-only scope is rejected on + /// purpose: the session would fan out to every observed edge node, and on a tree that is empty + /// (the case this input exists for) there are none to fan to. + /// + private string? ManualScopeError => SparkplugRebirthScope.Describe(_manualScope); + + /// Adopts the typed {group}/{edgeNode} as the rebirth scope (Gitea #514). + private void ApplyManualScope() + { + if (!RebirthStillAvailable()) { return; } + if (!SparkplugRebirthScope.IsUsable(_manualScope)) { return; } + + var (groupId, edgeNodeId) = SparkplugRebirthScope.Split(_manualScope); + _rebirthTarget = new RebirthTarget( + $"{groupId}/{edgeNodeId}", + $"edge node {edgeNodeId}", + $"Edge node '{edgeNodeId}' in group '{groupId}' republishes its whole metric set. " + + "This node was named directly, so it need not have appeared in the tree.", + IsGroup: false); + _rebirthConfirming = false; + _rebirthOutcome = null; + _rebirthError = null; + } + /// Disarms the confirm step without publishing anything. private void CancelRebirth() => _rebirthConfirming = false; @@ -516,6 +629,9 @@ private async Task ConfirmRebirthAsync() { if (_rebirthTarget is null) { return; } + // #510 again, at the last moment before publishing: the reap can land between arming the confirm + // step and clicking it, and this is the one path that reaches a live broker. + if (!RebirthStillAvailable()) { return; } _rebirthBusy = true; _rebirthOutcome = null; _rebirthError = null; @@ -558,6 +674,34 @@ ResetRebirthState(); } + /// + /// Reads the driver's configured Sparkplug group id out of the merged config, or null when + /// it is absent or the blob is unreadable. Purely a convenience seed for + /// — never a gate, so any failure is simply "no seed". + /// + /// The configured group id, or null. + private string? TryReadConfiguredGroupId() + { + try + { + // Case-insensitive: the merged blob is assembled from the driver + device configs and this + // is a display convenience, not a binding — it must not depend on which one won. + var root = System.Text.Json.Nodes.JsonNode.Parse(_mergedConfig)?.AsObject(); + var sparkplug = root? + .FirstOrDefault(p => string.Equals(p.Key, "Sparkplug", StringComparison.OrdinalIgnoreCase)) + .Value?.AsObject(); + var groupId = sparkplug? + .FirstOrDefault(p => string.Equals(p.Key, "GroupId", StringComparison.OrdinalIgnoreCase)) + .Value?.GetValue(); + + return string.IsNullOrWhiteSpace(groupId) ? null : groupId.Trim(); + } + catch + { + return null; + } + } + /// Clears every rebirth-panel field (modal re-open, browser close). private void ResetRebirthState() { @@ -586,12 +730,22 @@ .ToList(); if (_commitErrors.Count > 0) { return; } - var rows = _selected.Values + var selected = _selected.Values.ToList(); + var rows = selected .Select(s => RawBrowseCommitMapper.MapLeaf( _driverType, s.NodeId, s.Name, s.DriverDataType, DefaultDataType, _groupPrefix, s.FolderPath, _createGroups, s.AddressFields)) .ToList(); + // A leaf name is coerced into a legal RawPath segment (#509), and that coercion is not + // injective — 'A/B' and 'A_B' both land on 'A_B'. Refuse the colliding SET by name here: + // letting it through would silently merge two distinct metrics into one tag bound to + // whichever address survived, which is worse than the outright refusal #509 replaced. + _commitErrors = RawBrowseCommitMapper + .DescribeNameCollisions(selected.Select(s => s.Name).ToList(), rows) + .ToList(); + if (_commitErrors.Count > 0) { return; } + var outcome = await Svc.ImportTagsAsync(_effectiveDeviceId, rows); if (outcome.Errors.Count > 0) { @@ -613,6 +767,8 @@ var t = _token; _token = Guid.Empty; _canRebirth = false; + _rebirthExpired = false; + _manualScope = ""; ResetRebirthState(); Visible = false; _open = false; diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/RawBrowseCommitMapper.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/RawBrowseCommitMapper.cs index 43fde69d..a76d6295 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/RawBrowseCommitMapper.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/RawBrowseCommitMapper.cs @@ -25,7 +25,10 @@ public static class RawBrowseCommitMapper /// The owning device's driver type (a value). /// The leaf's driver-side full reference (the BrowseNode.NodeId = the /// captured DriverAttributeInfo.FullName) — becomes the driver-typed address field. - /// The leaf's browse name — becomes the raw tag Name (a RawPath segment). + /// The leaf's browse name. Becomes the raw tag Name after + /// coerces it into a legal RawPath segment — a Sparkplug metric name + /// legitimately contains / and a Name may not (Gitea #509). The leaf's binding address is + /// unaffected: it travels separately into the TagConfig. /// The leaf's name (from the browse /// attribute side-channel), or null when the browser cannot report a type (e.g. the OPC UA Client tree). /// The OPC-UA built-in type name to fall back to when @@ -52,6 +55,7 @@ public static class RawBrowseCommitMapper { var dataType = MapDataType(driverDataType) ?? defaultDataType; var tagConfig = BuildTagConfig(driverType, fullName, addressFields); + var name = ToRawSegment(browseName); var mirrored = createGroups && folderPath is { Count: > 0 } ? string.Join(RawPaths.Separator, folderPath) : null; @@ -61,9 +65,99 @@ public static class RawBrowseCommitMapper // no poll-group (batching is authored later). return new RawTagImportRow( groupPath, - new RawTagInput(browseName, dataType, TagAccessLevel.Read, WriteIdempotent: false, PollGroupId: null, tagConfig)); + new RawTagInput(name, dataType, TagAccessLevel.Read, WriteIdempotent: false, PollGroupId: null, tagConfig)); } + /// + /// Coerces a browse leaf's name into a legal RawPath segment (Gitea #509): trims the ends and + /// replaces every with . + /// + /// The leaf's browse name, as the driver reported it. + /// A -clean segment; blank only if the input was blank. + /// + /// + /// The Name and the address are two fields, and only the Name is constrained. A tag Name is + /// a RawPath segment, so it may not contain /; the driver-side reference it binds by has no + /// such rule and is written into the TagConfig untouched by . + /// Refusing the commit outright conflated the two — and it refused a metric the Sparkplug B spec + /// makes mandatory on every edge node (Node Control/Rebirth), so the one metric an + /// operator is most likely to browse for was the one that could not be committed. + /// + /// + /// This deliberately covers every driver, not just MQTT. The argument is about what a Name + /// is, not about who produced it: any browser whose leaf name carries a / hits the same + /// wall, and for all of them the binding address travels separately and survives intact. + /// + /// + /// The rule set is 's, not an invented one — empty, + /// an embedded separator, and leading/trailing whitespace are exactly what it rejects, so trimming + /// and replacing is total rather than a best-effort guess. RawBrowseCommitMapperTests pins + /// that by asserting the output validates, so a new rule there fails here instead of silently + /// letting an illegal Name through to the import. + /// + /// + /// Sanitising is not injective, so two distinct leaves can land on one Name — see + /// , which is why a caller must run that check rather than + /// trusting this in isolation. + /// + /// + public static string ToRawSegment(string? browseName) + { + var trimmed = (browseName ?? "").Trim(); + return trimmed.Length == 0 ? "" : trimmed.Replace(RawPaths.Separator, SeparatorReplacement); + } + + /// What substitutes for a separator inside a browse name. + private const char SeparatorReplacement = '_'; + + /// + /// Names the browse leaves that would land on one tag Name within one + /// target group, or an empty list when the mapped rows are all distinct. + /// + /// The ORIGINAL leaf names, in the same order as . + /// The mapped import rows. + /// One operator-facing message per colliding set; empty when there is nothing to refuse. + /// + /// Sanitising maps many names onto one — A/B and A_B both become A_B — and a + /// silent collision is materially worse than the refusal #509 replaced: two distinct metrics would + /// become one tag bound to whichever address the import happened to keep. So the coercion is only + /// safe when paired with this check. + /// + /// Scoped per target group, because the Name only has to be unique within its group — the same + /// metric name under two devices is not a collision. + /// + /// + /// The message names the specific pair rather than failing the batch anonymously. The commit + /// itself stays all-or-nothing, matching the CSV import's staged-review semantics: nothing is + /// written while any row is unresolvable. + /// + /// + public static IReadOnlyList DescribeNameCollisions( + IReadOnlyList browseNames, + IReadOnlyList rows) + { + ArgumentNullException.ThrowIfNull(browseNames); + ArgumentNullException.ThrowIfNull(rows); + + return rows + .Select((row, i) => (row, original: i < browseNames.Count ? browseNames[i] : row.Tag.Name)) + .GroupBy(x => (x.row.TagGroupPath ?? "", x.row.Tag.Name), TupleComparer) + .Where(g => g.Count() > 1) + .Select(g => + $"'{g.Key.Item2}' would be the name of {g.Count()} different tags" + + (string.IsNullOrEmpty(g.Key.Item1) ? "" : $" in group '{g.Key.Item1}'") + + $": {string.Join(", ", g.Select(x => $"'{x.original}'"))}. " + + "Their names differ only by characters that are not legal in a tag name. Commit them " + + "separately, or author them by manual entry with distinct names.") + .ToList(); + } + + /// Ordinal comparer for the (group path, name) collision key — RawPath comparison is ordinal. + private static readonly IEqualityComparer<(string, string)> TupleComparer = + EqualityComparer<(string, string)>.Create( + (a, b) => RawPaths.Comparer.Equals(a.Item1, b.Item1) && RawPaths.Comparer.Equals(a.Item2, b.Item2), + x => HashCode.Combine(RawPaths.Comparer.GetHashCode(x.Item1), RawPaths.Comparer.GetHashCode(x.Item2))); + /// /// Maps a enum NAME (as reported by the browse attribute side-channel) to the /// OPC-UA built-in type name a carries. Mirrors diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/MqttTagConfigModel.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/MqttTagConfigModel.cs index 68587153..6ceae71d 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/MqttTagConfigModel.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/MqttTagConfigModel.cs @@ -196,10 +196,12 @@ public sealed class MqttTagConfigModel /// dataType is the one key both modes write, from two different typed fields /// ( for Plain — always present; for /// Sparkplug — omitted unless the operator set an override): which field wins is decided by - /// at write time. Every other key is unconditional — mode-inapplicable keys - /// (e.g. topic while Sparkplug, or groupId while Plain) are written/cleared exactly - /// as their typed field says, which naturally preserves a retyped tag's other-mode leftovers - /// untouched until the operator edits that field too. + /// at write time. payloadFormat is the one key written for Plain + /// only (Gitea #513) — see its line below for why it alone needs the mode test. Every other + /// key is unconditional: mode-inapplicable keys (e.g. topic while Sparkplug, or + /// groupId while Plain) are written/cleared exactly as their typed field says, and because + /// each of those fields is blank-⇒-omit, an absent key stays absent while a retyped tag's real + /// other-mode leftovers survive untouched until the operator edits that field too. /// /// The serialised TagConfig JSON string. public string ToJson() @@ -207,7 +209,14 @@ public sealed class MqttTagConfigModel // Blank ⇒ omit, like every other optional here: toggling the shape dropdown on an untouched tag // must not leave a stray "topic":"" behind. TagConfigJson.Set(_bag, TopicKey, string.IsNullOrWhiteSpace(Topic) ? null : Topic.Trim()); - TagConfigJson.Set(_bag, PayloadFormatKey, PayloadFormat); + + // Plain-only (Gitea #513). Unlike every other Plain key here, PayloadFormat is a NON-NULLABLE + // typed field, so an unconditional write does not merely preserve a retyped tag's leftover — it + // INVENTS "payloadFormat":"Json" in the blob of a Sparkplug tag that never had one. The key names + // a decode path that Sparkplug does not use (MqttTagDefinitionFactory routes on the descriptor + // keys and never reads it), so it is a trap for anyone diagnosing a decode problem from the blob. + TagConfigJson.Set(_bag, PayloadFormatKey, Mode == MqttMode.SparkplugB ? null : (object)PayloadFormat); + TagConfigJson.Set(_bag, JsonPathKey, string.IsNullOrWhiteSpace(JsonPath) ? null : JsonPath.Trim()); TagConfigJson.Set(_bag, QosKey, Qos); TagConfigJson.Set(_bag, RetainSeedKey, RetainSeed); diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttBrowseSessionTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttBrowseSessionTests.cs index 28f128f2..39cffaa9 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttBrowseSessionTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttBrowseSessionTests.cs @@ -410,7 +410,7 @@ public sealed class MqttBrowseSessionTests a.Length.ShouldBe("-browse-".Length + 8); var opts = new MqttDriverOptions { ClientId = "otopcua-node1" }; - MqttConnection.BuildClientOptions(opts, a).ClientId.ShouldBe("otopcua-node1" + a); + MqttClientOptionsFactory.BuildClientOptions(opts, a).ClientId.ShouldBe("otopcua-node1" + a); } [Fact] diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttBrowserLayeringTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttBrowserLayeringTests.cs new file mode 100644 index 00000000..79c36015 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttBrowserLayeringTests.cs @@ -0,0 +1,85 @@ +using System.Reflection; +using Shouldly; +using Xunit; + +namespace ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests; + +/// +/// Pins the MQTT project layering that Gitea #512 restored: .Browser depends on the shared +/// .Contracts + .Transport leaves, never on the runtime .Driver. +/// +/// +/// +/// Why this is worth a test. .Browser used to reference the runtime driver, and the +/// AdminUI references .Browser — so the AdminUI's build/deploy graph transitively contained +/// the whole MQTT driver runtime plus Core, Polly.Core and Serilog. Nothing failed; the cost was +/// invisible. A reference re-added out of convenience would be invisible in exactly the same way, +/// which is precisely what a guard is for. +/// +/// +/// What it can and cannot catch. It asserts on emitted assembly references, so it fires when +/// the browser actually uses a driver-runtime type — the case that matters. A +/// ProjectReference re-added but never used would not trip it (the compiler emits no +/// reference), so the csproj comment remains the primary record of the decision. +/// +/// +[Trait("Category", "Unit")] +public sealed class MqttBrowserLayeringTests +{ + private const string BrowserAssembly = "ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Browser"; + private const string DriverRuntimeAssembly = "ZB.MOM.WW.OtOpcUa.Driver.Mqtt"; + private const string ContractsAssembly = "ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Contracts"; + private const string TransportAssembly = "ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Transport"; + + private static string[] BrowserReferences() + { + var browser = Assembly.Load(BrowserAssembly); + return [.. browser.GetReferencedAssemblies().Select(a => a.Name!)]; + } + + [Fact] + public void Browser_does_not_reference_the_runtime_driver() + => BrowserReferences().ShouldNotContain( + DriverRuntimeAssembly, + "the MQTT browser is back to referencing the runtime driver — every other bespoke browser " + + "in this repo (OpcUaClient, Galaxy) depends on its .Contracts leaf only, and this reference " + + "reaches the AdminUI. If shared connection policy is needed, put it in .Transport; if shared " + + "protocol/codec is needed, put it in .Contracts. Do NOT copy the TLS/CA-pin logic."); + + [Fact] + public void Browser_reaches_the_shared_leaves_instead() + { + // The positive control. Asserting only the absence above would also pass if the browser stopped + // referencing anything MQTT at all — e.g. if this test loaded the wrong assembly, or the type + // names drifted — so pin what it SHOULD depend on in the same breath. + var references = BrowserReferences(); + + references.ShouldContain(ContractsAssembly); + references.ShouldContain(TransportAssembly); + } + + [Fact] + public void Transport_stays_a_leaf_that_does_not_reach_back_into_the_runtime_driver() + { + // .Transport holds connection POLICY only. If it ever referenced the runtime driver, routing the + // browser through it would restore the coupling this whole change removed, one hop further out. + var references = Assembly.Load(TransportAssembly) + .GetReferencedAssemblies() + .Select(a => a.Name!) + .ToArray(); + + references.ShouldNotContain(DriverRuntimeAssembly); + references.ShouldNotContain(BrowserAssembly); + } + + [Fact] + public void Contracts_stays_transport_free() + => Assembly.Load(ContractsAssembly) + .GetReferencedAssemblies() + .Select(a => a.Name!) + .ShouldNotContain( + "MQTTnet", + "Driver.Mqtt.Contracts is deliberately transport-free — it carries the Sparkplug wire " + + "schema and codec, and is referenced by ControlPlane, which has no business with a " + + "broker client. MQTTnet-typed connection policy belongs in .Transport."); +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttConnectionTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttConnectionTests.cs index 11287ce1..f6a82601 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttConnectionTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttConnectionTests.cs @@ -13,7 +13,7 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests; /// /// Covers 's two Task-3 responsibilities: the pure -/// options assembly (TLS / CA pin / +/// options assembly (TLS / CA pin / /// credentials / protocol knobs) and the bounded /// deadline. /// @@ -189,7 +189,7 @@ public sealed class MqttConnectionTests var opts = new MqttDriverOptions { Host = Host, Port = 8883, UseTls = true, CaCertificatePath = missing }; - var built = MqttConnection.BuildClientOptions(opts, clientIdSuffix: null); + var built = MqttClientOptionsFactory.BuildClientOptions(opts, clientIdSuffix: null); var tls = built.ChannelOptions.ShouldBeOfType().TlsOptions; tls.CertificateValidationHandler.ShouldNotBeNull(); @@ -206,7 +206,7 @@ public sealed class MqttConnectionTests try { var opts = new MqttDriverOptions { Host = Host, Port = 8883, UseTls = true, CaCertificatePath = caPath }; - var built = MqttConnection.BuildClientOptions(opts, clientIdSuffix: null); + var built = MqttClientOptionsFactory.BuildClientOptions(opts, clientIdSuffix: null); var tls = built.ChannelOptions.ShouldBeOfType().TlsOptions; using var stranger = SelfSignedLeaf(Host); @@ -232,7 +232,7 @@ public sealed class MqttConnectionTests try { var opts = new MqttDriverOptions { Host = Host, Port = 8883, UseTls = true, CaCertificatePath = caPath }; - var built = MqttConnection.BuildClientOptions(opts, clientIdSuffix: null); + var built = MqttClientOptionsFactory.BuildClientOptions(opts, clientIdSuffix: null); var tls = built.ChannelOptions.ShouldBeOfType().TlsOptions; tls.CertificateValidationHandler!(ValidationArgs(built, leaf)).ShouldBeTrue(); @@ -259,7 +259,7 @@ public sealed class MqttConnectionTests try { var opts = new MqttDriverOptions { Host = Host, Port = 8883, UseTls = true, CaCertificatePath = caPath }; - var built = MqttConnection.BuildClientOptions(opts, clientIdSuffix: null); + var built = MqttClientOptionsFactory.BuildClientOptions(opts, clientIdSuffix: null); var tls = built.ChannelOptions.ShouldBeOfType().TlsOptions; tls.CertificateValidationHandler!(ValidationArgs(built, leaf, intermediate)).ShouldBeTrue(); @@ -330,7 +330,7 @@ public sealed class MqttConnectionTests ConnectTimeoutSeconds = 7, }; - var built = MqttConnection.BuildClientOptions(opts, clientIdSuffix: null); + var built = MqttClientOptionsFactory.BuildClientOptions(opts, clientIdSuffix: null); built.ProtocolVersion.ShouldBe(MQTTnet.Formatter.MqttProtocolVersion.V311); built.ClientId.ShouldBe("otopcua-node-1"); @@ -347,12 +347,12 @@ public sealed class MqttConnectionTests { var opts = new MqttDriverOptions { Host = Host, ClientId = "otopcua" }; - MqttConnection.BuildClientOptions(opts, clientIdSuffix: "-browse").ClientId.ShouldBe("otopcua-browse"); + MqttClientOptionsFactory.BuildClientOptions(opts, clientIdSuffix: "-browse").ClientId.ShouldBe("otopcua-browse"); } [Fact] public void BuildClientOptions_NoUsername_SendsNoCredentials() - => MqttConnection.BuildClientOptions(new MqttDriverOptions { Host = Host }, clientIdSuffix: null) + => MqttClientOptionsFactory.BuildClientOptions(new MqttDriverOptions { Host = Host }, clientIdSuffix: null) .Credentials.ShouldBeNull(); [Fact] @@ -360,7 +360,7 @@ public sealed class MqttConnectionTests { var opts = new MqttDriverOptions { Host = Host, Username = "svc", Password = "pw" }; - var built = MqttConnection.BuildClientOptions(opts, clientIdSuffix: null); + var built = MqttClientOptionsFactory.BuildClientOptions(opts, clientIdSuffix: null); built.Credentials.ShouldNotBeNull(); built.Credentials.GetUserName(built).ShouldBe("svc"); @@ -1166,7 +1166,7 @@ public sealed class MqttConnectionTests } private static MqttClientTlsOptions BuildTls(MqttDriverOptions opts) - => MqttConnection.BuildClientOptions(opts, clientIdSuffix: null) + => MqttClientOptionsFactory.BuildClientOptions(opts, clientIdSuffix: null) .ChannelOptions.ShouldBeOfType().TlsOptions; private static readonly DateTimeOffset NotBefore = DateTimeOffset.UtcNow.AddDays(-1); diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Browsing/SparkplugRebirthScopeTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Browsing/SparkplugRebirthScopeTests.cs new file mode 100644 index 00000000..d999a7cf --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Browsing/SparkplugRebirthScopeTests.cs @@ -0,0 +1,84 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.AdminUI.Browsing; + +namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests.Browsing; + +/// +/// Unit tests for — the operator-typed {group}/{edgeNode} +/// rebirth scope that breaks the birth-driven-tree deadlock (Gitea #514). +/// +/// +/// The rule is deliberately advisory — MqttBrowseSession.ResolveRebirthTargets re-resolves the +/// scope and refuses an unusable one before publishing — so these tests pin the affordance +/// behaviour: an operator must not be able to arm a two-click confirm that cannot succeed, and must +/// not be blocked from a scope the session would accept. +/// +[Trait("Category", "Unit")] +public sealed class SparkplugRebirthScopeTests +{ + [Theory] + [InlineData("OtOpcUaSim/EdgeA")] + [InlineData(" OtOpcUaSim/EdgeA ")] + [InlineData("g/e")] + [InlineData("Plant 1/Edge Node 2")] // spaces are legal inside a Sparkplug id + public void Accepts_a_group_and_edge_node(string scope) + { + SparkplugRebirthScope.Describe(scope).ShouldBeNull(); + SparkplugRebirthScope.IsUsable(scope).ShouldBeTrue(); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + public void Rejects_a_blank_scope(string? scope) + => SparkplugRebirthScope.Describe(scope).ShouldNotBeNullOrWhiteSpace(); + + [Fact] + public void Rejects_a_group_on_its_own() + { + // The session resolves a bare group by enumerating OBSERVED edge nodes, of which an empty tree + // has none — so accepting it here would arm a confirm step that resolves to nothing, in exactly + // the situation this input exists for. + var error = SparkplugRebirthScope.Describe("OtOpcUaSim"); + + error.ShouldNotBeNullOrWhiteSpace(); + error!.ShouldContain("group/edgeNode"); + } + + [Theory] + [InlineData("g/e/extra")] + [InlineData("g//e")] + [InlineData("/EdgeA")] + [InlineData("OtOpcUaSim/")] + public void Rejects_a_scope_that_is_not_exactly_two_segments(string scope) + => SparkplugRebirthScope.Describe(scope).ShouldNotBeNullOrWhiteSpace(); + + [Theory] + [InlineData("g/+")] + [InlineData("#/e")] + [InlineData("g/Edge#")] + public void Rejects_an_mqtt_wildcard(string scope) + { + // A rebirth NCMD is addressed to one edge node on a literal topic; a wildcard names no single + // node, and letting one through would build a topic the broker rejects. + var error = SparkplugRebirthScope.Describe(scope); + + error.ShouldNotBeNullOrWhiteSpace(); + error!.ShouldContain("wildcard"); + } + + [Fact] + public void Split_returns_the_trimmed_pair() + { + var (groupId, edgeNodeId) = SparkplugRebirthScope.Split(" OtOpcUaSim/EdgeA "); + + groupId.ShouldBe("OtOpcUaSim"); + edgeNodeId.ShouldBe("EdgeA"); + } + + [Fact] + public void Split_refuses_a_scope_Describe_would_reject() + => Should.Throw(() => SparkplugRebirthScope.Split("OtOpcUaSim")); +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/MqttTagConfigModelTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/MqttTagConfigModelTests.cs index 5ad88baf..9104fb30 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/MqttTagConfigModelTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/MqttTagConfigModelTests.cs @@ -160,6 +160,58 @@ public sealed class MqttTagConfigModelTests json.ShouldNotContain("FullName", Case.Sensitive); } + // ── payloadFormat is Plain-only (Gitea #513) ──────────────────────────────────────────────── + // + // payloadFormat (Json/Raw/Scalar) selects a PLAIN-mode decode path. MqttTagDefinitionFactory routes + // a Sparkplug tag on its descriptor keys and never reads it, so a payloadFormat in a Sparkplug blob + // is not mis-binding anything — it is a key that looks like it selects a decode path and does not, + // in a blob the operator reads (the raw-JSON textarea, a config export). It was written because + // PayloadFormat is the one non-nullable Plain typed field, so "write what the field says" INVENTED + // the key rather than preserving one. + + [Fact] + public void ToJson_omits_payloadFormat_for_a_sparkplug_tag() + { + var json = MqttTagConfigModel.FromJson( + """{"groupId":"Plant1","edgeNodeId":"EdgeA","metricName":"Node Control/Rebirth"}""").ToJson(); + + JsonNode.Parse(json)!.AsObject().ContainsKey("payloadFormat").ShouldBeFalse( + "payloadFormat is a Plain-mode decode selector; a Sparkplug tag that never carried one must " + + "not gain it on save. Actual blob: " + json); + } + + [Fact] + public void ToJson_still_writes_payloadFormat_for_a_plain_tag() + { + // The negative control for the test above: the omission must be mode-conditional, not a blanket + // removal. A Plain tag's decode path is genuinely selected by this key. + var json = new MqttTagConfigModel { Mode = MqttMode.Plain, Topic = "a/b", PayloadFormat = MqttPayloadFormat.Raw } + .ToJson(); + + JsonNode.Parse(json)!.AsObject()["payloadFormat"]!.GetValue().ShouldBe("Raw"); + } + + [Fact] + public void ToJson_clears_a_stale_payloadFormat_when_a_plain_tag_is_retyped_to_sparkplug() + { + // The retype path is how a real blob acquires the stale key. Clearing it is legitimate precisely + // because payloadFormat is a key this editor OWNS — the unknown-key assertions below pin that the + // clearing stays surgical and does not start dropping foreign keys (the issue's explicit warning). + var json = MqttTagConfigModel.FromJson( + """ + {"topic":"a/b","payloadFormat":"Raw","groupId":"Plant1","edgeNodeId":"EdgeA","metricName":"Temp", + "isHistorized":true,"someFutureScalar":"keep me"} + """).ToJson(); + + var o = JsonNode.Parse(json)!.AsObject(); + o.ContainsKey("payloadFormat").ShouldBeFalse(); + + // Descriptor keys intact, and the keys this editor does not own are untouched. + o["metricName"]!.GetValue().ShouldBe("Temp"); + o["isHistorized"]!.GetValue().ShouldBeTrue(); + o["someFutureScalar"]!.GetValue().ShouldBe("keep me"); + } + // ── Validate: Sparkplug rules (Task 24) ───────────────────────────────────────────────────── // // Mirrors MqttTagDefinitionFactory.FromSparkplugTagConfig exactly: groupId/edgeNodeId/metricName diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/RawBrowseCommitMapperTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/RawBrowseCommitMapperTests.cs index e414184b..832496c1 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/RawBrowseCommitMapperTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/RawBrowseCommitMapperTests.cs @@ -3,6 +3,7 @@ using System.Text.Json.Nodes; using Shouldly; using Xunit; using ZB.MOM.WW.OtOpcUa.AdminUI.Uns; +using ZB.MOM.WW.OtOpcUa.Commons.Types; using ZB.MOM.WW.OtOpcUa.Configuration.Enums; using ZB.MOM.WW.OtOpcUa.Core.Abstractions; using ZB.MOM.WW.OtOpcUa.Driver.Mqtt; @@ -397,4 +398,128 @@ public sealed class RawBrowseCommitMapperTests row.Tag.DataType.ShouldBe("Double"); JsonNode.Parse(row.Tag.TagConfig)!.AsObject()["nodeId"]!.GetValue().ShouldBe("ns=2;s=Tag1"); } + + // ---- ToRawSegment / name sanitisation (Gitea #509) -------------------------------------------- + // + // A tag Name is a RawPath segment and may not contain '/'; a Sparkplug metric name legitimately + // may, and the spec-MANDATORY 'Node Control/Rebirth' does. Committing used to be refused whole. + // The address is a separate field and must survive the coercion untouched. + + [Theory] + [InlineData("Node Control/Rebirth", "Node Control_Rebirth")] + [InlineData("Properties/Hardware Make", "Properties_Hardware Make")] + [InlineData("a/b/c", "a_b_c")] + [InlineData(" padded ", "padded")] + [InlineData("plain", "plain")] + public void ToRawSegment_coerces_a_browse_name_into_a_legal_segment(string browseName, string expected) + => RawBrowseCommitMapper.ToRawSegment(browseName).ShouldBe(expected); + + [Theory] + [InlineData("Node Control/Rebirth")] + [InlineData("/leading")] + [InlineData("trailing/")] + [InlineData(" /spaced/ ")] + [InlineData("a/b/c/d/e")] + [InlineData("no-change-needed")] + public void ToRawSegment_output_satisfies_the_RawPaths_segment_rule(string browseName) + { + // Pinned against the AUTHORITY, not against a charset copied into this test. RawPaths.ValidateSegment + // is the rule the import will apply; if it gains a rule the sanitiser does not implement, this fails + // here rather than letting an illegal Name reach ImportTagsAsync. + var segment = RawBrowseCommitMapper.ToRawSegment(browseName); + + RawPaths.ValidateSegment(segment).ShouldBeNull( + $"'{browseName}' sanitised to '{segment}', which RawPaths still rejects"); + } + + [Fact] + public void MapLeaf_commits_a_slashed_sparkplug_metric_with_the_exact_metricName_preserved() + { + var row = RawBrowseCommitMapper.MapLeaf( + driverType: DriverTypeNames.Mqtt, + fullName: "OtOpcUaSim/EdgeA::Node Control/Rebirth", + browseName: "Node Control/Rebirth", + driverDataType: "Boolean", + defaultDataType: "String", + groupPrefix: null, + folderPath: null, + createGroups: false, + addressFields: new Dictionary + { + [MqttTagConfigKeys.GroupId] = "OtOpcUaSim", + [MqttTagConfigKeys.EdgeNodeId] = "EdgeA", + [MqttTagConfigKeys.MetricName] = "Node Control/Rebirth", + }); + + // The Name is coerced... + row.Tag.Name.ShouldBe("Node Control_Rebirth"); + RawPaths.ValidateSegment(row.Tag.Name).ShouldBeNull(); + + // ...and the binding address is NOT. This is the whole point: the two fields already existed + // separately, and only the Name derivation conflated them. + var o = JsonNode.Parse(row.Tag.TagConfig)!.AsObject(); + o[MqttTagConfigKeys.MetricName]!.GetValue().ShouldBe("Node Control/Rebirth"); + o[MqttTagConfigKeys.GroupId]!.GetValue().ShouldBe("OtOpcUaSim"); + o[MqttTagConfigKeys.EdgeNodeId]!.GetValue().ShouldBe("EdgeA"); + } + + // ---- DescribeNameCollisions ------------------------------------------------------------------ + + [Fact] + public void DescribeNameCollisions_refuses_two_metrics_that_sanitise_onto_one_name() + { + // 'A/B' and 'A_B' are distinct metrics that both land on 'A_B'. Silently merging them would be + // worse than the refusal #509 replaced — one tag bound to whichever address survived. + var names = new[] { "A/B", "A_B" }; + var rows = names + .Select(n => RawBrowseCommitMapper.MapLeaf( + DriverTypeNames.Mqtt, $"g/e::{n}", n, "Float32", "Double", null, null, false, + new Dictionary + { + [MqttTagConfigKeys.GroupId] = "g", + [MqttTagConfigKeys.EdgeNodeId] = "e", + [MqttTagConfigKeys.MetricName] = n, + })) + .ToList(); + + var errors = RawBrowseCommitMapper.DescribeNameCollisions(names, rows); + + errors.Count.ShouldBe(1); + // The message must name the specific pair — an anonymous batch failure is what made the original + // refusal hard to act on. + errors[0].ShouldContain("A_B"); + errors[0].ShouldContain("'A/B'"); + } + + [Fact] + public void DescribeNameCollisions_allows_the_same_metric_name_under_two_different_groups() + { + // Uniqueness is per target group, so this is NOT a collision. Without the scoping, mirroring a + // browse tree with a repeated metric name under two devices would be refused wholesale. + var rows = new[] + { + RawBrowseCommitMapper.MapLeaf( + DriverTypeNames.OpcUaClient, "ns=2;s=1", "Temp", null, "Double", + groupPrefix: "DeviceA", folderPath: null, createGroups: false), + RawBrowseCommitMapper.MapLeaf( + DriverTypeNames.OpcUaClient, "ns=2;s=2", "Temp", null, "Double", + groupPrefix: "DeviceB", folderPath: null, createGroups: false), + }; + + RawBrowseCommitMapper.DescribeNameCollisions(new[] { "Temp", "Temp" }, rows).ShouldBeEmpty(); + } + + [Fact] + public void DescribeNameCollisions_passes_a_clean_selection() + => RawBrowseCommitMapper.DescribeNameCollisions( + new[] { "Node Control/Rebirth", "Temperature" }, + new[] + { + RawBrowseCommitMapper.MapLeaf( + DriverTypeNames.OpcUaClient, "ns=2;s=1", "Node Control/Rebirth", null, "Double", + null, null, false), + RawBrowseCommitMapper.MapLeaf( + DriverTypeNames.OpcUaClient, "ns=2;s=2", "Temperature", null, "Double", + null, null, false), + }).ShouldBeEmpty(); }