Skip to main content

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

  • From: Bill Burke <bburke@...>
  • To: jsr339-experts@...
  • Subject: [jsr339-experts] Re: HEADS-UP, IMPORTANT: Problems with JAX-RS interceptors
  • 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


[jsr339-experts] HEADS-UP, IMPORTANT: Problems with JAX-RS interceptors

Marek Potociar 04/26/2012

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

Sergey Beryozkin 04/26/2012

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

Bill Burke 04/26/2012

[jsr339-experts] Example 3: Why current ReaderInterceptors are cool.

Bill Burke 04/26/2012

[jsr339-experts] Re: Example 3: Why current ReaderInterceptors are cool.

Sergey Beryozkin 04/27/2012

[jsr339-experts] Re: Example 3: Why current ReaderInterceptors are cool.

Sergey Beryozkin 04/27/2012

[jsr339-experts] Re: Example 3: Why current ReaderInterceptors are cool.

Bill Burke 04/27/2012

[jsr339-experts] Re: Example 3: Why current ReaderInterceptors are cool.

Sergey Beryozkin 04/27/2012

[jsr339-experts] Re: Example 3: Why current ReaderInterceptors are cool.

Bill Burke 04/27/2012

[jsr339-experts] Re: Example 3: Why current ReaderInterceptors are cool.

Marek Potociar 04/27/2012

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

Bill Burke 04/26/2012

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

Marek Potociar 04/27/2012

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

Bill Burke 04/27/2012
 
 
Close
loading
Please Confirm
Close