From f49427e992ffa9e96276ea5f698f1ab629e8d595 Mon Sep 17 00:00:00 2001 From: Laila Winner Date: Fri, 30 Oct 2015 15:05:43 -0700 Subject: [PATCH] WIP: Use json_schema instead of json-schema 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 https://github.com/thoughtbot/json_matchers/issues/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](https://github.com/brandur/json_schema/issues/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#add_schemata_to_document_store`. --- json_matchers.gemspec | 3 +- lib/json_matchers/matcher.rb | 44 +++-- .../match_response_schema_spec.rb | 171 +++++++----------- 3 files changed, 90 insertions(+), 128 deletions(-) diff --git a/json_matchers.gemspec b/json_matchers.gemspec index 82388d3..590cf3e 100644 --- a/json_matchers.gemspec +++ b/json_matchers.gemspec @@ -18,7 +18,8 @@ Gem::Specification.new do |spec| spec.test_files = spec.files.grep(%r{^(test|spec|features)/}) spec.require_paths = ["lib"] - spec.add_dependency("json-schema", "~> 2.7") + spec.add_dependency("json_schema") + spec.add_dependency("activesupport", '>= 3.0.0') spec.add_development_dependency "bundler", "~> 1.7" spec.add_development_dependency "pry" diff --git a/lib/json_matchers/matcher.rb b/lib/json_matchers/matcher.rb index e91f0c3..31f7f74 100644 --- a/lib/json_matchers/matcher.rb +++ b/lib/json_matchers/matcher.rb @@ -1,5 +1,4 @@ -require "json-schema" -require "json_matchers/validator" +require "json_schema" module JsonMatchers class Matcher @@ -11,14 +10,22 @@ def initialize(schema_path, options = {}) def matches?(response) validator = build_validator(response) - self.errors = validator.validate! - - errors.empty? - rescue JSON::Schema::ValidationError => error - self.errors = [error.message] - false - rescue JSON::Schema::JsonParseError - raise InvalidSchemaError + begin + add_schemata_to_document_store + schema_data = JSON.parse(File.read(@schema_path.to_s)) + response_body = JSON.parse(@response.body) + json_schema = JsonSchema.parse!(schema_data) + + json_schema.expand_references!(store: document_store) + json_schema.validate!(response_body) + rescue RuntimeError => ex + @validation_failure_message = ex.message + return false + rescue JsonSchema::SchemaError, JSON::ParserError => ex + raise InvalidSchemaError + end + + true end def validation_failure_message @@ -28,18 +35,17 @@ def validation_failure_message private attr_reader :schema_path, :options - attr_accessor :errors - def default_options - JsonMatchers.configuration.options || {} + def add_schemata_to_document_store + Dir.glob("#{JsonMatchers.schema_root}/**/*.json").each do |path| + schema_data = JSON.parse(File.read(path)) + extra_schema = JsonSchema.parse!(schema_data) + document_store.add_schema(extra_schema) + end end - def build_validator(response) - Validator.new( - options: options, - response: response, - schema_path: schema_path, - ) + def document_store + @document_store ||= JsonSchema::DocumentStore.new end end end diff --git a/spec/json_matchers/match_response_schema_spec.rb b/spec/json_matchers/match_response_schema_spec.rb index e68ea10..9d49903 100644 --- a/spec/json_matchers/match_response_schema_spec.rb +++ b/spec/json_matchers/match_response_schema_spec.rb @@ -14,111 +14,29 @@ end it "fails when the body is missing a required property" do - create_schema("foo_schema", + create_schema("foo_schema", { "type": "object", "required": ["foo"], - ) + }) expect(response_for({})).not_to match_response_schema("foo_schema") end - context "when passed a Hash" do - it "validates when the schema matches" do - create_schema("foo_schema", { - "type": "object", - "required": [ - "id", - ], - "properties": { - "id": { "type": "number" }, - }, - "additionalProperties": false, - }) - - expect({ "id": 1 }).to match_response_schema("foo_schema") - end - - it "fails with message when negated" do - create_schema("foo_schema", { - "type": "object", - "required": [ - "id", - ], - "properties": { - "id": { "type": "number" }, - }, - "additionalProperties": false, - }) - - expect { - expect({ "id": "1" }).to match_response_schema("foo_schema") - }.to raise_formatted_error(%{{ "type": "number" }}) - end - end - - context "when passed a Array" do - it "validates when the schema matches" do - create_schema("foo_schema", { - "type": "array", - "items": { - "required": [ - "id", - ], - "properties": { - "id": { "type": "number" }, - }, - "additionalProperties": false, - } - }) - - expect([{ "id": 1 }]).to match_response_schema("foo_schema") - end - - it "fails with message when negated" do - create_schema("foo_schema", { - "type": "array", - "items": { - "type": "object", - "required": [ - "id", - ], - "properties": { - "id": { "type": "number" }, - }, - "additionalProperties": false, - } - }) - - expect { - expect([{ "id": "1" }]).to match_response_schema("foo_schema") - }.to raise_formatted_error(%{{ "type": "number" }}) - end - end - - context "when JSON is a string" do - before(:each) do - create_schema("foo_schema", { - "type": "object", - "required": [ - "id", - ], - "properties": { - "id": { "type": "number" }, - }, - "additionalProperties": false, - }) - end - - it "validates when the schema matches" do - expect({ "id": 1 }.to_json). - to match_response_schema("foo_schema") - end + it "accepts options for the validator" do + create_schema("foo_schema", { + "type": "object", + "required": [ + "id", + ], + "properties": { + "id": { "type": "number" }, + "title": {"type": "string"}, + }, + "additionalProperties": false, + }) - it "fails with message when negated" do - expect { - expect({ "id": "1" }.to_json).to match_response_schema("foo_schema") - }.to raise_formatted_error(%{{ "type": "number" }}) - end + expect(response_for({ "id": 1, "title": "bar" })). + to match_response_schema("foo_schema", strict: false) end it "fails when the body contains a property with the wrong type" do @@ -177,23 +95,60 @@ end it "supports $ref" do - create_schema("single", { + create_schema("user", { + "id": "file:/#{JsonMatchers.schema_root}/user.json#", "type": "object", - "required": ["foo"], + "required": ["id"], "properties": { - "foo": { "type": "string" }, + "id": { + "type": "integer" + } } }) - create_schema("collection", { - "type": "array", - "items": { "$ref": "single.json" }, + create_schema("users", { + "id": "file:/#{JsonMatchers.schema_root}/users.json#", + "type": "object", + "definitions": { + "users": { + "description": "A collection of users", + "example": [{ "id": "1" }], + "type": "array", + "items": { "$ref": "file:/#{JsonMatchers.schema_root}/user.json#" } + } + }, + "required": ["users"], + "properties": { "users": { "$ref": "#/definitions/users" } } }) - valid_response = response_for([{ "foo": "is a string" }]) - invalid_response = response_for([{ "foo": 0 }]) + valid_response = response_for({ "users": [{ "id": 1 }] }) + invalid_response = response_for({ "users": [{ "id": "invalid" }]}) + + expect(valid_response).to match_response_schema("users") + expect(invalid_response).not_to match_response_schema("users") + end + + it "supports the 'id' keyword" do + create_schema("top-level-schema", { + "$schema": "http://json-schema.org/draft-04/schema#", + "type": "object", + "properties": { + "a": { "$ref": "file:/#{JsonMatchers.schema_root}/nested.json#" } + } + }) + create_schema("nested-schema", { + "$schema": "http://json-schema.org/draft-04/schema#", + "id": "file:/#{JsonMatchers.schema_root}/nested.json#", + "type": "object", + "required": ["b"], + "properties": { "b": { "type": "string" } }, + }) + response_json = { a: { b: "foo" } } + invalid_response_json = { a: { b: 4 } } - expect(valid_response).to match_response_schema("collection") - expect(invalid_response).not_to match_response_schema("collection") + expect(response_for(response_json)). + to match_response_schema("top-level-schema") + expect(response_for(invalid_response_json)). + not_to match_response_schema("top-level-schema") end context "when options are passed directly to the matcher" do