diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/TagConfigJson.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/TagConfigJson.cs index 034dfe4c..4ed9c856 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/TagConfigJson.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/TagConfigJson.cs @@ -8,10 +8,13 @@ public enum JsonEnumRead /// The property is absent, or present but not a JSON string (nothing to validate). Absent, - /// The property is a JSON string that parses (case-insensitively) to the enum. + /// The property is a JSON string naming (case-insensitively) a declared enum member. Valid, - /// The property is a JSON string that does NOT parse to the enum (a typo'd value). + /// + /// The property is a JSON string that names no declared enum member — a typo, or a numeric + /// spelling such as "8", which is not a name at all. + /// Invalid, } @@ -67,6 +70,19 @@ public static class TagConfigJson /// treatment of an absent or present-but-non-string property — so a caller that extracts "the /// string value, or null" gets the runtime's leniency for free. /// + /// + /// Only a member NAME is valid — a numeric spelling is refused before the parse. + /// accepts "1" for any enum + /// and hands back whatever member sits at that ordinal, and an out-of-range "99" parses + /// just as readily — so a TagConfig carrying a numerically-serialised enum (the repo-wide + /// authoring trap) read at all six driver parsers and bound a + /// tag to a type nobody named, under Good quality. The same idiom guards + /// RawBrowseCommitMapper.NativeType and MTConnectTagDefinitionFactory's own type + /// parse. then covers what the leading-character + /// check cannot: TryParse also bitwise-ORs a comma-separated list for a + /// non-[Flags] enum, so "Beta, Gamma" would otherwise parse to a value no member + /// declares. + /// /// /// The enum type to parse. /// The authored value's text, or when there is none. @@ -80,12 +96,26 @@ public static class TagConfigJson value = default; return JsonEnumRead.Absent; } - if (Enum.TryParse(text, ignoreCase: true, out value)) + if (IsMemberNameSpelling(text) && Enum.TryParse(text, ignoreCase: true, out value) && Enum.IsDefined(value)) return JsonEnumRead.Valid; value = default; return JsonEnumRead.Invalid; } + /// + /// Whether is spelled as a member name rather than as a number. Judged on + /// the TRIMMED text because trims + /// before parsing, so " 8 " is the numeric spelling too; a C# member name can never begin + /// with a digit or a sign, so no valid name is lost. + /// + /// The authored value's text. + /// when the text could be a member name. + private static bool IsMemberNameSpelling(string text) + { + var span = text.AsSpan().Trim(); + return span.Length > 0 && !char.IsAsciiDigit(span[0]) && span[0] is not ('-' or '+'); + } + /// /// Lenient enum read with today's exact semantics: absent OR invalid ⇒ ; /// only a present, parseable string yields the parsed value. Bit-for-bit equivalent to the six diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/MqttTagConfigModel.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/MqttTagConfigModel.cs index 521957c5..f05bf28c 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/MqttTagConfigModel.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/MqttTagConfigModel.cs @@ -1,6 +1,8 @@ using System.Text.Json.Nodes; using ZB.MOM.WW.OtOpcUa.Core.Abstractions; using ZB.MOM.WW.OtOpcUa.Driver.Mqtt; +using RuntimeJsonEnumRead = ZB.MOM.WW.OtOpcUa.Core.Abstractions.JsonEnumRead; +using RuntimeTagConfigJson = ZB.MOM.WW.OtOpcUa.Core.Abstractions.TagConfigJson; namespace ZB.MOM.WW.OtOpcUa.AdminUI.Uns.TagEditors; @@ -415,13 +417,20 @@ public sealed class MqttTagConfigModel /// The property name to read. /// The parsed enum value, or null. private static TEnum? GetEnumNullable(JsonObject o, string name) where TEnum : struct, Enum - => TagConfigJson.GetString(o, name) is { } s && Enum.TryParse(s, ignoreCase: true, out var v) ? v : null; + => RuntimeTagConfigJson.ClassifyEnumText(TagConfigJson.GetString(o, name), out var v) + == RuntimeJsonEnumRead.Valid + ? v + : null; /// /// Describes a present-but-invalid enum field, or null when it is absent or valid. /// Mirrors TagConfigJson.TryReadEnum's absent/valid/invalid split exactly — including its /// treatment of a present-but-non-string value as ABSENT — so the editor never blocks a blob the - /// driver would happily default. + /// driver would happily default. The VERDICT is the runtime's own + /// , never a local + /// : a numeric spelling such as + /// "1" parses there and names no member (Driver.Sql.Contracts-002), so restating the rule + /// here would accept a blob the driver refuses — the Gitea #549 direction. /// /// The enum type expected. /// The TagConfig key bag. @@ -433,7 +442,7 @@ public sealed class MqttTagConfigModel { return null; } - return Enum.TryParse(raw, ignoreCase: true, out _) + return RuntimeTagConfigJson.ClassifyEnumText(raw, out _) == RuntimeJsonEnumRead.Valid ? null : $"'{raw}' is not a valid {name}; valid: {string.Join(", ", Enum.GetNames())}."; } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/SqlTagConfigModel.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/SqlTagConfigModel.cs index e55b6e28..24d62a85 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/SqlTagConfigModel.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/SqlTagConfigModel.cs @@ -1,6 +1,7 @@ using System.Text.Json.Nodes; using ZB.MOM.WW.OtOpcUa.Core.Abstractions; using ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts; +using RuntimeTagConfigJson = ZB.MOM.WW.OtOpcUa.Core.Abstractions.TagConfigJson; namespace ZB.MOM.WW.OtOpcUa.AdminUI.Uns.TagEditors; @@ -156,7 +157,9 @@ public sealed class SqlTagConfigModel // Reads an enum-valued string field the way the parser's TryReadEnumStrict does: absent OR present-but- // non-string ⇒ (null, null) — nothing to validate; a present string that parses ⇒ (value, null); a - // present string that does NOT parse ⇒ (null, offendingString) so Validate can reject it. + // present string that does NOT parse ⇒ (null, offendingString) so Validate can reject it. The verdict is + // the RUNTIME's own ClassifyEnumText, not a local Enum.TryParse — restating it is how the two doors drift + // (Gitea #549), and a numeric spelling like "1" is exactly where they would have (Driver.Sql.Contracts-002). private static (TEnum? value, string? invalidRaw) ReadEnumStrict(JsonObject o, string name) where TEnum : struct, Enum { @@ -164,7 +167,9 @@ public sealed class SqlTagConfigModel { return (null, null); } - return Enum.TryParse(s, ignoreCase: true, out var parsed) ? (parsed, null) : (null, s); + return RuntimeTagConfigJson.ClassifyEnumText(s, out var parsed) == JsonEnumRead.Valid + ? (parsed, null) + : (null, s); } } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/TagConfigJson.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/TagConfigJson.cs index 47c925a7..6537f462 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/TagConfigJson.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/TagConfigJson.cs @@ -1,5 +1,6 @@ using System.Text.Json; using System.Text.Json.Nodes; +using JsonEnumRead = ZB.MOM.WW.OtOpcUa.Core.Abstractions.JsonEnumRead; using RuntimeTagConfigJson = ZB.MOM.WW.OtOpcUa.Core.Abstractions.TagConfigJson; namespace ZB.MOM.WW.OtOpcUa.AdminUI.Uns.TagEditors; @@ -57,14 +58,23 @@ public static class TagConfigJson public static bool? GetBoolNullable(JsonObject o, string name) => o.TryGetPropertyValue(name, out var n) && n is JsonValue v && v.TryGetValue(out var b) ? b : null; - /// Reads an enum by its serialised name, or if absent/unparseable. + /// + /// Reads an enum by its serialised NAME, or if absent/unreadable. The + /// name/number distinction comes from + /// rather than from + /// directly, so a numerically-spelled + /// value the runtime refuses is not DISPLAYED here as the member sitting at that ordinal — the + /// editor would show, and then save, a type nobody authored. + /// /// The enum type to parse. /// The JSON object to read from. /// The property name to read. - /// The value to return if the property is absent or unparseable. + /// The value to return if the property is absent or unreadable. /// The parsed enum value, or . public static TEnum GetEnum(JsonObject o, string name, TEnum fallback) where TEnum : struct, Enum - => GetString(o, name) is { } s && Enum.TryParse(s, ignoreCase: true, out var v) ? v : fallback; + => RuntimeTagConfigJson.ClassifyEnumText(GetString(o, name), out var v) == JsonEnumRead.Valid + ? v + : fallback; /// /// Describes a present-but-invalid enum field the way the RUNTIME parser judges it, or diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/TagConfigJsonTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/TagConfigJsonTests.cs index 5a574b4a..7dae33c5 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/TagConfigJsonTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/TagConfigJsonTests.cs @@ -35,6 +35,79 @@ public sealed class TagConfigJsonTests v.ShouldBe(expectedValue); } + // ---- A numeric spelling is not a member NAME (Driver.Sql.Contracts-002) ---- + + /// + /// accepts a numeric STRING for any + /// enum — "1" hands back whatever member sits at ordinal 1, and an out-of-range "99" + /// parses just as happily — so a TagConfig carrying a numerically-serialised enum (the repo-wide + /// authoring trap: a page that serialises enums as numbers into a string-typed field) used to read + /// at every one of the six driver parsers. The tag was then served + /// under a data type nobody named, publishing a confident Good — the exact failure R2-11's + /// strict read exists to prevent. A digit- or sign-leading value is refused BEFORE the parse. + /// + [Theory] + [InlineData("\"1\"")] // in range — parses today, and IsDefined would confirm it + [InlineData("\"99\"")] // out of range — parses today too + [InlineData("\"0\"")] + [InlineData("\"-1\"")] // sign-leading + [InlineData("\"+1\"")] + [InlineData("\" 2 \"")] // Enum.TryParse trims, so the guard must judge the TRIMMED text + public void TryReadEnum_aNumericSpelling_isInvalid_notAMemberName(string token) + { + TagConfigJson.TryReadEnum(Root($"{{\"k\":{token}}}"), "k", out var v) + .ShouldBe(JsonEnumRead.Invalid); + v.ShouldBe(default(Sample)); + } + + /// + /// The half of the guard, which the digit-leading check + /// cannot cover: also accepts a + /// COMMA-SEPARATED list for a non-[Flags] enum and bitwise-ORs the members, so + /// "Beta, Gamma" parses to 3 — a value no member declares. Deleting the + /// IsDefined check turns this case green again. + /// + [Fact] + public void TryReadEnum_aCombinationThatNoMemberDeclares_isInvalid() + { + TagConfigJson.TryReadEnum(Root("{\"k\":\"Beta, Gamma\"}"), "k", out var v) + .ShouldBe(JsonEnumRead.Invalid); + v.ShouldBe(default(Sample)); + } + + /// + /// Falsifiability control for the two guards above: a guard that refused everything would satisfy + /// them all. Member NAMES — the only spelling a TagConfig may carry — keep parsing, whatever their + /// case and surrounding whitespace. + /// + [Theory] + [InlineData("Alpha", Sample.Alpha)] + [InlineData("beta", Sample.Beta)] + [InlineData("GAMMA", Sample.Gamma)] + [InlineData(" Gamma ", Sample.Gamma)] + public void ClassifyEnumText_stillAcceptsEveryMemberName(string text, Sample expected) + { + TagConfigJson.ClassifyEnumText(text, out var v).ShouldBe(JsonEnumRead.Valid); + v.ShouldBe(expected); + } + + /// + /// A numeric spelling is refused with the SAME wording every other rejection carries — naming the + /// field, the offending value and the members that are allowed — so the operator is told what to + /// write instead of being told nothing. + /// + [Fact] + public void DescribeInvalidEnum_aNumericSpelling_namesTheAllowedMembers() + { + var msg = TagConfigJson.DescribeInvalidEnum(Root("{\"dataType\":\"8\"}"), "dataType"); + msg.ShouldNotBeNull(); + msg.ShouldContain("'8'"); + msg.ShouldContain("dataType"); + msg.ShouldContain("Alpha"); + msg.ShouldContain("Beta"); + msg.ShouldContain("Gamma"); + } + [Theory] [InlineData("{\"k\":\"Gamma\"}", Sample.Gamma)] // valid → parsed [InlineData("{\"k\":\"typo\"}", Sample.Alpha)] // invalid → fallback @@ -50,6 +123,8 @@ public sealed class TagConfigJsonTests [InlineData("{}", true, Sample.Alpha)] // absent → fallback, ok [InlineData("{\"k\":42}", true, Sample.Alpha)] // non-string → Absent → fallback, ok [InlineData("{\"k\":\"typo\"}", false, default(Sample))] // invalid (typo) → REJECT (false) + [InlineData("{\"k\":\"1\"}", false, default(Sample))] // numeric spelling → REJECT (Driver.Sql.Contracts-002) + [InlineData("{\"k\":\"99\"}", false, default(Sample))] // …in range or out of it public void TryReadEnumStrict_rejects_only_invalid(string json, bool expectedOk, Sample expectedValue) { var ok = TagConfigJson.TryReadEnumStrict(Root(json), "k", Sample.Alpha, out var v); diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlEquipmentTagParserTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlEquipmentTagParserTests.cs index bec219f7..ae5e37c7 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlEquipmentTagParserTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlEquipmentTagParserTests.cs @@ -196,4 +196,31 @@ public class SqlEquipmentTagParserTests => SqlEquipmentTagParser.TryParse( """{"driver":"Sql","model":"KeyValue","table":"T","keyColumn":"k","keyValue":"v","valueColumn":"c","type":"Nonsense"}""", "raw", out _).ShouldBeFalse(); + + /// + /// Driver.Sql.Contracts-002. A numerically-spelled enum is not a member name, and the strict read + /// must refuse it exactly as it refuses a typo. It used to parse — "99" gave + /// (DriverDataType)99, a declared type nothing can map, and SqlPollReader.TryCoerce + /// then fell through its switch to the String arm and published a stringified cell under it. + /// + [Theory] + [InlineData("""{"driver":"Sql","model":"KeyValue","table":"T","keyColumn":"k","keyValue":"v","valueColumn":"c","type":"99"}""")] + [InlineData("""{"driver":"Sql","model":"KeyValue","table":"T","keyColumn":"k","keyValue":"v","valueColumn":"c","type":"8"}""")] + // "1" is SqlTagModel.WideRow's ordinal — an otherwise-complete WideRow blob, so the rejection can only + // come from the enum read and not from a missing field. + [InlineData("""{"driver":"Sql","model":"1","table":"T","columnName":"c","rowSelector":{"topByTimestamp":"ts"}}""")] + public void TryParse_aNumericallySpelledEnum_rejectsTag(string json) + => SqlEquipmentTagParser.TryParse(json, "raw", out _).ShouldBeFalse(); + + /// + /// …and the deploy gate explains it in the same words every other invalid enum draws, rather than + /// falling through to the backstop warning that names no field at all. + /// + [Fact] + public void Inspect_aNumericallySpelledType_namesTheFieldAndTheValidMembers() + { + var warnings = SqlTagDefinitionFactory.Inspect( + """{"driver":"Sql","model":"KeyValue","table":"T","keyColumn":"k","keyValue":"v","valueColumn":"c","type":"99"}"""); + warnings.ShouldContain(w => w.Contains("'99'") && w.Contains("type") && w.Contains(nameof(DriverDataType.Float64))); + } } 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 index 6bb8c2c0..ec558401 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/TagConfigAuthoringDeployParityTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/TagConfigAuthoringDeployParityTests.cs @@ -220,6 +220,37 @@ public sealed class TagConfigAuthoringDeployParityTests + "import or API is unchecked — the exact G-7 asymmetry."); } + /// + /// Driver.Sql.Contracts-002, fleet-wide. A numerically-SPELLED enum ("1", not the JSON + /// number 1) used to satisfy Enum.TryParse at every driver, so both doors accepted it + /// and the tag was served under whatever member sat at that ordinal — or, out of range, under a + /// value no member declares. The shared TagConfigJson.ClassifyEnumText now calls it what it + /// is, and because BOTH gates ask that one method, they moved together: the fix is only real if + /// each driver refuses it at the modal AND at the deploy gate. + /// + [Theory] + // Complete but for the enum spelling, so the refusal can only come from the enum read: "1" is + // SqlTagModel.WideRow's ordinal, and every WideRow-required field is present. + [InlineData(DriverTypeNames.Sql, """{"model":"1","table":"dbo.T","columnName":"c","rowSelector":{"topByTimestamp":"ts"}}""")] + [InlineData(DriverTypeNames.Sql, """{"model":"KeyValue","table":"dbo.T","keyColumn":"k","keyValue":"v","valueColumn":"c","type":"8"}""")] + [InlineData(DriverTypeNames.Modbus, """{"region":"1","address":1}""")] + [InlineData(DriverTypeNames.Modbus, """{"region":"HoldingRegisters","address":1,"dataType":"99"}""")] + [InlineData(DriverTypeNames.S7, """{"address":"DB1.DBW0","dataType":"1"}""")] + [InlineData(DriverTypeNames.AbCip, """{"tagPath":"Program:Main.Speed","dataType":"1"}""")] + [InlineData(DriverTypeNames.AbLegacy, """{"address":"N7:0","dataType":"1"}""")] + [InlineData(DriverTypeNames.TwinCAT, """{"symbolPath":"MAIN.bStart","dataType":"1"}""")] + [InlineData(DriverTypeNames.FOCAS, """{"address":"R100","dataType":"1"}""")] + [InlineData(DriverTypeNames.Mqtt, """{"topic":"otopcua/fixture/a","payloadFormat":"1"}""")] + [InlineData(DriverTypeNames.Mqtt, """{"topic":"otopcua/fixture/a","dataType":"8"}""")] + public void A_numerically_spelled_enum_is_refused_by_both_gates(string driverType, string tagConfig) + { + TagConfigValidator.Validate(driverType, tagConfig).ShouldNotBeNullOrEmpty( + $"{driverType}: the modal would let an operator save an enum spelling the driver refuses."); + EquipmentTagConfigInspector.Inspect(driverType, tagConfig).ShouldNotBeEmpty( + $"{driverType}: the deploy gate accepted a numeric enum spelling, so a CSV-imported or " + + "API-authored tag binds to a type nobody named."); + } + // ------------------------------------------------------------------------------------------------- // Gitea #550 — MTConnect's second spelling of the coercion type. The assertions below drive the REAL // runtime reader (MTConnectTagDefinitionFactory.TryFromTagConfig) rather than the deploy gate alone,