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

Meta-schema for enum should allow empty and non-unique arrays #717

Closed
Zac-HD opened this issue Feb 24, 2019 · 11 comments
Closed

Meta-schema for enum should allow empty and non-unique arrays #717

Zac-HD opened this issue Feb 24, 2019 · 11 comments

Comments

@Zac-HD
Copy link

Zac-HD commented Feb 24, 2019

Currently, the meta-schemas for (at least) drafts 3, 4, 6, and 7 contain the following:

"enum": {
"type": "array",
"items": true,
"minItems": 1,
"uniqueItems": true
},

However, the prose specification says:

The value of this [i.e. the "enum"] keyword MUST be an array.
This array SHOULD have at least one element. Elements in the array SHOULD be unique.

Validation therefore ought to allow schema authors to use empty or non-unique arrays, because the RFC 2119 definition of SHOULD explicitly permits ignoring such clauses after considering the circumstances. Alternatively, the specification could replace SHOULD with MUST for the relevant constraints.

Possibly related to #618? First reported at python-jsonschema/jsonschema#529, via hypothesis-jsonschema.

@handrews
Copy link
Contributor

For such short documents, the number of meta-schema bugs is astounding :-/
(I say that having been responsible for a fair number of them, sadly).

Certainly related to #618 in that it would be fantastic if someone implemented #618.

I supposed we should update them for this (and I think writeOnly is missing from draft-07 as well, although I can't find a bug on it). Which will annoy some people and please others- next draft we move to date-based versioning for meta-schemas (#714) so we can just add new versions with less confusion. Hopefully.

The good news is that the meta-schemas are not normative, so feel free to edit your local copy.

@handrews handrews added this to the draft-08 milestone Feb 27, 2019
@Relequestual Relequestual self-assigned this Apr 11, 2019
Relequestual added a commit to Relequestual/json-schema-spec that referenced this issue May 21, 2019
Issue: json-schema-org#717

enum no longer requires minimum number of items or unique items, in accordance with the spec.
@Relequestual
Copy link
Member

@Zac-HD Correctness is good, but I have to ask, do you have a use case for this?
I can think of one, but it's not very strong.

@Zac-HD
Copy link
Author

Zac-HD commented May 28, 2019

I have hypothesis-jsonschema, which takes a schema and produces documents that validate against it. I would like to generate schemas from the meta-schemas.

More generally though, the sole purpose of validation libraries is correctness. If we can't implement the spec that actually exists, why would anyone choose to use jsonschema?

@Relequestual
Copy link
Member

Ah OK, yup that makes sense.

And yes, of course, I agree. I wasn't suggesting we don't fix the issue (I made a PR and it's merged already before my comment). I was more asking, should we change the spec to require this or not.

The meta-schemas are always informative as opposed to normative, so if the meta-schema doesn't line up with the spec, we have a problem, and should fix it.

@Relequestual
Copy link
Member

@Zac-HD Given #739 is merged, can we close this issue as resolved? =D

@Zac-HD
Copy link
Author

Zac-HD commented May 31, 2019

Yep, I'd certainly consider this closed by #739. Thanks so much for your work!

@Zac-HD Zac-HD closed this as completed May 31, 2019
@Relequestual
Copy link
Member

Minor fix. More than welcome!
Feel free to come back the JSON Schema slack!
I also have a tip jar link on my github profile.

@Julian
Copy link
Member

Julian commented Nov 1, 2022

For breadcrumbs for future-us in case this ever comes up again (and I'm today finding this issue again during some livestream archaeology) --

This is something that changed in Draft 6, so only part of what's here is/was correct -- Specifically, in Draft 4 (and 3), the quoted language is not there, it indeed says MUST. From §5.5.1.1:

The value of this keyword MUST be an array. This array MUST have at least one element. Elements in the array MUST be unique.

Compared to Draft 6 and later which have the quoted language, i.e. SHOULD not MUST.

This seems to have changed in cf0ec72

It somehow looked from the above like we'd concluded to change the Draft 3 and 4 metaschemas as well (to not contain uniqueItems) although at least as of today (2022/11/1) they still do, which is correct given the MUST language in those drafts. So... I think nothing is broken here, just leaving the above in case 5 years down the line I end up on this same issue :)

Julian added a commit to python-jsonschema/jsonschema that referenced this issue Nov 1, 2022
For draft 4 this pulls in upstream fixes which were not present locally,
notably fixing `id` to not have `format: uri` in it, because location
independent identifiers are indeed not URIs (they're URI references as
later metaschemas use).

For enum, on draft 3 and 4, this also *re-adds* constraints that enum
items MUST be unique and the array non-empty. In later drafts, this
restriction was loosened (see
json-schema-org/json-schema-spec@cf0ec72)
as well as json-schema-org/json-schema-spec#717 (comment)
but in drafts 3 and 4 it is present.

The draft 4 metaschema contains these assertions, the draft 3 one is
still buggy and does not, so they're just applied locally here.

Ref: json-schema-org/json-schema-spec#310
@erohmensing
Copy link

erohmensing commented Nov 8, 2022

Looks like the proposed changes were made in #739, but the meta-schema listed for draft7 on json-schema.org still contains the restraints. Wanted to put that on your radar since that is how I ended up in the rabbit hole that landed me on this issue!

@handrews
Copy link
Contributor

handrews commented Nov 8, 2022

@erohmensing those changes went into the meta-schema for the draft that changed the syntax. The older drafts do not change, so their meta-schemas don't either.

@erohmensing
Copy link

Huh, I thought that draft 7 already contained that change (I added the wrong link above, I apologize and have corrected it) -- referring to the syntax as listed here underneath the Draft7 header versus the metadata schema listed under the same header.

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

5 participants