fix(tagconfig): close the four #551 residuals — accept-gate-first Inspect, three authoring rules, CSV/service gate, doc sweep

Gitea #551. 1. CSV import ran no driver-typed config gate. Its typed columns were always strictly parsed, but the free-form TagConfigJson column reached the database unread, so a CSV could author a tag the driver rejects — one that deploys green and reads BadNodeIdUnknown forever. RawTagCsvMapper.Parse now names the offending row in the staged review grid, and RawTreeService enforces the same gate on CreateTag/UpdateTag/ImportTags: a form cannot be the guarantee (the API and any future caller reach none of them, and this repo has no bUnit). A rejected import row is an ERROR — the batch inserts nothing and the row is named — never a skip. Note the brief's premise that manual entry rides ImportTagsAsync is wrong: RawManualTagEntryModal commits row-by-row via CreateTagAsync, which is why the gate had to land on all three write paths. 2/3. The seven pre-#549 Inspect methods ran a hand-list of DescribeInvalidEnum checks, so every rejection cause not on the list was invisible. They are now ACCEPT-GATE-FIRST (the MTConnect/Sql/OpcUaClient/Calculation shape): ask the driver's own reader for the verdict, then explain the refusal, with a catch-all so "the parse rejects but Inspect said nothing" is unreachable. That closes S7's absent dataType, AbCip members[], Modbus stringByteOrder and the type-dependent guards, plus causes the issue did not name (absent region/address/tagPath/symbolPath/topic, TwinCAT Structure, MQTT's Sparkplug tuple). FOCAS and MQTT keep advisories about ACCEPTED blobs, so they diagnose rather than return empty. Authoring half: ModbusTagConfigModel gains stringByteOrder; AbCipTagConfigModel asks the new AbCipTagDefinitionFactory.DescribeInvalidMember; S7TagConfigModel captures whether the SOURCE blob declared dataType and asks S7TagDefinitionFactory.DataTypeKey/DeclaresDataType. One authority per rule. 4. Doc sweep: the six factory Inspect summaries, EquipmentTagConfigInspector's class remarks and AdminOperationsActor's comment all claimed invalid enums are "silently defaulted by the lenient runtime" — false since R2-11 Phase C made the runtime reads strict. They describe rejections now.
2026-08-10 00:08:06 -04:00
parent b0d2fa30e3
commit 05b424788f
24 changed files with 1846 additions and 100 deletions
@@ -130,18 +130,41 @@ public static class ModbusTagDefinitionFactory
    }

    /// <summary>
-    ///     Deploy-time inspection of an equipment-tag <c>TagConfig</c> blob (05/CONV-2). Returns
-    ///     human-readable warnings for present-but-invalid enum values (<c>region</c> / <c>dataType</c> /
-    ///     <c>byteOrder</c> — which the lenient runtime silently defaults) and for a structurally
-    ///     unparseable TagConfig (which the runtime turns into a silent <c>BadNodeIdUnknown</c>). Empty when
-    ///     the blob is clean or is not an equipment-tag TagConfig object. Never throws.
+    ///     The RawPath handed to <see cref="FromTagConfig"/> during inspection. The mapper only copies it
+    ///     onto <see cref="ModbusTagDefinition.Name"/> and never validates it, so inspection is free to pass
+    ///     a placeholder — the deploy gate is inspecting a blob, not a placed tag.
    /// </summary>
+    private const string InspectionRawPath = "(inspect)";
+
+    /// <summary>
+    ///     Deploy-time inspection of an equipment-tag <c>TagConfig</c> blob (05/CONV-2). Returns
+    ///     human-readable warnings for a blob <see cref="FromTagConfig"/> would <b>reject</b> — a
+    ///     structurally unparseable or non-object document, an absent <c>region</c>, an absent or
+    ///     out-of-range <c>address</c>, a present-but-invalid <c>region</c> / <c>dataType</c> /
+    ///     <c>byteOrder</c> / <c>stringByteOrder</c> (each rejects the tag: the runtime enum reads are
+    ///     STRICT since R2-11 Phase C), or a type-dependent guard (<c>String</c> with no
+    ///     <c>stringLength</c>, <c>BitInRegister</c> with a <c>bitIndex</c> outside 0–15). Such a tag is
+    ///     skipped by the driver and reads <c>BadNodeIdUnknown</c> forever. Empty when the mapper accepts
+    ///     the blob, when it is blank, or when it is not an equipment-tag TagConfig object. Never throws.
+    /// </summary>
+    /// <remarks>
+    ///     <b>The verdict comes from <see cref="FromTagConfig"/>, not from a mirror</b> (Gitea #551). The
+    ///     rule this gate must never break is being stricter than the runtime — a blob the driver serves
+    ///     must not draw a warning calling it invalid — and running the mapper first makes that structural
+    ///     rather than a promise. It also closes the opposite gap: the previous hand-list of enum checks
+    ///     was silent for every rejection cause that was not on it (an absent <c>region</c>, a bad
+    ///     <c>address</c>, <c>stringByteOrder</c>), so a tag the driver refuses deployed clean.
+    /// </remarks>
    /// <param name="reference">The equipment tag's TagConfig JSON.</param>
    /// <returns>The warnings; empty when clean.</returns>
    public static IReadOnlyList<string> Inspect(string reference)
    {
        var warnings = new List<string>();
        if (string.IsNullOrWhiteSpace(reference) || reference[0] != '{') return warnings;
+
+        // The accept gate: whatever the driver reads draws nothing. See the remarks.
+        if (FromTagConfig(reference, InspectionRawPath, out _)) return warnings;
+
        try
        {
            using var doc = JsonDocument.Parse(reference);
@@ -151,23 +174,90 @@ public static class ModbusTagDefinitionFactory
                warnings.Add("Modbus TagConfig root is not a JSON object — the tag will not resolve (BadNodeIdUnknown).");
                return warnings;
            }
-            foreach (var w in new[]
-            {
-                TagConfigJson.DescribeInvalidEnum<ModbusRegion>(root, "region"),
-                TagConfigJson.DescribeInvalidEnum<ModbusDataType>(root, "dataType"),
-                TagConfigJson.DescribeInvalidEnum<ModbusByteOrder>(root, "byteOrder"),
-            })
-            {
-                if (w is not null) warnings.Add(w);
-            }
+
+            DescribeRejection(root, warnings);
        }
        catch (JsonException)
        {
            warnings.Add("Modbus TagConfig is not valid JSON — the tag will not resolve (BadNodeIdUnknown).");
+            return warnings;
        }
+
+        // Backstop: the mapper refused for a reason nothing above names (it also swallows FormatException
+        // and InvalidOperationException). Saying so beats reporting a clean blob the driver will skip.
+        if (warnings.Count == 0)
+        {
+            warnings.Add(
+                "Modbus TagConfig was rejected by the driver's tag reader — the driver will skip this tag "
+                + "and it will read BadNodeIdUnknown.");
+        }
+
        return warnings;
    }

+    /// <summary>
+    ///     Explains why <see cref="FromTagConfig"/> refused an object-rooted blob, walking its checks in the
+    ///     same order. Every branch mirrors a <c>return false</c> there — and it runs only after the mapper
+    ///     has already refused, so it can never invent strictness of its own.
+    /// </summary>
+    /// <param name="root">The TagConfig root object.</param>
+    /// <param name="warnings">The list to append warnings to.</param>
+    private static void DescribeRejection(JsonElement root, List<string> warnings)
+    {
+        if (!root.TryGetProperty("region", out _))
+        {
+            warnings.Add(
+                "Modbus TagConfig has no 'region' — every Modbus tag must name one (valid: "
+                + string.Join(", ", Enum.GetNames<ModbusRegion>())
+                + "); the driver will skip this tag and it will read BadNodeIdUnknown.");
+        }
+
+        if (!root.TryGetProperty("address", out var addr))
+        {
+            warnings.Add("Modbus TagConfig has no 'address' — the tag will not resolve (BadNodeIdUnknown).");
+        }
+        else if (addr.ValueKind != JsonValueKind.Number
+                 || !addr.TryGetInt32(out var address)
+                 || address < 0 || address > ushort.MaxValue)
+        {
+            warnings.Add(
+                $"value '{addr.GetRawText()}' for 'address' is not a Modbus register address (an integer "
+                + $"0–{ushort.MaxValue}); the tag will not resolve (BadNodeIdUnknown).");
+        }
+
+        // Strict enums, in the order FromTagConfig reads them. stringByteOrder is read as strictly as its
+        // three siblings, and was the one the previous hand-list forgot (Gitea #551).
+        foreach (var w in new[]
+        {
+            TagConfigJson.DescribeInvalidEnum<ModbusRegion>(root, "region"),
+            TagConfigJson.DescribeInvalidEnum<ModbusDataType>(root, "dataType"),
+            TagConfigJson.DescribeInvalidEnum<ModbusByteOrder>(root, "byteOrder"),
+            TagConfigJson.DescribeInvalidEnum<ModbusStringByteOrder>(root, "stringByteOrder"),
+        })
+        {
+            if (w is not null) warnings.Add(w);
+        }
+
+        // Type-dependent guards. The byte/ushort NARROWING is part of the rule, not an accident of the
+        // mapper: it reads `(byte)bitIndex` / `(ushort)stringLength`, so 256 is bit 0 and is ACCEPTED.
+        // Describing the un-narrowed value here would call a blob invalid that the driver serves.
+        if (!TagConfigJson.TryReadEnumStrict(root, "dataType", ModbusDataType.Int16, out var dataType)) return;
+
+        if (dataType == ModbusDataType.String && (ushort)ReadInt(root, "stringLength") < 1)
+        {
+            warnings.Add(
+                "Modbus String tag has no 'stringLength' of at least 1 — a zero-length string reads zero "
+                + "registers, which is an illegal Modbus request, so the tag is rejected (BadNodeIdUnknown).");
+        }
+
+        if (dataType == ModbusDataType.BitInRegister && (byte)ReadInt(root, "bitIndex") > 15)
+        {
+            warnings.Add(
+                "Modbus BitInRegister tag has a 'bitIndex' outside 0–15 — the bit mask would overflow a "
+                + "16-bit register, so the tag is rejected (BadNodeIdUnknown).");
+        }
+    }
+
    private static int ReadInt(JsonElement o, string name)
        => o.TryGetProperty(name, out var e) && e.ValueKind == JsonValueKind.Number
           && e.TryGetInt32(out var v) ? v : 0;