merge: #558 — one hand-authored list for the browse-commit seam (stale-exemption direction gained a real guard); browser DriverType literals repointed to DriverTypeNames + Ordinal membership guard

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
     }
 
     /// <inheritdoc />
-    // Hardcoded literal: this project references Driver.Galaxy.Contracts, not Driver.Galaxy,
+    /// <remarks>
-    // so GalaxyDriverFactoryExtensions.DriverTypeName isn't available here.
+    ///     Sourced from the shared <see cref="DriverTypeNames"/> set, reached through
-    public string DriverType => "GalaxyMxGateway";
+    ///     <c>Commons</c>. This project references Driver.Galaxy.Contracts, not Driver.Galaxy, so
+    ///     <c>GalaxyDriverFactoryExtensions.DriverTypeName</c> 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):
+    ///     <see cref="DriverTypeNames.Galaxy"/> holds the same value and is the constant
+    ///     <c>DriverTypeNamesGuardTests</c> already pins to the factory's registration.
+    /// </remarks>
+    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

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
     }
 
     /// <inheritdoc />
-    public string DriverType => "OpcUaClient";
+    /// <remarks>
+    ///     Sourced from the shared <see cref="DriverTypeNames"/> set rather than the hand-written literal
+    ///     this line used to carry (Gitea #558). The value is unchanged; the constant is what
+    ///     <c>DriverTypeNamesGuardTests</c> 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.
+    /// </remarks>
+    public string DriverType => DriverTypeNames.OpcUaClient;
 
     /// <inheritdoc />
     public async Task<IBrowseSession> OpenAsync(string configJson, CancellationToken cancellationToken)

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).

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;
 
 /// <summary>
-///     Parity guards for the two remaining per-driver dispatch maps (<c>deferment.md</c> G-5, G-6).
+///     Parity guards for the per-driver <b>CSV column</b> dispatch map (<c>deferment.md</c> G-5). It was
-///     Both were already testable structures — unlike the two Razor <c>@switch</c> blocks that needed
+///     already a testable structure — unlike the two Razor <c>@switch</c> blocks that needed extracting
-///     extracting first — so nothing but the tests was missing.
+///     first — so nothing but the tests was missing.
 /// </summary>
+/// <remarks>
+///     <b>The browse-commit seam (G-6) is guarded next door, not here (Gitea #558).</b> This class used to
+///     also hold <c>Browse_commit_falls_back_to_the_generic_address_key_for_exactly_the_non_browsable_drivers</c>,
+///     over the same <c>RawBrowseCommitMapper.BuildTagConfig</c> seam
+///     <see cref="RawBrowseCommitMapperParityTests"/> 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 (<c>NonBrowsableDriverTypes</c> really meant "types
+///     that produce the generic key": <c>AbLegacy</c> is genuinely non-browsable yet correctly absent,
+///     because it has a typed branch). The surviving list is
+///     <c>RawBrowseCommitMapperParityTests.GenericKeyExemptions</c>, which is named for what it holds,
+///     carries a written reason per entry, and is checked against a <b>derived</b> browsable set rather
+///     than a second hand-authored one.
+/// </remarks>
 [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)}");
     }
-
-    /// <summary>
-    ///     Every driver type either produces a TYPED blob from browse-commit, or is on the documented
-    ///     non-browsable list. The generic <c>{"address": …}</c> 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 (<c>deferment.md</c> G-6) because the fallback's comment claimed "browsable drivers
-    ///     are all handled above", which stopped being true the moment <c>SqlDriverBrowser</c> registered.
-    ///     <para><b>Asserted in both directions on purpose.</b> 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.</para>
-    /// </summary>
-    [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<string, string>(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)}");
-    }
-
-    /// <summary>
-    ///     Driver types with no browser, for which the generic <c>address</c> key is the correct commit.
-    ///     <list type="bullet">
-    ///       <item><b>Modbus</b> — register addresses are numeric coordinates, not a browsable namespace.</item>
-    ///       <item><b>Calculation</b> — a pseudo-driver over other tags' RawPaths; there is no device.</item>
-    ///     </list>
-    ///     Adding a type here is a claim that it cannot be browsed. If it can, give it a branch instead.
-    /// </summary>
-    private static readonly string[] NonBrowsableDriverTypes =
-    [
-        DriverTypeNames.Modbus,
-        DriverTypeNames.Calculation,
-    ];
 }

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 <see cref="DriverTypeNames.All"/>, so a newly-registered driver is triaged the first time
 ///     the suite runs — the idiom <c>DriverFormMapParityTests</c> / <c>DriverDispatchMapParityTests</c> use.
 ///     </para>
+///     <para>
+///     <b>The only guard over this seam (Gitea #558).</b> <c>DriverDispatchMapParityTests</c> briefly held a
+///     second one, enumerating the same mapper against its own hand-authored
+///     <c>NonBrowsableDriverTypes</c>. Two lists for one question can only drift, so it was retired into
+///     this class: <see cref="GenericKeyExemptions"/> 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
+///     <see cref="Every_generic_key_exemption_still_commits_to_the_generic_key"/>.
+///     </para>
 /// </summary>
 /// <remarks>
 ///     <para>
@@ -138,6 +147,70 @@ public sealed class RawBrowseCommitMapperParityTests
             + "Remove the exemption and give the driver a BuildTagConfig branch.");
     }
 
+    /// <summary>
+    ///     The other direction: an exemption must still DESCRIBE the mapper. A driver that has since gained
+    ///     a typed <c>BuildTagConfig</c> 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 <em>missing</em> from the list.
+    /// </summary>
+    /// <remarks>
+    ///     Inherited from <c>DriverDispatchMapParityTests</c>'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
+    ///     <see cref="GenericKeyExemptions"/> is a live description of what the mapper does today, not an
+    ///     archived one.
+    /// </remarks>
+    [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.");
+    }
+
+    /// <summary>
+    ///     Every registered browser is indexed under a <see cref="DriverTypeNames"/> value, exactly.
+    ///     <c>BrowserSessionService</c> 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.
+    /// </summary>
+    /// <remarks>
+    ///     Added with Gitea #558, alongside repointing <c>OpcUaClientDriverBrowser</c> and
+    ///     <c>GalaxyDriverBrowser</c> off string literals (<c>"OpcUaClient"</c> / <c>"GalaxyMxGateway"</c>)
+    ///     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
+    ///     <c>DriverTypeNamesGuardTests</c>, 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.
+    ///     <para>
+    ///     Compared ORDINALLY, not case-insensitively: <c>BrowserSessionService</c>'s index and the
+    ///     persisted <c>DriverType</c> must agree on the exact string, and "differs only in case" is
+    ///     precisely the <c>TwinCat</c>/<c>TwinCAT</c> drift <see cref="DriverTypeNames"/> exists to end.
+    ///     </para>
+    /// </remarks>
+    [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.");
+    }
+
     /// <summary>An exemption for a driver type that no longer exists is stale bookkeeping.</summary>
     [Fact]
     public void Every_generic_key_exemption_names_a_declared_driver_type()