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

"requiredProperties" keyword #846

Open
awwright opened this issue Feb 4, 2020 · 19 comments
Open

"requiredProperties" keyword #846

awwright opened this issue Feb 4, 2020 · 19 comments

Comments

@awwright
Copy link
Member

awwright commented Feb 4, 2020

When accepting JSON documents for submission, it is frequently the case the documents are objects that have at least one property that is required, such as an ID or name field.

As an authoring convenience, JSON Schema should have a "requiredProperties" keyword that is similar to "properties" in that it defines a sub-schema to apply to the respective properties; but in addition, each of those properties must exist, similar to the "required" keyword.

This:

{
  "requiredProperties": {
    "name": { "type": "string", "minLength":1 }
  }
}

Would be the same as this:

{
  "required": ["name"],
  "properties": {
    "name": { "type": "string", "minLength":1 }
  }
}

Alternatively, this functionality could be built into the "required" keyword (so that "required" accepts an array or an object). This may even be preferable, since older validators will (usually) bail if they see an object where an array is expected:

{
  "required": {
    "name": { "type": "string", "minLength":1 }
  }
}

Examples of confused users:

This issue is intended to restate and close #659 #681 #734

@gregsdennis
Copy link
Member

Is this really the right direction?

I started working with JSON Schema at draft 4, so I don't have any details, but does anyone know why required was removed from the property schema into the required property when moving from draft 3 -> 4?

Some insight into that decision would probably be helpful here.

@awwright
Copy link
Member Author

awwright commented Feb 4, 2020

@gregsdennis Boolean "required" didn't work the same way that other keywords worked; it only had meaning in a subschema in certain keywords, mostly "properties". It was nonsensical if used inside e.g. "patternProperties" or "additionalItems", and there's even more considerations, like what if you use it inside "oneOf" or "not"?

It also made it more difficult to create composable schemas; referencing a schema that had required: true could cause a little bit of a problem depending on where the $ref was located.

I actually argued against the change at the time because it was a nice authoring convenience compared to the array form; but I think this is superior to boolean "required" by far.

(rewritten from #734 (comment) )

@handrews
Copy link
Contributor

handrews commented Feb 4, 2020

While I'd be strongly against changing required to take an object form, I'm at least tentatively open to the idea of requiredProperties as a new keyword. Not sold, but keeping an open mind.

I want to keep the keyword taxonomy clear, in terms of which keywords behave as applicators, assertions, or annotations.

@awwright Adding an object syntax to required would make it a keyword that has two completely different syntaxes, with different taxonomy for each. This is exactly why we split dependencies into dependentSchemas (an applicator, taking a map of subschemas) and dependentRequired (an assertion, taking a map of string arrays).

But requiredProperties would be a single syntax that functions both as an applicator and an assertion. My purist impulses object to that, but since it is always functioning as both I could be convinced.

My practical concern with the taxonomy has to do with how truly generic implementations could manage plugins for unknown vocabularies. It's too involved to go into at the moment, I just mention it so people know that there's a point other than "Henry likes his own theoretical ideas a lot." (although that is also true).

@handrews
Copy link
Contributor

handrews commented Feb 4, 2020

@gregsdennis I think requiredProperties is a better proposal than the boolean required, because boolean required doesn't really fit into the processing model we've developed.

Superficially similar keywords like readOnly, writeOnly, and deprecated are fine because they are annotations, and by definition get attached to the location where they are applied. The attachment information includes the instance path, and therefore the relevant property name.

But assertions are intended to assert things about the instance location to which they are applied. Boolean required actually asserts something about the parent of the instance location to which it is applied:

{
    "type": "object",
    "properties": {
        "foo": {
            "type": "integer",
            "required": true
        },
        "bar": {
            "type": "string"
        }
    }
}

with instance

{
    "foo": 1,
    "bar": "something"
}

The root schema gets applied to the whole instance. We look at properties, and notice that we have subschemas for "foo" and "bar". We apply each one of them in turn. That "required": true in the subschema applied to #/foo in the instance doesn't say anything about #/foo (which has the value 1). It says that the root instance object must contain a property named "foo".

Now, because of how required works as a concept, we know that if it's getting applied at all, the property must have been present, so we can return a result without having to go inspect the parent.

But let's say we couldn't assume that. We would have to pass the parent object to the child assertion in order to determine the result. Nothing in JSON Schema assertion processing looks up into a parent. Applying a schema to an instance location only involves that location and its children, never the parent.

That is not a behavior we should change.

@gregsdennis
Copy link
Member

@awwright / @handrews all very good insight. Thanks. I just wanted to ensure that readers of this issue have context and history; not advocating for a reversion.

@gregsdennis
Copy link
Member

@awwright are you proposing also that we deprecate required? I've seen schemas that only have required, but no further restrictions on those properties. It's like specifying your new keyword with "myProp": true, which is actually more verbose.

@jdesrosiers
Copy link
Member

requiredProperties makes me uncomfortable for theoretical purity reasons, but I can also see that it could be potentially useful.

I think this could be the first of keyword in a vocabulary of helper keywords that address concerns similar to this. It could even be officially supported by the JSON Schema team, but I don't think it belongs in the official specification.

@awwright
Copy link
Member Author

awwright commented Feb 4, 2020

I imagine requiredProperties as more of a syntactic sugar than anything else; it makes sense to me that sometimes syntatic sugar will combine applicators and validators.

@gregsdennis I don't think it would make sense to deprecate required; authors may still wish to declare "required" and "properties" separately, for composibility reasons.

@handrews
Copy link
Contributor

handrews commented Feb 5, 2020

@awwright There are a number of syntactic sugar proposals, and as @jdesrosiers notes maybe making them an additional vocabulary outside of the Core and Validation specification documents would be a good approach. We would then keep the six vocabularies in those two specifications as minimal as possible.

They do have some keywords that are technically unnecessary (dependentRequired, dependentSchemas, if/then/else can all be written in other ways, and there's substantial overlap between patternProperties and propertyNames). So I don't know where to draw the line. We didn't have vocabularies when all of those went in.

@handrews
Copy link
Contributor

handrews commented Feb 5, 2020

Regarding the purity issue, what I'm thinking of (not proposing here- I'll file a separate issue if and when it seems appropriate) is the ability for a vocabulary to indicate which keywords fall into which classifications, and how they interact.

Then extensions and generic implementations could be designed around those behaviors. Run in-place applicators first, because some keywords depend on the results of adjacent in-place applicators (at the moment, they specifically name keywords and because there's no generic way to do this with extension applicators). Run assertions when validating. If you're not collecting annotations then you can just ignore those keywords entirely. etc.

This is a half-formed idea which is why I haven't filed it, but that's where I'm keeping an eye on purity. A keyword that would need both an applicator hook and an assertion hook seems reasonable when those two behaviors can be modeled separately. Which is the case with requiredProperties. I'm hoping to get some real feedback on how people build vocabularies once we're out in the OAS 3.1 ecosystem. This idea may or may not be worth pursuing, we won't know until we get feedback.

So that's why I'm tentatively open to requiredProperties despite it violating a strict keyword classification concept. You can just treat it as a keyword that's run both ways, without having to do any more complex new processing.

@Relequestual
Copy link
Member

Relequestual commented Feb 7, 2020

Marking this to resolve for draft-09. I've seen this requested enough times to want to make it possible. I'm OK with requiredProperties.

Edit: I prefer #1144. I've outlined why on the PR in a comment. (Thumbs up were pre-edit)

@racquetmaster
Copy link

My 2 Cents:
I think requiredProperties is a great idea.
I understand that the processing model is at odds with have a required: true attribute. Ultimately, this solution would be even less verbose for cases where the majority are required.

It is such a common requirement so it would reduce the boilerplate significantly. For example, we generate our schemas in js and so most of our schemas have this at the bottom to work around it:
schema.required = Object.keys(schema.properties);. It would be nice to declare that explicitly by changing properties to requiredProperties in our schemas.

@racquetmaster
Copy link

Any status on this?

@handrews
Copy link
Contributor

handrews commented May 3, 2021

@racquetmaster It is in the draft-09 milestone for consideration in the next draft. Or as a supplemental vocabulary which would not necessarily need to wait for a whole new draft, and might motivate more people to implement support for supplemental vocabularies.

@handrews
Copy link
Contributor

handrews commented May 3, 2021

Actually it looks like draft-09 was renamed draft-next, presumably because we're not really using the old-style draft numbers anymore.

Anyway, I'm increasingly inclined to support doing this somewhere.

@hheavener-kyd
Copy link

Bump. Any news on this?

@Relequestual
Copy link
Member

@hheavener-kyd Given it's in the draft-next milestone, we aim to resolve this issue as part of that work.
We have not yet set a deadline for this milestone.
We may do this after we have completed the other in-progress release.

Stay tuned =]

@jdesrosiers
Copy link
Member

@hheavener-kyd See #1112 and it's PR #1144. It's an alternative to this keyword that solves the same problem in a better way and it's on track for inclusion in the next release of JSON Schema. It's currently on hold while we focus on getting the patch release finished, but I'm hoping to get it merged once the patch release is out of the way.

@Relequestual
Copy link
Member

I much prefer #1144 to this proposal. See #1144 (comment)

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

7 participants