perf(site-runtime): per-attribute child routing in InstanceActor — attribute changes reach only interested children (P2)

2026-07-09 00:55:26 -04:00
parent 1673268aee
commit d124025cc7
4 changed files with 343 additions and 16 deletions
@@ -53,6 +53,25 @@ public class InstanceActor : ReceiveActor
    private readonly Dictionary<string, IActorRef> _scriptActors = new();
    private readonly Dictionary<string, IActorRef> _alarmActors = new();
    private readonly Dictionary<string, IActorRef> _nativeAlarmActors = new();
+
+    /// <summary>
+    /// 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
+    /// <see cref="CreateChildActors"/> from each child's trigger config and consulted by
+    /// <see cref="PublishAndNotifyChildren"/> 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.
+    /// </summary>
+    private readonly Dictionary<string, List<IActorRef>> _childrenByMonitoredAttribute = new();
+
+    /// <summary>
+    /// 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 <see cref="_childrenByMonitoredAttribute"/>.
+    /// </summary>
+    private readonly List<IActorRef> _allChangeChildren = new();
    /// <summary>
    /// Native alarm kind per source-binding canonical name (the same value handed
    /// to each <see cref="NativeAlarmActor"/>'s <c>_nativeKind</c>). 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 <c>HandleAttributeValueChanged</c>.
    /// </summary>
+    /// <summary>
+    /// 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.
+    /// </summary>
+    private void RouteChild(IActorRef child, TriggerRoutingDecision decision)
+    {
+        switch (decision.Kind)
+        {
+            case TriggerRoutingKind.MonitoredAttribute:
+                if (!_childrenByMonitoredAttribute.TryGetValue(decision.MonitoredAttribute!, out var list))
+                {
+                    list = new List<IActorRef>();
+                    _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
    /// </summary>
    public int AlarmActorCount => _alarmActors.Count;

+    /// <summary>
+    /// Read-only view of the per-attribute child routing map (P2, for testing/diagnostics).
+    /// </summary>
+    internal IReadOnlyDictionary<string, List<IActorRef>> ChildrenByMonitoredAttribute => _childrenByMonitoredAttribute;
+
+    /// <summary>
+    /// Read-only view of the all-changes child routing bucket (P2, for testing/diagnostics).
+    /// </summary>
+    internal IReadOnlyList<IActorRef> AllChangeChildren => _allChangeChildren;
+
    /// <summary>
    /// Internal message for async override loading result.
    /// </summary>
@@ -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(
@@ -0,0 +1,122 @@
+using System.Text.Json;
+using ZB.MOM.WW.ScadaBridge.Commons.Types.Enums;
+
+namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Scripts;
+
+/// <summary>How an attribute change should be routed to a child (script/alarm) actor.</summary>
+internal enum TriggerRoutingKind
+{
+    /// <summary>Reacts only to changes of a single named attribute.</summary>
+    MonitoredAttribute,
+
+    /// <summary>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).</summary>
+    AllChanges,
+
+    /// <summary>Never reacts to attribute changes (interval / call-only scripts).</summary>
+    None
+}
+
+/// <summary>
+/// Result of classifying a child's trigger for attribute-change routing.
+/// </summary>
+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);
+}
+
+/// <summary>
+/// Single source of truth for the monitored attribute a script/alarm trigger reacts to (P2).
+/// Shared by <see cref="ScriptActor"/> (attribute-name extraction) and
+/// <c>InstanceActor.CreateChildActors</c> (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
+/// <see cref="TriggerRoutingDecision"/> rather than the plan's original bool-out signature —
+/// a bool cannot distinguish "hears everything" from "hears nothing".
+/// </summary>
+internal static class TriggerRouting
+{
+    /// <summary>
+    /// Reads the monitored attribute name from a trigger config, honoring both the
+    /// <c>"attributeName"</c> (scripts + alarms) and <c>"attribute"</c> (alarm alias) keys.
+    /// Returns false when the JSON is missing, malformed, or carries no attribute name.
+    /// </summary>
+    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;
+        }
+    }
+
+    /// <summary>
+    /// 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).
+    /// </summary>
+    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;
+        }
+    }
+
+    /// <summary>
+    /// 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).
+    /// </summary>
+    public static TriggerRoutingDecision ForAlarm(string? triggerType, string? triggerConfigJson)
+    {
+        if (Enum.TryParse<AlarmTriggerType>(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;
+    }
+}
@@ -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;
+
+/// <summary>
+/// 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.
+/// </summary>
+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<SiteStorageService>.Instance);
+        _storage.InitializeAsync().GetAwaiter().GetResult();
+        _compilationService = new ScriptCompilationService(
+            NullLogger<ScriptCompilationService>.Instance);
+        _sharedScriptLibrary = new SharedScriptLibrary(
+            _compilationService, NullLogger<SharedScriptLibrary>.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<InstanceActor>(
+            Props.Create(() => new InstanceActor(
+                config.InstanceUniqueName,
+                JsonSerializer.Serialize(config),
+                _storage,
+                _compilationService,
+                _sharedScriptLibrary,
+                null,
+                _options,
+                NullLogger<InstanceActor>.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
+    }
+}