voxpupuli / json-schema

Ruby JSON Schema Validator
MIT License
1.53k stars 242 forks source link

`fully_validate` // `anyOf`, `typeOf`, `allOf` don't raise validation errors when using `record_errors: true` #300

Closed thewatts closed 8 years ago

thewatts commented 8 years ago

Took me forever to figure out why the validation errors weren't getting added into the errors hash in the attribute validations. Ex: https://github.com/ruby-json-schema/json-schema/blob/master/lib/json-schema/attributes/anyof.rb#L31

Partial solution: The record_errors: true option needs to be passed into the validate! method.

This will now add those errors and assign them as sub_errors here: https://github.com/ruby-json-schema/json-schema/blob/master/lib/json-schema/attributes/anyof.rb#L42

The only kicker - is that the validation then is never raised, even though validation doesn't pass. This is because https://github.com/ruby-json-schema/json-schema/blob/master/lib/json-schema/attributes/anyof.rb#L42 just returns a hash.

What line do I need to add to these files so that I can raise a validation error w/ its sub-errors?

I see that the ValidationError class has a to_string method, which I could use to override the message method for ValidationError, and then call this after line 42:

raise processor.validation_errors.last

This fixes my validation problem, but breaks the error output for the test suite.

Am I missing something?

iainbeeston commented 8 years ago

Sorry, I feel like I'm missing something!

Can you please explain again what behaviour you're seeing? And what you would expect to see? An example schema and json text that reproduces the problem would also be helpful

thewatts commented 8 years ago

Hey @iainbeeston!

Really sorry for the cryptic message. It was super late on my end, I should have re-read it!

Schema: https://gist.github.com/thewatts/8d7802af38c1a05ff2e4#file-schema-json Data: https://gist.github.com/thewatts/8d7802af38c1a05ff2e4#file-data-json

Current Behavior:

Running JSON::Validator.validate!(schema, data) fails correctly with this error:

JSON::Schema::ValidationError:
       The property '#/registration' of type Hash did not match any of the required schemas in schema 4c0ec013-5c14-5fe6-a148-2f81fc4075f0

Which is great! That's to be expected. However, I want to have the validation error still raise - but also know why validation broke, ex:

JSON::Schema::ValidationError:
      The property '#/registration' of type Hash did not match any of the required schemas. The schema specific errors were:

- oneOf #0:
    - The property '#/registration' of type Hash did not match the following type: null
- oneOf #1:
    - The property '#/registration' did not contain a required property of 'check_in'

After some sleep - I realize now that adding the options { record_errors: true }, or using JSON::Schema.fully_validate does in fact keep track of those errors, but instead of raising a ValidationError, it will instead just return the errors.

Expected:

I suppose I expected that adding the { record_errors: true } options to validate! would still raise a ValidationError, but also show the individual places where it did in fact error out.

thewatts commented 8 years ago

@iainbeeston - is there a way to currently do this? Or is this perhaps in the roadmap?

If not - no worries. Just making sure I fully understand :)

Thanks!

( And thanks for this killer gem! Using it as a validator for my API has vastly improved the way I test! )

iainbeeston commented 8 years ago

It sounds like we have the correct behaviour when record_errors is true, but we should also use the same information when we raise errors (when record_errors is false). It appears that we already have the information available but we're not using it in exceptions.

If you want that behaviour right now I'd suggest making a wrapper class for json schema that sets record_errors to true then raising an exception if any errors are returned. I'm afraid it's not on the roadmap as yet but if you have the time to make a pull request to add this feature I'd happily take a look.

thewatts commented 8 years ago

It appears that we already have the information available but we're not using it in exceptions.

@iainbeeston when record_errors is set to false, the errors themselves unfortunately don't get captured.

There are a couple things:

  1. Without the record_errors option, the line that adds the sub-errors never gets run because the ValidationError raises an exception directly without record_errors (https://github.com/ruby-json-schema/json-schema/blob/master/lib/json-schema/attribute.rb#L15-L18)
  2. Without the record_errors option, this line is never run: https://github.com/ruby-json-schema/json-schema/blob/master/lib/json-schema/attributes/anyof.rb#L31. So even if we told the exception to raise with the ValidationError and its sub_errors, the sub_errors would be blank because they aren't being recorded during the schema validation process.

We'd have to make an adjustment in both places, as well as adjust ValidationError's message method to output sub_errors as well.