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

The property '#/.../patternProperties/required' of type TrueClass did not match the following type: object #29

Open
phs opened this issue Jun 27, 2013 · 16 comments

Comments

@phs
Copy link

phs commented Jun 27, 2013

Consider this minimal reproducing endpoint:

---
name: url
route: /url
method: PUT
definitions:
  - message_type: request
    versions: ["0.1"]
    schema:
      type: object
      patternProperties:
        '^[^:\s]+$':
          type: string
    examples:
      - "Server": "nginx"
  - message_type: response
    versions: ["0.1"]
    status_codes: ["2xx"]
    schema: {}
    examples: []

The request schema itself does not validate, which manifests when trying to validate the examples:

  1) MozLinks::Apps::API url (v 0.1) has valid data for example 1
     Failure/Error: define_test(description) { example.validate! }
     Interpol::ValidationError:
       Found 1 error(s) when validating against endpoint url (request v. 0.1). Errors:
         - The property '#/patternProperties/required' of type TrueClass did not match the following type: object in schema http://json-schema.org/draft-03/schema#.

       Data:
       {"type"=>"object", "patternProperties"=>{"^[^:\\s]+$"=>{"type"=>"string", "required"=>true}, "required"=>true}, "required"=>true}
     # /usr/local/var/rbenv/versions/jruby-1.7.4/lib/ruby/gems/shared/gems/interpol-0.10.2/lib/interpol/endpoint.rb:220:in `validate_data!'

It appears to be complaining about this, in the context of the patternProperties clause.

All keys of a patternProperties must be regexs (strings), but the values must all be schemas (objects.) The true that interpol slips in doesn't conform.

@myronmarston
Copy link
Contributor

Definitely a bug. We haven't used patternProperties at all, FWIW, so I'm not surprised to see bugs there.

@phs
Copy link
Author

phs commented Jun 27, 2013

Generally speaking, I'm not really a fan of the implicit schema munging I see running around here. It's asking for trouble, for a perhaps limited return.

@myronmarston
Copy link
Contributor

Generally speaking, I'm not really a fan of the implicit schema munging I see running around here. It's asking for trouble, for a perhaps limited return.

I'd love to get a way from the schema munging if we can find a way to give ourselves the strong validation without having to list required: true and additionalProperties: false everywhere. JSON schema v3, for some reason I can't understand, has very unsafe defaults: a schema like:

type: object
properties:
  foo:
    type: integer

...will happily say { foi: "a string" } is valid. Since foo isn't required by default and additional properties are allowed by default, a JSON document that has nothing in common with the schema will validate just fine by default.

As we've defined more and more endpoints, we've noticed that the time it takes to load all the endpoints has increased noticeably, and I suspect that the fact we traverse every endpoint schema and munge it has to do with that. So yeah -- let's find a way to get away from this! I'd be curious to hear if v4 is any better on this front. I guess I need to set aside some time to look at it.

@phs
Copy link
Author

phs commented Jun 27, 2013

I think v4 keeps the same lax-by-default philosophy, which I agree is a little silly. However, I'm inclined to solve the problem by informing users (in README or wiki) of the issue and just putting up with the required and additionalProperties tags.

As a compromise, we could make the munging opt-in behavior: sibling to the schema key could be schema_strictness: true (default false). Then existing apps (well, app) would just need to add one easy line to each endpoint, instead of having to rethink requirement rules more thoroughly.

@myronmarston
Copy link
Contributor

Another solution that @benkirzhner and I discussed is subclassing the json-schema gem's validator and making it strict-by-default.

The other thing to bear in mind is that our schema munging is now also used to make scalars nullable by default since (based on a config setting) since we wanted that behavior as well. We'd have to find a way to get a similar behavior with whatever solution we use.

@phs
Copy link
Author

phs commented Jun 27, 2013

Subclassing would be fine by me, as long as it's also possible to opt in or out.

I don't actually want interpol to change the semantics of json schema at all, since that's another layer of interaction/complexity that I'd have to be aware of.

@bkirz
Copy link
Contributor

bkirz commented Jul 2, 2013

Since the json-schema gem uses pluggable JSON validators, perhaps that could be exposed in the interpol config API? Something like:

Interpol.default_configuration do |config|
  config.json_validator MyStrictValidator
end

@phs
Copy link
Author

phs commented Nov 12, 2013

Bump. Just got bit by this again. I would very much like to see power over the schema returned to the application authors: I'm a big boy and know how to write one.

@myronmarston
Copy link
Contributor

PRs gladly accepted :-)

Sent from my iPhone

On Nov 11, 2013, at 9:51 PM, Phil Smith notifications@github.com wrote:

Bump. Just got bit by this again. I would very much like to see power over the schema returned to the application authors: I'm a big boy and know how to write one.


Reply to this email directly or view it on GitHub.

@phs
Copy link
Author

phs commented Nov 12, 2013

Are they? The PR I would create would require other apps to rejigger their own schemas. I've been waiting a few months now for some sort a plugging solution on your end: I don't know what would work for your apps.

@myronmarston
Copy link
Contributor

Sorry, I just realized that might have come across as "you're on your own" :). Didn't mean it to come across that way. I don't have the capacity to tackle this at the moment with the Dallas move being a priority.

@undfined -- we should find time for @proby, @benkirzhner or I to work on this at some point.

@hoxworth
Copy link

hoxworth commented Jan 1, 2014

I'd be interested in putting a little effort towards a strict validation option in json-schema; I see a lot of value in that.

@myronmarston
Copy link
Contributor

@hoxworth -- that would be fantastic :).

@hoxworth
Copy link

And done. I have added a :strict option to validation to strictly validate object properties in 1.2.0 and 2.2.0. I specifically released this as a feature release in the 1.x branch as I am pretty sure you guys haven't updated to 2.x, which uses draft-04 by default, and I didn't want to cause issues with existing schemas that might not specify a "$schema" => "http://json-schema.org/draft-03/schema#" property.

There's an example on the README, but usage should be pretty straightforward:

schema = {
  "type" => "object",
  "properties" => {
    "a" => {"type" => "integer"},
    "b" => {"type" => "integer"}
  }
}

JSON::Validator.validate(schema, {"a" => 1, "b" => 2}, :strict => true)            # ==> true
JSON::Validator.validate(schema, {"a" => 1, "b" => 2, "c" => 3}, :strict => true)  # ==> false
JSON::Validator.validate(schema, {"a" => 1}, :strict => true)                      # ==> false

The additionalProperties property is still respected, as well, so all properties that match the additionalProperties schema will be allowed.

Let me know if this works for you at all...

@hoxworth
Copy link

Of course, I forgot patternProperties... I'll get to that later today.

@hoxworth
Copy link

Okay, 1.2.1 and 2.2.1 have been released that respect the patternProperties property as well when :strict is enforced. Again, let me know if you have any issues.

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

4 participants