On Aug 16, 2012, at 9:41 PM, Bill Burke <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> 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.
>
>>>
>>> * 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".
>
>>> Call AsyncResponse.resume()?
>>
>> Should not, but implementations should prevent this.
>>
>>> If so, how can you avoid an infinite loop?
>>
>> Up to implementation to prevent this.
>>
>>> I question all this behavior.
>>>
>>> * I don't like the current behavior of ResumeCallback and think it should be removed.
>>>
>>> * I think we should have similar Servlet 3.0 callbacks.
>>>
>>> CompletionCallback
>>> OnErrorCallback
>>
>> Please define the API for those 2.
>
> Same as ResumeCallback except it gets called after a response is sent back to the client instead of before.
>
> 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.
@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?
>>
>>> 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.
Marek