-
-
Notifications
You must be signed in to change notification settings - Fork 6.6k
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] [client] [java] [native] [csharp-netcore] Multi use of schema params within deepobjects #13662
[BUG] [client] [java] [native] [csharp-netcore] Multi use of schema params within deepobjects #13662
Conversation
@Reinhard-PTV thanks for the PR. Can you please PM me via Slack when you've time? https://join.slack.com/t/openapi-generator/shared_invite/zt-12jxxd7p2-XUeQM~4pzsU9x~eGLQqX2g |
@@ -5077,7 +5077,6 @@ public CodegenParameter fromParameter(Parameter parameter, Set<String> imports) | |||
properties.entrySet().stream() | |||
.map(entry -> { | |||
CodegenProperty property = fromProperty(entry.getKey(), entry.getValue(), requiredVarNames.contains(entry.getKey())); | |||
property.baseName = codegenParameter.baseName + "[" + entry.getKey() + "]"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI @OpenAPITools/generator-core-team change to baseName has been rolled back.
I did a test with java (native) client but got the following:
I added the following to petstore spec:
Does it work for you locally? |
I couldn't reproduce your result locally. https://petstore.swagger.io:443/v2/fake/deep_object_test?testPet[id]=123&testPet[name]=Hello World&inputOptions[id]=456&inputOptions[name]=new category This looks ok for me. Another problem is the category object. This does not work, because nested object does not work in deep objects. See :
https://swagger.io/docs/specification/serialization/ This is our generated code for the query parameters:
We generated our code with |
I did another test using Pet as the
Right. That's another issue. #13909 (for Go client) can be served as a reference |
@Reinhard-PTV I think we'll accept this test for the time being as it reverts changes to |
cc @bbdouglas (2017/07) @sreeshas (2017/08) @jfiala (2017/08) @lukoyanov (2017/09) @cbornet (2017/09) @jeff9finger (2018/01) @karismann (2019/03) @Zomzog (2019/04) @lwlee2608 (2019/10) cc @mandrean (2017/08) @frankyjuang (2019/09) @shibayan (2020/02) @Blackclaws (2021/03) @lucamazzanti (2021/05) |
FYI. I've filed #14142 to add a test to cover the baseName change in the future. |
@Reinhard-PTV FYI. I've merged #14378 to support deepObject in java native client. Please pull the latest to give it a try when you've time. |
@wing328, sorry for the delay. The 6.3.0-SNAPHOT version does not work for our API. I will build a small example to show our problem. |
@wing328 Before (for example with version 6.0) Now with version 6.3.0 (there is a [0]) |
@mandrean (2017/08) @frankyjuang (2019/09) @shibayan (2020/02) @Blackclaws (2021/03) @lucamazzanti (2021/05) @wing328
If a schema is used multiple times as a parameter with different namesin deepObjects, then the parameters have all the same name.
Fixes Issue: #13658
PR checklist
This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
These must match the expectations made by your contribution.
You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example
./bin/generate-samples.sh bin/configs/java*
.For Windows users, please run the script in Git BASH.
master
(6.1.0) (minor release - breaking changes with fallbacks),7.0.x
(breaking changes without fallbacks)