On 4/30/12 8:40 AM, Martin Matula wrote:
> Hi Bill,
> I think the following should work:
>
> 1) bufferEntity() should buffer the entity by the way of doing
> readEntity(byte[].class or InputStream.class) - i.e. applicable
> interceptors would get called, entity would be buffered in
> partially-intercepted state so that at least the global interceptors
> don't have to be called again.
bufferEntity() should buffer whatever is on the wire, no? If you don't
you could effect things like hash and signature calculations.
> 2) readEntity() should make an implicit call to bufferEntity() before it
> does anything. This is just for filters! Not for reading in the resource
> method or the client.
Just a thought..Maybe it shouldn't be implicit? Maybe the filter is
reading the entity, then replacing it with writeEntity(), so a buffer is
overkill? Or maybe its just better to protect the filter developer from
himself? I don't know...
> 3) maybe - remove get/setEntityStream() from filters (could not find a
> use-case that could not be covered by interceptors or a combination of
> an interceptor and a filter that triggers the interception by calling
> bufferEntity()). Or maybe we want to keep it to be safe. Which leaves us
> with just the two things above.
>
> Here is the rationale:
>
> - on the receiving end, you want "decoding" interceptors to work based
> on the headers, not based on the annotation - i.e. if the entity in the
> incoming message is zipped, it is not the resource method that tells me
> it is, it is the sender who should tell me - by the way of the headers.
> So, typically no annotations processing should be needed to make the
> stream usable for other consumers. At least I think I would never add a
> feature that would not have this kind of characteristics.
>
hashes or signatures?
> - given that, with the current API, I can implement gzip/decryption
> either as a prematching filter (by wrapping the stream using
> get/setEntityStream() on the filter context) - this is not reusable by
> the client; or even better (and reusable by the client) as an untyped
> global (unbound) interceptor. The interceptor can wrap the entity stream
> and remove the encoding info from the header, to make sure it does not
> try to unzip an already unzipped stream later in the processing. It may
> be a good practice to add a "mark" to the request property bag
> (indicating the message was zipped or encrypted - which can be useful if
> you want to have encryption enforcement filter somewhere in the chain
> that ensures that only encrypted messages are allowed).
>
FYI, resteasy implements gzip as an interceptor. The code is reusable
on client and server.
> - there can be bound interceptors as well, but I don't think they should
> be essential for the filters to work OK (I am still talking about the
> receiving end) - at least I did not come up with anything like that I
> would want to implement. Note your signature checking interceptor is
> bound, but also not essential for reading by the filter - it is needed
> just to enforce and verify the signature before the resource method
> reads the entity. That's fine.
>
> Now when filters use the entity or entity stream, I think the following
> should work:
>
> - get/setEntityStream work with the unintercepted stream. However, I
> can't come up with a use-case that could not be covered by an
> interceptor or a combination of interceptor and filter doing
> bufferEntity() (e.g. logging filter could be implemented like that). So,
> maybe these methods can be completely removed from filters. If we keep
> them, the filter should not use them to change entities in a way that
> may break headers (i.e. replace a zipped entity with an unzipped one
> while keeping the zip encoding header).
>
I'm fine removing them.
> - bufferEntity() should in my view buffer the entity by doing read as a
> byte array or stream, that would trigger applicable interceptors (such
> as gzip or so) - i.e. that way we would make it possible to buffer the
> entity in a state when we don't have to run certain interceptors again -
> gzip would remove the encoding header and not be called multiple times
> for subsequent calls.
>
Again, I disagree. It may screw up hashes or signatures no?
> - I tend to agree readEntity() *in filters only* should do
> bufferEntity() implicitly and then read from the buffer - bound
> interceptors would kick in for every read based on the parameters passed
> to readEntity()
>
> - writeEntity() again won't trigger unecessary zipping/encryption, since
> filter would not pass any annotation and the header would no longer have
> the encoding info either. So things should work fine there.
>
> I think on the outbound side, given we have interceptors, I don't see a
> need for get/setEntityStream() methods in filters either. Again - should
> be implementable using interceptors. Otherwise things are quite clear
> and I don't see other issues.
>
In summary I think:
* bufferEntity() buffers the entire wire stream. If you don't hash or
signature calculation may break.
* interceptors are triggered on Fitler.readEntity(), but a new
interceptor chain is created based on the annotation data passed into
readEntity() regardless of any bound interceptors.
* I don't know whether or not readEntity() should automatically buffer
the entity. Protecting the user from himself is good sometimes.
* writeEntity() should have a new interceptor chain created based on the
annotation data passed into it.
One last issue:
* If you call writeEntity() what should happen to headers? For example,
if the original wire bytes are gzip encoded and you call writeEntity()
what happens to the Content-Encoding header? In this case, maybe we
writeEntity() should have an additional headers parameter?
public <T> void writeEntity(
final MultivalueMap<String, Object> newHeaders,
final GenericType<T> genericType,
final Annotation annotations[],
final MediaType mediaType,
final T entity);
* BTW I think writeEntity() is the most dangerous method of all.
readEntity() we can get around potential problems with the workaround of
buffering the entity. writeEntity() has no such guarantees.
> Did I miss something?
> Martin
>
> On 4/27/12 9:52 PM, Bill Burke wrote:
>>
>>
>> On 4/27/12 2:11 PM, Martin Matula wrote:
>>> Hi Bill,
>>> I think you convinced me. So, basically, if things stay the way they
>>> are, we can be sure all the use-cases are covered. Will have to be
>>> careful writing the filters (as it is possible especially for the
>>> pre-matching filters that read/writeEntity does not make much sense)
>>> and interceptors to ensure they can be reasonably combined. And you
>>> are saying you haven't run into the sort of clashes Marek outlined
>>> with RestEasy over the last few years you have been using
>>> interceptors. So as long as we do a reasonable job in implementing
>>> the types of features you mention below the right way and our users
>>> combine them in sensible ways, things will work.
>>
>> Resteasy does not have a PreProcessor interceptor that reads the
>> entity. You guys have a valid strong use case that does. It is a valid
>> issue that we should talk about and work through to see what
>> constraints we should add to the API or to see if we can get an
>> improved API.
>>
>> I freaked out because I thought you were really serious about removing
>> interceptors. Maybe start over and discuss again?
>>
>> I think:
>>
>> a) Removing generics might be a good idea. I've never used them. Have
>> you guys? Then again, what if it could be used as a deployment check?
>> If an typed interceptor is assigned to a resource method with
>> non-matching types, it could be flagged at deployment time
>>
>> b) We should investigate the behavior of calling write/readEntity
>> within a filter.
>>
>> c) Maybe writeEntity within PreProcessing should create its own
>> interceptor chain based on the type and annotation[] array passed into
>> it?
>>
>> d) Maybe readEntity() should automatically buffer the request?
>>
>> e) Maybe methods on ContainerRequestContext should be added like
>> wasBuffered(), wasRead() etc., then other interceptors will know that
>> a read/write/buffering happened?
>>
>>
>>
>> Bill
>
--
Bill Burke
JBoss, a division of Red Hat
http://bill.burkecentral.com