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

[BUG] Generating models with enum discriminator #16073

Open
4 of 6 tasks
isaac33zhang opened this issue Jul 11, 2023 · 4 comments
Open
4 of 6 tasks

[BUG] Generating models with enum discriminator #16073

isaac33zhang opened this issue Jul 11, 2023 · 4 comments

Comments

@isaac33zhang
Copy link

Bug Report Checklist

  • Have you provided a full/minimal spec to reproduce the issue?
  • Have you validated the input using an OpenAPI validator (example)?
  • Have you tested with the latest master to confirm the issue still exists?
  • Have you searched for related issues/PRs?
  • What's the actual output vs expected output?
  • [Optional] Sponsorship to speed up the bug fix or feature request (example)
Description

The model for an object with an enum discriminator generates a constructor that causes a compilation error. Related issue in swagger-codegen: swagger-api/swagger-codegen#10441
For Enum type discriminator, a constructor is being generated that assigns a string to the enum discriminator. For example:
public ExternalKeySpecDetails() { this.ekmType = this.getClass().getSimpleName(); }

openapi-generator version

4.0.1

OpenAPI declaration file content or url
Generation Details
Steps to reproduce
Related issues/PRs

swagger-api/swagger-codegen#10441

Suggest a fix

It doesn't seem the constructor is used anywhere. Can we just get rid of it during the generation?

@meldorq
Copy link

meldorq commented Aug 1, 2023

Here is a real-world example:
https://github.com/europace/baufismart-antraege-api/raw/v2.62/swagger.yaml

The openapi-generator-maven-plugin up to 6.6.0 generates the wrong constructor.

@isaac33zhang
Copy link
Author

Here is a real-world example: https://github.com/europace/baufismart-antraege-api/raw/v2.62/swagger.yaml

The openapi-generator-maven-plugin up to 6.6.0 generates the wrong constructor.

Thanks! Isn't 6.6.0 the latest? So it means we don't have a fix yet?

@meldorq
Copy link

meldorq commented Aug 22, 2023

Thanks! Isn't 6.6.0 the latest? So it means we don't have a fix yet?

Apparently there is no fix yet. I reproduced the problem with the latest master 7.0.0-SNAPSHOT. This means you can tick the checkbox in the Bug Report Checklist to complete the pending task. Perhaps it helps speeding up this issue.

As a workaround, I converted my project to use swagger-codegen instead. This works, but triggers a few warnings about deprecated dependencies (which had been the reason to switch from swagger to openapi earlier).

@adam-sickmiller-collibra
Copy link

adam-sickmiller-collibra commented Aug 23, 2023

I have been doing some additional research on this topic, and I can share what was relevant to me, and I believe also in general.

In the example in this ticket, and in the problematic specification that I was investigating, the problematic object acts in a self-referential way where it has a property needing discrimination on types which are its own children. This in itself doesn't seem to bother the client generators (though maybe it should?). What does cause the client generator to produce uncompilable code is when the allowable values of the discriminated property are also enumerated.

That's a bit of a mouthful and I may not have explained well, so here is an example starting with the inheritance example from the docs and simply adding an enum to the petType property as so:

        petType:
          type: string
          enum: [ Cat, Dog, Lizard ]

Exploding this into a full spec might look like this:

openapi: 3.0.3
info:
  title: Example of using discriminator with the Petstore
  version: '2.0'
  x-audience: public
servers:
  - url: "/rest/2.0"
    variables: {}
paths:
  "/pet/{petId}":
    get:
      summary: Find pet by id
      operationId: getBetById
      parameters:
        - name: petId
          in: path
          required: true
          schema:
            type: string
      responses:
        '200':
          description: OK - The request has succeeded.
          content:
            application/json:
              schema:
                type: array
                items:
                  "$ref": "#/components/schemas/Pet"

components:
  schemas:
    Pet:
      type: object
      required:
        - petType
      properties:
        petType:
          type: string
          enum: [ Cat, Dog, Lizard ]
      discriminator:
        propertyName: petType
        mapping:
          dog: Dog
    Cat:
      allOf:
        - $ref: '#/components/schemas/Pet'
        - type: object
          # all other properties specific to a `Cat`
          properties:
            name:
              type: string
    Dog:
      allOf:
        - $ref: '#/components/schemas/Pet'
        - type: object
          # all other properties specific to a `Dog`
          properties:
            bark:
              type: string
    Lizard:
      allOf:
        - $ref: '#/components/schemas/Pet'
        - type: object
          # all other properties specific to a `Lizard`
          properties:
            lovesRocks:
              type: boolean

To me, this example (regardless of the enum) seems to go against the following language from the spec:
The discriminator object is legal only when using one of the composite keywords oneOf, anyOf, allOf.

Considering this, we can correct this problem by adding a parent object, specifying the enumerable items on it using 'oneOf', and moving the discriminator to this object. for example:

---
openapi: 3.0.3
info:
  title: Example of using discriminator with the Petstore
  version: '2.0'
  x-audience: public
servers:
  - url: "/rest/2.0"
    variables: {}
paths:
  "/pet/{petId}":
    get:
      summary: Find pet by id
      operationId: getBetById
      parameters:
        - name: petId
          in: path
          required: true
          schema:
            type: string
      responses:
        '200':
          description: OK - The request has succeeded.
          content:
            application/json:
              schema:
                type: array
                items:
                  "$ref": "#/components/schemas/PetResponse"
components:
  schemas:
    PetResponse:
      oneOf:
      - $ref: '#/components/schemas/Cat'
      - $ref: '#/components/schemas/Dog'
      - $ref: '#/components/schemas/Lizard'
      discriminator:
        propertyName: petType
        mapping:
          dog: Dog
    Pet:
      type: object
      required:
      - petType
      properties:
        petType:
          type: string
          enum: [ Cat, Dog, Lizard ]
    Cat:
      allOf:
      - $ref: '#/components/schemas/Pet'
      - type: object
        # all other properties specific to a `Cat`
        properties:
          name:
            type: string
    Dog:
      allOf:
      - $ref: '#/components/schemas/Pet'
      - type: object
        # all other properties specific to a `Dog`
        properties:
          bark:
            type: string
    Lizard:
      allOf:
      - $ref: '#/components/schemas/Pet'
      - type: object
        # all other properties specific to a `Lizard`
        properties:
          lovesRocks:
            type: boolean

By adding a parent object we can reduce the purposes given to the 'pet' object and we can create code that the openapi codegen tool will be able to work with.

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