fix(r2-02): non-lossy ResilienceFormModel round-trip — preserve unknown keys, never overwrite unparseable stored JSON (04/C-7)

2026-07-13 10:18:31 -04:00
parent 07eec11d31
commit 9e0a05dc6a
3 changed files with 180 additions and 60 deletions
@@ -14,8 +14,8 @@
    { "id": 10, "subject": "U-6 delete (Core): remove BulkheadMaxConcurrent/MaxQueue from DriverResilienceOptions + parser shape/merge/doc-example; add BulkheadKeys_InStoredJson_AreIgnored (migration-free contract)", "status": "completed", "blockedBy": [2, 6] },
    { "id": 11, "subject": "U-6 delete (AdminUI): strip bulkhead fields from ResilienceFormModel + the two razor inputs; update ResilienceFormModelTests", "status": "completed", "blockedBy": [5, 10] },
    { "id": 12, "subject": "U-6 doc sweep (seam xmldocs, OTOPCUA0001 message + analyzer tests, operator docs; leave migrations/WedgeDetector/historical plans) + Options_properties_are_exactly_the_pipeline_wired_set knob-inertness guard", "status": "completed", "blockedBy": [10, 11] },
-    { "id": 13, "subject": "C-7 RED tests: unknown top-level key / unknown capability / unknown per-policy field survive round-trip; malformed JSON sets ParseFailed and ToJson returns the original text", "status": "pending", "blockedBy": [11] },
-    { "id": 14, "subject": "C-7 implement: JsonObject-bag FromJson/ToJson (TagConfigJson idiom), ParseFailed + RawStoredJson, blank->null contract preserved, parser-interop test green", "status": "pending", "blockedBy": [13] },
+    { "id": 13, "subject": "C-7 RED tests: unknown top-level key / unknown capability / unknown per-policy field survive round-trip; malformed JSON sets ParseFailed and ToJson returns the original text", "status": "completed", "blockedBy": [11] },
+    { "id": 14, "subject": "C-7 implement: JsonObject-bag FromJson/ToJson (TagConfigJson idiom), ParseFailed + RawStoredJson, blank->null contract preserved, parser-interop test green", "status": "completed", "blockedBy": [13] },
    { "id": 15, "subject": "C-7 razor: ParseFailed warning banner + input lockout + explicit discard button; raw pane shows the stored ResilienceConfig; live-/run on docker-dev :9200 (rebuild BOTH centrals; SQL-mangle + unknown-key survival checks; also drive task 5 warnings and task 9 disabled cells)", "status": "pending", "blockedBy": [14] },
    { "id": 16, "subject": "S-7 builder: add options-generation to PipelineKey + GetOrCreate(optionsGeneration=0) + thread through CapabilityInvoker; StaleGeneration_ReCache_IsNotServed_ToNewGeneration", "status": "pending", "blockedBy": [3] },
    { "id": 17, "subject": "S-7 factory: Interlocked per-Create generation stamp + Respawn_interleaved_with_old_invoker_call_still_applies_new_options wiring proof; update Invalidate comment (cleanup, not correctness)", "status": "pending", "blockedBy": [16] },
@@ -1,14 +1,22 @@
 using System.Text.Json;
-using System.Text.Json.Serialization;
+using System.Text.Json.Nodes;
 using ZB.MOM.WW.OtOpcUa.Core.Abstractions;

 namespace ZB.MOM.WW.OtOpcUa.AdminUI.Components.Shared.Drivers;

 /// <summary>
-///     Mutable, all-nullable form model for the driver resilience override. Binds the typed
-///     fields in DriverResilienceSection; null/blank = "use the driver's tier default", so a
-///     blank form serializes back to null (preserving DriverInstance.ResilienceConfig = null).
-///     Emits / reads the exact override JSON shape DriverResilienceOptionsParser consumes.
+///     Mutable, all-nullable form model for the driver resilience override. Binds the typed fields in
+///     DriverResilienceSection; null/blank = "use the driver's tier default", so a blank form serializes
+///     back to null (preserving DriverInstance.ResilienceConfig = null). Emits / reads the exact override
+///     JSON shape DriverResilienceOptionsParser consumes.
+///     <para>
+///     Round-trip is <b>non-lossy</b> (04/C-7): the stored JSON is kept as a <see cref="JsonObject"/> bag,
+///     and <see cref="ToJson"/> overlays only the known fields onto that bag — so unknown top-level keys,
+///     unknown capability entries, and unknown per-policy fields all survive a load→save, mirroring the
+///     driver tag editors' preserve-unknown discipline (<c>TagConfigJson</c>). A stored blob that could not
+///     be parsed sets <see cref="ParseFailed"/> and is <b>never</b> overwritten — <see cref="ToJson"/>
+///     returns the original text verbatim.
+///     </para>
 /// </summary>
 public sealed class ResilienceFormModel
 {
@@ -23,6 +31,18 @@ public sealed class ResilienceFormModel
    public Dictionary<string, CapabilityRow> Policies { get; set; } =
        Capabilities.ToDictionary(c => c, _ => new CapabilityRow(), StringComparer.OrdinalIgnoreCase);

+    /// <summary>
+    ///     True when the stored ResilienceConfig could not be parsed as JSON. The section locks editing and
+    ///     <see cref="ToJson"/> returns <see cref="RawStoredJson"/> unchanged so an unrelated keystroke can
+    ///     never silently overwrite an unreadable column.
+    /// </summary>
+    public bool ParseFailed { get; private set; }
+
+    /// <summary>The original stored text when <see cref="ParseFailed"/> — returned verbatim by <see cref="ToJson"/>.</summary>
+    public string? RawStoredJson { get; private set; }
+
+    private JsonObject _bag = new();
+
    /// <summary>Per-capability timeout/retry/breaker override row; null fields fall back to the tier default.</summary>
    public sealed class CapabilityRow
    {
@@ -68,79 +88,125 @@ public sealed class ResilienceFormModel
        return msgs;
    }

-    private static readonly JsonSerializerOptions ReadOpts = new() { PropertyNameCaseInsensitive = true };
-    private static readonly JsonSerializerOptions WriteOpts = new()
-    {
-        PropertyNamingPolicy = JsonNamingPolicy.CamelCase,
-        DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull,
-    };
-
-    /// <summary>Parses the override JSON into a form model; malformed or blank JSON yields an empty (all-default) form.</summary>
+    /// <summary>
+    ///     Parse the override JSON into a form model, preserving every key in an internal bag. Malformed JSON
+    ///     sets <see cref="ParseFailed"/> + <see cref="RawStoredJson"/> and yields an otherwise-empty model.
+    /// </summary>
    /// <param name="json">The raw resilience-override JSON, or null/blank if there is no override.</param>
-    /// <returns>A populated form model, or an all-default one when <paramref name="json"/> is blank or malformed.</returns>
+    /// <returns>A populated form model.</returns>
    public static ResilienceFormModel FromJson(string? json)
    {
        var model = new ResilienceFormModel();
        if (string.IsNullOrWhiteSpace(json)) return model;

-        Shape? shape;
-        try { shape = JsonSerializer.Deserialize<Shape>(json, ReadOpts); }
-        catch (JsonException) { return model; }   // malformed -> empty form; raw view (next task) shows the text
-        if (shape is null) return model;
+        JsonObject bag;
+        try
+        {
+            bag = JsonNode.Parse(json) as JsonObject ?? new JsonObject();
+        }
+        catch (JsonException)
+        {
+            model.ParseFailed = true;
+            model.RawStoredJson = json;
+            return model;
+        }

-        model.RecycleIntervalSeconds = shape.RecycleIntervalSeconds;
-        if (shape.CapabilityPolicies is not null)
-            foreach (var (cap, p) in shape.CapabilityPolicies)
-                if (model.Policies.TryGetValue(cap, out var row))
+        model._bag = bag;
+        model.RecycleIntervalSeconds = GetIntOrNull(bag, "recycleIntervalSeconds");
+
+        if (bag.TryGetPropertyValue("capabilityPolicies", out var cpNode) && cpNode is JsonObject cp)
+        {
+            foreach (var (capName, polNode) in cp)
+            {
+                // Known capability names populate rows (reading only the known per-policy fields — unknown
+                // per-policy fields stay untouched in the bag). Unknown capability entries stay in the bag.
+                if (polNode is JsonObject pol && model.Policies.TryGetValue(capName, out var row))
                {
-                    row.TimeoutSeconds = p.TimeoutSeconds;
-                    row.RetryCount = p.RetryCount;
-                    row.BreakerFailureThreshold = p.BreakerFailureThreshold;
+                    row.TimeoutSeconds = GetIntOrNull(pol, "timeoutSeconds");
+                    row.RetryCount = GetIntOrNull(pol, "retryCount");
+                    row.BreakerFailureThreshold = GetIntOrNull(pol, "breakerFailureThreshold");
                }
+            }
+        }
        return model;
    }

-    /// <summary>Emit only the non-null overrides; returns null when nothing is overridden.</summary>
-    /// <returns>The serialized override JSON, or null when no field in this form is overridden.</returns>
+    /// <summary>
+    ///     Overlay the model's non-null known values onto the preserved bag and serialize. Returns null when
+    ///     the result is empty (blank form = tier defaults). When <see cref="ParseFailed"/>, returns the
+    ///     original unparseable text unchanged.
+    /// </summary>
+    /// <returns>The serialized override JSON, the original text when parse failed, or null when nothing is set.</returns>
    public string? ToJson()
    {
-        var caps = Policies
-            .Where(kv => !kv.Value.IsEmpty)
-            .ToDictionary(kv => kv.Key, kv => new PolicyShape
-            {
-                TimeoutSeconds = kv.Value.TimeoutSeconds,
-                RetryCount = kv.Value.RetryCount,
-                BreakerFailureThreshold = kv.Value.BreakerFailureThreshold,
-            });
+        if (ParseFailed) return RawStoredJson;

-        var hasAny = RecycleIntervalSeconds is not null || caps.Count > 0;
-        if (!hasAny) return null;
+        SetOrRemoveInt(_bag, "recycleIntervalSeconds", RecycleIntervalSeconds);

-        var shape = new Shape
+        var cp = _bag.TryGetPropertyValue("capabilityPolicies", out var cpNode) && cpNode is JsonObject existing
+            ? existing
+            : null;
+
+        foreach (var (cap, row) in Policies)
        {
-            RecycleIntervalSeconds = RecycleIntervalSeconds,
-            CapabilityPolicies = caps.Count > 0 ? caps : null,
-        };
-        return JsonSerializer.Serialize(shape, WriteOpts);
+            // Find this known capability's existing policy object (case-insensitive), else its canonical key.
+            var polKey = cap;
+            JsonObject? pol = null;
+            if (cp is not null)
+            {
+                foreach (var (k, v) in cp)
+                {
+                    if (string.Equals(k, cap, StringComparison.OrdinalIgnoreCase) && v is JsonObject vo)
+                    {
+                        polKey = k;
+                        pol = vo;
+                        break;
+                    }
+                }
+            }
+
+            if (row.IsEmpty)
+            {
+                // Clear the known fields; drop the policy entry only if no unknown fields remain.
+                if (pol is not null)
+                {
+                    pol.Remove("timeoutSeconds");
+                    pol.Remove("retryCount");
+                    pol.Remove("breakerFailureThreshold");
+                    if (pol.Count == 0) cp!.Remove(polKey);
+                }
+            }
+            else
+            {
+                if (cp is null)
+                {
+                    cp = new JsonObject();
+                    _bag["capabilityPolicies"] = cp;
+                }
+                if (pol is null)
+                {
+                    pol = new JsonObject();
+                    cp[polKey] = pol;
+                }
+                SetOrRemoveInt(pol, "timeoutSeconds", row.TimeoutSeconds);
+                SetOrRemoveInt(pol, "retryCount", row.RetryCount);
+                SetOrRemoveInt(pol, "breakerFailureThreshold", row.BreakerFailureThreshold);
+            }
+        }
+
+        if (cp is not null && cp.Count == 0) _bag.Remove("capabilityPolicies");
+
+        return _bag.Count == 0 ? null : _bag.ToJsonString();
    }

-    /// <summary>Wire shape of the resilience-override JSON, as consumed by DriverResilienceOptionsParser.</summary>
-    private sealed class Shape
-    {
-        /// <summary>Gets or sets the driver recycle-interval override, in seconds.</summary>
-        public int? RecycleIntervalSeconds { get; set; }
-        /// <summary>Gets or sets the per-capability overrides, keyed by capability name.</summary>
-        public Dictionary<string, PolicyShape>? CapabilityPolicies { get; set; }
-    }
+    private static int? GetIntOrNull(JsonObject o, string name)
+        => o.TryGetPropertyValue(name, out var n) && n is JsonValue v && v.TryGetValue<int>(out var i)
+            ? i
+            : null;

-    /// <summary>Wire shape of a single capability's timeout/retry/breaker override.</summary>
-    private sealed class PolicyShape
+    private static void SetOrRemoveInt(JsonObject o, string name, int? value)
    {
-        /// <summary>Gets or sets the timeout override, in seconds.</summary>
-        public int? TimeoutSeconds { get; set; }
-        /// <summary>Gets or sets the retry-count override.</summary>
-        public int? RetryCount { get; set; }
-        /// <summary>Gets or sets the circuit-breaker failure-threshold override.</summary>
-        public int? BreakerFailureThreshold { get; set; }
+        if (value is null) o.Remove(name);
+        else o[name] = JsonValue.Create(value.Value);
    }
 }
@@ -1,3 +1,4 @@
+using System.Text.Json.Nodes;
 using Shouldly;
 using Xunit;
 using ZB.MOM.WW.OtOpcUa.AdminUI.Components.Shared.Drivers;
@@ -6,6 +7,59 @@ using ZB.MOM.WW.OtOpcUa.Core.Resilience;

 public class ResilienceFormModelTests
 {
+    // ---- 04/C-7: non-lossy round-trip ----
+
+    [Fact]
+    public void Unknown_top_level_key_survives_round_trip()
+    {
+        // bulkheadMaxConcurrent doubles as the 01/U-6 continuity proof: the deleted knob's key survives.
+        var json = """{"bulkheadMaxConcurrent":16,"capabilityPolicies":{"Read":{"timeoutSeconds":5}}}""";
+
+        var back = ResilienceFormModel.FromJson(json).ToJson();
+
+        back.ShouldNotBeNull();
+        var obj = JsonNode.Parse(back)!.AsObject();
+        obj["bulkheadMaxConcurrent"]!.GetValue<int>().ShouldBe(16);
+        obj["capabilityPolicies"]!["Read"]!["timeoutSeconds"]!.GetValue<int>().ShouldBe(5);
+    }
+
+    [Fact]
+    public void Unknown_capability_entry_survives_round_trip()
+    {
+        var json = """{"capabilityPolicies":{"Read":{"timeoutSeconds":5},"FutureCap":{"timeoutSeconds":9}}}""";
+
+        var back = ResilienceFormModel.FromJson(json).ToJson();
+
+        back.ShouldNotBeNull();
+        var obj = JsonNode.Parse(back)!.AsObject();
+        obj["capabilityPolicies"]!["FutureCap"]!["timeoutSeconds"]!.GetValue<int>().ShouldBe(9);
+        obj["capabilityPolicies"]!["Read"]!["timeoutSeconds"]!.GetValue<int>().ShouldBe(5);
+    }
+
+    [Fact]
+    public void Unknown_per_policy_field_survives_round_trip()
+    {
+        var json = """{"capabilityPolicies":{"Read":{"timeoutSeconds":5,"futureField":7}}}""";
+
+        var back = ResilienceFormModel.FromJson(json).ToJson();
+
+        back.ShouldNotBeNull();
+        var read = JsonNode.Parse(back)!.AsObject()["capabilityPolicies"]!["Read"]!.AsObject();
+        read["timeoutSeconds"]!.GetValue<int>().ShouldBe(5);
+        read["futureField"]!.GetValue<int>().ShouldBe(7);
+    }
+
+    [Fact]
+    public void Malformed_json_sets_ParseFailed_and_ToJson_returns_original_text()
+    {
+        const string json = "{ not json";
+
+        var m = ResilienceFormModel.FromJson(json);
+
+        m.ParseFailed.ShouldBeTrue();
+        m.ToJson().ShouldBe(json);   // never overwrite what it couldn't read
+    }
+
    [Fact]
    public void Blank_form_serializes_to_null()
        => new ResilienceFormModel().ToJson().ShouldBeNull();