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

Use json_schema instead of json-schema #31

Merged
merged 1 commit into from
Apr 13, 2018

Conversation

malandrina
Copy link

The problem

  • json_matchers cannot easily be used concurrently with Heroku's JSON API tools, i.e. prmd and committee, because json_matchers makes different assumptions about the structure of the user's schemata. An example of an incompatibility can be found in Having an id parameter in the schema messes up schema nesting #25: json_matchers breaks when the id property is present within a schema, but the Heroku tools require the presence of the id property (reference).
  • This is happening because the libraries used to dereference JSON pointers behave differently. json-schema, the library we're currently using, appears to conform less strictly to the JSON Schema specification than the library the Heroku tools use, json_schema.

The solution

  • One solution to this problem is to update json_matchers to use the same approach to validating schemata as the Heroku tools. This will require the following changes:
    1. Use json_schema instead of json-schema to validate schemata
    2. Update documentation to instruct readers to follow Heroku's
      guidelines for structuring schemata:
      https://github.com/interagent/prmd/blob/master/docs/schemata.md
    3. Update documentation to reflect that the strict option is no longer supported
  • In this commit I've replaced json-schema with json_schema and updated the schemata fixtures in the specs. Per this json_schema issue, in order to dereference JSON pointers referencing schemata in other files we need to access the gem's DocumentStore API directly. This is done in Matcher#add_schemata_to_document_store.

"$schema": "http://json-schema.org/draft-04/schema#",
"type": "object",
"properties": {
"a": { "$ref": "file:/#{JsonMatchers.schema_root}/nested.json#" }

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put a comma after the last item of a multiline hash.

@malandrina
Copy link
Author

@seanpdoyle I'd love your feedback on my comments above when you have time. There's no rush -- this is a work in progress.

},
})
create_schema("nested-schema", {
"$schema": "http://json-schema.org/draft-04/schema#",

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use 2 spaces for indentation in a hash, relative to the first position after the preceding left parenthesis.

maurogeorge added a commit to maurogeorge/active_model_serializers that referenced this pull request Dec 21, 2015
After some debug was discovered that the JSON pointers are not working [1].

This patch make the JSON Pointers working again following the recommendation
from @brandur [2].

As a note json_matchers has a issue that they are doing the movement from
json-schema to json_schema [3].

[1]: 0852b49#commitcomment-14792738
[2]: brandur/json_schema#41 (comment)
[3]: thoughtbot/json_matchers#31
@thoughtbot thoughtbot deleted a comment from houndci-bot Apr 13, 2018
@thoughtbot thoughtbot deleted a comment from houndci-bot Apr 13, 2018
@thoughtbot thoughtbot deleted a comment from houndci-bot Apr 13, 2018
@thoughtbot thoughtbot deleted a comment from houndci-bot Apr 13, 2018
@thoughtbot thoughtbot deleted a comment from houndci-bot Apr 13, 2018
@thoughtbot thoughtbot deleted a comment from houndci-bot Apr 13, 2018
@thoughtbot thoughtbot deleted a comment from houndci-bot Apr 13, 2018
@thoughtbot thoughtbot deleted a comment from houndci-bot Apr 13, 2018
@thoughtbot thoughtbot deleted a comment from houndci-bot Apr 13, 2018
@seanpdoyle seanpdoyle force-pushed the lw-use-other-json-schema-validator branch from 5533850 to 06862a5 Compare April 13, 2018 14:50
"required": [plural],
"properties": {
plural => {
"$ref": "#/definitions/#{plural}"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/TrailingCommaInHashLiteral: Put a comma after the last item of a multiline hash.

@@ -41,5 +44,15 @@ def build_validator(payload)
schema_path: schema_path,
)
end

def document_store

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lint/DuplicateMethods: Method JsonMatchers::Matcher#document_store is defined at both lib/json_matchers/matcher.rb:28 and lib/json_matchers/matcher.rb:48.

seanpdoyle added a commit that referenced this pull request Apr 13, 2018
In preparation for [#31][#31], this commit removes support for global
and matcher-specific options, like `strict: true`.

The [`json_schema` gem][gem] does not accept similar configuration
options, so we'll remove support entirely.

[#31]: #31
[gem]: https://github.com/brandur/json_schema#programmatic
seanpdoyle added a commit that referenced this pull request Apr 13, 2018
In preparation for [#31][#31], this commit removes support for global
and matcher-specific options, like `strict: true`.

The [`json_schema` gem][gem] does not accept similar configuration
options, so we'll remove support entirely.

[#31]: #31
[gem]: https://github.com/brandur/json_schema#programmatic
seanpdoyle added a commit that referenced this pull request Apr 13, 2018
In preparation for [#31][#31], this commit removes support for global
and matcher-specific options, like `strict: true`.

The [`json_schema` gem][gem] does not accept similar configuration
options, so we'll remove support entirely.

[#31]: #31
[gem]: https://github.com/brandur/json_schema#programmatic
seanpdoyle added a commit that referenced this pull request Apr 13, 2018
In preparation for [#31][#31], this commit deprecates support for global
and matcher-specific options, like `strict: true`.

The [`json_schema` gem][gem] does not accept similar configuration
options, so in the future we'll need to remove support entirely.

[#31]: #31
[gem]: https://github.com/brandur/json_schema#programmatic
seanpdoyle added a commit that referenced this pull request Apr 13, 2018
In preparation for [#31][#31], this commit deprecates support for global
and matcher-specific options, like `strict: true`.

The [`json_schema` gem][gem] does not accept similar configuration
options, so in the future we'll need to remove support entirely.

Bump gem version to `0.9.0`.

[#31]: #31
[gem]: https://github.com/brandur/json_schema#programmatic
seanpdoyle added a commit that referenced this pull request Apr 13, 2018
In preparation for [#31][#31], this commit deprecates support for global
and matcher-specific options, like `strict: true`.

The [`json_schema` gem][gem] does not accept similar configuration
options, so in the future we'll need to remove support entirely.

Bump gem version to `0.9.0`.

[#31]: #31
[gem]: https://github.com/brandur/json_schema#programmatic
seanpdoyle added a commit that referenced this pull request Apr 13, 2018
In preparation for [#31][#31], this commit removes support for global
and matcher-specific options, like `strict: true`.

The [`json_schema` gem][gem] does not accept similar configuration
options, so we'll remove support entirely.

[#31]: #31
[gem]: https://github.com/brandur/json_schema#programmatic
seanpdoyle added a commit that referenced this pull request Apr 13, 2018
In preparation for [#31][#31], this commit removes support for global
and matcher-specific options, like `strict: true`.

The [`json_schema` gem][gem] does not accept similar configuration
options, so we'll remove support entirely.

[#31]: #31
[gem]: https://github.com/brandur/json_schema#programmatic
seanpdoyle added a commit that referenced this pull request Apr 13, 2018
In preparation for [#31][#31], this commit removes support for global
and matcher-specific options, like `strict: true`.

The [`json_schema` gem][gem] does not accept similar configuration
options, so we'll remove support entirely.

[#31]: #31
[gem]: https://github.com/brandur/json_schema#programmatic
@seanpdoyle seanpdoyle force-pushed the lw-use-other-json-schema-validator branch from 8aaaf02 to 5c4a437 Compare April 13, 2018 19:29
Copy link

@mike-burns mike-burns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes!

README.md Outdated
To learn more about `$ref`, check out
[Understanding JSON Schema Structuring][$ref].

[$ref]: http://spacetelescope.github.io/understanding-json-schema/structuring.html

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https now!

Dir.glob("#{JsonMatchers.schema_root}/**/*.json").
map { |path| Pathname.new(path) }.
map { |schema_path| Parser.new(schema_path).parse }.
map { |schema| document_store.add_schema(schema) }

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably an #each here.

@seanpdoyle seanpdoyle force-pushed the lw-use-other-json-schema-validator branch from d69cf35 to dfba814 Compare April 13, 2018 20:30
The problem:

* `json_matchers` cannot easily be used concurrently with Heroku's
  JSON API tools, i.e. `prmd` and `committee`, because `json_matchers`
  makes different assumptions about the structure of the user's
  schemata. An example of an incompatibility can be found in
  #25: `json_matchers`
  breaks when the `id` property is present within a schema, but the Heroku
  tools require the presence of the `id` property
  ([reference](https://github.com/interagent/prmd/blob/master/docs/schemata.md#meta-data)).

  This is happening because the libraries used to dereference JSON
  pointers behave differently. `json-schema`, the library we're
  currently using, appears to conform less strictly to the JSON Schema
  specification than the library the Heroku tools use, `json_schema`.

The solution:

* One solution to this problem is to update `json_matchers` to use the
  same approach to validating schemata as the Heroku tools. This will
  require the following changes:

  1. Use `json_schema` instead of `json-schema` to validate schemata
  2. Update documentation to instruct readers to follow Heroku's
  guidelines for structuring schemata:
https://github.com/interagent/prmd/blob/master/docs/schemata.md

* In this commit I've replaced `json-schema` with `json_schema` and
  updated the schemata fixtures in the specs. Per [this json_schema
  issue](brandur/json_schema#22), in order to
  dereference JSON pointers referencing schemata in other files we need
  to access the gem's DocumentStore API directly. This is done in
  `Matcher#build_and_populate_document_store`.
@seanpdoyle seanpdoyle force-pushed the lw-use-other-json-schema-validator branch from dfba814 to e8b3cc2 Compare April 13, 2018 20:31
@seanpdoyle seanpdoyle merged commit e8b3cc2 into master Apr 13, 2018
@seanpdoyle seanpdoyle deleted the lw-use-other-json-schema-validator branch April 13, 2018 20:35
@seanpdoyle seanpdoyle mentioned this pull request Nov 9, 2018
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.

None yet

6 participants