Hi Ondrej,
I had to peek into your proposal little bit and there is a couple of
things, which are not clear to me:
- what if there is some technology (now or in the future), which will be
able to take advantage of having direct access to Invocation.Builder? I
see that you are always converting CompletableFuture. That is fine, but
couldn't that become a limitation in the future?
(I'm not sure about the answer, but I'd like to know your thoughts,
especially if you already have an opinion or maybe an answer.)
- I believe <T> T unwrap() is not enough, the implementation most likely
will always need to somehow choose a provider (in your case) and there
is no T.class in java. So then the unwrap will be "<T> T
unwrap(Class<T>)", which is almost the same as "<T extends RxInvoker> T
rx(Class<? extends RxInvokerProvider<T>> clazz)" - in terms that you are
providing "some" class somewhere in the chain. Because that's the only
difference I do see (for the user).
(I guess I should share my vision: RxJava provides rxjava-jaxrs module,
where is the FlowableRxInvokerProvider stored and user will use that ..
for each rx(...) invocation. It's not perfect, but on the other hand,
everything is statically typed and user doesn't need to care about
anything else - for example about converting CompletableFuture ro
something else, since Flowable will be returned directly.).
I guess the second quiestion is almost reduced to the first one - do we
really want to have CompletableFuture ALWAYS in the chain? (or to be
more on point - do we want to create JaxRsCompletableFuture and use that
as an "rx extension point"?)
Thanks again for your proposal!
Pavel
On 15/01/2017 18:30, Ondrej Mihályi wrote:
> In my suggstion, rx() still has to remain to get the default,
> obviously. The difference is that rx() would return an extension of
> CompletionStage, making it is possible to add new methods for
> extensibility (like the convert()/unwrap() method in my example). It
> makes the main interface clean, without additional rx() methods except
> the default ones, but still makes possible to use whatever extension
> desired.
>
> Added benefit for extensions is that it would be much easier to create
> the glue code, as an extension would only need to implement the
> conversion from CompletionStage to something else (around 10 lines of
> code for Flowable in RxJava2). With the current Pavel's version, it is
> necessary to implement the whole RxInvoker interface (with 25 methods
> !! ).
>
> Compare:
> - default (using CS)
>
> client.request().rx().get(). // we get the RxCompletionStage here, which extends CompletionStage,
> // we use CompletionStage methods from now on
> .thenAccept(System.out::println);
> - using Flowable in RxJava2
> client.request().rx().get(). // we get the RxCompletionStage here, which extends CompletionStage
> .<Flowable>unwrap() // additional method in RxCompletionStage, on top of CompletionStage,
> // to convert to Flowable (as Markus suggested)
> .subscribe(System.out::println);
> Ondrej
> 2017-01-15 17:20 GMT+01:00 Sergey Beryozkin <sberyozkin_at_talend.com
> <mailto:sberyozkin_at_talend.com>>:
>
> IMHO rx() needs to remain in order to get the default, this is as
> simple as it can get. Doing more than rx() in order to get
> CompletionStage would be bad. But I'm warming to something similar
> to what you are suggesting. .unwrap() is redundant IMHO. either
> rx() for the default or, example, RxInvoker<Observable> rxInvoker
> = rx(Observable.class) The reason it works better than the
> prototype suggested by Pavel is that this code is *portable*.
> Clearly, in most cases, it won't be the users who will create
> RxInvoker<Observable>, etc but Jersey, CXF, RestEasy will have
> dedicated modules with META-INF linking, in this case, Observable
> to a specific RxInvoker. This will be documented and users willing
> to work with non-default rx() will add a given module to the
> classpath...Sergey
> On 14/01/17 14:28, Markus KARG wrote:
>>
>> Sounds good for me, but actually I think it would be even simpler
>> if we name it "unwrap()", and do not fill in any parameters. As I
>> said, I think applications will typically use only one technology
>> at the time. So "unwrap()" simply can check if there is a
>> @Provider registered having a method with the same parameters as
>> your proposed "conver()", we finally have the requested
>> plugability, no additonal complexity, follow existing JAX-RS
>> patterns, and get rid of repeating lots of "rx(Class)". :-)
>>
>> -Markus
>>
>> *From:*Ondrej Mihályi [mailto:ondrej.mihalyi_at_gmail.com
>> <mailto:ondrej.mihalyi_at_gmail.com>] *Sent:* Samstag, 14. Januar
>> 2017 12:54 *To:* jsr370-experts_at_jax-rs-spec.java.net
>> <mailto:jsr370-experts_at_jax-rs-spec.java.net> *Subject:*
>> Suggestion to refactor rx invoker API
>>
>> I was thinking about an alternative to current rx API, to
>> simplify it in context of supporting extensibility.
>>
>> My idea is to modify the CompletionStageRxInvoker, so that it
>> returns an extension to CompletionStage, which would contain
>> additional method to convert the interface to any other reactive
>> interface: public interface RxCompletionStage<T> extends
>> CompletionStage<T> { <NEW> NEW
>> convert(Function<CompletionStage<T>, NEW> converter); }
>>
>> With this, we would move all the complexity to this new
>> interface, which still can be used as a usual CompletionStage,
>> but with the additional convert method, it provides an extension
>> point to other interfaces. And it's based on standard
>> CompletionStage, therefore we don't need additional rx invokers.
>>
>> We could remove RxInvokerProvider, the 2 rx() methods from the
>> Builder, which accept RxInvokerProvider, and even remove the
>> RxInvoker interface, as CompletionStageRxInvoker would be the
>> only required implementation.
>>
>> Here is an example of what I mean, with RxJava2 as an example:
>>
>> *client*.request().rx().get(). // we get the RxCompletionStage
>> here, which extends CompletionStage
>> .convert(this::flowableFromStage) // accepts a function that
>> converts the CompletionStage to another interface
>> .subscribe(s -> testResult = s, Throwable::printStackTrace);
>> An example of a working code here
>> <https://github.com/OndrejM-demonstrations/JavaEEReactive/blob/rxjava/src/test/java/reactivejavaee/CompletionStageRxJavaTest.java#L120>
>> (although not using JAX-RS, just wrapping a method that returns a
>> CompletionStage).
>> Ondrej
>