diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/AdminOperations/EquipmentTagConfigInspector.cs b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/AdminOperations/EquipmentTagConfigInspector.cs index 7e8a25e5..b17ad450 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/AdminOperations/EquipmentTagConfigInspector.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/AdminOperations/EquipmentTagConfigInspector.cs @@ -1,10 +1,14 @@ using ZB.MOM.WW.OtOpcUa.Core.Abstractions; using ZB.MOM.WW.OtOpcUa.Driver.AbCip; using ZB.MOM.WW.OtOpcUa.Driver.AbLegacy; +using ZB.MOM.WW.OtOpcUa.Driver.Calculation; using ZB.MOM.WW.OtOpcUa.Driver.FOCAS; using ZB.MOM.WW.OtOpcUa.Driver.Modbus; using ZB.MOM.WW.OtOpcUa.Driver.Mqtt; +using ZB.MOM.WW.OtOpcUa.Driver.MTConnect; +using ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient; using ZB.MOM.WW.OtOpcUa.Driver.S7; +using ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts; using ZB.MOM.WW.OtOpcUa.Driver.TwinCAT; namespace ZB.MOM.WW.OtOpcUa.ControlPlane.AdminOperations; @@ -20,13 +24,19 @@ namespace ZB.MOM.WW.OtOpcUa.ControlPlane.AdminOperations; /// *.Contracts assemblies — so a driver that gains an inspector cannot be silently left out. This /// is the same guard shape the driver-form dispatch maps use, and for the same reason: the previous /// hand-maintained list had drifted (MQTT shipped an Inspect and was never added). -/// Residual gap, deliberately recorded rather than papered over. Galaxy, OpcUaClient, Sql, -/// MTConnect and Calculation have no Inspect in a Contracts assembly, so this deploy-time -/// gate cannot check them — while the AdminUI's TagConfigValidator DOES cover all but Galaxy via -/// its *TagConfigModel.Validate() models, which live in the AdminUI project and cannot be -/// referenced from here. The asymmetry runs the wrong way: config authored through the modal is checked, -/// but config arriving by CSV import or API is not. Closing it means promoting those models (or an -/// equivalent parser) into each driver's Contracts assembly. Tracked as deferment-register G-7. +/// The G-7 gap is CLOSED (2026-08-09). Sql, MTConnect, Calculation and OpcUaClient used to +/// have no Inspect in a Contracts assembly, so this gate was blind to them while the AdminUI's +/// TagConfigValidator checked them at authoring — the asymmetry ran the wrong way, since config +/// arriving by CSV import or API meets ONLY this gate. Each now ships a Contracts-side +/// *TagDefinitionFactory.Inspect, wired below. Three of the four wrap the driver's own runtime +/// parse rather than mirroring the AdminUI model, which is what keeps the gate from ever being stricter +/// than the runtime: SqlTagDefinitionFactory wraps SqlEquipmentTagParser, and the +/// MTConnect / OpcUaClient / Calculation blob readers were MOVED into their Contracts assemblies (the +/// drivers now call them) rather than copied. +/// Galaxy is the sole remaining unmapped type, deliberately. Its tag reference is a dotted +/// tag_name.AttributeName authored through the Galaxy address picker, with no typed shape to +/// check — the AdminUI has no typed editor for it either (it uses the raw-JSON textarea), so this is +/// symmetric rather than a gap. It gains an entry here if it ever gains a Contracts-side parser. /// public static class EquipmentTagConfigInspector { @@ -40,6 +50,11 @@ public static class EquipmentTagConfigInspector [DriverTypeNames.TwinCAT] = TwinCATTagDefinitionFactory.Inspect, [DriverTypeNames.FOCAS] = FocasTagDefinitionFactory.Inspect, [DriverTypeNames.Mqtt] = MqttTagDefinitionFactory.Inspect, + // G-7 (2026-08-09): the four the gate used to be blind to. + [DriverTypeNames.Sql] = SqlTagDefinitionFactory.Inspect, + [DriverTypeNames.MTConnect] = MTConnectTagDefinitionFactory.Inspect, + [DriverTypeNames.Calculation] = CalculationTagDefinitionFactory.Inspect, + [DriverTypeNames.OpcUaClient] = OpcUaClientTagDefinitionFactory.Inspect, }; /// Inspects a tag's TagConfig for the given driver type. Returns the driver's warnings, diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/ZB.MOM.WW.OtOpcUa.ControlPlane.csproj b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/ZB.MOM.WW.OtOpcUa.ControlPlane.csproj index 05f6a470..e4f43208 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/ZB.MOM.WW.OtOpcUa.ControlPlane.csproj +++ b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/ZB.MOM.WW.OtOpcUa.ControlPlane.csproj @@ -50,6 +50,18 @@ + + + + + diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/TagConfigAuthoringDeployParityTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/TagConfigAuthoringDeployParityTests.cs new file mode 100644 index 00000000..7dcac8ce --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/TagConfigAuthoringDeployParityTests.cs @@ -0,0 +1,146 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.AdminUI.Uns.TagEditors; +using ZB.MOM.WW.OtOpcUa.ControlPlane.AdminOperations; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests.Uns; + +/// +/// Deferment-register G-7 pin: the AdminUI's authoring-time and the +/// deploy-time must agree in the direction that matters — +/// anything the modal lets an operator save must sail through the deploy gate. +/// +/// The reverse direction is deliberately NOT asserted, and must not be: the deploy gate follows the +/// runtime parser, which is lenient in places the editor is opinionated (a calc tag with no +/// trigger, a sub-50 ms timer). Config authored by CSV import or API meets only the deploy gate, so +/// a gate stricter than the runtime would block deployments the driver would have served — and the +/// gate's warnings can be configured (TagConfigValidationMode.Error) into blocking errors. +/// +/// +/// The configs are produced by the models' own ToJson() rather than hand-written JSON, +/// which is the only way this proves anything about the real authoring path: a hand-written blob is +/// a guess at what the editor writes, and the editors normalise keys on save (MTConnect collapses +/// dataItemId/address onto fullName; Sql rewrites its enums as names). This +/// lives in AdminUI.Tests because it is the only suite that can see both sides — the models are +/// internal to the AdminUI project, which references ControlPlane. +/// +/// +public sealed class TagConfigAuthoringDeployParityTests +{ + /// A Sql tag as the typed editor saves it: both the editor and the gate accept it. + [Fact] + public void Sql_config_the_editor_accepts_draws_no_deploy_warning() + { + var model = new SqlTagConfigModel + { + Model = ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts.SqlTagModel.KeyValue, + Table = "dbo.TagValues", + KeyColumn = "TagName", + KeyValue = "Line1.Speed", + ValueColumn = "Value", + Type = DriverDataType.Float32, + }; + AssertParity(DriverTypeNames.Sql, model.Validate(), model.ToJson()); + } + + /// The WideRow shape too — its row selector is the half of the parser a KeyValue tag never + /// exercises, and the editor and the parser have to agree that a where-pair needs BOTH keys. + [Fact] + public void Sql_wideRow_config_the_editor_accepts_draws_no_deploy_warning() + { + var model = new SqlTagConfigModel + { + Model = ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts.SqlTagModel.WideRow, + Table = "dbo.LineSnapshot", + ColumnName = "Speed", + RowSelector = new SqlRowSelectorModel { WhereColumn = "LineId", WhereValue = "L1" }, + }; + AssertParity(DriverTypeNames.Sql, model.Validate(), model.ToJson()); + } + + [Fact] + public void MTConnect_config_the_editor_accepts_draws_no_deploy_warning() + { + var model = new MTConnectTagConfigModel + { + FullName = "Xact", + DataType = DriverDataType.Float64, + MtCategory = "SAMPLE", + MtType = "POSITION", + Units = "MILLIMETER", + }; + AssertParity(DriverTypeNames.MTConnect, model.Validate(), model.ToJson()); + } + + [Fact] + public void Calculation_config_the_editor_accepts_draws_no_deploy_warning() + { + var model = new CalculationTagConfigModel + { + ScriptId = "8f1c2f6e-0000-4000-8000-000000000001", + ChangeTriggered = true, + TimerIntervalMs = 5000, + }; + AssertParity(DriverTypeNames.Calculation, model.Validate(), model.ToJson()); + } + + /// Change-trigger only — the editor omits timerIntervalMs entirely rather than writing a + /// null or a zero, and an absent key must not read as a discarded value at the gate. + [Fact] + public void Calculation_change_triggered_only_config_draws_no_deploy_warning() + { + var model = new CalculationTagConfigModel { ScriptId = "S1", ChangeTriggered = true }; + AssertParity(DriverTypeNames.Calculation, model.Validate(), model.ToJson()); + } + + [Fact] + public void OpcUaClient_config_the_editor_accepts_draws_no_deploy_warning() + { + var model = new OpcUaClientTagConfigModel { NodeId = "nsu=urn:plc:sim;s=Line1.Speed" }; + AssertParity(DriverTypeNames.OpcUaClient, model.Validate(), model.ToJson()); + } + + /// + /// Falsifiability control. Every assertion above would also pass against a build where the four G-7 + /// inspectors were never wired — an unmapped driver type returns no warnings for anything. This one + /// proves the gate is actually looking: the same driver types must warn about a blob the editor + /// itself would refuse. + /// + [Theory] + [InlineData(DriverTypeNames.Sql, """{"model":"KeyValu","table":"dbo.T"}""")] + // Note the MTConnect case is the DIGITS-only dataType, not a spelling typo: MTConnectTagConfigModel + // reads dataType leniently (an unrecognised NAME silently loads as String, and a save rewrites it), so + // "Strng" is accepted at authoring while the strict runtime rejects it — a pre-existing + // authoring-weaker-than-runtime gap in the editor, untouched here and out of G-7's scope. It does not + // affect the parity direction this file pins, because a blob saved through the modal always carries a + // valid NAME. + [InlineData(DriverTypeNames.MTConnect, """{"fullName":"avail","dataType":"8"}""")] + [InlineData(DriverTypeNames.Calculation, """{"changeTriggered":true}""")] + [InlineData(DriverTypeNames.OpcUaClient, """{"nodeId":""}""")] + public void The_same_driver_types_do_warn_about_a_config_the_editor_refuses(string driverType, string tagConfig) + { + TagConfigValidator.Validate(driverType, tagConfig).ShouldNotBeNullOrEmpty( + $"{driverType}: the authoring gate is expected to refuse this blob — if it accepts it, the " + + "control below is testing the wrong thing."); + EquipmentTagConfigInspector.Inspect(driverType, tagConfig).ShouldNotBeEmpty( + $"{driverType}: the deploy gate accepted a blob the modal refuses, so config arriving by CSV " + + "import or API is unchecked — the exact G-7 asymmetry."); + } + + /// Asserts the editor accepted the config (so the parity claim is not vacuous) and that the + /// deploy gate then found nothing to say about it. + /// The canonical driver-type string both gates dispatch on. + /// What the model's own Validate() returned. + /// The config the model serialised. + private static void AssertParity(string driverType, string? validationError, string tagConfig) + { + validationError.ShouldBeNull( + $"{driverType}: the model must accept this config, or the parity assertion below proves nothing."); + TagConfigValidator.Validate(driverType, tagConfig).ShouldBeNull( + $"{driverType}: the dispatched authoring validator must accept what the model itself accepts."); + EquipmentTagConfigInspector.Inspect(driverType, tagConfig).ShouldBeEmpty( + $"{driverType}: the deploy gate must never be stricter than the authoring gate — a config the " + + "operator was allowed to save cannot draw a deploy warning."); + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/EquipmentTagConfigInspectorCoverageTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/EquipmentTagConfigInspectorCoverageTests.cs index d453cc3e..d6b7aed9 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/EquipmentTagConfigInspectorCoverageTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/EquipmentTagConfigInspectorCoverageTests.cs @@ -78,9 +78,13 @@ public sealed class EquipmentTagConfigInspectorCoverageTests /// project-referenced assembly to the output regardless of IL usage, so the directory is the honest /// source. /// Boundary, stated so it is not mistaken for more: this sees the Contracts assemblies - /// ControlPlane project-references. A driver whose Contracts project is not referenced at all (Sql, - /// MTConnect, Calculation, OpcUaClient) is invisible here — adding the reference is itself the act - /// that makes this guard start demanding a map entry. + /// ControlPlane project-references. A driver whose Contracts project is not referenced at all is + /// invisible here — adding the reference is itself the act that makes this guard start demanding a + /// map entry. That was the whole shape of deferment-register G-7: Sql, MTConnect, Calculation and + /// OpcUaClient were unreferenced and therefore unseen. All four were referenced and wired on + /// 2026-08-09, so the only driver left outside this sweep is Galaxy, which ships no Contracts-side + /// parser at all (its dotted tag_name.AttributeName reference has no typed shape to check, and + /// the AdminUI has no typed editor for it either). /// private static IReadOnlyList DiscoverInspectableFactories() => Directory.GetFiles(AppContext.BaseDirectory, "ZB.MOM.WW.OtOpcUa.Driver.*.Contracts.dll") diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/EquipmentTagConfigInspectorTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/EquipmentTagConfigInspectorTests.cs index b99a5052..0a9b19da 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/EquipmentTagConfigInspectorTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/EquipmentTagConfigInspectorTests.cs @@ -5,7 +5,11 @@ using ZB.MOM.WW.OtOpcUa.ControlPlane.AdminOperations; namespace ZB.MOM.WW.OtOpcUa.ControlPlane.Tests; /// R2-11 (05/CONV-2): the DriverType-dispatched deploy-time TagConfig inspector — known drivers -/// route to their parser's Inspect; unmapped drivers (Galaxy/OpcUaClient) skip; keys are case-insensitive. +/// route to their parser's Inspect; the one unmapped driver (Galaxy) skips; keys are case-insensitive. +/// The Sql / MTConnect / Calculation / OpcUaClient cases below landed with deferment-register G-7 +/// (2026-08-09), which gave those four a Contracts-side Inspect. Each follows the same three-case +/// shape as its siblings: a canonical valid config warns about nothing, a present-but-invalid field draws a +/// warning that NAMES the field, and structurally broken JSON draws a warning without throwing. public sealed class EquipmentTagConfigInspectorTests { [Fact] @@ -25,9 +29,11 @@ public sealed class EquipmentTagConfigInspectorTests .ShouldBeEmpty(); } + // Galaxy is the last unmapped type and is so by design (no Contracts-side parser, and no typed AdminUI + // editor either). "Unknown" is the never-heard-of-it control. OpcUaClient used to sit in this list — + // G-7 moved it out; the pair of tests below is the pin that it cannot silently drift back. [Theory] [InlineData("GalaxyMxGateway")] - [InlineData("OpcUaClient")] [InlineData("Unknown")] public void Unmapped_driver_types_are_skipped(string driverType) { @@ -35,16 +41,169 @@ public sealed class EquipmentTagConfigInspectorTests EquipmentTagConfigInspector.IsMapped(driverType).ShouldBeFalse(); } + /// G-7: the four drivers the deploy gate used to be blind to are dispatched now. Named + /// explicitly rather than derived, so a map entry deleted alongside its Contracts reference (which would + /// take the reflection coverage guard dark with it) still fails something. + [Theory] + [InlineData("Sql")] + [InlineData("MTConnect")] + [InlineData("Calculation")] + [InlineData("OpcUaClient")] + public void The_G7_driver_types_are_mapped(string driverType) + => EquipmentTagConfigInspector.IsMapped(driverType).ShouldBeTrue(driverType); + [Theory] [InlineData("modbus")] [InlineData("FOCAS")] [InlineData("focas")] [InlineData("twincat")] + [InlineData("sql")] + [InlineData("mtconnect")] + [InlineData("calculation")] + [InlineData("opcuaclient")] public void Driver_type_matching_is_case_insensitive(string driverType) { EquipmentTagConfigInspector.IsMapped(driverType).ShouldBeTrue(); } + // ---- G-7: Sql ---- + + [Fact] + public void Clean_sql_config_has_no_warnings() + => EquipmentTagConfigInspector.Inspect( + "Sql", + """{"model":"KeyValue","table":"dbo.TagValues","keyColumn":"TagName","keyValue":"Line1.Speed","valueColumn":"Value"}""") + .ShouldBeEmpty(); + + [Fact] + public void Sql_typo_in_model_names_the_field() + { + var warnings = EquipmentTagConfigInspector.Inspect( + "Sql", + """{"model":"KeyValu","table":"dbo.T","keyColumn":"k","keyValue":"v","valueColumn":"val"}"""); + warnings.ShouldContain(w => w.Contains("model") && w.Contains("KeyValu")); + } + + [Fact] + public void Sql_deferred_query_model_is_reported() + => EquipmentTagConfigInspector.Inspect("Sql", """{"model":"Query","table":"dbo.T"}""") + .ShouldContain(w => w.Contains("Query")); + + [Fact] + public void Sql_unparseable_config_warns_without_throwing() + { + IReadOnlyList warnings = []; + Should.NotThrow(() => warnings = EquipmentTagConfigInspector.Inspect("Sql", "{not json")); + warnings.ShouldNotBeEmpty(); + } + + // ---- G-7: MTConnect ---- + + [Fact] + public void Clean_mtconnect_config_has_no_warnings() + => EquipmentTagConfigInspector.Inspect( + "MTConnect", """{"fullName":"avail","dataType":"String"}""") + .ShouldBeEmpty(); + + [Fact] + public void MTConnect_typo_in_dataType_names_the_field() + { + var warnings = EquipmentTagConfigInspector.Inspect( + "MTConnect", """{"fullName":"avail","dataType":"Strng"}"""); + warnings.ShouldContain(w => w.Contains("dataType") && w.Contains("Strng")); + } + + /// The numerically-serialized-enum trap: Enum.TryParse takes "8" as readily as a name, + /// so a digits-only value must be reported rather than silently becoming member 8. + [Fact] + public void MTConnect_numeric_dataType_is_reported() + => EquipmentTagConfigInspector.Inspect("MTConnect", """{"fullName":"avail","dataType":"8"}""") + .ShouldContain(w => w.Contains("dataType")); + + [Fact] + public void MTConnect_config_naming_no_dataitem_is_reported() + => EquipmentTagConfigInspector.Inspect("MTConnect", """{"dataType":"String"}""") + .ShouldContain(w => w.Contains("fullName")); + + [Fact] + public void MTConnect_unparseable_config_warns_without_throwing() + { + IReadOnlyList warnings = []; + Should.NotThrow(() => warnings = EquipmentTagConfigInspector.Inspect("MTConnect", "{not json")); + warnings.ShouldNotBeEmpty(); + } + + // ---- G-7: Calculation ---- + + [Fact] + public void Clean_calculation_config_has_no_warnings() + => EquipmentTagConfigInspector.Inspect( + "Calculation", """{"scriptId":"S1","changeTriggered":true,"timerIntervalMs":5000}""") + .ShouldBeEmpty(); + + [Fact] + public void Calculation_non_boolean_changeTriggered_names_the_field() + { + var warnings = EquipmentTagConfigInspector.Inspect( + "Calculation", """{"scriptId":"S1","changeTriggered":"false"}"""); + warnings.ShouldContain(w => w.Contains("changeTriggered")); + } + + [Fact] + public void Calculation_non_numeric_timer_names_the_field() + => EquipmentTagConfigInspector.Inspect("Calculation", """{"scriptId":"S1","timerIntervalMs":"5000"}""") + .ShouldContain(w => w.Contains("timerIntervalMs")); + + [Fact] + public void Calculation_config_naming_no_script_is_reported() + => EquipmentTagConfigInspector.Inspect("Calculation", """{"changeTriggered":true}""") + .ShouldContain(w => w.Contains("scriptId")); + + /// Falsification control for the never-stricter-than-the-runtime rule: the calc driver ACCEPTS + /// a tag with no trigger at all (it simply never computes) and honours a sub-50 ms timer the AdminUI + /// editor refuses, so the deploy gate — whose warnings can be configured into blocking errors — must + /// stay silent on both. + [Theory] + [InlineData("""{"scriptId":"S1","changeTriggered":false}""")] + [InlineData("""{"scriptId":"S1","changeTriggered":false,"timerIntervalMs":10}""")] + public void Calculation_shapes_the_runtime_accepts_draw_no_warning(string tagConfig) + => EquipmentTagConfigInspector.Inspect("Calculation", tagConfig).ShouldBeEmpty(); + + [Fact] + public void Calculation_unparseable_config_warns_without_throwing() + { + IReadOnlyList warnings = []; + Should.NotThrow(() => warnings = EquipmentTagConfigInspector.Inspect("Calculation", "{not json")); + warnings.ShouldNotBeEmpty(); + } + + // ---- G-7: OpcUaClient ---- + + [Fact] + public void Clean_opcuaclient_config_has_no_warnings() + => EquipmentTagConfigInspector.Inspect( + "OpcUaClient", """{"nodeId":"nsu=urn:plc:sim;s=Line1.Speed"}""") + .ShouldBeEmpty(); + + // The OpcUaClient blob carries no enum, so its present-but-unusable case is a nodeId that is blank or + // not a string — both of which the driver drops from its resolution table without a word. + [Theory] + [InlineData("""{"nodeId":""}""")] + [InlineData("""{"nodeId":" "}""")] + [InlineData("""{"nodeId":123}""")] + [InlineData("""{"tagName":"Line1.Speed"}""")] + public void OpcUaClient_unusable_nodeId_names_the_field(string tagConfig) + => EquipmentTagConfigInspector.Inspect("OpcUaClient", tagConfig) + .ShouldContain(w => w.Contains("nodeId")); + + [Fact] + public void OpcUaClient_unparseable_config_warns_without_throwing() + { + IReadOnlyList warnings = []; + Should.NotThrow(() => warnings = EquipmentTagConfigInspector.Inspect("OpcUaClient", "{not json")); + warnings.ShouldNotBeEmpty(); + } + [Fact] public void Focas_writable_true_dispatches_read_only_warning() {