Skip to content

Conversation

@shockey
Copy link

@shockey shockey commented Apr 12, 2019

Description

This PR is the second phase of my schema validation improvement work (phase 1 was introduced via #1982 and #1984).

It includes changes to the OpenAPI validation schema documents, our AJV instance, and one small modification to our local JSON Schema meta-document.

A few techniques have been deployed here:

  • migrating usage of oneOf to artificial switch constructs that pivot the validation schema based on specific keys
    • most helpfully, using inline content schemas unless a $ref property is present
  • providing custom, contextual error messages for assertion keywords within specific Objects
  • flattening unnecessary constructs in the validation schemas

Notably, this modifies generated errors to report their locations with JSON Pointer-compliant strings. Downstream consumers of schema errors may need to make changes accordingly, but this is not a breaking change since schema errors are within an internal subsystem.

Motivation and Context

Fixes #1853.
Fixes #1832.
Fixes #1808.
Fixes #1797.
Fixes #1711.
Fixes #1709.
Fixes #1672.
Fixes #1519.
Fixes #1511.
Fixes #1489.
Fixes #1480.
Fixes #1394.

How Has This Been Tested?

No tests for now - I may add tests for the linked issues before merging.

Checklist

My PR contains...

  • No code changes (src/ is unmodified: changes to documentation, CI, metadata, etc.)
  • Dependency changes (any modification to dependencies in package.json)
  • Bug fixes (non-breaking change which fixes an issue)
  • Improvements (misc. changes to existing features)
  • Features (non-breaking change which adds functionality)

My changes...

  • are breaking changes to a public API (config options, System API, major UI change, etc).
  • are breaking changes to a private API (Redux, component props, utility functions, etc.).
  • are breaking changes to a developer API (npm script behavior changes, new dev system dependencies, etc).
  • are not breaking changes.

Documentation

  • My changes do not require a change to the project documentation.
  • My changes require a change to the project documentation.
  • If yes to above: I have updated the documentation accordingly.

Automated tests

  • My changes can not or do not need to be tested.
  • My changes can and should be tested by unit and/or integration tests.
  • If yes to above: I have added tests to cover my changes.
  • If yes to above: I have taken care to cover edge cases in my tests.
  • All new and existing tests passed.

@shockey
Copy link
Author

shockey commented Apr 12, 2019

please build

@shockey shockey marked this pull request as ready for review April 17, 2019 20:26
@shockey
Copy link
Author

shockey commented Apr 18, 2019

Fin

@shockey shockey merged commit 06a5ad2 into swagger-api:master Apr 18, 2019
@shockey shockey changed the title improvement: schema validation error quality (phase 2) improvement: schema validation error quality, phase 2 Apr 18, 2019
shockey added a commit to shockey/swagger-editor that referenced this pull request May 23, 2019
…i#1985)

* adopt @webron's OpenAPI 3.0 schema from OAI/OpenAPI-Specification#1270

permalink: https://github.com/OAI/OpenAPI-Specification/blob/92e15eba1d4591ebfe8c11898c48241e72854381/schemas/v3.0/schema.yaml

* add ajv-errors

* address error messages for swagger-api#1808's Swagger 2.0 example

clarifies the schema and adds custom error messages for unclear error conditions

* address error messages for swagger-api#1808's OpenAPI 3.0 example

* restrict underlying JSON Schema `type` field to simple types only (for swagger-api#1832)

* fix limitation in JSON Pointer conversion helper

* add clear `not` error message (for swagger-api#1489)

* add additionalProperties message (for swagger-api#1394)

* add ajv-keywords

* use `switch` to intelligently identify inline vs referenced content (for swagger-api#1853)

* use `switch` to XOR `schema` and `content` (for swagger-api#1853)

* use `switch` to pivot security scheme based on type

(for swagger-api#1672)

* use switch to fall-through to inline security scheme validation (for swagger-api#1672)

* rewrite more Reference oneOfs (for swagger-api#1519)

* add custom message for `Schema.required` type error (for swagger-api#1519)

* rewrite Response/Reference oneOf (for swagger-api#1489)

* use switch in ParameterLocation validation (for swagger-api#1797)

* define pivot key switches for SecurityDefinitions (for swagger-api#1711)

* give helpful `format: uri` messages for SecurityDefinitions (for swagger-api#1711)

* eliminate NonBodyParameter; pivot on `Parameter.in` with a switch (for swagger-api#1511)

* oneOf -> switch for Parameters.items reference

* (for swagger-api#1711)

* remove redundant semantic validator (for swagger-api#1511)

* adjust wording of custom error message (for swagger-api#1853)

* add regression tests for all related issues

* revert to expect@^1.20.2

* linter fixes

* fix messaging flaw for swagger-api#1832

* improve messaging for swagger-api#1394

* use literal key for `$ref` in Reference Object

* remove commented legacy data from OAS3 schema

* remove superfluous quotation marks

* normalize test case paths to `/`

* normalize openapi fields to 3.0.0

* drop unused `paths` information

* ensure clear errors for 3.0 Parameter style/content exclusivity

* add `required` assertions to switch statements that pivot on a key's value

this prevents false positives when the pivot key is missing entirely

* remove stray space
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment