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

deepProperties and deepRequired #18

Open
epoberezkin opened this issue Dec 22, 2016 · 15 comments
Open

deepProperties and deepRequired #18

epoberezkin opened this issue Dec 22, 2016 · 15 comments

Comments

@epoberezkin
Copy link
Member

Same as properties and required, but instead of property names JSON pointers are used (where the root means current part of the data instance).

{
  "deepProperties": {
    "/user/role": { "enum": ["admin"] }
  }
}

is equivalent to

{
  "properties": {
    "user": {
      "properties": {
        "role": { "enum": ["admin"] }
      }
    }
  }
}

and

{ "deepRequired": ["/user/role"] }

is equivalent to

{
   "required": ["user"],
   "properties": {
     "user": {
       "type": "object",
       "required": ["role"]
     }
   }
}

I had this situation a couple of times and it gets very annoying (and visually noisy) when you have to drill down into the hierarchy with "properties" keywords. It is particularly useful when you have conditionals :).

Given that JSON pointers can have indices in arrays, numeric segments should probably be treated as array items rather than as property names, so this schema:

{
  "deepProperties": {
    "/users/1/role": { "enum": ["admin"] }
  }
}

should be equivalent to

{
  "properties": {
    "users": {
      "items": [
        {},
        {
          "properties": {
            "role": { "enum": ["admin"] }
          }
        }
      ]
    }
  }
}

and

{ "deepRequired": [ "/users/1/role" ] }

should be equivalent to

{
   "required": ["users"],
   "properties": {
     "users": {
       "type": "array",
       "minItems": 2,
       "items": [
         {},
         {
           "type": "object",
           "required": ["role"]
         }
       ]
     }
   }
}
@handrews
Copy link
Contributor

@epoberezkin why is this needed? I agree it's visually noisy with all of the nesting, but I feel like that is better addressed with tooling and a nice graphical editor sort of thing.

What makes me nervous here is that this sort of thing is really hard to grasp mentally due to the very different mechanisms involved:

{
    "type": "object",
    "properties": {
        "foo": {"properties": {"bar": {"properties": {"baz": {"type": "string"}}}}}
    },
    "deepRequired": ["/foo/bar/baz"]
}

which is a bit pathological (why would you mix them in something so simple) but as things get more complex this could produce really unexpected situations.

Also, don't you hate JSON Pointer? ;-)

@epoberezkin
Copy link
Member Author

Touché. I like JSON pointers actually. It's not particularly wide use case, I agree. Let's see what others think. It's in my TODO list of custom keywords anyway, just need to figure out the way to collect usage statistics :)...

but I feel like that is better addressed with tooling and a nice graphical editor sort of thing.

I'm definitely not a fan of tools, in general... Try to use as few as possible, only those I can't do without. /rant

@epoberezkin
Copy link
Member Author

It's somewhat related to json-schema-org/json-schema-spec#141 btw with the difference that this is still structural validation that is hard-coded in the schema (essentially a syntax sugar), while json-schema-org/json-schema-spec#141 suggests dynamic pointers inside the data (completely new functionality).

@epoberezkin
Copy link
Member Author

Also, even though it's sugar it can be more efficiently implemented than the equivalent schemas using the existing keywords

@epoberezkin
Copy link
Member Author

Added custom keywords deepProperties and deepRequired to ajv-keywords.

@handrews
Copy link
Contributor

@epoberezkin This proposal has grown on me somewhat. We need to sort out the question of relative JSON Pointers in general, because if we use relative pointers then we should use them here.

I get what you're trying to do with the locally-rooted absolute pointers, but I find them more confusing. I have to pay attention to which keyword is being used to know whether that leading slash is the current location or the document root, and that's a likely source of errors.

The one thing that I am concerned about is the question of array indices vs property names that happen to look like numbers. In normal JSON Pointer usage, the data type is defined external to the pointer, so there is no ambiguity. In this case, we're using pointers to define structure, so at minimum we would have to forbid property names that are numbers if we wanted to allow creating arrays this way.

It may be that something like JSON Path, while not a standard, would be a better system to consider. It's considerably more complex, though.

@epoberezkin
Copy link
Member Author

I get what you're trying to do with the locally-rooted absolute pointers, but I find them more confusing.

@handrews Relative pointers are fine, in this case an absolute pointer can mean absolute in the JSON instance being validated (that how it works in my $data implementation e.g., although it's not directly related, it would be consistent to have the same here). I'd rather we don't go into JSON path though...

we would have to forbid property names that are numbers if we wanted to allow creating arrays this way.

We are not creating anything here, the structure would still be independently enforced as usual. We are simply matching some parts of the structure with schema or require that some part is present. So we can either 1) decide that numbers in JSON pointers mean only array indices in the context of these keywords (which would be less consistent with how JSON pointers are used) or 2) that both array elements and properties should be matched (that would be more consistent - in this case both array item and property would have to be valid according to the schema in case of deepProperties and that either item or property presence would be sufficient to validate deepRequired).

I prefer approach 2 (to take both items and properties into account) and that's what my implementation in ajv-keywords is doing as well, both for deepRequired and for deepProperties. The above examples are simplified though.

@handrews
Copy link
Contributor

that how it works in my $data implementation e.g., although it's not directly related, it would be consistent to have the same here

Blerg. Have fun migrating that if we go with $data as normally proposed. I really need it to be able to reference containing fields so a a $data that behaves like that would be useless for my needs.

in this case both array item and property would have to be valid according to the schema

So we'd still need to define that level as an array vs object with `"type" somewhere? Otherwise how can I tell that you're example using array means what you state instead of:

{
   "required": ["users"],
   "properties": {
     "users": {
       "type": "object",
       "properties": {
         "1": {
           "type": "object",
           "required": ["role"]
         }
       }
     }
   }
}

@epoberezkin
Copy link
Member Author

epoberezkin commented Feb 20, 2017

Blerg. Have fun migrating that if we go with $data as normally proposed. I really need it to be able to reference containing fields so a a $data that behaves like that would be useless for my needs.

@handrews What I meant is that my $data supports both relative and absolute pointers, so what's the problem? :)
There are use-cases for both usages, as it appeared. I added absolute pointers support later.

So we'd still need to define that level as an array vs object with `"type" somewhere? Otherwise how can I tell that you're example using array means what you state instead of

As I wrote, the example above is simplified and if we want an equivalent schema (although it's not even needed, strictly speaking) it can be done with anyOf keyword:

{
   "required": ["users"],
   "properties": {
     "users": {
       "anyOf": [
         {
           "type": "array",
           "minItems": 2,
           "items": [
             {},
             {
               "type": "object",
               "required": ["role"]
             }
           ]
         },
         {
           "type": "object",
           "required": ["1"],
           "properties": {
             "1": {
               "type": "object",
               "required": ["role"]
             }
           }
         }
       ]
     }
   }
}

But we don't really need an equivalent schema to define a keyword, we need verbal explanation, meta-schema and tests. Actual implementation code for deepRequired is much simpler than that.

An equivalent schema for "deepProperties" will simply have to have both "properties" and "items" keyword on that level.

@epoberezkin
Copy link
Member Author

@handrews also see json-schema-org/json-schema-spec#115 re extension to relative pointers - it is needed for ordering of arrays, unless we want a separate keyword (and I'd rather we have $data - I can see people are using it).

@handrews handrews changed the title Validation: deepProperties and deepRequired deepProperties and deepRequired Sep 28, 2017
@handrews
Copy link
Contributor

This proposal would significantly complicate the applicability model introduced in draft-07. That doesn't mean we can't implement these (I'm already proposing additions to the processing model in json-schema-org/json-schema-spec#515). But I think it's worth considering.

Currently, you figure out which subschemas apply to an instance location, evaluate their assertions, and (optionally) collect their annotations. The deep* keywords would mean that subschemas that are not applicable to a given instance location may still have an affect on that location, putting us in "spooky action at a distance" territory.

I'm not sure the use cases here are strong enough to complicate that processing model, but I'm definitely willing to listen to arguments to the contrary.

@awwright
Copy link
Member

awwright commented Dec 5, 2017

Yeah, I think there's something to be said for adding keywords as author conveniences.
But I'm not sure this would get enough usage to be worth the additional keywords -- too many additional keywords itself can make schemas harder to read.

@stueynz
Copy link

stueynz commented Mar 27, 2018

Here's an example of why I think deepRequired could be very useful. We're writing OpenAPI definitions; being somewhat old school, I build a data model of objects separately from including them in the various POST, PUT & PATCH operations of my API.

So with a suitably realistic non-trivial data model we might need to make some nested properties required for certain operations.

#/components/schemas/Person:

{
       "properties": {
        "id": {"type": "integer", "minimum": 0, "readOnly": true},
        "name": {
            "type": "object",
            "properties" : {
                "firstName" : "string",
                "middleName" : {  "type" : "array", "items": "string" },
                "lastName" : "string"
            },
        },
       "demographics": { 
            "type" : "object",
            "properties" : {
                 "gender" : "string",
                 "ethnicity" : { "type" : "array", "items" : "string"}
            }
      }
}

...and then we have PUT & POST operations with body

{
   "schema" : {
       "allOf" : [ "$ref: "#/components/schemas/Person",
                       "deepRequired" : [  "/name/firstName",  "/name/lastName",  "/demographics/gender" ]
       ] 
   }
}

... and PATCH operation with body

{
   "schema" : {
        "$ref: "#/components/schemas/Person"
   }
}

@handrews
Copy link
Contributor

Moving keyword proposals without clear immediate support into the vocabularies repository.

@handrews handrews transferred this issue from json-schema-org/json-schema-spec Feb 28, 2020
@jashworth
Copy link

I don't really see what business "required" has in a model. "required" for what purpose? "required" only makes sense if you know for what set of operations the prop is required.

If you made all props optional in models, and listed deepRequired only in operations, it should be possible (for a machine, at least) to work out how far "required" can be "lifted" up the composition tree for a given API. Wouldn't that be useful for code generators?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants