diff --git a/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json b/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json index 764340fc..55e05832 100644 --- a/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json +++ b/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json @@ -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] }, diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/ResilienceFormModel.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/ResilienceFormModel.cs index fa9aeb7e..e5f2a7ca 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/ResilienceFormModel.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/ResilienceFormModel.cs @@ -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; /// -/// 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. +/// +/// Round-trip is non-lossy (04/C-7): the stored JSON is kept as a bag, +/// and 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 (TagConfigJson). A stored blob that could not +/// be parsed sets and is never overwritten — +/// returns the original text verbatim. +/// /// public sealed class ResilienceFormModel { @@ -23,6 +31,18 @@ public sealed class ResilienceFormModel public Dictionary Policies { get; set; } = Capabilities.ToDictionary(c => c, _ => new CapabilityRow(), StringComparer.OrdinalIgnoreCase); + /// + /// True when the stored ResilienceConfig could not be parsed as JSON. The section locks editing and + /// returns unchanged so an unrelated keystroke can + /// never silently overwrite an unreadable column. + /// + public bool ParseFailed { get; private set; } + + /// The original stored text when — returned verbatim by . + public string? RawStoredJson { get; private set; } + + private JsonObject _bag = new(); + /// Per-capability timeout/retry/breaker override row; null fields fall back to the tier default. 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, - }; - - /// Parses the override JSON into a form model; malformed or blank JSON yields an empty (all-default) form. + /// + /// Parse the override JSON into a form model, preserving every key in an internal bag. Malformed JSON + /// sets + and yields an otherwise-empty model. + /// /// The raw resilience-override JSON, or null/blank if there is no override. - /// A populated form model, or an all-default one when is blank or malformed. + /// A populated form model. public static ResilienceFormModel FromJson(string? json) { var model = new ResilienceFormModel(); if (string.IsNullOrWhiteSpace(json)) return model; - Shape? shape; - try { shape = JsonSerializer.Deserialize(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; } - /// Emit only the non-null overrides; returns null when nothing is overridden. - /// The serialized override JSON, or null when no field in this form is overridden. + /// + /// 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 , returns the + /// original unparseable text unchanged. + /// + /// The serialized override JSON, the original text when parse failed, or null when nothing is set. 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(); } - /// Wire shape of the resilience-override JSON, as consumed by DriverResilienceOptionsParser. - private sealed class Shape - { - /// Gets or sets the driver recycle-interval override, in seconds. - public int? RecycleIntervalSeconds { get; set; } - /// Gets or sets the per-capability overrides, keyed by capability name. - public Dictionary? CapabilityPolicies { get; set; } - } + private static int? GetIntOrNull(JsonObject o, string name) + => o.TryGetPropertyValue(name, out var n) && n is JsonValue v && v.TryGetValue(out var i) + ? i + : null; - /// Wire shape of a single capability's timeout/retry/breaker override. - private sealed class PolicyShape + private static void SetOrRemoveInt(JsonObject o, string name, int? value) { - /// Gets or sets the timeout override, in seconds. - public int? TimeoutSeconds { get; set; } - /// Gets or sets the retry-count override. - public int? RetryCount { get; set; } - /// Gets or sets the circuit-breaker failure-threshold override. - public int? BreakerFailureThreshold { get; set; } + if (value is null) o.Remove(name); + else o[name] = JsonValue.Create(value.Value); } } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ResilienceFormModelTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ResilienceFormModelTests.cs index 301797b7..fd4a8765 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ResilienceFormModelTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ResilienceFormModelTests.cs @@ -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().ShouldBe(16); + obj["capabilityPolicies"]!["Read"]!["timeoutSeconds"]!.GetValue().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().ShouldBe(9); + obj["capabilityPolicies"]!["Read"]!["timeoutSeconds"]!.GetValue().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().ShouldBe(5); + read["futureField"]!.GetValue().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();