From 678aa3aa538cd4ea5b1a257ca585d404b6dc7c66 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 10 Aug 2026 08:07:15 -0400 Subject: [PATCH] fix(sql): read keyValue with the same scalar read as whereValue (Driver.Sql.Contracts-001) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The two value-bearing fields were read asymmetrically and nothing said why. whereValue went through ReadScalar (string/number/boolean — the design doc's own wide-row example authors it as the number 7); keyValue went through the string-only ReadString, so a numerically-authored key refused the tag outright. Nothing downstream justified the split: both are BOUND as DbParameters and both are matched back as text — SqlPollReader.IndexByKey stringifies the key cell precisely so an authored key can face an INTEGER key column. The refusal was also mis-explained. Inspect branched on ReadString(root, "keyValue") is null, so a numerically-authored key drew "Sql KeyValue tag has no 'keyValue'" — telling the operator the field is missing when it is right there in the blob, the #549 shape of an authoring surface disagreeing with the runtime about WHY. keyValue now uses ReadScalar; Inspect mirrors it with HasScalar and says what a usable key is; SqlTagConfigModel loads it with the shared TagConfigJson.GetScalarText (which also replaces SqlRowSelectorModel's private copy), so the editor is not left stricter than the parser. An object/array/null key is still refused. docs/drivers/Sql.md updated. RED first: 4 failing in Driver.Sql.Tests. --- docs/drivers/Sql.md | 9 +++- .../SqlEquipmentTagParser.cs | 15 ++++-- .../SqlTagDefinitionFactory.cs | 9 +++- .../Uns/TagEditors/SqlTagConfigModel.cs | 16 ++---- .../Uns/TagEditors/TagConfigJson.cs | 17 +++++++ .../SqlEquipmentTagParserTests.cs | 49 +++++++++++++++++++ .../Uns/SqlTagConfigModelTests.cs | 26 ++++++++++ 7 files changed, 123 insertions(+), 18 deletions(-) diff --git a/docs/drivers/Sql.md b/docs/drivers/Sql.md index 38163f50..b5bf2128 100644 --- a/docs/drivers/Sql.md +++ b/docs/drivers/Sql.md @@ -80,8 +80,13 @@ the nested form. } ``` -`whereValue` may be authored as a JSON string, number or boolean (it is bound as a parameter either -way). `keyValue` on the `KeyValue` model must be a **string**. +Both value-bearing fields — `whereValue` here and `keyValue` on the `KeyValue` model — may be +authored as a JSON string, number or boolean, and are captured as text. They are **bound as +parameters** either way, and matched back as text (the key cell is stringified), so a key column +that holds an INTEGER station id can be authored either `"7"` or `7`. A JSON object, array or `null` +is not a value and is refused. (They were read asymmetrically until 2026-08-10, so a numerically +authored `keyValue` refused the tag while the identically-bound `whereValue` was served — and the +deploy gate reported the key as *missing* when it was right there in the blob.) ### `Query` — deferred to P3 diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts/SqlEquipmentTagParser.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts/SqlEquipmentTagParser.cs index 006089f8..1414da48 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts/SqlEquipmentTagParser.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts/SqlEquipmentTagParser.cs @@ -62,7 +62,7 @@ public static class SqlEquipmentTagParser case SqlTagModel.KeyValue: { var keyColumn = ReadString(root, "keyColumn"); - var keyValue = ReadString(root, "keyValue"); + var keyValue = ReadScalar(root, "keyValue"); var valueColumn = ReadString(root, "valueColumn"); if (string.IsNullOrWhiteSpace(keyColumn) || string.IsNullOrWhiteSpace(valueColumn) @@ -115,8 +115,17 @@ public static class SqlEquipmentTagParser /// /// Reads a scalar property as its textual form — a JSON string yields its contents, a number or - /// boolean yields its raw token — so an authored whereValue of either shape is captured - /// verbatim for later parameter binding. Null when absent or not a scalar. + /// boolean yields its raw token — so an authored value of any of those shapes is captured verbatim + /// for later parameter binding. Null when absent or not a scalar. + /// + /// Both value-bearing fields go through here, keyValue as well as + /// whereValue, because there is no downstream difference between them: the reader BINDS + /// each as a and matches each back as text — + /// SqlPollReader.IndexByKey stringifies the key cell precisely so an authored key can + /// face an INTEGER key column. Reading one of them string-only refused a tag the other shape + /// of the same authoring mistake was served under, and the deploy gate then reported the field + /// as MISSING when it was right there in the blob. + /// /// private static string? ReadScalar(JsonElement o, string name) { diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts/SqlTagDefinitionFactory.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts/SqlTagDefinitionFactory.cs index cf06e1d3..bad2b98d 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts/SqlTagDefinitionFactory.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts/SqlTagDefinitionFactory.cs @@ -129,8 +129,13 @@ public static class SqlTagDefinitionFactory warnings.Add("Sql KeyValue tag has no 'keyColumn'."); if (string.IsNullOrWhiteSpace(ReadString(root, "valueColumn"))) warnings.Add("Sql KeyValue tag has no 'valueColumn'."); - if (ReadString(root, "keyValue") is null) - warnings.Add("Sql KeyValue tag has no 'keyValue' (an empty string is allowed; an absent key is not)."); + if (!HasScalar(root, "keyValue")) + { + warnings.Add( + "Sql KeyValue tag has no usable 'keyValue' — it must be a string, number or " + + "boolean (an empty string is allowed; an absent key, or an object/array, is not)."); + } + break; case SqlTagModel.WideRow: diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/SqlTagConfigModel.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/SqlTagConfigModel.cs index 24d62a85..48928649 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/SqlTagConfigModel.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/SqlTagConfigModel.cs @@ -25,7 +25,9 @@ public sealed class SqlTagConfigModel /// : the column matched against . public string? KeyColumn { get; set; } - /// : this tag's key value (bound as a parameter at read time). + /// : this tag's key value (bound as a parameter at read time). + /// Captured as text whether the blob authored it as a JSON string or a JSON number/boolean, mirroring + /// the parser's scalar read. public string? KeyValue { get; set; } /// : the column the value is read from. @@ -72,7 +74,7 @@ public sealed class SqlTagConfigModel Model = model ?? SqlTagModel.KeyValue, Table = TagConfigJson.GetString(o, "table"), KeyColumn = TagConfigJson.GetString(o, "keyColumn"), - KeyValue = TagConfigJson.GetString(o, "keyValue"), + KeyValue = TagConfigJson.GetScalarText(o, "keyValue"), ValueColumn = TagConfigJson.GetString(o, "valueColumn"), TimestampColumn = TagConfigJson.GetString(o, "timestampColumn"), ColumnName = TagConfigJson.GetString(o, "columnName"), @@ -204,7 +206,7 @@ public sealed class SqlRowSelectorModel return new SqlRowSelectorModel { WhereColumn = TagConfigJson.GetString(bag, "whereColumn"), - WhereValue = ReadScalarText(bag, "whereValue"), + WhereValue = TagConfigJson.GetScalarText(bag, "whereValue"), TopByTimestamp = TagConfigJson.GetString(bag, "topByTimestamp"), _bag = bag, }; @@ -221,12 +223,4 @@ public sealed class SqlRowSelectorModel TagConfigJson.Set(o, "topByTimestamp", TopByTimestamp); return o.Count == 0 ? null : o; } - - // Reads a scalar as its textual form: a JSON string yields its contents, a number/boolean yields its raw - // token — so an authored whereValue of either shape is captured verbatim (mirrors the parser's ReadScalar). - private static string? ReadScalarText(JsonObject o, string name) - { - if (!o.TryGetPropertyValue(name, out var n) || n is not JsonValue v) { return null; } - return v.TryGetValue(out var s) ? s : v.ToJsonString(); - } } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/TagConfigJson.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/TagConfigJson.cs index 6537f462..38d4a5d8 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/TagConfigJson.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/TagConfigJson.cs @@ -34,6 +34,23 @@ public static class TagConfigJson public static string? GetString(JsonObject o, string name) => o.TryGetPropertyValue(name, out var n) && n is JsonValue v && v.TryGetValue(out var s) ? s : null; + /// + /// Reads a scalar as its textual form — a JSON string yields its contents, a number or boolean + /// yields its raw token; null when absent, JSON null, or a non-scalar node. Mirrors + /// the runtime parsers' ReadScalar, for the value-bearing fields a driver BINDS as a + /// parameter and so may legitimately author as a number (a Sql keyValue facing an INTEGER + /// key column, say). Reading such a field with makes the editor refuse a + /// blob the driver serves. + /// + /// The JSON object to read from. + /// The property name to read. + /// The scalar's text, or null when absent or not a scalar. + public static string? GetScalarText(JsonObject o, string name) + { + if (!o.TryGetPropertyValue(name, out var n) || n is not JsonValue v) { return null; } + return v.TryGetValue(out var s) ? s : v.ToJsonString(); + } + /// Reads an int value, or if absent/null/non-numeric (incl. object/array nodes). /// The JSON object to read from. /// The property name to read. diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlEquipmentTagParserTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlEquipmentTagParserTests.cs index ae5e37c7..4734ee30 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlEquipmentTagParserTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlEquipmentTagParserTests.cs @@ -28,6 +28,55 @@ public class SqlEquipmentTagParserTests def.Name.ShouldBe("Plant/Sql/dev1/Speed"); // Name == RawPath (forward-router key) } + /// + /// Driver.Sql.Contracts-001. The two value-bearing fields are read the same way: a JSON string, + /// number or boolean is captured as text, because both end up BOUND as a DbParameter and + /// both are matched back as text — SqlPollReader.IndexByKey stringifies the key cell + /// precisely so an authored key can face an INTEGER key column. keyValue used to go through + /// the narrower string-only read, so a numerically-authored key refused the tag outright while the + /// identically-bound whereValue sailed through as the design doc's own example authors it. + /// + [Theory] + [InlineData("7", "7")] + [InlineData("-12.5", "-12.5")] + [InlineData("true", "true")] + public void TryParse_KeyValue_aNonStringScalarKey_isCapturedAsTextLikeWhereValue(string token, string expected) + { + var json = + $$"""{"driver":"Sql","model":"KeyValue","table":"dbo.TagValues","keyColumn":"station_id","keyValue":{{token}},"valueColumn":"num_value"}"""; + SqlEquipmentTagParser.TryParse(json, "raw", out var def).ShouldBeTrue(); + def.KeyValue.ShouldBe(expected); + } + + /// + /// …and a non-scalar key is still refused — the widening is to the scalar shapes a parameter can + /// carry, not to anything at all. + /// + [Theory] + [InlineData("""{"driver":"Sql","model":"KeyValue","table":"T","keyColumn":"k","keyValue":{"a":1},"valueColumn":"c"}""")] + [InlineData("""{"driver":"Sql","model":"KeyValue","table":"T","keyColumn":"k","keyValue":[1],"valueColumn":"c"}""")] + [InlineData("""{"driver":"Sql","model":"KeyValue","table":"T","keyColumn":"k","keyValue":null,"valueColumn":"c"}""")] + [InlineData("""{"driver":"Sql","model":"KeyValue","table":"T","keyColumn":"k","valueColumn":"c"}""")] + public void TryParse_KeyValue_aNonScalarOrAbsentKey_rejectsTag(string json) + => SqlEquipmentTagParser.TryParse(json, "raw", out _).ShouldBeFalse(); + + /// + /// The Inspect half of the same rule. A numerically-authored key is served, so the deploy + /// gate must say nothing about it; an absent one is refused, and the gate must still name it. + /// + [Fact] + public void Inspect_aNumericallyAuthoredKeyValue_drawsNoWarning() + => SqlTagDefinitionFactory.Inspect( + """{"driver":"Sql","model":"KeyValue","table":"T","keyColumn":"k","keyValue":7,"valueColumn":"c"}""") + .ShouldBeEmpty(); + + /// Falsifiability control for the assertion above: a genuinely absent key still warns. + [Fact] + public void Inspect_anAbsentKeyValue_stillNamesTheField() + => SqlTagDefinitionFactory.Inspect( + """{"driver":"Sql","model":"KeyValue","table":"T","keyColumn":"k","valueColumn":"c"}""") + .ShouldContain(w => w.Contains("keyValue")); + [Fact] public void TryParse_invalidModelEnum_rejectsTag() => SqlEquipmentTagParser.TryParse( diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/SqlTagConfigModelTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/SqlTagConfigModelTests.cs index bfd0ad86..370de1ad 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/SqlTagConfigModelTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/SqlTagConfigModelTests.cs @@ -184,6 +184,32 @@ public sealed class SqlTagConfigModelTests => SqlTagConfigModel.FromJson("""{"model":"KeyValue","table":"t","keyColumn":"k","valueColumn":"c"}""") .Validate().ShouldNotBeNull(); + /// + /// Driver.Sql.Contracts-001. The parser reads keyValue with the same scalar read it uses for + /// whereValue, so a numerically-authored key is SERVED — the editor must load it (rather + /// than seeing an absent key), let it save, and normalise it to a string on the way out, exactly as + /// it already does for whereValue. An editor that refused it would be stricter than the + /// runtime, which is the drift this parity suite exists to catch. + /// + [Fact] + public void A_numerically_authored_keyValue_loads_validates_and_normalises_to_a_string() + { + const string json = + """{"model":"KeyValue","table":"t","keyColumn":"station_id","keyValue":7,"valueColumn":"c"}"""; + SqlEquipmentTagParser.TryParse(json, "raw", out var served).ShouldBeTrue( + "the runtime must serve this blob, or the authoring parity below proves nothing."); + served.KeyValue.ShouldBe("7"); + + var m = SqlTagConfigModel.FromJson(json); + m.KeyValue.ShouldBe("7"); + m.Validate().ShouldBeNull(); + + var saved = m.ToJson(); + saved.ShouldContain("\"keyValue\":\"7\""); + SqlEquipmentTagParser.TryParse(saved, "raw", out var afterSave).ShouldBeTrue(); + afterSave.KeyValue.ShouldBe("7"); + } + [Fact] public void Validate_rejects_wideRow_without_a_selector() => SqlTagConfigModel.FromJson("""{"model":"WideRow","table":"t","columnName":"c"}""")