We've been looking at the generated Spring Web Reactive interfaces and the response type wrappings seem superfluous, this is using version openapi-generator:3.3.1. For an API that returns a collection of type T, we get a Java interface with the following response signature
Mono<ResponseEntity<Flux<T>>>
From all of our testing this behaves identically to Flux<T> from a reactive client's perspective. The additional Mono<ResponseEntity> adds additional code to each controller to add the type wrappings.
Since the ServerWebExchange is already present and gives all the control over the HTTP session needed. It seems the generated code could be simplified
The suggestion is that an option could be added to generate the reactive interfaces w/o the wrapping Mono<ResponseEntity>
openapi-generator:3.3.1
In that case, that should be done for both reactive and non-reactive.
In non-reactive you also have the possibility to add headers, change the status code etc... by injecting the request-scoped NativeWebRequest.
Any update on this? The signatures are barely usable as-is.
I don`t like the ResponseEntity on the interface as well... there should be an option to remove it.
I confirm the issue.
I am using openapi-generator-maven-plugin 4.3.0 with
<generatorName>spring</generatorName>
<library>spring-boot</library>
<configOptions>
<reactive>true</reactive>
<java8>true</java8>
<interfaceOnly>true</interfaceOnly>
<skipDefaultInterface>true</skipDefaultInterface>
</configOptions>
and the generator creates methods that return Mono<ResponseEntity<Flux<T>>>
It should be just Flux<T> and Mono<T>.
any workaround for spring reactive? I'm experiencing the same issues
I understood from https://github.com/OpenAPITools/openapi-generator/pull/571 that the "inconvenience" is not only the complex signature of return type, but the fact that ResponseEntity.status() need to be set upfront before fetching data which can be not always possible if the status depends on the data on the stream. Is that right?
Aside of this, is it a good practice for an API to return directly an array [] like Flux<Foo> instead of an object containing the array like { someSequence: [] } like Mono<MyRespDto>?
I'm asking because I found problems only with the first approach because of type information being lost on polymorphic collections (plus other inconvenients like the response not being extensible/flexible: you can't extend an array with more properties). If I wrap it in a top level object (second approach), the generated type changes from Flux<Foo> to List<Foo> inside MyRespDto.
I question myself if the generated response model in case 2 should be like this (which is not the case):
public class MyRespDto {
private Flux<Foo> someSequence; // <-- instead of List<Foo>
// getters, setters,...
}
EDIT:
I apologize because I'm probably messing concepts when asking about private Flux<Foo> someSequence;.
But, looking at https://github.com/Playtika/feign-reactive/issues/209 discussion, and from the arguments of @kptfh , the current signature that includes Mono<ResponseEntity<MyDto>> does not follow the Reacctive approach. And quoting (notice that when he/she says "Spring team", actually refers to "openapi-generator":
Mono
>> allows you to subscribe on response, read headers and drop body if needed. In your case body will be always deserialized.
Mono>> allows to subscribe to result and process flux of elements. And if you need omit rest of elements at some points. In your case all elements will be deserialized. I think that spring should fix their implementation to support ResponseEntity > .
And what I understand for the above statement, you could even make a request, grab some info from response like the response headers and completely discard the body, in a way that the body would not be serialized (and I understand not sent over the wire). Maybe I'm too newbie on this, but what @kptfh tells about having this signature, seems to make complete sense whilst being really reactive: Mono<ResponseEntity<Mono/Flux<R>>>.
And following this issue, when supressing the ResponseEntity from the equation as this issue sugests, the first 2 layers of generics would be gone, I mean Mono<ResponseEntity<. So what I say only applies for the current case where ResponseEntity is there, hence maybe it should be reported as a separate issue.
Any thughts @cbornet , @jrobison153 , @gmcouto ?
@gerardbosch excellent explanation!
@gerardbosch I think what you're talking about is the code generated for the spring client using Webclient. This discussion is about the server side (Webflux) where we already generate with Mono<ResponseEntity<Flux<T>>>. And the request is to generate only Flux<T> with a RestController which is something supported by Spring.
Hello @cbornet , nope, I'm talking about the server side-Webflux generator :)
As per mi testing, the generated code is Mono<ResponseEntity<T>> and not Mono<ResponseEntity<Mono/Flux<T>> (tested with the 4.3.1 and 5.0.0-beta).
I've linked above the feign-reactive discussion where the difference is aparently explained as @gmcouto also showed in that thread.
Of course removing the ResponseEntity fixes the reactive problem as the response would then be just Mono/Flux<T> being fully reactive; so I'm talking about change Mono<ResponseEntity<T>> to Mono<ResponseEntity<Mono/Flux<T>> in server generated code for the current generation where ResponseEntity is there yet ^_^.
So there's a regression as it used to work.
Hi @cbornet I have tried with 3.3.4, 4.0.0, 4.0.2, 4.0.3, 4.1.0, 4.1.1, 4.1.2, 4.1.3, 4.2.0, 4.2.1, 4.2.2, 4.2.3, 4.3.0, 4.3.1 and 5.0.0-beta of the openapi-generator-maven-plugin. The generated code is Mono<ResponseEntity<T>> in all cases, instead of Mono<ResponseEntity<Mono/Flux<T>>. Cheers!
hi
i see the ongoing discussion, i solved it by changing the Mustache template locally on my project as a workaround
i can share it if you need
Hi @cbornet I have tried with 3.3.4, 4.0.0, 4.0.2, 4.0.3, 4.1.0, 4.1.1, 4.1.2, 4.1.3, 4.2.0, 4.2.1, 4.2.2, 4.2.3, 4.3.0, 4.3.1 and 5.0.0-beta of the openapi-generator-maven-plugin. The generated code is Mono
> in all cases, instead of Mono >. Cheers!
I just generated the petstore with v4.3.1 and got
default Mono<ResponseEntity<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, ServerWebExchange exchange) {
return getDelegate().findPetsByStatus(status, exchange);
}
Also the description of this issue says that what is generated is Mono<ResponseEntity<Flux<T>>>.
@gerardbosch can you share your openapi-generator-maven-plugin config and an OAS reproducing your issue ? Also can you open another issue for this as it's not the same as this one ?
please see the example in my GitHub gist
i'm using gradle plugin so you have to reference those files for the open-api plugin
see example
```openApiGenerate {
generatorName = "spring"
inputSpec = "$projectDir/openApi/service.yaml" // your yaml file lcoation
outputDir = "$generateServerLocation"
apiPackage = "xyz.api.generated"
invokerPackage = xyz.generated.invokerd"
modelPackage = "cxyz.model"
templateDir = "$projectDir/openApi/templates" // the dir that includes all files from my gist
...
}
```
I see what happens @cbornet , I've tried with Petstore and the generated code is of type:
Mono<ResponseEntity<Flux<Pet>>> findPetsByStatus(...)
for those responses declared of type: array in the OAS definition file.
But of type:
Mono<ResponseEntity<Pet>> addPet(...)
for those responses of type: object, instead of being Mono<ResponseEntity<Mono<Pet>>> addPet(...) as @kptfh explains in https://github.com/Playtika/feign-reactive/issues/209 to let the API be fully reactive (notice the additional Mono<Pet>).
I can open a separate issue if you think is better to tackle this. Cheers!
@gerardbosch oh, I understand now : you mean the single item case ! Mono<ResponseEntity<Mono<>>> is indeed more reactive as you can send the status code and headers before getting the body. But in reality does it really happen ? I mean, if you have an error while fetching the information to fill the body, you will want to send a 500, not a 200.
It also makes the signature more complex so I'm not sure the people in this thread will be happy :smiley:
Note that the situation is not the same on the client side : it can be interesting to read the status code and just ignore the body.
Ok, I understand your point. Maybe the solution to all pain is to remove the ResponseEntity, letting:
Flux<Pet> findPetsByStatus(@NotNull @Valid @RequestParam(value = "status", required = true) List<String> status, ServerWebExchange exchange);
Mono<Pet> addPet(@Valid @RequestBody Mono<Pet> pet, ServerWebExchange exchange); // <-- Reactive response body (Mono)
Much simpler signatures, fully-reactive response bodies for both object and array return types.
This will also conform to the reactive-feign for those interested in using it.
I'm never writing APIs returning array because I found problems in Java losing type info for polymorphic collections (List<Animal>, List<Cat>, List<Dog>). And it is not possible to extend an array whilst it is possible to extend an object; but I guess many can benefit from Flux<T> streaming.
Ok, I understand your point. Maybe the solution to all pain is to remove the
ResponseEntity, letting:Flux<Pet> findPetsByStatus(@NotNull @Valid @RequestParam(value = "status", required = true) List<String> status, ServerWebExchange exchange); Mono<Pet> addPet(@Valid @RequestBody Mono<Pet> pet, ServerWebExchange exchange); // <-- Reactive response body (Mono)Much simpler signatures, fully-reactive response bodies for both
objectandarrayreturn types.
This will also conform to the reactive-feign for those interested in using it.I'm never writing APIs returning
arraybecause I found problems in Java losing type info for polymorphic collections (List<Animal>,List<Cat>,List<Dog>). And it is not possible to extend an array whilst it is possible to extend an object; but I guess many can benefit fromFlux<T>streaming.
the solution i added above does remove the ResponseEntity and create the simple signature with simple Mono/Flux
Thanks @erohana , it would be nice to see it in a pull request :)
Most helpful comment
Thanks @erohana , it would be nice to see it in a pull request :)