fix(sql): read keyValue with the same scalar read as whereValue (Driver.Sql.Contracts-001)
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.
This commit is contained in:
Joseph Doherty
+7
-2
@@ -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
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
/// <summary>
|
||||
/// 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 <c>whereValue</c> 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.
|
||||
/// <para>
|
||||
/// <b>Both value-bearing fields go through here</b>, <c>keyValue</c> as well as
|
||||
/// <c>whereValue</c>, because there is no downstream difference between them: the reader BINDS
|
||||
/// each as a <see cref="System.Data.Common.DbParameter"/> and matches each back as text —
|
||||
/// <c>SqlPollReader.IndexByKey</c> 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.
|
||||
/// </para>
|
||||
/// </summary>
|
||||
private static string? ReadScalar(JsonElement o, string name)
|
||||
{
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -25,7 +25,9 @@ public sealed class SqlTagConfigModel
|
||||
/// <summary><see cref="SqlTagModel.KeyValue"/>: the column matched against <see cref="KeyValue"/>.</summary>
|
||||
public string? KeyColumn { get; set; }
|
||||
|
||||
/// <summary><see cref="SqlTagModel.KeyValue"/>: this tag's key value (bound as a parameter at read time).</summary>
|
||||
/// <summary><see cref="SqlTagModel.KeyValue"/>: 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.</summary>
|
||||
public string? KeyValue { get; set; }
|
||||
|
||||
/// <summary><see cref="SqlTagModel.KeyValue"/>: the column the value is read from.</summary>
|
||||
@@ -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<string>(out var s) ? s : v.ToJsonString();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<string>(out var s) ? s : null;
|
||||
|
||||
/// <summary>
|
||||
/// Reads a scalar as its textual form — a JSON string yields its contents, a number or boolean
|
||||
/// yields its raw token; <c>null</c> when absent, JSON <c>null</c>, or a non-scalar node. Mirrors
|
||||
/// the runtime parsers' <c>ReadScalar</c>, for the value-bearing fields a driver BINDS as a
|
||||
/// parameter and so may legitimately author as a number (a Sql <c>keyValue</c> facing an INTEGER
|
||||
/// key column, say). Reading such a field with <see cref="GetString"/> makes the editor refuse a
|
||||
/// blob the driver serves.
|
||||
/// </summary>
|
||||
/// <param name="o">The JSON object to read from.</param>
|
||||
/// <param name="name">The property name to read.</param>
|
||||
/// <returns>The scalar's text, or <c>null</c> when absent or not a scalar.</returns>
|
||||
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<string>(out var s) ? s : v.ToJsonString();
|
||||
}
|
||||
|
||||
/// <summary>Reads an int value, or <paramref name="fallback"/> if absent/null/non-numeric (incl. object/array nodes).</summary>
|
||||
/// <param name="o">The JSON object to read from.</param>
|
||||
/// <param name="name">The property name to read.</param>
|
||||
|
||||
@@ -28,6 +28,55 @@ public class SqlEquipmentTagParserTests
|
||||
def.Name.ShouldBe("Plant/Sql/dev1/Speed"); // Name == RawPath (forward-router key)
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// 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 <c>DbParameter</c> and
|
||||
/// both are matched back as text — <c>SqlPollReader.IndexByKey</c> stringifies the key cell
|
||||
/// precisely so an authored key can face an INTEGER key column. <c>keyValue</c> used to go through
|
||||
/// the narrower string-only read, so a numerically-authored key refused the tag outright while the
|
||||
/// identically-bound <c>whereValue</c> sailed through as the design doc's own example authors it.
|
||||
/// </summary>
|
||||
[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);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// …and a non-scalar key is still refused — the widening is to the scalar shapes a parameter can
|
||||
/// carry, not to anything at all.
|
||||
/// </summary>
|
||||
[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();
|
||||
|
||||
/// <summary>
|
||||
/// The <c>Inspect</c> 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.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void Inspect_aNumericallyAuthoredKeyValue_drawsNoWarning()
|
||||
=> SqlTagDefinitionFactory.Inspect(
|
||||
"""{"driver":"Sql","model":"KeyValue","table":"T","keyColumn":"k","keyValue":7,"valueColumn":"c"}""")
|
||||
.ShouldBeEmpty();
|
||||
|
||||
/// <summary>Falsifiability control for the assertion above: a genuinely absent key still warns.</summary>
|
||||
[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(
|
||||
|
||||
@@ -184,6 +184,32 @@ public sealed class SqlTagConfigModelTests
|
||||
=> SqlTagConfigModel.FromJson("""{"model":"KeyValue","table":"t","keyColumn":"k","valueColumn":"c"}""")
|
||||
.Validate().ShouldNotBeNull();
|
||||
|
||||
/// <summary>
|
||||
/// Driver.Sql.Contracts-001. The parser reads <c>keyValue</c> with the same scalar read it uses for
|
||||
/// <c>whereValue</c>, 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 <c>whereValue</c>. An editor that refused it would be stricter than the
|
||||
/// runtime, which is the drift this parity suite exists to catch.
|
||||
/// </summary>
|
||||
[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"}""")
|
||||
|
||||
Reference in New Issue
Block a user