diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Browser/GalaxyDriverBrowser.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Browser/GalaxyDriverBrowser.cs index b3c67ec5..8722086e 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Browser/GalaxyDriverBrowser.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Browser/GalaxyDriverBrowser.cs @@ -4,6 +4,7 @@ using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; using ZB.MOM.WW.MxGateway.Client; using ZB.MOM.WW.OtOpcUa.Commons.Browsing; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; using ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Config; using ZB.MOM.WW.Secrets.Abstractions; @@ -47,9 +48,15 @@ public sealed class GalaxyDriverBrowser : IDriverBrowser } /// - // Hardcoded literal: this project references Driver.Galaxy.Contracts, not Driver.Galaxy, - // so GalaxyDriverFactoryExtensions.DriverTypeName isn't available here. - public string DriverType => "GalaxyMxGateway"; + /// + /// Sourced from the shared set, reached through + /// Commons. This project references Driver.Galaxy.Contracts, not Driver.Galaxy, so + /// GalaxyDriverFactoryExtensions.DriverTypeName is not available here — but that was never a + /// reason to hand-write the string, which is what this line used to do (Gitea #558): + /// holds the same value and is the constant + /// DriverTypeNamesGuardTests already pins to the factory's registration. + /// + public string DriverType => DriverTypeNames.Galaxy; // Deserializes a GalaxyDriverOptions blob, opens a transient GalaxyRepositoryClient // against the configured gateway endpoint, and returns a browse session over it. The diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Browser/OpcUaClientDriverBrowser.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Browser/OpcUaClientDriverBrowser.cs index 69805ce4..9a9d822e 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Browser/OpcUaClientDriverBrowser.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Browser/OpcUaClientDriverBrowser.cs @@ -6,6 +6,7 @@ using Opc.Ua; using Opc.Ua.Client; using Opc.Ua.Configuration; using ZB.MOM.WW.OtOpcUa.Commons.Browsing; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; using ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient; // OpcUaClientDriverOptions + NamespaceMap namespace ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Browser; @@ -41,7 +42,13 @@ public sealed class OpcUaClientDriverBrowser : IDriverBrowser } /// - public string DriverType => "OpcUaClient"; + /// + /// Sourced from the shared set rather than the hand-written literal + /// this line used to carry (Gitea #558). The value is unchanged; the constant is what + /// DriverTypeNamesGuardTests pins to the factory's own registration, so a rename on either + /// side can no longer leave this browser indexed under a key nothing dispatches to. + /// + public string DriverType => DriverTypeNames.OpcUaClient; /// public async Task OpenAsync(string configJson, CancellationToken cancellationToken) 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 024c251a..97b50e60 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/RawBrowseCommitMapper.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/RawBrowseCommitMapper.cs @@ -271,7 +271,9 @@ public static class RawBrowseCommitMapper // and holds this line to two claims rather than to the assertion the comment used to make: // - a driver type with a bespoke IDriverBrowser registered by AddAdminUI() — read back from the // real registration, not a list — must NOT reach here; and - // - any type that does reach here must be named in that test's GenericKeyExemptions with a reason. + // - any type that does reach here must be named in that test's GenericKeyExemptions with a reason, + // and every name on that list must still reach here (Gitea #558) — an exemption for a driver + // that has since gained a branch above is a stale claim, and a silently inert one. // The universal DiscoveryDriverBrowser's arm is not derivable from the AdminUI assembly (it needs // the runtime drivers + the Host's IDriverFactory), so it is covered by the exemption list rather // than mechanically — see the test's remarks for the residual trust that leaves (Gitea #555). diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/DriverDispatchMapParityTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/DriverDispatchMapParityTests.cs index 0cb9f9ff..2b778e1b 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/DriverDispatchMapParityTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/DriverDispatchMapParityTests.cs @@ -1,16 +1,28 @@ using Shouldly; using Xunit; -using ZB.MOM.WW.OtOpcUa.AdminUI.Uns; using ZB.MOM.WW.OtOpcUa.AdminUI.Uns.TagEditors; using ZB.MOM.WW.OtOpcUa.Core.Abstractions; namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests.Uns; /// -/// Parity guards for the two remaining per-driver dispatch maps (deferment.md G-5, G-6). -/// Both were already testable structures — unlike the two Razor @switch blocks that needed -/// extracting first — so nothing but the tests was missing. +/// Parity guards for the per-driver CSV column dispatch map (deferment.md G-5). It was +/// already a testable structure — unlike the two Razor @switch blocks that needed extracting +/// first — so nothing but the tests was missing. /// +/// +/// The browse-commit seam (G-6) is guarded next door, not here (Gitea #558). This class used to +/// also hold Browse_commit_falls_back_to_the_generic_address_key_for_exactly_the_non_browsable_drivers, +/// over the same RawBrowseCommitMapper.BuildTagConfig seam +/// now enumerates. Two guards over one seam means two +/// hand-authored lists of "drivers that legitimately reach the generic key", and the pair can only +/// drift — this one's was additionally MISNAMED (NonBrowsableDriverTypes really meant "types +/// that produce the generic key": AbLegacy is genuinely non-browsable yet correctly absent, +/// because it has a typed branch). The surviving list is +/// RawBrowseCommitMapperParityTests.GenericKeyExemptions, which is named for what it holds, +/// carries a written reason per entry, and is checked against a derived browsable set rather +/// than a second hand-authored one. +/// [Trait("Category", "Unit")] public sealed class DriverDispatchMapParityTests { @@ -47,52 +59,4 @@ public sealed class DriverDispatchMapParityTests stale.ShouldBeEmpty($"CsvColumnMap has entries for undeclared driver types: {string.Join(", ", stale)}"); } - - /// - /// Every driver type either produces a TYPED blob from browse-commit, or is on the documented - /// non-browsable list. The generic {"address": …} fallback opens EMPTY in a typed editor and - /// blanks the field on save — the MTConnect branch was added for exactly that, and Sql had the same - /// bug unnoticed (deferment.md G-6) because the fallback's comment claimed "browsable drivers - /// are all handled above", which stopped being true the moment SqlDriverBrowser registered. - /// Asserted in both directions on purpose. A one-way check would let a newly-browsable - /// driver be quietly added to the exclusion list instead of getting a branch. Requiring the two sets - /// to match exactly means a driver gaining or losing a browser forces this list to be revisited. - /// - [Fact] - public void Browse_commit_falls_back_to_the_generic_address_key_for_exactly_the_non_browsable_drivers() - { - // Address fields as a browser supplies them; a driver whose browser states none must still produce - // something its typed editor can read from the leaf name alone. - var addressFields = new Dictionary(StringComparer.Ordinal) - { - ["schema"] = "dbo", - ["table"] = "Readings", - ["columnName"] = "Speed", - }; - - var fellBack = DriverTypeNames.All - .Where(t => RawBrowseCommitMapper.BuildTagConfig(t, "Speed", addressFields) == """{"address":"Speed"}""") - .OrderBy(t => t, StringComparer.Ordinal) - .ToList(); - - fellBack.ShouldBe( - NonBrowsableDriverTypes.OrderBy(t => t, StringComparer.Ordinal).ToList(), - $"the set of driver types committing to the generic \"address\" key drifted. Extra entries are " - + $"browsable drivers whose typed editor will open EMPTY and blank the field on save; missing " - + $"entries gained a typed branch and should leave this list. Got: {string.Join(", ", fellBack)}"); - } - - /// - /// Driver types with no browser, for which the generic address key is the correct commit. - /// - /// Modbus — register addresses are numeric coordinates, not a browsable namespace. - /// Calculation — a pseudo-driver over other tags' RawPaths; there is no device. - /// - /// Adding a type here is a claim that it cannot be browsed. If it can, give it a branch instead. - /// - private static readonly string[] NonBrowsableDriverTypes = - [ - DriverTypeNames.Modbus, - DriverTypeNames.Calculation, - ]; } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/RawBrowseCommitMapperParityTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/RawBrowseCommitMapperParityTests.cs index b8a33313..9d1771f1 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/RawBrowseCommitMapperParityTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/RawBrowseCommitMapperParityTests.cs @@ -30,6 +30,15 @@ namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests.Uns; /// class walks , so a newly-registered driver is triaged the first time /// the suite runs — the idiom DriverFormMapParityTests / DriverDispatchMapParityTests use. /// +/// +/// The only guard over this seam (Gitea #558). DriverDispatchMapParityTests briefly held a +/// second one, enumerating the same mapper against its own hand-authored +/// NonBrowsableDriverTypes. Two lists for one question can only drift, so it was retired into +/// this class: is now the single hand-authored list for browse-commit, +/// and the one assertion the old fact made that nothing here covered — that an exemption still describes +/// the mapper — came with it as +/// . +/// /// /// /// @@ -138,6 +147,70 @@ public sealed class RawBrowseCommitMapperParityTests + "Remove the exemption and give the driver a BuildTagConfig branch."); } + /// + /// The other direction: an exemption must still DESCRIBE the mapper. A driver that has since gained + /// a typed BuildTagConfig branch no longer reaches the generic key, so its exemption is a + /// stale claim — and worse, a silently inert one, since the enumeration above only ever flags + /// drivers that are missing from the list. + /// + /// + /// Inherited from DriverDispatchMapParityTests's retired browse-commit fact, which asserted + /// set EQUALITY against its own hand-authored list and so covered this direction as a side effect + /// (Gitea #558). Consolidating onto one list would have dropped the assertion had it not moved here. + /// Keeping it also keeps the list honest as a piece of documentation: every reason written in + /// is a live description of what the mapper does today, not an + /// archived one. + /// + [Fact] + public void Every_generic_key_exemption_still_commits_to_the_generic_key() + { + var superseded = GenericKeyExemptions + .Where(e => !CommitsToTheGenericAddressKey(e.DriverType)) + .OrderBy(e => e.DriverType, StringComparer.Ordinal) + .ToList(); + + superseded.ShouldBeEmpty( + "these driver types are exempted from needing a typed branch, but RawBrowseCommitMapper now " + + $"gives them one: {string.Join(", ", superseded.Select(e => e.DriverType))}. The exemption is " + + "stale — delete it, so the list keeps meaning what it says."); + } + + /// + /// Every registered browser is indexed under a value, exactly. + /// BrowserSessionService looks a browser up by the driver type persisted on the device, so a + /// browser whose key is anything else is registered but unreachable — the picker reports "no browser + /// for this driver" and the operator falls back to typing the address by hand. + /// + /// + /// Added with Gitea #558, alongside repointing OpcUaClientDriverBrowser and + /// GalaxyDriverBrowser off string literals ("OpcUaClient" / "GalaxyMxGateway") + /// onto the constants. The values matched, so nothing was broken — but nothing prevented the next + /// browser from hand-writing its key either, and a browser project cannot be reached by + /// DriverTypeNamesGuardTests, which pins the driver FACTORIES' names rather than the + /// browsers'. This is the seam where the two meet: the same DI read-back the guards above use + /// already has every browser instance in hand. + /// + /// Compared ORDINALLY, not case-insensitively: BrowserSessionService's index and the + /// persisted DriverType must agree on the exact string, and "differs only in case" is + /// precisely the TwinCat/TwinCAT drift exists to end. + /// + /// + [Fact] + public void Every_registered_browser_is_keyed_by_a_declared_driver_type_name() + { + var declared = DriverTypeNames.All.ToHashSet(StringComparer.Ordinal); + + var unreachable = RegisteredBespokeBrowserDriverTypes() + .Where(t => !declared.Contains(t)) + .OrderBy(t => t, StringComparer.Ordinal) + .ToList(); + + unreachable.ShouldBeEmpty( + "these IDriverBrowser registrations are keyed by a string that is not a DriverTypeNames member, " + + $"so BrowserSessionService can never match a device to them: {string.Join(", ", unreachable)}. " + + "Use the DriverTypeNames constant."); + } + /// An exemption for a driver type that no longer exists is stale bookkeeping. [Fact] public void Every_generic_key_exemption_names_a_declared_driver_type()