jsr339-experts@jax-rs-spec.java.net

[jsr339-experts] callbacks continued...

From: Bill Burke <bburke_at_redhat.com>
Date: Fri, 17 Aug 2012 10:23:47 -0400

On 8/17/2012 5:55 AM, Marek Potociar wrote:
>
> On Aug 16, 2012, at 9:41 PM, Bill Burke <bburke_at_redhat.com
> <mailto:bburke_at_redhat.com>> wrote:
>
>>
>>
>> On 8/16/2012 2:35 PM, Marek Potociar wrote:
>>>
>>> On Aug 16, 2012, at 5:39 PM, Bill Burke <bburke_at_redhat.com
>>> <mailto:bburke_at_redhat.com>> wrote:
>>>
>>>> * Why have a separate setTimeoutHandler() method? Why not just
>>>> create and register a TimeoutCallback?
>>>
>>> because you may want to change the timeout values (e.g. from within
>>> the TimeoutHandler)
>>>
>>
>> Why can't this be implemented as a callback?
>>
>> TimeoutCallback.timeout(AsyncResponse res) {
>>
>> res.resume(Response.status(500).build());
>>
>> }
>
> For me, callbacks in this context are listeners. (We can rename to
> XxxListener if you want...) TimeoutHandler however is not a listener and
> thus since it may modify the processing state machine I want to have
> only one timeout handler registered at a time. I don't want to deal in
> spec with issues of one handler resuming a response and other suspending
> it etc. Thus I do not want to register timeout handler with other
> callbacks/listeners. Because it is a handler, not a listener.
>

Since we already have a untyped register() method, whats the difference
on whether its a handler or a callback or a listener? Each specific
interface for each specific callback defines the semantics on what can
and can't be done within it.


>>
>>>>
>>>> * register() returns a boolean in some instances and AsyncResponse
>>>> in others and the javadoc is off. IMO, you don't need a boolean
>>>> return value , instead throw an UnsupportedOperationException or
>>>> something.
>>>
>>> Fixed, nice catch. The decision to return boolean rather than failing
>>> is to provide optional (impl. specific) callback support. IOW, users
>>> may register even callbacks that are not understood and
>>> implementation should work normally, just without calling those
>>> callbacks. (See also the definition of OPTIONAL.)
>>>
>>>>
>>>> * If anything, ResumeCallback happens before response is sent, isn't
>>>> this what a ContainerResponseFilter already does? Also, how could
>>>> ResumeCallback modify the response?
>>>
>>> It doesn't. It's a listener.
>>>
>>
>> I don't get it then. Why invoke it before the actual response is sent
>> back if you can't modify the response?
>
> As I said later in my reply, I don't have a strong use case... off the
> top of my head - perhaps if you are doing some distributed computation
> or result search, you may use this event to terminate workers that are
> still computing? It's just another event - like "on error" or "on complete".
>

I do have a use case for post-response sent. Again, the case where you
want to commit a transaction if and only if the client successfully
received the response, or rollback on an error.

>> interface CompletionCallback {
>>
>> void completed(AsyncResponse res, Response res) {
>> }
>> }
>
> I wonder if this is something that could be useful also for sync
> invocations? But how to register it? Should we instead consider an
> annotation-based approach similar to CDI/EJB interceptors? E.g.
>

You are right. Sync would be interested in Connection closed, error,
and completion too. But, not sure I like an annotation approach. For
something like @OnComplete in your example, how do you know what request
was completed? @Callbacks requires callbacks with no state. The use
cases I have for various callbacks will have to be initialized within code.

> @Path("myresource")
> public class MyResource {
> @OnComplete
> public void completionCallback(CallbackEvent event) { ... }
>
> @Get
> @Callbacks(MyResource.class)
> public void doGet() { ... }
> }
>
> Obviously, that is something that would span beyond our async topic. And
> something that would have to be thought through more...
>
>> Called when there was an error sending the response back to the client.
>>
>> interface ErrorCallback {
>>
>> void error(AsyncResponse res, Response res) {
>> }
>> }
>
> Does the above make sense? What's in the Response? Where's the error
> information?
>

Error happens on an I/O error back to the client. Since the socket
layer could be throwing proprietary exceptions, what's the point of
passing the exception along? I guess AsyncResponse and Response
parameters aren't really needed.

>>>
>>>> TimeoutCallback
>>>
>>> We have a TimeoutHandler. Since this is the special type of callback
>>> that may modify the execution state, I prefer we keep it separate
>>> from other callbacks that are intended to be used as pure listeners.
>>
>> If you look at the Servlet AsyncListener, onComplete() and onError()
>> are basically pure listeners, while onTimeout() allows you to modify
>> execution state.
>
> Yes, sadly, Servlet guys seem to have their own interpretation of term
> "listener". Also, they do not bother defining in their spec what happens
> if one such registered "listener" calls AsyncContext.complete() and then
> another one calls AsyncContext.dispatch(...). Leaves a space for
> implementation-specific differences that inhibit code portability.
>
> JAX-RS should not be a mere extension to a servlet spec IMO. It makes
> sense to cherry-pick the good parts of their API, but let's not try to
> mimic all their APIs including all their mistakes.
>

Agreed, I was just pointing out that callbacks don't have to be limited
to just listening.


-- 
Bill Burke
JBoss, a division of Red Hat
http://bill.burkecentral.com