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
21.75k stars 6.56k forks source link

[BUG] [Go] Execution responses and errors are values not pointers #8483

Closed aeneasr closed 2 years ago

aeneasr commented 3 years ago

Bug Report Checklist

Description

Generating a Go-client yields in code that does not adhere to Golang's error handling. Request executors return GenericOpenAPIError as a value:

func (r AdminApiApiDeleteIdentityRequest) Execute() (*_nethttp.Response, GenericOpenAPIError) {
    return r.ApiService.DeleteIdentityExecute(r)
}

which makes it impossible to check if err != nil as the variable is a value (and thus always not nil) and not a pointer. Instead, this should read:

func (r AdminApiApiDeleteIdentityRequest) Execute() (*_nethttp.Response, *GenericOpenAPIError) {
    return r.ApiService.DeleteIdentityExecute(r)
}

The same applies to return values which should also be pointers, not structs:

func (a *HealthApiService) IsInstanceReadyExecute(r HealthApiApiIsInstanceReadyRequest) (HealthStatus, *_nethttp.Response, GenericOpenAPIError) {

should be:

func (a *HealthApiService) IsInstanceReadyExecute(r *HealthApiApiIsInstanceReadyRequest) (HealthStatus, *_nethttp.Response, *GenericOpenAPIError) {
openapi-generator version

5.0.0

OpenAPI declaration file content or url

https://gist.github.com/aeneasr/3c11c5b454df6c5e4d8e7a3e16d04db7

Generation Details
# gen.go.yml
disallowAdditionalPropertiesIfNotPresent: false
packageName: kratos
generateInterfaces: false
isGoSubmodule: false
structPrefix: true
$ npm run openapi-generator-cli -- generate -i ".schema/api.openapi.json" \
                -g go \
                -o "internal/httpclient" \
                --git-user-id ory \
                --git-repo-id kratos-client-go \
                --git-host github.com \
                -c .schema/openapi/gen.go.yml
Steps to reproduce

See above.

Related issues/PRs
Suggest a fix

I know too little java and internals of how this system works. But basically calls to GenericOpenAPIError{/* ... */} should always be pointers &GenericOpenAPIError{ /* ... */ }. Same for API return values.

aeneasr commented 3 years ago

I see that this is partially a duplicate - however the other issues and fixes do not appear to talk about the response values, only errors. Response values should also be pointers because:

  1. Response values are empty if an error occurs and should thus be nil
  2. There is no point in having pass-by-value as the original value is not used anywhere else - it is created during the call to Execute
aeneasr commented 2 years ago

Fixed