Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use of --keep-spec-order leads to empty response structs in v0.30.5 #2949

Open
nwidger opened this issue Jun 14, 2023 · 2 comments · May be fixed by #3041
Open

Use of --keep-spec-order leads to empty response structs in v0.30.5 #2949

nwidger opened this issue Jun 14, 2023 · 2 comments · May be fixed by #3041
Labels
bug generate client pending PR

Comments

@nwidger
Copy link

nwidger commented Jun 14, 2023

Problem statement

First off, thanks for go-swagger, it's a great tool and I'm sure it's not an easy one to maintain!

I recently tried upgrading from go-swagger v0.30.4 to v0.30.5 and noticed that some of the generated response structs are now empty. I've tried to reduce it down to a minimal reproducible example below.

Swagger specification

swagger: '2.0'
info:
  title: Users API
  version: 0.0.1

paths:
  /api/v1/users/:
    get:
      responses:
        200:
          $ref: '#/responses/Users'

responses:
  Users:
    description: List of users
    schema:
      allOf:
        - $ref: '#/definitions/APIResponse'
        - properties:
            data:
              $ref: '#/definitions/Users'

definitions:
  APIResponse:
    type: object
    properties:
      timestamp:
        type: string
  Users:
    type: array
    items:
      $ref: '#/definitions/User'
    x-omitempty: false
  User:
    type: object
    properties:
      id:
        type: integer
      admin:
        type: boolean
      name:
        type: string

Steps to reproduce

Put the above spec in a file swagger.yaml and then run the following in the same directory as the swagger.yaml file using a v0.30.4 go-swagger binary:

rm -rf gen/client
mkdir -p gen/client
swagger-v0.30.4 generate client --target gen/client --name Users --spec swagger.yaml --name Users --client-package users --keep-spec-order

open the file gen/client/users/operations/get_api_v1_users_responses.go and note that the GetAPIV1UsersOK struct contains a Payload field:

type GetAPIV1UsersOK struct {
        Payload *GetAPIV1UsersOKBody
}

Now, run the same command with a v0.30.5 go-swagger binary:

rm -rf gen/client
mkdir -p gen/client
swagger-v0.30.5 generate client --target gen/client --name Users --spec swagger.yaml --name Users --client-package users --keep-spec-order

and open the same gen/client/users/operations/get_api_v1_users_responses.go file and note that the GetAPIV1UsersOK struct is now empty:

type GetAPIV1UsersOK struct {
}

Finally, run the command with a v0.30.5 go-swagger binary but omit the --keep-spec-order option:

rm -rf gen/client
mkdir -p gen/client
swagger-v0.30.5 generate client --target gen/client --name Users --spec swagger.yaml --name Users --client-package users

open the file gen/client/users/operations/get_api_v1_users_responses.go and note that the GetAPIV1UsersOK struct once again has a Payload field:

type GetAPIV1UsersOK struct {
        Payload *GetAPIV1UsersOKBody
}

I am not by any means a Swagger expert but this behavior was very surprising to me. Is the spec I'm using invalid or is this a regression of some sort on the part of go-swagger? Please let me know if I can supply any more information that would be helpful.

Environment

swagger version: 0.30.5
go version: 1.20
OS: CentOS 7
@roberth1988
Copy link

I can confirm we have the same issue and it looks like with --keep-spec-order the resolution of $ref are not done fully in our case which leads to invalid code.

@nikimanoledaki
Copy link

+1! 👀 I can also confirm that using v0.30.5 and generate client --keep-spec-order removed all Payload *models.<Name> from the generated client's response structs. I'm guessing that this a bug since it was unexpected / unintended behaviour from this flag.

Can also confirm that the payload fields are there when generating the client with --keep-spec-order with go-swagger v0.30.4.

Environment

swagger version: v0.30.5
go version: 1.20.5
OS: MacOS Ventura

fredbi added a commit to fredbi/go-swagger that referenced this issue Jan 3, 2024
@fredbi
fix(codegen): fixed calling order for WithAutoXOrder
49e49b5 
This PR moves the call to WithAutoXOrder _before_ we validate and
flatten the spec for codegen purpose.

* fixes go-swagger#2949

Other changes:
* layout: moved all the spec massaging to a dedicated source
* allowed WithAutoXOrder to report about errors rather than panicking

Overall, WithAutoXOrder is simplistic and could be improved a lot:
* supporting JSON
* supporting remote $refs, circular $refs
* removing the dependency to yaml.v2
* not producing a temporary file that hangs around
* etc

These improvements are beyond the scope of this PR, which is just to
fix responses. Perhaps a follow up could bring a more general solution to this problem.

Signed-off-by: Frédéric BIDON <fredbi@yahoo.com>
@fredbi fredbi linked a pull request Jan 3, 2024 that will close this issue
Draft
fredbi added a commit to fredbi/go-swagger that referenced this issue Jan 6, 2024
@fredbi
fix(codegen): fixed calling order for WithAutoXOrder
65bde3e 
This PR moves the call to WithAutoXOrder _before_ we validate and
flatten the spec for codegen purpose.

* fixes go-swagger#2949

Other changes:
* layout: moved all the spec massaging to a dedicated source
* allowed WithAutoXOrder to report about errors rather than panicking

Overall, WithAutoXOrder is simplistic and could be improved a lot:
* supporting JSON
* supporting remote $refs, circular $refs
* removing the dependency to yaml.v2
* not producing a temporary file that hangs around
* etc

These improvements are beyond the scope of this PR, which is just to
fix responses. Perhaps a follow up could bring a more general solution to this problem.

Signed-off-by: Frédéric BIDON <fredbi@yahoo.com>
fredbi added a commit to fredbi/go-swagger that referenced this issue Jan 7, 2024
@fredbi
fix(codegen): fixed calling order for WithAutoXOrder
46641cb 
This PR moves the call to WithAutoXOrder _before_ we validate and
flatten the spec for codegen purpose.

* fixes go-swagger#2949

Other changes:
* layout: moved all the spec massaging to a dedicated source
* allowed WithAutoXOrder to report about errors rather than panicking

Overall, WithAutoXOrder is simplistic and could be improved a lot:
* supporting JSON
* supporting remote $refs, circular $refs
* removing the dependency to yaml.v2
* not producing a temporary file that hangs around
* etc

These improvements are beyond the scope of this PR, which is just to
fix responses. Perhaps a follow up could bring a more general solution to this problem.

Signed-off-by: Frédéric BIDON <fredbi@yahoo.com>
fredbi added a commit to fredbi/go-swagger that referenced this issue Jan 7, 2024
@fredbi
fix(codegen): fixed calling order for WithAutoXOrder
b1d8ab4 
This PR moves the call to WithAutoXOrder _before_ we validate and
flatten the spec for codegen purpose.

* fixes go-swagger#2949

Other changes:
* layout: moved all the spec massaging to a dedicated source
* allowed WithAutoXOrder to report about errors rather than panicking

Overall, WithAutoXOrder is simplistic and could be improved a lot:
* supporting JSON
* supporting remote $refs, circular $refs
* removing the dependency to yaml.v2
* not producing a temporary file that hangs around
* etc

These improvements are beyond the scope of this PR, which is just to
fix responses. Perhaps a follow up could bring a more general solution to this problem.

Signed-off-by: Frédéric BIDON <fredbi@yahoo.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug generate client pending PR
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(codegen): fixed calling order for WithAutoXOrder fredbi/go-swagger
4 participants
@nwidger @roberth1988 @fredbi @nikimanoledaki