fix(adminui): MTConnect editor must read driverDataType at the runtime's precedence and drop the alias on save (#550)

MTConnectTagDefinitionFactory.TryFromTagConfig resolves the coercion type as
FirstNonBlank(blob.DriverDataType, blob.DataType) — driverDataType OUT-RANKS
dataType — while MTConnectTagConfigModel read and wrote dataType alone. That
broke in both directions at once:

  * a blob carrying "driverDataType": "Strng" was accepted at authoring while
    the runtime rejected the tag (the residual slice of #549), and
  * a VALID driverDataType silently overrode whatever type the editor displayed
    and saved, so the editor showed a lie and a save RETYPED the tag.

Both halves land together, mirroring the existing IdKeys pattern:

  * FromJson reads DataTypeKeys = [driverDataType, dataType] in the runtime's
    precedence, with the runtime's own per-key semantics — absent/JSON-null and
    present-but-blank fall through, a non-string token stops the walk (the
    runtime deserialise throws JsonException on it and rejects the whole tag),
    and the raw TOKEN is captured so Validate() judges what the driver judges.
  * ToJson drops every non-canonical DataTypeKeys entry and writes dataType as
    the enum NAME, so the alias can never out-rank the key the editor writes on
    the next load.
  * Validate() now names the offending spelling rather than always saying
    "dataType".

Doing only one half silently retypes existing tags on their next save; the
falsification runs recorded in the PR confirm each half-fix reddens a distinct
test.
This commit is contained in:
Joseph Doherty
2026-08-09 23:44:14 -04:00
parent b0d2fa30e3
commit 8971861b7c
4 changed files with 267 additions and 25 deletions
@@ -218,4 +218,81 @@ public sealed class MTConnectTagConfigModelTests
public void Validate_rejects_a_dataType_written_as_an_unquoted_number()
=> MTConnectTagConfigModel.FromJson("""{"fullName":"dev1_pos","dataType":8}""")
.Validate().ShouldNotBeNull().ShouldContain("8");
// ---------------------------------------------------------------------------------------------
// Gitea #550 — driverDataType is a SECOND spelling of the coercion type, and the runtime prefers it.
// ---------------------------------------------------------------------------------------------
/// <summary>
/// The retype half of #550. <c>driverDataType</c> out-ranks <c>dataType</c> at the driver
/// (<c>FirstNonBlank(blob.DriverDataType, blob.DataType)</c>), so this blob reads as
/// <c>Int32</c> at the runtime while the editor used to show — and save — <c>String</c>. Opening
/// and saving the tag therefore RETYPED it, silently, without the operator touching the control.
/// The load must show what the driver reads, and the save must keep it.
/// </summary>
[Fact]
public void A_driverDataType_out_ranks_dataType_on_load_and_survives_the_save()
{
var m = MTConnectTagConfigModel.FromJson(
"""{"driverDataType":"Int32","dataType":"String","fullName":"x"}""");
m.DataType.ShouldBe(DriverDataType.Int32);
var saved = m.ToJson();
saved.ShouldContain("\"dataType\":\"Int32\"");
saved.ShouldNotContain("driverDataType");
}
/// <summary>
/// The normalisation half. Leaving the alias in the bag would let it out-rank the key this editor
/// writes on the very next load, so every later type change would be silently ignored by the
/// driver — the alias must be DROPPED, exactly as <c>dataItemId</c>/<c>address</c> are.
/// </summary>
[Fact]
public void ToJson_never_emits_the_driverDataType_alias()
{
var m = MTConnectTagConfigModel.FromJson("""{"fullName":"x","driverDataType":"Float64"}""");
m.DataType = DriverDataType.Int16;
var saved = m.ToJson();
saved.ShouldNotContain("driverDataType");
saved.ShouldContain("\"dataType\":\"Int16\"");
// …and a reload of the saved blob agrees, which is what "the two spellings cannot drift" means.
MTConnectTagConfigModel.FromJson(saved).DataType.ShouldBe(DriverDataType.Int16);
}
/// <summary>A present-but-blank alias declares nothing — the runtime's <c>FirstNonBlank</c> falls
/// through to <c>dataType</c>, so this load must too rather than reading the blank as "String".</summary>
[Theory]
[InlineData("""{"fullName":"x","driverDataType":"","dataType":"Int32"}""")]
[InlineData("""{"fullName":"x","driverDataType":" ","dataType":"Int32"}""")]
[InlineData("""{"fullName":"x","driverDataType":null,"dataType":"Int32"}""")]
public void A_blank_driverDataType_falls_through_to_dataType(string json)
{
var m = MTConnectTagConfigModel.FromJson(json);
m.DataType.ShouldBe(DriverDataType.Int32);
m.Validate().ShouldBeNull();
}
/// <summary>
/// The residual #549 slice: a misspelled alias was accepted at authoring because nothing read the
/// alias, while the driver rejected the tag outright. It is non-blank, so it WINS — a valid
/// <c>dataType</c> alongside it does not rescue the blob at the runtime and must not here either.
/// </summary>
[Theory]
[InlineData("""{"driverDataType":"Strng","fullName":"x"}""")]
[InlineData("""{"driverDataType":"Strng","dataType":"Int32","fullName":"x"}""")]
[InlineData("""{"driverDataType":"8","fullName":"x"}""")]
public void Validate_rejects_a_misspelled_driverDataType_even_when_dataType_is_valid(string json)
=> MTConnectTagConfigModel.FromJson(json).Validate().ShouldNotBeNull().ShouldContain("driverDataType");
/// <summary>A NON-string alias token throws <c>JsonException</c> in the runtime's deserialise and takes
/// the whole tag down, so it can never fall through to a valid <c>dataType</c>.</summary>
[Theory]
[InlineData("""{"fullName":"x","driverDataType":8}""")]
[InlineData("""{"fullName":"x","driverDataType":8,"dataType":"Int32"}""")]
public void Validate_rejects_a_driverDataType_written_as_an_unquoted_number(string json)
=> MTConnectTagConfigModel.FromJson(json).Validate().ShouldNotBeNull().ShouldContain("8");
}
@@ -3,6 +3,7 @@ using Xunit;
using ZB.MOM.WW.OtOpcUa.AdminUI.Uns.TagEditors;
using ZB.MOM.WW.OtOpcUa.ControlPlane.AdminOperations;
using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
using ZB.MOM.WW.OtOpcUa.Driver.MTConnect;
namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests.Uns;
@@ -25,6 +26,14 @@ namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests.Uns;
/// lives in AdminUI.Tests because it is the only suite that can see both sides — the models are
/// internal to the AdminUI project, which references ControlPlane.
/// </para>
/// <para>
/// <b>One narrow slice of the reverse direction IS asserted</b>, and the caveat above does not cover
/// it: where the RUNTIME rejects a TagConfig outright — a present-but-invalid enum spelling — the
/// modal must refuse it too, or config authored by CSV import or API reaches a driver that skips the
/// tag with <c>BadNodeIdUnknown</c> (Gitea #549, and its MTConnect <c>driverDataType</c> residual
/// #550). That is the opposite of the editor being merely opinionated: it is the editor being WEAKER
/// than the parser, which is never legitimate.
/// </para>
/// </summary>
public sealed class TagConfigAuthoringDeployParityTests
{
@@ -194,6 +203,80 @@ public sealed class TagConfigAuthoringDeployParityTests
+ "import or API is unchecked — the exact G-7 asymmetry.");
}
// -------------------------------------------------------------------------------------------------
// 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,
// because the defect is about which VALUE the runtime resolves, not merely whether it accepts.
// -------------------------------------------------------------------------------------------------
/// <summary>
/// Authoring accepts ⇔ the runtime accepts, across every mixed shape of the two spellings — and the
/// blob the editor SAVES resolves to the same type the runtime read from the original (no retype).
/// </summary>
[Theory]
// driverDataType alone — the driver's own tags[] spelling, and what a hand-authored blob carries.
[InlineData("""{"fullName":"x","driverDataType":"Int32"}""", DriverDataType.Int32)]
// dataType alone — what this editor writes.
[InlineData("""{"fullName":"x","dataType":"Int32"}""", DriverDataType.Int32)]
// Both, agreeing — the shape a #550-era save produced when the operator happened to pick the same type.
[InlineData("""{"fullName":"x","driverDataType":"Int32","dataType":"Int32"}""", DriverDataType.Int32)]
// Both, DISAGREEING — the alias wins at the driver, so it must win here.
[InlineData("""{"driverDataType":"Int32","dataType":"String","fullName":"x"}""", DriverDataType.Int32)]
// A present-but-blank alias declares nothing and falls through (FirstNonBlank).
[InlineData("""{"fullName":"x","driverDataType":"","dataType":"Float64"}""", DriverDataType.Float64)]
[InlineData("""{"fullName":"x","driverDataType":" ","dataType":"Float64"}""", DriverDataType.Float64)]
[InlineData("""{"fullName":"x","driverDataType":null,"dataType":"Float64"}""", DriverDataType.Float64)]
// Casing and surrounding whitespace are the runtime's business, and it accepts both.
[InlineData("""{"fullName":"x","driverDataType":" float64 "}""", DriverDataType.Float64)]
public void MTConnect_dataType_spellings_the_runtime_accepts_are_authorable_and_never_retyped(
string tagConfig, DriverDataType expected)
{
// The runtime's verdict on the ORIGINAL blob, which is the thing that must be preserved.
MTConnectTagDefinitionFactory.TryFromTagConfig(tagConfig, out _, out var authored).ShouldBeTrue(
"the runtime must accept this blob, or the authoring parity below proves nothing.");
authored.ShouldNotBeNull().DriverDataType.ShouldBe(expected);
// Authoring agrees: it neither blocks the save nor draws a deploy warning.
var model = MTConnectTagConfigModel.FromJson(tagConfig);
model.Validate().ShouldBeNull("the editor refused a blob the runtime serves.");
TagConfigValidator.Validate(DriverTypeNames.MTConnect, tagConfig).ShouldBeNull();
EquipmentTagConfigInspector.Inspect(DriverTypeNames.MTConnect, tagConfig).ShouldBeEmpty();
// …and it DISPLAYS the type the runtime read, not the key the runtime is overriding.
model.DataType.ShouldBe(expected);
// The no-retype invariant: one load→save leaves a blob the runtime resolves identically.
var saved = model.ToJson();
saved.Contains("driverDataType", StringComparison.OrdinalIgnoreCase).ShouldBeFalse(
"the alias must be dropped on save, or it out-ranks the key the editor writes on the next load.");
MTConnectTagDefinitionFactory.TryFromTagConfig(saved, out _, out var afterSave).ShouldBeTrue();
afterSave.ShouldNotBeNull().DriverDataType.ShouldBe(expected);
}
/// <summary>
/// The falsification direction, and the residual slice of #549 that #550 names: every blob here is
/// REJECTED by the runtime, so authoring must refuse it too. A non-blank but invalid alias wins over
/// a perfectly valid <c>dataType</c> — it is not rescued — and a non-string alias token throws in the
/// runtime's deserialise and takes the whole tag down.
/// </summary>
[Theory]
[InlineData("""{"driverDataType":"Strng","fullName":"x"}""")]
[InlineData("""{"driverDataType":"Strng","dataType":"Int32","fullName":"x"}""")]
[InlineData("""{"fullName":"x","driverDataType":"8"}""")]
[InlineData("""{"fullName":"x","driverDataType":8}""")]
[InlineData("""{"fullName":"x","driverDataType":8,"dataType":"Int32"}""")]
// A blank alias falls through — onto a dataType that is itself unreadable.
[InlineData("""{"fullName":"x","driverDataType":"","dataType":"Strng"}""")]
public void MTConnect_dataType_spellings_the_runtime_rejects_are_refused_at_authoring(string tagConfig)
{
MTConnectTagDefinitionFactory.TryFromTagConfig(tagConfig, out _, out _).ShouldBeFalse(
"the runtime must reject this blob, or this control is testing the wrong thing.");
TagConfigValidator.Validate(DriverTypeNames.MTConnect, tagConfig).ShouldNotBeNullOrEmpty(
"the modal would let an operator save a blob the driver skips with BadNodeIdUnknown.");
EquipmentTagConfigInspector.Inspect(DriverTypeNames.MTConnect, tagConfig).ShouldNotBeEmpty();
}
/// <summary>Asserts the editor accepted the config (so the parity claim is not vacuous) and that the
/// deploy gate then found nothing to say about it.</summary>
/// <param name="driverType">The canonical driver-type string both gates dispatch on.</param>