Skip to content

Commit

Permalink
encoding/jsonschema: use allowedTypes in if/then/else constraints
Browse files Browse the repository at this point in the history
Unlike, `allOf` and friends, the `if/then/else` logic was not benefiting
from the fact that we can eliminate some elements from the disjunctions
created in the subschemas. By doing this, we improve performance
in some schemas by quite a bit (for example, it improved vet time
on one of the SchemaStore schemas [1] by 40%).

[1] https://github.com/SchemaStore/schemastore/blob/64f44fd870573eff20d44582f83edac5907e543e/src/schemas/json/dependabot-2.0.json

Signed-off-by: Roger Peppe <[email protected]>
Change-Id: I0e4aca10c1bec02cb6820db1c17953b21bd29760
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1202609
TryBot-Result: CUEcueckoo <[email protected]>
Unity-Result: CUE porcuepine <[email protected]>
Reviewed-by: Daniel Martí <[email protected]>
  • Loading branch information
rogpeppe committed Oct 15, 2024
1 parent 72023e6 commit 16986b2
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 31 deletions.
38 changes: 35 additions & 3 deletions encoding/jsonschema/constraints_combinator.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,13 +177,45 @@ func constraintNot(key string, n cue.Value, s *state) {
}

func constraintIf(key string, n cue.Value, s *state) {
s.ifConstraint = s.schema(n)
s.ifConstraint = n
}

func constraintThen(key string, n cue.Value, s *state) {
s.thenConstraint = s.schema(n)
s.thenConstraint = n
}

func constraintElse(key string, n cue.Value, s *state) {
s.elseConstraint = s.schema(n)
s.elseConstraint = n
}

// constraintIfThenElse is not implemented as a standard constraint
// function because it needs to operate knowing about the presence
// of all of "if", "then" and "else".
func constraintIfThenElse(s *state) {
hasIf, hasThen, hasElse := s.ifConstraint.Exists(), s.thenConstraint.Exists(), s.elseConstraint.Exists()
if !hasIf || (!hasThen && !hasElse) {
return
}
var ifExpr, thenExpr, elseExpr ast.Expr
ifExpr, ifSub := s.schemaState(s.ifConstraint, s.allowedTypes, nil)
if hasThen {
// The allowed types of the "then" constraint are constrained both
// by the current constraints and the "if" constraint.
thenExpr, _ = s.schemaState(s.thenConstraint, s.allowedTypes&ifSub.allowedTypes, nil)
}
if hasElse {
elseExpr, _ = s.schemaState(s.elseConstraint, s.allowedTypes, nil)
}
if thenExpr == nil {
thenExpr = top()
}
if elseExpr == nil {
elseExpr = top()
}
s.all.add(s.pos, ast.NewCall(
ast.NewIdent("matchIf"),
ifExpr,
thenExpr,
elseExpr,
))
}
26 changes: 4 additions & 22 deletions encoding/jsonschema/decode.go
Original file line number Diff line number Diff line change
Expand Up @@ -423,9 +423,9 @@ type state struct {
minContains *uint64
maxContains *uint64

ifConstraint ast.Expr
thenConstraint ast.Expr
elseConstraint ast.Expr
ifConstraint cue.Value
thenConstraint cue.Value
elseConstraint cue.Value

schemaVersion Version
schemaVersionPresent bool
Expand Down Expand Up @@ -501,7 +501,6 @@ func (s *state) finalize() (e ast.Expr) {
// we might be inside an allOf or oneOf with other valid constraints.
return bottom()
}
s.addIfThenElse()

conjuncts := []ast.Expr{}
disjuncts := []ast.Expr{}
Expand Down Expand Up @@ -651,24 +650,6 @@ outer:
return e
}

func (s *state) addIfThenElse() {
if s.ifConstraint == nil || (s.thenConstraint == nil && s.elseConstraint == nil) {
return
}
if s.thenConstraint == nil {
s.thenConstraint = top()
}
if s.elseConstraint == nil {
s.elseConstraint = top()
}
s.all.add(s.pos, ast.NewCall(
ast.NewIdent("matchIf"),
s.ifConstraint,
s.thenConstraint,
s.elseConstraint,
))
}

func (s *state) comment() *ast.CommentGroup {
// Create documentation.
doc := strings.TrimSpace(s.title)
Expand Down Expand Up @@ -782,6 +763,7 @@ func (s0 *state) schemaState(n cue.Value, types cue.Kind, idRef []label) (ast.Ex
c.fn(key, value, s)
})
}
constraintIfThenElse(s)

return s.finalize(), s
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -748,8 +748,8 @@
}
},
"skip": {
"v2": "extract error: keyword \"$dynamicAnchor\" not yet implemented (and 3 more errors)",
"v3": "extract error: keyword \"$dynamicAnchor\" not yet implemented (and 3 more errors)"
"v2": "extract error: keyword \"$dynamicRef\" not yet implemented (and 3 more errors)",
"v3": "extract error: keyword \"$dynamicRef\" not yet implemented (and 3 more errors)"
},
"tests": [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1422,8 +1422,8 @@
"unevaluatedItems": false
},
"skip": {
"v2": "extract error: keyword \"prefixItems\" not yet implemented (and 1 more errors)",
"v3": "extract error: keyword \"prefixItems\" not yet implemented (and 1 more errors)"
"v2": "extract error: keyword \"unevaluatedItems\" not yet implemented",
"v3": "extract error: keyword \"unevaluatedItems\" not yet implemented"
},
"tests": [
{
Expand Down
4 changes: 2 additions & 2 deletions encoding/jsonschema/testdata/txtar/ifthenelse.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,10 @@
}
-- out/decode/extract --
@jsonschema(schema="https://json-schema.org/draft/2019-09/schema")
matchIf(null | bool | number | string | [...] | {
matchIf({
a!: number
...
}, null | bool | number | string | [...] | {
}, {
b!: number
...
}, _) & {
Expand Down

0 comments on commit 16986b2

Please sign in to comment.