fix(ui): per-field sub-schema for escape-hatch Monaco editors + dispose guard (#261)

2026-06-19 03:28:58 -04:00
parent 282bc8b53c
commit e814bf5efb
3 changed files with 171 additions and 24 deletions
@@ -53,12 +53,6 @@ else
    private InboundApiSchema? _rootSchema;
    private string? _parsedFor;

-    // The resolved root schema serialized back to a JSON Schema string (with every
-    // {"$ref":"lib:Name"} inlined). Fed to the raw-JSON escape-hatch Monaco editor
-    // so Monaco's built-in JSON language offers schema-driven hover + completion on
-    // the entered JSON. Computed once per parse alongside the resolved schema.
-    private string? _resolvedSchemaJson;
-
    // Raw text and per-field parse/validation errors keyed by canonical PATH
    // (e.g. "order.id", "tags[0]") so nested inputs each carry their own state.
    private readonly Dictionary<string, string> _rawText = new();
@@ -76,9 +70,22 @@ else
        _shapes = ScriptParameterNames.ParseShapes(ParameterDefinitions);
        _rootSchema = await ParseRootAsync();
        _topLevelFields = _rootSchema?.Type == "object" ? _rootSchema.Fields : Array.Empty<InboundApiSchemaField>();
-        _resolvedSchemaJson = _rootSchema is null ? null : ToJsonSchema(_rootSchema);
    }

+    /// <summary>
+    /// Serializes the escape-hatch FIELD's own (resolved) sub-schema to a JSON
+    /// Schema string for that field's Monaco editor — NOT the whole root object
+    /// schema. So a field's raw-JSON editor validates/completes against just that
+    /// field's portion of the type, not the entire object. Any
+    /// <c>{"$ref":"lib:Name"}</c> the parse already inlined is emitted as its
+    /// resolved shape; a node whose ref could NOT be resolved (Type
+    /// <c>"ref"</c>) — or any unknown type — has no derivable sub-schema and
+    /// collapses to a permissive <c>{}</c>, so the editor still validates
+    /// well-formed JSON without false errors. Cheap and on-demand: only the rare
+    /// escape-hatch nodes pay for it, and only at render time.
+    /// </summary>
+    private static string FieldSchemaJson(InboundApiSchema fieldSchema) => ToJsonSchema(fieldSchema);
+
    /// <summary>
    /// Serializes a (resolved) <see cref="InboundApiSchema"/> back to a JSON Schema
    /// string for the escape-hatch Monaco editor. Any <c>{"$ref":"lib:Name"}</c> the
@@ -250,7 +257,7 @@ else
            default: // unresolved $ref / unknown — raw-JSON escape hatch (Monaco).
                <div class="param-json-editor" data-path="@path">
                    <MonacoEditor Language="json" Height="120px" ShowToolbar="false"
-                                  JsonSchema="@_resolvedSchemaJson"
+                                  JsonSchema="@FieldSchemaJson(schema)"
                                  Value="@AsRaw(path)"
                                  ValueChanged="@(v => SetJson(path, v))" />
                </div>
@@ -330,12 +330,16 @@
        // A json editor with a schema needs an explicitly-URI'd model so the
        // schema's fileMatch can target exactly this editor.
        let model = null;
+        let hasSchema = false;
        if (isJson) {
            const uri = modelUriFor(id);
            model = monaco.editor.getModel(uri);
            if (model) { model.setValue(options.value || ""); }
            else { model = monaco.editor.createModel(options.value || "", "json", uri); }
-            if (options.jsonSchema) { applyJsonSchema(id, options.jsonSchema); }
+            if (options.jsonSchema && options.jsonSchema.trim().length > 0) {
+                applyJsonSchema(id, options.jsonSchema);
+                hasSchema = true;
+            }
        }

        const editor = monaco.editor.create(host, {
@@ -372,7 +376,7 @@
            dotNetRef.invokeMethodAsync("OnValueChanged", value).catch(function () {});
            if (options.language === "csharp") scheduleDiagnostics();
        });
-        editors[id] = { editor: editor, dotNetRef: dotNetRef, isJson: isJson };
+        editors[id] = { editor: editor, dotNetRef: dotNetRef, isJson: isJson, hasSchema: hasSchema };

        // Run an initial diagnostic pass so existing scripts show their markers.
        if (language === "csharp") scheduleDiagnostics();
@@ -381,6 +385,9 @@
    function setJsonSchema(id, schemaJson) {
        const entry = editors[id];
        if (!entry || !entry.isJson) return;
+        // Track whether this editor currently owns a schema entry in jsonDefaults
+        // so dispose can skip the teardown interop when there is nothing to drop.
+        entry.hasSchema = !!(schemaJson && schemaJson.trim().length > 0);
        applyJsonSchema(id, schemaJson);
    }

@@ -436,7 +443,9 @@
        if (!entry) return;
        // Drop this editor's json schema entry and its explicitly-URI'd model so
        // a disposed json editor leaves nothing behind in the global defaults.
-        if (entry.isJson) {
+        // Skip the schema teardown for a json editor that never had a schema —
+        // there is no entry to remove, so the applyJsonSchema interop is spurious.
+        if (entry.isJson && entry.hasSchema) {
            try { applyJsonSchema(id, null); } catch (e) {}
        }
        try { entry.editor.dispose(); } catch (e) {}
@@ -7,13 +7,15 @@ using ZB.MOM.WW.ScadaBridge.CentralUI.Services;
 namespace ZB.MOM.WW.ScadaBridge.CentralUI.Tests.Design;

 /// <summary>
-/// M9-T31: the raw-JSON escape hatch in <see cref="ParameterValueForm"/> (the
-/// fallback for an unresolved <c>$ref</c> / unknown-type node) is a Monaco
-/// <c>json</c> editor wired with the form's RESOLVED schema, so Monaco's
-/// built-in JSON language gives schema-driven hover + completion. These tests
-/// assert at the C#/interop boundary that the resolved schema (with
-/// <c>{"$ref":"lib:Name"}</c> inlined) reaches the editor's <c>JsonSchema</c>
-/// parameter — the JS hover/completion itself is Monaco-built-in and not
+/// M9-T31 (+#261 follow-up): the raw-JSON escape hatch in
+/// <see cref="ParameterValueForm"/> (the fallback for an unresolved <c>$ref</c> /
+/// unknown-type node) is a Monaco <c>json</c> editor wired with THAT FIELD's own
+/// sub-schema — not the whole root object schema — so each field's editor
+/// validates/completes against just its own portion of the type. These tests
+/// assert at the C#/interop boundary that the correct per-field fragment reaches
+/// each editor's <c>JsonSchema</c> parameter (and that a sibling field's resolved
+/// shape never leaks in), using <c>FindComponents</c> when 2+ escape-hatch fields
+/// are present. The JS hover/completion itself is Monaco-built-in and not
 /// unit-testable here.
 /// </summary>
 public class ParameterValueFormMonacoSchemaTests : BunitContext
@@ -56,7 +58,7 @@ public class ParameterValueFormMonacoSchemaTests : BunitContext
    }

    [Fact]
-    public void EscapeHatch_MonacoEditor_ReceivesResolvedSchema_WithRefInlined()
+    public void EscapeHatch_MonacoEditor_ReceivesFieldSubSchema_NotRootSchema()
    {
        // The library entry "Address" resolves to an object with street/city.
        const string libSchema = """
@@ -73,6 +75,8 @@ public class ParameterValueFormMonacoSchemaTests : BunitContext

        // Root has a RESOLVABLE $ref field (shipTo → Address) plus an
        // UNRESOLVABLE one (payload) that forces the escape-hatch Monaco editor.
+        // The payload editor must receive PAYLOAD's own sub-schema — NOT the root
+        // object schema, which would leak the sibling shipTo's street/city shape.
        const string schema = """
            {
              "type": "object",
@@ -91,11 +95,138 @@ public class ParameterValueFormMonacoSchemaTests : BunitContext
        var jsonSchema = editor.Instance.JsonSchema;

        Assert.False(string.IsNullOrWhiteSpace(jsonSchema));
-        // The RESOLVED shape is fed to Monaco: the $ref target's fields are
-        // inlined (street/city present), and the lib: pointer text is gone.
-        Assert.Contains("street", jsonSchema!);
-        Assert.Contains("city", jsonSchema!);
-        Assert.DoesNotContain("lib:Address", jsonSchema!);
+        // The escape hatch fires only for the UNRESOLVED payload field. Its own
+        // sub-schema has no derivable shape (the ref dangled), so it collapses to
+        // a permissive {} — and it must NOT carry the sibling shipTo's resolved
+        // street/city fields (the root-schema leak this fix closes).
+        Assert.DoesNotContain("street", jsonSchema!);
+        Assert.DoesNotContain("city", jsonSchema!);
+        Assert.DoesNotContain("shipTo", jsonSchema!);
+        Assert.DoesNotContain("payload", jsonSchema!);
        Assert.DoesNotContain("$ref", jsonSchema!);
    }
+
+    [Fact]
+    public void EscapeHatch_WithResolvableSiblingObject_ScopesFieldEditorToItsOwnFragment()
+    {
+        // A fully RESOLVABLE sibling object (order → typed inline object) renders
+        // typed inputs, NOT an escape hatch. Only the unresolvable payload drops
+        // to Monaco — and its schema must be the field fragment, never the root
+        // (which carries order's id/name).
+        const string schema = """
+            {
+              "type": "object",
+              "properties": {
+                "order": {
+                  "type": "object",
+                  "properties": {
+                    "id": { "type": "integer" },
+                    "name": { "type": "string" }
+                  }
+                },
+                "payload": { "$ref": "lib:Missing" }
+              }
+            }
+            """;
+
+        var cut = Render<ParameterValueForm>(p => p
+            .Add(x => x.ParameterDefinitions, schema)
+            .Add(x => x.Values, new Dictionary<string, object?>()));
+
+        // Exactly one escape-hatch editor (for payload); order renders typed inputs.
+        var editor = cut.FindComponent<MonacoEditor>();
+        var jsonSchema = editor.Instance.JsonSchema;
+
+        Assert.False(string.IsNullOrWhiteSpace(jsonSchema));
+        // Must not leak the sibling order field's declared shape.
+        Assert.DoesNotContain("order", jsonSchema!);
+        Assert.DoesNotContain("\"id\"", jsonSchema!);
+    }
+
+    [Fact]
+    public void MultipleEscapeHatchFields_EachEditorReceivesItsOwnFieldFragment_NotRootSchema()
+    {
+        // TWO unresolvable-$ref fields each render their OWN escape-hatch Monaco
+        // editor. Before the per-field fix, both received the SAME root object
+        // schema (carrying both sibling field names) — this test would have caught
+        // that. With the fix, each editor gets only its field's sub-schema (a
+        // permissive {} for a dangling ref), carrying neither sibling's name.
+        const string schema = """
+            {
+              "type": "object",
+              "properties": {
+                "left": { "$ref": "lib:MissingA" },
+                "right": { "$ref": "lib:MissingB" }
+              }
+            }
+            """;
+
+        var cut = Render<ParameterValueForm>(p => p
+            .Add(x => x.ParameterDefinitions, schema)
+            .Add(x => x.Values, new Dictionary<string, object?>()));
+
+        // FindComponents (plural): both unresolved-ref fields must render editors.
+        var editors = cut.FindComponents<MonacoEditor>();
+        Assert.Equal(2, editors.Count);
+
+        foreach (var editor in editors)
+        {
+            var jsonSchema = editor.Instance.JsonSchema;
+            Assert.False(string.IsNullOrWhiteSpace(jsonSchema));
+            // Per-field fragment only — neither editor carries the root object's
+            // property names (left/right) nor a residual $ref.
+            Assert.DoesNotContain("left", jsonSchema!);
+            Assert.DoesNotContain("right", jsonSchema!);
+            Assert.DoesNotContain("$ref", jsonSchema!);
+            Assert.DoesNotContain("properties", jsonSchema!);
+        }
+    }
+
+    [Fact]
+    public void MultipleEscapeHatchFields_DoNotLeakAResolvableSiblingsShape()
+    {
+        // The library entry "Address" resolves to street/city. The root mixes one
+        // RESOLVABLE $ref (shipTo) with two UNRESOLVABLE ones (a, b). Each escape
+        // hatch must be scoped to its own field — none may carry the resolved
+        // street/city of the sibling shipTo (the precise root-schema leak).
+        const string libSchema = """
+            {
+              "type": "object",
+              "properties": {
+                "street": { "type": "string" },
+                "city": { "type": "string" }
+              }
+            }
+            """;
+        _library.GetSchemaMapAsync(Arg.Any<CancellationToken>())
+            .Returns(new Dictionary<string, string> { ["Address"] = libSchema });
+
+        const string schema = """
+            {
+              "type": "object",
+              "properties": {
+                "shipTo": { "$ref": "lib:Address" },
+                "a": { "$ref": "lib:MissingA" },
+                "b": { "$ref": "lib:MissingB" }
+              }
+            }
+            """;
+
+        var cut = Render<ParameterValueForm>(p => p
+            .Add(x => x.ParameterDefinitions, schema)
+            .Add(x => x.Values, new Dictionary<string, object?>()));
+
+        // shipTo resolves to typed inputs; only a and b drop to escape hatches.
+        var editors = cut.FindComponents<MonacoEditor>();
+        Assert.Equal(2, editors.Count);
+
+        foreach (var editor in editors)
+        {
+            var jsonSchema = editor.Instance.JsonSchema;
+            Assert.False(string.IsNullOrWhiteSpace(jsonSchema));
+            Assert.DoesNotContain("street", jsonSchema!);
+            Assert.DoesNotContain("city", jsonSchema!);
+            Assert.DoesNotContain("shipTo", jsonSchema!);
+        }
+    }
 }