dohertj2/lmxopcua
1
0
Fork 0
Code Issues 32 Pull Requests Actions Packages Projects Releases Wiki Activity

perf(calc-contracts): single-parse Inspect (Driver.Calculation.Contracts-001)

Browse Source 
Inspect parsed the TagConfig blob once itself (root kind + the present-but-discarded
changeTriggered/timerIntervalMs checks) and then called TryReadTagConfig(reference, out _),
which parsed the same string from scratch again for the scriptId rule. The field-by-field
read is now TryReadRoot(JsonElement), which both entry points share, so Inspect consults
the one authority over the root it already has. The public string-based TryReadTagConfig
signature other modules consume is unchanged.

Pure refactor: added CalculationTagDefinitionFactoryInspectTests (17 cases pinning the
exact wording, the rejection-vs-advisory split and their order, and the
never-stricter-than-the-runtime-read invariant). They pass unchanged against the
pre-refactor code — verified by stashing the production change.
This commit is contained in:
Joseph Doherty
2026-08-10 08:02:51 -04:00
parent 235e06a875
commit 73230a42c5
2 changed files with 150 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Calculation.Contracts/CalculationTagDefinitionFactory.cs
+39 -21
View File
@@ -68,26 +68,7 @@ public static class CalculationTagDefinitionFactory
try
{
using var doc = JsonDocument.Parse(tagConfig);
var root = doc.RootElement;
if (root.ValueKind != JsonValueKind.Object) return false;
var scriptId = root.TryGetProperty("scriptId", out var sidEl) && sidEl.ValueKind == JsonValueKind.String
? sidEl.GetString() : null;
if (string.IsNullOrWhiteSpace(scriptId)) return false;
var scriptSource = root.TryGetProperty("scriptSource", out var srcEl) && srcEl.ValueKind == JsonValueKind.String
? srcEl.GetString() ?? string.Empty : string.Empty;
// changeTriggered defaults to true (mini-design §2) — only an explicit false disables it.
var changeTriggered = !(root.TryGetProperty("changeTriggered", out var ctEl)
&& ctEl.ValueKind == JsonValueKind.False);
var timerMs = root.TryGetProperty("timerIntervalMs", out var tEl)
&& tEl.ValueKind == JsonValueKind.Number && tEl.TryGetInt32(out var ms) && ms > 0
? ms : (int?)null;
fields = new CalculationTagConfigFields(scriptId!, scriptSource, changeTriggered, timerMs);
return true;
return TryReadRoot(doc.RootElement, out fields);
}
catch (JsonException)
{
@@ -95,6 +76,41 @@ public static class CalculationTagDefinitionFactory
}
}
/// <summary>
/// The field-by-field read, over an already-parsed root. Split out of
/// <see cref="TryReadTagConfig"/> so <see cref="Inspect"/> — which has necessarily parsed the blob
/// already to reach its root-kind and present-but-discarded checks — can consult the one authority
/// on what a usable <c>scriptId</c> is without parsing the same string a second time
/// (Driver.Calculation.Contracts-001). The public string-based entry point is what other modules
/// consume, so it keeps its signature.
/// </summary>
/// <param name="root">The parsed <c>TagConfig</c> root element.</param>
/// <param name="fields">The read fields when this returns <see langword="true"/>.</param>
/// <returns><see langword="true"/> when the root is an object naming a script.</returns>
private static bool TryReadRoot(JsonElement root, out CalculationTagConfigFields fields)
{
fields = default;
if (root.ValueKind != JsonValueKind.Object) return false;
var scriptId = root.TryGetProperty("scriptId", out var sidEl) && sidEl.ValueKind == JsonValueKind.String
? sidEl.GetString() : null;
if (string.IsNullOrWhiteSpace(scriptId)) return false;
var scriptSource = root.TryGetProperty("scriptSource", out var srcEl) && srcEl.ValueKind == JsonValueKind.String
? srcEl.GetString() ?? string.Empty : string.Empty;
// changeTriggered defaults to true (mini-design §2) — only an explicit false disables it.
var changeTriggered = !(root.TryGetProperty("changeTriggered", out var ctEl)
&& ctEl.ValueKind == JsonValueKind.False);
var timerMs = root.TryGetProperty("timerIntervalMs", out var tEl)
&& tEl.ValueKind == JsonValueKind.Number && tEl.TryGetInt32(out var ms) && ms > 0
? ms : (int?)null;
fields = new CalculationTagConfigFields(scriptId!, scriptSource, changeTriggered, timerMs);
return true;
}
/// <summary>
/// Deploy-time inspection of a calc tag's <c>TagConfig</c> blob. Warns for a blob
/// <see cref="TryReadTagConfig"/> rejects (unparseable JSON, a non-object root, or no usable
@@ -130,7 +146,9 @@ public static class CalculationTagDefinitionFactory
return warnings;
}
if (!TryReadTagConfig(reference, out _))
// The SAME read the runtime performs, over the root already parsed above — never a second
// parse of the same string, and never a reimplementation of the scriptId rule.
if (!TryReadRoot(root, out _))
{
warnings.Add(
"Calculation TagConfig has no usable 'scriptId' (a non-blank string) — the driver does not "
tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Calculation.Tests/CalculationTagDefinitionFactoryInspectTests.cs
+111
View File
@@ -0,0 +1,111 @@
using Shouldly;
using Xunit;
namespace ZB.MOM.WW.OtOpcUa.Driver.Calculation.Tests;
/// <summary>
/// Deploy-time gate contract for <see cref="CalculationTagDefinitionFactory.Inspect"/>. Its outcomes
/// were pinned only by the AdminUI parity corpus (a DB-backed suite); these fix the wording and the
/// rejection/advisory split locally, so the single-parse refactor
/// (Driver.Calculation.Contracts-001 — <c>Inspect</c> now reads its own already-parsed root instead of
/// re-parsing the blob through <c>TryReadTagConfig</c>) is provably behaviour-preserving.
/// <para>The invariant underneath all of it: <c>Inspect</c> is never STRICTER than
/// <see cref="CalculationTagDefinitionFactory.TryReadTagConfig"/> — the deploy gate can be configured
/// to turn warnings into blocking errors, so it may never flag a blob the driver serves happily.</para>
/// </summary>
public sealed class CalculationTagDefinitionFactoryInspectTests
{
[Fact]
public void A_clean_blob_draws_nothing()
{
CalculationTagDefinitionFactory
.Inspect("""{"scriptId":"s1","scriptSource":"return 1.0;","changeTriggered":true,"timerIntervalMs":5000}""")
.ShouldBeEmpty();
}
[Theory]
[InlineData("")]
[InlineData(" ")]
[InlineData("[1,2]")] // not object-shaped at all — some other authoring surface's blob, not ours
[InlineData("plain text")]
public void A_blank_or_non_object_shaped_blob_is_left_alone(string blob)
{
CalculationTagDefinitionFactory.Inspect(blob).ShouldBeEmpty();
}
[Fact]
public void Unparseable_json_is_reported_as_a_skip()
{
CalculationTagDefinitionFactory.Inspect("{\"scriptId\":").ShouldBe(
[
"Calculation TagConfig is not valid JSON — the driver will skip this tag and it will never compute.",
]);
}
// The "root is not a JSON object" warning is unreachable from Inspect by construction: the leading-'{'
// pre-filter above returns early for every non-object root, and anything that starts with '{' and
// parses IS an object. It is kept as a belt-and-braces guard for a future change to that pre-filter,
// so there is nothing to pin here beyond the pre-filter cases already covered.
[Theory]
[InlineData("{}")]
[InlineData("""{"scriptId":" "}""")]
[InlineData("""{"scriptId":42}""")]
public void A_blob_with_no_usable_scriptId_is_reported_as_a_skip(string blob)
{
CalculationTagDefinitionFactory.Inspect(blob).ShouldContain(
"Calculation TagConfig has no usable 'scriptId' (a non-blank string) — the driver does not "
+ "treat this as a calc tag, skips it, and it never computes.");
}
[Fact]
public void A_non_boolean_changeTriggered_draws_the_present_but_discarded_advisory()
{
CalculationTagDefinitionFactory.Inspect("""{"scriptId":"s1","changeTriggered":"false"}""").ShouldBe(
[
"value '\"false\"' for 'changeTriggered' is not a JSON boolean; anything but an explicit false "
+ "is read as TRUE, so this tag is change-triggered whatever the value looks like.",
]);
}
[Theory]
[InlineData("0")]
[InlineData("-5")]
[InlineData("1.5")]
[InlineData("\"5000\"")]
public void A_timerIntervalMs_that_is_not_a_positive_whole_number_draws_the_present_but_discarded_advisory(string raw)
{
var warnings = CalculationTagDefinitionFactory.Inspect($$"""{"scriptId":"s1","timerIntervalMs":{{raw}}}""");
warnings.ShouldBe(
[
$"value '{raw}' for 'timerIntervalMs' is not a positive whole number of milliseconds; it is "
+ "discarded and the tag gets NO periodic re-evaluation.",
]);
}
/// <summary>The rejection and the advisories are independent: a blob can draw both, and the rejection
/// always comes first.</summary>
[Fact]
public void A_rejected_blob_still_reports_its_discarded_fields()
{
var warnings = CalculationTagDefinitionFactory.Inspect("""{"changeTriggered":1,"timerIntervalMs":0}""");
warnings.Count.ShouldBe(3);
warnings[0].ShouldStartWith("Calculation TagConfig has no usable 'scriptId'");
warnings[1].ShouldStartWith("value '1' for 'changeTriggered'");
warnings[2].ShouldStartWith("value '0' for 'timerIntervalMs'");
}
/// <summary>Falsification control for the "never stricter than the runtime read" invariant: every blob
/// the runtime ACCEPTS but this gate warns about must still be accepted by the runtime read, so an
/// advisory can never be mistaken for a rejection.</summary>
[Theory]
[InlineData("""{"scriptId":"s1","changeTriggered":"yes"}""")]
[InlineData("""{"scriptId":"s1","timerIntervalMs":0}""")]
public void An_advisory_never_means_the_runtime_read_rejects_the_blob(string blob)
{
CalculationTagDefinitionFactory.Inspect(blob).ShouldNotBeEmpty();
CalculationTagDefinitionFactory.TryReadTagConfig(blob, out _).ShouldBeTrue();
}
}