On Mar 21, 2012, at 8:51 PM, Bill Burke wrote:
>
>
> On 3/21/12 3:20 PM, Marek Potociar wrote:
>> Hi Bill,
>>
>> On Mar 21, 2012, at 7:06 PM, Bill Burke wrote:
>>
>>>
>>>
>>> On 3/21/12 11:29 AM, Santiago Pericas-Geertsen wrote:
>>>> Hi Bill,
>>>>
>>>> At last, I found some time to look at your proposal. Here are some questions:
>>>>
>>>> i) From what I gather, entity interceptor chains can no longer be triggered from a filter (there's no readEntity()). Given that, is there a use case for a filter to update an entity interceptor chain?
>>>>
>>>
>>> I don't think so. Plus, with the current API, I don't think you can modify the entity interceptor chain anyways? Correct?
>>
>> Looking at the proposal, I wonder if the interceptors make sense anymore, given the fact that the filters can easily wrap the I/O streams, which I thought was the primary job of the interceptors? In fact, Jersey 1.x filter implementation is conceptually very close to your proposal and is able to fully cover all the functionality intended for JAX-RS 2.0 interceptors. Thoughts?
>>
>
> In my experience on the Resteasy side, filters rarely wrap i/o streams. They either add/remove headers or abort/redirect request/responses. What I found that is the i/o stream wrapping happens in MessageBodyReader/Writer interceptors. This is because reading/writing an entity is usually independent of whether this is happening on the client or server while abortion/redirection is specific to its context.
Ok, makes sense. What I am only trying to point out is that reader/writer interception can be substituted with a filter wrapping the input stream. Arguably, R/W interceptors make it more convenient and suitable for common (client/server) use cases.
> Currently, I just don't see how you could *not* have the general interceptor architecture we currently have based on the requirements I spelled out last year:
>
> http://bill.burkecentral.com/2011/05/24/interceptors-in-jax-rs-2-0/
>
> But, I do think the API has room for improvement.
I was thinking about a possibility to let filters manipulate reader/writer interceptor chains (add/remove interceptors). Would that make sense to you?
>>>
>>>> ii) Is the response filter chain executed after a call to abort(Response)? Question applies to server and client.
>>>>
>>>
>>> This requires more thought. Resteasy interceptor framework just aborts without calling the response chain, but I don't know what the right answer is. On the server side, it makes sense to call the response chain, but I'm not so sure on the client.
>>
>> What makes the client side different in this context?
>>
>>>
>>>> iii) What's the benefit of ClientResponse extending Response? Exceptions?
>>>>
>>>
>>> Yes. We might have an exception hierarchy that is re-used from client and server. i.e. WAE is one such exception and it takes a Response as a parameter.
>>
>> What would be the intended ClientResponse behavior for the inherited Response.getEntity() method?
>
> You have the same issue with our current API do you not? Would probably have to be defined in the same way.
I do, but I was hoping that if we are about to replace the current API losing the benefits of unified Req/Resp, we are going to resolve the issues of the current API.
>>>> iv) The RequestFilterContext has a few methods that only seem to apply to pre-match filters. Presumably, these will throw some sort of exception otherwise. Wouldn't be in the philosophy of this new approach to split these contexts?
>>>>
>>>
>>> Yup a specific PreMatchFilter/Context might be a better/cleaner approach.
>>
>> Perhaps we could name the server-side interfaces to:
>> - ContainerRequestFilter/Context (pre-matching)
>> - ResourceRequestFilter/Context (post-matching)
>> - ResponseFilter/Context (...guess...)
>>
>
> Up to you on what you call them.
>
>> ?
>>
>> We should perhaps also think about whether client-side components should not go into a separate package so that it is easier to split the client and server api into separate jars in the future.
>>
>
> +1
>
>>> All and all, my proposal probably needs some polishing, but the idea is to have specific contracts at each interception point. I think this is important because we can control exactly what a user can (and more importantly) *cannot* do. With the current API, IMO, there's just way too many weird edge cases you have to code for.
>>
>> I think the main disadvantage of the current API is that it is not mutable, (which is impossible to achieve by extending JAX-RS 1.x Request/Response in a sane way...). Another, but solvable, disadvantage is that it is not possible to clearly scope filters to a particular side.
>>
>
> The scoping is solvable with the current API with a scoping annotation (to be scanning friendly). But...if you require the annotation, this is no different, IMO, than having client/server specific interfaces. If you don't require it, you run the danger of users forgetting to apply the annotation and problems surfacing at runtime.
>
> Also:
>
> A 3rd disadvantage of the current API is that some methods have different behavior in different contexts.
I can't recall any. It is however true, that in different contexts the req & resp are available in different states. But that can also be said about your ClientResponse, which is something that we should fix.
FWIW internal mutability of the current immutable Req/Resp API is what would be my issue number 3.
Marek