From f5475868f79a59e1e1ec7bdae4cda81d037f3b25 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 10 Aug 2026 03:07:18 -0400 Subject: [PATCH] fix(inspect): trim before the framing check; stop advisories swallowing the backstop (#556); pin the MQTT payloadFormat mode contract (#557 item 2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #556 item 1 — leading whitespace defeated every Inspect's pre-filter. Seven factories (Modbus/S7/AbCip/AbLegacy/TwinCAT/FOCAS/Mqtt) tested `reference[0] != '{'`, so a padded blob returned EMPTY and a tag the driver refuses deployed clean. The check now trims first, matching the idiom Sql/Calculation/MTConnect/OpcUaClient already used. The raw reference is still what is handed to the mapper — the driver reads the stored blob, not a trimmed copy — so the gate keeps mirroring the runtime rather than becoming lenient. Both directions are pinned: a padded blob the mapper accepts still draws nothing, and one that is not object-framed AFTER trimming still draws nothing (it is not an equipment-tag TagConfig at all). Six of the seven mappers hand the blob to a whitespace-tolerant JsonDocument.Parse, so their verdict is unchanged by padding. MQTT is the exception: both its readers carry the same framing gate, so a padded blob is genuinely refused — which is why the untrimmed pre-filter was a real defect there and not a cosmetic mismatch. #556 item 2 — FOCAS and MQTT appended advisories unconditionally and then gated the catch-all rejection message on `warnings.Count == 0`, so an advisory (which fires on ACCEPTED blobs too, and says nothing about a refusal) silently swallowed the only message naming a refusal nothing else can. The two classes now accumulate in SEPARATE lists and the backstop gates on the rejection count alone — structural, not string matching. MQTT's swallow was reachable in production (a padded blob its readers refuse while every describe branch finds the parsed document well-formed); FOCAS's backstop is currently unreachable by audit, so its guard is protective. #557 item 2 — decided and pinned: KEEP the union verdict. Evidence: FromSparkplugTagConfig hard-codes PayloadFormat=Json and never consults the key; only MqttSubscriptionManager (the Plain ingest path) decodes by it; and MqttTagConfigModel.ToJson deliberately omits the key entirely in Sparkplug mode (#513). So a Sparkplug driver genuinely serves the tag and rejecting it would be stricter than the runtime. The cost — served behaviour depends on the driver's MqttMode, which no tag row records — is paid by an ADVISORY naming the key and the mode dependency, not by changing the verdict, so both gates keep agreeing. The same invalid value on a blob the union REFUSES stays a rejection; the class is decided by the accept gate, never by the key. Falsified: reverting Modbus's trim reddens the padded-rejection theory; neutering MQTT's backstop guard to the pre-#556 shape reddens the advisory-plus-unnameable-rejection test; disabling the #557 advisory reddens the contract pin; and for FOCAS, removing a describe branch alone stays green (the backstop covers it) while additionally neutering the guard goes red. --- .../AbCipTagDefinitionFactory.cs | 10 +- .../AbLegacyTagDefinitionFactory.cs | 10 +- .../FocasTagDefinitionFactory.cs | 81 +++++++--- .../ModbusTagDefinitionFactory.cs | 10 +- .../MqttTagDefinitionFactory.cs | 141 ++++++++++++++---- .../S7TagDefinitionFactory.cs | 10 +- .../TwinCATTagDefinitionFactory.cs | 10 +- .../AbCipInspectAcceptGateTests.cs | 61 ++++++++ .../AbLegacyInspectAcceptGateTests.cs | 61 ++++++++ .../FocasInspectAcceptGateTests.cs | 101 +++++++++++++ .../ModbusInspectAcceptGateTests.cs | 61 ++++++++ .../MqttInspectAcceptGateTests.cs | 130 ++++++++++++++++ .../S7InspectAcceptGateTests.cs | 61 ++++++++ .../TwinCATInspectAcceptGateTests.cs | 61 ++++++++ 14 files changed, 752 insertions(+), 56 deletions(-) diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Contracts/AbCipTagDefinitionFactory.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Contracts/AbCipTagDefinitionFactory.cs index 77770c8c..e637abce 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Contracts/AbCipTagDefinitionFactory.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Contracts/AbCipTagDefinitionFactory.cs @@ -212,9 +212,15 @@ public static class AbCipTagDefinitionFactory public static IReadOnlyList Inspect(string reference) { var warnings = new List(); - if (string.IsNullOrWhiteSpace(reference) || reference[0] != '{') return warnings; + // Gitea #556 item 1: the framing check TRIMS FIRST. FromTagConfig hands the blob straight to + // JsonDocument.Parse, which tolerates leading whitespace — so an untrimmed check let " {…}" be + // read (and possibly REFUSED) by the driver while drawing zero deploy warnings. A reference that + // still does not start with '{' after trimming is not an equipment-tag TagConfig at all and keeps + // drawing nothing. + if (string.IsNullOrWhiteSpace(reference) || reference.TrimStart().FirstOrDefault() != '{') return warnings; - // The accept gate: whatever the driver reads draws nothing. + // The accept gate: whatever the driver reads draws nothing. The RAW reference is handed to the + // mapper — the driver reads the stored blob, not a trimmed copy of it. if (FromTagConfig(reference, InspectionRawPath, out _)) return warnings; try diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Contracts/AbLegacyTagDefinitionFactory.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Contracts/AbLegacyTagDefinitionFactory.cs index 7cc458a2..79b48a1b 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Contracts/AbLegacyTagDefinitionFactory.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Contracts/AbLegacyTagDefinitionFactory.cs @@ -148,9 +148,15 @@ public static class AbLegacyTagDefinitionFactory public static IReadOnlyList Inspect(string reference) { var warnings = new List(); - if (string.IsNullOrWhiteSpace(reference) || reference[0] != '{') return warnings; + // Gitea #556 item 1: the framing check TRIMS FIRST. FromTagConfig hands the blob straight to + // JsonDocument.Parse, which tolerates leading whitespace — so an untrimmed check let " {…}" be + // read (and possibly REFUSED) by the driver while drawing zero deploy warnings. A reference that + // still does not start with '{' after trimming is not an equipment-tag TagConfig at all and keeps + // drawing nothing. + if (string.IsNullOrWhiteSpace(reference) || reference.TrimStart().FirstOrDefault() != '{') return warnings; - // The accept gate: whatever the driver reads draws nothing. + // The accept gate: whatever the driver reads draws nothing. The RAW reference is handed to the + // mapper — the driver reads the stored blob, not a trimmed copy of it. if (FromTagConfig(reference, InspectionRawPath, out _)) return warnings; try diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Contracts/FocasTagDefinitionFactory.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Contracts/FocasTagDefinitionFactory.cs index 7b20ca1c..f676f6fc 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Contracts/FocasTagDefinitionFactory.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Contracts/FocasTagDefinitionFactory.cs @@ -122,16 +122,37 @@ public static class FocasTagDefinitionFactory /// /// Empty for a clean, accepted blob, for a blank one, or for one that is not an equipment-tag /// object. Never throws. + /// + /// The distinction is structural, not a matter of wording (Gitea #556 item 2): the two + /// classes accumulate in separate lists, and the catch-all backstop — the only thing that + /// reports a refusal neither branch can name — fires on "no + /// REJECTION was produced", never on "no warning at all". Gated on the total, an advisory + /// (which fires on an accepted blob too) would silently swallow it. + /// /// /// The equipment tag's TagConfig JSON. /// The warnings; empty when clean. public static IReadOnlyList Inspect(string reference) { - var warnings = new List(); - if (string.IsNullOrWhiteSpace(reference) || reference[0] != '{') return warnings; + // Gitea #556 item 2: the two classes live in SEPARATE lists, and the backstop below gates on the + // REJECTION list alone. Sharing one list meant an advisory (which fires on an accepted blob too) + // could satisfy a `warnings.Count == 0` guard and silently swallow the catch-all rejection message + // — the only thing that reports a refusal neither DescribeRejection branch can name. The split is + // structural: nothing string-matches a message to work out which class it is. + var rejections = new List(); + var advisories = new List(); + + // Gitea #556 item 1: the framing check TRIMS FIRST. FromTagConfig hands the blob straight to + // JsonDocument.Parse, which tolerates leading whitespace — so an untrimmed check let " {…}" be + // read (and possibly REFUSED) by the driver while drawing zero deploy warnings. A reference that + // still does not start with '{' after trimming is not an equipment-tag TagConfig at all and keeps + // drawing nothing. + if (string.IsNullOrWhiteSpace(reference) || reference.TrimStart().FirstOrDefault() != '{') + return rejections; // The accept gate: only a blob the driver REFUSES gets a rejection diagnosis. The advisories below - // apply either way — see the summary. + // apply either way — see the summary. The RAW reference is handed to the mapper: the driver reads + // the stored blob, not a trimmed copy of it. var accepted = FromTagConfig(reference, InspectionRawPath, out _); try @@ -140,27 +161,43 @@ public static class FocasTagDefinitionFactory var root = doc.RootElement; if (root.ValueKind != JsonValueKind.Object) { - warnings.Add("FOCAS TagConfig root is not a JSON object — the tag will not resolve (BadNodeIdUnknown)."); - return warnings; + rejections.Add("FOCAS TagConfig root is not a JSON object — the tag will not resolve (BadNodeIdUnknown)."); + return rejections; } - if (!accepted) DescribeRejection(root, warnings); - DescribeAdvisories(root, warnings); + if (!accepted) DescribeRejection(root, rejections); + DescribeAdvisories(root, advisories); } catch (JsonException) { - warnings.Add("FOCAS TagConfig is not valid JSON — the tag will not resolve (BadNodeIdUnknown)."); - return warnings; + rejections.Add("FOCAS TagConfig is not valid JSON — the tag will not resolve (BadNodeIdUnknown)."); + return rejections; } - if (!accepted && warnings.Count == 0) + // Backstop: the mapper refused for a reason no branch above names. Gated on the REJECTION count, + // never on the total — an advisory says nothing about why the tag was refused. + if (!accepted && rejections.Count == 0) { - warnings.Add( + rejections.Add( "FOCAS TagConfig was rejected by the driver's tag reader — the driver will skip this tag " + "and it will read BadNodeIdUnknown."); } - return warnings; + return Combine(rejections, advisories); + } + + /// + /// Flattens the two warning classes into the single list returns, rejections + /// first — the order the pre-split implementation produced, so no caller sees a reordering. + /// + /// The rejection-class warnings. + /// The advisory-class warnings. + /// The combined list. + private static List Combine(List rejections, List advisories) + { + if (advisories.Count == 0) return rejections; + rejections.AddRange(advisories); + return rejections; } /// @@ -168,21 +205,22 @@ public static class FocasTagDefinitionFactory /// same order. Runs only after the mapper has already refused, so it can never invent strictness. /// /// The TagConfig root object. - /// The list to append warnings to. - private static void DescribeRejection(JsonElement root, List warnings) + /// The REJECTION-class list to append to (never the advisory list — see + /// 's backstop). + private static void DescribeRejection(JsonElement root, List rejections) { if (!root.TryGetProperty("address", out var addr) || addr.ValueKind != JsonValueKind.String) { - warnings.Add( + rejections.Add( "FOCAS TagConfig has no string 'address' — the driver will skip this tag and it will read " + "BadNodeIdUnknown."); } else if (string.IsNullOrWhiteSpace(addr.GetString())) { - warnings.Add("FOCAS TagConfig has a blank 'address' — the tag will not resolve (BadNodeIdUnknown)."); + rejections.Add("FOCAS TagConfig has a blank 'address' — the tag will not resolve (BadNodeIdUnknown)."); } - if (TagConfigJson.DescribeInvalidEnum(root, "dataType") is { } w) warnings.Add(w); + if (TagConfigJson.DescribeInvalidEnum(root, "dataType") is { } w) rejections.Add(w); } /// @@ -190,8 +228,9 @@ public static class FocasTagDefinitionFactory /// "advisories" bullet on . /// /// The TagConfig root object. - /// The list to append warnings to. - private static void DescribeAdvisories(JsonElement root, List warnings) + /// The ADVISORY-class list to append to. These fire whether or not the mapper + /// accepted, so they must never reach the rejection list — see 's backstop. + private static void DescribeAdvisories(JsonElement root, List advisories) { // Gitea #540: an address the driver cannot parse is skipped at init and reads BadNodeIdUnknown // forever, while driver health stays Healthy with zero errors. The AdminUI modal rejects it @@ -202,13 +241,13 @@ public static class FocasTagDefinitionFactory && !string.IsNullOrWhiteSpace(address) && FocasAddress.TryParse(address) is null) { - warnings.Add( + advisories.Add( $"FOCAS address '{address.Trim()}' is not parseable (expected X0.0 / R100, " + "PARAM:1020, MACRO:500 or FIXED:Identity/SeriesNumber) — the driver will skip " + "this tag at startup and it will read BadNodeIdUnknown."); } if (root.TryGetProperty("writable", out var wr) && wr.ValueKind == JsonValueKind.True) - warnings.Add("FOCAS wire is read-only — an authored 'writable:true' still fails at the wire with BadNotWritable."); + advisories.Add("FOCAS wire is read-only — an authored 'writable:true' still fails at the wire with BadNotWritable."); } } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Contracts/ModbusTagDefinitionFactory.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Contracts/ModbusTagDefinitionFactory.cs index 6c10215f..8f230c12 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Contracts/ModbusTagDefinitionFactory.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Contracts/ModbusTagDefinitionFactory.cs @@ -160,9 +160,15 @@ public static class ModbusTagDefinitionFactory public static IReadOnlyList Inspect(string reference) { var warnings = new List(); - if (string.IsNullOrWhiteSpace(reference) || reference[0] != '{') return warnings; + // Gitea #556 item 1: the framing check TRIMS FIRST. FromTagConfig hands the blob straight to + // JsonDocument.Parse, which tolerates leading whitespace — so an untrimmed check let " {…}" be + // read (and possibly REFUSED) by the driver while drawing zero deploy warnings. A reference that + // still does not start with '{' after trimming is not an equipment-tag TagConfig at all and keeps + // drawing nothing, which is the pre-existing convention shared by all eleven inspectors. + if (string.IsNullOrWhiteSpace(reference) || reference.TrimStart().FirstOrDefault() != '{') return warnings; - // The accept gate: whatever the driver reads draws nothing. See the remarks. + // The accept gate: whatever the driver reads draws nothing. See the remarks. The RAW reference is + // handed to the mapper — the driver reads the stored blob, not a trimmed copy of it. if (FromTagConfig(reference, InspectionRawPath, out _)) return warnings; try diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Contracts/MqttTagDefinitionFactory.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Contracts/MqttTagDefinitionFactory.cs index c8f08782..7d5ebf61 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Contracts/MqttTagDefinitionFactory.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Contracts/MqttTagDefinitionFactory.cs @@ -60,6 +60,14 @@ public static class MqttTagDefinitionFactory /// for it. Rejecting it at runtime would turn an authoring mistake into a silent /// BadNodeIdUnknown with no stated cause. /// + /// + /// ⚠️ payloadFormat is the one key whose verdict differs between the two readers + /// (Gitea #557 item 2). It is read STRICTLY here, so a typo takes the tag dark in Plain mode; + /// never reads it at all, so the same blob is served + /// happily in Sparkplug mode. That asymmetry is intentional — see the contract note there — + /// and reports it as an ADVISORY because no tag-level gate can see which + /// mode the tag's driver runs. + /// /// /// The authored equipment-tag TagConfig JSON. /// The tag's RawPath — becomes the definition's identity (Name). @@ -139,6 +147,26 @@ public static class MqttTagDefinitionFactory /// validated — a blob retyped in the editor keeps them, and rejecting the tag for a stale /// leftover key would take a correctly-authored Sparkplug tag dark with no stated cause. /// + /// + /// The payloadFormat contract (Gitea #557 item 2), decided and pinned. An invalid + /// payloadFormat is ACCEPTED here and REFUSED by , and the + /// union verdict and the AdminUI's MqttTagConfigModel both use + /// therefore admits it. That is the intended contract, on three pieces of evidence: this reader + /// hard-codes and never consults the key; the Sparkplug + /// ingest path (SparkplugIngestor → this method) never reads + /// — only MqttSubscriptionManager, the + /// PLAIN path, decodes by it; and MqttTagConfigModel.ToJson deliberately omits the key + /// entirely in Sparkplug mode (Gitea #513). So in Sparkplug mode the tag genuinely works, and a + /// gate that rejected it would be stricter than the runtime — the one thing these gates must + /// never be. + /// + /// + /// ⚠️ The cost is that the served behaviour depends on the DRIVER's , + /// which no tag row records. closes that with an ADVISORY naming the key + /// and the mode dependency, so the operator is told without the gate over-claiming. The + /// authoring gate keeps the same verdict (accept) by construction — it consults the identical + /// union — and its own Plain-mode half already refuses the value outright. + /// /// /// The authored equipment-tag TagConfig JSON. /// The tag's RawPath — becomes the definition's identity (Name). @@ -221,12 +249,22 @@ public static class MqttTagDefinitionFactory /// tuple. Such a tag is skipped by the driver and reads BadNodeIdUnknown forever. /// /// - /// Advisory — a wildcard tag topic (+ / #). The mapper - /// deliberately ACCEPTS it (ambiguous, not unparseable), so this runs either way. + /// Advisories — a wildcard tag topic (+ / #), which the mapper + /// deliberately ACCEPTS (ambiguous, not unparseable) so this runs either way; and a + /// present-but-invalid payloadFormat on an ACCEPTED blob, which can only mean the + /// Sparkplug reader took it and the Plain reader would not (Gitea #557 item 2). /// /// /// Empty for a clean, readable blob, for a blank one, or for one that is not an equipment-tag /// TagConfig object. Never throws. + /// + /// The two classes are kept in separate lists (Gitea #556 item 2), and the catch-all + /// backstop fires on "no REJECTION was produced", never on "no warning at all" — otherwise an + /// advisory would silently swallow the only message that reports a refusal nothing can name. + /// For MQTT that shape is reachable in production: a leading-whitespace blob is refused by both + /// mappers' own framing gates while every rejection branch finds the parsed document perfectly + /// well-formed. + /// /// /// /// The accept gate is the UNION of the two mappers (Gitea #551), because Inspect is @@ -239,9 +277,26 @@ public static class MqttTagDefinitionFactory /// The warnings; empty when clean. public static IReadOnlyList Inspect(string reference) { - var warnings = new List(); - if (string.IsNullOrWhiteSpace(reference) || reference[0] != '{') return warnings; + // Gitea #556 item 2: the two classes live in SEPARATE lists, and the backstop below gates on the + // REJECTION list alone. Sharing one list meant an advisory (which fires on an accepted blob too) + // could satisfy a `warnings.Count == 0` guard and silently swallow the catch-all rejection message + // — the only thing that reports a refusal neither DescribeRejection branch can name, which for + // MQTT is a REACHABLE shape, not a theoretical one: see the leading-whitespace note below. + var rejections = new List(); + var advisories = new List(); + // Gitea #556 item 1: the framing check TRIMS FIRST, so a blob whose only sin is leading whitespace + // is no longer waved through in silence. MQTT is the one factory of the seven whose RUNTIME + // mappers carry the same `[0] != '{'` gate, so — unlike its siblings, whose mappers hand the blob + // to a whitespace-tolerant JsonDocument.Parse — " {…}" is genuinely REFUSED by this driver. Which + // is exactly why the untrimmed check was a defect: the tag read BadNodeIdUnknown forever and the + // deploy gate said nothing. A reference that still does not start with '{' after trimming is not + // an equipment-tag TagConfig at all and keeps drawing nothing. + if (string.IsNullOrWhiteSpace(reference) || reference.TrimStart().FirstOrDefault() != '{') + return rejections; + + // The RAW reference is handed to the mappers — the driver reads the stored blob, not a trimmed + // copy of it, so the verdict here is the verdict the driver will reach. var accepted = FromTagConfig(reference, InspectionRawPath, out _) || FromSparkplugTagConfig(reference, InspectionRawPath, out _); @@ -251,35 +306,66 @@ public static class MqttTagDefinitionFactory var root = doc.RootElement; if (root.ValueKind != JsonValueKind.Object) { - warnings.Add("Mqtt TagConfig root is not a JSON object — the tag will not resolve (BadNodeIdUnknown)."); - return warnings; + rejections.Add("Mqtt TagConfig root is not a JSON object — the tag will not resolve (BadNodeIdUnknown)."); + return rejections; } - if (!accepted) DescribeRejection(root, warnings); - - // Advisory — the mappers accept a wildcard topic on purpose; see the summary. - var topic = ReadString(root, MqttTagConfigKeys.Topic); - if (topic.IndexOfAny(TopicWildcards) >= 0) - { - warnings.Add( - $"value '{topic}' for 'topic' contains an MQTT wildcard (+ or #); a tag's topic must be " + - "concrete, or the tag will be fed by every matching topic."); - } + if (!accepted) DescribeRejection(root, rejections); + DescribeAdvisories(root, accepted, advisories); } catch (JsonException) { - warnings.Add("Mqtt TagConfig is not valid JSON — the tag will not resolve (BadNodeIdUnknown)."); - return warnings; + rejections.Add("Mqtt TagConfig is not valid JSON — the tag will not resolve (BadNodeIdUnknown)."); + return rejections; } - if (!accepted && warnings.Count == 0) + // Backstop: neither mapper read the blob and no branch above names why. Gated on the REJECTION + // count, never on the total — an advisory says nothing about why the tag was refused. + if (!accepted && rejections.Count == 0) { - warnings.Add( + rejections.Add( "Mqtt TagConfig was rejected by the driver's tag reader — the driver will skip this tag " + "and it will read BadNodeIdUnknown."); } - return warnings; + if (advisories.Count == 0) return rejections; + rejections.AddRange(advisories); // rejections first — the order the pre-split shape produced + return rejections; + } + + /// + /// Warnings about a blob at least one mapper reads (or would read but for another fault) that the + /// driver still cannot serve as authored — see the "advisories" bullet on . + /// + /// The TagConfig root object. + /// Whether the union accept gate read the blob. + /// The ADVISORY-class list to append to. These must never reach the rejection + /// list — see 's backstop. + private static void DescribeAdvisories(JsonElement root, bool accepted, List advisories) + { + // The mappers accept a wildcard topic on purpose (ambiguous, not unparseable), so this runs + // whether or not the blob was accepted. + var topic = ReadString(root, MqttTagConfigKeys.Topic); + if (topic.IndexOfAny(TopicWildcards) >= 0) + { + advisories.Add( + $"value '{topic}' for 'topic' contains an MQTT wildcard (+ or #); a tag's topic must be " + + "concrete, or the tag will be fed by every matching topic."); + } + + // Gitea #557 item 2 — the ONE key whose verdict depends on the DRIVER'S mode, reported as an + // advisory because no tag-level gate can see that mode. A present-but-invalid payloadFormat is + // fatal to the Plain parse and invisible to the Sparkplug one, so a union accept here can only + // have come from Sparkplug. Promoting it to a rejection would be stricter than the runtime for + // every Sparkplug deployment (the tag IS served); staying silent would hide that the same row + // goes dark the moment its driver is Plain. See the contract note on FromSparkplugTagConfig. + if (accepted && TagConfigJson.DescribeInvalidEnum(root, "payloadFormat") is { } pf) + { + advisories.Add( + pf + " — this tag is accepted only because the Sparkplug B reader ignores 'payloadFormat' " + + "entirely. If this device's driver runs Plain mode the tag is REJECTED and reads " + + "BadNodeIdUnknown; either fix the value or remove the key."); + } } /// @@ -288,17 +374,22 @@ public static class MqttTagDefinitionFactory /// already refused, so it can never invent strictness. /// /// The TagConfig root object. - /// The list to append warnings to. - private static void DescribeRejection(JsonElement root, List warnings) + /// The REJECTION-class list to append to (never the advisory list — see + /// 's backstop). + private static void DescribeRejection(JsonElement root, List rejections) { foreach (var w in new[] { + // payloadFormat appears in BOTH classes, and which one it lands in is decided by the accept + // gate, not by the key: here it is a genuine rejection cause (both mappers refused, so the + // driver serves the tag in neither mode), while on an ACCEPTED blob the same invalid value is + // the mode-dependent advisory in DescribeAdvisories (Gitea #557 item 2). TagConfigJson.DescribeInvalidEnum(root, "payloadFormat"), TagConfigJson.DescribeInvalidEnum(root, "dataType"), DescribeInvalidQos(root), }) { - if (w is not null) warnings.Add(w); + if (w is not null) rejections.Add(w); } // Address: Plain needs a topic, Sparkplug needs the whole binding tuple. Neither is satisfied, or @@ -309,7 +400,7 @@ public static class MqttTagDefinitionFactory && !string.IsNullOrWhiteSpace(ReadString(root, MqttTagConfigKeys.MetricName)); if (!hasTopic && !hasSparkplugTuple) { - warnings.Add( + rejections.Add( "Mqtt TagConfig names no address — a Plain-mode tag needs a 'topic', a Sparkplug B tag " + "needs 'groupId' + 'edgeNodeId' + 'metricName'. The driver will skip this tag and it " + "will read BadNodeIdUnknown."); diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Contracts/S7TagDefinitionFactory.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Contracts/S7TagDefinitionFactory.cs index 6d1839b4..f89fa18a 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Contracts/S7TagDefinitionFactory.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Contracts/S7TagDefinitionFactory.cs @@ -161,9 +161,15 @@ public static class S7TagDefinitionFactory public static IReadOnlyList Inspect(string reference) { var warnings = new List(); - if (string.IsNullOrWhiteSpace(reference) || reference[0] != '{') return warnings; + // Gitea #556 item 1: the framing check TRIMS FIRST. FromTagConfig hands the blob straight to + // JsonDocument.Parse, which tolerates leading whitespace — so an untrimmed check let " {…}" be + // read (and possibly REFUSED) by the driver while drawing zero deploy warnings. A reference that + // still does not start with '{' after trimming is not an equipment-tag TagConfig at all and keeps + // drawing nothing. + if (string.IsNullOrWhiteSpace(reference) || reference.TrimStart().FirstOrDefault() != '{') return warnings; - // The accept gate: whatever the driver reads draws nothing. + // The accept gate: whatever the driver reads draws nothing. The RAW reference is handed to the + // mapper — the driver reads the stored blob, not a trimmed copy of it. if (FromTagConfig(reference, InspectionRawPath, out _)) return warnings; try diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Contracts/TwinCATTagDefinitionFactory.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Contracts/TwinCATTagDefinitionFactory.cs index b7ed7357..c0a255af 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Contracts/TwinCATTagDefinitionFactory.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Contracts/TwinCATTagDefinitionFactory.cs @@ -132,9 +132,15 @@ public static class TwinCATTagDefinitionFactory public static IReadOnlyList Inspect(string reference) { var warnings = new List(); - if (string.IsNullOrWhiteSpace(reference) || reference[0] != '{') return warnings; + // Gitea #556 item 1: the framing check TRIMS FIRST. FromTagConfig hands the blob straight to + // JsonDocument.Parse, which tolerates leading whitespace — so an untrimmed check let " {…}" be + // read (and possibly REFUSED) by the driver while drawing zero deploy warnings. A reference that + // still does not start with '{' after trimming is not an equipment-tag TagConfig at all and keeps + // drawing nothing. + if (string.IsNullOrWhiteSpace(reference) || reference.TrimStart().FirstOrDefault() != '{') return warnings; - // The accept gate: whatever the driver reads draws nothing. + // The accept gate: whatever the driver reads draws nothing. The RAW reference is handed to the + // mapper — the driver reads the stored blob, not a trimmed copy of it. if (FromTagConfig(reference, InspectionRawPath, out _)) return warnings; try diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipInspectAcceptGateTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipInspectAcceptGateTests.cs index 725eb52c..e03c9f35 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipInspectAcceptGateTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipInspectAcceptGateTests.cs @@ -97,4 +97,65 @@ public sealed class AbCipInspectAcceptGateTests [Fact] public void Unparseable_json_names_the_json() => AbCipTagDefinitionFactory.Inspect("{oops").ShouldContain(w => w.Contains("not valid JSON")); + + // ---- Gitea #556 item 1: leading whitespace no longer defeats the framing pre-filter ---- + + /// + /// Padding a blob with leading whitespace. A driver is handed whatever authoring persisted, and + /// FromTagConfig passes it straight to JsonDocument.Parse, which skips leading + /// whitespace — so the MAPPER's verdict is identical padded or not, and the gate's must be too. + /// + private const string Pad = " \t\r\n "; + + /// + /// Never stricter, under padding: a padded blob the mapper still accepts draws nothing. (This + /// direction already held before #556 — the pre-filter returned empty for everything padded — and + /// is kept so a future "just trim everywhere" change cannot start warning about a served tag.) + /// + /// An accepted fixture, padded below. + [Theory] + [MemberData(nameof(Accepted))] + public void Leading_whitespace_leaves_an_accepted_config_silent(string tagConfig) + { + var padded = Pad + tagConfig; + AbCipTagDefinitionFactory.FromTagConfig(padded, RawPath, out _) + .ShouldBeTrue("the mapper's JsonDocument.Parse skips leading whitespace"); + AbCipTagDefinitionFactory.Inspect(padded).ShouldBeEmpty(); + } + + /// + /// Never silent, under padding — the #556 item 1 defect itself. The pre-filter tested + /// reference[0] != '{', so EVERY padded blob returned empty and a padded config the driver + /// refuses deployed clean. Asserting equality with the untrimmed inspection (rather than merely + /// "not empty") also pins that padding changes no message. + /// + /// A rejected fixture, padded below. + [Theory] + [MemberData(nameof(Rejected))] + public void Leading_whitespace_draws_exactly_the_warnings_the_untrimmed_blob_draws(string tagConfig) + { + var padded = Pad + tagConfig; + AbCipTagDefinitionFactory.FromTagConfig(padded, RawPath, out _) + .ShouldBeFalse("the mapper's verdict is unchanged by padding"); + AbCipTagDefinitionFactory.Inspect(padded).ShouldBe(AbCipTagDefinitionFactory.Inspect(tagConfig)); + } + + /// + /// The other half of the same fix, and the one it would be easy to lose: trimming widens the gate + /// ONLY for JSON objects. A reference that is still not object-framed after the trim is not an + /// equipment-tag TagConfig at all — the pre-existing convention shared by all eleven inspectors — + /// so it keeps drawing nothing even though the mapper refuses it. + /// + /// A padded, non-object reference. + [Theory] + [InlineData(" just a string")] + [InlineData(" [1,2,3]")] + [InlineData(" \t\r\n 42")] + [InlineData(" \"a\"")] + [InlineData(" ")] + public void A_non_object_reference_still_draws_nothing_after_trimming(string reference) + { + AbCipTagDefinitionFactory.FromTagConfig(reference, RawPath, out _).ShouldBeFalse(); + AbCipTagDefinitionFactory.Inspect(reference).ShouldBeEmpty(); + } } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyInspectAcceptGateTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyInspectAcceptGateTests.cs index ae656596..777c12e9 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyInspectAcceptGateTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyInspectAcceptGateTests.cs @@ -61,4 +61,65 @@ public sealed class AbLegacyInspectAcceptGateTests [Fact] public void Unparseable_json_names_the_json() => AbLegacyTagDefinitionFactory.Inspect("{oops").ShouldContain(w => w.Contains("not valid JSON")); + + // ---- Gitea #556 item 1: leading whitespace no longer defeats the framing pre-filter ---- + + /// + /// Padding a blob with leading whitespace. A driver is handed whatever authoring persisted, and + /// FromTagConfig passes it straight to JsonDocument.Parse, which skips leading + /// whitespace — so the MAPPER's verdict is identical padded or not, and the gate's must be too. + /// + private const string Pad = " \t\r\n "; + + /// + /// Never stricter, under padding: a padded blob the mapper still accepts draws nothing. (This + /// direction already held before #556 — the pre-filter returned empty for everything padded — and + /// is kept so a future "just trim everywhere" change cannot start warning about a served tag.) + /// + /// An accepted fixture, padded below. + [Theory] + [MemberData(nameof(Accepted))] + public void Leading_whitespace_leaves_an_accepted_config_silent(string tagConfig) + { + var padded = Pad + tagConfig; + AbLegacyTagDefinitionFactory.FromTagConfig(padded, RawPath, out _) + .ShouldBeTrue("the mapper's JsonDocument.Parse skips leading whitespace"); + AbLegacyTagDefinitionFactory.Inspect(padded).ShouldBeEmpty(); + } + + /// + /// Never silent, under padding — the #556 item 1 defect itself. The pre-filter tested + /// reference[0] != '{', so EVERY padded blob returned empty and a padded config the driver + /// refuses deployed clean. Asserting equality with the untrimmed inspection (rather than merely + /// "not empty") also pins that padding changes no message. + /// + /// A rejected fixture, padded below. + [Theory] + [MemberData(nameof(Rejected))] + public void Leading_whitespace_draws_exactly_the_warnings_the_untrimmed_blob_draws(string tagConfig) + { + var padded = Pad + tagConfig; + AbLegacyTagDefinitionFactory.FromTagConfig(padded, RawPath, out _) + .ShouldBeFalse("the mapper's verdict is unchanged by padding"); + AbLegacyTagDefinitionFactory.Inspect(padded).ShouldBe(AbLegacyTagDefinitionFactory.Inspect(tagConfig)); + } + + /// + /// The other half of the same fix, and the one it would be easy to lose: trimming widens the gate + /// ONLY for JSON objects. A reference that is still not object-framed after the trim is not an + /// equipment-tag TagConfig at all — the pre-existing convention shared by all eleven inspectors — + /// so it keeps drawing nothing even though the mapper refuses it. + /// + /// A padded, non-object reference. + [Theory] + [InlineData(" just a string")] + [InlineData(" [1,2,3]")] + [InlineData(" \t\r\n 42")] + [InlineData(" \"a\"")] + [InlineData(" ")] + public void A_non_object_reference_still_draws_nothing_after_trimming(string reference) + { + AbLegacyTagDefinitionFactory.FromTagConfig(reference, RawPath, out _).ShouldBeFalse(); + AbLegacyTagDefinitionFactory.Inspect(reference).ShouldBeEmpty(); + } } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasInspectAcceptGateTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasInspectAcceptGateTests.cs index 8a0da241..1e5dbffa 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasInspectAcceptGateTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasInspectAcceptGateTests.cs @@ -95,4 +95,105 @@ public sealed class FocasInspectAcceptGateTests [Fact] public void Unparseable_json_names_the_json() => FocasTagDefinitionFactory.Inspect("{oops").ShouldContain(w => w.Contains("not valid JSON")); + + // ---- Gitea #556 item 2: an advisory can no longer swallow the catch-all rejection ---- + + /// + /// The structural invariant the backstop exists to hold: a blob the mapper REFUSES always draws + /// strictly more than the advisories that same blob would draw if it were accepted. Before #556 the + /// backstop was gated on warnings.Count == 0 — and an advisory fires on an ACCEPTED blob + /// too, so it says nothing about the refusal while still satisfying that guard. Any refusal no + /// DescribeRejection branch can name was therefore dropped whenever an advisory was present. + /// Comparing the two counts states the property without string-matching a message to guess its + /// class, which is the whole point of keeping the two classes in separate lists. + /// + /// ⚠️ FOCAS's own backstop is UNREACHABLE today (audited): every return false in + /// — absent / non-string / blank + /// address, invalid dataType — has a matching describe branch, and + /// TryReadEnumStrict and DescribeInvalidEnum share one classifier so they cannot + /// disagree. So the guard is protective here rather than a live fix; MQTT's is live (a padded + /// blob its own mappers refuse and every branch finds well-formed — see + /// MqttInspectAcceptGateTests). What this pins is the property that must survive the + /// next rejection cause someone adds without a describe branch. + /// + /// + [Fact] + public void A_rejected_config_always_draws_more_than_its_advisories_alone() + { + // Identical but for the dataType typo, so both carry the SAME two advisories. + const string Accepted = """{"address":"not-an-address","dataType":"Int32","writable":true}"""; + const string Rejected = """{"address":"not-an-address","dataType":"Intt32","writable":true}"""; + + FocasTagDefinitionFactory.FromTagConfig(Accepted, RawPath, out _).ShouldBeTrue("control must be accepted"); + FocasTagDefinitionFactory.FromTagConfig(Rejected, RawPath, out _).ShouldBeFalse("subject must be rejected"); + + var advisoriesOnly = FocasTagDefinitionFactory.Inspect(Accepted); + var withRejection = FocasTagDefinitionFactory.Inspect(Rejected); + + advisoriesOnly.Count.ShouldBe(2, "the control's warnings are purely advisory"); + withRejection.Count.ShouldBeGreaterThan( + advisoriesOnly.Count, + "a refusal must add a rejection-class warning — the backstop supplies one when nothing names it"); + } + + // ---- Gitea #556 item 1: leading whitespace no longer defeats the framing pre-filter ---- + + /// + /// Padding a blob with leading whitespace. A driver is handed whatever authoring persisted, and + /// FromTagConfig passes it straight to JsonDocument.Parse, which skips leading + /// whitespace — so the MAPPER's verdict is identical padded or not, and the gate's must be too. + /// + private const string Pad = " \t\r\n "; + + /// + /// Never stricter, under padding: a padded blob the mapper still accepts draws nothing. (This + /// direction already held before #556 — the pre-filter returned empty for everything padded — and + /// is kept so a future "just trim everywhere" change cannot start warning about a served tag.) + /// + /// An accepted fixture, padded below. + [Theory] + [MemberData(nameof(CleanAndAccepted))] + public void Leading_whitespace_leaves_an_accepted_config_silent(string tagConfig) + { + var padded = Pad + tagConfig; + FocasTagDefinitionFactory.FromTagConfig(padded, RawPath, out _) + .ShouldBeTrue("the mapper's JsonDocument.Parse skips leading whitespace"); + FocasTagDefinitionFactory.Inspect(padded).ShouldBeEmpty(); + } + + /// + /// Never silent, under padding — the #556 item 1 defect itself. The pre-filter tested + /// reference[0] != '{', so EVERY padded blob returned empty and a padded config the driver + /// refuses deployed clean. Asserting equality with the untrimmed inspection (rather than merely + /// "not empty") also pins that padding changes no message. + /// + /// A rejected fixture, padded below. + [Theory] + [MemberData(nameof(Rejected))] + public void Leading_whitespace_draws_exactly_the_warnings_the_untrimmed_blob_draws(string tagConfig) + { + var padded = Pad + tagConfig; + FocasTagDefinitionFactory.FromTagConfig(padded, RawPath, out _) + .ShouldBeFalse("the mapper's verdict is unchanged by padding"); + FocasTagDefinitionFactory.Inspect(padded).ShouldBe(FocasTagDefinitionFactory.Inspect(tagConfig)); + } + + /// + /// The other half of the same fix, and the one it would be easy to lose: trimming widens the gate + /// ONLY for JSON objects. A reference that is still not object-framed after the trim is not an + /// equipment-tag TagConfig at all — the pre-existing convention shared by all eleven inspectors — + /// so it keeps drawing nothing even though the mapper refuses it. + /// + /// A padded, non-object reference. + [Theory] + [InlineData(" just a string")] + [InlineData(" [1,2,3]")] + [InlineData(" \t\r\n 42")] + [InlineData(" \"a\"")] + [InlineData(" ")] + public void A_non_object_reference_still_draws_nothing_after_trimming(string reference) + { + FocasTagDefinitionFactory.FromTagConfig(reference, RawPath, out _).ShouldBeFalse(); + FocasTagDefinitionFactory.Inspect(reference).ShouldBeEmpty(); + } } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusInspectAcceptGateTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusInspectAcceptGateTests.cs index 7ecfd2bb..0b48d404 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusInspectAcceptGateTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusInspectAcceptGateTests.cs @@ -125,4 +125,65 @@ public sealed class ModbusInspectAcceptGateTests [Fact] public void Unparseable_json_names_the_json() => ModbusTagDefinitionFactory.Inspect("{oops").ShouldContain(w => w.Contains("not valid JSON")); + + // ---- Gitea #556 item 1: leading whitespace no longer defeats the framing pre-filter ---- + + /// + /// Padding a blob with leading whitespace. A driver is handed whatever authoring persisted, and + /// FromTagConfig passes it straight to JsonDocument.Parse, which skips leading + /// whitespace — so the MAPPER's verdict is identical padded or not, and the gate's must be too. + /// + private const string Pad = " \t\r\n "; + + /// + /// Never stricter, under padding: a padded blob the mapper still accepts draws nothing. (This + /// direction already held before #556 — the pre-filter returned empty for everything padded — and + /// is kept so a future "just trim everywhere" change cannot start warning about a served tag.) + /// + /// An accepted fixture, padded below. + [Theory] + [MemberData(nameof(Accepted))] + public void Leading_whitespace_leaves_an_accepted_config_silent(string tagConfig) + { + var padded = Pad + tagConfig; + ModbusTagDefinitionFactory.FromTagConfig(padded, RawPath, out _) + .ShouldBeTrue("the mapper's JsonDocument.Parse skips leading whitespace"); + ModbusTagDefinitionFactory.Inspect(padded).ShouldBeEmpty(); + } + + /// + /// Never silent, under padding — the #556 item 1 defect itself. The pre-filter tested + /// reference[0] != '{', so EVERY padded blob returned empty and a padded config the driver + /// refuses deployed clean. Asserting equality with the untrimmed inspection (rather than merely + /// "not empty") also pins that padding changes no message. + /// + /// A rejected fixture, padded below. + [Theory] + [MemberData(nameof(Rejected))] + public void Leading_whitespace_draws_exactly_the_warnings_the_untrimmed_blob_draws(string tagConfig) + { + var padded = Pad + tagConfig; + ModbusTagDefinitionFactory.FromTagConfig(padded, RawPath, out _) + .ShouldBeFalse("the mapper's verdict is unchanged by padding"); + ModbusTagDefinitionFactory.Inspect(padded).ShouldBe(ModbusTagDefinitionFactory.Inspect(tagConfig)); + } + + /// + /// The other half of the same fix, and the one it would be easy to lose: trimming widens the gate + /// ONLY for JSON objects. A reference that is still not object-framed after the trim is not an + /// equipment-tag TagConfig at all — the pre-existing convention shared by all eleven inspectors — + /// so it keeps drawing nothing even though the mapper refuses it. + /// + /// A padded, non-object reference. + [Theory] + [InlineData(" just a string")] + [InlineData(" [1,2,3]")] + [InlineData(" \t\r\n 42")] + [InlineData(" \"a\"")] + [InlineData(" ")] + public void A_non_object_reference_still_draws_nothing_after_trimming(string reference) + { + ModbusTagDefinitionFactory.FromTagConfig(reference, RawPath, out _).ShouldBeFalse(); + ModbusTagDefinitionFactory.Inspect(reference).ShouldBeEmpty(); + } } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttInspectAcceptGateTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttInspectAcceptGateTests.cs index 2750f36d..1af21085 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttInspectAcceptGateTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttInspectAcceptGateTests.cs @@ -85,4 +85,134 @@ public sealed class MqttInspectAcceptGateTests [Fact] public void Unparseable_json_names_the_json() => MqttTagDefinitionFactory.Inspect("{oops").ShouldContain(w => w.Contains("not valid JSON")); + + // ---- Gitea #556 item 1: leading whitespace no longer defeats the framing pre-filter ---- + + /// Padding a blob with leading whitespace. + private const string Pad = " \t\r\n "; + + /// + /// MQTT is the one factory of the seven whose RUNTIME mappers carry the same [0] != '{' + /// framing gate the inspector did, so a padded blob is genuinely REFUSED by this driver — which is + /// precisely why the untrimmed pre-filter was a defect rather than a cosmetic mismatch: the tag + /// read BadNodeIdUnknown forever and the deploy gate said nothing at all. Its siblings hand + /// the blob to a whitespace-tolerant JsonDocument.Parse and so keep accepting it; both + /// behaviours are correct, because each inspector mirrors its own mapper. + /// + /// A fixture that is accepted unpadded. + [Theory] + [MemberData(nameof(Accepted))] + public void A_padded_config_the_readers_refuse_is_no_longer_passed_over_in_silence(string tagConfig) + { + var padded = Pad + tagConfig; + EitherModeAccepts(tagConfig).ShouldBeTrue("the unpadded fixture must be accepted"); + EitherModeAccepts(padded).ShouldBeFalse("both MQTT readers carry their own framing gate"); + MqttTagDefinitionFactory.Inspect(padded).ShouldNotBeEmpty(); + } + + /// + /// Trimming widens the gate ONLY for JSON objects — a reference still not object-framed after the + /// trim is not an equipment-tag TagConfig at all and keeps drawing nothing. + /// + /// A padded, non-object reference. + [Theory] + [InlineData(" just a string")] + [InlineData(" [1,2,3]")] + [InlineData(" \t\r\n 42")] + [InlineData(" \"a\"")] + [InlineData(" ")] + public void A_non_object_reference_still_draws_nothing_after_trimming(string reference) + { + EitherModeAccepts(reference).ShouldBeFalse(); + MqttTagDefinitionFactory.Inspect(reference).ShouldBeEmpty(); + } + + // ---- Gitea #556 item 2: an advisory can no longer swallow the catch-all rejection ---- + + /// + /// The advisory-plus-unnameable-rejection shape, and for MQTT it is REACHABLE in production rather + /// than theoretical: a padded blob is refused by both readers' own framing gates, while every + /// DescribeRejection branch — which walks the PARSED document — finds it perfectly + /// well-formed and names nothing. The wildcard advisory fires regardless. Before #556 the backstop + /// was gated on warnings.Count == 0, so that advisory satisfied the guard and the operator + /// was told only that the topic had a wildcard — never that the tag would not resolve at all. + /// + [Fact] + public void A_refusal_nothing_can_name_still_reports_the_backstop_when_an_advisory_is_present() + { + var padded = Pad + """{"topic":"plant/+/temp"}"""; + + EitherModeAccepts(padded).ShouldBeFalse("the framing gate in both readers refuses a padded blob"); + + var warnings = MqttTagDefinitionFactory.Inspect(padded); + warnings.ShouldContain(w => w.Contains("wildcard"), "the advisory still fires"); + warnings.ShouldContain( + w => w.Contains("rejected by the driver's tag reader"), + "the backstop must survive the advisory — it is the only message naming the refusal"); + } + + // ---- Gitea #557 item 2: the payloadFormat mode-asymmetry contract ---- + + /// + /// The decided contract. An invalid payloadFormat is fatal to the Plain reader and invisible + /// to the Sparkplug one (which hard-codes Json and never consults the key), so the UNION + /// verdict both gates use ADMITS a Sparkplug-shaped blob carrying one. That is intended: in + /// Sparkplug mode the driver genuinely serves the tag, and a gate that rejected it would be + /// stricter than the runtime. The cost — the served behaviour depends on the driver's + /// MqttMode, which no tag row records — is paid by an ADVISORY, not by changing the verdict. + /// + [Fact] + public void An_invalid_payloadFormat_on_a_Sparkplug_blob_is_accepted_and_advised_not_rejected() + { + const string Blob = + """{"groupId":"OtOpcUaSim","edgeNodeId":"EdgeA","metricName":"Temp","payloadFormat":"Jsonn"}"""; + + MqttTagDefinitionFactory.FromSparkplugTagConfig(Blob, RawPath, out _) + .ShouldBeTrue("the Sparkplug reader never consults payloadFormat"); + MqttTagDefinitionFactory.FromTagConfig(Blob, RawPath, out _) + .ShouldBeFalse("the Plain reader reads payloadFormat strictly (and this blob has no topic)"); + + var warnings = MqttTagDefinitionFactory.Inspect(Blob); + warnings.ShouldContain(w => w.Contains("payloadFormat") && w.Contains("Jsonn")); + warnings.ShouldContain(w => w.Contains("Plain mode"), "the advisory must name the mode dependency"); + + // The verdict itself is unchanged — this is an advisory, not a rejection. A rejection would name + // the tag as unresolvable, which is false for every Sparkplug deployment. + warnings.ShouldNotContain(w => w.Contains("rejected by the driver's tag reader")); + } + + /// + /// The falsification control for the rule above: the SAME invalid payloadFormat on a blob + /// the union REFUSES is a rejection, not an advisory — so the advisory branch cannot be a blanket + /// "payloadFormat is always advisory", which would under-report a Plain tag that is dead in every + /// mode. The class a key lands in is decided by the accept gate, never by the key. + /// + [Fact] + public void The_same_invalid_payloadFormat_on_a_rejected_blob_is_a_rejection() + { + const string Blob = """{"topic":"plant/line1/temp","payloadFormat":"Jsonn"}"""; + + EitherModeAccepts(Blob).ShouldBeFalse("Plain refuses the enum, Sparkplug has no binding tuple"); + + var warnings = MqttTagDefinitionFactory.Inspect(Blob); + warnings.ShouldContain(w => w.Contains("payloadFormat") && w.Contains("Jsonn")); + warnings.ShouldNotContain(w => w.Contains("Plain mode"), "an unserved tag is not a mode-dependent case"); + } + + /// + /// Both gates must keep agreeing on the VERDICT (Gitea #557 item 2). The #554 authoring consult in + /// MqttTagConfigModel asks the identical union and only words a REFUSAL, so an advisory can + /// never leak into the modal as a blocking error. Re-stated here as the union itself, since the + /// AdminUI model is not referenced from this project. + /// + [Fact] + public void The_authoring_consults_union_is_the_same_union_the_deploy_gate_uses() + { + const string Blob = + """{"groupId":"OtOpcUaSim","edgeNodeId":"EdgeA","metricName":"Temp","payloadFormat":"Jsonn"}"""; + + // Both gates read this verdict; the deploy gate's non-empty inspection above does not change it. + EitherModeAccepts(Blob).ShouldBeTrue(); + MqttTagDefinitionFactory.Inspect(Blob).ShouldNotBeEmpty("advisories are allowed on an accepted blob"); + } } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7InspectAcceptGateTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7InspectAcceptGateTests.cs index af0913f4..b34c50aa 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7InspectAcceptGateTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7InspectAcceptGateTests.cs @@ -95,4 +95,65 @@ public sealed class S7InspectAcceptGateTests [Fact] public void Unparseable_json_names_the_json() => S7TagDefinitionFactory.Inspect("{oops").ShouldContain(w => w.Contains("not valid JSON")); + + // ---- Gitea #556 item 1: leading whitespace no longer defeats the framing pre-filter ---- + + /// + /// Padding a blob with leading whitespace. A driver is handed whatever authoring persisted, and + /// FromTagConfig passes it straight to JsonDocument.Parse, which skips leading + /// whitespace — so the MAPPER's verdict is identical padded or not, and the gate's must be too. + /// + private const string Pad = " \t\r\n "; + + /// + /// Never stricter, under padding: a padded blob the mapper still accepts draws nothing. (This + /// direction already held before #556 — the pre-filter returned empty for everything padded — and + /// is kept so a future "just trim everywhere" change cannot start warning about a served tag.) + /// + /// An accepted fixture, padded below. + [Theory] + [MemberData(nameof(Accepted))] + public void Leading_whitespace_leaves_an_accepted_config_silent(string tagConfig) + { + var padded = Pad + tagConfig; + S7TagDefinitionFactory.FromTagConfig(padded, RawPath, out _) + .ShouldBeTrue("the mapper's JsonDocument.Parse skips leading whitespace"); + S7TagDefinitionFactory.Inspect(padded).ShouldBeEmpty(); + } + + /// + /// Never silent, under padding — the #556 item 1 defect itself. The pre-filter tested + /// reference[0] != '{', so EVERY padded blob returned empty and a padded config the driver + /// refuses deployed clean. Asserting equality with the untrimmed inspection (rather than merely + /// "not empty") also pins that padding changes no message. + /// + /// A rejected fixture, padded below. + [Theory] + [MemberData(nameof(Rejected))] + public void Leading_whitespace_draws_exactly_the_warnings_the_untrimmed_blob_draws(string tagConfig) + { + var padded = Pad + tagConfig; + S7TagDefinitionFactory.FromTagConfig(padded, RawPath, out _) + .ShouldBeFalse("the mapper's verdict is unchanged by padding"); + S7TagDefinitionFactory.Inspect(padded).ShouldBe(S7TagDefinitionFactory.Inspect(tagConfig)); + } + + /// + /// The other half of the same fix, and the one it would be easy to lose: trimming widens the gate + /// ONLY for JSON objects. A reference that is still not object-framed after the trim is not an + /// equipment-tag TagConfig at all — the pre-existing convention shared by all eleven inspectors — + /// so it keeps drawing nothing even though the mapper refuses it. + /// + /// A padded, non-object reference. + [Theory] + [InlineData(" just a string")] + [InlineData(" [1,2,3]")] + [InlineData(" \t\r\n 42")] + [InlineData(" \"a\"")] + [InlineData(" ")] + public void A_non_object_reference_still_draws_nothing_after_trimming(string reference) + { + S7TagDefinitionFactory.FromTagConfig(reference, RawPath, out _).ShouldBeFalse(); + S7TagDefinitionFactory.Inspect(reference).ShouldBeEmpty(); + } } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATInspectAcceptGateTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATInspectAcceptGateTests.cs index 468e40e0..60494626 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATInspectAcceptGateTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATInspectAcceptGateTests.cs @@ -68,4 +68,65 @@ public sealed class TwinCATInspectAcceptGateTests [Fact] public void Unparseable_json_names_the_json() => TwinCATTagDefinitionFactory.Inspect("{oops").ShouldContain(w => w.Contains("not valid JSON")); + + // ---- Gitea #556 item 1: leading whitespace no longer defeats the framing pre-filter ---- + + /// + /// Padding a blob with leading whitespace. A driver is handed whatever authoring persisted, and + /// FromTagConfig passes it straight to JsonDocument.Parse, which skips leading + /// whitespace — so the MAPPER's verdict is identical padded or not, and the gate's must be too. + /// + private const string Pad = " \t\r\n "; + + /// + /// Never stricter, under padding: a padded blob the mapper still accepts draws nothing. (This + /// direction already held before #556 — the pre-filter returned empty for everything padded — and + /// is kept so a future "just trim everywhere" change cannot start warning about a served tag.) + /// + /// An accepted fixture, padded below. + [Theory] + [MemberData(nameof(Accepted))] + public void Leading_whitespace_leaves_an_accepted_config_silent(string tagConfig) + { + var padded = Pad + tagConfig; + TwinCATTagDefinitionFactory.FromTagConfig(padded, RawPath, out _) + .ShouldBeTrue("the mapper's JsonDocument.Parse skips leading whitespace"); + TwinCATTagDefinitionFactory.Inspect(padded).ShouldBeEmpty(); + } + + /// + /// Never silent, under padding — the #556 item 1 defect itself. The pre-filter tested + /// reference[0] != '{', so EVERY padded blob returned empty and a padded config the driver + /// refuses deployed clean. Asserting equality with the untrimmed inspection (rather than merely + /// "not empty") also pins that padding changes no message. + /// + /// A rejected fixture, padded below. + [Theory] + [MemberData(nameof(Rejected))] + public void Leading_whitespace_draws_exactly_the_warnings_the_untrimmed_blob_draws(string tagConfig) + { + var padded = Pad + tagConfig; + TwinCATTagDefinitionFactory.FromTagConfig(padded, RawPath, out _) + .ShouldBeFalse("the mapper's verdict is unchanged by padding"); + TwinCATTagDefinitionFactory.Inspect(padded).ShouldBe(TwinCATTagDefinitionFactory.Inspect(tagConfig)); + } + + /// + /// The other half of the same fix, and the one it would be easy to lose: trimming widens the gate + /// ONLY for JSON objects. A reference that is still not object-framed after the trim is not an + /// equipment-tag TagConfig at all — the pre-existing convention shared by all eleven inspectors — + /// so it keeps drawing nothing even though the mapper refuses it. + /// + /// A padded, non-object reference. + [Theory] + [InlineData(" just a string")] + [InlineData(" [1,2,3]")] + [InlineData(" \t\r\n 42")] + [InlineData(" \"a\"")] + [InlineData(" ")] + public void A_non_object_reference_still_draws_nothing_after_trimming(string reference) + { + TwinCATTagDefinitionFactory.FromTagConfig(reference, RawPath, out _).ShouldBeFalse(); + TwinCATTagDefinitionFactory.Inspect(reference).ShouldBeEmpty(); + } }