[BUG] [Go] Go client query parameters generated as reflect.Value value instead of actual string values

[hxy9243]

Bug Report Checklist

• Have you provided a full/minimal spec to reproduce the issue?
• Have you validated the input using an OpenAPI validator (example)?
• Have you tested with the latest master to confirm the issue still exists?
• Have you searched for related issues/PRs?
• What's the actual output vs expected output?
• [Optional] Sponsorship to speed up the bug fix or feature request (example)

Description

openapi-generator version

7.0.0 snapshot from https://oss.sonatype.org/content/repositories/snapshots/org/openapitools/openapi-generator-cli/7.0.0-SNAPSHOT/

Jar file link address: https://oss.sonatype.org/content/repositories/snapshots/org/openapitools/openapi-generator-cli/7.0.0-SNAPSHOT/openapi-generator-cli-7.0.0-20221012.083708-4.jar

Verified the behaviors are correct (not generating reflect.Value value but actual string value correctly) on:

• 6.0.0
• 6.0.1
• 6.2.1

It might be a regression in 6.3.0.

OpenAPI declaration file content or URL

  /books/{book-id}:
    get:
      operationId: GET-book
      parameters:
        - in: path
          name: book-id
          schema:
            type: string
          required: true
        - in: query
          name: book-details
          schema:
            type: array
            items:
              type: string
        - in: query
          name: book-author
          schema:
            type: string
      responses:
        "200":
          description: get one book based on item id
          content:
            application/json:
              schema:
                $ref: "./book.yaml#/components/schemas/book"

Complete API spec example along with example client code here as a gist: https://gist.github.com/hxy9243/0378a0601bc329f5c20cc5d8efaf9c6c#file-api-yaml-L80

An example project here with full example server and client code: https://github.com/hxy9243/learn-go-by-examples/tree/main/src/openapi

Generation Details

We used the client code in the actual client code, and try to get the actual request URL and parameters sent to the server.

	book, resp, err := apicli.DefaultApi.GETBook(context.TODO(), books[0].GetId()).
		BookDetails([]string{"book", "details", "example"}).
		BookAuthor("exampleauthor").
		Execute()

	log.Printf("Getting book request: %+v", *resp.Request.URL)

The output that's related to the code snippet above:

...
2023/02/22 21:23:48 Getting book request: {Scheme:http Opaque: User: Host::8000 Path:/books/a09414b2-ed22-42a9-9563-84f38a53b09f RawPath: ForceQuery:false RawQuery:book-author=exampleauthor&book-details=reflect.Value+value&book-details=reflect.Value+value&book-details=reflect.Value+value Fragment: RawFragment:}

where book-details are not using the actual string value but instead showed as reflect.Value in request URL.

In previous versions, the URL looks more correct:

...
2023/02/22 21:46:19 Getting book request: {Scheme:http Opaque: User: Host::8000 Path:/books/78e0c156-96f6-458b-9f86-778f61c7fb60 RawPath: ForceQuery:false RawQuery:book-author=whatever&book-details=book&book-details=details&book-details=example Fragment: RawFragment:}

There's another problem on the server side where the server is only getting one of the parameters instead of three. From the Go-server generated code, it seems it's only querying the first query parameter and expect it to be comma separated. And this is not the default behavior on client side. See example Go server code below:

	params := mux.Vars(r)
	query := r.URL.Query()
	bookIdParam := params["book-id"]

	bookDetailsParam := strings.Split(query.Get("book-details"), ",")
	bookAuthorParam := query.Get("book-author")

And the query.Get function only gets the first parameter as the default behavior of Go's net/url package:

// Get gets the first value associated with the given key.
// If there are no values associated with the key, Get returns
// the empty string. To access multiple values, use the map
// directly.
func (v Values) Get(key string) string {
	if v == nil {
		return ""
	}
	vs := v[key]
	if len(vs) == 0 {
		return ""
	}
	return vs[0]
}

It'll be nice to have both behaviors consistent on server and client, and the generated code can be used out of box without hacks.

Steps to reproduce

Go Server code generation command:

java -jar /home/kevin/downloads/openapi-generator-cli-7.0.0-20221012.083708-4.jar generate -g go-server \
        -i api/api.yaml -o server/gen/ \
        --git-host github.com --git-user-id hxy9243 --git-repo-id go-example \
        --additional-properties=outputAsLibrary=true,sourceFolder=openapi

Go Client code generation command:

java -jar /home/huke/downloads/openapi-generator-cli-7.0.0-20221012.083708-4.jar generate -g go \
        -i api/api.yaml -o client/gen/openapi \
        --git-host github.com --git-user-id hxy9243 --git-repo-id go-example \
        --additional-properties=outputAsLibrary=true,sourceFolder=openapi

See more at gist: https://gist.github.com/hxy9243/0378a0601bc329f5c20cc5d8efaf9c6c#file-api-yaml-L80

Related issues/PRs

Suggest a fix

There are two issues with the code that can be improved:

• As described above, the client query parameters are not generated correctly in the final output.
• The default Go server code is only querying the first parameters and cannot be used out-of-box with the generated client code.

[castelblanque]

Confirmed same problem in version 6.6.0.

[thiagoarrais]

This is fixed for me on master as of now (ba0c73e)

[hxy9243]

This is fixed for me on master as of now (ba0c73e)

What are your steps to verify? Could you paste an example of the generated code? Would love to close this issue.

Thanks.

[hxy9243]

Tested with the latest main, the client code is fixed. The multi-parameter request generation on the client is correct:

2023/02/22 21:46:19 Getting book request: {Scheme:http Opaque: User: Host::8000 Path:/books/78e0c156-96f6-458b-9f86-778f61c7fb60 RawPath: ForceQuery:false RawQuery:book-author=whatever&book-details=book&book-details=details&book-details=example Fragment: RawFragment:}

There's still one issue on the server side. In api_default.go, the go-server code generated for GETBook() is still using query.Get(), which returns only the first parameter:

// GETBook -
func (c *DefaultAPIController) GETBook(w http.ResponseWriter, r *http.Request) {
	params := mux.Vars(r)
	query := r.URL.Query()
        ...
	bookDetailsParam := strings.Split(query.Get("book-details"), ",")
        ...
	result, err := c.service.GETBook(r.Context(), bookIdParam, bookDetailsParam, bookAuthorParam)
...

which assumes that the client parameters are concatenated with comma, while the default behavior on client is the query shown above:

book-author=whatever&book-details=book&book-details=details&book-details=example

A quick fix would be to simply return all the list as []string, where query is of type map[string][]string.

	bookDetailsParam := query["book-details"]

There's a workaround to this, which is to use comma-concatenation on the client side as well. Please let me know if this is worth a fix. Right now I'm happy to close this issue as is.

Thanks.

[thiagoarrais]

Nice breakdown, @hxy9243. My tests really only covered the generated client.

[mytlogos]

This is fixed for me on master as of now (ba0c73e)

What are your steps to verify? Could you paste an example of the generated code? Would love to close this issue.

Thanks.

It should be commit 117e511 which fixed this by unwrapping the reflect.Value (though it would not have been my preferred solution).

I think changing the first lines of the function parameterAddToHeaderOrQuery of the client template to this:

        // previously: var v = reflect.ValueOf(obj)
	var v reflect.Value
	if refValue, ok := obj.(reflect.Value); ok {
		v = refValue
	} else {
		v = reflect.ValueOf(obj)
	}

would be more future proof?

[hxy9243]

It should be commit 117e511 which fixed this by unwrapping the reflect.Value (though it would not have been my preferred solution).

If this is confirmed fixed for now, I'd like to close this issue. For the improvement mentioned above, I'd prefer if you could create another ticket.

@mytlogos Thoughts?

[mytlogos]

I tested it in version 7.0.0 (and manually) and this fixes it.
I created a new issue with #16614
This issue can be closed in my opinion.

[hxy9243]

Sounds good. Thanks all for the effort.