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.62k stars 6.29k forks source link

[BUG][GO] Generated client code creates duplicate type names & anonymous interfaces rather than types #14235

Open mikeb26 opened 1 year ago

mikeb26 commented 1 year ago
Description

I am unable to generate golang client bindings for the lichess public api (https://lichess.org/api). The generated code fails to compile due to duplicate type names being generated. Secondly, in many cases the generated code employs anonymous interface{} for many objects which should instead be defined & named types based on the input description.

Input was validated with https://apidevtools.org/swagger-parser/online

openapi-generator version

6.2.0

OpenAPI declaration file content or url

https://github.com/mikeb26/lichess-go-sdk/blob/285cd412550179208649530859434e3b812cf231/openapi.json

Generation Details
java -jar ./openapi-generator-cli.jar generate -g go -o ./openapi --git-host github.com --git-user-id mikeb26 --git-repo-id lichess-go-sdk -i openapi.json
Steps to reproduce
git clone https://raw.githubusercontent.com/mikeb26/lichess-go-sdk
cd lichess-go-sdk
git checkout 285cd412
rm -rf openapi
make
Related issues/PRs

could not find related PRs

Suggest a fix

It appears there are 2 problems:

  1. in a couple of cases the input re-uses the same type across different api endpoints. an easy fix would be to have the generator prefix the generated type names with the api name. (i did this manually by manually editing the generated code).
  2. the generated code is returning responses with anonymous interface{} types rather than defining a type based on the spec. the input spec. I don't have a guess as to why this is occurring.
wtrocki commented 1 year ago

Generally it is good to point issues in the generator and create single issue per problem.

in a couple of cases the input re-uses the same type across different api endpoints. an easy fix would be to have the generator prefix the generated type names with the api name. (i did this manually by manually editing the generated code).

That is controlled by one of the generator options.

the generated code is returning responses with anonymous interface{} types rather than defining a type based on the spec. the input spec. I don't have a guess as to why this is occurring

Would that be an issue with openapi file? If you can point where openapi generator does work not as intented we could action this. Otherwise it is more like stackoverflow question

janhartigan commented 1 year ago

That is controlled by one of the generator options.

@wtrocki would you mind highlighting which option does this? I'm having a hard time finding anything.

wtrocki commented 1 year ago

https://openapi-generator.tech/docs/generators/go/#:~:text=false-,structPrefix,-whether%20to%20prefix

janhartigan commented 1 year ago

@wtrocki it seems that option doesn't impact that particular issue. Taking a simplified spec with two endpoints:

openapi: 3.0.0
info:
  title: Fake API
  version: '1.0'
paths:
  /foo:
    post:
      summary: Foo
      operationId: post-foo
      tags:
        - Something
      responses:
        '200':
          description: OK
        '400':
          description: Bad request
          $ref: '#/components/responses/BadRequestError'
      requestBody:
        $ref: '#/components/requestBodies/FooRequest'
  /moo:
    post:
      summary: Moo
      operationId: post-moo
      tags:
        - Something
      responses:
        '200':
          description: OK
        '400':
          description: Bad request
          $ref: '#/components/responses/BadRequestError'
      requestBody:
        $ref: '#/components/requestBodies/MooRequest'
components:
  responses:
    BadRequestError:
      description: Input is invalid
      content:
        application/json:
          schema:
            type: object
            properties:
              message:
                type: string
            required:
              - message
  requestBodies:
    FooRequest:
      content:
        application/json:
          schema:
            type: object
            properties:
              thing:
                type: string
            required:
              - thing
    MooRequest:
      content:
        application/json:
          schema:
            type: object
            properties:
              thing:
                type: string
            required:
              - thing
tags:
  - name: Something
    description: Related to something

In this case running this:

openapi-generator-cli generate
    --generator-name go
    --input-spec ./openapi.yaml
    --output ./
    --git-repo-id something
    --git-user-id somebody
    --additional-properties disallowAdditionalPropertiesIfNotPresent=false,enumClassPrefix=true,generateInterfaces=true,structPrefix=false

we get a client that looks like this:

Screenshot 2023-03-29 at 16 44 23

Notice how there are only two models: PostFooRequest and PostFoo400Response. If we run this with structPrefix=true, it appends the API tag's prefix to the outer request, but it still uses a duplicated PostFooRequest in each of the two structs:

Screenshot 2023-03-29 at 16 50 26 Screenshot 2023-03-29 at 16 50 43

The best I can tell, whenever it encounters two identical schemas it picks the first one that shows up in the paths and uses that for its name. This might not be that big of a problem for the request bodies since it wraps the model, but in the case of the error responses this can cause some breaking changes with certain usage patterns.

So for example if you look for the 400 response, it throws a PostFoo400Response for both /foo and /moo. But if I change the order of the paths in the spec, that changes to a PostMoo400Response for both paths. It's probably pretty uncommon that you reorder your paths, but you might add new paths to the top of the list. So if I have some code somewhere that does this:

badRequestError, ok := genericOpenApiError.Model().(myapi.PostFoo400Response)

If I add a new path /goo to the top, that code is now broken because the type will actually be myapi.PostGoo400Response.

Let me know if you need me to clarify anything.

janhartigan commented 1 year ago

And just to be clear, what would solve this problem for me is an option to not deduplicate models at all, even if that results in some more code.

wtrocki commented 1 year ago

I understand now. What I'm doing for such cases is to simplify schema pre generator so annonymous interfaces do not exist.

janhartigan commented 1 year ago

@wtrocki I'm not sure I totally understand. Do you mean to avoid using the requestBodies and responses entirely? The duplicate responses above is kind of a contrived example, but reusing a BadRequest response is something I use all the time because it would bloat a spec a lot if I had to put that in each path that uses it. Not sure if that's what you mean tho.

janhartigan commented 1 year ago

@wtrocki I'm curious if you have an further suggestions here. This feels like a bug in the Go generator, but I just want to make sure I'm not missing some configuration parameter that disables this behavior.

dtrunk90 commented 3 months ago

workaround:

before (which results in deduplication):

components:
  schemas:
    Foo:
      type: array
      items:
        type: object
        properties:
          foo:
            type: string
    Bar:
      type: array
      items:
        type: object
        properties:
          foo:
            type: string

after (without anonymous interfaces; no deduplication anymore):

components:
  schemas:
    Foo:
      type: array
      items:
        $ref: '#/components/schemas/FooInner'
    FooInner:
      type: object
      properties:
        foo:
          type: string
    Bar:
      type: array
      items:
        $ref: '#/components/schemas/BarInner'
    BarInner:
      type: object
      properties:
        foo:
          type: string