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

Spring generates duplicated JsonSubTypes #13150

Closed
xibz opened this issue Aug 10, 2022 · 14 comments · Fixed by #14984
Closed

Spring generates duplicated JsonSubTypes #13150

xibz opened this issue Aug 10, 2022 · 14 comments · Fixed by #14984

Comments

@xibz
Copy link

xibz commented Aug 10, 2022

Hello everyone,

When I use the openapi-generator with a discriminator and mapping, it generates two @JsonSubTypes

discriminator:
    propertyName: type
    mapping:
         foo: "#components/schemas/Foo"

Generates something like this

@JsonSubTypes.Type(value = Foo.class, name = "Foo")
@JsonSubTypes.Type(value = Foo.class, name = "foo")

Notice the duplication. The Foo is the class name of the class, and the lowercase foo is my mapping. Wtf is going on lol. This doesn't seem right.

The Foo class is defined something like

Foo:
    allOf:
         - $ref: "#components/schemas/someParent"
         - type: object
            properties:
                type:
                    type: string

I am using version 6.0.1 and the schema version is openapi: '3.0.0'

Here's the full spec to generate the incorrect thing:

openapi: '3.0.0'
info:
  version: '1.0.0'
  title: 'FooService'
paths:
  /parent:
    put:
      summary: put parent
      operationId: putParent
      parameters:
        - name: name
          in: path
          required: true
          description: Name of the account being updated.
          schema:
            type: string
      requestBody:
        description: The updated account definition to save.
        required: true
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/Parent'
      responses:
        '200':
          $ref: '#/components/responses/Parent'
components:
  schemas:
    Parent:
      type: object
      description: Defines an account by name.
      properties:
        name:
          type: string
          description: The account name.
        type:
          type: string
          description: The account type discriminator.
      required:
        - name
        - type
      discriminator:
        propertyName: type
        mapping:
          foo: '#/components/schemas/Foo'

    Foo:
      allOf:
        - $ref: "#/components/schemas/Parent"
        - type: object
          properties:
            fooType:
              type: string 
  responses:
    Parent:
      description: The saved account definition.
      content:
        application/json:
          schema:
            $ref: '#/components/schemas/Parent'

I generated this by running
java -jar openapi-generator-cli.jar generate -i schema.yaml -o foo -g spring

@xibz xibz closed this as completed Aug 10, 2022
@xibz xibz reopened this Aug 10, 2022
@xibz xibz changed the title [BUG] Description Spring generates duplicated JsonSubTypes Aug 10, 2022
@dim5
Copy link

dim5 commented Aug 22, 2022

I also came across this bug using 6.0.0.
As a temporary workaround I renamed the schema to start with lower case (foo) in the api-doc, since it was breaking deserialization on the remote.

@julius-d
Copy link

Hi!
This bug also prevents me from updating from 5.4.0 to 6.1.0

@tjuckel
Copy link

tjuckel commented Sep 28, 2022

For me, this is also a problem. We make heavy use of inheritance and Jackson will serialize the values incorrect. The type value is now Foo instead foo, which is not even part of my discriminator options: { "type": "Foo" }

@arandth
Copy link

arandth commented Oct 7, 2022

Same problem for me, event with version 6.2.0 of the openapi-generator-maven-plugin

@gelli
Copy link

gelli commented Oct 14, 2022

if you add an x-discriminator-value to the components you can even force a third one:

Foo:
  allOf:
    - $ref: "#/components/schemas/Parent"
    - type: object
      properties:
        fooType:
          type: string
  x-discriminator-value: FooDiscriminator

will lead to:

@JsonSubTypes.Type(value = Foo.class, name = "Foo")
@JsonSubTypes.Type(value = Foo.class, name = "foo")
@JsonSubTypes.Type(value = Foo.class, name = "FooDiscriminator")

tested with 6.2.0

bernie-schelberg-mywave added a commit to bernie-schelberg-mywave/openapi-generator that referenced this issue Oct 24, 2022
@wing328
Copy link
Member

wing328 commented Oct 24, 2022

cc @cachescrubber (2022/02) @welshm (2022/02) @MelleD (2022/02) @atextor (2022/02) @manedev79 (2022/02) @javisst (2022/02) @borsch (2022/02) @banlevente (2022/02) @Zomzog (2022/09)

@bernie-schelberg-mywave
Copy link
Contributor

I created 2 PRs to resolve this issue - the first one (#13802) adds a new configuration option to control the behaviour. This may be preferred as there may be users depending on the multiple schema mapping names. Alternatively, the second one (#13815) treats the issue as a bug, and will not add the default mapping name if a custom name or an x-discriminator-value is specified (no additional configuration option). Neither of these PRs prevent a user adding multiple mappings by specifying both a mapping for a discriminator and an x-disciminator-value for the same mapping.

@xibz
Copy link
Author

xibz commented Oct 25, 2022

This may be preferred as there may be users depending on the multiple schema mapping names

I think this is the exception to the rule. I believe most people want a one to one mapping, and if the use case arises for multiple, allow people to just specify that, makes the most sense. If I specify a single mapping, why would two generate?

However, I understand that this would be a breaking change for users if they relied on the side effect of producing multiple JSON sub types, but to say it isn't a bug, seems wrong. Further it is evident in the discussion here that in earlier versions it didn't always do this. So it seems like a regression happened somewhere. So with all that said, Im in favor of #13815

With all that said, thank you for the PRs!

@arandth
Copy link

arandth commented Dec 16, 2022

Hi,
does anybody know how this PR will be merged? I saw it is approved, but the branch not merged yet.

@bernie-schelberg-mywave
Copy link
Contributor

@arandth The advice I've received is that it will likely be included in the 7.0.0 release, scheduled for Q1/Q2 next year.

@arandth
Copy link

arandth commented Dec 19, 2022

Oh, that's disappointing that it takes so long to integrate a bug-fix in the tool :-(
In the meantime: is there a possibility to use a intermediate built version (e.g. as built from bugfix-branch)?

@WeihmannDev
Copy link
Contributor

Thank you for the pull request. I really hope this issue is resolved soon.

wing328 added a commit that referenced this issue Mar 24, 2023
…14984)

* fix #13150 Do not add schema / class name mapping where custom mapping exists

* update test spec

* improve import

* fix import for mapped models

* fix python

* code clean up

* fix dart client import

* fix dart:core import

* better import

* add tests

---------

Co-authored-by: Bernie Schelberg <[email protected]>
@cristian-bdn
Copy link

cristian-bdn commented Sep 4, 2023

Hello, could we reopen this issue? The problem is still there, still getting duplicated JsonSubTypes in 7.0.0 (compared to 5.4.0 for example).
The steps OP provided are still relevant.

update: maybe my issue was something different, however legacyDiscriminatorBehavior removes the duplicate JsonSubTypes

@N1ark
Copy link

N1ark commented Sep 28, 2023

Hello, could we reopen this issue? The problem is still there, still getting duplicated JsonSubTypes in 7.0.0 (compared to 5.4.0 for example). The steps OP provided are still relevant.

update: maybe my issue was something different, however legacyDiscriminatorBehavior removes the duplicate JsonSubTypes

Can confirm, issue had returned and specifying legacyDiscriminatorBehavior: true in the config fixed it again.

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

Successfully merging a pull request may close this issue.