On 4/27/12 5:57 AM, Martin Matula wrote:
> Hi Bill,
>
> On 4/26/12 6:29 PM, Bill Burke wrote:
>> On 4/26/12 8:29 AM, Marek Potociar wrote:
>>> 1. Name-bound interceptors seem to clash with pre-matching filters. If
>>> a pre-matching filter will try to manipulate the entity, the
>>> name-bound interceptors would not get invoked, which may be fatal
>>> (see Example 1).
>>
>> There's a few things we can do here. See later comments.
>
> Haven't seen anything in your e-mail indicating why we need name-bound
> interceptors.
The asymetric problem *IS NOT AN INTERCEPTOR ISSUE*. It is a Name-Bound
issue. If you remove interceptors, but keep name-bound filters, you
still ahve the same issue. So, you *cannot* use this argument for the
justification of removing interceptors.
* Reader/WriterInterceptor re-use is a reality. I've proven this in
Resteasy.
* The caching example is an example of Filters and Interceptors working
nicely together.
* This model works as its similar toan existing implementation Resteasy.
This pretty much summarizes my thoughts. So, can we please keep the
filter/interceptor model as is? Then we can focus emails on fixing the
issues you are concerned about and we wouldn't have to spend so much
energy and time defending the current model (energy that we all spent
months ago).
I have further specific responses to your comments, if you want to read
further.
> To me, interceptors are something that encodes/decodes/processes the
> "on-the-wire" stream based on the headers - e.g. gzip,
> encryption/decryption, signature checking. Caching being special - see
> below.
>
We have a number of name-bound features in Resteasy (it is irrelevant
whether they trigger filter or interception):
@GZip
@Cache - Automatic setting of cache control headers
@Signed - DOSETA signing
@Verified - DOSETA verification
@ServerCache - server-side caching of responses
Security annotations could be implemented in this way as well. Also
thinking of a SMIME annotation that would both change enitty and the
content-type of the response.
>>> o interceptors should be executed just once per request/response
>>> when reading from/writing to the wire, not for intermediate
>>> transformations (hence calling it MBW interceptors may be
>>> misleading)
>>
>> -1. See counter proposal.
>
> Is this -1 on the name or on the note that it should be called only
> once. I hope it is the first one. As I don't think you want to
> zip/unzip, encrypt/decrypt, etc. for the intermediate transformations
> repeatedly as you are doing read/writeEntity in the filters. IMO the
> interceptors should kick in only when reading the original stream from
> the wire and when writing it back to the wire. Again, caching being
> seemingly the only special thing that seems to break it.
> See later for an example of filters that read/write entities.
>
You can pass in annotations to writeEntity right? What if one of those
annotations were meant to trigger an interception? Maybe the right
answer is that a new intereptor chain is built based on parameters to
writeEntity()?
>>
>>> o filters should work with entities (decoded) - with one possible
>>> kind of entity being a stream
>>> + drop the get/setEntityStream methods as redundant (or keep
>>> them just as convenience shortcuts for read/writeEntity
>>> counterparts)
>>>
>>
>> -1.
>
> Why?
>
Because its clearer to do simple set/getEntityStream then
writeEntity(...entityStream). read/write are marshal/unmarshal.
set/getStream are not.
>>
>>> Option 1 seems to allow the interceptors to be reused between client and
>>> server side. But not sure if this is not only useful in a very limited
>>> set of use cases (e.g. real life may show that in most cases a server
>>> and client-side implementation may need to be separate anyway due to the
>>> need of attaching/checking different headers on the client side than on
>>> the server side etc.).
>>>
>>
>>
>> Again, I've already used a similar structure "in real life".
>> Interceptor re-usability is a reality.
>>
>>
>> Counter Proposal:
>>
>> - Consider not allowing unmarshalling within a PreMatch filter (but
>> allow get/set of Stream). PreMatching should exist primarily to
>> reroute the request, modify headers (i.e. the Accept header), or
>> change the HTTP Method. This is a reasonable restriction, IMO.
>> - Consider removing marshalling/unmarshalling capabilities altogether
>> from within Filters. Only allow get/set of Streams. This is Resteasy's
>> current behavior. We do not allow marshalling/unmarshalling within
>> Filters.
>
> Logging, authentication filters (auth params may be in headers, query
> params as well as in the entity),
> Post method override filter (that
> reads the method that should override post from the form param) are just
> a few examples of filters we have in Jersey that access the entity.
Yes, but these are all examples of *READING* the entity, not writing the
entity.
>>
>>
>> If you want to keep readEntity() in all conditions here are some options:
>>
>> * Always buffer the entity if you unmarshall within a Filter, makes
>> things simpler to define.
>
> +1, I believe that's already the case, but the interceptor chain should
> kick in when buffering IMO - i.e. the entity should be buffered in the
> "decoded" state, since when somebody needs to work with entity, I don't
> think you want to run interceptors (i.e. unzip and decrypt) every time
> you read from the buffer, do you?
>
I disagree. You want to buffer in a decoded state? Then doesn't that
trigger your asymetric problem in the name-bound signature verification
> Seems to me there are two classes of use-cases that have conflicting
> requirements - both are now mixed into the interceptors proposal. These
> are:
> 1) Encoding/decoding the on-the-wire stream - Gzip, encrypt, signature
> interceptors fall into this category
> 2) caching - is it a one-member class? any other fall here?
>
I completely disagree with your analysis that there are conflicting
requirements. IMO, this is more about a PreMatchingFilter screwing up a
Name-Bound filters or interceptor. Again, I do not think this issue is
solvable.
> Marek's proposal mainly considers class 1) and for that I think it is
> the right way to go. For these you want to invoke the interceptors only
> when working with the original stream, not repeatedly.
>
> For class 2) you want to invoke the read interceptor per type you pass
> to the readEntity() method (or send in writeEntity()).
> So perhaps this should be looked at separately and not mixed into the
> same single concept as it really is a different thing.
>
> A heretic question might be if we think class 2) is generic enough -
> i.e. do we expect many people providing custom implementation of this
> kind of interceptor - so that it has to be part of the standard rather
> than being an implementation detail of read/writeEntity method of a
> particular JAX-RS impl.
>
> On the other hand, supporting this use-case may be as simple as coming
> up with appropriate decorator annotation indicating that the interceptor
> is entity specific - and defining that those would kick in for every
> read/writeEntity(), not just when reading the on-the-wire stream (and
> IMO should always be called after stream-oriented read
> interceptors/before stream-oriented write interceptors).
>
> Does that seem reasonable?
No it does not seam more reasonable.
* Nothing you suggest solves your asymetric problem.
* You forget that re-usable interceptors are a reality too.
Bill
--
Bill Burke
JBoss, a division of Red Hat
http://bill.burkecentral.com