Skip to content
This repository has been archived by the owner on Nov 2, 2023. It is now read-only.

Design pattern: Using "propertyNames" to catch unexpected properties. #77

Closed
handrews opened this issue Feb 19, 2017 · 13 comments
Closed

Comments

@handrews
Copy link
Contributor

handrews commented Feb 19, 2017

Note: Originally filed as json-schema-org/json-schema-spec#214, but really more of a usage to document and explain rather than an issue for the spec.

Now that we have "propertyNames", it is easier to cause validation to fail on unexpected properties without using "additionalProperties": false. The general idea is that names in "properties" become an "enum" in "propertyNames", while each pattern in "patternProperties" becomes a "pattern" in "propertyNames".

It seems like this would be worth writing up as a possible solution on the web site. The following description is not the most concise and confused at least one person on the old issue, so it's worth discussing some here to find a good way to present this.


While the names and patterns involved still must be listed twice, it is possible to do this for each schema that needs to be re-used. Then, unlike with "additionalProperties", you can combine them however you want. This makes the process relatively easy to manage with well-structured "definitions" (and perhaps we can come up with a way to make it simpler?)

{
    "definitions": {
        "fooSchema": {
            "properties": {
                "foo1": {"type": "number"},
                "foo2": {"type": "boolean"}
            },
            "patternProperties": {
                "foo[A-Z][a-z0-9]*": {"type": "string"}
            }
        },
        "fooProperties": {
            "propertyNames": {
                "$comment": "Need to anyOf these or else the enum and pattern conflict",
                "anyOf": [
                    {"enum": ["foo1", "foo2"]},
                    {"pattern": "foo[A-Z][a-z0-9]*"}
                ]
            }
        },
        "barSchema": {
            "properties": {
                "bar1": {"type": "null"},
                "bar2": {"type": "integer"}
            }
        },
        "barProperties": {
            "propertyNames": {"enum": ["bar1", "bar2"]}
        }
    },
    "$comment": "Even with allOf schemas, need anyOf propertyNames or else they conflict",
    "allOf": [
        {"$ref": "#/definitions/fooSchema"},
        {"$ref": "#/definitions/barSchema"}
    ],  
    "anyOf": [
        {"$ref": "#/definitions/fooProperties"},
        {"$ref": "#/definitions/barProperties"}
    ]
}

This is much better than:

{
    "definitions": {
        "fooSchema": {
            "properties": {
                "foo1": {"type": "number"},
                "foo2": {"type": "boolean"}
            },
            "patternProperties": {
                "foo[A-Z][a-z0-9]*": {"type": "string"}
            }
        },
        "barSchema": {
            "properties": {
                "bar1": {"type": "null"},
                "bar2": {"type": "integer"}
            }
        }
    },
    "allOf": [
        {"$ref": "#/definitions/fooSchema"},
        {"$ref": "#/definitions/barSchema"},
        {
            "properties": {
                "foo1": true,
                "foo2": true,
                "bar1": true,
                "bar2": true
            },
            "patternProperties": {
                "foo[A-Z][a-z0-9]*": true
            },
            "additionalProperties": false
        }
    ]
}

In this version, you have to construct the correct "additionalProperties": false schema for every combination individually. With the "anyOf" + "propertyNames" approach, you can package up the property name rules and re-use them as easily as you do the corresponding schemas.

While this is not the simplest construct possible, it is much simpler than "$combine"/"$combinable", (#119) and unlike "$merge"/"$patch" (#15) it cannot be abused to produce JSON that is not even a schema, or to splice things in ways that authors of the source of the splice never intended.

I feel like this is a more promising avenue than the other proposals so far. Would it be reasonable to recommend this and put some guidance on the web site and see how that goes in draft 6? Then maybe we'll find that we don't really need the more complex solutions at all.

@Relequestual
Copy link
Member

Should barSchema have properties as a child rather than directly the properties themselves, in your example? If not, can you explain please, because I'm lost.

@handrews
Copy link
Contributor Author

Should barSchema have properties as a child rather than directly the properties themselves

oops, yes, thanks! I edited it to fix that now.

@ruifortes
Copy link

Sorry for posting without reading the full story but if I'm right assuming that it's about "extending" a type why not just use something like spread in js?
Instead of creating complicated semantics for the json-schema itself it could be assumed that some basic operations would be possible in simple json like "$ref" is.
"#ref" has nothing to do with the schema itself, just json.
So, why not use a similar approach using "#spread" like the js obj2 = {...obj1, otherProp, yetAnotherOne} but declarative.

If you have to basic shemas foo and bar

{
    description: 'foo schema',
    type: 'object',
    properties: {
        foo1: {type: 'number'},
        foo2: {type: 'boolean'}
    }
}
{
    description: 'bar shema' ,
    type: 'object'
    properties: {
        bar1: {type: 'string'},
        bar2: {type: 'boolean'}
    }
}

you could extend foo like:

{
    description: 'foo and more shema' ,
    type: 'object'
    properties: {
        more1: {type: 'string'},
        more2: {type: 'boolean'},
        $spread: {$ref: 'pathto/foo#properties'}
    }
}

eventually $spread position in would be relevant for overriding props.

To create a foobar schema that extends both you could:

{
    description: 'foobar and more shema' ,
    type: 'object'
    properties: {
        foobarSpecific: {type: 'string'},
        $spread: [
            {$ref: 'pathto/foo#properties'},
            {$ref: 'pathto/baar#properties'}
        ]
    }
}

The idea would be that json-schema would be a simple schema definition without fancy inheritance semantics and there would be some json (object) declarative manipulations mechanism like $ref and something like $spread could be another one

I'm I completely off?

@handrews
Copy link
Contributor Author

handrews commented Mar 3, 2017

@handrews
Copy link
Contributor Author

handrews commented Mar 3, 2017

@ruifortes in any event, this repository is for documenting things, not debating features. If you want to debate features please find or file issues in the json-schema-spec repo. Although as you'll see in the issue I linked, this particular idea (or set of similar ideas) has been beaten to death for years and isn't going anywhere.

@jdesrosiers
Copy link
Member

@handrews I'm looking at this closely for the first time. I couldn't see how it worked, so I put the example into the AJV Draft-06 validator with some test data and confirmed that it doesn't work as expected. To convince my self why it doesn't work, I removed the patternProperties part (to make it as simple as possible) and did some schema algebra. I came up with this equivalent schema.

{
  "type": "object",
    "properties": {
      "foo1": {"type": "number"},
      "foo2": {"type": "boolean"},
      "bar1": {"type": "null"},
      "bar2": {"type": "integer"}
    },
    "anyOf": [
        { "propertyNames": { "enum": ["foo1", "foo2"] } },
        { "propertyNames": { "enum": ["bar1", "bar2"] } }
    ]
}

It's now clear what the problem is. All property names must match either anyOf/0/propertyNames or anyOf/1/propertyNames or both. If we have instance data that includes all four defined property names, anyOf/0 will fail because "bar1" and "bar2" fail its enum check and anyOf/1 will fail because "foo1" and "foo2" fail its enum check.

Unless I'm missing something, we have the same problem we always had with additionalProperties.

@handrews
Copy link
Contributor Author

@jdesrosiers thank you for working through this! You are, of course correct :-/

However, I think this will work:

{
  "type": "object",
  "properties": {
    "foo1": {"type": "number"},
    "foo2": {"type": "boolean"},
    "bar1": {"type": "null"},
    "bar2": {"type": "integer"}
  },
  "propertyNames": { 
    "anyOf": [
      { "enum": ["foo1", "foo2"] },
      { "enum": ["bar1", "bar2"] }
    ]
  }
}

Now each property has to individual satisfy the "anyOf", which works fine. So in terms of re-use:

{
  "definitions": {
    "foo": {
      "type": "object",
      "properties": {
        "foo1": {"type": "number"},
        "foo2": {"type": "boolean"}
      }
    },
    "fooNames": { "enum": ["foo1", "foo2"] },
    "bar": {
      "type": "object",
      "properties": {
        "bar1": {"type": "null"},
        "bar2": {"type": "integer"}
      }
    },
    "barNames": { "enum": ["bar1", "bar2"] }
  },
  "allOf": [
    { "$ref": "#/definitions/foo" },
    { "$ref": "#/definitions/bar" },
    {
      "propertyNames": { 
        "anyOf": [
          { "$ref": "#/definitions/fooNames" },
          { "$ref": "#/definitions/barNames" }
        ]
      }
    }
  ]
}

That's definitely cumbersome, but perhaps we can come up with a way to express this more concisely?

@handrews
Copy link
Contributor Author

@ruifortes re-reading your proposal now it makes more sense to me, and is not the same as #15. I'm going to file it separately, credit you, and add it to the vote-a-rama.

@jdesrosiers
Copy link
Member

Yep that works! It also works with patternProperties given in the original example.

{
  "definitions": {
    "foo": {
      "type": "object",
      "properties": {
        "foo1": { "type": "number" },
        "foo2": { "type": "boolean" }
      },
      "patternProperties": {
        "foo[A-Z][a-z0-9]*": { "type": "string" }
      }
    },
    "fooNames": {
      "anyOf": [
        { "enum": ["foo1", "foo2"] },
        { "pattern": "foo[A-Z][a-z0-9]*" }
      ]
    },
    "bar": {
      "type": "object",
      "properties": {
        "bar1": { "type": "null" },
        "bar2": { "type": "integer" }
      }
    },
    "barNames": { "enum": ["bar1", "bar2"] }
  },
  "allOf": [
    { "$ref": "#/definitions/foo" },
    { "$ref": "#/definitions/bar" },
    {
      "propertyNames": { 
        "anyOf": [
          { "$ref": "#/definitions/fooNames" },
          { "$ref": "#/definitions/barNames" }
        ]
      }
    }
  ]
}

It certainly is cumbersome, but still preferable to "additionalProperties": false.

@renewooller
Copy link

Is there a good reason we can't specify the validation algorithm to 'look ahead' and figure out which properties will be included on an object based on all the conditionals within it?

@handrews
Copy link
Contributor Author

@renewooller you probably want to look at json-schema-org/json-schema-spec#556, which I think more or less does what you want. I turns out to be rather complex, but we think worth putting into draft-08

@aaron-meyers
Copy link

Just found this issue after filing one in the test suite:
json-schema-org/JSON-Schema-Test-Suite#255

The release notes for draft-06 currently have a copy of the pattern in the opening post from this thread, which doesn't actually work (which was discovered later in this thread):
http://json-schema.org/draft-06/json-schema-release-notes.html#q-what-happened-to-all-the-discussions-around-re-using-schemas-with-additionalproperties

I think these release notes should be updated with the correct pattern. I just spent a couple hours figuring this out myself :(

@handrews
Copy link
Contributor Author

handrews commented May 4, 2021

We now have unevaluatedProperties which removes the need for this.

@handrews handrews closed this as completed May 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants