[Java] [Spring] Fix reactive return type for list (#16884)

* Improve Add Async - Spring Cloud test

By testing the generated code.
This will be used to detect regressions.

* Spring: Fix reactive return type for list

Fix for #16883.

When *reactive* is enabled and response entities is *disabled*,
use `Flux<Item>` to stream the output instead of `Mono<Flux<Item>>`
With Spring Reactive, the expected return type for an array of item is
`Flux<Item>`.
Without this patch, the generated code is `Mono<Flux<Item>>`.

This is fixed by introducing specific handling for return types when
reactive is enabled.
In particular, "responseWrapper" is not used anymore in such situations.

* Fix methodBody

* Fix invalid test

* Fix SSE

* Fix methodBody when isArray and useResponseEntity

* methodBody: Flux.empty() instead of s -> {}
This commit is contained in:
Vladimir Svoboda 2024-03-02 14:19:34 +01:00 committed by GitHub
parent 009fda5e3d
commit 5d43c88540
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 72 additions and 18 deletions

View File

@ -669,7 +669,12 @@ public class SpringCodegen extends AbstractJavaCodegen
additionalProperties.put(RESPONSE_WRAPPER, "CompletableFuture"); additionalProperties.put(RESPONSE_WRAPPER, "CompletableFuture");
} }
if (reactive) { if (reactive) {
additionalProperties.put(RESPONSE_WRAPPER, "Mono"); // The response wrapper when Reactive is enabled must depend on the return type:
// Flux<X> when X is an array
// Mono<X> otherwise
// But there are corner cases when also using response entity.
// When reactive is enabled, all this is managed in the mustache templates.
additionalProperties.put(RESPONSE_WRAPPER, "");
} }
// Some well-known Spring or Spring-Cloud response wrappers // Some well-known Spring or Spring-Cloud response wrappers

View File

@ -57,7 +57,7 @@ public interface {{classname}} {
accept = "{{{vendorExtensions.x-accepts}}}"{{/vendorExtensions.x-accepts}}{{#vendorExtensions.x-content-type}}, accept = "{{{vendorExtensions.x-accepts}}}"{{/vendorExtensions.x-accepts}}{{#vendorExtensions.x-content-type}},
contentType = "{{{vendorExtensions.x-content-type}}}"{{/vendorExtensions.x-content-type}} contentType = "{{{vendorExtensions.x-content-type}}}"{{/vendorExtensions.x-content-type}}
) )
{{#responseWrapper}}{{.}}<{{/responseWrapper}}ResponseEntity<{{>returnTypes}}>{{#responseWrapper}}>{{/responseWrapper}} {{operationId}}( {{>responseType}} {{operationId}}(
{{#allParams}}{{>queryParams}}{{>pathParams}}{{>headerParams}}{{>bodyParams}}{{>formParams}}{{>cookieParams}}{{^-last}}, {{#allParams}}{{>queryParams}}{{>pathParams}}{{>headerParams}}{{>bodyParams}}{{>formParams}}{{>cookieParams}}{{^-last}},
{{/-last}}{{/allParams}} {{/-last}}{{/allParams}}
){{#unhandledException}} throws Exception{{/unhandledException}}; ){{#unhandledException}} throws Exception{{/unhandledException}};

View File

@ -50,7 +50,7 @@ Mono<Void> result = Mono.empty();
{{^examples}} {{^examples}}
exchange.getResponse().setStatusCode({{#returnSuccessCode}}HttpStatus.OK{{/returnSuccessCode}}{{^returnSuccessCode}}HttpStatus.NOT_IMPLEMENTED{{/returnSuccessCode}}); exchange.getResponse().setStatusCode({{#returnSuccessCode}}HttpStatus.OK{{/returnSuccessCode}}{{^returnSuccessCode}}HttpStatus.NOT_IMPLEMENTED{{/returnSuccessCode}});
{{/examples}} {{/examples}}
return result{{#allParams}}{{#isBodyParam}}{{^isArray}}{{#paramName}}.then({{.}}){{/paramName}}{{/isArray}}{{#isArray}}{{#paramName}}.thenMany({{.}}){{/paramName}}{{/isArray}}{{/isBodyParam}}{{/allParams}}.then(Mono.empty()); return result{{#allParams}}{{#isBodyParam}}{{^isArray}}{{#paramName}}.then({{.}}){{/paramName}}{{/isArray}}{{#isArray}}{{#paramName}}.thenMany({{.}}){{/paramName}}{{/isArray}}{{/isBodyParam}}{{/allParams}}{{#isArray}}{{#useResponseEntity}}.then(Mono.empty()){{/useResponseEntity}}{{^useResponseEntity}}.thenMany(Flux.empty()){{/useResponseEntity}}{{/isArray}}{{^isArray}}.then(Mono.empty()){{/isArray}};
{{/vendorExtensions.x-sse}} {{/vendorExtensions.x-sse}}
{{#vendorExtensions.x-sse}} {{#vendorExtensions.x-sse}}
exchange.getResponse().setStatusCode({{#returnSuccessCode}}HttpStatus.valueOf({{{statusCode}}}){{/returnSuccessCode}}{{^returnSuccessCode}}HttpStatus.NOT_IMPLEMENTED{{/returnSuccessCode}}); exchange.getResponse().setStatusCode({{#returnSuccessCode}}HttpStatus.valueOf({{{statusCode}}}){{/returnSuccessCode}}{{^returnSuccessCode}}HttpStatus.NOT_IMPLEMENTED{{/returnSuccessCode}});

View File

@ -1 +1 @@
{{^vendorExtensions.x-sse}}{{#responseWrapper}}{{.}}<{{/responseWrapper}}{{#useResponseEntity}}ResponseEntity<{{/useResponseEntity}}{{>returnTypes}}{{#useResponseEntity}}>{{/useResponseEntity}}{{#responseWrapper}}>{{/responseWrapper}}{{/vendorExtensions.x-sse}}{{#vendorExtensions.x-sse}}{{>returnTypes}}{{/vendorExtensions.x-sse}} {{^vendorExtensions.x-sse}}{{#reactive}}{{#useResponseEntity}}Mono<ResponseEntity<{{#isArray}}Flux<{{/isArray}}{{>returnTypes}}{{#isArray}}>{{/isArray}}>>{{/useResponseEntity}}{{^useResponseEntity}}{{#isArray}}Flux{{/isArray}}{{^isArray}}Mono{{/isArray}}<{{>returnTypes}}>{{/useResponseEntity}}{{/reactive}}{{^reactive}}{{#responseWrapper}}{{.}}<{{/responseWrapper}}{{#useResponseEntity}}ResponseEntity<{{/useResponseEntity}}{{>returnTypes}}{{#useResponseEntity}}>{{/useResponseEntity}}{{#responseWrapper}}>{{/responseWrapper}}{{/reactive}}{{/vendorExtensions.x-sse}}{{#vendorExtensions.x-sse}}{{#isArray}}Flux{{/isArray}}{{^isArray}}Mono{{/isArray}}<{{>returnTypes}}>{{/vendorExtensions.x-sse}}

View File

@ -1 +1 @@
{{#isMap}}Map<String, {{{returnType}}}>{{/isMap}}{{#isArray}}{{#reactive}}Flux{{/reactive}}{{^reactive}}{{{returnContainer}}}{{/reactive}}<{{{returnType}}}>{{/isArray}}{{^returnContainer}}{{#useResponseEntity}}{{{returnType}}}{{/useResponseEntity}}{{^useResponseEntity}}{{#isDelegate}}{{#isVoid}}{{#responseWrapper}}{{{returnType}}}{{/responseWrapper}}{{^responseWrapper}}void{{/responseWrapper}}{{/isVoid}}{{^isVoid}}{{{returnType}}}{{/isVoid}}{{/isDelegate}}{{^isDelegate}}{{#async}}{{{returnType}}}{{/async}}{{^async}}{{#isVoid}}{{#responseWrapper}}{{{returnType}}}{{/responseWrapper}}{{^responseWrapper}}void{{/responseWrapper}}{{/isVoid}}{{^isVoid}}{{{returnType}}}{{/isVoid}}{{/async}}{{/isDelegate}}{{/useResponseEntity}}{{/returnContainer}} {{#isMap}}Map<String, {{{returnType}}}>{{/isMap}}{{#isArray}}{{#reactive}}{{{returnType}}}{{/reactive}}{{^reactive}}{{{returnContainer}}}<{{{returnType}}}>{{/reactive}}{{/isArray}}{{^returnContainer}}{{#useResponseEntity}}{{{returnType}}}{{/useResponseEntity}}{{^useResponseEntity}}{{#isDelegate}}{{#isVoid}}{{#responseWrapper}}{{{returnType}}}{{/responseWrapper}}{{^responseWrapper}}void{{/responseWrapper}}{{/isVoid}}{{^isVoid}}{{{returnType}}}{{/isVoid}}{{/isDelegate}}{{^isDelegate}}{{#async}}{{{returnType}}}{{/async}}{{^async}}{{#isVoid}}{{#responseWrapper}}{{{returnType}}}{{/responseWrapper}}{{^responseWrapper}}void{{/responseWrapper}}{{/isVoid}}{{^isVoid}}{{{returnType}}}{{/isVoid}}{{/async}}{{/isDelegate}}{{/useResponseEntity}}{{/returnContainer}}

View File

@ -541,14 +541,19 @@ public class SpringCodegenTest {
} }
@Test @Test
public void springcloudWithAsyncAndJava8HasResponseWrapperCompletableFuture() { public void springcloudWithAsyncAndJava8HasResponseWrapperCompletableFuture() throws IOException {
final SpringCodegen codegen = new SpringCodegen(); Map<String, Object> additionalProperties = new HashMap<>();
codegen.additionalProperties().put(SpringCodegen.ASYNC, true); additionalProperties.put(SpringCodegen.ASYNC, "true");
codegen.additionalProperties().put(CodegenConstants.LIBRARY, "spring-cloud"); additionalProperties.put(CodegenConstants.LIBRARY, "spring-cloud");
codegen.processOpts(); additionalProperties.put(CodegenConstants.MODEL_TESTS, "false");
additionalProperties.put(CodegenConstants.MODEL_DOCS, "false");
additionalProperties.put(CodegenConstants.APIS, "true");
additionalProperties.put(CodegenConstants.SUPPORTING_FILES, "false");
Assert.assertEquals(codegen.additionalProperties().get("jdk8-default-interface"), false); Map<String, File> files = generateFromContract("src/test/resources/3_0/petstore.yaml", SPRING_BOOT, additionalProperties);
Assert.assertEquals(codegen.additionalProperties().get(RESPONSE_WRAPPER), "CompletableFuture");
assertFileContains(files.get("PetApi.java").toPath(), "CompletableFuture<ResponseEntity<Void>> deletePet");
assertFileNotContains(files.get("PetApi.java").toPath(), "default CompletableFuture<ResponseEntity<Void>> deletePet");
} }
@Test @Test
@ -1281,6 +1286,50 @@ public class SpringCodegenTest {
assertFileNotContains(Paths.get(outputPath + "/src/main/java/org/openapitools/api/SomeApiDelegate.java"), "Mono<DummyRequest>"); assertFileNotContains(Paths.get(outputPath + "/src/main/java/org/openapitools/api/SomeApiDelegate.java"), "Mono<DummyRequest>");
} }
@Test
public void reactiveArrayShouldBeWrappedInFluxWithoutMono() throws IOException {
Map<String, Object> additionalProperties = new HashMap<>();
additionalProperties.put(SpringCodegen.DELEGATE_PATTERN, "false");
additionalProperties.put(SpringCodegen.REACTIVE, "true");
additionalProperties.put(SpringCodegen.USE_RESPONSE_ENTITY, "false");
additionalProperties.put(SpringCodegen.USE_SPRING_BOOT3, "true");
additionalProperties.put(CodegenConstants.MODEL_TESTS, "false");
additionalProperties.put(CodegenConstants.MODEL_DOCS, "false");
additionalProperties.put(CodegenConstants.APIS, "true");
additionalProperties.put(CodegenConstants.SUPPORTING_FILES, "false");
Map<String, File> files = generateFromContract("src/test/resources/3_0/petstore.yaml", SPRING_BOOT, additionalProperties);
JavaFileAssert
.assertThat(files.get("PetApi.java"))
.assertMethod("addPet").hasReturnType("Mono<Pet>")
.toFileAssert()
.assertMethod("findPetsByStatus").hasReturnType("Flux<Pet>")
.toFileAssert()
.assertMethod("deletePet").hasReturnType("Mono<Void>");
}
@Test
public void reactiveArrayShouldBeWrappedInMonoFluxWhenUsingResponseEntity() throws IOException {
Map<String, Object> additionalProperties = new HashMap<>();
additionalProperties.put(SpringCodegen.DELEGATE_PATTERN, "false");
additionalProperties.put(SpringCodegen.REACTIVE, "true");
additionalProperties.put(SpringCodegen.USE_RESPONSE_ENTITY, "true");
additionalProperties.put(SpringCodegen.USE_SPRING_BOOT3, "true");
additionalProperties.put(CodegenConstants.MODEL_TESTS, "false");
additionalProperties.put(CodegenConstants.MODEL_DOCS, "false");
additionalProperties.put(CodegenConstants.APIS, "true");
additionalProperties.put(CodegenConstants.SUPPORTING_FILES, "false");
Map<String, File> files = generateFromContract("src/test/resources/3_0/petstore.yaml", SPRING_BOOT, additionalProperties);
JavaFileAssert
.assertThat(files.get("PetApi.java"))
.assertMethod("addPet").hasReturnType("Mono<ResponseEntity<Pet>>")
.toFileAssert()
.assertMethod("findPetsByStatus").hasReturnType("Mono<ResponseEntity<Flux<Pet>>>")
.toFileAssert()
.assertMethod("deletePet").hasReturnType("Mono<ResponseEntity<Void>>");
}
@Test @Test
public void shouldGenerateValidCodeForReactiveControllerWithoutParams_issue14907() throws IOException { public void shouldGenerateValidCodeForReactiveControllerWithoutParams_issue14907() throws IOException {
File output = Files.createTempDirectory("test").toFile().getCanonicalFile(); File output = Files.createTempDirectory("test").toFile().getCanonicalFile();

View File

@ -146,7 +146,7 @@ public interface PetApi {
) )
@ResponseStatus(HttpStatus.OK) @ResponseStatus(HttpStatus.OK)
default Mono<Flux<Pet>> findPetsByStatus( default Flux<Pet> findPetsByStatus(
@NotNull @ApiParam(value = "Status values that need to be considered for filter", required = true, allowableValues = "available, pending, sold") @Valid @RequestParam(value = "status", required = true) List<String> status, @NotNull @ApiParam(value = "Status values that need to be considered for filter", required = true, allowableValues = "available, pending, sold") @Valid @RequestParam(value = "status", required = true) List<String> status,
@ApiIgnore final ServerWebExchange exchange @ApiIgnore final ServerWebExchange exchange
) { ) {
@ -189,7 +189,7 @@ public interface PetApi {
) )
@ResponseStatus(HttpStatus.OK) @ResponseStatus(HttpStatus.OK)
default Mono<Flux<Pet>> findPetsByTags( default Flux<Pet> findPetsByTags(
@NotNull @ApiParam(value = "Tags to filter by", required = true) @Valid @RequestParam(value = "tags", required = true) Set<String> tags, @NotNull @ApiParam(value = "Tags to filter by", required = true) @Valid @RequestParam(value = "tags", required = true) Set<String> tags,
@ApiIgnore final ServerWebExchange exchange @ApiIgnore final ServerWebExchange exchange
) { ) {

View File

@ -76,7 +76,7 @@ public interface PetApiDelegate {
* or Invalid status value (status code 400) * or Invalid status value (status code 400)
* @see PetApi#findPetsByStatus * @see PetApi#findPetsByStatus
*/ */
default Mono<Flux<Pet>> findPetsByStatus(List<String> status, default Flux<Pet> findPetsByStatus(List<String> status,
ServerWebExchange exchange) { ServerWebExchange exchange) {
Mono<Void> result = Mono.empty(); Mono<Void> result = Mono.empty();
exchange.getResponse().setStatusCode(HttpStatus.NOT_IMPLEMENTED); exchange.getResponse().setStatusCode(HttpStatus.NOT_IMPLEMENTED);
@ -92,7 +92,7 @@ public interface PetApiDelegate {
break; break;
} }
} }
return result.then(Mono.empty()); return result.thenMany(Flux.empty());
} }
@ -107,7 +107,7 @@ public interface PetApiDelegate {
* @see PetApi#findPetsByTags * @see PetApi#findPetsByTags
*/ */
@Deprecated @Deprecated
default Mono<Flux<Pet>> findPetsByTags(Set<String> tags, default Flux<Pet> findPetsByTags(Set<String> tags,
ServerWebExchange exchange) { ServerWebExchange exchange) {
Mono<Void> result = Mono.empty(); Mono<Void> result = Mono.empty();
exchange.getResponse().setStatusCode(HttpStatus.NOT_IMPLEMENTED); exchange.getResponse().setStatusCode(HttpStatus.NOT_IMPLEMENTED);
@ -123,7 +123,7 @@ public interface PetApiDelegate {
break; break;
} }
} }
return result.then(Mono.empty()); return result.thenMany(Flux.empty());
} }