Ondrej,
It’s an interesting observation, and certainly makes sense on paper. But the are details that make this difficult to realize in practice. RxInvoker<T> is a parametric type whose get() method returns T; AsyncInvoker’s get() returns Future<Response>.
It may be easier to support AsyncInvoker in the new scheme that the other way around because of backward compatibility.
— Santiago
> On Jan 17, 2017, at 9:11 AM, Ondrej Mihályi <ondrej.mihalyi_at_gmail.com> wrote:
>
> I agree with Sergey here that Rx is mostly a sugar around the AsyncInvoker's methods that accept InvocationCallback.
>
> In fact, for a while I was thinking: Why we just don't add new methods to the AsyncInvoker instead of creating all the fuss around RxInvoker? If we blend it with the RxCompletionStage which I suggested in another thread, it would be simple, easy to understand and use. I find it hard to justify having another rx() method next to the async() method, unless the AsyncInvoker cannot be extended due to breaking backwards compatibility (I don't think so).
>
> In the end, reactive is just async with callback, very often providing something like CompletionStage to avoid callback hell. On top of it, the callback can be executed multiple times for stream of events, but I don't see a reason why a REST client call would emit more events for a single request.
>
> I understand that Jersey decided to provide a separate RxInvoker to avoid interfering with the current standard AsyncInvoker interface. But within a standard, we can afford to add methods to AsyncInvoker interface, or not?
>
> Just a food for thought, to realize if we didn't stray too much from the original intentions...
>
> Ondrej
>
> 2017-01-17 14:24 GMT+01:00 Sergey Beryozkin <sberyozkin_at_talend.com <mailto:sberyozkin_at_talend.com>>:
> Sure, I see how it works, as far as having the sync call staged as part of the reactive chain.
> To be honest this is not how I thought about Rx initially.
>
> For example, the Jersey blog I referred to earlier (and not only the Jersey one) have referred to Rx as a remedy against the InvocationCallback
> noise which is obviously seen only with .async(). That is why I started implementing with the initial assumption that for users Rx is = an easier Async, easier composable, no callbacks, etc.
>
> While you are now saying that well, it is all about running the sync action on the executor thread.
> Isn't it what Async effectively also about ?
>
> I'd fine with having it all simplified, the simpler the better, and settle down at SyncInvoker providing a method call action for the CompletionStage/etc.
>
> But I wonder won't we miss something if we do it ? Is running Async calls totally orthogonal to the idea of Rx ? I.e, will our users ask us one day, why exactly I can't stage the async calls as part of my RX flows ?
>
>
>
> Cheers, Sergey
>
> On 17/01/17 11:20, Pavel Bucek wrote:
>> Just a thought:
>>
>>
>> Why would we need to have access to Invocation.Builder#async() for creating CompletionStage (or others?). Isnt that (for now) about running the (synchronous!) action on an executor service (container or explicitly specified)? Async would create just another thread from that thread.
>>
>>
>> Does it make sense? :-)
>>
>>
>> I was looking at following code when I was thinking about the issue you raised:
>>
>>
>> https://github.com/jersey/jersey/blob/2.26-b01/core-client/src/main/java/org/glassfish/jersey/client/JerseyCompletionStageRxInvoker.java#L65 <https://github.com/jersey/jersey/blob/2.26-b01/core-client/src/main/java/org/glassfish/jersey/client/JerseyCompletionStageRxInvoker.java#L65>
>>
>> Please let me know what you think about that and if you reached same / different conclusion.
>>
>> Thanks!
>>
>> Pavel
>>
>> On 16/01/2017 19:10, Sergey Beryozkin wrote:
>>> Hi Pavel
>>>
>>> I'm not sure at the moment.
>>> In CXF, say, a CompletionStage invoker, only works on the async HTTP transport. So we do supply a Supplier and the async thread will wait inside this Supplier till the result is avail from the async transport thread.
>>> Expecting RxInvoker implementations will work with the SyncInvoker alone to support them may not always work...
>>> I glanced earlier at JerseyCompletionStageRxInvoker and looks like an HTTP invocation over the sync transport is .supplyAsync-ed.
>>> But as I said we do it over the async transport only - may be it is not needed, but may be it is ?
>>> I wonder if some new abstraction may need to be introduced.
>>>
>>> Cheers, Sergey
>>>
>>>
>>> On 16/01/17 17:27, Pavel Bucek wrote:
>>>> Hi Sergey,
>>>>
>>>> good catch!
>>>>
>>>> Would it be enough to change the param to SyncInvoker? (Invocation.Builder already extends that, so it would be very simple change).
>>>>
>>>> Thanks,
>>>> Pavel
>>>>
>>>> On 16/01/2017 17:55, Sergey Beryozkin wrote:
>>>>> Hi Pavel
>>>>>
>>>>> Looks like Invocation.Builder.rx() methods which accept
>>>>> RxInvokerProvider are visible to these RxInvokerProviders.
>>>>>
>>>>> This is problematic. I see Jersey RxInvokerProviders delegate back to Invocation.Builder so I can appreciate why Invocation.Builder is passed on,
>>>>>
>>>>> but it just does not look right to me that rx() and similarly, async() bridges, are still visible to the providers.
>>>>>
>>>>> Cheers, Sergey
>>>>>
>>>>> On 13/01/17 20:36, Pavel Bucek wrote:
>>>>>> Dear experts,
>>>>>>
>>>>>> please review following wiki and APIs:
>>>>>>
>>>>>> https://java.net/projects/jax-rs-spec/pages/RxClient <https://java.net/projects/jax-rs-spec/pages/RxClient>
>>>>>> All added classes related to Reactive Client APIs are linked from that page, but let me allow a short recap.
>>>>>>
>>>>>> JAX-RS Client is being extended by the ability to provide a way how to process responses in reactive fashion. The change consists of:
>>>>>>
>>>>>> - adding rx(...) methods to Invocation.Builder
>>>>>> - defining RxInvoker
>>>>>> - allowing users to extend this API by providing RxInvokerProvider
>>>>>>
>>>>>> Specification will mandate implementation for CompletionStage from Java SE 8.
>>>>>>
>>>>>> Client code examples:
>>>>>>
>>>>>> - basic use
>>>>>> CompletionStage<List<String>> cs =
>>>>>> client.target("remote/forecast/{destination}")
>>>>>> .resolveTemplate("destination", "mars")
>>>>>> .request()
>>>>>> .header("Rx-User", "Java8")
>>>>>> .rx() // gets CompletionStageRxInvoker
>>>>>> .get(new GenericType<List<String>>() {
>>>>>> });
>>>>>>
>>>>>> cs.thenAccept(System.out::println);
>>>>>>
>>>>>> - using custom RxInvokerFactory (this is little artificial, since the factory just returns CompletionStageRxInvoker, but support for Observable from RxJava or ListenableFuture from Guava can be done in the exact same manner)
>>>>>>
>>>>>> CompletionStage<List<String>> cs =
>>>>>> client.target("remote/forecast/{destination}")
>>>>>> .resolveTemplate("destination", "mars")
>>>>>> .request()
>>>>>> .header("Rx-User", "Java8")
>>>>>> .rx(CompletionStageRxInvokerProvider.class)
>>>>>> .get(new GenericType<List<String>>() {
>>>>>> });
>>>>>>
>>>>>> cs.thenAccept(System.out::println);
>>>>>>
>>>>>> Source links:
>>>>>>
>>>>>> - https://github.com/jax-rs/api/blob/2.1-m02/jaxrs-api/src/main/java/javax/ws/rs/client/Invocation.java#L298 <https://github.com/jax-rs/api/blob/2.1-m02/jaxrs-api/src/main/java/javax/ws/rs/client/Invocation.java#L298>
>>>>>> jax-rs/api <https://github.com/jax-rs/api/blob/2.1-m02/jaxrs-api/src/main/java/javax/ws/rs/client/Invocation.java#L298>
>>>>>> github.com <http://github.com/>
>>>>>> api - JAX-RS API Source Code
>>>>>>
>>>>>> - https://github.com/jax-rs/api/blob/2.1-m02/jaxrs-api/src/main/java/javax/ws/rs/client/RxInvoker.java <https://github.com/jax-rs/api/blob/2.1-m02/jaxrs-api/src/main/java/javax/ws/rs/client/RxInvoker.java>
>>>>>> - https://github.com/jax-rs/api/blob/2.1-m02/jaxrs-api/src/main/java/javax/ws/rs/client/RxInvokerProvider.java <https://github.com/jax-rs/api/blob/2.1-m02/jaxrs-api/src/main/java/javax/ws/rs/client/RxInvokerProvider.java>
>>>>>> Examples & tests:
>>>>>>
>>>>>> - https://github.com/jax-rs/api/blob/2.1-m02/jaxrs-api/src/test/java/javax/ws/rs/core/RxClientTest.java <https://github.com/jax-rs/api/blob/2.1-m02/jaxrs-api/src/test/java/javax/ws/rs/core/RxClientTest.java>
>>>>>> - https://github.com/jersey/jersey/blob/2.x/core-client/src/test/java/org/glassfish/jersey/client/ClientRxTest.java#L86 <https://github.com/jersey/jersey/blob/2.x/core-client/src/test/java/org/glassfish/jersey/client/ClientRxTest.java#L86>
>>>>>> The last link is to the Jersey repository. Jersey version 2.26 will be JAX-RS 2.1 RI and branch 2.x is where the development will happen. Jersey 2.26-b01 (which is being released right now) implements all rx(...) methods; feel free to test/evaluate it there.
>>>>>>
>>>>>> Looking forward to your feedback!
>>>>>>
>>>>>> Thanks and regards,
>>>>>> Pavel
>>>>>>
>>>>>
>>>>
>>>
>>
>
>