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

[ZEN-3765] Example Data: Validate examples against OpenAPI Schema Object #486

Merged
merged 8 commits into from
Apr 9, 2019

Conversation

ghillairet
Copy link
Member

@ghillairet ghillairet commented Mar 20, 2019

See update #486 (comment)

Add preliminary support for example validation in OpenAPI3 and Swagger.

oai

…nt with OAS conformance suite

Add preliminary support for example validation in OpenAPI3 and Swagger.
@ghillairet ghillairet requested a review from tedepstein March 20, 2019 10:28
…nt with OAS conformance suite

This adds normalization of schemas present in Swagger/OpenAPI. When an example is detected, the object being it's schema is passed through the normalizer to be transformed into a single object where all references are resolved.
This also extends the validation preferences for Swagger/OpenAPI to add an option to enable example validation. This option is set to false by default.
@ghillairet
Copy link
Member Author

@tedepstein Ready to review, see PR https://github.com/ModelSolv/RepreZen/pull/1876 for build

Example validator now normalizes schemas in Swagger/OpenAPI specs to resolve any references before running the validation.

Added Validation Preferences

Screen Shot 2019-03-21 at 09 42 10

Screen Shot 2019-03-21 at 09 55 41

@tedepstein
Copy link
Collaborator

tedepstein commented Mar 21, 2019

Needs to be modified to deal with recursive (circular) references. These cannot be inlined, they must be localized. See the Normalizer documentation and codebase for a full explanation.

We also need the preference setting to be enabled by default.

…nt with OAS conformance suite

Add extension point.
…nt with OAS conformance suite

Removing example validators as they will be implemented as extensions inside API Studio.
Also modified the preference pages and initialisers so that that validation extensions can add and initialise their own values.
…nt with OAS conformance suite

KZOE and API Studio both provide their own version of json-schema-validator and this is causing an exception to be thrown when a validator extension is created in API Studio and called from KZOE. To avoid this issue, all call to json-schema-validator should be done through the KZOE class `JsonSchemaValidator` that will execute the validation without exposing any json-schema-validator API.
…nt with OAS conformance suite

Add the extension point `com.reprezen.swagedit.preferences` that allow clients to provide their own preferences to KaiZen editors.
…nt with OAS conformance suite

Add missing change in plugin.xml
@ghillairet
Copy link
Member Author

Final changes to this PR are about adding extensions points to KZOE, so that clients (namely API Studio) can provide their own validation processors.

The 2 extensions points are:

  • com.reprezen.swagedit.validator that should provide an implementation of ValidationProvider
  • com.reprezen.swagedit.preferences that should provide an implementation of PreferenceProvider

ValidationProviders are called inside the Validation class on each nodes. The clients that provide an implementation for it can then execute custom validation on each node in the document.

PreferenceProvider can be used to provide custom preferences. Those preferences will be added in Swagger or OpenAPI preference pages. The extension point provides an attribute preferencePage that will be used to identify which preference page should be extended.

@tedepstein
Copy link
Collaborator

Should be extended to validate example property on a Schema Object.

try {
return (T) e;
} catch (ClassCastException ex) {
return null;
Copy link
Contributor

@nbhusare nbhusare Apr 5, 2019

Choose a reason for hiding this comment

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

A log would help trace the problem. .filter(Objects:: nonNull) will filter out null-values and the only way to identify the problem would be to debug this code. Debugging lambdas in Eclipse is not easy though.

try {
return jsonSchema.validate(document, true);
jsonSchema = factory.getJsonSchema(schema, schemaPointer);
} catch (ProcessingException e) {
Activator.getDefault().logError(e.getLocalizedMessage(), e);
return null;
Copy link
Contributor

Choose a reason for hiding this comment

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

You should return an empty Set or document warning the consumer that the method may return a null value if things go wrong.

@tedepstein
Copy link
Collaborator

tedepstein commented Apr 8, 2019

NOTE: See updated comments on ModelSolv/RepreZen/pull/1876

QA Test Results: Fail

The feature generally works, and we can merge the PR. But it has at least one unrecoverable error, meaning there are valid OpenAPI documents that cannot be used because they get a false-negative (error) result from the validator.

There are other issues where the validation yields false-positives in some cases (i.e. validation doesn't check what it should be checking) and false-negatives in other cases (raising errors where the schema is valid).

Pending further testing, we can merge the PR. (Further comments to follow here...)

Tests were created using examples within the following template, by adding examples to the request and response of the post operation in the /pets path item:

openapi: "3.0.0"
info:
  version: 1.0.0
  title: Expanded Petstore example.
paths:
  /pets:
    post:
      requestBody:
        content:
          application/json:
            example:
              name: Frisbee
              tag: dog
            schema:
              $ref: '#/components/schemas/NewPet'
      responses:
        200:
          description: pet response
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/Pet"
components:
  schemas:
    Pet:
      allOf:
        - $ref: '#/components/schemas/NewPet'
        - type: object
          required:
          - id
          properties:
            id:
              type: integer
              format: int64
    NewPet:
      type: object
      required:
        - name  
      properties:
        name:
          type: string
        tag:
          type: string    

Each test case will describe modifications that were made to this template to get the reported results.

Referenced Object Schema: Pass

Changing the example to break the rules yielded expected schema validation errors.

      requestBody:
        content:
          application/json:
            example:
              name: 1234  # should be string, not number
              tag: 
              - arrays are 
              - not allowed.
            schema:
              $ref: '#/components/schemas/NewPet'

Places a single error marker on the example line:

Multiple markers at this line
 - instance type (array) does not match any allowed primitive type (allowed: ["string"])
 - instance type (integer) does not match any allowed primitive type (allowed: ["string"])
 - 5 added lines

Changing to multiple examples yields the same error message on the second example (array element), as expected:

      requestBody:
        content:
          application/json:
            examples:
              validExample:
                value:
                  name: Snoopy
                  tag: Peanuts
              notValidExample:
                value:
                  name: 1234  # should be string, not number
                  tag: 
                  - arrays are 
                  - not allowed.
            schema:
              $ref: '#/components/schemas/NewPet'

Inline Object Schema: Pass

Works as expected with the schema moved inline:

      requestBody:
        content:
          application/json:
            example:
              id: 465
              name: 1234  #should be string, not number
              tag: 
              - arrays are 
              - not allowed.
            schema:
                type: object
                required:
                  - name  
                properties:
                  id:
                    type: integer
                    format: int64
                  name:
                    type: string
                  tag:
                    type: string

Note: In the above example, we added the id property with an int64 format. The validation error now includes an additional warning:

Multiple markers at this line 
- instance type (integer) does not match any allowed primitive type (allowed: ["string"])
- instance type (array) does not match any allowed primitive type (allowed: ["string"])
- format attribute "int64" not supported

If the errors are corrected (first two items), the error marker changes to a warning marker, and showing only the 'format attribute "int64" not supported' warning message. This is not a critical problem, but it would be best to filter those warnings out, when they occur on standard OpenAPI formats.

Inline or referenced allOf Schema: Fail

In the OpenAPI test template, the Pet schema is composed using allOf. Referencing this schema or copying it inline doesn't work. Schema validation appears not to take place at all.

      requestBody:
        content:
          application/json:
            example:
              id: BadValue  
              name: 1324  #should be string, not number
              tag: Dog
            schema:
              allOf:
              - $ref: '#/components/schemas/NewPet'
              - type: object
                required:
                - id
                properties:
                  id:
                    type: integer
                    format: int64

The Eclipse error log shows entries like this:

com.github.fge.jsonschema.core.exceptions.ProcessingException: fatal: JSON Reference "#/components/schemas/NewPet" cannot be resolved
    level: "fatal"
    schema: {"loadingURI":"#","pointer":"/allOf/0"}
    ref: "#/components/schemas/NewPet"

	at com.github.fge.jsonschema.core.load.RefResolver.rawProcess(RefResolver.java:121)
	at com.github.fge.jsonschema.core.load.RefResolver.rawProcess(RefResolver.java:51)
	at com.github.fge.jsonschema.core.processing.RawProcessor.process(RawProcessor.java:77)
	at com.github.fge.jsonschema.core.processing.RawProcessor.process(RawProcessor.java:41)
	at com.github.fge.jsonschema.core.processing.ProcessorChain$ProcessorMerger.process(ProcessorChain.java:189)
	at com.github.fge.jsonschema.core.processing.ProcessingResult.of(ProcessingResult.java:79)
	at com.github.fge.jsonschema.core.processing.CachingProcessor$1.load(CachingProcessor.java:128)
	at com.github.fge.jsonschema.core.processing.CachingProcessor$1.load(CachingProcessor.java:120)
	at com.google.common.cache.LocalCache$LoadingValueReference.loadFuture(LocalCache.java:3716)
	at com.google.common.cache.LocalCache$Segment.loadSync(LocalCache.java:2424)
	at com.google.common.cache.LocalCache$Segment.lockedGetOrLoad(LocalCache.java:2298)
	at com.google.common.cache.LocalCache$Segment.get(LocalCache.java:2211)
	at com.google.common.cache.LocalCache.get(LocalCache.java:4154)
	at com.google.common.cache.LocalCache.getOrLoad(LocalCache.java:4158)
	at com.google.common.cache.LocalCache$LocalLoadingCache.get(LocalCache.java:5147)
	at com.github.fge.jsonschema.core.processing.CachingProcessor.process(CachingProcessor.java:109)
	at com.github.fge.jsonschema.processors.validation.ValidationChain.process(ValidationChain.java:107)
	at com.github.fge.jsonschema.processors.validation.ValidationChain.process(ValidationChain.java:57)
	at com.github.fge.jsonschema.core.processing.ProcessorMap$Mapper.process(ProcessorMap.java:166)
	at com.github.fge.jsonschema.core.processing.ProcessingResult.of(ProcessingResult.java:79)
	at com.github.fge.jsonschema.core.processing.CachingProcessor$1.load(CachingProcessor.java:128)
	at com.github.fge.jsonschema.core.processing.CachingProcessor$1.load(CachingProcessor.java:120)
	at com.google.common.cache.LocalCache$LoadingValueReference.loadFuture(LocalCache.java:3716)
	at com.google.common.cache.LocalCache$Segment.loadSync(LocalCache.java:2424)
	at com.google.common.cache.LocalCache$Segment.lockedGetOrLoad(LocalCache.java:2298)
	at com.google.common.cache.LocalCache$Segment.get(LocalCache.java:2211)
	at com.google.common.cache.LocalCache.get(LocalCache.java:4154)
	at com.google.common.cache.LocalCache.getOrLoad(LocalCache.java:4158)
	at com.google.common.cache.LocalCache$LocalLoadingCache.get(LocalCache.java:5147)
	at com.github.fge.jsonschema.core.processing.CachingProcessor.process(CachingProcessor.java:109)
	at com.github.fge.jsonschema.processors.validation.InstanceValidator.process(InstanceValidator.java:110)
	at com.github.fge.jsonschema.processors.validation.InstanceValidator.process(InstanceValidator.java:59)
	at com.github.fge.jsonschema.keyword.validator.draftv4.AllOfValidator.validate(AllOfValidator.java:68)
	at com.github.fge.jsonschema.processors.validation.InstanceValidator.process(InstanceValidator.java:129)
	at com.github.fge.jsonschema.processors.validation.ValidationProcessor.process(ValidationProcessor.java:56)
	at com.github.fge.jsonschema.processors.validation.ValidationProcessor.process(ValidationProcessor.java:34)
	at com.github.fge.jsonschema.core.processing.ProcessingResult.of(ProcessingResult.java:79)
	at com.github.fge.jsonschema.main.JsonSchema.doValidate(JsonSchema.java:76)
	at com.github.fge.jsonschema.main.JsonSchema.validate(JsonSchema.java:109)
	at com.reprezen.swagedit.core.validation.JsonSchemaValidator.doValidate(JsonSchemaValidator.java:99)
	at com.reprezen.swagedit.core.validation.JsonSchemaValidator.validate(JsonSchemaValidator.java:81)
	at com.modelsolv.reprezen.ui.editors.validation.ExampleValidationProvider.doValidate(ExampleValidationProvider.java:147)
	at com.modelsolv.reprezen.ui.editors.validation.ExampleValidationProvider.validate(ExampleValidationProvider.java:77)
	at com.reprezen.swagedit.core.validation.Validator.lambda$0(Validator.java:133)
	at java.base/java.lang.Iterable.forEach(Iterable.java:75)
	at com.reprezen.swagedit.core.validation.Validator.validateDocument(Validator.java:131)
	at com.reprezen.swagedit.core.validation.Validator.validate(Validator.java:107)
	at com.reprezen.swagedit.openapi3.validation.OpenApi3Validator.validate(OpenApi3Validator.java:99)
	at com.reprezen.swagedit.core.validation.Validator.validate(Validator.java:89)
	at com.reprezen.swagedit.core.editor.ValidationOperation.validateSwagger(ValidationOperation.java:109)
	at com.reprezen.swagedit.core.editor.ValidationOperation.run(ValidationOperation.java:90)
	at com.reprezen.swagedit.core.editor.JsonEditor$6.doRunInWorkspace(JsonEditor.java:429)
	at com.reprezen.swagedit.core.editor.JsonEditor$SafeWorkspaceJob.runInWorkspace(JsonEditor.java:503)
	at org.eclipse.core.internal.resources.InternalWorkspaceJob.run(InternalWorkspaceJob.java:39)
	at org.eclipse.core.internal.jobs.Worker.run(Worker.java:56)

Referenced Example Object: Fail

Adding a reusable Example Object here:

components:
  examples:
    NewPetExample:
      value:
        id: foo # should be number
        name: 1324 # should be string
        tag: 
        - should
        - be
        - string

And referencing it here:

            examples:
              foo:
                $ref: "#/components/examples/NewPetExample"

Puts an error marker on referencedExample:

object has missing required properties (["name"])

It seems the validation code does not try to resolve the $ref to the example object.

Object Array Schema: Pass

This raises validation errors as expected:

      requestBody:
        content:
          application/json:
            examples:
              referencedExample:
                value: 
                - id: 1234
                  name: zing
                - name: fred
                  tag: 1234
            schema:
                type: array
                items:
                  type: object
                  required:
                    - name  
                  properties:
                    id:
                      type: integer
                      format: int64
                    name:
                      type: string
                    tag:
                      type: string

Referenced array schema also works, with this NewPets reusable schema :

components:
  schemas:
    NewPets:
      type: array
      items:
        $ref: "#/components/schemas/NewPet"
    NewPet:
      type: object
      required:
        - name  
      properties:
        name:
          type: string
        tag:
          type: string    

... and this reference to NewPets:

      requestBody:
        content:
          application/json:
            examples:
              referencedExample:
                value: 
                - id: 1234
                  name: zing
                - name: fred
                  tag: 1234
            schema:
              $ref: "#/components/schemas/NewPets"

We see the expected validation errors.

Primitive Array Schema: Pass

This works:

      requestBody:
        content:
          application/json:
            examples:
              example1:
                value: 
                - The quick
                - Brown
                - Fox
            schema:
              type: array
              items:
                type: string
                maxLength: 5

Primitive Schema: Pass

This works:

      requestBody:
        content:
          application/json:
            examples:
              example1:
                value: Fighters
            schema:
              type: string
              maxLength: 5

Schema Object example property: Fail

This was known from our walkthrough. Schemas can contain an example keyword, but this doesn't trigger validation in the current implementation. it would be helpful to have this working.

    NewPets:
      type: array
      items:
        $ref: "#/components/schemas/NewPet"
      example:
        - id: 234
          name: Fred
          tag: 982734

Property Subschema References: Pass

This works, raising expected validation errors:

      requestBody:
        content:
          application/json:
            examples:
              example1:
                value: 
                  orderID: ASDFASDF
                  orderDate: "2018-12-01"
                  pet:
                    foo: bar
                    name: Simone
                    tag: 67
            schema:
              type: object
              title: Pet Order
              properties:
                orderID:
                  type: string
                  maxLength: 5
                orderDate:
                  type: string
                  format: date
                pet:
                  $ref: "#/components/schemas/NewPet"
              required:
                - orderID

It works the same if I extract the Pet Order schema to schema/components/PetOrder and reference it there.

OpenAPI-Specific Keywords: Pass with issues.

In this schema, the readOnly keyword is part of the OpenAPI spec, but not part of JSON Schema proper.

        content:
          application/json:
            examples:
              example1:
                value: 
                  orderID: ASDF
                  orderDate: "2018-12-01"
                  pet:
                    foo: bar
                    name: Simone
                    tag: Cockapoo
            schema:
              type: object
              title: Pet Order
              properties:
                orderID:
                  readOnly: true
                  type: string
                  maxLength: 5
                orderDate:
                  type: string
                  format: date
                pet:
                  $ref: "#/components/schemas/NewPet"
              required:
                - orderID

The readOnly property is tolerated, but shows a warning.

Multiple markers at this line - format attribute "date" not supported - the following keywords are unknown and will be 
 ignored: [readOnly]

It would be good to filter out this warning on keywords that are defined in the OpenAPI spec.

JSON Format Example

This works as expected:

      requestBody:
        content:
          application/json:
            examples:
              example1:
                value: { 
                  'orderID': 'ASDF',
                  'orderDate': "2018-12-01",
                  'pet': {
                    'foo': 'bar',
                    'name': 'Simone',
                    'tag': 'Cockapoo'
                  }
                }
            schema:
              type: object
              title: Pet Order
              properties:
                orderID:
                  readOnly: true
                  type: string
                  maxLength: 5
                orderDate:
                  type: string
                  format: date
                pet:
                  $ref: "#/components/schemas/NewPet"
              required:
                - orderID

JSON or YAML as Embedded String Value: Fail

This may be a problem:

      requestBody:
        content:
          application/json:
            examples:
              example1:
                value: | 
                  { 
                    'orderID': 'ASDF',
                    'orderDate': "2018-12-01",
                    'pet': {
                      'name': 'Simone',
                      'tag': 'Cockapoo'
                    }
                  }
            schema:
              type: object
              title: Pet Order
              properties:
                orderID:
                  type: string
                  maxLength: 5
                orderDate:
                  type: string
                pet:
                  $ref: "#/components/schemas/NewPet"
              required:
                - orderID

The example1: line is marked with the following error:

instance type (string) does not match any allowed primitive type (allowed: ["object"])

The error is the same regardless of the media type. Changing the media type to "application/xml", "text/csv", or other values doesn't change the result. If the string is valid XML, validation is bypassed. But any non-JSON, non-XML string causes the error.

There are a couple of questions here:

  1. Do we want to allow JSON and YAML example values as strings? The mock service has a bit of extra logic to "unwrap" string-valued examples. If the string evaluates as YAML or JSON, it is treated the same as if the embedded string value were specified inline as YAML or JSON. If not, the value is just treated as a string. It's not a critical feature, but it would be good for the example validator to employ similar logic.
  2. Are there cases where a schema is provided, but the example content needs to be a string, not embedded JSON or YAML? There are two scenarios where this is required:
    • An OpenAPI schema can be be used to describe non-JSON formats that can be mapped to JSON. As of now, this is an edge case.
    • There is an Alternative Schema feature planned for OpenAPI 3.1. This will make OpenAPI much more complete and viable for APIs that use non-JSON wire formats. We might see non-JSON example string values as a much more common occurrence in these cases.

I think we could accommodate these cases with more robust media type logic, and with some new preference settings.

Preference for handling of example string values

We could add a checkbox like this:

image

Note that the current "Enable examples validation" checkbox has been renamed for clarity.

Improved Media Type Logic

I think example validation should be dependent on media types, with three categories of media types:

  • "Known Non-JSON" media types - Identified by a list of regular expressions, excluded from schema validation.
  • "Known JSON" media types - Identified by a list of regular expressions, defaults to error severity if the value is not valid JSON, valid YAML, or a string that can be parsed as JSON or YAML.
  • "Possible JSON" media types - Not one of the known JSON or non-JSON media types. Defaults warning severity if the value is not valid JSON, valid YAML, or a string that can be parsed as JSON or YAML.

Here's the logic I would propose:

  • if schema is missing, null, empty string, blank string (whitespace), empty object, or true, don't attempt to validate.
  • else if the media type is known to be non-JSON, don't validate.
    • The list of known non-json media types could include xml and maybe some other common non-json media types. XML can be captured as a pair of regular expressions:
      • \/xml (matches application/xml or any 6 other media types containing "/xml")
      • \+xml$ (matches 804 known media types ending in "+xml")
    • The list could be made configurable in a later release with an "Exclude media types..." button under the "Parse example string values" checkbox. Clicking the button would show the editable list of RegEx list for known non-JSON types in a modal dialog.
  • else if the media type is known to be JSON
    • if the value is JSON or YAML validate it.
    • else if "parse example values as JSON" is enabled
      • if the example is a string that successfully parses as JSON or YAML, unwrap and validate.
      • else flag an error on the example:
        "Schema validation could not process non-JSON example string with known JSON media type."
        • The severity could be made user-configurable as error, warning, or ignore, with an entry in a categorized list of validations:
          "Schema validation could not process non-JSON example string with known JSON media type."
        • Known JSON media types would include a list of two regular expressions:
          • \/json (matches application/json and others)
          • \+json$ (matches any media type ending in +json, like application/vnd.hal+json)
        • The list could be made configurable in a later release with a "Known JSON media types..." button under the "Parse example string values" checkbox. Clicking the button would show the editable list of RegEx list for known non-JSON types in a modal dialog.
    • else don't validate
  • else
    • if the value is JSON or YAML validate it.
    • else if "parse example values as JSON" is enabled
      • if the example is a string that successfully parses as JSON or YAML, unwrap and validate.
      • else flag a warning on the example:
        "Schema validation could not process non-JSON example string with possible JSON media type."
        • The severity could be made user-configurable as error, warning, or ignore, with an entry in a categorized list of validations:
          "Schema validation could not process non-JSON example string with possible JSON media type."
    • else don't validate

Non-JSON media types

Example Object in Parameter and Header (Direct)

Example Object in Parameter and Header with Media Type Object

Circular Reference

Shared Reference

Issues:

  • allOf schemas don't seem to work at all.
  • We get warnings on use of int64 and other format values that are not part of standard JSON Schema. Since they are only warnings, they are not a critical issue.
  • Similar warnings on readOnly and other OpenAPI schema keywords that are not part of the JSON Schema standard. We should filter these out if possible.
  • Validation errors on subschemas should specify a property name, relative JSON pointer, or line number if possible. Without this, it's hard to tell which subschema has the validation problem. Alternatively, we could place error markers specifically on the subschema content or containing keyword, but I expect this would be harder.

@nbhusare nbhusare merged commit 5c68cd5 into master Apr 9, 2019
@ghillairet
Copy link
Member Author

@tedepstein tedepstein changed the title [ZEN-3017] OAS3 Level 3 Validation: Schema Objects, examples, alignment with OAS conformance suite [ZEN-3765] Example Data: Validate examples against OpenAPI Schema Object Apr 9, 2019
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.

3 participants