Openapi-generator: Duplicate classes / Models are generated if ( $ref) referenced externally.

Created on 19 Apr 2019  ·  18Comments  ·  Source: OpenAPITools/openapi-generator

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

Awaiting feedback Bug Swagger-Parser

Most helpful comment

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.

All 18 comments

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

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

(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

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

waiting for a feedback from swagger-parser team.

Any update would be grateful. Thanks

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

Hi

Any news about this issue?

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.

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

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.

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

Any updates?

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

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

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?

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.

Was this page helpful?
0 / 5 - 0 ratings