openapi-generator
openapi-generator copied to clipboard
issue #11401 - Go client generator doesn't support deepObject in query
Hi all, this change addresses the issue mentioned in #11401 and it should report correctly the parameters with the deep object specification.
Please review and indicate me if there's anything more i should address.
cc @antihax (2017/11) @grokify (2018/07) @kemokemo (2018/09) @jirikuncar (2021/01) @ph4r5h4d (2021/04)
Can you please update the samples by running ./bin/generate-samples.sh
(using gitbash if you're on Windows) so that the CI can test the change ?
sure, in a moment
EDIT: i'm fixing the issue with samples
i seem to have missed the functionality based on header parameters, i'll add it and push soon
Now everything should be in place, i've also regenerated the samples and the tests produced fail in the same way as on master.
@wing328 could you please check again the change again? thanks.
CI failed:
go version go1.14 linux/amd64
go: found github.com/stretchr/testify/assert in github.com/stretchr/testify v1.8.0
go: github.com/stretchr/testify upgrade => v1.8.0
go: golang.org/x/oauth2 upgrade => v0.0.0-20221014153046-6fdb5e3db783
go: downloading golang.org/x/oauth2 v0.0.0-20221014153046-6fdb5e3db783
go: downloading golang.org/x/net v0.0.0-20221014081412-f15817d10f9b
go: found golang.org/x/net/context in golang.org/x/net v0.0.0-20221014081412-f15817d10f9b
go: golang.org/x/net upgrade => v0.0.0-20221014081412-f15817d10f9b
=== RUN TestOAuth2
TestOAuth2: auth_test.go:56: Error while deleting pet by id: 404 Not Found
--- FAIL: TestOAuth2 (0.23s)
=== RUN TestBasicAuth
TestBasicAuth: auth_test.go:91: Error while deleting pet by id: 404 Not Found
--- FAIL: TestBasicAuth (0.01s)
=== RUN TestAccessToken
TestAccessToken: auth_test.go:121: Error while deleting pet by id: 404 Not Found
--- FAIL: TestAccessToken (0.01s)
=== RUN TestAPIKeyNoPrefix
TestAPIKeyNoPrefix: auth_test.go:150: Error while deleting pet by id: 404 Not Found
--- FAIL: TestAPIKeyNoPrefix (0.00s)
=== RUN TestAPIKeyWithPrefix
TestAPIKeyWithPrefix: auth_test.go:185: Error while deleting pet by id: 404 Not Found
--- FAIL: TestAPIKeyWithPrefix (0.00s)
=== RUN TestDefaultHeader
TestDefaultHeader: auth_test.go:219: Error while deleting pet by id: 404 Not Found
--- FAIL: TestDefaultHeader (0.00s)
=== RUN TestHostOverride
--- PASS: TestHostOverride (0.01s)
=== RUN TestSchemeOverride
--- PASS: TestSchemeOverride (0.01s)
=== RUN TestPutBodyWithFileSchema
--- PASS: TestPutBodyWithFileSchema (0.00s)
=== RUN TestAddPet
--- PASS: TestAddPet (0.00s)
=== RUN TestAddPetMock
--- PASS: TestAddPetMock (0.00s)
=== RUN TestFindPetsByStatusWithMissingParam
--- PASS: TestFindPetsByStatusWithMissingParam (0.00s)
=== RUN TestGetPetById
TestGetPetById: pet_api_test.go:291: Error while getting pet by id: 404 Not Found
--- FAIL: TestGetPetById (0.00s)
=== RUN TestGetPetByIdWithInvalidID
--- PASS: TestGetPetByIdWithInvalidID (0.00s)
=== RUN TestUpdatePetWithForm
TestUpdatePetWithForm: pet_api_test.go:89: Error while updating pet by id: 404 Not Found
--- FAIL: TestUpdatePetWithForm (0.00s)
=== RUN TestFindPetsByTag
TestFindPetsByTag: pet_api_test.go:108: Error no pets returned
TestFindPetsByTag: pet_api_test.go:121: Error while getting pet by tag could not find 12830
--- FAIL: TestFindPetsByTag (0.00s)
=== RUN TestFindPetsByStatus
TestFindPetsByStatus: pet_api_test.go:137: Error no pets returned
--- FAIL: TestFindPetsByStatus (0.00s)
=== RUN TestUploadFile
TestUploadFile: pet_api_test.go:160: Error while uploading file: 404 Not Found
--- FAIL: TestUploadFile (0.01s)
=== RUN TestUploadFileRequired
--- PASS: TestUploadFileRequired (0.00s)
=== RUN TestDeletePet
TestDeletePet: pet_api_test.go:190: Error while deleting pet by id: 404 Not Found
--- FAIL: TestDeletePet (0.00s)
=== RUN TestPlaceOrder
TestPlaceOrder: store_api_test.go:29: Skipping error for parsing time with `+0000` UTC offset as Petstore Test Server does not return valid RFC 3339 datetime
--- PASS: TestPlaceOrder (0.02s)
=== RUN TestCreateUser
--- PASS: TestCreateUser (0.01s)
=== RUN TestCreateUsersWithArrayInput
--- PASS: TestCreateUsersWithArrayInput (0.01s)
=== RUN TestGetUserByName
TestGetUserByName: user_api_test.go:86: Error while getting user by id: 404 Not Found
--- FAIL: TestGetUserByName (0.00s)
=== RUN TestGetUserByNameWithInvalidID
--- PASS: TestGetUserByNameWithInvalidID (0.00s)
=== RUN TestUpdateUser
TestUpdateUser: user_api_test.go:137: Error while getting user by id: 404 Not Found
--- FAIL: TestUpdateUser (0.00s)
FAIL
exit status 1
FAIL github.com/OpenAPITools/openapi-generator/samples/client/petstore/go 0.339s
[ERROR] Failed to execute goal org.codehaus.mojo:exec-maven-plugin:1.2.1:exec (go-test) on project GoPetstore: Command execution failed.: Process exited with an error: 1 (Exit value: 1) -> [Help 1]
[ERROR]
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
ref: https://app.circleci.com/pipelines/github/OpenAPITools/openapi-generator/20107/workflows/e33d6565-687e-4ca1-a786-bda17bdd7ac1/jobs/51746
For some reason using the "http" schema in the tests was broken and would only return error 405, changing it to "https" runs correct.
Then there were some actual issues in the code that previous result had covered, now locally the tests are green.
You will need to run the petstore server locally: https://github.com/OpenAPITools/openapi-generator/wiki/Integration-Tests
You will need to run the petstore server locally: https://github.com/OpenAPITools/openapi-generator/wiki/Integration-Tests
Ok, however the tests already pass on CI so i think we can go ahead
@wing328 Probably i'm missing something, could you please indicate what exactly is missing to merge the issue? thanks.
@altitude I wonder if you can also test/review this change as you've a bug bounty on https://github.com/OpenAPITools/openapi-generator/issues/11401. Thanks.
No update on this?
@altitude @wing328 Hi all, i'd really like to close this, is there anything i can do to accelerate the process? thanks.
@parvit I did some tests locally with a new endpoint in petstore spec as follows:
/fake/deep_object_test:
get:
tags:
- fake
operationId: test_query_deep_object
parameters:
- name: test_pet
in: query
required: false
style: deepObject
schema:
$ref: '#/components/schemas/Pet'
explode: true
- name: inputOptions
in: query
required: false
style: deepObject
schema:
$ref: '#/components/schemas/Category'
explode: true
responses:
'200':
description: OK
but here is the result with tcpdump:
14:48:08.484748 IP localhost.52322 > localhost.http: Flags [P.], seq 10209:10701, ack 7291, win 6265, options [nop,nop,TS val 477774952 ecr 729668614], length 492: HTTP: GET /v2/fake/deep_object_test?inputOptions%5Bid%5D=0xc000018590&inputOptions%5Bname%5D=gopher+world&test_pet%5Bid%5D=0xc000018580&test_pet%5Bname%5D=hello+gopher&test_pet%5BphotoUrls%5D%5B0%5D=http%3A%2F%2F1.com&test_pet%5BphotoUrls%5D%5B1%5D=http%3A%2F%2F2.com&test_pet%5Bstatus%5D=0xc000210a30&test_pet%5Btags%5D%5B0%5D=%7B0xc000018588+0xc000210a40+map%5B%5D%7D HTTP/1.1
I don't think that's the correct result.
Does the fix work for you locally?
The deepObject format in and of itself seems to be working when you decode the URL:
What would you expect in place of the value of "inputOptions[id]=0xc000018590" for example?
The deepObject format in and of itself seems to be working when you decode the URL:
I don't think the users expect these square brackets to be encoded in the URL.
Please refer to the following SO for more info:
https://stackoverflow.com/questions/56279024/openapi-parameter-with-brackets-and-variable-name https://stackoverflow.com/questions/46004257/swagger-openapi-spec-for-arrays-of-objects-in-url-query-parameter https://stackoverflow.com/questions/48491688/how-to-define-parameters-with-square-brackets-in-openapi-swagger
https://github.com/OpenAPITools/openapi-generator/issues/11401#issuecomment-1268147781 also contains an example.
Expected call /test?filter[search]=query, not /test?filter=%7B0x14000098dc0%7D
Ok i'll see if i can change it to not being encoded.
Hi, i've managed to implement leaving the square brackets without escaping, also i've integrated your specification in a test that you can find in fake_api_test.go
But when i've pulled the master this error has come up related to changes to the workflow from master:
Is it something we can safely ignore or is it related to the code somehow? thanks
@parvit looks like the PR is messed up with 4000+ file changes.
Can you please file a new one based on the latest master by cherry-picking the commits?
There were merge conflicts with the master so i've pulled it and resolved them on the branch, but if you want to solve them manually i'll make the PR without it.
totally ok that you try to resolve the merged conflicts, but if you review the files changed in this PR, many unrelated-changes are included.
If you review https://github.com/OpenAPITools/openapi-generator/pull/13643/commits/d0703afd72c6a84be8468e584fa849771018d166, you will find lots of changes unrelated to your PR.
Yeah when you pull master it tends to happen to pull unrelated changes, however now i'm cherrypicking the changes on a new branch.
Il giorno ven 4 nov 2022 alle ore 07:16 William Cheng < @.***> ha scritto:
totally ok that you try to resolve the merged conflicts, but if you review the files changed in this PR, many unrelated-changes are included.
— Reply to this email directly, view it on GitHub https://github.com/OpenAPITools/openapi-generator/pull/13643#issuecomment-1303024852, or unsubscribe https://github.com/notifications/unsubscribe-auth/AWGJD4AMRHOIARUFGBON4E3WGSS37ANCNFSM6AAAAAARAF5QR4 . You are receiving this because you modified the open/close state.Message ID: @.***>
-- Vittorio Parrella, Software Developer