Skip to content

Commit

Permalink
encoding/jsonschema: encode oneOf etc using matchN
Browse files Browse the repository at this point in the history 
This CL updates JSON Schema generation to use the newly added
`matchN` primitive for `allOf`, `anyOf` and `oneOf`.

This has a much closer correlation with the JSON Schema primitives
than the current approach of using CUE's `&` and `|` operators.
Specifically:
- the schema arguments should not affect the final result other
than to validate it, but both `&` and `|` can affect the result.
- the result could become non-concrete due to `|` ambiguity in `anyOf`.

Although this does fix a bunch of issues, there are some regressions.
See issues #3418, #3420, #3422 for details.

Comparative test stats (pre-matchN before post-matchN):

```
v2:
	schema extract (pass / total): 971 / 1637 = 59.3%
	schema extract (pass / total): 975 / 1637 = 59.6%
	tests (pass / total): 3081 / 7175 = 42.9%
	tests (pass / total): 3140 / 7175 = 43.8%
	tests on extracted schemas (pass / total): 3081 / 3542 = 87.0%
	tests on extracted schemas (pass / total): 3140 / 3546 = 88.6%

v3:
	schema extract (pass / total): 971 / 1637 = 59.3%
	schema extract (pass / total): 967 / 1637 = 59.1%
	tests (pass / total): 3063 / 7175 = 42.7%
	tests (pass / total): 3074 / 7175 = 42.8%
	tests on extracted schemas (pass / total): 3063 / 3542 = 86.5%
	tests on extracted schemas (pass / total): 3074 / 3538 = 86.9%
```

For #3380
For #3165

Signed-off-by: Roger Peppe <rogpeppe@gmail.com>
Change-Id: I2630a6d2b1614b2479802e788c16249d2cf4aa6b
Dispatch-Trailer: {"type":"trybot","CL":1200526,"patchset":1,"ref":"refs/changes/26/1200526/1","targetBranch":"master"}
  • Loading branch information
@rogpeppe @cueckoo
rogpeppe authored and cueckoo committed Sep 2, 2024
1 parent d6ad8dc commit 56e2bde
Show file tree
Hide file tree
Showing 30 changed files with 372 additions and 322 deletions.
69 changes: 57 additions & 12 deletions encoding/jsonschema/constraints_combinator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,28 @@
package jsonschema

import (
"strconv"

"cuelang.org/go/cue"
"cuelang.org/go/cue/ast"
"cuelang.org/go/cue/token"
)

// TODO We're invoking the `matchN` builtin in these constaints but what
// if there's a field named "matchN" in scope ? How can we guard against
// that while avoiding the use of __matchN in the usual case?

// Constraint combinators.

func constraintAllOf(key string, n cue.Value, s *state) {
var a []ast.Expr
var knownTypes cue.Kind
for _, v := range s.listItems("allOf", n, false) {
items := s.listItems("allOf", n, false)
if len(items) == 0 {
s.errf(n, "allOf requires at least one subschema")
return
}
a := make([]ast.Expr, 0, len(items))
for _, v := range items {
x, sub := s.schemaState(v, s.allowedTypes, nil, true)
s.allowedTypes &= sub.allowedTypes
if sub.hasConstraints() {
Expand All @@ -44,33 +55,60 @@ func constraintAllOf(key string, n cue.Value, s *state) {
// as that's a known-impossible assertion?
if len(a) > 0 {
s.knownTypes &= knownTypes
s.all.add(n, ast.NewBinExpr(token.AND, a...))
s.all.add(n, ast.NewCall(
ast.NewIdent("matchN"),
// TODO it would be nice to be able to use a special sentinel "all" value
// here rather than redundantly encoding the length of the list.
&ast.BasicLit{
Kind: token.INT,
Value: strconv.Itoa(len(items)),
},
ast.NewList(a...),
))
}
}

func constraintAnyOf(key string, n cue.Value, s *state) {
var types cue.Kind
var a []ast.Expr
var knownTypes cue.Kind
for _, v := range s.listItems("anyOf", n, false) {
items := s.listItems("anyOf", n, false)
if len(items) == 0 {
s.errf(n, "anyOf requires at least one subschema")
return
}
a := make([]ast.Expr, 0, len(items))
for _, v := range items {
x, sub := s.schemaState(v, s.allowedTypes, nil, true)
types |= sub.allowedTypes
knownTypes |= sub.knownTypes
a = append(a, x)
}
s.allowedTypes &= types
if len(a) > 0 {
s.knownTypes &= knownTypes
s.all.add(n, ast.NewBinExpr(token.OR, a...))
}
s.knownTypes &= knownTypes
s.all.add(n, ast.NewCall(
ast.NewIdent("matchN"),
&ast.UnaryExpr{
Op: token.GEQ,
X: &ast.BasicLit{
Kind: token.INT,
Value: "1",
},
},
ast.NewList(a...),
))
}

func constraintOneOf(key string, n cue.Value, s *state) {
var types cue.Kind
var knownTypes cue.Kind
var a []ast.Expr
hasSome := false
for _, v := range s.listItems("oneOf", n, false) {
items := s.listItems("oneOf", n, false)
if len(items) == 0 {
s.errf(n, "oneOf requires at least one subschema")
return
}
a := make([]ast.Expr, 0, len(items))
for _, v := range items {
x, sub := s.schemaState(v, s.allowedTypes, nil, true)
types |= sub.allowedTypes

Expand All @@ -89,7 +127,14 @@ func constraintOneOf(key string, n cue.Value, s *state) {
s.allowedTypes &= types
if len(a) > 0 && hasSome {
s.knownTypes &= knownTypes
s.all.add(n, ast.NewBinExpr(token.OR, a...))
s.all.add(n, ast.NewCall(
ast.NewIdent("matchN"),
&ast.BasicLit{
Kind: token.INT,
Value: "1",
},
ast.NewList(a...),
))
}

// TODO: oneOf({a:x}, {b:y}, ..., not(anyOf({a:x}, {b:y}, ...))),
Expand Down
6 changes: 5 additions & 1 deletion encoding/jsonschema/decode_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,10 @@ func TestDecode(t *testing.T) {
test.Run(t, func(t *cuetxtar.Test) {
cfg := &jsonschema.Config{}

if t.HasTag("brokenInV2") && t.M.Name() == "v2" {
t.Skip("skipping because test is broken under the v2 evaluator")
}

if t.HasTag("openapi") {
cfg.Root = "#/components/schemas/"
cfg.Map = func(p token.Pos, a []string) ([]ast.Label, error) {
Expand Down Expand Up @@ -119,7 +123,7 @@ func TestDecode(t *testing.T) {
// Verify that the generated CUE compiles.
schemav := ctx.CompileBytes(b, cue.Filename("generated.cue"))
if err := schemav.Err(); err != nil {
t.Fatal(errors.Details(err, nil))
t.Fatal(errors.Details(err, nil), qt.Commentf("generated code: %q", b))
}
testEntries, err := fs.ReadDir(fsys, "test")
if err != nil {
Expand Down
4 changes: 2 additions & 2 deletions encoding/jsonschema/testdata/external/tests/draft2019-09/additionalItems.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -230,8 +230,8 @@
],
"valid": true,
"skip": {
"v2": "5 errors in empty disjunction:\nconflicting values [1,null] and {...} (mismatched types list and struct):\n generated.cue:2:1\n generated.cue:2:41\n instance.json:1:1\nconflicting values bool and [1,null] (mismatched types bool and list):\n generated.cue:2:8\n instance.json:1:1\nconflicting values null and [1,null] (mismatched types null and list):\n generated.cue:2:1\n instance.json:1:1\nconflicting values number and [1,null] (mismatched types number and list):\n generated.cue:2:15\n instance.json:1:1\nconflicting values string and [1,null] (mismatched types string and list):\n generated.cue:2:24\n instance.json:1:1\n",
"v3": "conflicting values [1,null] and {...} (mismatched types list and struct):\n generated.cue:2:41\n instance.json:1:1\nconflicting values bool and [1,null] (mismatched types bool and list):\n generated.cue:2:8\n instance.json:1:1\nconflicting values null and [1,null] (mismatched types null and list):\n generated.cue:2:1\n instance.json:1:1\nconflicting values number and [1,null] (mismatched types number and list):\n generated.cue:2:15\n instance.json:1:1\nconflicting values string and [1,null] (mismatched types string and list):\n generated.cue:2:24\n instance.json:1:1\nincompatible list lengths (1 and 2):\n instance.json:1:1\n"
"v2": "invalid value [1,null] (does not satisfy matchN(1, [null | bool | number | string | [int] | {}])): 0 matched, expected 1:\n generated.cue:2:1\n generated.cue:1:1\n generated.cue:2:8\n instance.json:1:1\n",
"v3": "invalid value [1,null] (does not satisfy matchN(1, [null | bool | number | string | [int] | {}])): 0 matched, expected 1:\n generated.cue:2:1\n generated.cue:1:1\n generated.cue:2:8\n instance.json:1:1\n"
}
}
]
Expand Down
30 changes: 24 additions & 6 deletions encoding/jsonschema/testdata/external/tests/draft2019-09/allOf.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,20 @@
"data": {
"foo": "baz"
},
"valid": false
"valid": false,
"skip": {
"v3": "unexpected success"
}
},
{
"description": "mismatch first",
"data": {
"bar": 2
},
"valid": false
"valid": false,
"skip": {
"v3": "unexpected success"
}
},
{
"description": "wrong type",
Expand Down Expand Up @@ -118,22 +124,31 @@
"bar": 2,
"baz": null
},
"valid": false
"valid": false,
"skip": {
"v3": "unexpected success"
}
},
{
"description": "mismatch second allOf",
"data": {
"foo": "quux",
"bar": 2
},
"valid": false
"valid": false,
"skip": {
"v3": "unexpected success"
}
},
{
"description": "mismatch both",
"data": {
"bar": 2
},
"valid": false
"valid": false,
"skip": {
"v3": "unexpected success"
}
}
]
},
Expand Down Expand Up @@ -369,7 +384,10 @@
{
"description": "allOf: false, anyOf: true, oneOf: true",
"data": 15,
"valid": false
"valid": false,
"skip": {
"v2": "unexpected success"
}
},
{
"description": "allOf: true, anyOf: false, oneOf: false",
Expand Down
24 changes: 10 additions & 14 deletions encoding/jsonschema/testdata/external/tests/draft2019-09/anyOf.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,17 @@
false
]
},
"skip": {
"v3": "extract error: cannot compile resulting schema: explicit error (_|_ literal) in source:\n generated.cue:2:17\n"
},
"tests": [
{
"description": "any value is valid",
"data": "foo",
"valid": true
"valid": true,
"skip": {
"v3": "could not compile schema"
}
}
]
},
Expand All @@ -111,16 +117,14 @@
]
},
"skip": {
"v2": "extract error: cannot compile resulting schema: 2 errors in empty disjunction:\nexplicit error (_|_ literal) in source:\n generated.cue:2:1\nexplicit error (_|_ literal) in source:\n generated.cue:2:7\n",
"v3": "extract error: cannot compile resulting schema: explicit error (_|_ literal) in source:\n generated.cue:2:1\nexplicit error (_|_ literal) in source:\n generated.cue:2:7\n"
"v3": "extract error: cannot compile resulting schema: explicit error (_|_ literal) in source:\n generated.cue:2:14\n"
},
"tests": [
{
"description": "any value is invalid",
"data": "foo",
"valid": false,
"skip": {
"v2": "could not compile schema",
"v3": "could not compile schema"
}
}
Expand Down Expand Up @@ -159,22 +163,14 @@
"data": {
"bar": 2
},
"valid": true,
"skip": {
"v2": "incomplete value {bar:2} | {bar:2,foo!:string}\n",
"v3": "incomplete value {bar:2} | {bar:2,foo!:string}\n"
}
"valid": true
},
{
"description": "second anyOf valid (complex)",
"data": {
"foo": "baz"
},
"valid": true,
"skip": {
"v2": "incomplete value {foo:\"baz\",bar!:int} | {foo:\"baz\"}\n",
"v3": "incomplete value {foo:\"baz\",bar!:int} | {foo:\"baz\"}\n"
}
"valid": true
},
{
"description": "both anyOf valid (complex)",
Expand Down
48 changes: 10 additions & 38 deletions encoding/jsonschema/testdata/external/tests/draft2019-09/oneOf.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,7 @@
{
"description": "both oneOf valid",
"data": 3,
"valid": false,
"skip": {
"v2": "unexpected success",
"v3": "unexpected success"
}
"valid": false
},
{
"description": "neither oneOf valid",
Expand Down Expand Up @@ -67,11 +63,7 @@
{
"description": "both oneOf valid",
"data": "foo",
"valid": false,
"skip": {
"v2": "unexpected success",
"v3": "unexpected success"
}
"valid": false
}
]
},
Expand Down Expand Up @@ -194,8 +186,7 @@
},
"valid": true,
"skip": {
"v2": "incomplete value {bar:2} | {bar:2,foo!:string}\n",
"v3": "incomplete value {bar:2} | {bar:2,foo!:string}\n"
"v3": "invalid value {bar:2} (does not satisfy matchN(1, [null | bool | number | string | [] | {bar!:int},null | bool | number | string | [] | {foo!:string}])): 2 matched, expected 1:\n generated.cue:2:1\n generated.cue:1:1\n generated.cue:2:8\n instance.json:1:1\n"
}
},
{
Expand All @@ -205,8 +196,7 @@
},
"valid": true,
"skip": {
"v2": "incomplete value {foo:\"baz\",bar!:int} | {foo:\"baz\"}\n",
"v3": "incomplete value {foo:\"baz\",bar!:int} | {foo:\"baz\"}\n"
"v3": "invalid value {foo:\"baz\"} (does not satisfy matchN(1, [null | bool | number | string | [] | {bar!:int},null | bool | number | string | [] | {foo!:string}])): 2 matched, expected 1:\n generated.cue:2:1\n generated.cue:1:1\n generated.cue:2:8\n instance.json:1:1\n"
}
},
{
Expand All @@ -215,11 +205,7 @@
"foo": "baz",
"bar": 2
},
"valid": false,
"skip": {
"v2": "unexpected success",
"v3": "unexpected success"
}
"valid": false
},
{
"description": "neither oneOf valid (complex)",
Expand Down Expand Up @@ -293,23 +279,15 @@
"foo": 1,
"bar": 2
},
"valid": true,
"skip": {
"v2": "incomplete value {foo:1,bar:2} | {foo:1,bar:2,baz!:_}\n",
"v3": "incomplete value {foo:1,bar:2} | {foo:1,bar:2,baz!:_}\n"
}
"valid": true
},
{
"description": "second valid - valid",
"data": {
"foo": 1,
"baz": 3
},
"valid": true,
"skip": {
"v2": "incomplete value {foo:1,baz:3,bar!:_} | {foo:1,baz:3}\n",
"v3": "incomplete value {foo:1,baz:3,bar!:_} | {foo:1,baz:3}\n"
}
"valid": true
},
{
"description": "both valid - invalid",
Expand All @@ -318,11 +296,7 @@
"bar": 2,
"baz": 3
},
"valid": false,
"skip": {
"v2": "unexpected success",
"v3": "unexpected success"
}
"valid": false
}
]
},
Expand Down Expand Up @@ -358,8 +332,7 @@
},
"valid": true,
"skip": {
"v2": "incomplete value {bar:8,baz?:_} | {bar:8,foo!:_}\n",
"v3": "incomplete value {bar:8,baz?:_} | {bar:8,foo!:_}\n"
"v3": "invalid value {bar:8} (does not satisfy matchN(1, [null | bool | number | string | [] | {bar!:_,baz?:_},null | bool | number | string | [] | {foo!:_}])): 2 matched, expected 1:\n generated.cue:2:1\n generated.cue:1:1\n generated.cue:2:8\n instance.json:1:1\n"
}
},
{
Expand All @@ -369,8 +342,7 @@
},
"valid": true,
"skip": {
"v2": "incomplete value {foo:\"foo\",bar!:_,baz?:_} | {foo:\"foo\"}\n",
"v3": "incomplete value {foo:\"foo\",bar!:_,baz?:_} | {foo:\"foo\"}\n"
"v3": "invalid value {foo:\"foo\"} (does not satisfy matchN(1, [null | bool | number | string | [] | {bar!:_,baz?:_},null | bool | number | string | [] | {foo!:_}])): 2 matched, expected 1:\n generated.cue:2:1\n generated.cue:1:1\n generated.cue:2:8\n instance.json:1:1\n"
}
},
{
Expand Down
Loading

0 comments on commit 56e2bde

Please sign in to comment.