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.

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

[BUG][Go][Bounty] Go client generator doesn't support deepObject in query #11401

Closed
5 of 6 tasks
kamilsk opened this issue Jan 25, 2022 · 18 comments
Closed
5 of 6 tasks

Comments

@kamilsk
Copy link

kamilsk commented Jan 25, 2022

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

Go client generator doesn't support deepObject in query.

openapi-generator version

v5.3.1

OpenAPI declaration file content or url
openapi: 3.0.0

info:
  title: Example
  description: Example of deepObject failure.
  version: 1.0.0

paths:
  /test:
    get:
      operationId: test
      parameters:
        - name: filter
          in: query
          style: deepObject
          explode: true
          schema:
            type: object
            properties:
              search:
                description: Filter.
                type: string
      responses:
        '200':
          description: Response.
Generation Details
$ docker run --rm -it \
    -v "$(pwd)"/api.yaml:/specs/api.yaml \
    openapitools/openapi-generator-cli:v5.3.1 generate -i /specs/api.yaml -g go -o /sdk/go
Steps to reproduce
  1. store example in api.yaml
  2. run example from "generation details"

expected result: exit code 0 and successful generated content

actual result:

  Exception: null
        at org.openapitools.codegen.DefaultGenerator.processOperation(DefaultGenerator.java:1175)
        at org.openapitools.codegen.DefaultGenerator.processPaths(DefaultGenerator.java:1066)
        at org.openapitools.codegen.DefaultGenerator.generateApis(DefaultGenerator.java:566)
        at org.openapitools.codegen.DefaultGenerator.generate(DefaultGenerator.java:907)
        at org.openapitools.codegen.cmd.Generate.execute(Generate.java:441)
        at org.openapitools.codegen.cmd.OpenApiGeneratorCommand.run(OpenApiGeneratorCommand.java:32)
        at org.openapitools.codegen.OpenAPIGenerator.main(OpenAPIGenerator.java:66)
Caused by: java.lang.NullPointerException
Related issues/PRs
Suggest a fix
@gfyrag
Copy link

gfyrag commented May 6, 2022

Same with v5.4.0 and latest.

@Christina0114
Copy link

I meet the similar problem in the path parameters. I guess it is because of this line
CodegenProperty property = fromProperty(entry.getKey(), entry.getValue(), requiredVarNames.contains(entry.getKey())); property.baseName = codegenParameter.baseName + "[" + entry.getKey() + "]"; return property;
in DefaultCodegen Class. It updates the key property.baseName in the schemaCodegenPropertyCache map.

@altitude
Copy link

altitude commented Oct 4, 2022

Similar issue on our end, we just added a $250 bounty on this one: https://app.bountysource.com/issues/112016967-bug-go-go-client-generator-doesn-t-support-deepobject-in-query

@KN4CK3R
Copy link

KN4CK3R commented Oct 4, 2022

The given example works fine with 6.2.0. Do you have another example which does not work?

@kamilsk
Copy link
Author

kamilsk commented Oct 5, 2022

I will check it and maybe will find another example. Thanks for the comment.

@kamilsk
Copy link
Author

kamilsk commented Oct 5, 2022

test result

version result
v5.3.1 failure
v5.4.0 failure
v6.0.0 success

Output is valid

...
	if r.filter != nil {
		localVarQueryParams.Add("filter", parameterToString(*r.filter, ""))
	}
...


// TestFilterParameter struct for TestFilterParameter
type TestFilterParameter struct {
	// Filter.
	Search *string `json:"search,omitempty"`
}

but logic is wrong

func parameterToString(obj interface{}, collectionFormat string) string {
	var delimiter string

	switch collectionFormat {
	case "pipes":
		delimiter = "|"
	case "ssv":
		delimiter = " "
	case "tsv":
		delimiter = "\t"
	case "csv":
		delimiter = ","
	}

	if reflect.TypeOf(obj).Kind() == reflect.Slice {
		return strings.Trim(strings.Replace(fmt.Sprint(obj), " ", delimiter, -1), "[]")
	} else if t, ok := obj.(time.Time); ok {
		return t.Format(time.RFC3339)
	}

	return fmt.Sprintf("%v", obj)
}

it makes an API call with something like this "/test?filter=%7B0x14000098dc0%7D", because TestFilterParameter implements only json.Marshaler and doesn't implement Stringer/GoStringer

package main

import "fmt"

type TestFilterParameter struct {
	// Filter.
	Search *string `json:"search,omitempty"`
}

func (t TestFilterParameter) GoString() string {
	return "yop"
}

func (t TestFilterParameter) String() string {
	return "yep"
}

func main() {
	s := "query"
	obj := TestFilterParameter{&s}
	fmt.Printf("%v\n", obj)
	fmt.Printf("%#v\n", obj)
}

@kamilsk
Copy link
Author

kamilsk commented Oct 5, 2022

Full example

package main

import (
	"context"
	"fmt"
	"testme/openapi"
)

type TestFilterParameter struct {
	// Filter.
	Search *string `json:"search,omitempty"`
}

func (t TestFilterParameter) GoString() string {
	return "yop"
}

func (t TestFilterParameter) String() string {
	return "yep"
}

func main() {
	q := "query"
	obj := TestFilterParameter{&q}
	fmt.Printf("%v\n", obj)
	fmt.Printf("%#v\n", obj)

	ctx := context.Background()
	cl := openapi.NewAPIClient(openapi.NewConfiguration())

	req := cl.DefaultApi.Test(ctx)
	req = req.Filter(openapi.TestFilterParameter{Search: &q})

	resp, err := cl.DefaultApi.TestExecute(req)
	fmt.Println(resp, err)
}

Expected call /test?filter[search]=query, not /test?filter=%7B0x14000098dc0%7D

@wing328 wing328 changed the title [BUG][Go] Go client generator doesn't support deepObject in query [BUG][Go][Bounty] Go client generator doesn't support deepObject in query Oct 5, 2022
parvit added a commit to parvit/openapi-generator that referenced this issue Oct 8, 2022
@parvit
Copy link
Contributor

parvit commented Oct 8, 2022

Hi all, i've created a PR for this feature, please review it.

@parvit
Copy link
Contributor

parvit commented Oct 11, 2022

Any feedback on this?

@exitflynn
Copy link

is this issue still open?

@parvit
Copy link
Contributor

parvit commented Oct 30, 2022 via email

@kamilsk kamilsk closed this as not planned Won't fix, can't repro, duplicate, stale Oct 31, 2022
@parvit
Copy link
Contributor

parvit commented Oct 31, 2022

@kamilsk @altitude Are you going to approve the completion of the bounty then? Seems unfair that i've completed the work and the ticket gets closed without merging.

@kamilsk
Copy link
Author

kamilsk commented Oct 31, 2022

@parvit I know nothing about the bounty. I closed it because I no longer wait for it and use a different stack. Feel free to reopen the issue, but I will unsubscribe from it.

@kamilsk kamilsk reopened this Oct 31, 2022
parvit added a commit to parvit/openapi-generator that referenced this issue Nov 4, 2022
wing328 pushed a commit that referenced this issue Nov 20, 2022
…ecification (#13909)

* issue #11401 - Go client generator doesn't support deepObject in query

* samples generation

* fix generation

* fix generation

* generated samples

# Conflicts:
#	samples/client/petstore/go/go-petstore/model_200_response.go
#	samples/client/petstore/go/go-petstore/model_additional_properties_any_type.go
#	samples/client/petstore/go/go-petstore/model_client.go

* Fixed unit tests

* revert to http connection for tests

* fix model_simple generation

* Fix parameter encoding issue

* simplified routine

* fix test url

* adapted for latest master, necessary generation

* samples generation

* sync with new master, regenerate samples

* added api client test
@wing328
Copy link
Member

wing328 commented Nov 20, 2022

Fixed via #13909. Thanks @parvit for the PR.

@altitude Can you please pull the latest master to give it a try and send @parvit the bug bounty if the fix is good? Thanks.

@wing328 wing328 closed this as completed Nov 20, 2022
@gfyrag
Copy link

gfyrag commented Nov 21, 2022

@wing328 Is the code on the latest docker image?

@gfyrag
Copy link

gfyrag commented Nov 24, 2022

@wing328 Tested today with the latest image (built 16 hours ago), and same results on my end.

@wing328
Copy link
Member

wing328 commented Nov 29, 2022

do you mind sharing the spec so that we can use it to reproduce the issue?

@gfyrag
Copy link

gfyrag commented Nov 29, 2022

@wing328 Retested from scratch with latest image, and it works as expected. Don't know why it was failing few days ago. Maybe a problem between the keyboard and the chair.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants