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 9833df5d..034dfe4c 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/TagConfigJson.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/TagConfigJson.cs @@ -48,7 +48,39 @@ public static class TagConfigJson value = default; return JsonEnumRead.Absent; } - if (Enum.TryParse(e.GetString(), ignoreCase: true, out value)) + return ClassifyEnumText(e.GetString(), out value); + } + + /// + /// The text-level heart of , and with it the single authority on + /// which enum spellings a TagConfig may carry: classifies an already-extracted enum STRING. + /// + /// It is public because the authoring side reads the same blob through a different door — the + /// AdminUI's typed tag-config editors hold a mutable JsonObject key bag, not a + /// — and an authoring gate that RESTATED the accept/reject boundary + /// would drift from the runtime the moment either side moved, in the one direction that hurts: + /// a value the modal accepts and the driver then rejects (Gitea #549). Both doors reach this + /// method. + /// + /// + /// is , matching the JSON reader's + /// 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. + /// + /// + /// The enum type to parse. + /// The authored value's text, or when there is none. + /// The parsed enum on ; otherwise default. + /// The read outcome. + public static JsonEnumRead ClassifyEnumText(string? text, out TEnum value) + where TEnum : struct, Enum + { + if (text is null) + { + value = default; + return JsonEnumRead.Absent; + } + if (Enum.TryParse(text, ignoreCase: true, out value)) return JsonEnumRead.Valid; value = default; return JsonEnumRead.Invalid; @@ -120,10 +152,26 @@ public static class TagConfigJson /// The warning text, or . public static string? DescribeInvalidEnum(JsonElement o, string name) where TEnum : struct, Enum + => TryReadEnum(o, name, out _) != JsonEnumRead.Invalid + ? null + : DescribeInvalidEnumText(name, o.TryGetProperty(name, out var e) ? e.GetString() : null); + + /// + /// The text-level counterpart of , + /// over the same verdict: a human-readable warning for a + /// present-but-invalid enum value, or when it is absent + /// ( ) or valid. The authoring gate uses this so its + /// wording, and not only its verdict, comes from the same place as the deploy gate's. + /// + /// The enum type expected. + /// The property name to describe. + /// The authored value's text, or when there is none. + /// The warning text, or . + public static string? DescribeInvalidEnumText(string name, string? text) + where TEnum : struct, Enum { - if (TryReadEnum(o, name, out _) != JsonEnumRead.Invalid) return null; - var raw = o.TryGetProperty(name, out var e) ? e.GetString() : null; + if (ClassifyEnumText(text, out _) != JsonEnumRead.Invalid) return null; var valid = string.Join(", ", Enum.GetNames()); - return $"value '{raw}' for '{name}' is not a valid {typeof(TEnum).Name}; valid: {valid}"; + return $"value '{text}' for '{name}' is not a valid {typeof(TEnum).Name}; valid: {valid}"; } } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Contracts/MTConnectTagDefinitionFactory.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Contracts/MTConnectTagDefinitionFactory.cs index 129fba5e..02b843f3 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Contracts/MTConnectTagDefinitionFactory.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Contracts/MTConnectTagDefinitionFactory.cs @@ -154,6 +154,28 @@ public static class MTConnectTagDefinitionFactory return warnings; } + /// + /// The single authority on which coercion-type spellings an MTConnect tag may carry, consulted + /// by the runtime + deploy gate (through / ) and + /// by the AdminUI's authoring model (MTConnectTagConfigModel.Validate) alike. + /// + /// It is public for exactly that second caller. The authoring model used to read the type with a + /// lenient enum parse that fell back to String on ANY unrecognised value, so a misspelled + /// name ("Strng") from a hand-authored blob, a CSV import or the API was accepted at authoring — + /// and silently rewritten — while this parse rejected it and the tag died at the driver + /// (Gitea #549). A second copy of the rule in the AdminUI would have reopened that gap the + /// moment either side moved, so there is one rule and both sides ask it. + /// + /// + /// A blank/absent value is ACCEPTED: it declares no override, and + /// then falls back to the Agent's /probe declaration rather than rejecting the tag. + /// + /// + /// The authored type text, or /blank when none was authored. + /// when the driver would accept this authored type. + public static bool IsAcceptedDataTypeName(string? raw) + => string.IsNullOrWhiteSpace(raw) || TryParseDataType(raw.Trim(), out _); + /// /// Parses an authored coercion type by NAME. A digits-only value is rejected outright, BEFORE the /// parse: accepts "8" as readily diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/AbCipTagConfigModel.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/AbCipTagConfigModel.cs index 4dd9044c..ace7cbc6 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/AbCipTagConfigModel.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/AbCipTagConfigModel.cs @@ -52,7 +52,18 @@ public sealed class AbCipTagConfigModel } /// Validation hook; returns an error message or null when the model is valid. + /// + /// The dataType check reads the ORIGINAL key bag rather than the leniently-defaulted + /// , because AbCipTagDefinitionFactory reads it with + /// TryReadEnumStrict — a present-but-invalid string rejects the whole tag at the driver + /// (BadNodeIdUnknown). Without this the blob would be accepted here, silently rewritten to + /// DInt on save, and — on the CSV-import and API paths, which save it verbatim — deployed + /// into a tag the driver skips (Gitea #549). + /// /// An error message describing the validation failure, or null when valid. public string? Validate() - => string.IsNullOrWhiteSpace(TagPath) ? "Tag path is required." : null; + { + if (TagConfigJson.DescribeInvalidEnum(_bag, "dataType") is { } dtError) { return dtError; } + return string.IsNullOrWhiteSpace(TagPath) ? "Tag path is required." : null; + } } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/AbLegacyTagConfigModel.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/AbLegacyTagConfigModel.cs index 3f297905..c560ddf0 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/AbLegacyTagConfigModel.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/AbLegacyTagConfigModel.cs @@ -55,6 +55,14 @@ public sealed class AbLegacyTagConfigModel /// An error message describing the validation failure, or null when valid. public string? Validate() { + // Strict enum, read from the ORIGINAL key bag: AbLegacyTagDefinitionFactory reads "dataType" with + // TryReadEnumStrict, so a present-but-invalid string rejects the whole tag at the driver + // (BadNodeIdUnknown) while the lenient load above would default it to Int. Accepting it here would + // let the CSV-import and API paths — which save the blob verbatim — deploy an unreadable tag + // (Gitea #549). + if (TagConfigJson.DescribeInvalidEnum(_bag, "dataType") is { } dtError) + return dtError; + if (string.IsNullOrWhiteSpace(Address)) return "Address is required."; diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/FocasTagConfigModel.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/FocasTagConfigModel.cs index b7a22faf..3a389e9b 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/FocasTagConfigModel.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/FocasTagConfigModel.cs @@ -59,6 +59,13 @@ public sealed class FocasTagConfigModel /// public string? Validate() { + // Strict enum, read from the ORIGINAL key bag: FocasTagDefinitionFactory reads "dataType" with + // TryReadEnumStrict, so a present-but-invalid string rejects the whole tag at the driver + // (BadNodeIdUnknown) while the lenient load above defaults it to Int32. Accepting it here would + // let the CSV-import and API paths — which save the blob verbatim — deploy an unreadable tag, + // the same shape as the #540 address gap this method already closes (Gitea #549). + if (TagConfigJson.DescribeInvalidEnum(_bag, "dataType") is { } dtError) return dtError; + if (string.IsNullOrWhiteSpace(Address)) return "Address is required."; return FocasAddress.TryParse(Address) is null ? DescribeUnparseableAddress(Address) : null; } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/MTConnectTagConfigModel.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/MTConnectTagConfigModel.cs index 30898795..c2679e46 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/MTConnectTagConfigModel.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/MTConnectTagConfigModel.cs @@ -1,5 +1,6 @@ using System.Text.Json.Nodes; using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Driver.MTConnect; namespace ZB.MOM.WW.OtOpcUa.AdminUI.Uns.TagEditors; @@ -57,12 +58,17 @@ public sealed class MTConnectTagConfigModel /// consumed by the driver or the runtime. public string? MtComponent { get; set; } - // The as-loaded raw "dataType" JSON value, kept only to detect the ParseEnum numeric-text trap in - // Validate() below — Enum.TryParse (which TagConfigJson.GetEnum uses) silently accepts a quoted - // number ("8" -> Float64) exactly as readily as a name, so a config that was ever hand-authored or - // migrated with a numeric dataType would otherwise load into a *valid-looking* enum value with no - // signal that it isn't a name. ToJson() always re-writes DataType as a name, so this can only ever - // be non-null on the FIRST load of an already-poisoned config, never after a save through this model. + // The as-loaded raw "dataType" JSON token, kept so Validate() below can put it to the DRIVER's own + // accept/reject rule (MTConnectTagDefinitionFactory.IsAcceptedDataTypeName) rather than to the + // lenient load. TagConfigJson.GetEnum falls back to String on ANY unrecognised value, so without this + // a misspelled name ("Strng") or a quoted number ("8" -> Float64, the ParseEnum numeric-text trap) + // loads as a *valid-looking* enum value with no signal that it is not what the operator wrote — while + // the strict runtime rejects the tag outright (Gitea #549). Captured as the raw TOKEN, not via + // GetString: the runtime deserialises this key into a `string?` DTO field, so a NON-string token + // (`"dataType": 8`) throws JsonException there and rejects the tag too, where GetString would report + // it as simply absent. ToJson() always re-writes dataType as a name, so this can only ever be + // poisoned on the FIRST load of a blob authored elsewhere (hand-edited, CSV import, API), never after + // a save through this model. private string? _rawDataType; private JsonObject _bag = new(); @@ -96,7 +102,7 @@ public sealed class MTConnectTagConfigModel Units = TagConfigJson.GetString(o, "units"), MtDevice = TagConfigJson.GetString(o, "mtDevice"), MtComponent = TagConfigJson.GetString(o, "mtComponent"), - _rawDataType = TagConfigJson.GetString(o, "dataType"), + _rawDataType = ReadRawDataType(o), _bag = o, }; } @@ -124,6 +130,15 @@ public sealed class MTConnectTagConfigModel } /// Validation hook; returns an error message or null when the model is valid. + /// + /// The dataType rule is not stated here — it is asked of + /// , the same predicate the driver + /// and the deploy gate use. One authority, so this gate can never end up weaker than the runtime + /// again (Gitea #549). Note that the previous, digits-only check is NOT retained as a tailored + /// special case: the runtime draws no distinction between a bare number and a misspelled name — + /// both fail that one predicate and both draw the same Inspect warning — so a second message + /// here would be a second statement of a rule this model does not own. + /// /// An error message describing the validation failure, or null when the model is valid. public string? Validate() { @@ -132,16 +147,26 @@ public sealed class MTConnectTagConfigModel return "A DataItem id (fullName) is required."; } - // See the _rawDataType remarks: a quoted number parses as a "valid" enum today but is never - // something this editor itself would have written, so surface it instead of silently accepting it. - if (_rawDataType is { Length: > 0 } raw && raw.All(char.IsAsciiDigit)) + // See the _rawDataType remarks: the value the blob CARRIES is judged, not the leniently-defaulted + // DataType above, because the CSV-import and API paths save the blob verbatim. + if (!MTConnectTagDefinitionFactory.IsAcceptedDataTypeName(_rawDataType)) { - return $"dataType \"{raw}\" is a numeric value, not a named type — re-select a data type."; + return $"dataType \"{_rawDataType}\" is not a valid DriverDataType name — re-select a data type. " + + $"Valid: {string.Join(", ", Enum.GetNames())}."; } return null; } + // The as-loaded "dataType" TOKEN as text (see the _rawDataType remarks): a JSON string yields its + // value, any other present token yields its JSON text so Validate() judges what was actually written, + // and absent / JSON null yields null — the one shape the runtime treats as "no type declared". + private static string? ReadRawDataType(JsonObject o) + { + if (!o.TryGetPropertyValue("dataType", out var node) || node is null) { return null; } + return node is JsonValue v && v.TryGetValue(out var s) ? s : node.ToJsonString(); + } + // Normalises a blank/whitespace-only string to null so TagConfigJson.Set omits the key rather than // persisting an empty string. private static string? Blank(string? value) => string.IsNullOrWhiteSpace(value) ? null : value; diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/ModbusTagConfigModel.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/ModbusTagConfigModel.cs index 3ac57284..9396b38c 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/ModbusTagConfigModel.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/ModbusTagConfigModel.cs @@ -68,6 +68,18 @@ public sealed class ModbusTagConfigModel } /// Validation hook; returns an error message or null when the model is valid. + /// + /// All three enums are read from the ORIGINAL key bag, not from the leniently-defaulted typed + /// fields above: ModbusTagDefinitionFactory reads each with TryReadEnumStrict, so a + /// present-but-invalid string rejects the whole tag at the driver (BadNodeIdUnknown) where + /// the load silently substitutes HoldingRegisters/Int16/BigEndian. Accepting + /// one here would let the CSV-import and API paths — which save the blob verbatim — deploy a tag + /// the driver skips (Gitea #549). Also note a wrong-but-valid region is not this gate's + /// business: only a spelling the runtime cannot read is rejected. + /// /// An error message describing the validation failure, or null when the model is valid. - public string? Validate() => null; + public string? Validate() + => TagConfigJson.DescribeInvalidEnum(_bag, "region") + ?? TagConfigJson.DescribeInvalidEnum(_bag, "dataType") + ?? TagConfigJson.DescribeInvalidEnum(_bag, "byteOrder"); } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/S7TagConfigModel.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/S7TagConfigModel.cs index 6cd40543..5617bb3c 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/S7TagConfigModel.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/S7TagConfigModel.cs @@ -53,6 +53,16 @@ public sealed class S7TagConfigModel /// Validation hook; returns an error message or null when the model is valid. /// An error message describing the validation failure, or null when the model is valid. + /// + /// The dataType check reads the ORIGINAL key bag rather than the leniently-defaulted + /// : S7TagDefinitionFactory reads it with TryReadEnumStrict, + /// so a present-but-invalid string rejects the whole tag at the driver (BadNodeIdUnknown) + /// where the load silently substitutes Int16. Accepting it here would let the CSV-import + /// and API paths — which save the blob verbatim — deploy an unreadable tag (Gitea #549). + /// public string? Validate() - => string.IsNullOrWhiteSpace(Address) ? "Address is required." : null; + { + if (TagConfigJson.DescribeInvalidEnum(_bag, "dataType") is { } dtError) { return dtError; } + return string.IsNullOrWhiteSpace(Address) ? "Address is required." : null; + } } 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 01624144..47c925a7 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 RuntimeTagConfigJson = ZB.MOM.WW.OtOpcUa.Core.Abstractions.TagConfigJson; namespace ZB.MOM.WW.OtOpcUa.AdminUI.Uns.TagEditors; @@ -65,6 +66,33 @@ public static class TagConfigJson 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; + /// + /// Describes a present-but-invalid enum field the way the RUNTIME parser judges it, or + /// null when the field is absent, present-but-non-string, or valid. + /// + /// is deliberately lenient — it must be, because a model has to + /// LOAD a poisoned blob to let an operator fix it — but leniency at the save gate is the + /// Gitea #549 defect: every driver's runtime read is + /// TagConfigJson.TryReadEnumStrict, which REJECTS the whole tag on a present-but-invalid + /// string (R2-11 Phase C), so a typo the modal accepts and rewrites is a tag the driver would + /// have skipped with BadNodeIdUnknown on the CSV-import and API paths, which save the + /// blob verbatim. A model's Validate() therefore calls this on its ORIGINAL key bag. + /// + /// + /// The verdict and the wording both come from + /// — the same authority the + /// runtime parsers and the deploy gate consult — rather than being restated here. Passing + /// 's null-for-non-string result reproduces the runtime's + /// Absent treatment of a non-string token exactly. + /// + /// + /// The enum type expected. + /// The model's preserved TagConfig key bag, as loaded. + /// The property name to check. + /// The error text, or null when the runtime would accept the field. + public static string? DescribeInvalidEnum(JsonObject o, string name) where TEnum : struct, Enum + => RuntimeTagConfigJson.DescribeInvalidEnumText(name, GetString(o, name)); + /// Sets a string/number/enum-name value (enums via ToString()). A null value REMOVES the key, so it is omitted from the serialised JSON. /// The JSON object to mutate. /// The property name to set. diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/TwinCATTagConfigModel.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/TwinCATTagConfigModel.cs index ec9961ea..4cdff97e 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/TwinCATTagConfigModel.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/TwinCATTagConfigModel.cs @@ -53,6 +53,17 @@ public sealed class TwinCATTagConfigModel /// Validation hook; returns an error message or null when the model is valid. /// An error message describing the validation failure, or null when the model is valid. + /// + /// The dataType check reads the ORIGINAL key bag rather than the leniently-defaulted + /// : TwinCATTagDefinitionFactory reads it with + /// TryReadEnumStrict, so a present-but-invalid string rejects the whole tag at the driver + /// (BadNodeIdUnknown) where the load silently substitutes DInt. Accepting it here + /// would let the CSV-import and API paths — which save the blob verbatim — deploy an unreadable + /// tag (Gitea #549). + /// public string? Validate() - => string.IsNullOrWhiteSpace(SymbolPath) ? "Symbol path is required." : null; + { + if (TagConfigJson.DescribeInvalidEnum(_bag, "dataType") is { } dtError) { return dtError; } + return string.IsNullOrWhiteSpace(SymbolPath) ? "Symbol path is required." : null; + } } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ZB.MOM.WW.OtOpcUa.AdminUI.csproj b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ZB.MOM.WW.OtOpcUa.AdminUI.csproj index c3b46211..e014d2bf 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ZB.MOM.WW.OtOpcUa.AdminUI.csproj +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ZB.MOM.WW.OtOpcUa.AdminUI.csproj @@ -26,6 +26,12 @@ + + diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/AbCipTagConfigModelTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/AbCipTagConfigModelTests.cs index 73fb1c41..a80ae0dd 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/AbCipTagConfigModelTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/AbCipTagConfigModelTests.cs @@ -99,4 +99,25 @@ public sealed class AbCipTagConfigModelTests json.ShouldContain("\"deviceHostAddress\":\"ab://host\""); } + + /// + /// Gitea #549: the LOAD is lenient — an unrecognised name silently becomes DInt — but + /// AbCipTagDefinitionFactory reads dataType with TryReadEnumStrict and rejects + /// the whole tag, so the save gate must judge the value the blob actually carries rather than the + /// one the load substituted. + /// + [Fact] + public void Validate_refuses_a_dataType_the_driver_would_reject() + => AbCipTagConfigModel.FromJson("""{"tagPath":"Motor1.Status","dataType":"DIntt"}""") + .Validate().ShouldNotBeNull().ShouldContain("DIntt"); + + /// The other direction, which matters just as much: the check is the runtime's rule and not a + /// stricter one — a valid name in any casing, an absent key, and a present-but-NON-STRING token (which + /// TryReadEnum treats as absent) are all served by the driver and so must all save. + [Theory] + [InlineData("""{"tagPath":"Motor1.Status","dataType":"real"}""")] + [InlineData("""{"tagPath":"Motor1.Status"}""")] + [InlineData("""{"tagPath":"Motor1.Status","dataType":2}""")] + public void Validate_accepts_every_dataType_the_driver_accepts(string json) + => AbCipTagConfigModel.FromJson(json).Validate().ShouldBeNull(); } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/AbLegacyTagConfigModelTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/AbLegacyTagConfigModelTests.cs index e9e8f2f4..975bd68e 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/AbLegacyTagConfigModelTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/AbLegacyTagConfigModelTests.cs @@ -129,4 +129,25 @@ public sealed class AbLegacyTagConfigModelTests m.Validate().ShouldBeNull(); } + + /// + /// Gitea #549: the LOAD is lenient — an unrecognised name silently becomes Int — but + /// AbLegacyTagDefinitionFactory reads dataType with TryReadEnumStrict and + /// rejects the whole tag, so the save gate must judge the value the blob actually carries rather + /// than the one the load substituted. + /// + [Fact] + public void Validate_refuses_a_dataType_the_driver_would_reject() + => AbLegacyTagConfigModel.FromJson("""{"address":"N7:0","dataType":"Intt"}""") + .Validate().ShouldNotBeNull().ShouldContain("Intt"); + + /// The other direction, which matters just as much: the check is the runtime's rule and not a + /// stricter one — a valid name in any casing, an absent key, and a present-but-NON-STRING token (which + /// TryReadEnum treats as absent) are all served by the driver and so must all save. + [Theory] + [InlineData("""{"address":"N7:0","dataType":"float"}""")] + [InlineData("""{"address":"N7:0"}""")] + [InlineData("""{"address":"N7:0","dataType":2}""")] + public void Validate_accepts_every_dataType_the_driver_accepts(string json) + => AbLegacyTagConfigModel.FromJson(json).Validate().ShouldBeNull(); } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/FocasTagConfigModelTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/FocasTagConfigModelTests.cs index c5d2ae6c..20bf2953 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/FocasTagConfigModelTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/FocasTagConfigModelTests.cs @@ -125,4 +125,25 @@ public sealed class FocasTagConfigModelTests json.ShouldContain("\"deviceHostAddress\":\"ab://host\""); } + + /// + /// Gitea #549: the LOAD is lenient — an unrecognised name silently becomes Int32 — but + /// FocasTagDefinitionFactory reads dataType with TryReadEnumStrict and rejects + /// the whole tag, so the save gate must judge the value the blob actually carries. Same shape as + /// the #540 address gap this model already closes, one field over. + /// + [Fact] + public void Validate_refuses_a_dataType_the_driver_would_reject() + => FocasTagConfigModel.FromJson("""{"address":"R100","dataType":"Int322"}""") + .Validate().ShouldNotBeNull().ShouldContain("Int322"); + + /// The other direction, which matters just as much: the check is the runtime's rule and not a + /// stricter one — a valid name in any casing, an absent key, and a present-but-NON-STRING token (which + /// TryReadEnum treats as absent) are all served by the driver and so must all save. + [Theory] + [InlineData("""{"address":"R100","dataType":"float32"}""")] + [InlineData("""{"address":"R100"}""")] + [InlineData("""{"address":"R100","dataType":2}""")] + public void Validate_accepts_every_dataType_the_driver_accepts(string json) + => FocasTagConfigModel.FromJson(json).Validate().ShouldBeNull(); } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/MTConnectTagConfigModelTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/MTConnectTagConfigModelTests.cs index 81aff22e..9105f15e 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/MTConnectTagConfigModelTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/MTConnectTagConfigModelTests.cs @@ -176,4 +176,46 @@ public sealed class MTConnectTagConfigModelTests m.DataType.ShouldBe(DriverDataType.Float64); // 8 == DriverDataType.Float64 — parsed "successfully" m.Validate().ShouldNotBeNull(); } + + /// + /// Gitea #549: a MISSPELLED type name is the same defect the bare number was, and it was the more + /// likely one — TagConfigJson.GetEnum falls back to String on any unrecognised value, + /// so the blob was accepted here, silently rewritten to "String" on the next save, and + /// rejected outright by the driver's strict parse in between. The message must name the value the + /// operator actually wrote, not the String it was coerced to. + /// + [Fact] + public void Validate_rejects_a_misspelled_dataType_name_and_names_the_offending_value() + { + var m = MTConnectTagConfigModel.FromJson("""{"fullName":"dev1_pos","dataType":"Strng"}"""); + + m.DataType.ShouldBe(DriverDataType.String); // the lenient load's coercion — the trap itself + m.Validate().ShouldNotBeNull().ShouldContain("Strng"); + } + + /// The rejection is the RUNTIME's rule, so it must reject exactly what the runtime rejects and + /// no more: a valid name in any casing is accepted, and so is an absent, blank or JSON-null type (the + /// driver then falls back to the Agent's /probe declaration rather than rejecting the tag). + [Theory] + [InlineData("""{"fullName":"dev1_pos","dataType":"float64"}""")] + [InlineData("""{"fullName":"dev1_pos","dataType":"FLOAT64"}""")] + [InlineData("""{"fullName":"dev1_pos","dataType":" Float64 "}""")] + [InlineData("""{"fullName":"dev1_pos"}""")] + [InlineData("""{"fullName":"dev1_pos","dataType":""}""")] + [InlineData("""{"fullName":"dev1_pos","dataType":null}""")] + public void Validate_accepts_every_dataType_the_driver_accepts(string json) + => MTConnectTagConfigModel.FromJson(json).Validate().ShouldBeNull(); + + /// + /// A NON-string dataType token is rejected too, and for a reason the string cases do not + /// cover: the runtime deserialises this key into a string? DTO field, so an unquoted number + /// throws JsonException there and takes the whole tag down. Reading the value with + /// GetString (null for a non-string token) would have called that "absent" and accepted it — + /// which is why the model captures the raw TOKEN. The companion assertion in ControlPlane.Tests' + /// EquipmentTagConfigInspectorTests proves the runtime really does reject it. + /// + [Fact] + public void Validate_rejects_a_dataType_written_as_an_unquoted_number() + => MTConnectTagConfigModel.FromJson("""{"fullName":"dev1_pos","dataType":8}""") + .Validate().ShouldNotBeNull().ShouldContain("8"); } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/ModbusTagConfigModelTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/ModbusTagConfigModelTests.cs index 5fe913c3..353b2611 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/ModbusTagConfigModelTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/ModbusTagConfigModelTests.cs @@ -89,4 +89,29 @@ public sealed class ModbusTagConfigModelTests { new ModbusTagConfigModel().Validate().ShouldBeNull(); } + + /// + /// Gitea #549: the LOAD is lenient — an unrecognised name silently becomes the default — but + /// ModbusTagDefinitionFactory reads all THREE enums with TryReadEnumStrict and + /// rejects the whole tag on any one of them, so the save gate must judge the values the blob + /// actually carries. Each key is asserted separately: a check that covered only dataType + /// would leave a typo'd region or byte order accepted here and dead at the driver. + /// + [Theory] + [InlineData("""{"region":"HoldingRegister","address":1}""", "HoldingRegister")] + [InlineData("""{"region":"HoldingRegisters","address":1,"dataType":"Intt16"}""", "Intt16")] + [InlineData("""{"region":"HoldingRegisters","address":1,"byteOrder":"BigEndain"}""", "BigEndain")] + public void Validate_refuses_an_enum_the_driver_would_reject(string json, string offending) + => ModbusTagConfigModel.FromJson(json).Validate().ShouldNotBeNull().ShouldContain(offending); + + /// The other direction, which matters just as much: the check is the runtime's rule and not a + /// stricter one. Valid names in any casing are accepted, an absent key is accepted (the runtime + /// defaults it), and so is a present-but-NON-STRING token — TryReadEnum calls that "absent" and + /// defaults it too, so refusing it here would block a blob the driver serves. + [Theory] + [InlineData("""{"region":"holdingregisters","address":1,"dataType":"float32","byteOrder":"wordswap"}""")] + [InlineData("""{"address":1}""")] + [InlineData("""{"region":3,"address":1,"dataType":true}""")] + public void Validate_accepts_every_enum_value_the_driver_accepts(string json) + => ModbusTagConfigModel.FromJson(json).Validate().ShouldBeNull(); } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/S7TagConfigModelTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/S7TagConfigModelTests.cs index d56af92f..dbbccfa4 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/S7TagConfigModelTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/S7TagConfigModelTests.cs @@ -79,4 +79,28 @@ public sealed class S7TagConfigModelTests { new S7TagConfigModel { Address = "DB1.DBW0" }.Validate().ShouldBeNull(); } + + /// + /// Gitea #549: the LOAD is lenient — an unrecognised name silently becomes Int16 — but + /// S7TagDefinitionFactory reads dataType with TryReadEnumStrict and rejects the + /// whole tag, so the save gate must judge the value the blob actually carries. The blob reaches + /// deployment unrewritten on the API path, and the operator would see a tag that reads + /// BadNodeIdUnknown with nothing to explain it. + /// + [Fact] + public void Validate_refuses_a_dataType_the_driver_would_reject() + => S7TagConfigModel.FromJson("""{"address":"DB1.DBW0","dataType":"Int166"}""") + .Validate().ShouldNotBeNull().ShouldContain("Int166"); + + /// The other direction, which matters just as much: the check is the runtime's rule and not a + /// stricter one. A valid name in any casing is accepted, and so is a present-but-NON-STRING token — + /// TryReadEnum calls that "absent" and defaults it, so refusing it here would block a blob the + /// driver serves. (An absent key is deliberately not asserted: S7 alone hard-requires the key, an + /// asymmetry that predates this change and that its own Inspect does not report either.) + [Theory] + [InlineData("""{"address":"DB1.DBW0","dataType":"float32"}""")] + [InlineData("""{"address":"DB1.DBW0","dataType":"FLOAT32"}""")] + [InlineData("""{"address":"DB1.DBW0","dataType":7}""")] + public void Validate_accepts_every_dataType_the_driver_accepts(string json) + => S7TagConfigModel.FromJson(json).Validate().ShouldBeNull(); } 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 7dcac8ce..276c5992 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 @@ -73,6 +73,60 @@ public sealed class TagConfigAuthoringDeployParityTests AssertParity(DriverTypeNames.MTConnect, model.Validate(), model.ToJson()); } + /// + /// The six editors whose Validate() gained a strict enum check with Gitea #549. Each is + /// built with its DEFAULT enum values on purpose: the check reads the loaded key bag, so a model + /// that was never loaded from JSON has nothing to check, and the assertion that matters here is + /// that the enum names the editor itself writes are ones the runtime parser reads back — the + /// over-rejection direction the new check must not fall into. + /// + [Fact] + public void Modbus_config_the_editor_accepts_draws_no_deploy_warning() + { + var model = new ModbusTagConfigModel { Address = 1 }; + AssertParity(DriverTypeNames.Modbus, model.Validate(), model.ToJson()); + } + + /// + [Fact] + public void S7_config_the_editor_accepts_draws_no_deploy_warning() + { + var model = new S7TagConfigModel { Address = "DB1.DBW0" }; + AssertParity(DriverTypeNames.S7, model.Validate(), model.ToJson()); + } + + /// + [Fact] + public void AbCip_config_the_editor_accepts_draws_no_deploy_warning() + { + var model = new AbCipTagConfigModel { TagPath = "Program:Main.Speed" }; + AssertParity(DriverTypeNames.AbCip, model.Validate(), model.ToJson()); + } + + /// + [Fact] + public void AbLegacy_config_the_editor_accepts_draws_no_deploy_warning() + { + var model = new AbLegacyTagConfigModel { Address = "N7:0" }; + AssertParity(DriverTypeNames.AbLegacy, model.Validate(), model.ToJson()); + } + + /// + [Fact] + public void TwinCAT_config_the_editor_accepts_draws_no_deploy_warning() + { + var model = new TwinCATTagConfigModel { SymbolPath = "MAIN.bStart" }; + AssertParity(DriverTypeNames.TwinCAT, model.Validate(), model.ToJson()); + } + + /// + [Fact] + public void Focas_config_the_editor_accepts_draws_no_deploy_warning() + { + var model = new FocasTagConfigModel { Address = "R100" }; + AssertParity(DriverTypeNames.FOCAS, model.Validate(), model.ToJson()); + } + [Fact] public void Calculation_config_the_editor_accepts_draws_no_deploy_warning() { @@ -109,15 +163,27 @@ public sealed class TagConfigAuthoringDeployParityTests /// [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. + // Both MTConnect shapes of an unreadable type, and they used to disagree: the digits-only "8" was + // refused at authoring, while a misspelled NAME was silently loaded as String and rewritten on save — + // accepted here, rejected by the driver, which is Gitea #549. MTConnectTagConfigModel.Validate now + // asks MTConnectTagDefinitionFactory.IsAcceptedDataTypeName, the driver's own predicate, so the two + // shapes are one rule and neither gate can drift from the runtime. [InlineData(DriverTypeNames.MTConnect, """{"fullName":"avail","dataType":"8"}""")] + [InlineData(DriverTypeNames.MTConnect, """{"fullName":"avail","dataType":"Strng"}""")] [InlineData(DriverTypeNames.Calculation, """{"changeTriggered":true}""")] [InlineData(DriverTypeNames.OpcUaClient, """{"nodeId":""}""")] + // #549 survey: the same lenient-load / strict-runtime asymmetry existed at every other typed editor + // that reads an enum key, because R2-11 Phase C made every driver's TagConfig enum read strict + // (TryReadEnumStrict REJECTS the tag on a present-but-invalid string) while the editors kept loading + // them with a silent fallback. Each model's Validate() now consults the runtime's own + // TagConfigJson.DescribeInvalidEnumText, so a typo is refused by BOTH gates. + [InlineData(DriverTypeNames.Modbus, """{"region":"HoldingRegisters","address":1,"dataType":"Intt16"}""")] + [InlineData(DriverTypeNames.Modbus, """{"region":"HoldingRegister","address":1}""")] + [InlineData(DriverTypeNames.S7, """{"address":"DB1.DBW0","dataType":"Int166"}""")] + [InlineData(DriverTypeNames.AbCip, """{"tagPath":"Program:Main.Speed","dataType":"DIntt"}""")] + [InlineData(DriverTypeNames.AbLegacy, """{"address":"N7:0","dataType":"Intt"}""")] + [InlineData(DriverTypeNames.TwinCAT, """{"symbolPath":"MAIN.bStart","dataType":"DIntt"}""")] + [InlineData(DriverTypeNames.FOCAS, """{"address":"R100","dataType":"Int322"}""")] public void The_same_driver_types_do_warn_about_a_config_the_editor_refuses(string driverType, string tagConfig) { TagConfigValidator.Validate(driverType, tagConfig).ShouldNotBeNullOrEmpty( diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/TwinCATTagConfigModelTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/TwinCATTagConfigModelTests.cs index 1871c850..ff4186ca 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/TwinCATTagConfigModelTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/TwinCATTagConfigModelTests.cs @@ -99,4 +99,25 @@ public sealed class TwinCATTagConfigModelTests json.ShouldContain("\"deviceHostAddress\":\"ab://host\""); } + + /// + /// Gitea #549: the LOAD is lenient — an unrecognised name silently becomes DInt — but + /// TwinCATTagDefinitionFactory reads dataType with TryReadEnumStrict and + /// rejects the whole tag, so the save gate must judge the value the blob actually carries rather + /// than the one the load substituted. + /// + [Fact] + public void Validate_refuses_a_dataType_the_driver_would_reject() + => TwinCATTagConfigModel.FromJson("""{"symbolPath":"MAIN.bStart","dataType":"DIntt"}""") + .Validate().ShouldNotBeNull().ShouldContain("DIntt"); + + /// The other direction, which matters just as much: the check is the runtime's rule and not a + /// stricter one — a valid name in any casing, an absent key, and a present-but-NON-STRING token (which + /// TryReadEnum treats as absent) are all served by the driver and so must all save. + [Theory] + [InlineData("""{"symbolPath":"MAIN.bStart","dataType":"real"}""")] + [InlineData("""{"symbolPath":"MAIN.bStart"}""")] + [InlineData("""{"symbolPath":"MAIN.bStart","dataType":2}""")] + public void Validate_accepts_every_dataType_the_driver_accepts(string json) + => TwinCATTagConfigModel.FromJson(json).Validate().ShouldBeNull(); } 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 0a9b19da..1f522b97 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/EquipmentTagConfigInspectorTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/EquipmentTagConfigInspectorTests.cs @@ -120,6 +120,18 @@ public sealed class EquipmentTagConfigInspectorTests => EquipmentTagConfigInspector.Inspect("MTConnect", """{"fullName":"avail","dataType":"8"}""") .ShouldContain(w => w.Contains("dataType")); + /// + /// Gitea #549 companion to MTConnectTagConfigModelTests: an UNQUOTED number is rejected by + /// the runtime too, and by a different mechanism than the quoted one — the blob DTO's type field is + /// a string?, so the deserialize itself throws and the whole tag goes. The authoring model + /// therefore has to judge the raw JSON token rather than "the string value, or null", or it would + /// read this as an absent type and accept a blob the driver drops. + /// + [Fact] + public void MTConnect_unquoted_numeric_dataType_is_reported() + => EquipmentTagConfigInspector.Inspect("MTConnect", """{"fullName":"avail","dataType":8}""") + .ShouldNotBeEmpty(); + [Fact] public void MTConnect_config_naming_no_dataitem_is_reported() => EquipmentTagConfigInspector.Inspect("MTConnect", """{"dataType":"String"}""")