Add reactor support for publisher to DeferredResult and to SseEmitter. #71
Conversation
DeepCode Report (#d34c33)DeepCode analyzed this pull request. |
| * Utility class providing operations to convert Reactor reactive types into asynchronous Spring 4.x | ||
| * web types. | ||
| */ | ||
| @SuppressWarnings("NoFunctionalReturnType") |
There was a problem hiding this comment.
This is needed here as the Functions returned here are the native java.util.function.Functions which are functional interfaces as opposed to the io.reactivex.functions.Function.
| try { | ||
| sseEmitter.send(o, mediaType); | ||
| } catch (IOException e) { | ||
| throw new UncheckedIOException(e); |
There was a problem hiding this comment.
This exception must be explicitly caught here, where we don't need to do this in the RxSpring4Util equivalent, because this accepts a java util Function rather than the rx Function class
| @SuppressWarnings("NullAway") | ||
| private MockMvc mockMvc; | ||
|
|
||
| @BeforeEach |
There was a problem hiding this comment.
This is different than the rx test setup in the sense that we do this each time. Changing this to @BeforeAll means the scheduler is re-used between test runs and gives unexpected results. Most notably setting the time to 0 (closest thing to a reset) can also trigger responses sent
There was a problem hiding this comment.
Indeed, in Reactor VirtualTimeScheduler is global. This produces particularly weird results when multiple tests are running simultaneously in different parts of the build. Good that it can be mitigated in this test class but good too keep an eye out for unstable builds if we introduce it in other tests in this project.
|
|
||
| @GetMapping("/publisherToDeferredResult") | ||
| public DeferredResult<ImmutableList<String>> withPublisherToDeferredResult( | ||
| @RequestParam String value, @RequestParam(defaultValue = "0") int repeat) { |
There was a problem hiding this comment.
The default value of repeat here is 0 instead of 1. The repeat value in RxJava indicates how often things are emitted, so setting it to 0 means no emissions at all. In reactor this is really a replay, so setting it to 0 means things are emitted once.
| @GetMapping("/publisherToDeferredResult") | ||
| public DeferredResult<ImmutableList<String>> withPublisherToDeferredResult( | ||
| @RequestParam String value, @RequestParam(defaultValue = "0") int repeat) { | ||
| return Flux.defer(() -> Mono.fromCallable(defer(value))) |
There was a problem hiding this comment.
Slightly different with the defer method due to the fact that exceptions are not thrown for the java util Function as opposed to the rx Function
| @@ -81,6 +82,7 @@ | |||
| <version.jdk>1.8</version.jdk> | |||
| <version.maven>3.5.3</version.maven> | |||
| <version.nullaway>0.7.2</version.nullaway> | |||
| <version.reactor>3.2.11.RELEASE</version.reactor> | |||
There was a problem hiding this comment.
I think we should move this to https://github.com/PicnicSupermarket/oss-parent and use it as the parent?
| } | ||
|
|
||
| /** | ||
| * Provides a function that subscribes to a {@link Publisher} and exposes its emissions as {@link |
There was a problem hiding this comment.
In this part of the documentation we are interleaving link references to {@link Publisher} and plain old English function. Should we be consistent and always refer to the type {@link Function} or just use plain old English?
| @SuppressWarnings("NullAway") | ||
| private MockMvc mockMvc; | ||
|
|
||
| @BeforeEach |
There was a problem hiding this comment.
Indeed, in Reactor VirtualTimeScheduler is global. This produces particularly weird results when multiple tests are running simultaneously in different parts of the build. Good that it can be mitigated in this test class but good too keep an eye out for unstable builds if we introduce it in other tests in this project.
There are a couple of subtle differences between this implementation and the
RxSpring4Utilclass and test, which I will point out with some comments.