openapi-generator icon indicating copy to clipboard operation
openapi-generator copied to clipboard

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

Open its-atique1987 opened this issue 6 years ago • 48 comments
trafficstars

Description

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

  • ProblemDetails.java
  • ProblemDetails2.java

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

its-atique1987 avatar Apr 19 '19 14:04 its-atique1987

👍 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.

auto-labeler[bot] avatar Apr 19 '19 14:04 auto-labeler[bot]

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

ackintosh avatar Apr 20 '19 00:04 ackintosh

(It's under investigation...)

  • The definitions seems no problem
  • It may caused in Swagger Parser as the SwaggerParseResult has a duplicated schema definition. (ProblemDetails_2)

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

image

ackintosh avatar Apr 20 '19 01:04 ackintosh

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

ackintosh avatar Apr 20 '19 09:04 ackintosh

waiting for a feedback from swagger-parser team.

ackintosh avatar Apr 20 '19 09:04 ackintosh

Any update would be grateful. Thanks

its-atique1987 avatar Apr 23 '19 06:04 its-atique1987

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

ackintosh avatar May 18 '19 07:05 ackintosh

Hi

Any news about this issue?

jorgerod avatar Oct 15 '19 15:10 jorgerod

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.

binaryunary avatar Jan 13 '20 16:01 binaryunary

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

adamdecaf avatar Feb 04 '20 22:02 adamdecaf

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.

binaryunary avatar Feb 05 '20 09:02 binaryunary

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

schlagi123 avatar Apr 01 '20 10:04 schlagi123

Any updates?

hleb-albau avatar Jul 03 '20 17:07 hleb-albau

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

adamdecaf avatar Jul 03 '20 17:07 adamdecaf

@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?

hleb-albau avatar Jul 04 '20 06:07 hleb-albau

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).

adamdecaf avatar Jul 06 '20 16:07 adamdecaf

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 avatar Dec 07 '20 19:12 jimshowalter

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.

jimshowalter avatar Dec 07 '20 20:12 jimshowalter

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

yangguangxiadeyun avatar Dec 31 '20 06:12 yangguangxiadeyun

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?

rj93 avatar Apr 21 '21 12:04 rj93

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?

slaven3kopic avatar May 26 '21 07:05 slaven3kopic

@wing328 any news on this? Thanks

smasala avatar May 26 '21 07:05 smasala

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

stefancplace avatar May 26 '21 07:05 stefancplace

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

jimshowalter avatar May 26 '21 15:05 jimshowalter

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.

YvesBos avatar May 31 '21 10:05 YvesBos

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.

jimshowalter avatar May 31 '21 16:05 jimshowalter

any news?

mattrcc95 avatar Dec 07 '21 14:12 mattrcc95

Is there a workaround for this issue?

nnanda2016 avatar Jan 11 '22 05:01 nnanda2016

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

jimshowalter avatar Jan 12 '22 16:01 jimshowalter

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)

asoltesz avatar Feb 24 '22 16:02 asoltesz