OpenAPITools / openapi-generator

OpenAPI Generator allows generation of API client libraries (SDK generation), server stubs, documentation and configuration automatically given an OpenAPI Spec (v2, v3)
https://openapi-generator.tech
Apache License 2.0
21.58k stars 6.52k forks source link

[BUG] [java] Polymorphy of models broken in v4.3.1 #9615

Open dummyalias opened 3 years ago

dummyalias commented 3 years ago
Description

Using the java generator, I used to get models where child-classes extend parent classes with version 4.3.0. With version 4.3.1 the child classes do not extend the parent class anymore.

openapi-generator version

It's a regression of version 4.3.1. 4.3.0 worked.

OpenAPI declaration file content or url
openapi: "3.0.0"
info:
    version: 1.0.0
    title: Polymorphy demo
paths:
    /random-pet:
        get:
            responses:
                '200':
                    description: Returning a random pet.
                    content:
                        application/json:
                            schema:
                                $ref: "#/components/schemas/Pet"
components:
    schemas:
        Pet:
            oneOf: 
                - $ref: '#/components/schemas/Dog' 
                - $ref: '#/components/schemas/Cat' 
            discriminator: 
                propertyName: petType
                mapping: 
                    dog: '#/components/schemas/Dog' 
                    cat: '#/components/schemas/Cat' 
        PetBase:
            type: object
            required:
                - petType
            properties:
                petType:
                    type: string
                name:
                    type: string
        Dog:
            allOf:
                - $ref: '#/components/schemas/PetBase'
                - type: object
                  properties:
                      barksAlot:
                          type: boolean
        Cat:
            allOf:
                - $ref: '#/components/schemas/PetBase'
                - type: object
                  properties:
                      goesOutdoors:
                          type: boolean
Generation Details

I run the generator in version 4.3.0 and 4.3.1:

$ java -jar openapi-generator-cli-4.3.0.jar generate -g java -o 4.3.0_result -i polymorphy.yaml
$ java -jar openapi-generator-cli-4.3.1.jar generate -g java -o 4.3.1_result -i polymorphy.yaml
Steps to reproduce

After generating the code in 4.3.0 and 4.3.1, compare Cat.java. The generated model of 4.3.0 will contain public class Cat extends PetBase while the generated model of 4.3.1 will only contain public class Cat (without the inheritance).

marcdejonge commented 3 years ago

I've noticed the same in the Kotlin generator. It seems that something in the core has broken. There are many issues reported that relate to this I think. Is anyone able to see what is going on?

It is still very much broken in 5.1.1 since 4.3.0, which has been over a year!

marcdejonge commented 3 years ago

Ok, an update. In 4.3.1 it added support for the discriminator. After playing around I got the expected result for me by changing the models as follows:

openapi: "3.0.0"
info:
  version: 1.0.0
  title: Polymorphy demo
paths:
  /random-pet:
    get:
      responses:
        '200':
          description: Returning a random pet.
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/Pet"
components:
  schemas:
    Pet:
      type: object
      required:
        - petType
      properties:
        petType:
          type: string
        name:
          type: string
      discriminator:
        propertyName: petType
        mapping:
          dog: '#/components/schemas/Dog'
          cat: '#/components/schemas/Cat'
    Dog:
      allOf:
        - $ref: '#/components/schemas/Pet'
        - type: object
          properties:
            barksAlot:
              type: boolean
    Cat:
      allOf:
        - $ref: '#/components/schemas/Pet'
        - type: object
          properties:
            goesOutdoors:
              type: boolean
dummyalias commented 3 years ago

Very nice, thanks for your input, @marcdejonge! This seems to work perfectly, I'll try to apply this to my "real" project later.

It still seems like a workaround to me, though. Thus I will not yet close this issue.

marcdejonge commented 3 years ago

I think this is just how the discriminator is supposed to work now. I think it's mostly that the documentation around this is not really clear. Maybe that should be the take away from this.

The point is that the discriminator should really be on the base object and not where you wrap it. There is no support for the EXTERNAL_PROPERTY option (from Jackson) for example.

rgoers commented 3 years ago

The problem is that the base class can't always know all the classes that will extend it. In my case, #9756 it isn't even necessary as the base class isn't used for Serialization/Deserialization. But I need the base class for methods that operate on the fields it provides. In my environment we have a "BasePage" object for paged requests. All the services that need paging extend that. BasePage can't possibly know all the services that will support paging as it is in a common library that the services import. This change has made it impossible to support this.

epabst commented 1 year ago

After I got the Pet model working that @marcdejonge provided in https://github.com/OpenAPITools/openapi-generator/issues/9615#issuecomment-861304768, I figured out a workaround for my own issue. I had separate yaml files for my schema model classes, and by moving them into the same file as the endpoints under this, it worked for me!!!

components:
  schemas:

Note, I also applied this fix to avoid the extra AllOf classes: https://github.com/OpenAPITools/openapi-generator/issues/3100#issuecomment-703481081