users@jax-rs-spec.java.net

[jax-rs-spec users] [jsr339-experts] Re: HEADS-UP, IMPORTANT: Problems with JAX-RS interceptors

From: Bill Burke <bburke_at_redhat.com>
Date: Thu, 26 Apr 2012 12:29:01 -0400

I do not think removing interceptors is an option. Please review:

http://bill.burkecentral.com/2011/05/24/interceptors-in-jax-rs-2-0

Besides re-usability, there is an issue on the client side with
execution flow:

Consider this:

Response response = target.request().get();
MyObject obj = response.readEntity(MyObject.class);

Response response = target.request().put();
... no further code ...

The entity is unmarshalled outside of the scope of the request. Read
the blog for the client caching example too see how interception effects
things. There's also the case of metadata that you might want to pull
from an annotation.

The current interceptor/filter proposal is similar to Resteasy's. Our
approach evolved as we implemented various use-cases discussed in the
above blog (and additional ones not discussed). So I do have a lot of
experience with this. More comments follow:


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.

> 2. The generic type information on the interceptor seems redundant. It
> is not clear what should the runtime do when an interceptor is
> configured for a type that it does not support in it's generic type
> declaration.

I have yet to write an read interceptor that is type specific.

> 3. The reader interceptors are now not restricted in any way wrt.
> changing the returned entity Java type to a type that is
> incompatible with the Java type requested by the caller (either
> JAX-RS runtime or an interceptor up in the chain). Changing the type
> of the returned entity in a non-compatible way seems to always have
> fatal consequences as, by definition, the caller expects to receive
> the original type it asked for.

EJB Interception has a similar issue, so, IMO this is ok. Remember this
is an advanced feature. We shouldn't have to protect the user from
every stupid mistake they *possibly*, however improbable, could make.
BTW, I also remember having a similar discussion when EJB 3.0 was
defining interceptors.

> 4. Filters may un-intentionally corrupt the entity stream in
> applications configured asymmetrically wrt. interceptors (see
> Example 2)
>

#4 appears in the Servlet specification as well. There's nothing
stopping a servlet Filter from corrupting the InputStream. There's also
the case of HttpServletRequest.getParameters() and
HttpServletRequest.getInputStream(). If you getParameter() you could
possibly corrupt the InputStream. So, IMO, corrupting the stream is
acceptable behavior. We shouldn't have to protect the user from everything.

In Resteasy we do not allow unmarshalling within a Filter. Only
getting/setting of OutputStreams. Filters are generally only used to
check/add/remove/modify headers, at least in the features I've implemented.

> We haven't found any reasonable way to resolve the outlined issues
> without changing the API. Here are couple of options for a solution we
> can see:
>
> * /Option 1/
> o interceptors should not be typed

+1 Interceptors should not be typed. I have yet to write one that was.

> should be stream focused -

Not sure. I like the option of being able to change the returned object
even though I've never actually used that feature. Again...We don't
have to protect the user from being stupid. This is an advanced
feature, and EJB has similar issue.

> i.e. could be renamed to Input/OutputStreamInterceptors
> + people should use MBW and/or filters for entity
> type-dependent manipulation

-1. I like the current names and they fit.

> o interceptors should only be global (as they may need to be
> invoked in the pre-matching phase)

-1. I'll make a counter proposal later.

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

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

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


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.
* Offer a buffering method in the filter context so that unmarshalling
can be redone. (I think this already exists?)
* Throw an exception if there are Named-Bound Interceptors, and you
haven't buffered the request.


Some other comments:

* Unmarshalling an entity within a filter to a specific type is a rare
occurrence. I actually haven't come across it yet. The only thing I've
ever done is buffer the entity.
* Unless you can spell out exactly how you could implement the features
I've written about in my blog, I think filters/interceptors should be
entirely removed from the specification. If Resteasy (or any other
container) has to have a extended interceptor model because the
specification is inefficient, then the spec is a failure.


-- 
Bill Burke
JBoss, a division of Red Hat
http://bill.burkecentral.com