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

Made it possible to have a property named "$ref" #360

Merged
merged 1 commit into from
Sep 28, 2016

Conversation

iainbeeston
Copy link
Contributor

For a few weeks we've had a test from the json-schema common test suite
failing on master. The test was checking that it was possible to
validate a property named "$ref" (and that it wasn't reserved or
interpreted as a json-reference).

The problem was that whenever we had a property name that has special
meaning in json-schema (ie. one that matches the keys of the
Schema::Validator#attributes hash), we'd skip a level in the schema
object while validating, which would cause errors. To fix the problem
I've made sure we always descend one level at a time (see
attributes/additionalproperties.rb line 17).

Fixing this caused new tests to fail, in test/extends_nested_test.rb. It
turns out that these tests were depending on the old incorrect
behaviour.

What's more, the same tests were using an incorrect form of the extends
property. According to the json-schema spec, extends should take a
json-schema, or an array of json-schemas. However, these tests were
passing a string to extends. It seems that although this is incorrect,
we had code in attributes/extends.rb which would automatically correct
this incorrect syntax (ie. from {"extends": "foo.json"} to {"extends:
{"$ref": "foo.json"}}). I've removed that workaround, as well as
correcting the tests.

Fixes #357

@iainbeeston
Copy link
Contributor Author

@RST-J could you please take a look at this when you have time? It fixes test suite failures on master, so we can start merging pull requests again

@iainbeeston iainbeeston mentioned this pull request Sep 28, 2016
For a few weeks we've had a test from the json-schema common test suite
failing on master. The test was checking that it was possible to
validate a property named "$ref" (and that it wasn't reserved or
interpreted as a json-reference).

The problem was that whenever we had a property name that has special
meaning in json-schema (ie. one that matches the keys of the
Schema::Validator#attributes hash), we'd skip a level in the schema
object while validating, which would cause errors. To fix the problem
I've made sure we always descend one level at a time (see
attributes/additionalproperties.rb line 17).

Fixing this caused new tests to fail, in test/extends_nested_test.rb. It
turns out that these tests were depending on the old incorrect
behaviour.

What's more, the same tests were using an incorrect form of the extends
property. According to the json-schema spec, extends should take a
json-schema, or an array of json-schemas. However, these tests were
passing a string to extends. It seems that although this is incorrect,
we had code in attributes/extends.rb which would automatically correct
this incorrect syntax (ie. from {"extends": "foo.json"} to {"extends:
{"$ref": "foo.json"}}). I've removed that workaround, as well as
correcting the tests.

Fixes voxpupuli#357
@iainbeeston iainbeeston force-pushed the allow-property-named-ref branch from 910a88f to 8416911 Compare September 28, 2016 17:01
@RST-J RST-J merged commit 242e862 into voxpupuli:master Sep 28, 2016
@iainbeeston
Copy link
Contributor Author

Awesome! Thanks for the fast review!

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

Successfully merging this pull request may close these issues.

Test failure due to update in JSON-Schema-Test-Suite
2 participants