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.
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.
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.
- 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).
- 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).
- 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.
- 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.
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