-
-
Notifications
You must be signed in to change notification settings - Fork 7.3k
[FEAT] Support nullable Array<org.springframework.web.multipart.MultipartFile> in Kotlin-Spring generator
#21994
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
[FEAT] Support nullable Array<org.springframework.web.multipart.MultipartFile> in Kotlin-Spring generator
#21994
Conversation
org.springframework.web.multipart.MultipartFile in Kotlin-Spring generatorArray<org.springframework.web.multipart.MultipartFile> in Kotlin-Spring generator
not an expert in kotlin, given that required is already set to false, what's the difference between including and not including ? at the end as |
|
Hi @wing328, First of all, thanks for your reply and excuse me for my response delay :s Kotlin has a Null Safety feature in core (https://kotlinlang.org/docs/null-safety.html) Let's take the previous example and let me illustrate with curl commands. Regarding the example, I'm expecting that the For me, a fix should be to take the fact that the To illustrate: Example 1: should lead to which means :
And Example 2: should lead to which means :
If I use a cURL command: in example 1, this leads to a "java.lang.NullPointerException: Parameter specified as non-null is null " And that's why I'm a bit confused by the 2 keywords When I looked at the optionalDataType.mustache file, I saw that However, the aim of my fix is to correct the fact that a I don't know if it helps but tell me ;) Regards, Jérémy |
|
ok. not sure if it helps, the following mustache template files for kotlin-client contain modules/openapi-generator/src/main/resources/kotlin-client/model_room.mustache would these help understand how |
|
I think it helps :) In our case, I'll change the PR by replacing
Thanks ;) |
|
https://github.com/OpenAPITools/openapi-generator/actions/runs/18093579898/job/51946861968?pr=21994 please update the samples when you've time |
| @@ -1 +1 @@ | |||
| {{^isFile}}{{{dataType}}}{{^required}}{{^defaultValue}}?{{/defaultValue}}{{/required}}{{/isFile}}{{#isFile}}{{#isArray}}Array<{{/isArray}}org.springframework.web.multipart.MultipartFile{{#isArray}}>{{/isArray}}{{^isArray}}{{^required}}?{{/required}}{{/isArray}}{{/isFile}} No newline at end of file | |||
| {{^isFile}}{{{dataType}}}{{^required}}{{^defaultValue}}?{{/defaultValue}}{{/required}}{{/isFile}}{{#isFile}}{{#isArray}}Array<{{/isArray}}org.springframework.web.multipart.MultipartFile{{#isArray}}>{{/isArray}}{{#isNullable}}?{{/isNullable}}{{/isFile}} No newline at end of file | |||
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.
what about optional schema in which isNullable is set to false? should it also generate the ? at the end to make it nullable in kotlin?
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.
What do you mean exactly?
Something equivalent to Optional<T> in Java?
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.
I mean a schema (in OpenAPI spec) that is non required and nullable (3.0.x) spec) is set to true
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.
If both are managed by placeholders required and isNullable that's ok so ;)
If something (class, property,...) has nullable set to true, the type should have a ?
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.
i'll file another pr to show i mean by optional after merging yours
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.
I've filed a draft PR: #22176
please take a look to see if the change makes sense to you.
… Kotlin Spring generator - nullable is only supported for MultipartFile. However, Array<MultipartFile> could be also nullable
… Kotlin Spring generator
… Kotlin Spring generator Update kotlin-spring-additionalproperties samples
17420ec to
4511762
Compare
In kotlin-spring generator, for endpoints dealing with MultipartFile, we found out that it was not possible to declare
Array<MultipartFile>property as "nullable".Our use case consist on having a POST endpoint (multipart/form-data) that deals with 2 parts:
Basically, something like this:
As you can see,
filesis not marked asrequired.What we expected from the kotlin-spring generator, for the associated controller was:
but it generated
However, the
?was correctly added to File propertyIn this PR, we have adapted the
optionalDataType.mustachetemplate in order to put the?even for Array.We've also added some tests in file
org.openapitools.codegen.kotlin.spring.KotlinSpringServerCodegenTestHowever, we're a bit confused by
nullableandrequiredusage along the whole generator.In this particular use case, we could imagine that:
false)?Do you have some guidelines about when and where use these 2 keywords ?
Thanks for your handsome work !
@karismann (2019/03) @Zomzog (2019/04) @andrewemery (2019/10) @4brunu (2019/11) @yutaka0m (2020/03) @stefankoppier (2022/06) @e5l (2024/10)
PR checklist
Commit all changed files.
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*.IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
master(upcoming7.x.0minor release - breaking changes with fallbacks),8.0.x(breaking changes without fallbacks)"fixes #123"present in the PR description)