fix(csv): stop the typed-column merge laundering the TagConfigJson cell (#553)

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.
This commit is contained in:
Joseph Doherty
2026-08-10 01:18:55 -04:00
parent f896e0631c
commit cf2af8a5a2
3 changed files with 499 additions and 9 deletions
@@ -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 <see cref="IRawTreeService.ImportTagsAsync"/>, which re-runs it.
/// </para>
/// <para>
/// For that gate to mean anything the merge must hand it the row <b>as written</b>, which it did not
/// (Gitea #553): the model-backed maps round-tripped the free-form cell through a driver model's lenient
/// <c>FromJson</c>/<c>ToJson</c>, 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.
/// <see cref="CsvDriverTagMap.ApplyTypedColumns"/> 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.
/// </para>
/// </summary>
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);
}
@@ -19,9 +19,35 @@ namespace ZB.MOM.WW.OtOpcUa.AdminUI.Uns.TagEditors;
/// a header directly onto a JSON key.
/// </summary>
/// <param name="Header">The CSV column header (PascalCase, e.g. <c>ModbusDataType</c>).</param>
/// <param name="JsonKey">The root TagConfig JSON key this column reads/writes (camelCase, e.g. <c>dataType</c>).</param>
/// <param name="JsonKey">The root TagConfig JSON key this column <b>writes</b> (camelCase, e.g. <c>dataType</c>).</param>
/// <param name="ModelProperty">The backing model property name, or <c>null</c> for a model-less map.</param>
public sealed record CsvTypedColumn(string Header, string JsonKey, string? ModelProperty);
/// <param name="ReadKeys">
/// The full ordered set of root JSON keys the <b>runtime</b> reads for this column's value, most-preferred
/// first, or <c>null</c> when the column owns only <paramref name="JsonKey"/>. Supplied for the drivers
/// whose blob accepts more than one spelling of a field (MTConnect's <c>driverDataType</c>/<c>dataType</c>
/// and <c>fullName</c>/<c>dataItemId</c>/<c>address</c>) — see <see cref="OwnedJsonKeys"/> for why the CSV
/// path has to know the whole set and not just the one it writes.
/// </param>
public sealed record CsvTypedColumn(
string Header,
string JsonKey,
string? ModelProperty,
IReadOnlyList<string>? ReadKeys = null)
{
/// <summary>
/// Every root JSON key this column speaks for, in the <b>runtime's</b> read precedence — i.e.
/// <see cref="ReadKeys"/> when supplied, else just <see cref="JsonKey"/>.
/// <para>
/// A column that writes <c>dataType</c> while the driver prefers a <c>driverDataType</c> alias does
/// not actually control the tag's type: the alias, carried through in the free-form
/// <c>TagConfigJson</c> 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 <i>effective</i> value, and a firing
/// column clear the alias so it genuinely wins.
/// </para>
/// </summary>
public IReadOnlyList<string> OwnedJsonKeys { get; } = ReadKeys is { Count: > 0 } ? ReadKeys : [JsonKey];
}
/// <summary>
/// Per-driver mapping between a CSV row's <b>typed</b> columns and a tag's driver-specific
@@ -46,8 +72,50 @@ public abstract class CsvDriverTagMap
/// <summary>The typed column headers, in order.</summary>
public IReadOnlyList<string> Headers => Columns.Select(c => c.Header).ToArray();
/// <summary>The root TagConfig JSON keys this map owns (used to compute the export residual).</summary>
public IReadOnlyList<string> JsonKeys => Columns.Select(c => c.JsonKey).ToArray();
/// <summary>The root TagConfig JSON keys this map owns (used to compute the export residual) — every
/// <see cref="CsvTypedColumn.OwnedJsonKeys"/> entry, aliases included, so the residual can never carry a
/// spelling that out-ranks the typed column beside it (Gitea #553).</summary>
public IReadOnlyList<string> JsonKeys => Columns.SelectMany(c => c.OwnedJsonKeys).Distinct(StringComparer.Ordinal).ToArray();
/// <summary>
/// Applies one typed column's already-parsed effect to <paramref name="target"/>: removes every
/// <b>other</b> 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 <see cref="CsvTypedColumn.JsonKey"/>
/// themselves (model-backed maps do it through the model). Gitea #553.
/// </summary>
/// <param name="target">The merged TagConfig object being assembled.</param>
/// <param name="column">The typed column that supplied a value on this row.</param>
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);
}
}
}
/// <summary>
/// Reads one typed column's <b>effective</b> cell value out of a stored TagConfig for export: the
/// first non-blank string under <see cref="CsvTypedColumn.OwnedJsonKeys"/>, in the runtime's own
/// precedence. Exporting the canonical key alone would print the spelling the driver is <i>ignoring</i>
/// and leave the winning alias in the residual, which is how the re-import trap in Gitea #553 got set.
/// </summary>
/// <param name="source">The stored TagConfig object.</param>
/// <param name="column">The typed column to project.</param>
/// <returns>The effective raw node, or <c>null</c> when no spelling carries a value.</returns>
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<string>(out var s) && string.IsNullOrWhiteSpace(s)) { continue; }
return node;
}
return null;
}
/// <summary>
/// Folds the typed columns present (non-blank) in <paramref name="row"/> onto
@@ -122,18 +190,58 @@ public sealed class ModelBackedCsvDriverTagMap<TModel> : CsvDriverTagMap
public override IReadOnlyList<CsvTypedColumn> Columns { get; }
/// <inheritdoc/>
/// <remarks>
/// <b>The base cell's own tokens are restored verbatim over the model's serialisation (Gitea #553).</b>
/// The model round-trip is what merges the typed columns and stamps a default for a key the row
/// declares nowhere — but <c>FromJson</c> is deliberately lenient (it has to LOAD a poisoned blob so an
/// operator can fix it), so a <c>"dataType":"Intt16"</c> the row carried in its free-form
/// <c>TagConfigJson</c> cell came back out of <c>ToJson</c> as the model's <i>default</i> — 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.
/// <para>
/// 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 <i>good</i>: that stays with
/// <see cref="TagConfigValidator"/> on the assembled blob (and with
/// <c>IRawTreeService.ImportTagsAsync</c> behind it), which asks each driver model's own
/// <c>Validate()</c> — one authority. This method's only job is to stop destroying the evidence
/// before that authority is consulted.
/// </para>
/// <para>
/// A key a non-blank column DID override is <b>not</b> 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.
/// </para>
/// </remarks>
public override string ApplyTypedColumns(string? baseJson, IReadOnlyDictionary<string, string> row)
{
var baseObj = TagConfigJson.ParseOrNew(baseJson);
var model = _fromJson(baseJson);
var overridden = new HashSet<string>(StringComparer.Ordinal);
var fired = new List<CsvTypedColumn>();
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);
}
/// <inheritdoc/>
@@ -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<string, string>(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),