Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

evaluator: encoding of JSON Schema (and other) oneofs #3165

Open
myitcv opened this issue May 22, 2024 · 3 comments
Open

evaluator: encoding of JSON Schema (and other) oneofs #3165

myitcv opened this issue May 22, 2024 · 3 comments

Comments

@myitcv
Copy link
Member

myitcv commented May 22, 2024

What version of CUE are you using (cue version)?

$ cue version
cue version v0.0.0-20240522083645-3ce48c0beadf

go version go1.22.3
      -buildmode exe
       -compiler gc
  DefaultGODEBUG httplaxcontentlength=1,httpmuxgo121=1,tls10server=1,tlsrsakex=1,tlsunsafeekm=1
     CGO_ENABLED 1
          GOARCH arm64
            GOOS linux
             vcs git
    vcs.revision 3ce48c0beadfc8b3d22285a197c9ebd53f4bd59a
        vcs.time 2024-05-22T08:36:45Z
    vcs.modified false
cue.lang.version v0.9.0

Does this issue reproduce with the latest release?

Yes

What did you do?

Capturing an issue that deals with how to encode one-ofs from JSON Schema, protocol buffers (and potentially other languages). #943 suggests one approach for encoding oneofs; this issue is a more general exploration of the space.

The example in question is from JSON Schema:

{
  "oneOf": [
    {
      "required": [
        "x"
      ]
    },
    {
      "required": [
        "y"
      ]
    }
  ]
}

As can be seen from https://jsonschema.dev/s/5KO7a, this validates successfully when one field is present, and raises an error when both are specified https://jsonschema.dev/s/209Bo.

Turning to how this can be represented in CUE we have, AFAICT, two ways of doing this today:

// approach 1
_constraint: {x!: int} | {y!: int}
// approach 2
_constraint: {x!: int, y?: _|_} | {y!: int, x?: _|_}

Looking at how each behaves with both the old and new evaluator we see the following:

# Old evaluator - approach 1
env CUE_EXPERIMENT=''
! exec cue export x.cue approach1.cue
cmp stderr old_1_stderr.golden

# Old evaluator - approach 2
env CUE_EXPERIMENT=''
exec cue export x.cue approach2.cue
cmp stdout old_2_stdout.golden

# New evaluator - approach 1
env CUE_EXPERIMENT='evalv3'
! exec cue export x.cue approach1.cue
cmp stderr new_1_stderr.golden

# Old evaluator - approach 2
env CUE_EXPERIMENT='evalv3'
exec cue export x.cue approach2.cue
cmp stdout new_2_stdout.golden

-- approach1.cue --
package x

_constraint: {x!: int} | {y!: int}
-- approach2.cue --
package x

_constraint: {x!: int, y?: _|_} | {y!: int, x?: _|_}
-- x.cue --
package x

res: _constraint & {
	x: 5
}
-- old_1_stderr.golden --
res: incomplete value {x:5} | {x:5,y!:int}
-- new_1_stderr.golden --
res: incomplete value {x:5} | {x:5,y!:int}
-- old_2_stdout.golden --
{
    "res": {
        "x": 5
    }
}
-- new_2_stdout.golden --
{
    "res": {
        "x": 5
    }
}

What did you expect to see?

Unclear.

Export of approach 2 succeeds, but I don't think it is a scaleable approach to encoding oneofs. It is also not readily understandable by humans. It's not incorrect however that the cue export succeeds in this case.

Export of approach 1 fails. Given my understanding of when the required field is "checked", I think this therefore qualifies as "behaving as expected". But I'm not clear that this is the behaviour we want. I think it's reasonable to make the argument that cue export explicitly/implicitly says "I am not going to provide you with anything more". Through that lens, one could argue that the {y!: int} disjunct in approach 1 can be eliminated, because it fails with respect to {x: 5}, which would then allow the export to succeed.

Hence there's an argument that the test should fail, because (if you will excuse the double negative) approach 1 should succeed and give the same output as approach 2.

What did you see instead?

Passing test.

Related issues

In raising this issue I suggest we consolidate conversation from the following issues:

Update: those issues have now been closed to consolidate discussion here.

@myitcv
Copy link
Member Author

myitcv commented Jul 22, 2024

@rogpeppe also flags this canonical examples from JSON Schema: https://json-schema.org/understanding-json-schema/reference/combining#oneOf

{
  "oneOf": [
    { "type": "number", "multipleOf": 5 },
    { "type": "number", "multipleOf": 3 }
  ]
}

@chiragjn
Copy link

Adding one more example
We are facing troubles with correct code generation because oneOf has allOf + not anyOf

import "strings"

#SpecialText: {
  type: "special",
  text: string & =~"^.{1,}$"
}

#MyText: {
  content: strings.MinRunes(1) | #SpecialText
}
"components": {
        "schemas": {
            "MyText": {
                "type": "object",
                "required": [
                    "content"
                ],
                "properties": {
                    "content": {
                        "oneOf": [
                            {
                                "allOf": [
                                    {
                                        "type": "string",
                                        "minLength": 1
                                    },
                                    {
                                        "not": {
                                            "anyOf": [
                                                {
                                                    "$ref": "#/components/schemas/SpecialText"
                                                }
                                            ]
                                        }
                                    }
                                ]
                            },
                            {
                                "$ref": "#/components/schemas/SpecialText"
                            }
                        ]
                    }
                }
            },
            "SpecialText": {
                "type": "object",
                "required": [
                    "type",
                    "text"
                ],
                "properties": {
                    "type": {
                        "type": "string",
                        "enum": [
                            "special"
                        ]
                    },
                    "text": {
                        "type": "string",
                        "pattern": "^.{1,}$"
                    }
                }
            }
        }
    }

@myitcv
Copy link
Member Author

myitcv commented Aug 19, 2024

I've just raised #3380 to track the implementation of oneof and related constraints using the new matchN builtin.

cueckoo pushed a commit that referenced this issue Aug 19, 2024
The matchN validator can be used to implement a host
of useful validators, including the JSON schema
equavalent of "not", "oneOf", and "anyOf".

Issue #3176
Issue #3165

Signed-off-by: Marcel van Lohuizen <[email protected]>
Change-Id: I904bfe38a09a8a83d08a09d46866a69f87c8cc7c
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1198922
Unity-Result: CUE porcuepine <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>
Reviewed-by: Roger Peppe <[email protected]>
cueckoo pushed a commit that referenced this issue Sep 3, 2024
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%
```

This change also requires that we update the CI generated code
and remove the workaround for the previous `oneOf` limitation.

Also add a `brokenInV2` tag to the encoding/jsonschema tests to work
around the fact that some V2 tests are now broken.

For #3380
For #3165

Signed-off-by: Roger Peppe <[email protected]>
Change-Id: I2630a6d2b1614b2479802e788c16249d2cf4aa6b
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1200526
Reviewed-by: Daniel Martí <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>
cueckoo pushed a commit that referenced this issue Oct 8, 2024
Now Validators can be NonConcrete, arguments are
not necessarily fully evaluated. This is now addressed.

The also prevents other issues in an upcoming CL.

Issue #3165
Fixes #3418

Signed-off-by: Marcel van Lohuizen <[email protected]>
Change-Id: Ie103c9710eb9a89ee796a2486a9f4d365760f404
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1202211
Reviewed-by: Matthew Sackman <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>
Unity-Result: CUE porcuepine <[email protected]>
cueckoo pushed a commit that referenced this issue Oct 8, 2024
This is needed to pass various tests.

Bugs still remains, but the main point is that it
unblocks a next phase in structure sharing.

Issue #2854
Issue #2850
Issue #3165
Fixes #3410
Fixes #3420
Issue #3443

Signed-off-by: Marcel van Lohuizen <[email protected]>
Change-Id: Ibd2a41e25e07bd37899620af6bd9665435d68e8a
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1202212
TryBot-Result: CUEcueckoo <[email protected]>
Reviewed-by: Matthew Sackman <[email protected]>
Unity-Result: CUE porcuepine <[email protected]>
vanhtuan0409 pushed a commit to anduintransaction/cue that referenced this issue Oct 15, 2024
Now Validators can be NonConcrete, arguments are
not necessarily fully evaluated. This is now addressed.

The also prevents other issues in an upcoming CL.

Issue cue-lang#3165
Fixes cue-lang#3418

Signed-off-by: Marcel van Lohuizen <[email protected]>
Change-Id: Ie103c9710eb9a89ee796a2486a9f4d365760f404
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1202211
Reviewed-by: Matthew Sackman <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>
Unity-Result: CUE porcuepine <[email protected]>
vanhtuan0409 pushed a commit to anduintransaction/cue that referenced this issue Oct 15, 2024
This is needed to pass various tests.

Bugs still remains, but the main point is that it
unblocks a next phase in structure sharing.

Issue cue-lang#2854
Issue cue-lang#2850
Issue cue-lang#3165
Fixes cue-lang#3410
Fixes cue-lang#3420
Issue cue-lang#3443

Signed-off-by: Marcel van Lohuizen <[email protected]>
Change-Id: Ibd2a41e25e07bd37899620af6bd9665435d68e8a
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1202212
TryBot-Result: CUEcueckoo <[email protected]>
Reviewed-by: Matthew Sackman <[email protected]>
Unity-Result: CUE porcuepine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants