diff --git a/encoding/jsonschema/generate_test.go b/encoding/jsonschema/generate_test.go index 1e816e5bb..839019418 100644 --- a/encoding/jsonschema/generate_test.go +++ b/encoding/jsonschema/generate_test.go @@ -55,23 +55,24 @@ func TestGenerate(t *testing.T) { t.Writer("schema").Write(data) t.Logf("generated schema: %q", data) - // Round-trip test: convert generated JSON Schema back to CUE to validate - // First compile the AST to a CUE value, then marshal to JSON + // Round-trip test: convert generated JSON Schema back to CUE to validate. schemaValue := ctx.BuildExpr(r) qt.Assert(t, qt.IsNil(schemaValue.Err()), qt.Commentf("schema data: %q", data)) schemaBytes, err := schemaValue.MarshalJSON() qt.Assert(t, qt.IsNil(err)) - // Parse the JSON back to a CUE value for extraction schemaValue = ctx.CompileBytes(schemaBytes) qt.Assert(t, qt.IsNil(schemaValue.Err())) - // Extract back to CUE with strict validation extractedSchemaFile, err := jsonschema.Extract(schemaValue, &jsonschema.Config{ StrictFeatures: true, StrictKeywords: true, }) + if t.HasTag("brokenRoundTrip") { + t.Skipf("round-trip extraction skipped (brokenRoundTrip)") + return + } qt.Assert(t, qt.IsNil(err), qt.Commentf("generated JSON Schema should round-trip cleanly via Extract")) extractedSchemaValue := ctx.BuildFile(extractedSchemaFile) t.Logf("extracted schema: %#v", extractedSchemaValue) diff --git a/encoding/jsonschema/testdata/generate/closedness.txtar b/encoding/jsonschema/testdata/generate/closedness.txtar new file mode 100644 index 000000000..150346a51 --- /dev/null +++ b/encoding/jsonschema/testdata/generate/closedness.txtar @@ -0,0 +1,152 @@ +# Tests for closedness-related issues in JSON Schema generation. +# +# Issue 4352: closedness lost in disjunctions. +# +# BUG(4352): the struct elements inside the anyOf for Disj, NestedDisj, +# and TwoStructDisj should have additionalProperties: false but don't +# because the OrOp handler discards the closedness mode. +-- test.cue -- +package test + +#Disj: int | { + a?: int +} + +#Simple: { + a?: int + b?: string +} + +#NestedDisj: { + x?: string | { + y?: int + } +} + +#TwoStructDisj: {a?: int} | {b?: string} + +t01?: #Disj +t02?: #Simple +t03?: #NestedDisj +t04?: #TwoStructDisj + +-- datatest/testdata.cue -- +package datatest +disjInt: { + data: {t01: 42} +} +disjStruct: { + data: {t01: {a: 1}} +} +// BUG(4352): this should not be accepted because the struct disjunct of #Disj +// is closed, but the generated schema is missing additionalProperties: false +// on the struct disjunct. +disjStructExtra: { + data: {t01: {a: 1, extra: true}} +} +simple: { + data: {t02: {a: 1, b: "hello"}} +} +simpleExtra: { + data: {t02: {a: 1, extra: true}} + error: true +} +// BUG(4352): this should not be accepted because the inner struct disjunct +// of x is missing additionalProperties: false. +nestedDisjStructExtra: { + data: {t03: {x: {y: 1, extra: true}}} +} +// BUG(4352): this should not be accepted because both struct disjuncts +// should be closed. +twoStructDisjExtra: { + data: {t04: {a: 1, extra: true}} +} +-- out/generate-v3/schema -- +{ + $schema: "https://json-schema.org/draft/2020-12/schema" + $defs: { + Disj: { + anyOf: [{ + type: "integer" + }, { + type: "object" + properties: { + a: { + type: "integer" + } + } + }] + } + NestedDisj: { + type: "object" + additionalProperties: false + properties: { + x: { + anyOf: [{ + type: "string" + }, { + type: "object" + properties: { + y: { + type: "integer" + } + } + }] + } + } + } + Simple: { + type: "object" + additionalProperties: false + properties: { + a: { + type: "integer" + } + b: { + type: "string" + } + } + } + TwoStructDisj: { + anyOf: [{ + type: "object" + properties: { + a: { + type: "integer" + } + } + }, { + type: "object" + properties: { + b: { + type: "string" + } + } + }] + } + } + type: "object" + properties: { + t01: { + $ref: "#/$defs/Disj" + } + t02: { + $ref: "#/$defs/Simple" + } + t03: { + $ref: "#/$defs/NestedDisj" + } + t04: { + $ref: "#/$defs/TwoStructDisj" + } + } +} +-- out/generate-v3/disjInt -- +-- out/generate-v3/disjStruct -- +-- out/generate-v3/disjStructExtra -- +-- out/generate-v3/simple -- +-- out/generate-v3/simpleExtra -- +simpleExtra.data.t02.extra: field not allowed: + ./datatest/testdata.cue:18:21 +-- out/generate-v3/nestedDisjStructExtra -- +-- out/generate-v3/twoStructDisjExtra -- diff --git a/encoding/jsonschema/testdata/generate/closedness_explicitopen.txtar b/encoding/jsonschema/testdata/generate/closedness_explicitopen.txtar new file mode 100644 index 000000000..d4297e7ae --- /dev/null +++ b/encoding/jsonschema/testdata/generate/closedness_explicitopen.txtar @@ -0,0 +1,151 @@ +# Tests for closedness with the explicitopen experiment. +# With explicitopen, spread (#T...) opens a definition, +# while plain embedding (#T) keeps it closed. +-- test.cue -- +@experiment(explicitopen) +package test + +#WithSpread: { + #Inner... + y?: string +} + +#Inner: { + x?: int +} + +#MixedRef: { + #Inner... + child?: #Inner + z?: string +} +_foo: { + #Inner... + y?: int +} +_bar: { + #Inner + y?: int +} + +t01?: #WithSpread +t02?: #Inner +t03?: #MixedRef +t04?: _foo +t05?: _bar + +-- datatest/testdata.cue -- +package datatest +spreadValid: { + data: {t01: {x: 1, y: "hello"}} +} +spreadExtra: { + data: {t01: {x: 1, y: "hello", extra: true}} + error: true +} +innerValid: { + data: {t02: {x: 1}} +} +innerExtra: { + data: {t02: {x: 1, extra: true}} + error: true +} +mixedValid: { + data: {t03: {x: 1, child: {x: 2}, z: "hi"}} +} +mixedChildExtra: { + data: {t03: {x: 1, child: {x: 2, extra: true}, z: "hi"}} + error: true +} +-- out/generate-v3/schema -- +{ + $schema: "https://json-schema.org/draft/2020-12/schema" + $defs: { + Inner: { + type: "object" + additionalProperties: false + properties: { + x: { + type: "integer" + } + } + } + MixedRef: { + type: "object" + additionalProperties: false + properties: { + child: { + $ref: "#/$defs/Inner" + } + x: { + type: "integer" + } + z: { + type: "string" + } + } + } + WithSpread: { + type: "object" + additionalProperties: false + properties: { + x: { + type: "integer" + } + y: { + type: "string" + } + } + } + "_bar": { + type: "object" + properties: { + y: { + type: "integer" + } + } + $ref: "#/$defs/Inner" + } + "_foo": { + type: "object" + properties: { + x: { + type: "integer" + } + y: { + type: "integer" + } + } + } + } + type: "object" + properties: { + t01: { + $ref: "#/$defs/WithSpread" + } + t02: { + $ref: "#/$defs/Inner" + } + t03: { + $ref: "#/$defs/MixedRef" + } + t04: { + $ref: "#/$defs/_foo" + } + t05: { + $ref: "#/$defs/_bar" + } + } +} +-- out/generate-v3/spreadValid -- +-- out/generate-v3/spreadExtra -- +spreadExtra.data.t01.extra: field not allowed: + ./datatest/testdata.cue:6:33 +-- out/generate-v3/innerValid -- +-- out/generate-v3/innerExtra -- +innerExtra.data.t02.extra: field not allowed: + ./datatest/testdata.cue:13:21 +-- out/generate-v3/mixedValid -- +-- out/generate-v3/mixedChildExtra -- +mixedChildExtra.data.t03.child.extra: field not allowed: + ./datatest/testdata.cue:20:35 diff --git a/encoding/jsonschema/testdata/generate/closedness_nondef.txtar b/encoding/jsonschema/testdata/generate/closedness_nondef.txtar new file mode 100644 index 000000000..a8b3451cf --- /dev/null +++ b/encoding/jsonschema/testdata/generate/closedness_nondef.txtar @@ -0,0 +1,142 @@ +# Tests for non-definition reference closedness. +# When a definition's body is solely a reference to a non-definition, +# the definition should still be closed. +# +# BUG: WrapsHidden and WrapsShared should have additionalProperties: false +# because they're definitions. Currently they're just a $ref to the +# open non-definition, losing closedness. +-- test.cue -- +package test + +#WrapsHidden: _hidden + +_hidden: { + h?: int +} + +_shared: { + v?: int +} + +#WrapsShared: _shared +#WrapsConjunction: _shared & _hidden + +t01?: #WrapsHidden +t02open?: _shared +t02closed?: #WrapsShared +t03closed?: #WrapsConjunction +t03open?: _shared & _hidden + +-- datatest/testdata.cue -- +package datatest +hiddenValid: { + data: {t01: {h: 1}} +} +// BUG: this should not be accepted. #WrapsHidden is a definition +// wrapping an open non-definition. The definition should be closed, +// but the generated schema just uses $ref to the open non-definition, +// losing closedness. +hiddenExtra: { + data: {t01: {h: 1, extra: true}} +} +sharedOpenExtra: { + data: {t02open: {v: 1, extra: true}} +} +sharedClosedValid: { + data: {t02closed: {v: 1}} +} +// BUG: this should not be accepted (same issue as hiddenExtra). +sharedClosedExtra: { + data: {t02closed: {v: 1, extra: true}} +} +conjunctionOpenExtra: { + data: {t03open: {h: 1, v: 1, extra: true}} +} +// BUG: this correctly rejects extra properties, but the generated schema +// is structurally wrong: additionalProperties: false sits alongside $ref +// with no local properties, rejecting all fields rather than only extra ones. +conjunctionClosedExtra: { + data: {t03closed: {h: 1, v: 1, extra: true}} + error: true +} +-- out/generate-v3/schema -- +{ + $schema: "https://json-schema.org/draft/2020-12/schema" + $defs: { + WrapsConjunction: { + allOf: [{ + $ref: "#/$defs/_hidden" + }, { + type: "object" + additionalProperties: false + $ref: "#/$defs/_shared" + }] + } + WrapsHidden: { + $ref: "#/$defs/_hidden" + } + WrapsShared: { + $ref: "#/$defs/_shared" + } + "_hidden": { + type: "object" + properties: { + h: { + type: "integer" + } + } + } + "_shared": { + type: "object" + properties: { + v: { + type: "integer" + } + } + } + } + type: "object" + properties: { + t01: { + $ref: "#/$defs/WrapsHidden" + } + t02closed: { + $ref: "#/$defs/WrapsShared" + } + t02open: { + $ref: "#/$defs/_shared" + } + t03closed: { + $ref: "#/$defs/WrapsConjunction" + } + t03open: { + allOf: [{ + $ref: "#/$defs/_hidden" + }, { + type: "object" + $ref: "#/$defs/_shared" + }] + } + } +} +-- out/generate-v3/hiddenValid -- +-- out/generate-v3/hiddenExtra -- +-- out/generate-v3/sharedOpenExtra -- +-- out/generate-v3/sharedClosedValid -- +-- out/generate-v3/sharedClosedExtra -- +-- out/generate-v3/conjunctionClosedExtra -- +conjunctionClosedExtra.data.t03closed: invalid value {h:1,v:1,extra:true} (does not satisfy matchN): 1 matched, expected 2: + 1:88 + ./datatest/testdata.cue:29:21 + 1:573 +conjunctionClosedExtra.data.t03closed.extra: field not allowed: + 1:88 + ./datatest/testdata.cue:29:41 +conjunctionClosedExtra.data.t03closed.h: field not allowed: + 1:88 + ./datatest/testdata.cue:29:25 +conjunctionClosedExtra.data.t03closed.v: field not allowed: + 1:88 + ./datatest/testdata.cue:29:31 + 1:373 +-- out/generate-v3/conjunctionOpenExtra -- diff --git a/encoding/jsonschema/testdata/generate/closedness_recursive.txtar b/encoding/jsonschema/testdata/generate/closedness_recursive.txtar new file mode 100644 index 000000000..24bb0de92 --- /dev/null +++ b/encoding/jsonschema/testdata/generate/closedness_recursive.txtar @@ -0,0 +1,273 @@ +# Tests for closedness with recursive references. +# +# BUG(4352): the struct element of the anyOf for Expr should have +# additionalProperties: false. +# +# BUG: WrapsNode should have additionalProperties: false because +# it's a definition. Currently it's just a $ref to the open +# non-definition. +-- test.cue -- +package test + +t01?: #Tree +t02?: #A +t03?: #Expr +t04?: _node +t05?: #WrapsNode +t06?: #ClosedLeft + +#Tree: { + value?: int + children?: [...#Tree] +} + +#A: { + x?: int + b?: #B +} + +#B: { + y?: string + a?: #A +} + +#Expr: int | { + op?: string + left?: #Expr + right?: #Expr +} + +_node: { + value?: int + next?: _node +} + +#WrapsNode: _node + +_left: { + value?: int + right?: _right +} + +_right: { + value?: string + left?: _left +} + +#ClosedLeft: _left + +-- datatest/testdata.cue -- +package datatest +treeValid: { + data: {t01: {value: 1, children: [{value: 2}]}} +} +treeExtra: { + data: {t01: {value: 1, extra: true}} + error: true +} +treeNestedExtra: { + data: {t01: {value: 1, children: [{value: 2, extra: true}]}} + error: true +} +mutualValid: { + data: {t02: {x: 1, b: {y: "hello", a: {x: 2}}}} +} +mutualExtra: { + data: {t02: {x: 1, extra: true}} + error: true +} +mutualDeepExtra: { + data: {t02: {x: 1, b: {y: "hello", extra: true}}} + error: true +} +exprValid: { + data: {t03: {op: "+", left: 1, right: 2}} +} +// BUG(4352): this should not be accepted because the struct disjunct +// of #Expr is closed, but the generated schema is missing +// additionalProperties: false. +exprExtra: { + data: {t03: {op: "+", left: 1, right: 2, extra: true}} +} +nodeValid: { + data: {t04: {value: 1, next: {value: 2}}} +} +nodeExtra: { + data: {t04: {value: 1, extra: true}} +} +wrapsNodeValid: { + data: {t05: {value: 1, next: {value: 2}}} +} +// BUG: this should not be accepted. #WrapsNode is a definition +// wrapping an open recursive non-definition. The definition should +// be closed. +wrapsNodeExtra: { + data: {t05: {value: 1, extra: true}} +} +// BUG: this should not be accepted because #WrapsNode is a definition +// wrapping a recursive non-definition, and nested values should be closed. +wrapsNodeNextExtra: { + data: {t05: {value: 1, next: {value: 2, extra: true}}} +} +closedLeftValid: { + data: {t06: {value: 1, right: {value: "hello", left: {value: 2}}}} +} +// BUG: these should not be accepted. #ClosedLeft is a definition +// wrapping open mutually-recursive non-definitions. +closedLeftExtra: { + data: {t06: {value: 1, extra: true}} +} +closedLeftRightExtra: { + data: {t06: {value: 1, right: {value: "hello", extra: true}}} +} +closedLeftRightLeftExtra: { + data: {t06: {value: 1, right: {value: "hello", left: {value: 2, extra: true}}}} +} +-- out/generate-v3/schema -- +{ + $schema: "https://json-schema.org/draft/2020-12/schema" + $defs: { + A: { + type: "object" + additionalProperties: false + properties: { + b: { + $ref: "#/$defs/B" + } + x: { + type: "integer" + } + } + } + B: { + type: "object" + additionalProperties: false + properties: { + a: { + $ref: "#/$defs/A" + } + y: { + type: "string" + } + } + } + ClosedLeft: { + $ref: "#/$defs/_left" + } + Expr: { + anyOf: [{ + type: "integer" + }, { + type: "object" + properties: { + left: { + $ref: "#/$defs/Expr" + } + op: { + type: "string" + } + right: { + $ref: "#/$defs/Expr" + } + } + }] + } + Tree: { + type: "object" + additionalProperties: false + properties: { + children: { + type: "array" + items: { + $ref: "#/$defs/Tree" + } + } + value: { + type: "integer" + } + } + } + WrapsNode: { + $ref: "#/$defs/_node" + } + "_left": { + type: "object" + properties: { + right: { + $ref: "#/$defs/_right" + } + value: { + type: "integer" + } + } + } + "_node": { + type: "object" + properties: { + next: { + $ref: "#/$defs/_node" + } + value: { + type: "integer" + } + } + } + "_right": { + type: "object" + properties: { + left: { + $ref: "#/$defs/_left" + } + value: { + type: "string" + } + } + } + } + type: "object" + properties: { + t01: { + $ref: "#/$defs/Tree" + } + t02: { + $ref: "#/$defs/A" + } + t03: { + $ref: "#/$defs/Expr" + } + t04: { + $ref: "#/$defs/_node" + } + t05: { + $ref: "#/$defs/WrapsNode" + } + t06: { + $ref: "#/$defs/ClosedLeft" + } + } +} +-- out/generate-v3/treeValid -- +-- out/generate-v3/treeExtra -- +treeExtra.data.t01.extra: field not allowed: + ./datatest/testdata.cue:6:25 +-- out/generate-v3/treeNestedExtra -- +treeNestedExtra.data.t01.children.0.extra: field not allowed: + ./datatest/testdata.cue:10:47 +-- out/generate-v3/mutualValid -- +-- out/generate-v3/mutualExtra -- +mutualExtra.data.t02.extra: field not allowed: + ./datatest/testdata.cue:17:21 +-- out/generate-v3/mutualDeepExtra -- +mutualDeepExtra.data.t02.b.extra: field not allowed: + ./datatest/testdata.cue:21:37 +-- out/generate-v3/exprValid -- +-- out/generate-v3/exprExtra -- +-- out/generate-v3/nodeValid -- +-- out/generate-v3/nodeExtra -- +-- out/generate-v3/wrapsNodeValid -- +-- out/generate-v3/wrapsNodeExtra -- +-- out/generate-v3/closedLeftExtra -- +-- out/generate-v3/closedLeftRightExtra -- +-- out/generate-v3/closedLeftRightLeftExtra -- +-- out/generate-v3/closedLeftValid -- +-- out/generate-v3/wrapsNodeNextExtra -- diff --git a/encoding/jsonschema/testdata/generate/closedness_root.txtar b/encoding/jsonschema/testdata/generate/closedness_root.txtar new file mode 100644 index 000000000..6747d36da --- /dev/null +++ b/encoding/jsonschema/testdata/generate/closedness_root.txtar @@ -0,0 +1,40 @@ +# Tests for root-level definition reference closedness. +# +# Issue 4356: when the root value is solely a reference to a definition, +# additionalProperties: false and type: "object" should not appear at +# the root because they conflict with the $ref (additionalProperties +# only considers properties in the same schema object, and the root +# has none). +# +# BUG(4356): the type and additionalProperties keywords should +# not be present at the root; the root should be solely a $ref. +-- test.cue -- +package test + +#T + +#T: { + a?: int + b?: string +} +-- out/generate-v3/schema -- +{ + $schema: "https://json-schema.org/draft/2020-12/schema" + $defs: { + T: { + type: "object" + additionalProperties: false + properties: { + a: { + type: "integer" + } + b: { + type: "string" + } + } + } + } + type: "object" + additionalProperties: false + $ref: "#/$defs/T" +} diff --git a/encoding/jsonschema/testdata/generate/closedness_root_required.txtar b/encoding/jsonschema/testdata/generate/closedness_root_required.txtar new file mode 100644 index 000000000..5c6503a2b --- /dev/null +++ b/encoding/jsonschema/testdata/generate/closedness_root_required.txtar @@ -0,0 +1,39 @@ +# Tests for root-level definition reference with required fields. +# +# Issue 4356: same as closedness_root but with required fields. +# +# BUG(4356): the type and additionalProperties keywords should +# not be present at the root; the root should be solely a $ref. +# +#brokenRoundTrip +-- test.cue -- +package test + +#T + +#T: { + a!: int + b!: int +} +-- out/generate-v3/schema -- +{ + $schema: "https://json-schema.org/draft/2020-12/schema" + $defs: { + T: { + type: "object" + additionalProperties: false + properties: { + a: { + type: "integer" + } + b: { + type: "integer" + } + } + required: ["a", "b"] + } + } + type: "object" + additionalProperties: false + $ref: "#/$defs/T" +}