users@jax-rs-spec.java.net

[jax-rs-spec users] Re: JAX-RS Client Reactive API review

From: Ondrej Mihályi <ondrej.mihalyi_at_gmail.com>
Date: Fri, 20 Jan 2017 20:51:55 +0100

+1 to allowing CompletionStage as a return type (notice: I wouldn't
restrict it to CompletableFuture)

The framework would automatically add .whenComplete(asyncResponse::resume,
asyncResponse::resume) to the result to avoid unnecessary boiler plate.

But I think that this is technically unrelated to the client API and
deserves another thread. However, server-side API is equally important as
client side, even more because it is used much more often in the code (with
Javascript clients).

Ondrej

2017-01-20 19:02 GMT+01:00 Markus KARG <markus_at_headcrashing.eu>:

> Pavel,
>
>
>
> sorry for the delay. Regarding the servcer-sided async optimization, I
> think the following code illustrates rather well why I say "-20%" and why I
> want it to be discussed *together* with RX.
>
>
>
> The green stuff is the essence what the *application* programmer actually
> wants to focus upon: business code. Look how much code you need
> *currently* (first method), and compare it to the short *counterproposal*
> (second method). Code is effectively reduced by 20% (red = api-induced
> boilerplate). :-)
>
>
>
> /*
>
> * *Current API*
>
> *
>
> * Bad: *Parameters* are declared only for sake of the API, while the
> programmer simply wants to focus on *business* code.
>
> * Bad: *Boilerplate* "thenApply" just to tell JAX-RS when the RX
> operation is completed.
>
> * Bad: Result is declared *void*, but actually a *String* entity is the
> final outcome of this REST method!
>
> */
>
> @GET public *void* serverSideDemo(@Suspended final AsyncResponse
> asyncResponse) {
>
> CompletableFuture.*supplyAsync*(ReactiveDemo::
> *veryExpensiveOperation*, executor).thenApply(asyncResponse::resume);
>
> }
>
>
>
> /*
>
> * *Counterproposal*
>
> *
>
> * Better: Result is declares as "There will be a String some fine day!"
>
> * Better: No
>
> */
>
> @GET public *CompletableFuture<String>* serverSideDemo () {
>
> return CompletableFuture.*supplyAsync*(ReactiveDemo::
> *veryExpensiveOperation*, executor);
>
> }
>
>
>
> Again, my proposal does not solve any "techical problem". It just makes
> JAX-RS code look much more business-focused, as it deals much less with
> technical aspects but mostly with business-logic. :-)
>
>
>
> -Markus
>
>
>
>
>
> *From:* Pavel Bucek [mailto:pavel.bucek_at_oracle.com]
> *Sent:* Dienstag, 17. Januar 2017 12:14
>
> *To:* jsr370-experts_at_jax-rs-spec.java.net
> *Subject:* Re: JAX-RS Client Reactive API review
>
>
>
> 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
> <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
> <pavel.bucek_at_oracle.com>]
> *Sent:* Freitag, 13. Januar 2017 21:36
> *To:* 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
>
>
>
>
>
>
>
>
>