-
Notifications
You must be signed in to change notification settings - Fork 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
[Java] allow setting boolean getter (is, has, get) in templates #7344
Conversation
@@ -118,9 +118,16 @@ public class {{classname}} {{#parent}}extends {{{parent}}} {{/parent}}{{#parcela | |||
{{#vendorExtensions.extraAnnotation}} | |||
{{{vendorExtensions.extraAnnotation}}} | |||
{{/vendorExtensions.extraAnnotation}} | |||
{{#isBoolean}} |
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.
@wing328 I don't understand where you could configure to use either getMyBooleanProperty()
or isMyBooleanProperty()
.
Am I missing something?
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.
You can use the -t option with customized templates as follows to have getMyBooleanProperty
:
{{#isBoolean}}
public {{{datatypeWithEnum}}} get{{getter}}() {
return {{name}};
}
{{/isBoolean}}
{{^isBoolean}}
public {{{datatypeWithEnum}}} {{getter}}() {
return {{name}};
}
{{/isBoolean}}
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.
don't you think it would be easier to have a configuration option to choose between the two possibilities?
as a developer I wouldn't want to have to copy and customize a swagger mustache template to simply configure a standard-variant. Customizing requires additional efforts when e.g. upgrading.
Moreover, using is...()
with a non-primitive Boolean
instead of a primitive boolean
is not specified and can thus lead to discussions concerning support in e.g. data mappers like here: https://jira.spring.io/browse/SPR-9482
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.
@cbornet what is your opinion on that?
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.
don't you think it would be easier to have a configuration option to choose between the two possibilities?
Agreed that adding an option is one way but personally I try to avoid too many options as there're already quite a lot of options for Java client generator already.
Besides is
, get
, another possible choice is has
.
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.
@wing328 trying to keep the number of options as small as possible is a good approach. however, as the option in this case would allow using standard-behavior of data mappers, (which used to work before), I consider it worth adding instead of requiring customizations.
Personally, I think it is one of the strengths of swagger codegen to be able to generate standard-code from a swagger-file and directly use it without customizations.
And using the default data mapper of Spring Boot definitely is such a standard-setup.
Since this is standard behavior in Spring Boot I would prefer having an option too. If you don't need it, the additional option will do no harm to users. |
Thanks for the feedback. I'll merge this into master so that we can release v2.3.1. I'll create another task to track the enhancement adding another CLI option (booleanGetterPrefix) for Java generator. William |
PR checklist
./bin/
to update Petstore sample so that CIs can verify the change. (For instance, only need to run./bin/{LANG}-petstore.sh
and./bin/security/{LANG}-petstore.sh
if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in.\bin\windows\
.3.0.0
branch for changes related to OpenAPI spec 3.0. Default:master
.Description of the PR
To fix #7261
If this fix looks good, I'll apply the same change to other mustache templates: