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

Schema example generation should be straightened. #6470

Open
mathis-m opened this issue Oct 5, 2020 · 16 comments
Open

Schema example generation should be straightened. #6470

mathis-m opened this issue Oct 5, 2020 · 16 comments

Comments

@mathis-m
Copy link
Contributor

mathis-m commented Oct 5, 2020

Background

I have been working on a couple of issues and improvements related to examples of schemas.
The biggest problem is that there are many components that are displaying examples in different ways.
Some of them are using the util getSampleSchema(). This is a function that generates sample for a provided schema and content-type. Others just use stringify() on the examples provided for requestBody or responses.

Issues

Inconsistency in components

In my opinion all components that display samples of a schema should be using the utility getSampleSchema in order to respect the content-type.

Inconsistency in generation

While creating PR #6456 I noticed that the generation itself is inconsistent. The logic for integrating the example key of a schema into the generated sample is not the same for xml and json related content-type.

Swagger/OpenAPI definition:

openapi: 3.0.1
info:
 title: Example Swagger
 version: '1.0'
servers:
 - url: /api/v1
paths:
 /xmlTest:
   get:
     summary: sample issues
     operationId: registerSubscription
     parameters: []
     responses:
       '200':
         description: Simple example
         content:
           application/xml:
             schema:
               $ref: "#/components/schemas/Test"
           application/json:
             schema:
               $ref: "#/components/schemas/Test"
components:
 schemas:
   Test:
     type: object
     xml:
       name: root
     properties:
       x:
         type: string
       other:
         type: string
         format: email
     example: 
       x: what the f

The xml generator function sampleXmlFromSchema does:

  • Uses parent's example of children as fallback for child schema example (Only one nesting deep)
  • Uses example value merged with sample generated from schema without example
    image

The json generator function sampleFromSchema instead:

  • only uses only the example value or generated schema example exclusively
    image

xml generator function clearly does a better job from my side.
In addition I would implement example fall back to closest parent schema's example that contains example value for current schema.

Inconsistency in handling examples

I think it would be a nice enhancement if there would be a utility that allows to parse the example value to json.

  • xml -> json
  • csv -> json

With this the getSampleSchema could also integrate literal examples within the generation process.

schema -> use content-type to gen sample json
->merge ->use content-type to stringify merged sample
raw example ->parse to json

For #6442 when using getSampleSchema there is the need to override the root schema example with content-type or media-type example. In case a property's schema itself specifies an example, this will not lead to an override, because in case of an example defined in nested schema too it will use the nested for the sample. This is why I would like to introduce an optional parameter to getSampleSchema(..., overrideExample = undefined) this could be used to override all examples at any given nesting level. Other advantage would be not missing some example values that could be used to complete the sample.

Would like to hear some thoughts on this before I start developing.

Open Questions

  • Does the getSampleSchema fit the Response examples do not respect media-type #6442 use case with merge and override logic? Or would it be a better approach to just parse the example value to JSON and then stringify it respectingly to the content-type or media type without merging schema sample?
  • How to display just a response or requestBody example, without taking schema into consideration, using override with merge approach?
@mathis-m mathis-m changed the title Scheme example generation should be straightened. Schema example generation should be straightened. Oct 5, 2020
@tim-lai
Copy link
Contributor

tim-lai commented Oct 7, 2020

One issue is that schema examples may not actually have a parent/child relationship, particularly across contentTypes. So it would be better to replace and not merge. Using the provided definition, it's possible that the server response to application/json actively rejects the other field, or uses the other field to determine that it should handle the request as xml-specific.

@mathis-m
Copy link
Contributor Author

mathis-m commented Oct 8, 2020

In case it should be replace logic, shouldn't xml generator also use this logic?
Because currently the xml sample generation is doing things differently then in json generation.

@mathis-m
Copy link
Contributor Author

mathis-m commented Oct 8, 2020

Using the provided definition, it's possible that the server response to application/json actively rejects the other field, or uses the other field to determine that it should handle the request as xml-specific.

I think if this is a use case the content-types should be using different schemas to clarify this.

@mathis-m
Copy link
Contributor Author

mathis-m commented Oct 8, 2020

One issue is that schema examples may not actually have a parent/child relationship, particularly across contentTypes. So it would be better to replace and not merge.

If all the content-type sample generators would use the json logic this would be the case. Since it will return the example if found on prio 1.

@mathis-m
Copy link
Contributor Author

mathis-m commented Oct 8, 2020

How about examples that are schema related and not content-type. Should they use merge approach?

@tim-lai
Copy link
Contributor

tim-lai commented Oct 8, 2020

I think if this is a use case the content-types should be using different schemas to clarify this.

I agree. Unfortunately, we've seen issues where this is not the case.

@tim-lai
Copy link
Contributor

tim-lai commented Oct 8, 2020

How about examples that are schema related and not content-type. Should they use merge approach?

I think if content-type matches the schema, the examples should be merged. This would be my general expectation of a consistent api definition.

@webron any thoughts on this?

@mathis-m
Copy link
Contributor Author

mathis-m commented Oct 8, 2020

So from technical side I think it might make sense that if there is an example or examples key on requestBody/content/<content-type> or responses/<code>/content/<content-type> it will just use the value from the example key to generate a sample using the <content-type>. Of cause the same for <OAS3.
If there is no example that is defined for specific content-type (if it is defined on some schema itself) it would use the example's value defined on the schema + if object or array merge with the other sample parts generated from array items or object properties that are not covered with the example values.

@mathis-m
Copy link
Contributor Author

Started adding unit tests

@tim-lai
Copy link
Contributor

tim-lai commented Oct 16, 2020

@mathis-m I took another look at the OpenAPI specification regarding the Media Type Object. I was incorrect to state that I should expect a merge, instead it should be an overwrite. Basically, example/examples from media type takes precedence. This makes sense because one could and often does have nested schemas.

https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.1.0.md

Example of the media type. The example object SHOULD be in the correct format as specified by the media type. The example field is mutually exclusive of the examples field. Furthermore, if referencing a schema which contains an example, the example value SHALL override the example provided by the schema.

Examples of the media type. Each example object SHOULD match the media type and specified schema if present. The examples field is mutually exclusive of the example field. Furthermore, if referencing a schema which contains an example, the examples value SHALL override the example provided by the schema.

Regarding xml vs json, swagger-ui does extra work to make xml behave more like json. So we shouldn't try to go the other way and change json behavior. What could be done from a consistency perspective is creating a selector and/or helper functions outside of the component. It's not ideal that the current component tries to handle the logic, especially with the back-and-forth between ImmutableJS and plain JS.

@tim-lai
Copy link
Contributor

tim-lai commented Oct 16, 2020

Note, PR #6456 is now merged. However, open to refactoring as part of fixing #6475 as well, per consistency comment above.

@mathis-m
Copy link
Contributor Author

@tim-lai alright, thanks for the input. Will create a PR for the refactoring and fix for #6475. Shall the overwrite be implemented for xml, could include that as well?

@tim-lai
Copy link
Contributor

tim-lai commented Oct 20, 2020

@tim-lai alright, thanks for the input. Will create a PR for the refactoring and fix for #6475. Shall the overwrite be implemented for xml, could include that as well?

Overwrite for xml should be a separate PR. Thanks.

tim-lai added a commit that referenced this issue Nov 3, 2020
* ref: #6470 
* fixes: #6540
* fixes: #4943 

* add example override option to json
* add example override option to xml
* added basic oneOf and anyOf support
* fix anyof|oneof
* only lift xml to items


Co-authored-by: Tim Lai <[email protected]>
@crocom
Copy link

crocom commented Jan 20, 2021

Are there any plans to add support for yaml media type examples? Currently examples given as schema objects will render as either json or xml, but to get them to display as yaml I must copy and paste a separate example as a multi-line string.

Here is an example spec demonstrating the different behaviours:

openapi: 3.0.0
info:
  version: '0.1'
  title: Yaml vs Json Examples
  description: ''
paths:
  /resource1:
    post:
      summary: Add some resource - examples are json or xml
      operationId: myOp1
      responses:
        '201':
          description: resource created
        '400':
          description: Invalid input
      requestBody:
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/ModelA'
            examples:
              willRenderAsJson:
                $ref: '#/components/examples/example1'
          application/yaml:
            schema:
              $ref: '#/components/schemas/ModelA'
            examples:
              willRenderAsJson:
                $ref: '#/components/examples/example1'
          application/xml:
            schema:
              $ref: '#/components/schemas/ModelA'
            examples:
              willRenderAsXml:
                $ref: '#/components/examples/example1'                 
        description: Resource to add 
  /resource2:
    post:
      summary: Add some resource - examples are string or yaml
      operationId: myOp2
      responses:
        '201':
          description: resource created
        '400':
          description: Invalid input
      requestBody:
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/ModelA'
            examples:
              willRenderAsString:
                $ref: '#/components/examples/example2'
          application/yaml:
            schema:
              $ref: '#/components/schemas/ModelA'
            examples:
              willRenderAsYaml:
                $ref: '#/components/examples/example2' 
          application/xml:
            schema:
              $ref: '#/components/schemas/ModelA'
            examples:
              willRenderAsYaml:
                $ref: '#/components/examples/example2'          
        description: Resource to add         
components:
  examples:
    example1:
      summary: Example as schema object
      value:
        PropertyA: foo
        PropertyB: bar
    # to get example1 to show as YAML have to copy paste 
    # and pipe the value
    example2:
      summary: Example as literal YAML string
      value: |
        PropertyA: foo
        PropertyB: bar        
  schemas:
    ModelA:
      type: object
      properties:
        PropertyA:
          type: string
        PropertyB:
          type: string

Thanks

@mathis-m
Copy link
Contributor Author

mathis-m commented Jan 20, 2021

@tim-lai I think this would be pretty easy to integrate!

  1. Add new conditional return for yaml content-type
    export const getSampleSchema = (schema, contentType="", config={}, exampleOverride = undefined) => {
    if(schema && isFunc(schema.toJS))
    schema = schema.toJS()
    if(exampleOverride && isFunc(exampleOverride.toJS))
    exampleOverride = exampleOverride.toJS()
    if (/xml/.test(contentType)) {
    return getXmlSampleSchema(schema, config, exampleOverride)
    }
    return getStringifiedSampleForSchema(schema, config, contentType, exampleOverride)
    }
  2. Add getYAMLSchemaFromSample() call that wraps getStringifiedSampleForSchema():
  3. getYAMLSchemaFromSample should convert the json example result to yaml like swagger-editor does on paste:
import YAML from "js-yaml"
let yamlString
try {
  yamlString = YAML.safeDump(YAML.safeLoad(originalStr), {
    lineWidth: -1 // don't generate line folds
  })
} catch (e) {
  return
}

@tim-lai
Copy link
Contributor

tim-lai commented Jan 21, 2021

re: media type examples... moved to new issue: #6857

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

No branches or pull requests

3 participants