Skip to content
This repository has been archived by the owner on Nov 8, 2024. It is now read-only.

Adds JSON Schema Draft 6/7 support #355

Merged
merged 20 commits into from
Dec 6, 2019
Merged

Adds JSON Schema Draft 6/7 support #355

merged 20 commits into from
Dec 6, 2019

Conversation

artem-zakharchenko
Copy link
Contributor

@artem-zakharchenko artem-zakharchenko commented Nov 26, 2019

Contains backward-compatible changes that enable JSON Schema Draft 6/7 support (pre-requisite for OpenAPI support in Dredd).

Changelog

  • Splits legacy (Amanda, TV4) and "future" AJV validators into separate classes
  • Provides a master-class to determine a proper validator based on the JSON Schema explicit or implicit version
  • Adjusts existing unit/integration test suites to align with the introduced changes
  • FIX: Makes errors[n].location.property names consistent and validator-agnostic (property is not always an array that points to the deeply nested property, i.e. ['friends', 2, 'name'])
  • Added a single point of coercion of arbitrary validation structure to Gavel-compliant validation structure (toGavelResults()) and respective unit tests

GitHub

Roadmap

  • Split legacy and next JSON Schema Draft versions validation into separate validator classes
  • Provide a master class that would determine which validator class to use
  • Resolve "schema with key or id already exists" during V6-7 validation with AJV
  • Verify new schemas V6-7 are valid representations of their draft versions
  • Discuss/Revert previously present implicit logic to waterfall attempts to validate a given JSON Schema without an explicit $schema version (first as V3, then as V4)
  • Revert support of implicit JSON Schema version (fallback to V3 if no version is set via $schema property)
  • Adjust derived JsonExample class to implement JsonSchemaValidator properly
  • Adjust the usage surface of new JsonSchema()
    • validateBody
    • validateHeaders
  • Adjust affected tests
    • test/unit/validators/json-schema.test
    • amanda-to-gavel-shared
    • TextDiff unit tests
    • validate integration tests
  • Add new tests
    • Unit test to assert that amanda/tv4/ajv error format is coerced to Gavel-compliant format correctly
  • Infer a JSON Schema Draft V7 if a given JSON Schema contains a single boolean value

@artem-zakharchenko artem-zakharchenko changed the title feat: adds JSON Schema Draft 6/7 support Adds JSON Schema Draft 6/7 support Nov 26, 2019
@@ -7,6 +7,7 @@ class UnknownValidatorError extends Error {}
class NotValidatableError extends Error {}
class NotEnoughDataError extends Error {}
class JsonSchemaNotValid extends Error {}
class JsonSchemaNotSupported extends Error {}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Explicit error type to use in unit tests instead of asserting an exact message.

};

class JsonSchemaLegacy extends JsonSchemaValidator {
validateSchema() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Overriding JsonSchemaValidator.validateSchema() to use TV4 to validate both V3 and V4 (copying previous implementation, refactored).

}
};
const { JsonSchemaLegacy } = require('./json-schema-legacy');
const { JsonSchemaValidator, getSchemaVersion } = require('./json-schema-next');

class JsonSchema {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A wrapping class to determine which actual validator class to use. A temporary wrapper until we deprecate Amanda and TV4. Overkill: determines a JSON Schema's version twice (once in the wrapper class, another time in the actual validator class).

@artem-zakharchenko
Copy link
Contributor Author

artem-zakharchenko commented Nov 26, 2019

@kylef, please, do you mind giving my approach a look? I would like to verify my vision is optimal before I proceed. Also to make sure the valid/invalid schema versions I added are okay.

Thanks.

@artem-zakharchenko
Copy link
Contributor Author

artem-zakharchenko commented Nov 29, 2019

I've removed amanda-to-gavel-shared module entirely, since I've also removed evaluateOutputToResults(). Instead, JsonSchemaLegacy would use an internal method to coerce Amanda/TV4 result format to Gavel-compliant error format.

Since Gavel cannot accept Amanda/TV4 validation result as input, it's safe to remove and play around with these test suites. They are testing internal functionality.

I will add a new test suite to cover that the newly added method converts Amanda/TV4/AJV result format to Gavel-compliant error format properly as the next step.

message: 'Amanda error message'
},
1: {
property: ['friends[2]', 'online'],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Amanda produces prop[n] property, which appears to be invalid JSON pointer.

I've refactored the coercing logic so it produces ['fiends', 2, 'online'] JSON pointer format.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This would be considered a breaking change, but it's rather a bug fix, because it fixes an unexpected property format. It's expected to be a list of stings, and it was resolved to a plain string sometimes.

@artem-zakharchenko
Copy link
Contributor Author

@kylef, could you please take a look at this pull request? It should be ready. Thanks 😊

@artem-zakharchenko artem-zakharchenko merged commit 3215a44 into master Dec 6, 2019
@artem-zakharchenko artem-zakharchenko deleted the 90-ajv branch December 6, 2019 09:20
@ApiaryBot
Copy link
Collaborator

🎉 This PR is included in version 8.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use ajv JSON Schema Validator
3 participants