fix(adminui): OpcUaClient editor reads the legacy PascalCase NodeId at the reader's precedence (#557 item 1)

`OpcUaClientTagDefinitionFactory.TryReadNodeId` still honours a legacy PascalCase
`NodeId` when the v3 camelCase `nodeId` is wholly absent, but
`OpcUaClientTagConfigModel` read only `nodeId` — so a legacy blob was SERVED by
the driver yet opened with an empty reference field and could not be saved
untouched. Authoring stricter than the reader, which #554 names as never
legitimate: the editor was blocking config the runtime honours.

Fixed with the #550 pattern (read both spellings, normalise on save), NOT by
retiring the spelling from the reader — that would break every legacy blob the
driver currently serves at the next deploy, a data-plane regression traded for
an editor asymmetry.

The precedence is PRESENCE-shadows, not first-non-blank. The reader's
`TryGetProperty("nodeId", out n) || TryGetProperty("NodeId", out n)`
short-circuits on the mere PRESENCE of the camelCase key and then checks
string-and-non-blank ONCE, so a blank / non-string / JSON-null `nodeId` beside a
valid legacy `NodeId` is a REJECTED blob, not a fallback. Copying MTConnect's
`FirstNonBlank` walk here would have made authoring WEAKER than the reader —
the same defect in mirror image.

- `NodeIdKeys` drives both halves, so the read and the drop cannot drift.
- `ToJson` writes `nodeId` and drops `NodeId`; the TagModal history-merge keys
  (`isHistorized` / `historianTagname`) are disjoint and untouched.
- `Validate`'s required-reference fast path is now precedence-aware by
  construction, so it agrees with the #554 parse consult across the whole
  rejection surface.
- The `TagConfigNonEnumParityTests` exemption for OpcUaClient is REMOVED — its
  anti-vacuity guard `Every_deliberately_stricter_entry_is_still_exactly_that`
  went red on the fix, which is this change's falsification control. The legacy
  shapes moved into the corpus, where both fleet theories now cover them.

Tests: a nine-row precedence theory driving the authoring verdict against the
READER's verdict per shape, on the original blob AND on the `ToJson()` output;
no-retype (legacy loads showing its reference, saves canonical, same node id
before and after); no-drift (output never carries `NodeId`). Each half was
reverted in isolation to confirm it reddens tests of its own: read-only-revert
=> 6 red incl. the fleet parity theory; drop-only-revert => 4 red.

Suites: AdminUI.Tests 1459/1459, ControlPlane.Tests 261/261,
Driver.OpcUaClient.Tests 143/143, Driver.OpcUaClient.Browser.Tests 10/10.
This commit is contained in:
Joseph Doherty
2026-08-10 03:01:41 -04:00
parent ca3968cc31
commit be5899bc7d
6 changed files with 255 additions and 17 deletions
@@ -32,6 +32,15 @@ public static class OpcUaClientTagDefinitionFactory
/// Read the upstream node id string from an authored <c>TagConfig</c> blob's <c>nodeId</c> key
/// (camelCase, per the AdminUI <c>OpcUaClientTagConfigModel</c>; a legacy PascalCase <c>NodeId</c>
/// is also accepted). Never throws — a malformed blob yields <see langword="false"/>.
/// <para>
/// ⚠️ <b>The two spellings are PRESENCE-shadows, not first-non-blank.</b> The <c>||</c> below
/// short-circuits on the mere presence of <c>nodeId</c> — whatever it holds — and the string /
/// non-blank check then runs ONCE, on that one element. So the legacy <c>NodeId</c> is consulted
/// only when <c>nodeId</c> is <i>wholly absent</i>: a blank or non-string <c>nodeId</c> beside a
/// valid legacy <c>NodeId</c> is a rejected blob, not a fallback. The AdminUI model mirrors this
/// walk exactly (Gitea #557 item 1) rather than the looser first-non-blank shape MTConnect's
/// reader uses, which would let authoring accept a blob this reader refuses.
/// </para>
/// </summary>
/// <param name="tagConfig">The authored equipment-tag TagConfig JSON.</param>
/// <param name="nodeId">The upstream node id string when this returns <see langword="true"/>.</param>