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

Require all described properties. #562

Closed
lordcheeto opened this issue Mar 8, 2018 · 10 comments
Closed

Require all described properties. #562

lordcheeto opened this issue Mar 8, 2018 · 10 comments

Comments

@lordcheeto
Copy link

This keyword indicates that the object must adhere strictly to the defined schema. That is, all defined properties required, and no additional properties allowed.

Example:

{
    "type": "object",
    "properties": {
        "latitude": {
            "type": "number"
        },
        "longitude": {
            "type": "number"
        }
    },
    "strict": true
}

Which would be equivalent to the following schema.

{
    "type": "object",
    "properties": {
        "latitude": {
            "type": "number"
        },
        "longitude": {
            "type": "number"
        }
    },
    "required": [
        "latitude",
        "longitude"
    ],
    "additionalProperties": false
}

While not a huge issue in this trivial example, it would allow objects with many required properties to be expressed less verbosely, and allow for clearer communication of the author's intention.

@Relequestual
Copy link
Member

@lordcheeto thanks! That's an interesting use case.

We're currenlty hashing out a new keyword which almost does what you want: #556, except it sees through any applicability keywords, which a common issue when using additionalProperties false with anyOf for example.

This is a use case we hadn't considered. Pretty clear I think.
For now, you'll have to write it long hand, but we can sure look into this!

@handrews
Copy link
Contributor

handrews commented Mar 8, 2018

@lordcheeto as @Relequestual notes, #556's unevaluatedProperties takes care of the "don't allow further properties to be present" in a runtime-aware way so that conditionals like oneOf are handled properly.

Let's have this issue focus on the required side of things in this issue, which I think should be implemented by a separate keyword alongside of unevaluatedProperties rather than making a combination keyword.

Would you be OK with us changing the title to something like "require all described properties"? I'm guessing you got strict from Geraint's old proposal, but it's too generic of a name for the keyword given our usual naming patterns.

@Relequestual
Copy link
Member

Agreed re what to call the keyword if we move forward with this. Could be something like requireAllPoperties maybe.

@handrews
Copy link
Contributor

handrews commented Mar 8, 2018

Actually let's get confirmation from @lordcheeto as we may be assuming something more complex than they meant to request!

Given this schema:

{
    "properties": {
        "a": {...},
        "b": {...}
    },
    "oneOf": [
        {
            "properties": {
                "c": {"const": true},
                "d": {...}
            }
        },
        {
            "properties": {
                "c": {"const": false},
                "e": {...}
            }
        }
    ],
    "newKeywordHere": true
}

Does this only cause "a" and "b" to be required, or does it also cause "c" and either "d" or "e" (depending on the value of "c") to be required?

@lordcheeto
Copy link
Author

Just looked through the proposal on unevaluatedProperties, which doesn't seem to help with the "require all properties" use case. However, the use case of disallowing additional properties, in a static or runtime-aware way, is well handled by these keywords.

So, while I was imagining a combination keyword, I like @handrews' suggestion of focussing on just the required aspect of this. That's where the bulk of the verbosity comes from and having it as a separate keyword would allow for more flexibility.

My original language for the proposal said something to the effect that it would apply to all objects in its "scope". Slightly modifying your example:

    {
        "properties": {
            "a": {...},
            "b": {...}
        },
        "oneOf": [
            {
                "properties": {
                    "c": {"const": true},
                    "d": {...}
                }
            },
            {
                "properties": {
                    "c": {"const": false},
                    "e": {...}
                },
                "newKeywordHere": false
            }
        ],
        "newKeywordHere": true
    }

Added an additional "newKeywordHere": false to indicate how it might be used to exempt a branch from requiring all properties. If applied to the entire scope of the keyword, this would require "a", "b", and "d", depending on the value of "c".

I removed the "scope" language from my proposal because I'm not aware of any current keywords that operate on the scope like this. Perhaps the annotation collection mechanism described in #530 could be used for this? I haven't really wrapped my mind around that yet. I'm also not sure how or if it should operate across $refs.

As for what to call the keyword, requireAllProperties would work, but if it operates in scope, there might be a better keyword available to indicate that. strict seemed like the best word to describe the combined behavior; I wasn't able to find a proposal along these lines, and I'm not sure what Geraint's old proposal covered. This StackOverflow question was the only other material I was able to find along these lines.

I'll change the title, and I'll further describe my use case in another comment.

@lordcheeto lordcheeto changed the title Keyword "strict". Require all described properties. Mar 9, 2018
@handrews
Copy link
Contributor

handrews commented Mar 9, 2018

@lordcheeto thanks! Yeah, for the scope thing #530 annotations might work. I haven't entirely wrapped my head around it either and I came up with the idea :-)

I want to be a bit cautious about using such a new and different mechanism, but at the same time, finding real problems that are also solved by it (other than unevaluatedProperties) is a good clue that it's a useful mechanism.

Thanks for explaining the scopes more, that's an interesting idea. One of the things we're trying to clarify is where to put the boundary between things that really need to be in a standard vocabulary, and things that can go in extensions. So we're doing some work (#561, #563, #564) to figure out a better story around extensions and interoperability. Right now no one really wants to do extensions because they're not interoperable at all.

Once that's settled, we may decided that a concept like scopes fits into the general keyword model, but is better done as an extension. Maybe even a "feature" extension of some sort. Let's keep kicking the ideas around here, although this probably won't go into the immediate next draft. Although if we settle on a clear consensus we can move it up, certainly.

@handrews handrews added this to the draft-future milestone Mar 9, 2018
@Relequestual
Copy link
Member

As a comment, I think that additionalProperties work on the same "scope" level. In that it is only effected by adjacent keyword properties (and other adjacent key words).

Also, the SO link included a comment: https://stackoverflow.com/questions/30868023/json-schema-require-all-properties#comment79419830_30868023

It would be nice if the JSON Schema spec supported "required": true, where the boolean replaces the usual array.

Sounds reasonable to me! I thought of this but don't think I mentioned it.

@lordcheeto
Copy link
Author

My use of JSON Schema may be different than what has been considered before, so I thought I'd detail it. I'm working on a library to consume a third party JSON WebAPI. Since I don't control the API, I want to be able to ensure that the JSON returned does not change. If properties are removed, that could be a breaking change. If properties are added, that would be a non-breaking change, but I'd want to know about the newly exposed functionality.

I've been writing schemas for the endpoints to describe exactly the responses I'm receiving from the API, with the plan being to set up a cron job to validate the responses against the schema occasionally and alert me to any changes. Some of these endpoints return a lot of properties, so something like this would make the resulting schema much more compact.

@Relequestual
Copy link
Member

Looks like this was proposed some time ago: https://github.com/java-json-tools/json-schema-validator/wiki/v5%3A-strictProperties

@handrews
Copy link
Contributor

@lordcheeto there have been several proposals at least somewhat like this, and @awwright consolidated them all in #846. I'm going to close this issue in favor of that one, if you notice anything that isn't covered by the ideas in that issue, please comment over there.

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