perf(adminui): cache the modals' DynamicComponent parameter dictionaries (R04 H2)

DriverConfigModal, DeviceModal and RawTagModal built a new parameter dictionary and new EventCallbacks every render. ParameterDictionaryCache reuses the dictionary while the values it carries are unchanged and rebuilds it with the current value when one changes (a build-once cache would strand edits); the callbacks are created once. Blazor still re-sets an EventCallback-bearing child's parameters per parent render, so the forms' _lastParsed guards stay. Refs #745 Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT
2026-10-22 17:43:01 -04:00
parent 36e6ab590b
commit b0959f697b
5 changed files with 185 additions and 26 deletions
@@ -108,13 +108,19 @@
    private bool _enabled = true;
    private string _deviceConfigJson = "{}";

-    /// <summary>Parameters handed to the DynamicComponent-rendered device form. Rebuilt on every render —
-    /// see DriverConfigModal for why this must not be cached.</summary>
-    private Dictionary<string, object> _deviceFormParameters => new()
-    {
-        ["DeviceConfigJson"] = _deviceConfigJson,
-        ["DeviceConfigJsonChanged"] = EventCallback.Factory.Create<string>(this, v => _deviceConfigJson = v),
-    };
+    /// <summary>Parameters handed to the DynamicComponent-rendered device form. Cached on the value it
+    /// carries and rebuilt when that changes (never build-once: see DriverConfigModal), with the callback
+    /// created once (R04 H2).</summary>
+    private Dictionary<string, object> _deviceFormParameters => _deviceFormParameterCache.Get(
+        [_deviceConfigJson],
+        () => new()
+        {
+            ["DeviceConfigJson"] = _deviceConfigJson,
+            ["DeviceConfigJsonChanged"] = _deviceConfigJsonChanged ??=
+                EventCallback.Factory.Create<string>(this, v => _deviceConfigJson = v),
+        });
+    private readonly ParameterDictionaryCache _deviceFormParameterCache = new();
+    private EventCallback<string>? _deviceConfigJsonChanged;
    private byte[] _rowVersion = [];
    private string _parentDriverConfig = "{}";

@@ -111,16 +111,24 @@
    private string _name = "";
    private string _driverConfigJson = "{}";

-    /// <summary>Parameters handed to the DynamicComponent-rendered driver form. Rebuilt on every render so
-    /// the child receives the CURRENT json — a cached dictionary would pin the value captured at first
-    /// render and silently strand every subsequent edit.</summary>
-    private Dictionary<string, object> _formParameters => new()
-    {
-        ["DriverConfigJson"] = _driverConfigJson,
-        ["DriverConfigJsonChanged"] = EventCallback.Factory.Create<string>(this, v => _driverConfigJson = v),
-        ["ResilienceConfig"] = _resilienceConfig!,
-        ["ResilienceConfigChanged"] = EventCallback.Factory.Create<string?>(this, v => _resilienceConfig = v),
-    };
+    /// <summary>Parameters handed to the DynamicComponent-rendered driver form. The child must receive the
+    /// CURRENT json — a build-once dictionary would pin the value captured at first render and silently
+    /// strand every subsequent edit — so the cache is keyed on the values it carries and rebuilt only when
+    /// one changes; the callbacks are created once and reused (R04 H2).</summary>
+    private Dictionary<string, object> _formParameters => _formParameterCache.Get(
+        [_driverConfigJson, _resilienceConfig],
+        () => new()
+        {
+            ["DriverConfigJson"] = _driverConfigJson,
+            ["DriverConfigJsonChanged"] = _driverConfigJsonChanged ??=
+                EventCallback.Factory.Create<string>(this, v => _driverConfigJson = v),
+            ["ResilienceConfig"] = _resilienceConfig!,
+            ["ResilienceConfigChanged"] = _resilienceConfigChanged ??=
+                EventCallback.Factory.Create<string?>(this, v => _resilienceConfig = v),
+        });
+    private readonly ParameterDictionaryCache _formParameterCache = new();
+    private EventCallback<string>? _driverConfigJsonChanged;
+    private EventCallback<string?>? _resilienceConfigChanged;
    private string? _resilienceConfig;
    private byte[] _rowVersion = [];

@@ -0,0 +1,50 @@
+namespace ZB.MOM.WW.OtOpcUa.AdminUI.Components.Shared;
+
+/// <summary>
+///     Caches a <c>DynamicComponent</c> parameter dictionary (R04 H2) and rebuilds it only when one of the
+///     values it carries changed. The modals used to build a new dictionary, and new
+///     <c>EventCallback</c>s, on every render. A plain build-once cache is wrong: it would pin the JSON
+///     captured at first render and silently strand every later edit — so the cache is keyed on the
+///     current values, and the callbacks it hands out are created once by the caller and reused.
+///     <para>Note the limit: Blazor's change detection treats any <c>EventCallback</c> parameter as
+///     possibly changed, so a hosted form still sees <c>SetParametersAsync</c> on each parent render. The
+///     per-form <c>_lastParsed</c> guards remain load-bearing; this removes the per-render allocations and
+///     gives the child a stable dictionary and callback identity.</para>
+/// </summary>
+public sealed class ParameterDictionaryCache
+{
+    private object?[]? _keyValues;
+    private Dictionary<string, object>? _cached;
+
+    /// <summary>Returns the cached dictionary when <paramref name="keyValues"/> equal the previous call's,
+    /// otherwise builds, caches and returns a new one.</summary>
+    /// <param name="keyValues">The values the dictionary carries (compared with <see cref="object.Equals(object, object)"/>).</param>
+    /// <param name="build">Builds the dictionary from the current state.</param>
+    /// <returns>The parameter dictionary.</returns>
+    public Dictionary<string, object> Get(object?[] keyValues, Func<Dictionary<string, object>> build)
+    {
+        if (_cached is not null && _keyValues is not null && Same(_keyValues, keyValues)) return _cached;
+
+        _cached = build();
+        _keyValues = (object?[])keyValues.Clone();
+        return _cached;
+    }
+
+    /// <summary>Drops the cached dictionary (e.g. on modal close) so the next <see cref="Get"/> rebuilds.</summary>
+    public void Reset()
+    {
+        _cached = null;
+        _keyValues = null;
+    }
+
+    private static bool Same(object?[] a, object?[] b)
+    {
+        if (a.Length != b.Length) return false;
+        for (var i = 0; i < a.Length; i++)
+        {
+            if (!Equals(a[i], b[i])) return false;
+        }
+
+        return true;
+    }
+}
@@ -217,15 +217,23 @@
        }
    }

-    /// <summary>Parameters handed to the typed TagConfig editor. Internal so tests can assert them.</summary>
-    internal IDictionary<string, object> BuildEditorParameters() => new Dictionary<string, object>
-    {
-        ["ConfigJson"] = _form.TagConfig,
-        ["ConfigJsonChanged"] = EventCallback.Factory.Create<string>(this, v => _form.TagConfig = v),
-        ["DriverType"] = DriverType ?? "",
-        // Live accessor, not a snapshot: the picker reads it when the operator clicks Browse.
-        ["GetDriverConfigJson"] = (Func<string>)(() => _mergedConfigJson ?? "{}"),
-    };
+    /// <summary>Parameters handed to the typed TagConfig editor. Internal so tests can assert them. Cached
+    /// on the values it carries and rebuilt when one changes (never build-once, which would pin the first
+    /// render's TagConfig); the callback and accessor are created once (R04 H2).</summary>
+    internal IDictionary<string, object> BuildEditorParameters() => _editorParameterCache.Get(
+        [_form.TagConfig, DriverType ?? ""],
+        () => new Dictionary<string, object>
+        {
+            ["ConfigJson"] = _form.TagConfig,
+            ["ConfigJsonChanged"] = _configJsonChanged ??=
+                EventCallback.Factory.Create<string>(this, v => _form.TagConfig = v),
+            ["DriverType"] = DriverType ?? "",
+            // Live accessor, not a snapshot: the picker reads it when the operator clicks Browse.
+            ["GetDriverConfigJson"] = _getDriverConfigJson ??= () => _mergedConfigJson ?? "{}",
+        });
+    private readonly ParameterDictionaryCache _editorParameterCache = new();
+    private EventCallback<string>? _configJsonChanged;
+    private Func<string>? _getDriverConfigJson;

    private async Task SaveAsync()
    {
@@ -0,0 +1,87 @@
+using Microsoft.AspNetCore.Components;
+using Shouldly;
+using Xunit;
+using ZB.MOM.WW.OtOpcUa.AdminUI.Components.Shared;
+
+namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests.Raw;
+
+/// <summary>
+///     R04 H2: the modals' DynamicComponent parameter dictionaries are reused across renders while their
+///     values are unchanged, and rebuilt — carrying the current value — the moment one changes. The second
+///     half is the regression a naive build-once cache would introduce (stale JSON stranding every edit).
+/// </summary>
+[Trait("Category", "Unit")]
+public sealed class ParameterDictionaryCacheTests
+{
+    [Fact]
+    public void Unchanged_values_return_the_same_dictionary()
+    {
+        var cache = new ParameterDictionaryCache();
+        var builds = 0;
+        var json = "{}";
+        Dictionary<string, object> Build() { builds++; return new() { ["Json"] = json }; }
+
+        var first = cache.Get([json], Build);
+        cache.Get([json], Build).ShouldBeSameAs(first);
+        builds.ShouldBe(1);
+    }
+
+    [Fact]
+    public void A_changed_value_rebuilds_with_the_current_value()
+    {
+        var cache = new ParameterDictionaryCache();
+        var json = "{}";
+        Dictionary<string, object> Build() => new() { ["Json"] = json };
+
+        cache.Get([json], Build)["Json"].ShouldBe("{}");
+        json = "{\"a\":1}";
+        cache.Get([json], Build)["Json"].ShouldBe("{\"a\":1}");
+    }
+
+    [Fact]
+    public void Null_values_compare_and_reset_forces_a_rebuild()
+    {
+        var cache = new ParameterDictionaryCache();
+        var builds = 0;
+        Dictionary<string, object> Build() { builds++; return new(); }
+
+        cache.Get(["x", null], Build);
+        cache.Get(["x", null], Build);
+        builds.ShouldBe(1);
+
+        cache.Reset();
+        cache.Get(["x", null], Build);
+        builds.ShouldBe(2);
+    }
+
+    [Fact]
+    public void Modals_build_their_parameters_through_the_cache_with_reused_callbacks()
+    {
+        foreach (var (path, cacheField) in new[]
+                 {
+                     (new[] { "Components", "Shared", "Drivers", "DriverConfigModal.razor" }, "_formParameterCache.Get("),
+                     (new[] { "Components", "Shared", "Drivers", "DeviceModal.razor" }, "_deviceFormParameterCache.Get("),
+                     (new[] { "Components", "Shared", "Raw", "RawTagModal.razor" }, "_editorParameterCache.Get("),
+                 })
+        {
+            var src = AdminUiSource.Read(path);
+            src.ShouldContain(cacheField, customMessage: path[^1]);
+            // Every EventCallback is created behind a ??= so it is made once per component.
+            var all = System.Text.RegularExpressions.Regex.Matches(src, @"EventCallback\.Factory\.Create").Count;
+            var once = System.Text.RegularExpressions.Regex.Matches(src, @"\?\?=\s*EventCallback\.Factory\.Create").Count;
+            all.ShouldBeGreaterThan(0, path[^1]);
+            once.ShouldBe(all, path[^1]);
+        }
+    }
+
+    [Fact]
+    public void RawTagModal_reuses_its_callback_across_renders()
+    {
+        var modal = new ZB.MOM.WW.OtOpcUa.AdminUI.Components.Shared.Raw.RawTagModal();
+        var first = modal.BuildEditorParameters();
+        var second = modal.BuildEditorParameters();
+
+        second.ShouldBeSameAs(first);
+        ((EventCallback<string>)first["ConfigJsonChanged"]).HasDelegate.ShouldBeTrue();
+    }
+}