Santiago,
I agree that in reality, it may be more practical to use a completely new
invoker.
But adding new methods to existing AsyncInvoker seemed a raesonable
alternative to me.
I though that we could add new methods like "CompletionStage<T> get()" and
"RxInvoker<T> get(...)", but it's true that the one returning
CompletionStage would have to be renamed to avoid collision with the one
that returns Future.
Ondrej
2017-01-17 16:49 GMT+01:00 Santiago Pericasgeertsen <
santiago.pericasgeertsen_at_oracle.com>:
> 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>:
>
>> 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/JerseyCompletion
>> StageRxInvoker.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
>>
>> 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/mai
>> n/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
>> api - JAX-RS API Source Code
>>
>> - https://github.com/jax-rs/api/blob/2.1-m02/jaxrs-api/src/mai
>> n/java/javax/ws/rs/client/RxInvoker.java
>> - https://github.com/jax-rs/api/blob/2.1-m02/jaxrs-api/src/mai
>> n/java/javax/ws/rs/client/RxInvokerProvider.java
>>
>> Examples & tests:
>>
>> - https://github.com/jax-rs/api/blob/2.1-m02/jaxrs-api/src/tes
>> t/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
>>
>> 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
>>
>>
>>
>>
>>
>>
>>
>>
>
>