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.24k stars 6.43k forks source link

[BUG][Spring] oneOf keyword generates wrong subtype annotations. #15274

Open cnelson00 opened 1 year ago

cnelson00 commented 1 year ago

Description

When generating interfaces using the oneOf discriminator keyword, the generator produces both the correct subtype annotations, and also incorrect annotations that incorrectly match the names of the openapi spec schemas, not the generated objects. I’m think this is because of a bug, but it could be because we’ve incorrectly written the spec. I have run this through the spec validator successfully, however.

What is produced:

@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.PROPERTY, property = "petType", visible = true)
@JsonSubTypes({
  @JsonSubTypes.Type(value = Cat.class, name = "Cat"),
  @JsonSubTypes.Type(value = Dog.class, name = "Dog"),
  @JsonSubTypes.Type(value = Lizard.class, name = "Lizard"),
  @JsonSubTypes.Type(value = Cat.class, name = "cat"),
  @JsonSubTypes.Type(value = Dog.class, name = "dog"),
  @JsonSubTypes.Type(value = Lizard.class, name = "lizard")
})

What is expected:

@JsonSubTypes({
  @JsonSubTypes.Type(value = Cat.class, name = "cat"),
  @JsonSubTypes.Type(value = Dog.class, name = "dog"),
  @JsonSubTypes.Type(value = Lizard.class, name = "lizard")
})

When useOneOfInterfaces is false, the annotations are correct, but Pet is not generated as an interface.

Removing discriminator mapping removes the correct subtypes, leaving only the incorrect subtypes.

openapi-generator version

Latest/6.5

OpenAPI declaration file content or url

See here for the example yml:

example_petstore.yml

Command line used for generation

docker run --rm -v "${PWD}:/input" -v "${PWD}:/output" openapitools/openapi-generator-cli:latest generate `
    -g spring `
    --library spring-boot `
    -i /input/example_petstore.yml `
    -o /output `
    --model-package com.petstore.spec `
    --additional-properties=delegatePattern=true,interfaceOnly=false,defaultInterfaces=false,java8=true,dateLibrary=java8,hideGenerationTimestamp=true,useOneOfInterfaces=true

Steps to reproduce

Download or clone the above yml file into a clean directory. Run the docker command above while in that directory.

Related issues/PRs

Some related issues and PRs I have noted that are on the Java generator:

[BUG][JAVA] AllOf with mapping generates wrong JsonSubTypes annotations · Issue #14917 · OpenAPITools/openapi-generator

allow to specify the useOneOfInterfaces option for java by robbertvanwaveren · Pull Request #15042 · OpenAPITools/openapi-generator

Do not add schema / class name mapping where custom mapping exists #14984

Suggest a fix/enhancement

I am able to spend some time fixing this at work, but would need some guidance from someone more familiar with the project on a good place to start

fbl100 commented 1 year ago

@diyfr Any chance you can help point us in the right direction? OP works on my team and has time dedicated to figuring this out but needs a little guidance.

diyfr commented 1 year ago

have you tried to put the title attribute of your resources in lowercase?

Cat:
  title: cat
cnelson00 commented 1 year ago

That did not work, but changing the class name to cat: seems to fix the issue. I'm testing this with our code and also with the spec to see if I can get it working correctly.

diyfr commented 1 year ago

This will solve the problem for you but it doesn't solve issue.

Test by removing the title attribute. I think there is a conflict between the resource name and this overload with the title attribute

another test i would have done is to write the expected class and see how springdoc generates the documentation ( developper first, not contract first )

edit springdoc not springfox

alexanderzeltinger commented 1 year ago

Hey there, are there any updates here? We ran into the same issue and the proposed workaround did not solve our issue...

george-birsanuc-mc commented 11 months ago

Hi @diyfr,

I think have the same issue or at least a similar one.

As far as I can tell, even though an explicit mapping is defined for the oneOf, it adds the sub type annotations for both the explicit mapping and the default mapping (i.e. the schema object identifier).

daanw-cegeka commented 11 months ago

I had a similar issue and solved it by setting legacyDiscriminatorBehavior to true .

alexbandi commented 10 months ago

I think the problem lies in DefaultCodegen.java.

In the OpenApi specification it states:

In scenarios where the value of the discriminator field does not match the schema name or implicit mapping is not possible, an optional mapping definition MAY be used:

MyResponseType:
 oneOf:
 - $ref: '#/components/schemas/Cat'
 - $ref: '#/components/schemas/Dog'
 - $ref: '#/components/schemas/Lizard'
 - $ref: 'https://gigantic-server.com/schemas/Monster/schema.json'
 discriminator:
   propertyName: petType
   mapping:
     dog: '#/components/schemas/Dog'
     monster: 'https://gigantic-server.com/schemas/Monster/schema.json'

The generator first takes the optional mapping definitions and then adds - for oneOf/anyOf - all the components which have not been added already. Though, the default implicit value, i. e. Dog has not been added since the generator adds dog from the optional mapping definition resulting in duplicates for the same mapping. So the generator needs to check if the component has a mapping already and not if the mapping has a component.

A possible solution - which worked on my side already - could be the following:

        // if there are composed oneOf/anyOf schemas, add them to this discriminator
        if (ModelUtils.isComposedSchema(schema) && !this.getLegacyDiscriminatorBehavior()) {
            var uniqueDescendantsModelNames = uniqueDescendants.stream().map(MappedModel::getModelName).collect(Collectors.toSet());

            var otherDescendants = getOneOfAnyOfDescendants(schemaName, discriminatorPropertyName, schema).stream()
                    // Filter out models that have a mapping already
                    .filter(mappedModel -> !uniqueDescendantsModelNames.contains(mappedModel.getModelName()))
                    .collect(Collectors.toList());

            uniqueDescendants.addAll(otherDescendants);
        }
alexanderzeltinger commented 10 months ago

Nice, that works for me! Thank you :)

fabianschwiering commented 10 months ago

@alexbandi : Thank you for fixing our long-standing project issue.

akrepon commented 5 months ago

@alexbandi I can confirm this fix works.