fix(adminui): guard the driver dispatch maps and close G-1..G-6 (§8.4)
A parity guard already existed for DriverTypeNames <-> the /raw driver picker, but not for the two dispatch maps downstream, so a driver could be registered, offered in the picker, and then have no config form. That is exactly what happened to Calculation (G-1) and to Sql/MTConnect/Calculation on the device modal (G-2) — both survived review because nothing could see them. The maps had to become data first. A Razor @switch compiles into BuildRenderTree's IL, so no test can enumerate its cases; the picker guard works only because RawDriverTypeDialog keeps its data in a field the markup enumerates. Both modals now render from DriverConfigFormMap / DeviceFormMap through <DynamicComponent>, and being public those maps need none of the picker test's BindingFlags.NonPublic fragility. - G-1 CalculationDriverForm.razor — RunTimeout was unauthorable via the UI. - G-2 Sql/MTConnect/Calculation declared single-connection rather than given hollow device forms; each holds one connection at the driver level. - G-3 DriverConfigModal's hardcoded "Galaxy or Mqtt" -> IsSingleConnection, so Sql/MTConnect authors are no longer sent to a device form with no endpoint. - G-4 DriverFormMapParityTests (5, both directions) + DriverDispatchMapParityTests. - G-5 CsvColumnMap entries for Sql, Mqtt, MTConnect. - G-6 RawBrowseCommitMapper Sql branch — and the browser end, which the audit missed: SqlBrowseSession emitted NO AddressFields, so a browsed leaf carried only a column name, and a column alone cannot address a Sql tag. It now travels schema/table/column and the mapper builds a WideRow config from them. A branch alone would have produced a half-built blob. Two findings while writing the guards. The G-6 test showed Modbus and Calculation also hit the generic address fallback — correctly, as neither is browsable — so it asserts the fall-through set EQUALS a documented non-browsable list in both directions; a one-way check would let a newly browsable driver be quietly added to the exclusion list. And TagConfigDriverTypeNameGuardTests was hollow: it enumerated a hand-written TheoryData that had already drifted (omitting Galaxy, Sql and Mqtt), guarding renames but not gaps. Now enumerated from the map with a coverage test facing the other way. Live-verified on docker-dev, since this repo has no bUnit and no unit test reaches Blazor parameter binding: opened Calculation's config (previously "No typed config form"), typed 3500, saved, reopened — the value persisted, so the DynamicComponent two-way binding round-trips. Sql's form renders fully and now reads "This driver holds a single connection, authored above". AdminUI.Tests 930 passed.
This commit is contained in:
Joseph Doherty
@@ -0,0 +1,104 @@
|
||||
using Shouldly;
|
||||
using Xunit;
|
||||
using ZB.MOM.WW.OtOpcUa.AdminUI.Components.Shared.Drivers;
|
||||
using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
|
||||
|
||||
namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests.Drivers;
|
||||
|
||||
/// <summary>
|
||||
/// Closes <c>deferment.md</c> G-4. A guard already existed for <c>DriverTypeNames</c> ↔ the <c>/raw</c>
|
||||
/// driver PICKER, but not for the two dispatch maps downstream of it, so a driver could be registered,
|
||||
/// offered in the picker, and then have no config form — which is exactly what happened to
|
||||
/// <c>Calculation</c> (G-1) and to Sql/MTConnect/Calculation on the device modal (G-2). Both survived
|
||||
/// review because nothing could see them.
|
||||
/// <para>These are ordinary map lookups rather than reflection over a private field: the two modals were
|
||||
/// converted from a Razor <c>@switch</c> — unreachable from a test, since it compiles into
|
||||
/// <c>BuildRenderTree</c>'s IL — to <c>DriverConfigFormMap</c> / <c>DeviceFormMap</c>. There is no bUnit
|
||||
/// in this project, so a map is the only thing a test can hold on to.</para>
|
||||
/// </summary>
|
||||
[Trait("Category", "Unit")]
|
||||
public sealed class DriverFormMapParityTests
|
||||
{
|
||||
/// <summary>Every declared driver type must have a typed config form. There is no exemption: a driver
|
||||
/// with no configurable surface still needs a form to say so, or the operator meets a bare warning.</summary>
|
||||
[Fact]
|
||||
public void Every_declared_driver_type_has_a_typed_config_form()
|
||||
{
|
||||
var missing = DriverTypeNames.All
|
||||
.Where(t => DriverConfigFormMap.Resolve(t) is null)
|
||||
.OrderBy(t => t, StringComparer.Ordinal)
|
||||
.ToList();
|
||||
|
||||
missing.ShouldBeEmpty(
|
||||
$"these driver types are registered and offered in the /raw picker but DriverConfigModal has no "
|
||||
+ $"form for them, so their config is unauthorable through the UI: {string.Join(", ", missing)}");
|
||||
}
|
||||
|
||||
/// <summary>The reverse direction — a mapped type that is no longer a declared driver type is a stale
|
||||
/// entry, and would keep a deleted driver's form reachable.</summary>
|
||||
[Fact]
|
||||
public void Every_config_form_maps_to_a_declared_driver_type()
|
||||
{
|
||||
var declared = DriverTypeNames.All.ToHashSet(StringComparer.OrdinalIgnoreCase);
|
||||
var stale = DriverConfigFormMap.MappedDriverTypes
|
||||
.Where(t => !declared.Contains(t))
|
||||
.OrderBy(t => t, StringComparer.Ordinal)
|
||||
.ToList();
|
||||
|
||||
stale.ShouldBeEmpty($"DriverConfigFormMap has entries for undeclared driver types: {string.Join(", ", stale)}");
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Every declared driver type must EITHER have a typed device form OR be declared single-connection.
|
||||
/// <para>The either/or matters: demanding a device form from every driver would be wrong (Sql has one
|
||||
/// connection string, MTConnect one Agent URI, Calculation no connection at all), and a test that has
|
||||
/// to be suppressed for five drivers stops being read. Forcing the choice to be explicit is what makes
|
||||
/// this a guard — a new driver cannot silently fall through.</para>
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void Every_declared_driver_type_has_a_device_form_or_is_declared_single_connection()
|
||||
{
|
||||
var unaccounted = DriverTypeNames.All
|
||||
.Where(t => DeviceFormMap.Resolve(t) is null && !DeviceFormMap.IsSingleConnection(t))
|
||||
.OrderBy(t => t, StringComparer.Ordinal)
|
||||
.ToList();
|
||||
|
||||
unaccounted.ShouldBeEmpty(
|
||||
$"these driver types have neither a typed device form nor a place in "
|
||||
+ $"DeviceFormMap.SingleConnectionDriverTypes, so DeviceModal falls through to its "
|
||||
+ $"\"no typed device form\" warning: {string.Join(", ", unaccounted)}");
|
||||
}
|
||||
|
||||
/// <summary>The reverse direction for the device map.</summary>
|
||||
[Fact]
|
||||
public void Every_device_form_maps_to_a_declared_driver_type()
|
||||
{
|
||||
var declared = DriverTypeNames.All.ToHashSet(StringComparer.OrdinalIgnoreCase);
|
||||
var stale = DeviceFormMap.MappedDriverTypes
|
||||
.Concat(DeviceFormMap.SingleConnectionDriverTypes)
|
||||
.Where(t => !declared.Contains(t))
|
||||
.Distinct(StringComparer.OrdinalIgnoreCase)
|
||||
.OrderBy(t => t, StringComparer.Ordinal)
|
||||
.ToList();
|
||||
|
||||
stale.ShouldBeEmpty($"DeviceFormMap references undeclared driver types: {string.Join(", ", stale)}");
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// A driver that authors its connection on the DRIVER form must have a driver form to author it on.
|
||||
/// Without this, declaring a type single-connection would be a way to opt out of both maps at once
|
||||
/// and leave the connection unauthorable anywhere.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void Every_single_connection_driver_has_a_driver_config_form()
|
||||
{
|
||||
var missing = DeviceFormMap.SingleConnectionDriverTypes
|
||||
.Where(t => DriverConfigFormMap.Resolve(t) is null)
|
||||
.OrderBy(t => t, StringComparer.Ordinal)
|
||||
.ToList();
|
||||
|
||||
missing.ShouldBeEmpty(
|
||||
$"these types are declared single-connection but have no driver config form, so their connection "
|
||||
+ $"cannot be authored anywhere: {string.Join(", ", missing)}");
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,98 @@
|
||||
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).
|
||||
/// Both were already testable structures — unlike the two Razor <c>@switch</c> blocks that needed
|
||||
/// extracting first — so nothing but the tests was missing.
|
||||
/// </summary>
|
||||
[Trait("Category", "Unit")]
|
||||
public sealed class DriverDispatchMapParityTests
|
||||
{
|
||||
/// <summary>
|
||||
/// Driver types with a typed tag editor but no CSV map, so import/export silently degrades them to
|
||||
/// the raw <c>TagConfigJson</c> fallback. Empty — every editor-backed driver now has typed columns.
|
||||
/// <para>Deliberately expressed as "has a typed editor ⇒ has typed CSV columns" rather than
|
||||
/// "every driver type", because a driver with no typed editor has nothing to project into columns.
|
||||
/// Tying the two maps together is what makes this catch the next omission.</para>
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void Every_driver_with_a_typed_tag_editor_has_typed_csv_columns()
|
||||
{
|
||||
var missing = DriverTypeNames.All
|
||||
.Where(t => TagConfigEditorMap.Resolve(t) is not null && CsvColumnMap.Resolve(t) is null)
|
||||
.OrderBy(t => t, StringComparer.Ordinal)
|
||||
.ToList();
|
||||
|
||||
missing.ShouldBeEmpty(
|
||||
$"these driver types have a typed tag editor but no CsvColumnMap entry, so CSV import/export "
|
||||
+ $"drops them to the raw TagConfigJson fallback: {string.Join(", ", missing)}");
|
||||
}
|
||||
|
||||
/// <summary>A CSV map for a driver type that no longer exists is a stale entry.</summary>
|
||||
[Fact]
|
||||
public void Every_csv_map_targets_a_declared_driver_type()
|
||||
{
|
||||
var declared = DriverTypeNames.All.ToHashSet(StringComparer.OrdinalIgnoreCase);
|
||||
var stale = CsvColumnMap.All
|
||||
.Select(m => m.DriverType)
|
||||
.Where(t => !declared.Contains(t))
|
||||
.OrderBy(t => t, StringComparer.Ordinal)
|
||||
.ToList();
|
||||
|
||||
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,
|
||||
];
|
||||
}
|
||||
+37
-12
@@ -17,18 +17,43 @@ namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests.Uns;
|
||||
public sealed class TagConfigDriverTypeNameGuardTests
|
||||
{
|
||||
// Every DriverTypeNames constant that has a typed editor registered.
|
||||
public static TheoryData<string> EditorMappedDriverTypes() =>
|
||||
[
|
||||
DriverTypeNames.Modbus,
|
||||
DriverTypeNames.S7,
|
||||
DriverTypeNames.AbCip,
|
||||
DriverTypeNames.AbLegacy,
|
||||
DriverTypeNames.TwinCAT,
|
||||
DriverTypeNames.FOCAS,
|
||||
DriverTypeNames.OpcUaClient,
|
||||
DriverTypeNames.Calculation,
|
||||
DriverTypeNames.MTConnect,
|
||||
];
|
||||
//
|
||||
// Enumerated FROM the map rather than hand-written. The hand-written list this replaced guarded
|
||||
// against RENAMES but not against GAPS: a newly-added DriverTypeNames constant simply never appeared
|
||||
// here, so the guard stayed green while the map went unmapped — and it had already silently drifted,
|
||||
// omitting Galaxy, Sql and Mqtt. Pairing it with the coverage test below makes both directions real.
|
||||
public static TheoryData<string> EditorMappedDriverTypes()
|
||||
{
|
||||
var data = new TheoryData<string>();
|
||||
foreach (var t in DriverTypeNames.All.Where(t => TagConfigEditorMap.Resolve(t) is not null))
|
||||
{
|
||||
data.Add(t);
|
||||
}
|
||||
|
||||
return data;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Coverage direction: every declared driver type has a typed tag editor, except those documented as
|
||||
/// having none. Without this, enumerating the map above proves only that the map agrees with itself.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void Every_declared_driver_type_has_a_typed_tag_editor_or_is_documented_as_raw_json()
|
||||
{
|
||||
// Galaxy authors its tag reference as a dotted FullName through the Galaxy address picker rather
|
||||
// than a typed field editor, so it uses the generic raw-TagConfig-JSON textarea by design.
|
||||
string[] rawJsonByDesign = [DriverTypeNames.Galaxy];
|
||||
|
||||
var missing = DriverTypeNames.All
|
||||
.Where(t => TagConfigEditorMap.Resolve(t) is null)
|
||||
.Except(rawJsonByDesign, StringComparer.OrdinalIgnoreCase)
|
||||
.OrderBy(t => t, StringComparer.Ordinal)
|
||||
.ToList();
|
||||
|
||||
missing.ShouldBeEmpty(
|
||||
$"these driver types have no typed tag editor and are not documented as raw-JSON by design, so "
|
||||
+ $"the /uns TagModal falls back to a bare JSON textarea for them: {string.Join(", ", missing)}");
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[MemberData(nameof(EditorMappedDriverTypes))]
|
||||
|
||||
Reference in New Issue
Block a user