[BUG][GO] Request object parameter accessor methods don't work

[karaatanassov]

Bug Report Checklist

• [ X] 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?
• [X ] 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

ApiRequest methods for setting parameters do not work.

ApiRequest accessor methods MUST use pointer receivers as described in the Go tutorial to modify values in the Request object

https://tour.golang.org/methods/8

ApiRequest accessor methods are declared with value receiver and cannot modify the original request object.

For example with my test spec the api_default.go file has method like this

func (r ApiTestRequest) Test(test Test) ApiTestRequest {
	r.test = &test
	return r
}

Instead we need method like

func (r *ApiTestRequest) Test(test Test) *ApiTestRequest {
	r.test = &test
	return r
}

See go playground tests below

openapi-generator version

openapi-generator-cli 5.0.1
commit : c7fcb39
built : 2021-02-06T09:14:19Z

This line is changed/introduced when go-experimental was promoted to go in v5.0

OpenAPI declaration file content or url

---
openapi: 3.0.0
info:
  description: test
  version: 1.0.0
  title: test

paths:
  /test:
    summary: test
    post:
      operationId: "test"
      requestBody:
        content:
          "application/json":
            schema:
              $ref: '#/components/schemas/test'
      responses:
        '204':
          description: "success"

components:
    schemas:
      test:
        type: object
        properties:
          check:
            type: string

Generation Details

java -jar tools/openapi-generator-cli-5.0.1.jar generate -g go \
    -i openapi/schema.yaml \
    -o oneoftest 

Steps to reproduce

Try to set the test field using the Test request method.

Here is demo how it is not working in go play ground

https://play.golang.org/p/FvDgBM5UKQd

Here is fixed variant

https://play.golang.org/p/aWi7_hxAQdn

Related issues/PRs

#8744

If this bug is fixed I suppose the need to use reflection to access request fields will disappear.

Suggest a fix

The following line has to be changed

openapi-generator/modules/openapi-generator/src/main/resources/go/api.mustache

Line 59 in df5050f

func (r {{#structPrefix}}{{&classname}}{{/structPrefix}}Api{{operationId}}Request) {{vendorExtensions.x-export-param-name}}({{paramName}} {{{dataType}}}) {{#structPrefix}}{{&classname}}{{/structPrefix}}Api{{operationId}}Request {

Current:
func (r {{#structPrefix}}{{&classname}}{{/structPrefix}}Api{{operationId}}Request) {{vendorExtensions.x-export-param-name}}({{paramName}} {{{dataType}}}) {{#structPrefix}}{{&classname}}{{/structPrefix}}Api{{operationId}}Request {

Fixed:
func (r *{{#structPrefix}}{{&classname}}{{/structPrefix}}Api{{operationId}}Request) {{vendorExtensions.x-export-param-name}}({{paramName}} {{{dataType}}}) *{{#structPrefix}}{{&classname}}{{/structPrefix}}Api{{operationId}}Request {

* has to be added before the Request type name to use pointer receiver and return pointer.

[auto-labeler]

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

[wndhydrnt]

Hey @karaatanassov
I was wondering about this behavior myself. I came to the conclusion that the authors want users of the generated code to chain the method calls, like so:

// Other init code omitted.
c := petstore.NewAPIClient(cfg)
pet := petstore.Pet{
    Name: "Oy",
}
// Execute() is called on the modified struct returned by Body().
resp, err := c.PetApi.AddPet(context.TODO()).Body(pet).Execute()
// Do something with resp and err.

Given your example, what needs to be done to make it pass is to re-assign testReq:

testReq = testReq.Test(Test{
    Check: &opt,
})

Hope it helps.

[karaatanassov]

Immutable request objects make a lot of sense. They are safe to share. gRPC/Protobuf uses bindings like this. The go Context is like this.

Thanks for pointing it out.

I am not sure this is achieved given the writable ApiService member.

	ApiService *DefaultApiService

It would be nice to go fully one way or the other.

In the way you are proposing the ApiService and ctx need to have similar methods. ApiService to make it immutable. ctx because the value of sharing objects is also to be able to create a modified copy.

The current code is a bit confusing and frankly worse than either of the options of mutable or immutable requests.