fix(calculation): read cache carries real input status + timestamps
This commit is contained in:
Joseph Doherty
@@ -758,3 +758,15 @@ test could only pass once the production change landed.
|
||||
- `TagState.LastPublishedBad` (two-state) became `HasPublished` + `LastPublishedStatus` (uint), which is
|
||||
what lets a Bad→different-Bad transition republish and what makes `ReadAsync` agree with the
|
||||
subscriber. The evaluator is still handed bare values here — T14 hands it the snapshots.
|
||||
|
||||
### T14 — `CalculationEvaluator` snapshot overload
|
||||
|
||||
- RED: `--filter "…CalculationEvaluatorTests"` ⇒ **build failure**, `CS1503: cannot convert from
|
||||
'Dictionary<string, DataValueSnapshot>' to 'IReadOnlyDictionary<string, object?>'` (×3) — no snapshot
|
||||
overload existed.
|
||||
- GREEN: `Driver.Calculation.Tests` 64/64.
|
||||
- `BuildReadCache` is **deleted**, not fixed: the snapshots now pass straight through as the read cache.
|
||||
The value-only overload survives (tests use it) but is now the one that fabricates Good, and its
|
||||
xmldoc says so and points at the snapshot overload.
|
||||
- Docs: `docs/drivers/Calculation.md` gains an "Input quality" section (the policy table + the
|
||||
behaviour change for null-tolerant scripts) and the dedup bullet now says **(value, status)**.
|
||||
|
||||
@@ -102,7 +102,7 @@
|
||||
{
|
||||
"id": "T14",
|
||||
"subject": "CalculationEvaluator snapshot overload: read cache carries real input status + timestamps; passthrough fast path on snapshots",
|
||||
"status": "pending",
|
||||
"status": "done",
|
||||
"blockedBy": [
|
||||
"T13"
|
||||
]
|
||||
|
||||
@@ -81,10 +81,43 @@ Two deploy-time gates run over the result:
|
||||
- **Nothing publishes until every declared dependency has arrived at least once.** Before that a read
|
||||
returns `BadWaitingForInitialData`. This is the same gate `VirtualTagActor` uses, and it exists so a
|
||||
calculation cannot publish a confident-looking value derived from defaults.
|
||||
- **Equal results are deduped** — an evaluation producing the same value as last time does not
|
||||
re-publish.
|
||||
- **Equal results are deduped on (value, *status*)** — an evaluation producing the same value *and*
|
||||
the same status as last time does not re-publish. The status half is load-bearing: a device that
|
||||
comes back after an outage usually comes back holding the reading it had before, so a value-only
|
||||
dedup would swallow the recovery and leave the node Bad forever.
|
||||
- **Timers are grouped by interval**, one timer per distinct interval, not one per tag.
|
||||
|
||||
## Input quality
|
||||
|
||||
The contract is stated once, in
|
||||
[`docs/DriverLifecycle.md` §"The quality contract"](../DriverLifecycle.md#the-quality-contract): a
|
||||
value consumer never reports a quality better than its source's. A calc tag's inputs are other tags'
|
||||
live values, so its output inherits their quality:
|
||||
|
||||
| Worst input status | What the driver does | What the calc tag publishes |
|
||||
|---|---|---|
|
||||
| Good | evaluates | the computed value, `Good` |
|
||||
| Uncertain (any sub-code) | **evaluates** — an Uncertain value is present and usable | the computed value, stamped with that Uncertain sub-code |
|
||||
| Bad (any sub-code) | **holds** — the script does not run | the **last known value**, stamped with that input's Bad sub-code, once per change of status |
|
||||
| a dependency never pushed | holds (the pre-existing warm-up gate) | nothing — the node stays `BadWaitingForInitialData` |
|
||||
|
||||
Notes worth knowing before authoring against this:
|
||||
|
||||
- **The sub-code propagates**, so the output says *why*: `BadNoCommunication` means go look at the
|
||||
device feeding the input; `BadInternalError` means this tag's own script failed.
|
||||
- **Holding on Bad is what removes the NRE storm.** A comms-lost input used to arrive as *Good, null*,
|
||||
so `(double)ctx.GetTag("x").Value` threw on every trigger and filled `/script-log` with identical
|
||||
"script threw" entries. Now the script simply is not run, and **no script-log entry is written** — an
|
||||
input dying is not a script fault.
|
||||
- ⚠️ **A script that relied on receiving `null` for a dead input no longer runs at all** while that
|
||||
input is Bad. Rewrite such a script to assume its inputs are usable; the driver has already decided
|
||||
they are.
|
||||
- **`ctx.GetTag(path).StatusCode` and `.SourceTimestampUtc` are real.** The read cache used to stamp
|
||||
every input `Good` at evaluation time; it now passes the input's own snapshot through. Since Bad
|
||||
inputs never reach a script, what a script can observe is Good or Uncertain.
|
||||
- **`ReadAsync` reports the same status the subscription carried**, so a polled read and a subscribed
|
||||
client never disagree about the same node.
|
||||
|
||||
## Error semantics
|
||||
|
||||
An evaluator failure publishes **Bad quality carrying the last-known value** — the value is not
|
||||
|
||||
@@ -547,11 +547,9 @@ public sealed class CalculationDriver : IDriver, IDependencyConsumer, ISubscriba
|
||||
return;
|
||||
}
|
||||
|
||||
// Projected to bare values for now — the evaluator's own read cache still fabricates Good, which
|
||||
// the next task fixes by handing it the snapshots. Only reachable with USABLE inputs (the Bad
|
||||
// hold above returned), so nothing Bad is being flattened here.
|
||||
var depValues = pending.Dependencies.ToDictionary(kv => kv.Key, kv => kv.Value.Value, StringComparer.Ordinal);
|
||||
var result = _evaluator.Evaluate(def.RawPath, def.ScriptSource, depValues);
|
||||
// Snapshots, not bare values: the script's ctx.GetTag(path).StatusCode / .SourceTimestampUtc are
|
||||
// then the input's real ones rather than a fabricated Good stamped at evaluation time (05/STAB-19).
|
||||
var result = _evaluator.Evaluate(def.RawPath, def.ScriptSource, pending.Dependencies);
|
||||
|
||||
string? failureReason = null;
|
||||
(object? Value, uint StatusCode)? toPublish;
|
||||
|
||||
@@ -75,7 +75,44 @@ public sealed class CalculationEvaluator : IScriptCacheOwner, IDisposable
|
||||
/// <param name="calcTagId">Identity of the calc tag (bound to the script log; in the live path equals the script id).</param>
|
||||
/// <param name="scriptSource">The C# script source producing the tag's value.</param>
|
||||
/// <param name="dependencies">Read-only snapshot of dependency values keyed by RawPath.</param>
|
||||
/// <remarks>
|
||||
/// <b>Value-only overload.</b> Every dependency reaches the script stamped <c>Good</c> with an
|
||||
/// evaluation-time timestamp, because bare values carry nothing better. Prefer
|
||||
/// <see cref="Evaluate(string, string, IReadOnlyDictionary{string, DataValueSnapshot})"/>, which the
|
||||
/// driver uses: it carries each input's real status and source timestamp, so
|
||||
/// <c>ctx.GetTag(path).StatusCode</c> is honest (archreview 05/STAB-19). This overload remains for
|
||||
/// callers that genuinely hold only values — chiefly tests.
|
||||
/// </remarks>
|
||||
public VirtualTagEvalResult Evaluate(string calcTagId, string scriptSource, IReadOnlyDictionary<string, object?> dependencies)
|
||||
{
|
||||
ArgumentNullException.ThrowIfNull(dependencies);
|
||||
var nowUtc = DateTime.UtcNow;
|
||||
var snapshots = new Dictionary<string, DataValueSnapshot>(dependencies.Count, StringComparer.Ordinal);
|
||||
foreach (var kv in dependencies)
|
||||
snapshots[kv.Key] = new DataValueSnapshot(kv.Value, StatusCode: 0u, nowUtc, nowUtc);
|
||||
|
||||
return Evaluate(calcTagId, scriptSource, snapshots);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Evaluate <paramref name="scriptSource"/> for the calc tag <paramref name="calcTagId"/> against
|
||||
/// the dependency <b>snapshots</b> in <paramref name="dependencies"/> — value, status code and
|
||||
/// source timestamp per input. Never throws; script faults surface as
|
||||
/// <see cref="VirtualTagEvalResult.Failure"/>.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// The snapshots pass <b>through</b> to the script's read cache rather than being re-wrapped as
|
||||
/// Good (archreview 05/STAB-19). The old wrapping meant a script that inspected
|
||||
/// <c>ctx.GetTag(path).StatusCode</c> was told Good for every input, including one whose driver had
|
||||
/// stopped communicating — so a quality-aware script could not be written at all. The driver holds
|
||||
/// evaluation entirely while any input is Bad, so what a script sees here is Good or Uncertain.
|
||||
/// </remarks>
|
||||
/// <param name="calcTagId">Identity of the calc tag (bound to the script log).</param>
|
||||
/// <param name="scriptSource">The C# script source producing the tag's value.</param>
|
||||
/// <param name="dependencies">Read-only snapshot of dependency values keyed by RawPath.</param>
|
||||
/// <returns>The evaluation outcome.</returns>
|
||||
public VirtualTagEvalResult Evaluate(
|
||||
string calcTagId, string scriptSource, IReadOnlyDictionary<string, DataValueSnapshot> dependencies)
|
||||
{
|
||||
if (_disposed) return VirtualTagEvalResult.Failure("evaluator disposed");
|
||||
if (string.IsNullOrWhiteSpace(scriptSource)) return VirtualTagEvalResult.Failure("empty expression");
|
||||
@@ -86,12 +123,15 @@ public sealed class CalculationEvaluator : IScriptCacheOwner, IDisposable
|
||||
// returns null — Ok(null) here too. Near-misses fall through to the compiled path.
|
||||
if (PassthroughScript.TryMatch(scriptSource, out var passthroughRef))
|
||||
{
|
||||
return dependencies.TryGetValue(passthroughRef, out var ptValue)
|
||||
? VirtualTagEvalResult.Ok(ptValue)
|
||||
// The VALUE only — the caller stamps the output with the worst input status, which for a
|
||||
// single-dependency passthrough IS this input's status. Returning it here as well would put
|
||||
// the same decision in two places.
|
||||
return dependencies.TryGetValue(passthroughRef, out var ptSnapshot)
|
||||
? VirtualTagEvalResult.Ok(ptSnapshot.Value)
|
||||
: VirtualTagEvalResult.Ok(null);
|
||||
}
|
||||
|
||||
var readCache = BuildReadCache(dependencies);
|
||||
var readCache = dependencies;
|
||||
// Per-evaluation script logger: bind both ScriptId and CalcTagId (via the VirtualTagId slot, which
|
||||
// the Script-log page attributes on) from the calc-tag id so each line is attributable.
|
||||
var scriptLog = _scriptRoot
|
||||
@@ -173,20 +213,6 @@ public sealed class CalculationEvaluator : IScriptCacheOwner, IDisposable
|
||||
/// <inheritdoc />
|
||||
public void ClearCompiledScripts() => _cache.Clear();
|
||||
|
||||
private static IReadOnlyDictionary<string, DataValueSnapshot> BuildReadCache(
|
||||
IReadOnlyDictionary<string, object?> deps)
|
||||
{
|
||||
// VirtualTagContext.GetTag returns a DataValueSnapshot — wrap each raw dep value as Good-quality
|
||||
// so the script's `(int)ctx.GetTag("a").Value` pattern works. Null values stay null.
|
||||
var nowUtc = DateTime.UtcNow;
|
||||
var cache = new Dictionary<string, DataValueSnapshot>(StringComparer.Ordinal);
|
||||
foreach (var kv in deps)
|
||||
{
|
||||
cache[kv.Key] = new DataValueSnapshot(kv.Value, StatusCode: 0u, SourceTimestampUtc: nowUtc, ServerTimestampUtc: nowUtc);
|
||||
}
|
||||
return cache;
|
||||
}
|
||||
|
||||
/// <summary>Disposes the evaluator, draining + disposing every cached compile's collectible ALC.</summary>
|
||||
public void Dispose()
|
||||
{
|
||||
|
||||
@@ -4,6 +4,7 @@ using Microsoft.Extensions.Logging.Abstractions;
|
||||
using Serilog;
|
||||
using Shouldly;
|
||||
using Xunit;
|
||||
using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
|
||||
using ZB.MOM.WW.OtOpcUa.Core.Scripting;
|
||||
|
||||
namespace ZB.MOM.WW.OtOpcUa.Driver.Calculation.Tests;
|
||||
@@ -230,6 +231,74 @@ public sealed class CalculationEvaluatorTests
|
||||
return (int)countProp.GetValue(cache)!;
|
||||
}
|
||||
|
||||
// ── input quality reaches the script (archreview 05/STAB-19) ────────────────────────────
|
||||
|
||||
/// <summary>
|
||||
/// A script reading <c>ctx.GetTag(path).StatusCode</c> sees the input's <b>real</b> status.
|
||||
/// The read cache used to stamp every dependency Good, so a script that checked quality was lied
|
||||
/// to — and, since the driver now holds on Bad, an Uncertain input is precisely the case where a
|
||||
/// script can still make a qualified decision and needs the truth to do it.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void The_read_cache_carries_the_inputs_real_status_code()
|
||||
{
|
||||
const uint uncertain = 0x40000000u;
|
||||
using var sut = new CalculationEvaluator(NullLogger<CalculationEvaluator>.Instance, NoOpScriptRoot());
|
||||
var sourceStamp = new DateTime(2026, 8, 20, 9, 0, 0, DateTimeKind.Utc);
|
||||
|
||||
var result = sut.Evaluate(
|
||||
calcTagId: "calc-quality",
|
||||
scriptSource: "return (long)ctx.GetTag(\"a\").StatusCode;",
|
||||
dependencies: new Dictionary<string, DataValueSnapshot>
|
||||
{
|
||||
["a"] = new(7.0, uncertain, sourceStamp, sourceStamp),
|
||||
});
|
||||
|
||||
result.Success.ShouldBeTrue(result.Reason);
|
||||
result.Value.ShouldBe((long)uncertain);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// The dependency's own source timestamp reaches the script too. The old cache stamped
|
||||
/// evaluation time on every input, so a script comparing input ages compared its own clock with
|
||||
/// itself.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void The_read_cache_carries_the_inputs_real_source_timestamp()
|
||||
{
|
||||
using var sut = new CalculationEvaluator(NullLogger<CalculationEvaluator>.Instance, NoOpScriptRoot());
|
||||
var sourceStamp = new DateTime(2026, 8, 20, 9, 0, 0, DateTimeKind.Utc);
|
||||
|
||||
var result = sut.Evaluate(
|
||||
calcTagId: "calc-stamp",
|
||||
scriptSource: "return ctx.GetTag(\"a\").SourceTimestampUtc;",
|
||||
dependencies: new Dictionary<string, DataValueSnapshot>
|
||||
{
|
||||
["a"] = new(7.0, 0u, sourceStamp, DateTime.UtcNow),
|
||||
});
|
||||
|
||||
result.Success.ShouldBeTrue(result.Reason);
|
||||
result.Value.ShouldBe(sourceStamp);
|
||||
}
|
||||
|
||||
/// <summary>The passthrough fast path takes the same snapshot input and returns its value.</summary>
|
||||
[Fact]
|
||||
public void The_passthrough_fast_path_accepts_snapshots()
|
||||
{
|
||||
using var sut = new CalculationEvaluator(NullLogger<CalculationEvaluator>.Instance, NoOpScriptRoot());
|
||||
|
||||
var result = sut.Evaluate(
|
||||
calcTagId: "calc-mirror",
|
||||
scriptSource: "return ctx.GetTag(\"a\").Value;",
|
||||
dependencies: new Dictionary<string, DataValueSnapshot>
|
||||
{
|
||||
["a"] = new(42, 0u, DateTime.UtcNow, DateTime.UtcNow),
|
||||
});
|
||||
|
||||
result.Success.ShouldBeTrue(result.Reason);
|
||||
result.Value.ShouldBe(42);
|
||||
}
|
||||
|
||||
/// <summary>Minimal <see cref="ILogger{T}"/> that records emitted entries so the SetVirtualTag-drop
|
||||
/// test can assert the Debug "dropped" line was logged.</summary>
|
||||
private sealed class CapturingLogger : ILogger<CalculationEvaluator>
|
||||
|
||||
Reference in New Issue
Block a user