From 115a43dd47010a949d9edd7009e951373c9ab99d Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 24 Jul 2026 18:11:37 -0400 Subject: [PATCH] feat(mtconnect): typed AdminUI tag-config editor + browse round-trip (Task 18 Part A) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Registers the MTConnect typed tag editor in TagConfigEditorMap / TagConfigValidator (keyed off DriverTypeNames.MTConnect, never a literal) and adds the razor shell over Task 17's MTConnectTagConfigModel. The shell stays trivial: the /probe-sourced mtCategory/mtType/mtSubType/units row is rendered disabled + readonly with NO binding of any kind, and no inferred-type hint is shown — MTConnectDataTypeInference.Infer() also keys on the DataItem `representation`, which the tag-config model does not carry, so calling it here would agree with the browse picker for ordinary items and silently disagree for exactly the DATA_SET/TABLE/TIME_SERIES ones. Also fixes the picker -> editor round trip, which was broken in both directions: - RawBrowseCommitMapper.BuildTagConfig had no MTConnect branch, so a committed tag landed as the generic {"address": ""} while the model read only `fullName`. The data plane kept working (the driver accepts all three id spellings), so the only symptom was an empty DataItem-id field in the editor — and saving from that state stamped fullName:"" over the tag. - MTConnectTagConfigModel.FromJson now accepts dataItemId/address as fallbacks (fixing tags committed before this change) and ToJson normalises onto `fullName`, dropping the aliases so the two spellings cannot drift. --- .../TagEditors/MTConnectTagConfigEditor.razor | 118 ++++++++++++++++++ .../Uns/RawBrowseCommitMapper.cs | 5 + .../Uns/TagEditors/MTConnectTagConfigModel.cs | 37 +++++- .../Uns/TagEditors/TagConfigEditorMap.cs | 1 + .../Uns/TagEditors/TagConfigValidator.cs | 1 + .../MTConnectBrowseCommitRoundTripTests.cs | 102 +++++++++++++++ .../Uns/TagConfigDriverTypeNameGuardTests.cs | 2 + 7 files changed, 264 insertions(+), 2 deletions(-) create mode 100644 src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Uns/TagEditors/MTConnectTagConfigEditor.razor create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/MTConnectBrowseCommitRoundTripTests.cs diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Uns/TagEditors/MTConnectTagConfigEditor.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Uns/TagEditors/MTConnectTagConfigEditor.razor new file mode 100644 index 00000000..7a71f951 --- /dev/null +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Uns/TagEditors/MTConnectTagConfigEditor.razor @@ -0,0 +1,118 @@ +@* Typed tag-config editor for an MTConnect Agent tag. A thin shell over MTConnectTagConfigModel — every + rule (defaults, unknown-key preservation, the numeric-dataType trap) lives in that model, not here. + + MTConnect is READ-ONLY (the driver is deliberately not IWritable), so unlike Modbus there is no + "Writable" toggle to author. + + The mtCategory/mtType/mtSubType/units row is /probe-sourced device-model metadata: rendered disabled + + readonly with NO binding of any kind, so nothing an operator can do in this UI writes to it. The model + still round-trips the values so a browse-committed stamp survives an edit of the data type. + + NOTE: no inferred-type hint is shown. MTConnectDataTypeInference.Infer() also keys on the DataItem's + `representation` (DATA_SET/TABLE demote to String even on a SAMPLE; TIME_SERIES is honoured only on a + SAMPLE), and the tag-config model does not carry `representation`. Calling Infer() from here would + therefore agree with the browse picker for ordinary items and silently DISAGREE for exactly the + representation-driven ones — the failure that never shows up in the obvious test. The picker already + stamped the inferred type into `dataType` on commit; this editor is the override surface for it. *@ +@using ZB.MOM.WW.OtOpcUa.AdminUI.Uns.TagEditors +@using ZB.MOM.WW.OtOpcUa.Core.Abstractions + +
+
+ + +
The Agent DataItem's id attribute — the driver's read/subscribe key.
+
+
+ + +
Overrides the type the browse picker inferred. MTConnect is text on the wire — a wrong numeric type reads as Bad quality.
+
+
+ +@* Probe-sourced device-model metadata — display only. *@ +
+
+ + +
+
+ + +
+
+ + +
+
+ + +
+
+
From the Agent's /probe device model — not editable here. Re-pick the tag in the browser to refresh it.
+
+
+ +@* Author's own notes — not consumed by the driver or the runtime. *@ +
+
+ + +
+
+ + +
+
+ +@if (_m.Validate() is { } error) +{ +
@error
+} + +@code { + /// The tag's raw TagConfig JSON (supports @bind-ConfigJson). + [Parameter] public string? ConfigJson { get; set; } + /// Two-way config callback. + [Parameter] public EventCallback ConfigJsonChanged { get; set; } + /// DriverType of the selected driver — supplied by the hosting modal for browse-capable pickers. + [Parameter] public string DriverType { get; set; } = ""; + /// Live accessor for the selected driver's DriverConfig JSON (browse connect config). + [Parameter] public Func GetDriverConfigJson { get; set; } = () => "{}"; + + private MTConnectTagConfigModel _m = new(); + private string? _lastConfigJson; + + // Re-parse only when the incoming JSON actually changes, so an unrelated parent re-render + // (Blazor Server live-status pushes do this) can't reset the user's in-progress edits. + protected override void OnParametersSet() + { + if (ConfigJson == _lastConfigJson) { return; } + _lastConfigJson = ConfigJson; + _m = MTConnectTagConfigModel.FromJson(ConfigJson); + } + + // Placeholder for absent probe metadata: an empty read-only box reads as "the field is broken". + private static string Display(string? value) => string.IsNullOrWhiteSpace(value) ? "—" : value; + + // TryParse so a bad/empty change value can never throw into the Blazor circuit — it falls back. + private static TEnum ParseEnum(object? v, TEnum fallback) where TEnum : struct, Enum + => Enum.TryParse(v?.ToString(), out var r) ? r : fallback; + + private async Task Update(Action apply) + { + apply(); + await ConfigJsonChanged.InvokeAsync(_m.ToJson()); + } +} 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 47ef905d..49c48764 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/RawBrowseCommitMapper.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/RawBrowseCommitMapper.cs @@ -132,6 +132,11 @@ public static class RawBrowseCommitMapper return new FocasTagConfigModel { Address = address }.ToJson(); if (Is(driverType, DriverTypeNames.S7)) return new S7TagConfigModel { Address = address }.ToJson(); + // 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. + if (Is(driverType, DriverTypeNames.MTConnect)) + return new MTConnectTagConfigModel { FullName = address }.ToJson(); if (Is(driverType, DriverTypeNames.Galaxy)) return WriteSingleKey("attributeRef", address); 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 20bb263a..6f6d8096 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 @@ -27,7 +27,9 @@ namespace ZB.MOM.WW.OtOpcUa.AdminUI.Uns.TagEditors; /// public sealed class MTConnectTagConfigModel { - /// The MTConnect DataItem's id attribute — the driver's read/subscribe key. Required. + /// The MTConnect DataItem's id attribute — the driver's read/subscribe key. Required. + /// Loaded from fullName, or from the driver's alternate spellings dataItemId / + /// address (see ); always saved as fullName. public string FullName { get; set; } = ""; /// Logical data type override for the DataItem's value. Defaults to @@ -65,6 +67,18 @@ public sealed class MTConnectTagConfigModel private JsonObject _bag = new(); + /// + /// The accepted spellings of the DataItem id, in precedence order, mirroring the driver's own + /// MTConnectRawTagConfigDto: fullName (this model's canonical key), + /// dataItemId (what an operator hand-authoring the blob reaches for), and address + /// (what RawBrowseCommitMapper wrote for MTConnect before it grew a typed-editor branch). + /// Reading only fullName made every already-committed tag open with an EMPTY id field and + /// get blanked on save, while the data plane kept working — the symptom looked cosmetic. + /// normalises onto fullName and drops the aliases, so the two + /// spellings can never drift apart on a later edit. + /// + private static readonly string[] IdKeys = ["fullName", "dataItemId", "address"]; + /// Loads a model from a TagConfig JSON string, defaulting any absent field and retaining /// every original key (so fields this editor doesn't expose survive a load→save). /// The raw TagConfig JSON string, or null for a new/empty config. @@ -74,7 +88,7 @@ public sealed class MTConnectTagConfigModel var o = TagConfigJson.ParseOrNew(json); return new MTConnectTagConfigModel { - FullName = TagConfigJson.GetString(o, "fullName") ?? "", + FullName = ReadId(o), DataType = TagConfigJson.GetEnum(o, "dataType", DriverDataType.String), MtCategory = TagConfigJson.GetString(o, "mtCategory"), MtType = TagConfigJson.GetString(o, "mtType"), @@ -94,6 +108,10 @@ public sealed class MTConnectTagConfigModel /// The serialised TagConfig JSON string. public string ToJson() { + // Normalise onto the canonical key: a blob loaded via an alias must not keep the alias, or the + // two spellings drift on the next edit and the driver silently reads the stale one. + TagConfigJson.Set(_bag, "dataItemId", null); + TagConfigJson.Set(_bag, "address", null); TagConfigJson.Set(_bag, "fullName", FullName.Trim()); TagConfigJson.Set(_bag, "dataType", DataType); TagConfigJson.Set(_bag, "mtCategory", Blank(MtCategory)); @@ -127,4 +145,19 @@ public sealed class MTConnectTagConfigModel // 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; + + // First non-blank id spelling wins (see IdKeys). A present-but-blank "fullName" must not shadow a + // populated alias — that is exactly the state a save from the broken editor left behind. + private static string ReadId(JsonObject o) + { + foreach (var key in IdKeys) + { + if (Blank(TagConfigJson.GetString(o, key)) is { } id) + { + return id; + } + } + + return ""; + } } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/TagConfigEditorMap.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/TagConfigEditorMap.cs index 98a81ba2..11adc332 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/TagConfigEditorMap.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/TagConfigEditorMap.cs @@ -27,6 +27,7 @@ public static class TagConfigEditorMap // asserts bidirectional parity). Task 11 repoints all Sql keys onto DriverTypeNames.Sql. [SqlDriver.DriverTypeName] = typeof(Components.Shared.Uns.TagEditors.SqlTagConfigEditor), [DriverTypeNames.Mqtt] = typeof(Components.Shared.Uns.TagEditors.MqttTagConfigEditor), + [DriverTypeNames.MTConnect] = typeof(Components.Shared.Uns.TagEditors.MTConnectTagConfigEditor), }; /// Returns the editor component type for a driver type, or null if none is registered. diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/TagConfigValidator.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/TagConfigValidator.cs index bd56b19d..863f1552 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/TagConfigValidator.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/TagConfigValidator.cs @@ -27,6 +27,7 @@ public static class TagConfigValidator // Keyed off SqlDriver.DriverTypeName (= "Sql") — see the note in TagConfigEditorMap. [SqlDriver.DriverTypeName] = j => SqlTagConfigModel.FromJson(j).Validate(), [DriverTypeNames.Mqtt] = j => MqttTagConfigModel.FromJson(j).Validate(), + [DriverTypeNames.MTConnect] = j => MTConnectTagConfigModel.FromJson(j).Validate(), }; /// diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/MTConnectBrowseCommitRoundTripTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/MTConnectBrowseCommitRoundTripTests.cs new file mode 100644 index 00000000..2281fa3e --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/MTConnectBrowseCommitRoundTripTests.cs @@ -0,0 +1,102 @@ +using System.Text.Json.Nodes; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.AdminUI.Uns; +using ZB.MOM.WW.OtOpcUa.AdminUI.Uns.TagEditors; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests.Uns; + +/// +/// Pins the browse-picker → typed-editor round trip for MTConnect, the driver's primary +/// authoring path (a tag is committed from the universal browser, then confirmed/overridden in the +/// typed editor). +/// +/// +/// +/// The defect this guards: had no MTConnect +/// branch, so a committed tag landed as the generic {"address": "<dataItemId>"} +/// while read only fullName. The data plane kept +/// working (the driver accepts all three id spellings), so the ONLY symptom was an empty +/// DataItem-id field in the editor — and saving from that state stamped fullName: "" +/// over the tag. Razor is exactly what this repo cannot unit-test, so both halves of the fix +/// are pinned here in plain C#. +/// +/// +/// Both halves are asserted: the mapper now writes the canonical fullName (fixes new +/// commits), and the model accepts dataItemId/address as fallbacks (fixes tags +/// already committed before the fix). +/// +/// +[Trait("Category", "Unit")] +public sealed class MTConnectBrowseCommitRoundTripTests +{ + [Fact] + public void BuildTagConfig_writes_the_canonical_fullName_key_for_MTConnect() + { + var json = RawBrowseCommitMapper.BuildTagConfig(DriverTypeNames.MTConnect, "avail"); + + var o = JsonNode.Parse(json)!.AsObject(); + o["fullName"]!.GetValue().ShouldBe("avail"); + o.ContainsKey("address").ShouldBeFalse( + "the generic fallback key must not be used now that MTConnect has a typed editor"); + } + + [Fact] + public void A_browse_committed_tag_round_trips_through_the_typed_editor_model() + { + var committed = RawBrowseCommitMapper.BuildTagConfig(DriverTypeNames.MTConnect, "Xact"); + + var model = MTConnectTagConfigModel.FromJson(committed); + + model.FullName.ShouldBe("Xact"); + model.Validate().ShouldBeNull("a browse-committed tag must open in the editor already valid"); + MTConnectTagConfigModel.FromJson(model.ToJson()).FullName.ShouldBe("Xact"); + } + + // ---- legacy blobs committed before the mapper fix ------------------------------------------- + + [Theory] + [InlineData("address")] + [InlineData("dataItemId")] + public void FromJson_accepts_the_drivers_alternate_id_spellings(string key) + { + var model = MTConnectTagConfigModel.FromJson($$"""{"{{key}}":"Ypos"}"""); + + model.FullName.ShouldBe("Ypos"); + model.Validate().ShouldBeNull(); + } + + [Fact] + public void FullName_wins_over_dataItemId_which_wins_over_address() + { + MTConnectTagConfigModel + .FromJson("""{"fullName":"a","dataItemId":"b","address":"c"}""") + .FullName.ShouldBe("a"); + + MTConnectTagConfigModel + .FromJson("""{"dataItemId":"b","address":"c"}""") + .FullName.ShouldBe("b"); + } + + [Fact] + public void A_blank_alias_does_not_shadow_a_populated_one() + => MTConnectTagConfigModel + .FromJson("""{"fullName":" ","address":"c"}""") + .FullName.ShouldBe("c"); + + [Fact] + public void Saving_a_legacy_blob_normalises_it_onto_fullName_and_drops_the_aliases() + { + var model = MTConnectTagConfigModel.FromJson("""{"address":"Zpos","units":"MILLIMETER"}"""); + + var o = JsonNode.Parse(model.ToJson())!.AsObject(); + + o["fullName"]!.GetValue().ShouldBe("Zpos"); + o.ContainsKey("address").ShouldBeFalse( + "leaving a second id spelling behind lets the two drift apart on the next edit"); + o.ContainsKey("dataItemId").ShouldBeFalse(); + // Unrelated keys are still preserved — normalisation is not a purge. + o["units"]!.GetValue().ShouldBe("MILLIMETER"); + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/TagConfigDriverTypeNameGuardTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/TagConfigDriverTypeNameGuardTests.cs index 60c9c318..cc581114 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/TagConfigDriverTypeNameGuardTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/TagConfigDriverTypeNameGuardTests.cs @@ -27,6 +27,7 @@ public sealed class TagConfigDriverTypeNameGuardTests DriverTypeNames.FOCAS, DriverTypeNames.OpcUaClient, DriverTypeNames.Calculation, + DriverTypeNames.MTConnect, ]; [Theory] @@ -46,6 +47,7 @@ public sealed class TagConfigDriverTypeNameGuardTests DriverTypeNames.TwinCAT, DriverTypeNames.FOCAS, DriverTypeNames.OpcUaClient, + DriverTypeNames.MTConnect, ]; [Theory]