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

Add oneOf composite schema support? #97

Open
lzhoucs opened this issue Apr 28, 2019 · 2 comments
Open

Add oneOf composite schema support? #97

lzhoucs opened this issue Apr 28, 2019 · 2 comments
Labels
question Further information is requested

Comments

@lzhoucs
Copy link
Contributor

lzhoucs commented Apr 28, 2019

Currently we seems to always aggregate FieldDescriptors for different requests with the same path + method + mediaType, which is fine in most cases. However, in our case, we have an API that supports multiple schemas (within the same path + method + mediaType) by using a discriminator type polymorphism approach.

I created a simple pet demo branch for demo.

Currently the generated api spec looks like:

paths:
  /pet-demo:
    post:
      requestBody:
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/pet-demo-1939693445'
components:
  schemas:
    pet-demo-1939693445:
      type: object
      properties:
        dogProp:
          type: string
          description: A dog specific property
        petType:
          type: string
          description: cat
        name:
          type: string
          description: Name of cat
        catProp:
          type: string
          description: A cat specific property

Expected api spec:

paths:
  /pet-demo:
    post:
      requestBody:
        content:
          application/json:
            schema:
              oneOf:
                - $ref: '#/components/schemas/cat'
                - $ref: '#/components/schemas/dog'
              discriminator:
                propertyName: petType
components:
  schemas:
    cat:
      type: object
      properties:
        petType:
          type: string
          description: cat
        name:
          type: string
          description: Name of cat
        catProp:
          type: string
          description: A cat specific property
    dog:
      type: object
      properties:
        petType:
          type: string
          description: dog
        name:
          type: string
          description: Name of dog
        dogProp:
          type: string
          description: A dog specific property

Swagger UI recognizes it and render the oneOf composite schemas as follows:
image

Please let me know if there's any question. I can add more details if anything above is not clear.

@mduesterhoeft
Copy link
Contributor

mduesterhoeft commented Apr 29, 2019

I can understand your expectation. But the case is really different with restdocs. restdocs does not look at the source code to create a documentation (you swagger UI is generated by spring-fox?). spring-restdocs is completely test-driven (and this is a good idea if you ask me).

So how should we know that you want separate schemas for different request. I have seen cases where aggregating the schemas was exactly what we wanted.

But if you have a good idea on how to solve this I would be in to discuss. Otherwise I would tend to say that this is not in scope for restdocs-api-spec.

(Also in terms of API design I would question if it is a good idea for an endpoint to return completely different responses for different cases - but this is just my personal view)

@mduesterhoeft mduesterhoeft added the question Further information is requested label Apr 29, 2019
@lzhoucs
Copy link
Contributor Author

lzhoucs commented Apr 29, 2019

Thanks for the response. I completely understand what you said above. I agree the test driven approach is a good idea, otherwise we wouldn't switch to this tool which doesn't work on the source code level. Let me try to answer some of your questions:

you swagger UI is generated by spring-fox?

No, it was manually written based on the generated api spec from this tool. My "Expected api spec" above translate exactly to the SwaggerUI screenshot above. If you'd like to see a complete working example in swagger, please copy the following to http://editor.swagger.io/:

openapi: 3.0.1
info:
  title: My API
  description: An ecommerce sample demonstrating restdocs-api-spec
  version: 0.1.0
servers:
- url: https://localhost:8080
tags:
- name: carts
  description: Carts related resources
- name: products
  description: Products related resources
paths:
  /pet-demo:
    post:
      tags:
      - pet-demo
      summary: Create a dog
      description: Create a dog
      operationId: create-dog
      requestBody:
        content:
          application/json:
            schema:
              oneOf:
                - $ref: '#/components/schemas/cat'
                - $ref: '#/components/schemas/dog'
              discriminator:
                propertyName: petType
      responses:
        200:
          description: "200"
components:
  schemas:
    cat:
      type: object
      properties:
        petType:
          type: string
          description: cat
        name:
          type: string
          description: Name of cat
        catProp:
          type: string
          description: A cat specific property
    dog:
      type: object
      properties:
        petType:
          type: string
          description: dog
        name:
          type: string
          description: Name of dog
        dogProp:
          type: string
          description: A dog specific property

Please note that there is a known Swagger UI issue: swagger-api/swagger-ui#4819, so only "Schema" not "Example Value" works at the time of writing.

So how should we know that you want separate schemas for different request. I have seen cases where aggregating the schemas was exactly what we wanted.

You are exactly right. In most cases aggregating different requests are what we wanted, and probably should continue to be the default behavior if oneOf is going to be supported. I am thinking of introducing an optional method discriminatorOperator that takes an enum(oneOf anyOf etc):

.requestFields(
         fieldWithPath("petType").description("cat").discriminatorOperator(ONE_OF),

This would then allow the aggregation process to identify different requests with the same path + mediaType + method + discriminatorOperator, and then separate the schemas with specified discriminatorOperator instead of combine.

I understand this is not a mainstream API style, however it is supported by open api spec 3, as such I can imagine companies to use it. So I think it would nice to support it if possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants