From d124025cc70e06988b1646bcb22258fc40095241 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 9 Jul 2026 00:55:26 -0400 Subject: [PATCH] =?UTF-8?q?perf(site-runtime):=20per-attribute=20child=20r?= =?UTF-8?q?outing=20in=20InstanceActor=20=E2=80=94=20attribute=20changes?= =?UTF-8?q?=20reach=20only=20interested=20children=20(P2)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../Actors/InstanceActor.cs | 76 ++++++++- .../Actors/ScriptActor.cs | 16 +- .../Scripts/TriggerRouting.cs | 122 +++++++++++++++ .../Actors/InstanceActorChildRoutingTests.cs | 145 ++++++++++++++++++ 4 files changed, 343 insertions(+), 16 deletions(-) create mode 100644 src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/TriggerRouting.cs create mode 100644 tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/InstanceActorChildRoutingTests.cs diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/InstanceActor.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/InstanceActor.cs index 46590a68..ea42bb6e 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/InstanceActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/InstanceActor.cs @@ -53,6 +53,25 @@ public class InstanceActor : ReceiveActor private readonly Dictionary _scriptActors = new(); private readonly Dictionary _alarmActors = new(); private readonly Dictionary _nativeAlarmActors = new(); + + /// + /// Per-attribute child routing (P2): monitoredAttributeName → the script/alarm actors + /// that react only to that attribute (value-change / conditional scripts; + /// HiLo/ValueMatch/RangeViolation/RateOfChange alarms). Built once in + /// from each child's trigger config and consulted by + /// so an attribute change reaches only the + /// children interested in it instead of every child. The child's own trigger gate + /// remains authoritative — this routing is a fan-out optimization, not the correctness + /// check. + /// + private readonly Dictionary> _childrenByMonitoredAttribute = new(); + + /// + /// Children that must hear EVERY attribute change (P2): Expression-triggered scripts and + /// alarms (they keep a full attribute snapshot), plus any child whose trigger config was + /// unparseable (fail-open). Complements . + /// + private readonly List _allChangeChildren = new(); /// /// Native alarm kind per source-binding canonical name (the same value handed /// to each 's _nativeKind). Used to stamp @@ -1253,16 +1272,20 @@ public class InstanceActor : ReceiveActor // Publish to site-wide stream _streamManager?.PublishAttributeValueChanged(changed); - // Notify Script Actors (for value-change and conditional triggers) - foreach (var scriptActor in _scriptActors.Values) + // Route the change to exactly the children interested in it (P2): the all-changes + // bucket (Expression scripts/alarms + fail-open children) plus the children monitoring + // this specific attribute. Children still gate on their own trigger config — routing + // is a fan-out optimization, not the correctness check. + foreach (var child in _allChangeChildren) { - scriptActor.Tell(changed); + child.Tell(changed); } - - // Notify Alarm Actors (for alarm evaluation) - foreach (var alarmActor in _alarmActors.Values) + if (_childrenByMonitoredAttribute.TryGetValue(changed.AttributeName, out var interested)) { - alarmActor.Tell(changed); + foreach (var child in interested) + { + child.Tell(changed); + } } // WaitForAttribute (spec §4.2): re-evaluate any waiters on THIS attribute — @@ -1419,6 +1442,33 @@ public class InstanceActor : ReceiveActor /// would let a child constructor enumerate it on the child's mailbox thread while /// this actor mutates it in HandleAttributeValueChanged. /// + /// + /// Files a freshly-created child actor into the attribute-change routing maps (P2) + /// according to its trigger classification. Native alarm actors are NOT routed here — + /// they subscribe to their own native feed through the DCL and never react to computed + /// attribute changes. + /// + private void RouteChild(IActorRef child, TriggerRoutingDecision decision) + { + switch (decision.Kind) + { + case TriggerRoutingKind.MonitoredAttribute: + if (!_childrenByMonitoredAttribute.TryGetValue(decision.MonitoredAttribute!, out var list)) + { + list = new List(); + _childrenByMonitoredAttribute[decision.MonitoredAttribute!] = list; + } + list.Add(child); + break; + case TriggerRoutingKind.AllChanges: + _allChangeChildren.Add(child); + break; + case TriggerRoutingKind.None: + // Interval / call-only triggers never react to attribute changes. + break; + } + } + private void CreateChildActors() { if (_configuration == null) return; @@ -1463,6 +1513,7 @@ public class InstanceActor : ReceiveActor var actorRef = Context.ActorOf(props, $"script-{script.CanonicalName}"); _scriptActors[script.CanonicalName] = actorRef; + RouteChild(actorRef, TriggerRouting.ForScript(script.TriggerType, script.TriggerConfiguration)); } // Create Alarm Actors @@ -1522,6 +1573,7 @@ public class InstanceActor : ReceiveActor _alarmActors[alarm.CanonicalName] = actorRef; _alarmPriorities[alarm.CanonicalName] = alarm.PriorityLevel; _alarmTimestamps[alarm.CanonicalName] = DateTimeOffset.UtcNow; + RouteChild(actorRef, TriggerRouting.ForAlarm(alarm.TriggerType, alarm.TriggerConfiguration)); } // Create Native Alarm Actors — read-only mirror of each bound source's @@ -1620,6 +1672,16 @@ public class InstanceActor : ReceiveActor /// public int AlarmActorCount => _alarmActors.Count; + /// + /// Read-only view of the per-attribute child routing map (P2, for testing/diagnostics). + /// + internal IReadOnlyDictionary> ChildrenByMonitoredAttribute => _childrenByMonitoredAttribute; + + /// + /// Read-only view of the all-changes child routing bucket (P2, for testing/diagnostics). + /// + internal IReadOnlyList AllChangeChildren => _allChangeChildren; + /// /// Internal message for async override loading result. /// diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/ScriptActor.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/ScriptActor.cs index 32e6d0c1..11be2a59 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/ScriptActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/ScriptActor.cs @@ -563,23 +563,21 @@ public class ScriptActor : ReceiveActor, IWithTimers private static ValueChangeTriggerConfig? ParseValueChangeTrigger(string? json) { - if (string.IsNullOrEmpty(json)) return null; - try - { - var doc = JsonDocument.Parse(json); - var attr = doc.RootElement.GetProperty("attributeName").GetString()!; - return new ValueChangeTriggerConfig(attr); - } - catch { return null; } + // Share the monitored-attribute parser with InstanceActor's routing map (P2) so both + // agree on which attribute this trigger reacts to. + return TriggerRouting.TryReadAttributeName(json, out var attr) + ? new ValueChangeTriggerConfig(attr) + : null; } private static ConditionalTriggerConfig? ParseConditionalTrigger(string? json) { if (string.IsNullOrEmpty(json)) return null; + // Attribute name comes from the shared parser (P2); operator/threshold/mode stay local. + if (!TriggerRouting.TryReadAttributeName(json, out var attr)) return null; try { var doc = JsonDocument.Parse(json); - var attr = doc.RootElement.GetProperty("attributeName").GetString()!; var op = doc.RootElement.GetProperty("operator").GetString()!; var threshold = doc.RootElement.GetProperty("threshold").GetDouble(); return new ConditionalTriggerConfig( diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/TriggerRouting.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/TriggerRouting.cs new file mode 100644 index 00000000..034af596 --- /dev/null +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/TriggerRouting.cs @@ -0,0 +1,122 @@ +using System.Text.Json; +using ZB.MOM.WW.ScadaBridge.Commons.Types.Enums; + +namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Scripts; + +/// How an attribute change should be routed to a child (script/alarm) actor. +internal enum TriggerRoutingKind +{ + /// Reacts only to changes of a single named attribute. + MonitoredAttribute, + + /// Reacts to ANY attribute change (Expression triggers; or fail-open when the + /// trigger config is unparseable — the child's own gate then discards what it ignores). + AllChanges, + + /// Never reacts to attribute changes (interval / call-only scripts). + None +} + +/// +/// Result of classifying a child's trigger for attribute-change routing. +/// +internal readonly record struct TriggerRoutingDecision(TriggerRoutingKind Kind, string? MonitoredAttribute) +{ + public static readonly TriggerRoutingDecision AllChanges = new(TriggerRoutingKind.AllChanges, null); + public static readonly TriggerRoutingDecision None = new(TriggerRoutingKind.None, null); + public static TriggerRoutingDecision Monitored(string attribute) => new(TriggerRoutingKind.MonitoredAttribute, attribute); +} + +/// +/// Single source of truth for the monitored attribute a script/alarm trigger reacts to (P2). +/// Shared by (attribute-name extraction) and +/// InstanceActor.CreateChildActors (routing-map construction) so both agree on the +/// attribute for a given trigger config. +/// +/// NOTE: routing has three outcomes (monitored / all-changes / none), so this returns a +/// rather than the plan's original bool-out signature — +/// a bool cannot distinguish "hears everything" from "hears nothing". +/// +internal static class TriggerRouting +{ + /// + /// Reads the monitored attribute name from a trigger config, honoring both the + /// "attributeName" (scripts + alarms) and "attribute" (alarm alias) keys. + /// Returns false when the JSON is missing, malformed, or carries no attribute name. + /// + public static bool TryReadAttributeName(string? triggerConfigJson, out string attribute) + { + attribute = ""; + if (string.IsNullOrEmpty(triggerConfigJson)) return false; + try + { + using var doc = JsonDocument.Parse(triggerConfigJson); + var root = doc.RootElement; + if (root.TryGetProperty("attributeName", out var a) && a.GetString() is { Length: > 0 } n) + { + attribute = n; + return true; + } + if (root.TryGetProperty("attribute", out var a2) && a2.GetString() is { Length: > 0 } n2) + { + attribute = n2; + return true; + } + return false; + } + catch (JsonException) + { + return false; + } + } + + /// + /// Classifies a script trigger. valuechange/conditional → the named attribute (fail open to + /// all-changes if unparseable); expression → all-changes; interval/call → none; anything + /// unknown → all-changes (fail open — a misconfigured script still reacts, its own gate + /// remains the correctness check). + /// + public static TriggerRoutingDecision ForScript(string? triggerType, string? triggerConfigJson) + { + switch (triggerType?.ToLowerInvariant()) + { + case "valuechange": + case "conditional": + return TryReadAttributeName(triggerConfigJson, out var attr) + ? TriggerRoutingDecision.Monitored(attr) + : TriggerRoutingDecision.AllChanges; + case "expression": + return TriggerRoutingDecision.AllChanges; + case "interval": + case "call": + return TriggerRoutingDecision.None; + default: + return TriggerRoutingDecision.AllChanges; + } + } + + /// + /// Classifies an alarm trigger. Expression → all-changes; HiLo/ValueMatch/RangeViolation/ + /// RateOfChange → the named attribute (fail open to all-changes if unparseable); an + /// unknown/unparseable trigger type → all-changes (fail open). + /// + public static TriggerRoutingDecision ForAlarm(string? triggerType, string? triggerConfigJson) + { + if (Enum.TryParse(triggerType, ignoreCase: true, out var tt)) + { + switch (tt) + { + case AlarmTriggerType.Expression: + return TriggerRoutingDecision.AllChanges; + case AlarmTriggerType.HiLo: + case AlarmTriggerType.ValueMatch: + case AlarmTriggerType.RangeViolation: + case AlarmTriggerType.RateOfChange: + return TryReadAttributeName(triggerConfigJson, out var attr) + ? TriggerRoutingDecision.Monitored(attr) + : TriggerRoutingDecision.AllChanges; + } + } + return TriggerRoutingDecision.AllChanges; + } +} diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/InstanceActorChildRoutingTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/InstanceActorChildRoutingTests.cs new file mode 100644 index 00000000..93b4b1ed --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/InstanceActorChildRoutingTests.cs @@ -0,0 +1,145 @@ +using Akka.Actor; +using Akka.TestKit.Xunit2; +using Microsoft.Extensions.Logging.Abstractions; +using ZB.MOM.WW.ScadaBridge.Commons.Types.Flattening; +using ZB.MOM.WW.ScadaBridge.SiteRuntime.Actors; +using ZB.MOM.WW.ScadaBridge.SiteRuntime.Persistence; +using ZB.MOM.WW.ScadaBridge.SiteRuntime.Scripts; +using System.Text.Json; + +namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.Actors; + +/// +/// PLAN-03 Task 18 (P2): the Instance Actor routes each attribute change only to the +/// children interested in it. A value-change / conditional script (and a monitored-attribute +/// alarm) subscribes to a single named attribute; an Expression-triggered script/alarm hears +/// every change (it keeps a full attribute snapshot). These tests pin the routing-map +/// construction directly — the behavioral gate inside each child stays as defense in depth. +/// +public class InstanceActorChildRoutingTests : TestKit, IDisposable +{ + private readonly SiteStorageService _storage; + private readonly ScriptCompilationService _compilationService; + private readonly SharedScriptLibrary _sharedScriptLibrary; + private readonly SiteRuntimeOptions _options; + private readonly string _dbFile; + + public InstanceActorChildRoutingTests() + { + _dbFile = Path.Combine(Path.GetTempPath(), $"instance-routing-test-{Guid.NewGuid():N}.db"); + _storage = new SiteStorageService( + $"Data Source={_dbFile}", + NullLogger.Instance); + _storage.InitializeAsync().GetAwaiter().GetResult(); + _compilationService = new ScriptCompilationService( + NullLogger.Instance); + _sharedScriptLibrary = new SharedScriptLibrary( + _compilationService, NullLogger.Instance); + _options = new SiteRuntimeOptions + { + MaxScriptCallDepth = 10, + ScriptExecutionTimeoutSeconds = 30 + }; + } + + void IDisposable.Dispose() + { + Shutdown(); + try { File.Delete(_dbFile); } catch { /* cleanup */ } + } + + private static FlattenedConfiguration ScriptsAB_and_Expression(string instanceName) + => new() + { + InstanceUniqueName = instanceName, + Attributes = + [ + new ResolvedAttribute { CanonicalName = "A", Value = "1", DataType = "Double" }, + new ResolvedAttribute { CanonicalName = "B", Value = "2", DataType = "Double" } + ], + Scripts = + [ + new ResolvedScript + { + CanonicalName = "S_A", Code = "return 1;", + TriggerType = "ValueChange", + TriggerConfiguration = "{\"attributeName\":\"A\"}" + }, + new ResolvedScript + { + CanonicalName = "S_B", Code = "return 2;", + TriggerType = "ValueChange", + TriggerConfiguration = "{\"attributeName\":\"B\"}" + }, + new ResolvedScript + { + CanonicalName = "S_expr", Code = "return 3;", + TriggerType = "Expression", + TriggerConfiguration = "{\"expression\":\"true\"}" + } + ] + }; + + private InstanceActor BuildInstance(FlattenedConfiguration config) + { + var testRef = ActorOfAsTestActorRef( + Props.Create(() => new InstanceActor( + config.InstanceUniqueName, + JsonSerializer.Serialize(config), + _storage, + _compilationService, + _sharedScriptLibrary, + null, + _options, + NullLogger.Instance)), + "instance-" + Guid.NewGuid().ToString("N")); + return testRef.UnderlyingActor; + } + + [Fact] + public void RoutingMaps_AreBuiltFromTriggerConfigs() + { + var actor = BuildInstance(ScriptsAB_and_Expression("RoutePump")); + + // Two value-change scripts route to their own attribute; the expression script + // routes to the all-changes bucket. + Assert.Single(actor.ChildrenByMonitoredAttribute["A"]); + Assert.Single(actor.ChildrenByMonitoredAttribute["B"]); + Assert.Single(actor.AllChangeChildren); + } + + [Fact] + public void MonitoredAttributeAlarm_RoutesToItsAttribute_ExpressionAlarm_RoutesToAllChanges() + { + var config = new FlattenedConfiguration + { + InstanceUniqueName = "AlarmRoutePump", + Attributes = + [ + new ResolvedAttribute { CanonicalName = "Temp", Value = "10", DataType = "Double" } + ], + Alarms = + [ + new ResolvedAlarm + { + CanonicalName = "HiTemp", + TriggerType = "RangeViolation", + TriggerConfiguration = "{\"attributeName\":\"Temp\",\"min\":0,\"max\":100}", + PriorityLevel = 1 + }, + new ResolvedAlarm + { + CanonicalName = "ExprAlarm", + TriggerType = "Expression", + TriggerConfiguration = "{\"expression\":\"true\"}", + PriorityLevel = 1 + } + ] + }; + + var actor = BuildInstance(config); + + Assert.Single(actor.ChildrenByMonitoredAttribute["Temp"]); + Assert.Single(actor.AllChangeChildren); // the expression alarm + } +}