From 64be41d80c76f91c7b1259995c5365a746530a04 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 10 Aug 2026 07:35:12 -0400 Subject: [PATCH] fix(adminui): forward the browsed MTConnect leaf's inferred type into the committed TagConfig (#560) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RawBrowseCommitMapper.MapLeaf typed a browse-committed raw tag's OPC UA node from the leaf's probe-inferred DriverDataType, but its BuildTagConfig MTConnect branch emitted `new MTConnectTagConfigModel { FullName = address }` — and that model's DataType defaults to String with ToJson() unconditionally writing `dataType`. A non-blank authored type wins the driver's coercion precedence, so the Agent-probe inference step was never consulted and every browsed SAMPLE published text into a Double/Int64 node. Silent: the blob is perfectly valid, so neither the authoring gate nor the deploy gate had anything to say. Same defect class as #536, which is why the AbCip / TwinCAT / FOCAS branches forward the browsed native type via NativeType(addressFields, …). MapLeaf now passes driverDataType into BuildTagConfig, whose MTConnect branch uses it as the NativeType fallback — a stated native `dataType` address field still wins, so the branch reads like its siblings; the difference is only that MTConnect's fallback is the leaf's own inference rather than a fixed model default. Its coercion type IS a DriverDataType, the very enum the leaf carries, so nothing is re-derived. NativeType's numeric-string and unparseable-name guards are shared via the extracted ParseNativeType, so an unusable name still keeps String. Tests assert the PAIR (row OPC UA type and blob coercion type agree with the inference) — the blob alone would pass against a mapper that stopped typing the row — plus the parity control that a leaf inferred (or reported) as String still commits `"dataType":"String"`, so the forward cannot be implemented as a deletion. Observed red first: expected "Float64" but was "String". --- .../Uns/RawBrowseCommitMapper.cs | 53 ++++++++- .../Uns/RawBrowseCommitMapperTests.cs | 103 ++++++++++++++++++ 2 files changed, 150 insertions(+), 6 deletions(-) diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/RawBrowseCommitMapper.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/RawBrowseCommitMapper.cs index 97b50e60..dfdaec5f 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/RawBrowseCommitMapper.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/RawBrowseCommitMapper.cs @@ -57,7 +57,7 @@ public static class RawBrowseCommitMapper IReadOnlyDictionary? addressFields = null) { var dataType = MapDataType(driverDataType) ?? defaultDataType; - var tagConfig = BuildTagConfig(driverType, fullName, addressFields); + var tagConfig = BuildTagConfig(driverType, fullName, addressFields, driverDataType); var name = ToRawSegment(browseName); var mirrored = createGroups && folderPath is { Count: > 0 } ? string.Join(RawPaths.Separator, folderPath) @@ -212,16 +212,30 @@ public static class RawBrowseCommitMapper /// (group, edgeNode, device?, metric) tuple — so it is built from the browse session's stated /// instead. See . /// + /// + /// MTConnect binds by the driver-AGNOSTIC type, so it is forwarded from + /// rather than from a stated native field (Gitea #560). Its + /// coercion type IS a — the very enum the browse leaf carries, produced + /// by the driver's own /probe inference — so there is no separate native spelling for the + /// browser to state and nothing to re-derive. Left unforwarded, the model's String default was + /// written as a non-blank authored type, which OUT-RANKS that same inference at the driver: every + /// browsed SAMPLE published text into the numeric node had just typed from the + /// identical inference. Same defect class as #536, one door along. + /// /// /// The owning device's driver type. /// The leaf's driver-side full reference to write into the address field. /// The leaf's structured address, when the driver binds by a tuple — /// see . Ignored by every single-reference driver. + /// The leaf's NAME, for a driver whose tag + /// config carries that agnostic enum rather than one of its own (MTConnect); null when the browser + /// reported no type, which keeps the driver model's default. /// The serialised driver-typed TagConfig JSON. public static string BuildTagConfig( string driverType, string fullName, - IReadOnlyDictionary? addressFields = null) + IReadOnlyDictionary? addressFields = null, + string? driverDataType = null) { var address = fullName ?? ""; if (Is(driverType, DriverTypeNames.Mqtt)) @@ -253,8 +267,17 @@ public static class RawBrowseCommitMapper // MTConnect: the DataItem id. The driver also accepts "address"/"dataItemId", but the typed // editor's canonical spelling is fullName — committing under the generic key below would open // in that editor with an EMPTY id field and blank it on save. + // + // The coercion type is the leaf's own inferred DriverDataType (#560) — see the remarks above. + // A stated native "dataType" address field still wins where a browser supplies one, so this + // branch reads exactly like its AB CIP / TwinCAT / FOCAS siblings; the difference is only that + // MTConnect's fallback is the leaf's inference rather than a fixed model default. if (Is(driverType, DriverTypeNames.MTConnect)) - return new MTConnectTagConfigModel { FullName = address }.ToJson(); + return new MTConnectTagConfigModel + { + FullName = address, + DataType = NativeType(addressFields, ParseNativeType(driverDataType, DriverDataType.String)), + }.ToJson(); if (Is(driverType, DriverTypeNames.Galaxy)) return WriteSingleKey("attributeRef", address); // Sql: a browsed leaf is a COLUMN, and a column name alone cannot address a tag — SqlTagConfigModel @@ -451,9 +474,27 @@ public static class RawBrowseCommitMapper /// private static TEnum NativeType(IReadOnlyDictionary? addressFields, TEnum fallback) where TEnum : struct, Enum - => TryGetField(addressFields, "dataType") is { Length: > 0 } raw - && !char.IsAsciiDigit(raw[0]) && raw[0] is not ('-' or '+') - && Enum.TryParse(raw, ignoreCase: true, out var parsed) + => ParseNativeType(TryGetField(addressFields, "dataType"), fallback); + + /// + /// Parses one stated type NAME into a driver's data-type enum, keeping + /// for anything unusable — the rule applies to a stated address + /// field, split out so a type that arrives by another route (MTConnect's leaf-level + /// , Gitea #560) is judged by exactly the same one. + /// + /// The data-type enum the driver's tag config carries. + /// The stated type name, or null/blank. + /// The default to keep when the name is absent or unusable. + /// The parsed type, or . + /// + /// See for why an unparseable name keeps the default rather than + /// failing the commit, and why a numeric string is refused before parsing at all. + /// + private static TEnum ParseNativeType(string? raw, TEnum fallback) + where TEnum : struct, Enum + => raw?.Trim() is { Length: > 0 } name + && !char.IsAsciiDigit(name[0]) && name[0] is not ('-' or '+') + && Enum.TryParse(name, ignoreCase: true, out var parsed) && Enum.IsDefined(parsed) ? parsed : fallback; diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/RawBrowseCommitMapperTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/RawBrowseCommitMapperTests.cs index 7d858302..bdc5af13 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/RawBrowseCommitMapperTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/RawBrowseCommitMapperTests.cs @@ -248,6 +248,109 @@ public sealed class RawBrowseCommitMapperTests JsonNode.Parse(json)!.AsObject().ContainsKey("dataType").ShouldBeFalse(); } + // ---- MTConnect: the commit must not retype the node it just typed (Gitea #560) ---------------- + + /// + /// A browse-committed MTConnect tag's OPC UA node type and its TagConfig coercion type + /// must BOTH come from the leaf's probe-inferred — the same defect + /// class as #536 arriving through a different door. + /// + /// The pair is asserted together on purpose. The blob alone would pass against a mapper that + /// stopped typing the row, and the row alone was already right while the defect was live: + /// MapLeaf typed the node Double from the inference while BuildTagConfig + /// stamped the model's String default into the blob, and a non-blank authored type wins + /// the driver's coercion precedence — so every browsed SAMPLE published text into a numeric + /// node. Only the DISAGREEMENT is the bug, so only the agreement can pin the fix. + /// + /// + /// The the Agent's /probe inference reported. + /// The OPC UA built-in name the raw tag row must carry. + [Theory] + [InlineData("Float64", "Double")] + [InlineData("Int64", "Int64")] + [InlineData("Boolean", "Boolean")] + [InlineData("String", "String")] // an EVENT item — String is a real inference here, not a default + public void MapLeaf_types_an_MTConnect_node_and_its_coercion_alike(string inferred, string expectedNodeType) + { + var row = RawBrowseCommitMapper.MapLeaf( + driverType: DriverTypeNames.MTConnect, + fullName: "dev1_pos", + browseName: "position", + driverDataType: inferred, + defaultDataType: "Double", + groupPrefix: null, + folderPath: null, + createGroups: false); + + row.Tag.DataType.ShouldBe(expectedNodeType); + JsonNode.Parse(row.Tag.TagConfig)!.AsObject()["dataType"]!.GetValue().ShouldBe(inferred); + + // …and the blob stays one the typed editor and the driver both accept, so the commit is not + // merely self-consistent but authorable. + var model = MTConnectTagConfigModel.FromJson(row.Tag.TagConfig); + model.FullName.ShouldBe("dev1_pos"); + model.DataType.ShouldBe(Enum.Parse(inferred)); + model.Validate().ShouldBeNull(); + } + + /// + /// The inferred type is forwarded through BuildTagConfig itself, not patched on by the + /// caller — the same seam AB CIP / TwinCAT / FOCAS forward their browsed type through. + /// + [Fact] + public void BuildTagConfig_stamps_the_browsed_MTConnect_type() + { + var json = RawBrowseCommitMapper.BuildTagConfig( + DriverTypeNames.MTConnect, "dev1_pos", addressFields: null, driverDataType: "Float64"); + + var o = JsonNode.Parse(json)!.AsObject(); + o["fullName"]!.GetValue().ShouldBe("dev1_pos"); + o["dataType"]!.GetValue().ShouldBe("Float64"); + } + + /// + /// The parity control for the theory above: a leaf the browser reported NO type for keeps the + /// model's own String default — the pre-fix behaviour, and the right answer for a + /// weakly-typed Agent. A "fix" that emitted a blank or absent dataType would satisfy + /// "not String by default" while leaving the blob unauthorable, so this is what stops the + /// forward being implemented as a deletion. + /// + [Fact] + public void MapLeaf_keeps_the_MTConnect_String_default_when_the_browser_states_no_type() + { + var row = RawBrowseCommitMapper.MapLeaf( + driverType: DriverTypeNames.MTConnect, + fullName: "dev1_exec", + browseName: "execution", + driverDataType: null, + defaultDataType: "String", + groupPrefix: null, + folderPath: null, + createGroups: false); + + JsonNode.Parse(row.Tag.TagConfig)!.AsObject()["dataType"]!.GetValue().ShouldBe("String"); + MTConnectTagConfigModel.FromJson(row.Tag.TagConfig).Validate().ShouldBeNull(); + } + + /// + /// A type name does not carry — an older browser, or a driver that + /// stamped something else — keeps the model default rather than failing the commit or writing a + /// name the runtime would reject. Mirrors the sibling drivers' fallback exactly, including the + /// numeric-string guard: Enum.TryParse accepts "8" for any enum and hands back + /// whatever member sits at that ordinal. + /// + /// The unusable type name the browse leaf stated. + [Theory] + [InlineData("NotATypeThisDriverKnows")] + [InlineData("8")] + public void BuildTagConfig_falls_back_to_String_for_an_unusable_MTConnect_type(string stated) + { + var json = RawBrowseCommitMapper.BuildTagConfig( + DriverTypeNames.MTConnect, "dev1_pos", addressFields: null, driverDataType: stated); + + JsonNode.Parse(json)!.AsObject()["dataType"]!.GetValue().ShouldBe("String"); + } + // ---- BuildTagConfig: MQTT (the address is a DESCRIPTOR, not a single reference string) -------- /// The Sparkplug address a browse session states for a metric under a device.