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
20.85k stars 6.34k forks source link

Duplicate classes / Models are generated if ( $ref) referenced externally. #2701

Open its-atique1987 opened 5 years ago

its-atique1987 commented 5 years ago
Description

If Schemas are referenced in cyclic manner, the Models are generated twice. In below code,the Model ProblemDetails is generated twice as

Is the correct way to reference? If Not why? and How to fix the above case?

openapi-generator version

openapi-generator-cli-4.0.0-beta3

OpenAPI declaration file content or url

openapi.yaml

openapi: 3.0.0
info:
  title: Common Data Types
  version: "1.0"
paths:
  /{appId}/subscriptions:
    get:
      summary: read all of the active subscriptions for the app.
      operationId: getSubscriptionsById
      parameters:
        - name: appId
          in: path
          description: App ID
          required: true
          schema:
            type: integer
            format: int64
      responses:
        '200':
          description: OK (Successful)
          content:
            application/json:
              schema:
                type: array
                items:
                  $ref: '#/components/schemas/Subscription'
        '500':
          $ref: './common.yaml#/components/responses/E500'  
components:
  schemas:
      ProblemDetails:
        type: object
        properties:
          title:
            type: string
            description: A short, human-readable summary of the problem
          status:
            type: integer
            description: The HTTP status code for this occurrence of the problem.
      Subscription:
        type: string

common.yaml

openapi: 3.0.0
info:
title: Common Data Types
version: "1.0"
paths: {}
components:
responses:
E500:
description: Server Error
content:
application/json:
schema:
$ref: './openapi.yaml#/components/schemas/ProblemDetails'
Command line used for generation

java -jar openapi-generator-cli-4.0.0-beta3.jar generate -i openapi.yaml -g spring -o ./temp

auto-labeler[bot] commented 5 years ago

👍 Thanks for opening this issue! 🏷 I have applied any labels matching special text in your issue.

The team will review the labels and make any necessary changes.

ackintosh commented 5 years ago

Thanks for reporting this issue! I'll investigate this. 👀

ackintosh commented 5 years ago

(It's under investigation...)

https://github.com/OpenAPITools/openapi-generator/blob/9c7d4073f47ef2957028396a2df5e7ae6b42ce5a/modules/openapi-generator/src/main/java/org/openapitools/codegen/config/CodegenConfigurator.java#L605-L606

image

ackintosh commented 5 years ago

I've reported the issue in swagger-parser (https://github.com/swagger-api/swagger-parser/issues/1081)

ackintosh commented 5 years ago

waiting for a feedback from swagger-parser team.

its-atique1987 commented 5 years ago

Any update would be grateful. Thanks

ackintosh commented 5 years ago

UPDATE:

I've filed a PR to add a failing test case which reproduce the issue. https://github.com/swagger-api/swagger-parser/issues/1081#issuecomment-493655142

jorgerod commented 4 years ago

Hi

Any news about this issue?

binaryunary commented 4 years ago

This may be related to this issue. In my case duplicate models are generated even without cyclic references. The generator version I'm using is the Dockerized 4.2.3-SNAPSHOT.

I have the following layout:

.
├── catalog-api.yml
├── common-parameters.yml
├── common-responses.yml
├── common-schemas.yml
├── movies-api.yml
└── tvshows-api.yml

catalog-api.yml is the 'root' spec that includes individual APIs e.g. movies-api.yml. For movies-api.yml the dependency graph is as follows:

catalog-api.yml
`-> movies-api.yml
    |-> common-responses.yml
    |   `-> common-schemas.yml
    |-> common-parameters.yml
    `-> common-schemas.yml

You can see the reproducible example here: https://github.com/binaryunary/openapi-duplicate-models.

out/ contains openapi.json that was generated with

docker run --rm -v ${PWD}:/local openapitools/openapi-generator-cli generate -i /local/catalog-api.yml -g openapi -o /local/out

You can see it contains duplicate models like AssetBase and AssetBase_2.

adamdecaf commented 4 years ago

I was able to use a shared file in multiple specifications (which are merged into one later on) without duplicating a model. I think this is the same problem as what is posted here, except my situation has no cyclic deps in the files.

Edit: This is with 4.2.3 in https://github.com/moov-io/go-client

binaryunary commented 4 years ago

After playing around with external refs for a bit I discovered that swagger-parser will do some normalization on them, but it seems to happen lazily.

If you have a ref like

somefile.yml#/components/schemas/SomeSchema

then it will become

./somefile.yml#/components/schemas/SomeSchema

inside the parser after the first time this ref is resolved. Internally SomeSchema is cached when it is first read, when accessed the second time its source is different (somefile.yml != ./somefile.yml) so a number is appended to its name to avoid conflicts.

Prefixing relative paths with ./ solves this problem, but it seems the parser still has problems normalizing the paths relative to the root spec file as having complex relative references (e.g. different levels, from different subdirectories) still break the parser.

schlagi123 commented 4 years ago

Good news, the issue is resolved in the Swagger-Parser (https://github.com/swagger-api/swagger-parser/issues/1081#issuecomment-607169681, https://github.com/swagger-api/swagger-parser/issues/1292). We must only wait for the next release, to use it in the generator

hleb-albau commented 4 years ago

Any updates?

adamdecaf commented 4 years ago

I've been on 4.3.x without this issue. See how I did it above: https://github.com/OpenAPITools/openapi-generator/issues/2701#issuecomment-582152182

hleb-albau commented 4 years ago

@adamdecaf I have same problem as a @binaryunary on latest master. I have 3 files If I have next dependency structure

-> main.yml
    |-> other.yml
    |   ` -> main.yml
    |   `-> common.yml
    `-> common.yml

Than I have two classes from main.yml file, that referenced in other.yml

But if I change it to next scheme, everything okey.

-> main.yml
    |-> other.yml
       `-> main.yml
       `-> common.yml

will provide examples to reproduce latter

Bytheway. Is there way to eliminate cycle dependency for classes with discriminators located in different files?

adamdecaf commented 4 years ago

What is referenced from main.yml? Models? You could try a models.yml. I use a common.yml from a separate repo for global error types (i.e. error models).

jimshowalter commented 3 years ago

We're using the latest versions of swagger-core, swagger-parser, and swagger-codegen:

io.swagger.codegen.v3:swagger-codegen-cli:3.0.23 io.swagger.codegen.v3:swagger-codegen:3.0.23 io.swagger.core.v3:swagger-annotations:2.1.5 io.swagger.core.v3:swagger-core:2.1.5 io.swagger.core.v3:swagger-models:2.1.5 io.swagger.parser.v3:swagger-parser-core:2.0.24 io.swagger.parser.v3:swagger-parser-v3:2.0.24 io.swagger.parser.v3:swagger-parser:2.0.24

But we still have this problem.

I'd happily preprocess our schema files if there's some way to munge them into a format that won't cause this.

What would the munged format need to look like?

jimshowalter commented 3 years ago

When we fixed this locally in a fork, the fix looked like this:


if (existingModel != null) {
--
  | LOGGER.debug("A model for " + existingModel + " already exists");
  |  
  | /*
  | * Added an additional check here - schema.get$ref() != null to ensure we generate
  | * a single model file for a single model schema.
  | */
  | if(existingModel.get$ref() != null \|\| schema.get$ref() != null) {
  | // use the new model
  | existingModel = null;
  | }else{
  | //We add a number at the end of the definition name
  | int i = 2;
  | for (String name : schemas.keySet()) {
  | if (name.equals(possiblyConflictingDefinitionName)) {
  | tryName = possiblyConflictingDefinitionName + "_" + i;
  | existingModel = schemas.get(tryName);
  | i++;
  | }
  | }
  | }
  | }

That's not quite what the latest commit to ExternalRefProcessor in your github is doing.

yangguangxiadeyun commented 3 years ago

This may be the work-around for this case. https://openapi-generator.tech/docs/usage/#type-mappings-and-import-mappings

rj93 commented 3 years ago

I can see that the issue as been fixed in swagger-parser 2.0.19, and that the openapi-generation has updated the version used but this issue in present in both 5.0.1 and 5.1.0.

Strangely, downgrading to either 5.0.0 or 4.3.1 fixes the issue for me.

Any further updates of this?

slaven3kopic commented 3 years ago

It is happening with the newest (5.1.1) version as well. Downgrading to 5.0.0 or 4.3.2 didn't help.

Any updates of this?

smasala commented 3 years ago

@wing328 any news on this? Thanks

stefancplace commented 3 years ago

Having the same problem, is this going to be fixed?

jimshowalter commented 3 years ago

There are multiple problems related to external refs. I provided reproducible cases in https://github.com/swagger-api/swagger-parser/issues/1518

YvesBos commented 3 years ago

Has been fixed in https://github.com/swagger-api/swagger-parser/pull/1566, no duplicate classes are generated anymore when using version 2.0.26.

jimshowalter commented 3 years ago

Has it been tested with the reproducible cases in https://github.com/swagger-api/swagger-parser/issues/1518 ?

The history of this problem has been that a fix goes in for one symptom and that causes other symptoms to show up.

It's never been completely addressed.

mattrcc95 commented 2 years ago

any news?

nnanda2016 commented 2 years ago

Is there a workaround for this issue?

jimshowalter commented 2 years ago

nnanda2016, see the workaround I posted above on Dec 7, 2020 (sorry about the weird formatting).

asoltesz commented 2 years ago

This seems to be a very serious issue and a complete show-stopper for the generator.

(just bumped into it and it seems there is no workaround either, unless you fork it and fix the error yourself)

chialunwu commented 2 years ago

Bumping this. Same issue in typescript generator as well.

rafael-as-martins commented 2 years ago

Same problem here. Duplicated classes get generated once another yaml $ref another yaml's component. A bit sad not seeing any oficial workaround regarding this issue since it has been reported a while ago.

ssrm commented 2 years ago

.

sourav-jha commented 2 years ago

any workaround for this?

wing328 commented 2 years ago

As a workaround, you may want to give https://github.com/apiture/api-ref-resolver a try to consolidate multiple OpenAPI spec files into a single one.

ankush953 commented 2 years ago

https://github.com/OpenAPITools/openapi-generator/issues/2701#issuecomment-582320928 Thank you for your analysis. I was able to resolve it. I prefixed each path in $ref with ./ and duplicate classes are gone.

skumaridas10 commented 1 year ago

Duplicate model classes even referring internal $ref. Any solution yet?

Ilunko commented 1 year ago

Hi all, For example insted of: './common.yaml#/components/responses/E500' write './common.yaml/#/components/responses/E500'

After this modification I was able to remove duplicity ;)

ghost commented 1 year ago

What worked for us:

  1. Flatten your folder hierarchy. All YAML files (the parent as well as the children YAML files) should be in the same folder
  2. Change all URLs to start with ./
  3. Generate your classes

This seems to be an issue of generating duplicate classes due to the normalization of URLs and the resulting references being different. Would resolving the URLs into absolute paths not help in this case?

jimshowalter commented 1 year ago

Flattening was the first thing I tried. It didn't help in our case.

mauritsdebruin commented 5 months ago

Same issue here. This bug is annoying.

Olleritsch commented 3 months ago

Workaround that worked for me:

I have "directory up" paths in my $ref in the OAS YAML like this: $ref: ../models/Team.yaml and $ref: ../models/User.yaml

When generating server code with the openapi-generator, making use of the inlineSchemaNameMappings option fixed the issue for me: --inline-schema-name-mappings Team_1=Team,User_1=User, works with the Maven plugin as well (similar option there)

gregopet commented 2 weeks ago

Thanks for the workaround! To anyone else who can't get it working at first: the trick is to find the correct internal name OpenAPI uses for your particular object that may not be the same as it appears in your programming language (in our case we had to use State_1 and not State1 as it appeared in Java.

You can see the actual names in the log output when running the generator, or as the contents of @JsonTypeName in the generated source file (at least in the Java generator) - though we have found the logs to be more consistently reliable.

And I have to say I would appreciate at least a warning from OpenAPI generator when I'm telling it to remap a type that then doesn't actually exist :innocent: