Skip to content

[Go] report correctly the parameters with the deep object specification#13909

Merged
wing328 merged 16 commits intoOpenAPITools:masterfrom
parvit:issue-11401-newmaster
Nov 20, 2022
Merged

[Go] report correctly the parameters with the deep object specification#13909
wing328 merged 16 commits intoOpenAPITools:masterfrom
parvit:issue-11401-newmaster

Conversation

@parvit
Copy link
Copy Markdown
Contributor

@parvit parvit commented Nov 4, 2022

Hi all,
this change addresses the issue mentioned in #11401 and it should report correctly the parameters with the deep object specification.

Commits cherry-picked from previous branch to be based on current master

parvit added 11 commits November 4, 2022 07:20
# 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
@parvit
Copy link
Copy Markdown
Contributor Author

parvit commented Nov 4, 2022

@wing328 Here's the update PR you requested.

@wing328
Copy link
Copy Markdown
Member

wing328 commented Nov 4, 2022

can you please update the samples ?

https://github.com/OpenAPITools/openapi-generator/actions/runs/3391716534/jobs/5637208273

also I believe you've tested it locally, right?

Can you post the test result as well?

@parvit
Copy link
Copy Markdown
Contributor Author

parvit commented Nov 4, 2022

I'll need to update the mustache files first as now they don't generate correct code anymore.

@wing328
Copy link
Copy Markdown
Member

wing328 commented Nov 4, 2022

take your time. just let me know when it's ready. i'll review over the weekend or next week.

have a nice weekend.

@parvit
Copy link
Copy Markdown
Contributor Author

parvit commented Nov 4, 2022

Just a quick question:
image

Could you indicate how to make the "time" import generate in "model_one_of_primitive_types.go" ?

@wing328
Copy link
Copy Markdown
Member

wing328 commented Nov 4, 2022

again please make sure this PR is created from the latest master.

@parvit
Copy link
Copy Markdown
Contributor Author

parvit commented Nov 5, 2022

@wing328 master branch has the same issue, i've created a PR for that fix.

EDIT: Retesting from clean state i was not able to reproduce that issue, maybe it was just a local change that was throwing me off course

@parvit
Copy link
Copy Markdown
Contributor Author

parvit commented Nov 14, 2022

@wing328 Were you able to check the change?

@wing328
Copy link
Copy Markdown
Member

wing328 commented Nov 18, 2022

can you please resolve the merge conflicts when you've time?

I'll try to test again over the weekend.

@parvit
Copy link
Copy Markdown
Contributor Author

parvit commented Nov 18, 2022 via email

# Conflicts:
#	modules/openapi-generator/src/main/resources/go/model_simple.mustache
#	modules/openapi-generator/src/main/resources/go/utils.mustache
#	samples/client/petstore/go/go-petstore/model_animal.go
#	samples/client/petstore/go/go-petstore/model_category.go
#	samples/client/petstore/go/go-petstore/model_enum_test_.go
#	samples/client/petstore/go/go-petstore/model_format_test_.go
#	samples/client/petstore/go/go-petstore/model_name.go
#	samples/client/petstore/go/go-petstore/model_pet.go
#	samples/client/petstore/go/go-petstore/model_type_holder_default.go
#	samples/client/petstore/go/go-petstore/model_type_holder_example.go
#	samples/client/petstore/go/go-petstore/utils.go
#	samples/openapi3/client/extensions/x-auth-id-alias/go-experimental/utils.go
#	samples/openapi3/client/petstore/go/go-petstore/model_animal.go
#	samples/openapi3/client/petstore/go/go-petstore/model_apple_req.go
#	samples/openapi3/client/petstore/go/go-petstore/model_banana_req.go
#	samples/openapi3/client/petstore/go/go-petstore/model_category.go
#	samples/openapi3/client/petstore/go/go-petstore/model_duplicated_prop_parent.go
#	samples/openapi3/client/petstore/go/go-petstore/model_enum_test_.go
#	samples/openapi3/client/petstore/go/go-petstore/model_format_test_.go
#	samples/openapi3/client/petstore/go/go-petstore/model_name.go
#	samples/openapi3/client/petstore/go/go-petstore/model_pet.go
#	samples/openapi3/client/petstore/go/go-petstore/model_whale.go
#	samples/openapi3/client/petstore/go/go-petstore/model_zebra.go
#	samples/openapi3/client/petstore/go/go-petstore/utils.go
@parvit
Copy link
Copy Markdown
Contributor Author

parvit commented Nov 19, 2022

@wing328 the update is done

@wing328 wing328 added this to the 6.3.0 milestone Nov 20, 2022
@wing328
Copy link
Copy Markdown
Member

wing328 commented Nov 20, 2022

I did a test and got the following in tcpdump:

10:05:53.342677 IP localhost.52060 > localhost.http: Flags [P.], seq 3718:4248, ack 6277, win 6281, options [nop,nop,TS val 3234621852 ecr 4289403119], length 530: HTTP: GET /v2/fake/deep_object_test?inputOptions[F1]=1&inputOptions[F2]=teststring&inputOptions[F3]=null&inputOptions[id]=1&inputOptions[name]=TestCat&test_pet[F1]=1&test_pet[F2]=teststring&test_pet[F3]=null&test_pet[id]=1&test_pet[name]=Test&test_pet[photoUrls]=http%3A%2F%2Flocalhost&test_pet[tags][F1]=1&test_pet[tags][F2]=teststring&test_pet[tags][F3]=null&test_pet[tags][id]=2&test_pet[tags][name]=tag1 HTTP/1.1

What are those F1, F2, F3 ?

Here is a test I added to pet_api_test.go

// test deep object query parameter and verify via tcpdump
func TestDeepObjectQuery(t *testing.T) {
	newPet := (sw.Pet{Id: sw.PtrInt64(12830), Name: "gopher",
		PhotoUrls: []string{"http://1.com", "http://2.com"}, Status: sw.PtrString("pending"),
		Tags: []sw.Tag{{Id: sw.PtrInt64(1), Name: sw.PtrString("tag2")}}})
	newCategory := (sw.Category{Id: sw.PtrInt64(12830), Name: "cat"})
	configuration := sw.NewConfiguration()
	apiClient := sw.NewAPIClient(configuration)
	r, err := apiClient.FakeApi.TestQueryDeepObject(context.Background()).TestPet(newPet).InputOptions(newCategory).Execute()
	if err != nil {
		// for sure this will fail as the endpoint is fake
	}
	if r.StatusCode != 200 {
		t.Log(r)
	}
}

Can you please also include it in your PR so that we've a test ready for testing next time?

Thanks again for the PR.

@parvit
Copy link
Copy Markdown
Contributor Author

parvit commented Nov 20, 2022

The names come from the data in the AdditionalProperties map in the test i already made in fake_api_test.go, because i wanted to stress the deep reflection path of the code to confirm it works.

func TestQueryDeepObject(t *testing.T) {
	req := client.FakeApi.TestQueryDeepObject(context.Background())

	var id = int64(1)
	var idTag1 = int64(2)
	var nameTag1 = "tag1"
	req = req.TestPet(sw.Pet{
		Id: &id,
		Name: "Test",
		PhotoUrls: []string{ "http://localhost" },
		Tags: []sw.Tag{
			{
				Id: &idTag1,
				Name: &nameTag1,
				AdditionalProperties: map[string]interface{}{
					"F1": 1,
					"F2": "teststring",
					"F3": nil,
				},
			},
		},
		AdditionalProperties: map[string]interface{}{
			"F1": 1,
			"F2": "teststring",
			"F3": nil,
		},
	})
	var idcat = int64(1)
	req = req.InputOptions(sw.Category{
		Id: &idcat,
		Name: "TestCat",
		AdditionalProperties: map[string]interface{}{
			"F1": 1,
			"F2": "teststring",
			"F3": nil,
		},
	})

	r, _ := req.Execute()

	var expectedDeepObjectURL = testScheme + "://" + testHost + deepObjectURL

	assert.Equal(t,
		expectedDeepObjectURL,
		r.Request.URL.String() )
}

@wing328
Copy link
Copy Markdown
Member

wing328 commented Nov 20, 2022

Thanks for the explanation 👍 . I can confirm in my end the result is good.

@wing328 wing328 merged commit 4487042 into OpenAPITools:master Nov 20, 2022
@parvit parvit deleted the issue-11401-newmaster branch November 20, 2022 14:34
@wing328 wing328 changed the title Issue 11401 - report correctly the parameters with the deep object specification [Go] report correctly the parameters with the deep object specification Dec 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants