Skip to content

[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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/generators/java-microprofile.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,9 @@ These options may be applied as additional-properties (cli) or configOptions (pl
|licenseName|The name of the license| |Unlicense|
|licenseUrl|The URL of the license| |http://unlicense.org|
|microprofileFramework|Framework for microprofile. Possible values "kumuluzee"| |null|
|microprofileGlobalExceptionMapper|Should ApiExceptionMapper be annotated with @Provider making it a global exception mapper| |true|
|microprofileMutiny|Whether to use async types for microprofile (currently only Smallrye Mutiny is supported).| |null|
|microprofileRegisterExceptionMapper|Should generated API Clients be annotated with @RegisterProvider(ApiExceptionMapper.class).| |true|
|microprofileRestClientVersion|Version of MicroProfile Rest Client API.| |null|
|modelPackage|package for generated models| |org.openapitools.client.model|
|openApiNullable|Enable OpenAPI Jackson Nullable library. Not supported by `microprofile` library.| |true|
Expand Down
2 changes: 2 additions & 0 deletions docs/generators/java.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,9 @@ These options may be applied as additional-properties (cli) or configOptions (pl
|licenseName|The name of the license| |Unlicense|
|licenseUrl|The URL of the license| |http://unlicense.org|
|microprofileFramework|Framework for microprofile. Possible values "kumuluzee"| |null|
|microprofileGlobalExceptionMapper|Should ApiExceptionMapper be annotated with @Provider making it a global exception mapper| |true|
|microprofileMutiny|Whether to use async types for microprofile (currently only Smallrye Mutiny is supported).| |null|
|microprofileRegisterExceptionMapper|Should generated API Clients be annotated with @RegisterProvider(ApiExceptionMapper.class).| |true|
|microprofileRestClientVersion|Version of MicroProfile Rest Client API.| |null|
|modelPackage|package for generated models| |org.openapitools.client.model|
|openApiNullable|Enable OpenAPI Jackson Nullable library. Not supported by `microprofile` library.| |true|
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -375,6 +381,17 @@ public void processOpts() {
}
convertPropertyToStringAndWriteBack(MICROPROFILE_FRAMEWORK, this::setMicroprofileFramework);

convertPropertyToBooleanAndWriteBack(MICROPROFILE_GLOBAL_EXCEPTION_MAPPER, this::setMicroProfileGlobalExceptionMapper);
convertPropertyToBooleanAndWriteBack(MICROPROFILE_REGISTER_EXCEPTION_MAPPER, this::setMicroProfileRegisterExceptionMapper);

if (!additionalProperties.containsKey(MICROPROFILE_REGISTER_EXCEPTION_MAPPER)) {
additionalProperties.put(MICROPROFILE_REGISTER_EXCEPTION_MAPPER, true);
}

if (!additionalProperties.containsKey(MICROPROFILE_GLOBAL_EXCEPTION_MAPPER)) {
additionalProperties.put(MICROPROFILE_GLOBAL_EXCEPTION_MAPPER, true);
}

Copy link
Member

Choose a reason for hiding this comment

The 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:

        convertPropertyToBooleanAndWriteBack(MICROPROFILE_MUTINY, this::setMicroprofileMutiny);

        convertPropertyToStringAndWriteBack(MICROPROFILE_REST_CLIENT_VERSION, value->microprofileRestClientVersion=value);

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?

Copy link
Author

Choose a reason for hiding this comment

The 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

Copy link
Member

Choose a reason for hiding this comment

The 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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tested with the following:

java -jar modules/openapi-generator-cli/target/openapi-generator-cli.jar generate -g java --library microprofile -i modules/openapi-generator/src/test/resources/3_0/petstore.yaml -o /tmp/microprofile2/ --additional-properties microProfileGlobalExceptionMapper=false,microProfileRegisterExceptionMapper=false

but didn't notice anything change in the output compared with no additional properties

do these 2 options work for you locally?

Copy link
Author

Choose a reason for hiding this comment

The 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

java -jar modules/openapi-generator-cli/target/openapi-generator-cli.jar generate -g java --library microprofile -i modules/openapi-generator/src/test/resources/3_0/petstore.yaml -o /tmp/microprofile2/ --additional-properties microprofileGlobalExceptionMapper=false,microprofileRegisterExceptionMapper=false

Copy link
Author

Choose a reason for hiding this comment

The 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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ import {{rootJavaEEPackage}}.validation.constraints.*;
import {{rootJavaEEPackage}}.validation.Valid;
{{/useBeanValidation}}

{{#microprofileRegisterExceptionMapper}}
import org.eclipse.microprofile.rest.client.annotation.RegisterProvider;
{{/microprofileRegisterExceptionMapper}}
import org.eclipse.microprofile.rest.client.inject.RegisterRestClient;

{{#appName}}
Expand All @@ -41,7 +43,9 @@ import org.eclipse.microprofile.rest.client.inject.RegisterRestClient;
{{^microprofileServer}}
@RegisterRestClient{{#configKey}}(configKey="{{configKey}}"){{/configKey}}{{#configKeyFromClassName}}{{#operations}}(configKey="{{configKey}}"){{/operations}}{{/configKeyFromClassName}}
{{/microprofileServer}}
{{#microprofileRegisterExceptionMapper}}
@RegisterProvider(ApiExceptionMapper.class)
{{/microprofileRegisterExceptionMapper}}
@Path("{{#useAnnotatedBasePath}}{{contextPath}}{{/useAnnotatedBasePath}}{{commonPath}}")
public interface {{classname}} {
{{#operations}}
Expand Down
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems removing the @Provider annotation makes the generated class useless. If you agree, could you please make it so that microprofileGlobalExceptionMapper controls if this class is generated at all?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without the @Provider annotation, the mapper does nothing on its own, but you can still register it on RestClients using @RegisterProvider(ApiExceptionMapper.class)

I don't know if that's enough to keep the mapper?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Let's keep it then.

Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,14 @@ package {{apiPackage}};

import {{rootJavaEEPackage}}.ws.rs.core.MultivaluedMap;
import {{rootJavaEEPackage}}.ws.rs.core.Response;
{{#microprofileGlobalExceptionMapper}}
import {{rootJavaEEPackage}}.ws.rs.ext.Provider;
{{/microprofileGlobalExceptionMapper}}
import org.eclipse.microprofile.rest.client.ext.ResponseExceptionMapper;

{{#microprofileGlobalExceptionMapper}}
@Provider
{{/microprofileGlobalExceptionMapper}}
public class ApiExceptionMapper
implements ResponseExceptionMapper<ApiException> {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import org.openapitools.codegen.CodegenConstants;
import org.openapitools.codegen.DefaultGenerator;
import org.openapitools.codegen.java.assertions.JavaFileAssert;
import org.openapitools.codegen.languages.JavaClientCodegen;
import org.openapitools.codegen.languages.JavaMicroprofileServerCodegen;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;
Expand Down Expand Up @@ -127,4 +128,112 @@ public void testMicroprofileCanHandleCookieParamsSingleRequest() throws Exceptio
.assertInnerClass("GetCustomerRequest")
.assertMethod("cookieParameter");
}
}

@Test
public void testGeneratedApiHasApiExceptionMapperRegisteredWhenUsingDefaultConfiguration() throws Exception {
File output = Files.createTempDirectory("test").toFile().getCanonicalFile();
output.deleteOnExit();

OpenAPI openAPI = new OpenAPIParser()
.readLocation("src/test/resources/bugs/microprofile_cookie.yaml", null, new ParseOptions()).getOpenAPI();

codegen.setOutputDir(output.getAbsolutePath());

ClientOptInput input = new ClientOptInput()
.openAPI(openAPI)
.config(codegen);

List<File> files = new DefaultGenerator().opts(input).generate();

Map<String, File> filesMap = files.stream()
.collect(Collectors.toMap(File::getName, Function.identity()));

validateJavaSourceFiles(files);

JavaFileAssert.assertThat(filesMap.get("DefaultApi.java"))
.assertTypeAnnotations()
.containsWithName("RegisterProvider")
.containsWithNameAndAttributes("RegisterProvider", Map.of("value", "ApiExceptionMapper.class"));
}

@Test
public void testGeneratedApiDoesNotHaveApiExceptionMapperRegisteredWhenDisablingItInConfiguration() throws Exception {
File output = Files.createTempDirectory("test").toFile().getCanonicalFile();
output.deleteOnExit();

OpenAPI openAPI = new OpenAPIParser()
.readLocation("src/test/resources/bugs/microprofile_cookie.yaml", null, new ParseOptions()).getOpenAPI();

codegen.setOutputDir(output.getAbsolutePath());
codegen.additionalProperties().put(JavaClientCodegen.MICROPROFILE_REGISTER_EXCEPTION_MAPPER, "false");

ClientOptInput input = new ClientOptInput()
.openAPI(openAPI)
.config(codegen);

List<File> files = new DefaultGenerator().opts(input).generate();

Map<String, File> filesMap = files.stream()
.collect(Collectors.toMap(File::getName, Function.identity()));

validateJavaSourceFiles(files);

JavaFileAssert.assertThat(filesMap.get("DefaultApi.java"))
.assertTypeAnnotations()
.doesNotContainWithName("RegisterProvider");
}

@Test
public void testGeneratedApiExceptionMapperHasProviderAnnotationWhenUsingDefaultConfiguration() throws Exception {
File output = Files.createTempDirectory("test").toFile().getCanonicalFile();
output.deleteOnExit();

OpenAPI openAPI = new OpenAPIParser()
.readLocation("src/test/resources/bugs/microprofile_cookie.yaml", null, new ParseOptions()).getOpenAPI();

codegen.setOutputDir(output.getAbsolutePath());

ClientOptInput input = new ClientOptInput()
.openAPI(openAPI)
.config(codegen);

List<File> files = new DefaultGenerator().opts(input).generate();

Map<String, File> filesMap = files.stream()
.collect(Collectors.toMap(File::getName, Function.identity()));

validateJavaSourceFiles(files);

JavaFileAssert.assertThat(filesMap.get("ApiExceptionMapper.java"))
.assertTypeAnnotations()
.containsWithName("Provider");
}

@Test
public void testGeneratedApiExceptionMapperDoesNotHaveProviderAnnotationWhenDisablingItInConfiguration() throws Exception {
File output = Files.createTempDirectory("test").toFile().getCanonicalFile();
output.deleteOnExit();

OpenAPI openAPI = new OpenAPIParser()
.readLocation("src/test/resources/bugs/microprofile_cookie.yaml", null, new ParseOptions()).getOpenAPI();

codegen.setOutputDir(output.getAbsolutePath());
codegen.additionalProperties().put(JavaClientCodegen.MICROPROFILE_GLOBAL_EXCEPTION_MAPPER, "false");

ClientOptInput input = new ClientOptInput()
.openAPI(openAPI)
.config(codegen);

List<File> files = new DefaultGenerator().opts(input).generate();

Map<String, File> filesMap = files.stream()
.collect(Collectors.toMap(File::getName, Function.identity()));

validateJavaSourceFiles(files);

JavaFileAssert.assertThat(filesMap.get("ApiExceptionMapper.java"))
.assertTypeAnnotations()
.doesNotContainWithName("Provider");
}
}

Loading