-
-
Notifications
You must be signed in to change notification settings - Fork 6.9k
[java][Microprofile] add config options to disable usage of ApiExceptionMapper #20762
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -70,6 +70,8 @@ public class JavaClientCodegen extends AbstractJavaCodegen | |
public static final String CASE_INSENSITIVE_RESPONSE_HEADERS = "caseInsensitiveResponseHeaders"; | ||
public static final String MICROPROFILE_FRAMEWORK = "microprofileFramework"; | ||
public static final String MICROPROFILE_MUTINY = "microprofileMutiny"; | ||
public static final String MICROPROFILE_GLOBAL_EXCEPTION_MAPPER = "microprofileGlobalExceptionMapper"; | ||
public static final String MICROPROFILE_REGISTER_EXCEPTION_MAPPER = "microprofileRegisterExceptionMapper"; | ||
public static final String USE_ABSTRACTION_FOR_FILES = "useAbstractionForFiles"; | ||
public static final String DYNAMIC_OPERATIONS = "dynamicOperations"; | ||
public static final String SUPPORT_STREAMING = "supportStreaming"; | ||
|
@@ -118,6 +120,8 @@ public class JavaClientCodegen extends AbstractJavaCodegen | |
@Setter protected String microprofileFramework = MICROPROFILE_DEFAULT; | ||
@Setter protected String microprofileRestClientVersion = MICROPROFILE_REST_CLIENT_DEFAULT_VERSION; | ||
@Setter protected boolean microprofileMutiny = false; | ||
@Setter protected boolean microProfileGlobalExceptionMapper = true; | ||
@Setter protected boolean microProfileRegisterExceptionMapper = true; | ||
@Setter protected String configKey = null; | ||
@Setter(AccessLevel.PRIVATE) protected boolean configKeyFromClassName = false; | ||
|
||
|
@@ -226,6 +230,8 @@ public JavaClientCodegen() { | |
cliOptions.add(CliOption.newBoolean(CASE_INSENSITIVE_RESPONSE_HEADERS, "Make API response's headers case-insensitive. Available on " + OKHTTP_GSON + ", " + JERSEY2 + " libraries")); | ||
cliOptions.add(CliOption.newString(MICROPROFILE_FRAMEWORK, "Framework for microprofile. Possible values \"kumuluzee\"")); | ||
cliOptions.add(CliOption.newString(MICROPROFILE_MUTINY, "Whether to use async types for microprofile (currently only Smallrye Mutiny is supported).")); | ||
cliOptions.add(CliOption.newString(MICROPROFILE_REGISTER_EXCEPTION_MAPPER, "Should generated API Clients be annotated with @RegisterProvider(ApiExceptionMapper.class).").defaultValue("true")); | ||
cliOptions.add(CliOption.newString(MICROPROFILE_GLOBAL_EXCEPTION_MAPPER, "Should ApiExceptionMapper be annotated with @Provider making it a global exception mapper").defaultValue("true")); | ||
cliOptions.add(CliOption.newBoolean(USE_ABSTRACTION_FOR_FILES, "Use alternative types instead of java.io.File to allow passing bytes without a file on disk. Available on resttemplate, webclient, restclient, libraries")); | ||
cliOptions.add(CliOption.newBoolean(DYNAMIC_OPERATIONS, "Generate operations dynamically at runtime from an OAS", this.dynamicOperations)); | ||
cliOptions.add(CliOption.newBoolean(SUPPORT_STREAMING, "Support streaming endpoint (beta)", this.supportStreaming)); | ||
|
@@ -375,6 +381,12 @@ public void processOpts() { | |
} | ||
convertPropertyToStringAndWriteBack(MICROPROFILE_FRAMEWORK, this::setMicroprofileFramework); | ||
|
||
convertPropertyToBooleanAndWriteBack(MICROPROFILE_GLOBAL_EXCEPTION_MAPPER, this::setMicroProfileGlobalExceptionMapper); | ||
convertPropertyToBooleanAndWriteBack(MICROPROFILE_REGISTER_EXCEPTION_MAPPER, this::setMicroProfileRegisterExceptionMapper); | ||
|
||
additionalProperties.put(MICROPROFILE_REGISTER_EXCEPTION_MAPPER, microProfileRegisterExceptionMapper); | ||
additionalProperties.put(MICROPROFILE_GLOBAL_EXCEPTION_MAPPER, microProfileGlobalExceptionMapper); | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i think you will need to follow how the rest of microprofile options are set:
as my test shows setting these options to false have no effect in the output did you test the options locally by setting them to false? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @wing328 Yes, you were right. I changed the code and tested the cli locally. It works as expected now There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 i'll run some tests later this week and merge if no more question from me or the community There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. tested with the following:
but didn't notice anything change in the output compared with no additional properties do these 2 options work for you locally? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe it is because of a capitalization error in the input. I just tested, and it works as expected with a lowercase p in microprofile. This works
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @wing328 Can you take another look? |
||
convertPropertyToBooleanAndWriteBack(MICROPROFILE_MUTINY, this::setMicroprofileMutiny); | ||
|
||
convertPropertyToStringAndWriteBack(MICROPROFILE_REST_CLIENT_VERSION, value->microprofileRestClientVersion=value); | ||
|
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.
are these 2 lines still needed? don't see these 2 lines for other options.
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 believe so. If these lines are removed the parameters default to false
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 was curious and checked. @JacobOJ is right and these lines are needed - because in contrast to most other options, here the default is
true
instead offalse
. Not setting anything implicitly meansfalse
, which matches the intended default in most cases, but not here. Just for future reference :)