From cf2af8a5a2c78e098755391f382bdcb5d8474053 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 10 Aug 2026 01:18:55 -0400 Subject: [PATCH] fix(csv): stop the typed-column merge laundering the TagConfigJson cell (#553) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ModelBackedCsvDriverTagMap.ApplyTypedColumns round-tripped the row's free-form TagConfigJson cell through a driver model's FromJson/ToJson to fold the typed columns in. That load is deliberately lenient — it has to be, so a poisoned blob can be opened and repaired — so a typo'd value under a key the model owns ("dataType":"Intt16") came back out as the model's DEFAULT, before the #551 import gate or any later gate could judge it. The row imported clean and the tag the operator described was gone. Only keys the model did not own survived to be judged, which is why RawTagConfigGateTests looked like it covered this. MTConnect carried the same defect from the export end. It is the one driver whose blob accepts several spellings per field, and the runtime PREFERS driverDataType over the dataType key the MtDataType column writes. The export residual carried that alias (the map owned only the canonical key), so a re-imported row let it out-rank the very column the operator had just edited — accepted, then silently ignored at the driver — while the exported cell printed the spelling the driver was ignoring, or nothing at all. The fix preserves evidence; it does not move the verdict: - ApplyTypedColumns writes the base cell's own tokens back over the model's serialisation for every key no non-blank typed column overrode. The model still stamps defaults for keys the row declares nowhere, so fragment cells compose exactly as before. - CsvTypedColumn gains ReadKeys / OwnedJsonKeys: the full ordered set of spellings the RUNTIME reads for that column, mirroring TryFromTagConfig's FirstNonBlank pairs. A firing column clears the out-ranking aliases; the export residual drops them; the export cell shows the EFFECTIVE value. - The review grid's override provenance reads the whole owned set, so an overridden driverDataType is announced as an overridden dataType would be. Nothing here decides which values are legal. The verdict stays with TagConfigValidator on the assembled blob and with IRawTreeService.ImportTagsAsync behind it — the deliberate reason NOT to follow the issue's own "run the validator over the pre-merge cell" direction, which would have imposed completeness on a cell that is legitimately a fragment and false-rejected every typed-column row. Semantics chosen for the override contradiction: a cell token that a non-blank column overrides is ACCEPTED with the column winning, and reported by name in TypedOverrodeFallback. Typed-column-wins is the map's documented contract, the override is announced rather than silent, and erroring would remove the only way to repair a poisoned tag through the CSV surface. Every token that reaches the stored blob is judged; a token the operator overrode is discarded openly. Falsification: neutering the restore loop + alias handling turns exactly 7 of the 15 new tests red, while RawTagConfigGateTests (#551), RawTagCsvRoundTripTests and CsvColumnMapReflectionTests all stay green — layering, not replacement. AdminUI 1248, MTConnect 534, Modbus 373, S7 285, TwinCAT 205 — all green. --- .../Uns/RawTagCsvMapper.cs | 17 +- .../Uns/TagEditors/CsvColumnMap.cs | 136 ++++++- .../Uns/CsvMergeEvidenceTests.cs | 355 ++++++++++++++++++ 3 files changed, 499 insertions(+), 9 deletions(-) create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/CsvMergeEvidenceTests.cs diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/RawTagCsvMapper.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/RawTagCsvMapper.cs index 72b7e9d7..95059ebb 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/RawTagCsvMapper.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/RawTagCsvMapper.cs @@ -27,6 +27,15 @@ namespace ZB.MOM.WW.OtOpcUa.AdminUI.Uns; /// it is never silently skipped. This check is for the operator's benefit (it names the row before /// commit); the guarantee lives in , which re-runs it. /// +/// +/// For that gate to mean anything the merge must hand it the row as written, which it did not +/// (Gitea #553): the model-backed maps round-tripped the free-form cell through a driver model's lenient +/// FromJson/ToJson, so a typo'd value under a key the model owns was rewritten to that model's +/// default before any gate ran — the row passed, and the tag the operator actually described was gone. +/// now preserves the cell's own tokens for every key no +/// non-blank typed column overrode, and clears a column's out-ranking aliases when one does. The verdict on +/// those tokens is still the gate's alone — nothing about which values are legal is restated here. +/// /// public static class RawTagCsvMapper { @@ -162,7 +171,13 @@ public static class RawTagCsvMapper if (row.TryGetValue(col.Header, out var cv) && !string.IsNullOrWhiteSpace(cv)) { typedUsed.Add(col.Header); - if (baseKeys.Contains(col.JsonKey)) + + // Every spelling the column speaks for, not just the one it writes: an MTConnect cell + // carrying "driverDataType" is overridden by the MtDataType column exactly as a + // "dataType" cell is, and the operator must see that in the grid (Gitea #553). This + // report is also what makes accepting the override — rather than erroring on the + // contradiction — an open decision instead of a silent one. + if (col.OwnedJsonKeys.Any(baseKeys.Contains)) { overrodeFallback.Add(col.Header); } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/CsvColumnMap.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/CsvColumnMap.cs index 74ba100f..6df020d3 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/CsvColumnMap.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/CsvColumnMap.cs @@ -19,9 +19,35 @@ namespace ZB.MOM.WW.OtOpcUa.AdminUI.Uns.TagEditors; /// a header directly onto a JSON key. /// /// The CSV column header (PascalCase, e.g. ModbusDataType). -/// The root TagConfig JSON key this column reads/writes (camelCase, e.g. dataType). +/// The root TagConfig JSON key this column writes (camelCase, e.g. dataType). /// The backing model property name, or null for a model-less map. -public sealed record CsvTypedColumn(string Header, string JsonKey, string? ModelProperty); +/// +/// The full ordered set of root JSON keys the runtime reads for this column's value, most-preferred +/// first, or null when the column owns only . Supplied for the drivers +/// whose blob accepts more than one spelling of a field (MTConnect's driverDataType/dataType +/// and fullName/dataItemId/address) — see for why the CSV +/// path has to know the whole set and not just the one it writes. +/// +public sealed record CsvTypedColumn( + string Header, + string JsonKey, + string? ModelProperty, + IReadOnlyList? ReadKeys = null) +{ + /// + /// Every root JSON key this column speaks for, in the runtime's read precedence — i.e. + /// when supplied, else just . + /// + /// A column that writes dataType while the driver prefers a driverDataType alias does + /// not actually control the tag's type: the alias, carried through in the free-form + /// TagConfigJson cell, out-ranks it at the driver, so an operator's edit of the typed column + /// is accepted and then silently ignored (Gitea #553). Knowing the whole set is what lets the + /// export residual drop the alias, the export cell show the effective value, and a firing + /// column clear the alias so it genuinely wins. + /// + /// + public IReadOnlyList OwnedJsonKeys { get; } = ReadKeys is { Count: > 0 } ? ReadKeys : [JsonKey]; +} /// /// Per-driver mapping between a CSV row's typed columns and a tag's driver-specific @@ -46,8 +72,50 @@ public abstract class CsvDriverTagMap /// The typed column headers, in order. public IReadOnlyList Headers => Columns.Select(c => c.Header).ToArray(); - /// The root TagConfig JSON keys this map owns (used to compute the export residual). - public IReadOnlyList JsonKeys => Columns.Select(c => c.JsonKey).ToArray(); + /// The root TagConfig JSON keys this map owns (used to compute the export residual) — every + /// entry, aliases included, so the residual can never carry a + /// spelling that out-ranks the typed column beside it (Gitea #553). + public IReadOnlyList JsonKeys => Columns.SelectMany(c => c.OwnedJsonKeys).Distinct(StringComparer.Ordinal).ToArray(); + + /// + /// Applies one typed column's already-parsed effect to : removes every + /// other spelling the runtime would read for that column, so the value the column supplied is + /// the one the driver actually sees. Callers write the column's own + /// themselves (model-backed maps do it through the model). Gitea #553. + /// + /// The merged TagConfig object being assembled. + /// The typed column that supplied a value on this row. + protected static void ClearOutrankingAliases(JsonObject target, CsvTypedColumn column) + { + foreach (var key in column.OwnedJsonKeys) + { + if (!string.Equals(key, column.JsonKey, StringComparison.Ordinal)) + { + target.Remove(key); + } + } + } + + /// + /// Reads one typed column's effective cell value out of a stored TagConfig for export: the + /// first non-blank string under , in the runtime's own + /// precedence. Exporting the canonical key alone would print the spelling the driver is ignoring + /// and leave the winning alias in the residual, which is how the re-import trap in Gitea #553 got set. + /// + /// The stored TagConfig object. + /// The typed column to project. + /// The effective raw node, or null when no spelling carries a value. + protected static JsonNode? ReadEffective(JsonObject source, CsvTypedColumn column) + { + foreach (var key in column.OwnedJsonKeys) + { + if (!source.TryGetPropertyValue(key, out var node) || node is null) { continue; } + if (node is JsonValue sv && sv.TryGetValue(out var s) && string.IsNullOrWhiteSpace(s)) { continue; } + return node; + } + + return null; + } /// /// Folds the typed columns present (non-blank) in onto @@ -122,18 +190,58 @@ public sealed class ModelBackedCsvDriverTagMap : CsvDriverTagMap public override IReadOnlyList Columns { get; } /// + /// + /// The base cell's own tokens are restored verbatim over the model's serialisation (Gitea #553). + /// The model round-trip is what merges the typed columns and stamps a default for a key the row + /// declares nowhere — but FromJson is deliberately lenient (it has to LOAD a poisoned blob so an + /// operator can fix it), so a "dataType":"Intt16" the row carried in its free-form + /// TagConfigJson cell came back out of ToJson as the model's default — the + /// operator's typo replaced by a valid-looking value before the import gate, or any later gate, could + /// ever see it. Only keys the model does not own survived to be judged. + /// + /// So every key the base cell actually carried is written back afterwards, unless a non-blank + /// typed column overrode it. Nothing here decides whether a token is good: that stays with + /// on the assembled blob (and with + /// IRawTreeService.ImportTagsAsync behind it), which asks each driver model's own + /// Validate() — one authority. This method's only job is to stop destroying the evidence + /// before that authority is consulted. + /// + /// + /// A key a non-blank column DID override is not restored, and its out-ranking aliases are + /// cleared: the typed column winning over a conflicting cell key is this map's documented contract, + /// the review grid reports each such override by name, and refusing the row instead would leave an + /// operator no way to repair a poisoned cell through the very columns that exist for it. + /// + /// public override string ApplyTypedColumns(string? baseJson, IReadOnlyDictionary row) { + var baseObj = TagConfigJson.ParseOrNew(baseJson); var model = _fromJson(baseJson); + var overridden = new HashSet(StringComparer.Ordinal); + var fired = new List(); + foreach (var col in _columns) { if (row.TryGetValue(col.Meta.Header, out var raw) && !string.IsNullOrWhiteSpace(raw)) { col.Set(model, raw.Trim()); + fired.Add(col.Meta); + foreach (var key in col.Meta.OwnedJsonKeys) { overridden.Add(key); } } } - return _toJson(model); + var merged = TagConfigJson.ParseOrNew(_toJson(model)); + foreach (var (key, node) in baseObj) + { + if (!overridden.Contains(key)) + { + merged[key] = node?.DeepClone(); + } + } + + foreach (var col in fired) { ClearOutrankingAliases(merged, col); } + + return TagConfigJson.Serialize(merged); } /// @@ -214,6 +322,10 @@ public sealed class RawKeyCsvDriverTagMap : CsvDriverTagMap CsvRawKeyKind.Int => JsonValue.Create(CsvValues.ParseInt(col.Meta.Header, trimmed)), _ => JsonValue.Create(trimmed), }; + + // Without this the cell's alias survives beside the key just written and OUT-RANKS it at the + // driver, so the operator's column edit is accepted and then ignored (Gitea #553). + ClearOutrankingAliases(o, col.Meta); } } @@ -227,7 +339,7 @@ public sealed class RawKeyCsvDriverTagMap : CsvDriverTagMap var result = new Dictionary(StringComparer.Ordinal); foreach (var col in _columns) { - if (o.TryGetPropertyValue(col.Meta.JsonKey, out var node) && node is JsonValue v) + if (ReadEffective(o, col.Meta) is JsonValue v) { result[col.Meta.Header] = col.Kind switch { @@ -480,10 +592,18 @@ public static class CsvColumnMap new CsvRawKeyColumn(new CsvTypedColumn("MqttDeviceId", "deviceId", null), CsvRawKeyKind.String), }), + // MTConnect is the one driver whose blob accepts several spellings per field, so its two identity + // columns declare the runtime's whole read precedence rather than just the key they write — + // mirroring MTConnectTagDefinitionFactory.TryFromTagConfig's FirstNonBlank(...) pairs, and the + // MTConnectTagConfigModel load that already mirrors them for the editor (Gitea #550/#553). new RawKeyCsvDriverTagMap(DriverTypeNames.MTConnect, new[] { - new CsvRawKeyColumn(new CsvTypedColumn("DataItemId", "fullName", null), CsvRawKeyKind.String), - new CsvRawKeyColumn(new CsvTypedColumn("MtDataType", "dataType", null), CsvRawKeyKind.String), + new CsvRawKeyColumn( + new CsvTypedColumn("DataItemId", "fullName", null, ["fullName", "dataItemId", "address"]), + CsvRawKeyKind.String), + new CsvRawKeyColumn( + new CsvTypedColumn("MtDataType", "dataType", null, ["driverDataType", "dataType"]), + CsvRawKeyKind.String), new CsvRawKeyColumn(new CsvTypedColumn("MtCategory", "mtCategory", null), CsvRawKeyKind.String), new CsvRawKeyColumn(new CsvTypedColumn("MtType", "mtType", null), CsvRawKeyKind.String), new CsvRawKeyColumn(new CsvTypedColumn("MtSubType", "mtSubType", null), CsvRawKeyKind.String), diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/CsvMergeEvidenceTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/CsvMergeEvidenceTests.cs new file mode 100644 index 00000000..337c6a34 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/CsvMergeEvidenceTests.cs @@ -0,0 +1,355 @@ +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.Configuration.Enums; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Driver.MTConnect; +using EditorTagConfigJson = ZB.MOM.WW.OtOpcUa.AdminUI.Uns.TagEditors.TagConfigJson; + +namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests.Uns; + +/// +/// Gitea #553 — the CSV typed-column merge must hand the import gate the row as the operator wrote it. +/// +/// folded the free-form TagConfigJson cell through a +/// driver model's FromJson/ToJson, and that load is deliberately lenient — it must be, so a +/// poisoned blob can be opened and repaired. The consequence on this path was that a typo'd value under a key +/// the model owns ("dataType":"Intt16") came back out as the model's default, before the Gitea +/// #551 import gate — or any later gate — could judge it. The row passed; the tag the operator described was +/// gone. Only keys the model did not own survived to be judged, which is exactly why the sibling test +/// RawTagConfigGateTests looked like it covered this and did not. +/// +/// +/// MTConnect carried the same defect from the other end. It is the one driver whose blob accepts several +/// spellings per field, and the runtime prefers the driverDataType alias over the dataType key +/// the MtDataType column writes. The export residual carried that alias (the map did not own it), so a +/// re-imported row let it out-rank the very column the operator had just edited — the change accepted, then +/// silently ignored at the driver. +/// +/// +/// What is asserted here is evidence preservation, never a rule about which values are legal. Every +/// verdict below is 's, reached through each driver model's own +/// Validate(); this suite only proves the merge stops destroying the token before that authority is +/// consulted, and that behind it still reaches the same verdict +/// on the blob that would have been committed. Falsification is documented on +/// . +/// +/// +[Trait("Category", "Unit")] +public sealed class CsvMergeEvidenceTests +{ + private static string Csv(params string[] lines) => string.Join("\r\n", lines); + + private static RawCsvReviewRow SingleRow(string driverType, params string[] lines) + { + var result = RawTagCsvMapper.Parse(driverType, Csv(lines)); + result.FatalError.ShouldBeNull(); + return result.Rows.ShouldHaveSingleItem(); + } + + private static JsonObject Blob(RawCsvReviewRow row) + { + row.Error.ShouldBeNull(); + row.ImportRow.ShouldNotBeNull(); + return EditorTagConfigJson.ParseOrNew(row.ImportRow!.Tag.TagConfig); + } + + // ------------------------------------------------------------------ R1: the laundering is closed + + // The defect proper, across the three shapes a driver's dataType verdict can take: Modbus and TwinCAT + // judge the ORIGINAL key bag through TagConfigJson.DescribeInvalidEnum (the runtime's strict-enum read); + // S7 does the same and additionally rejects an ABSENT key; MTConnect asks the driver's own + // IsAcceptedDataTypeName predicate. All three used to launder identically because the laundering happened + // upstream of every one of them — in the merge, not in the verdict. + [Theory] + [InlineData(DriverTypeNames.Modbus, "Name,DataType,TagConfigJson", "t1,Int16,")] + [InlineData(DriverTypeNames.S7, "Name,DataType,Address,TagConfigJson", "t1,Int16,DB1.DBW0,")] + [InlineData(DriverTypeNames.TwinCAT, "Name,DataType,SymbolPath,TagConfigJson", "t1,Int16,MAIN.x,")] + public void A_typod_value_in_the_cell_that_no_typed_column_overrides_is_an_error_not_a_rewrite( + string driverType, string header, string rowPrefix) + { + var row = SingleRow(driverType, header, rowPrefix + "\"{\"\"dataType\"\":\"\"Intt16\"\"}\""); + + // An ERROR, never a skip and never a silent substitution (Gitea #553 / #551 item 1). + row.Error.ShouldNotBeNull(); + row.Error.ShouldContain("Intt16", Case.Sensitive, + "the operator's own token must appear in the message — a default substituted for it is the defect"); + row.Error.ShouldContain("dataType"); + row.ImportRow.ShouldBeNull(); + } + + [Fact] + public void The_mtconnect_cell_typo_is_an_error_too_and_names_the_spelling_that_carried_it() + { + var row = SingleRow( + DriverTypeNames.MTConnect, + "Name,DataType,DataItemId,MtDataType,TagConfigJson", + "t1,Double,di1,,\"{\"\"dataType\"\":\"\"Intt16\"\"}\""); + + row.Error.ShouldNotBeNull(); + row.Error.ShouldContain("Intt16"); + row.Error.ShouldContain("dataType"); + } + + // The merge is not allowed to invent a verdict of its own: the blob it produces for a poisoned row must be + // one the driver model's OWN validator rejects, reached through the same TagConfigValidator the service + // gate calls. This is what makes the error above a preserved token rather than a restated rule. + [Fact] + public void The_preserved_blob_is_rejected_by_the_drivers_own_validator_not_by_a_rule_stated_in_the_mapper() + { + var map = CsvColumnMap.Resolve(DriverTypeNames.Modbus).ShouldNotBeNull(); + + var merged = map.ApplyTypedColumns( + "{\"region\":\"HoldingRegisters\",\"address\":1,\"dataType\":\"Intt16\"}", + new Dictionary(StringComparer.Ordinal)); + + EditorTagConfigJson.ParseOrNew(merged)["dataType"]!.GetValue().ShouldBe("Intt16"); + TagConfigValidator.Validate(DriverTypeNames.Modbus, merged).ShouldNotBeNull(); + } + + // ------------------------------------------------------------------ R2: fragment composition is intact + + // The acceptance control for the whole change. The TagConfigJson cell is legitimately a FRAGMENT — the + // typed columns supply address/dataType/etc. — so nothing here may impose a completeness demand on the + // bare cell. (Running the full model validator over the pre-merge cell, the shape the issue itself + // proposed, would false-reject every one of these rows and break typed columns outright.) + [Fact] + public void A_blank_cell_plus_typed_columns_still_composes_and_still_stamps_the_model_defaults() + { + var row = SingleRow( + DriverTypeNames.Modbus, + "Name,DataType,Region,Address,ModbusDataType,TagConfigJson", + "t1,Int16,HoldingRegisters,7,Int32,"); + + var blob = Blob(row); + blob["region"]!.GetValue().ShouldBe("HoldingRegisters"); + blob["address"]!.GetValue().ShouldBe(7); + blob["dataType"]!.GetValue().ShouldBe("Int32"); + + // Keys neither the cell nor a column declared still get the model's default, exactly as before — + // preservation restores what the cell CARRIED, it does not suppress the model's contribution. + blob["byteOrder"]!.GetValue().ShouldBe("BigEndian"); + blob["stringLength"]!.GetValue().ShouldBe(0); + } + + [Fact] + public void An_S7_row_whose_type_lives_only_in_the_typed_column_is_accepted() + { + // S7 is the one driver that rejects an absent dataType key (Gitea #551), so it is the sharpest + // fragment control: the cell declares none, and the row must still pass on the column alone. + var row = SingleRow( + DriverTypeNames.S7, + "Name,DataType,Address,S7DataType,TagConfigJson", + "t1,Int16,DB1.DBW0,Int32,\"{\"\"pollHint\"\":\"\"fast\"\"}\""); + + var blob = Blob(row); + blob["dataType"]!.GetValue().ShouldBe("Int32"); + blob["pollHint"]!.GetValue().ShouldBe("fast", "an unowned cell key must still survive the merge"); + } + + [Fact] + public void A_valid_cell_value_under_an_owned_key_survives_unchanged() + { + var row = SingleRow( + DriverTypeNames.Modbus, + "Name,DataType,TagConfigJson", + "t1,Int32,\"{\"\"region\"\":\"\"Coils\"\",\"\"address\"\":42,\"\"dataType\"\":\"\"Bool\"\"}\""); + + var blob = Blob(row); + blob["region"]!.GetValue().ShouldBe("Coils"); + blob["address"]!.GetValue().ShouldBe(42); + blob["dataType"]!.GetValue().ShouldBe("Bool"); + } + + // ------------------------------------------------------------ The override-contradiction semantics + + /// + /// A cell token that a NON-BLANK typed column overrides is accepted, and the column wins — + /// deliberately, not by omission. + /// + /// + /// Typed-column-wins is this map's documented contract, and the review grid reports each such + /// override by name (), so the operator + /// SEES it. The #549 doctrine is against silent rewrites; this one is announced. + /// + /// + /// Erroring would remove the only way to repair a poisoned tag through the CSV surface: the + /// operator would have to hand-edit the JSON cell, which is the thing typed columns exist to + /// replace. An exported legacy row is exactly where a bad cell value meets a corrected column. + /// + /// + /// The overridden token never reaches the stored blob, so no evidence about what the runtime + /// will do is lost. The line the fix draws is therefore clean: every token that reaches the + /// blob is judged; a token the operator overrode is discarded openly and reported. + /// + /// + /// + [Fact] + public void A_cell_typo_that_a_non_blank_column_overrides_is_accepted_with_the_column_winning_and_reported() + { + var row = SingleRow( + DriverTypeNames.Modbus, + "Name,DataType,Region,Address,ModbusDataType,TagConfigJson", + "t1,Int32,HoldingRegisters,1,Int32,\"{\"\"dataType\"\":\"\"Intt16\"\"}\""); + + var blob = Blob(row); + blob["dataType"]!.GetValue().ShouldBe("Int32"); + row.TypedOverrodeFallback.ShouldContain("ModbusDataType"); + } + + [Fact] + public void An_mtconnect_alias_the_column_overrides_is_reported_as_an_override_too() + { + // The provenance report reads the column's whole owned key set, so an overridden "driverDataType" is + // announced exactly as an overridden "dataType" would be — otherwise the accept-and-win decision + // above would be silent for the one driver where it matters most. + var row = SingleRow( + DriverTypeNames.MTConnect, + "Name,DataType,DataItemId,MtDataType,TagConfigJson", + "t1,Double,di1,Int32,\"{\"\"driverDataType\"\":\"\"Float64\"\"}\""); + + row.Error.ShouldBeNull(); + row.TypedOverrodeFallback.ShouldContain("MtDataType"); + } + + // ---------------------------------------------- MTConnect export -> reimport, through the real runtime + + [Fact] + public void Export_surfaces_the_effective_type_in_the_typed_cell_and_leaves_no_alias_in_the_residual() + { + // The trap was set here, not at import: the residual carried the alias the driver PREFERS while the + // MtDataType cell printed the key it ignores (or nothing at all). + var rows = ParseCsv(RawTagCsvMapper.Export(DriverTypeNames.MTConnect, new[] + { + new RawCsvExportTag(null, "aliasOnly", "Double", TagAccessLevel.Read, false, null, + "{\"fullName\":\"di1\",\"driverDataType\":\"Float64\"}"), + new RawCsvExportTag(null, "bothSpellings", "Double", TagAccessLevel.Read, false, null, + "{\"fullName\":\"di2\",\"driverDataType\":\"Float64\",\"dataType\":\"String\"}"), + new RawCsvExportTag(null, "legacyIdAlias", "Double", TagAccessLevel.Read, false, null, + "{\"address\":\"di3\",\"dataType\":\"Int32\"}"), + })); + + // The cell shows what the DRIVER reads, in every spelling combination… + rows["aliasOnly"]["MtDataType"].ShouldBe("Float64"); + rows["bothSpellings"]["MtDataType"].ShouldBe("Float64"); + rows["legacyIdAlias"]["DataItemId"].ShouldBe("di3"); + + // …and the residual is empty, so a re-import cannot smuggle an out-ranking spelling back in. + rows["aliasOnly"]["TagConfigJson"].ShouldBeEmpty(); + rows["bothSpellings"]["TagConfigJson"].ShouldBeEmpty(); + rows["legacyIdAlias"]["TagConfigJson"].ShouldBeEmpty(); + } + + [Fact] + public void An_edited_column_beats_a_residual_alias_all_the_way_into_the_runtime_factory() + { + // (i) The issue's headline round-trip: an operator edits MtDataType in an exported CSV whose residual + // still carries driverDataType (the shape produced before the export fix, and the shape a + // hand-authored or driver-shaped blob has anyway). The edit must reach the driver. + var row = SingleRow( + DriverTypeNames.MTConnect, + "Name,DataType,DataItemId,MtDataType,TagConfigJson", + "t1,Double,di1,Int32,\"{\"\"driverDataType\"\":\"\"Float64\"\"}\""); + + var blob = Blob(row); + blob.ContainsKey("driverDataType").ShouldBeFalse("the out-ranking alias must be cleared, not left beside the key"); + blob["dataType"]!.GetValue().ShouldBe("Int32"); + + // Asked of the real runtime parser, which is the only thing whose opinion decides the tag's type. + MTConnectTagDefinitionFactory + .TryFromTagConfig(row.ImportRow!.Tag.TagConfig, out var dataItemId, out var authored) + .ShouldBeTrue(); + dataItemId.ShouldBe("di1"); + authored!.DriverDataType.ShouldBe(DriverDataType.Int32); + } + + [Fact] + public void An_empty_column_leaves_a_valid_alias_carrying_the_same_type_it_always_did() + { + // (ii) Nothing is overridden, so nothing is rewritten — and the type the runtime reads is unchanged. + var row = SingleRow( + DriverTypeNames.MTConnect, + "Name,DataType,DataItemId,MtDataType,TagConfigJson", + "t1,Double,di1,,\"{\"\"driverDataType\"\":\"\"Float64\"\"}\""); + + row.Error.ShouldBeNull(); + MTConnectTagDefinitionFactory + .TryFromTagConfig(row.ImportRow!.Tag.TagConfig, out _, out var authored) + .ShouldBeTrue(); + authored!.DriverDataType.ShouldBe(DriverDataType.Float64); + } + + [Fact] + public void An_invalid_alias_that_no_column_overrides_is_an_error() + { + // (iii) The alias is read at the runtime's precedence by the model's Validate(), so the row is named + // before commit rather than deploying a tag the driver refuses. + var row = SingleRow( + DriverTypeNames.MTConnect, + "Name,DataType,DataItemId,MtDataType,TagConfigJson", + "t1,Double,di1,,\"{\"\"driverDataType\"\":\"\"Flt64\"\"}\""); + + row.Error.ShouldNotBeNull(); + row.Error.ShouldContain("Flt64"); + row.Error.ShouldContain("driverDataType", Case.Sensitive, + "the message must name the spelling the operator wrote, not the canonical key"); + } + + // ------------------------------------------------------------------------------- Falsification note + + /// + /// Falsification evidence (recorded, not executed). Deleting the base-token restore loop from + /// ModelBackedCsvDriverTagMap.ApplyTypedColumns and the + /// ClearOutrankingAliases/ReadEffective calls turns exactly the tests in this class red + /// and nothing else: the six laundering/alias assertions fail, while the acceptance controls above, + /// the whole RawTagCsvRoundTripTests property suite, and the Gitea #551 service-gate tests in + /// RawTagConfigGateTests all stay green — which is the point. This layer preserves evidence; + /// the gate behind it, in , still owns the verdict and is + /// unchanged. A fix that had instead moved the verdict here would have taken those service-gate tests + /// with it. + /// + /// The live assertion of that layering is below: the exact blob the mapper stages is put to + /// — the same call ImportTagsAsync makes — and rejected + /// there too, so the two layers cannot drift into agreeing only by accident. + /// + /// + [Fact] + public void Neutering_the_preservation_reproduces_the_defect_and_leaves_the_service_gate_green() + { + var map = CsvColumnMap.Resolve(DriverTypeNames.MTConnect).ShouldNotBeNull(); + var empty = new Dictionary(StringComparer.Ordinal); + + // With no column firing, the merge is a pass-through and the gate — not the merge — refuses it. + var preserved = map.ApplyTypedColumns("{\"fullName\":\"di1\",\"driverDataType\":\"Flt64\"}", empty); + TagConfigValidator.Validate(DriverTypeNames.MTConnect, preserved).ShouldNotBeNull(); + + // With the column firing, the merge clears the alias and the gate accepts the operator's choice. + var overridden = map.ApplyTypedColumns( + "{\"fullName\":\"di1\",\"driverDataType\":\"Flt64\"}", + new Dictionary(StringComparer.Ordinal) { ["MtDataType"] = "Int32" }); + TagConfigValidator.Validate(DriverTypeNames.MTConnect, overridden).ShouldBeNull(); + MTConnectTagDefinitionFactory.TryFromTagConfig(overridden, out _, out var authored).ShouldBeTrue(); + authored!.DriverDataType.ShouldBe(DriverDataType.Int32); + } + + // Re-reads an exported document into header→cell maps keyed by the row's Name column. + private static Dictionary> ParseCsv(string csv) + { + var raw = ZB.MOM.WW.OtOpcUa.Commons.Csv.CsvParser.Parse(csv); + var headers = raw[0]; + var byName = new Dictionary>(StringComparer.Ordinal); + foreach (var cells in raw.Skip(1)) + { + if (cells.Length == 1 && cells[0].Length == 0) { continue; } + + var map = new Dictionary(StringComparer.Ordinal); + for (var i = 0; i < headers.Length; i++) { map[headers[i]] = i < cells.Length ? cells[i] : ""; } + + byName[map["Name"]] = map; + } + + return byName; + } +}