Hi Marcus,
please see inline.
On 16/01/2017 22:35, Markus KARG wrote:
>
> About rx() and async(): I didn't liked the introduction async()
> methods from day one, so similarity to it is a negative argument from
> my view. ;-)
>
ok, noted. I wasn't part of that discussion back then, but I don't want
to use this as an excuse.
I don't think we should change the approach which was already selected
some time ago - it would result in non-consistent mess. Also, I haven't
seen others raising this point yet, so it will most likely stay as it is
now.
> About type safety: The one and only _good_ type safe way is to ask RX
> framework vendors to agree upon a common API. As they did not, ANY
> solution will be bad.
>
so you are proposing to completely scratch this and don't do anything?
> About Future<T>: Compare the current code with my proposal and count
> the number of characters JAX-RS users have to write, and compute the
> difference in percentage. You will notice that the boilerplate for
> many applications will get reduced by about 20%. Also you will notice,
> that code is much cleaner. And, BTW, I will _not_ file a separate
> issue for that. The reason is that for me, RX is not just
> Client-API-RX, so I do _not_ want to separate these discussions.
>
I'm confused - the only difference is that instead of returning future
(your proposal), users must call ".get()". That's six characters,
usually not even close to 20% you are mentioning. Maybe there is some
other part of the proposal which I did not get from your previous email
- can you please clarify?
As I already wrote - of you'd talk about CompletionStage, I can
understand that and see its advantage. Returning Future doesn't seem to
be big improvement, since the difference in the code / container
implementation is really minimal. Also, you most likely can do it
already by implementing trivial MessageBodyWriter.
Best regards,
Pavel
> -Markus
>
> *From:*Pavel Bucek [mailto:pavel.bucek_at_oracle.com]
> *Sent:* Sonntag, 15. Januar 2017 19:05
> *To:* jsr370-experts_at_jax-rs-spec.java.net
> *Subject:* Re: JAX-RS Client Reactive API review
>
> Hi Markus,
>
> thanks for your response. Please see inline.
>
> On 13/01/2017 23:56, Markus KARG wrote:
>
> Pavel,
>
> thank you for sharing the code examples.
>
> In fact I don't know if you actually want us to repeat our
> arguments, but here is my personal judgement of the proposed RX API:
>
>
> I noticed the discussion already started in the other thread :-)
> Thanks for summing that up here!
>
> * The decision to make CompletableStage the default and most simple
> way to use RX in JAX-RS is appreciated, as it will never go away, due
> to de-facto being the sole RX interface in the Java world which is
> part of the JRE and / or any JCP standard currently and in near future.
>
>
> there is really no other option right now.
>
> (we are thinking about including "Flow-like" classes into some parts
> of the spec, for better future adoption - since we know, it WILL be
> there .. but that will be discussed in separate thread).
>
>
> * I don't like the rx() method which changes from "synchronous" to
> "reactive" invocation. I doubt that anybody would love to repeat rx()
> again and again, and I doubt that people want to write programs that
> shares sync and rx style invocations within the same code block. My
> counterproposal is to provide an additional RxClient class which
> always produces reactive invocations, and keep the normal Client
> untouched.
>
>
> I see what you mean, but what about async(), which is already on the
> client? It is almost similar in terms of the way how the API was extended.
>
> Also, there was similar enhancement related to nio, with the same
> approach - adding NioInovker, returned with method nio(). I know that
> this could be handled differently (and we will discuss that in future
> review), but it seems to me as "already established" way how to
> enhance JAX-RS client.
>
> User can always create factory method and don't need to write the same
> code over a and over again. I believe this is already happening, at
> least in the code I saw.
>
> Doing separate client for reactive programming style is interesting
> idea, but I believe it wouldn't fit into JAX-RS - when I thought about
> this, it was too similar to existing client, but would most likely
> require another copy of WebTarget, Invocation, etc., which doesn't
> seem to be worth it.
>
>
> * I don't like the need to write down the type of custom RX invoker
> type again and again. I doubt that people really will write software
> which shares CompletableStage, RxJava, Guava etc. within the same code
> block. I assume that people instead will write full applications in
> *either* CompletableStage *or* RxJava *or* Guava etc (so they decide
> per app, not per invocation). The resulting application source code
> will be easier to read and potentially less error-prone if there
> instead would be an SPI to replace JAX-RS's reactive provider at
> deploy time, e. g. from the default CompletableStage to RxJava Observable.
>
> I completely agree that having to add rx(..) method parameter over and
> over again is not elegant solution, but I don't know how else that can
> be solved. If we want to continue to have client statically typed (and
> I assume we do), there must be a way how to say "give me Observable"
> and the implementation must know which type users wants (to use
> correct provider - I already replied on this on the other thread).
> Then it seems that it is not really different, when we are providing
> class/instance of RxInvokerFactoty or a type, which should be
> returned, since both do define the same.
>
> (the provider approach is more complicated, since it would require the
> provider to be registered, so another "state" which needs to be set
> and potentially lots of hours spent on "why is rx(Observable.class)
> not working?").
>
>
>
> * The proposal only contains reactive support for the client API. I
> want to rise the question whether it might be a good idea to also
> extend server-sided async support at the same time. My proposal is to
> allow Future<T> as a return type of resource methods, so it is pretty
> easy to do concise async code like the example below (if we enforce
> CompletableFuture instead of CompletableStage). The idea is to get rid
> of the async boilerplate and the need to explicitly wait for the
> result of the RX invocation, but just to tell JAX-RS to set up the
> precessing chain and let it execute in the background by Java EE's
> default ManagedExecutorService (hence outside of the scop of the code
> block, hence outside the HTTP thread pool):
>
> @GET public Future<List<String>> collectResultsFromOtherServers() {
>
> return *client*.target(*"remote/forecast/{destination}"*)
> .resolveTemplate(*"destination"*, *"mars"*)
> .request()
> .header(*"Rx-User"*, *"Java8"*)
> .rx()
> .get(*new *GenericType<List<String>>() {
> });
> /*
> * Certainly the actual idea is to combine many parallel invocations
> instead of simple returning one!
> */
>
> }
>
>
> Even if there is some combined result, how would that be different
> compared calling Future.get() on the combined one and returning the
> result directly? If we are returning Future, there is always a
> "Future.get()" invocation, which is blocking by definition.
>
> I can see the enhancement if we would restrict this to CompletionStage
> (not CompletableFuture), since there is no get() method, so the impl.
> could register a callback and be notified after the event is done - in
> reactive, non-blocking fashion). Or we could return something like
> Flow.Publisher.. we are already thinking about something similar for
> Client SSE API, but that is different discussion I guess.
>
> Since it seems like current proposal brings enough discussion topics
> on its own, and this seems to be separate issue, can you please file
> this as a issue against JAX-RS on java.net? We can get back to this later.
>
> Thanks!
> Pavel
>
>
> -Markus
>
> *From:*Pavel Bucek [mailto:pavel.bucek_at_oracle.com]
> *Sent:* Freitag, 13. Januar 2017 21:36
> *To:* jsr370-experts_at_jax-rs-spec.java.net
> <mailto:jsr370-experts_at_jax-rs-spec.java.net>
> *Subject:* JAX-RS Client Reactive API review
>
> 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/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/RxInvoker.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/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
>