Adding some of the evaluation comments from my emails earlier.
On 12/06/2011 02:31 PM, Sergey Beryozkin wrote:
> Hi,
>
> We have discussed various options to do with the way Filter signatures should look like. Simplicity and extensibility
> have been two major topics during those discussions.
>
> Here is a brief overview of these options.
>
> 1. Filter implementation calls on FilterChain in order to indicate what it is willing to do with the current request:
> let it continue, block it or suspend it. FilterChain returns an opaque interface instance such as NextAction which will
> let the JAX-RS runtime decide what to do next.
>
> Example:
>
> public class MyFilter implements RequestFilter {
>
> public NextAction preFilter(FilterContext context) {
>
> if (stopProcessing) {
> ResponseBuilder rb = context.getResponseBuilder();
> return context.stop(rb.status(401).build())
> } else if (suspendProcessing) {
> return context.suspend(...);
> } else {
> RequestBuilder rb = context.getRequestBuilder();
> return context.continue(rb.setSomething().build());
> }
>
> }
> }
>
>
> Pros:
> FilterContext is effectively a read-only instance, developers need to 'return' it as opposed to saving it at the
> context itself.
Keeps the call stack small (good because JVM does not support tail call optimization (TCO) which does not seem to
change any time soon).
Non-blocking, parallel or asynchronous filter execution strategies are possible and easy to add extensions. This
enables high-performance implementations of execution chain leveraging the nature of multi-core environments.
Bytecode-optimization-friendly design
> Cons:
> The opaqueness of NextAction may cause questions like 'why it is here if we can't do anything with it' ?
>
> 2. Filter implementation calls on FilterChain in order to indicate what it is willing to do with the current request:
> let it continue, block it or suspend it. No response type is available in the signature
>
> Example:
>
> public class MyFilter implements RequestFilter {
>
> public void preFilter(FilterContext context) {
>
> if (stopProcessing) {
> ResponseBuilder rb = context.getResponseBuilder();
> context.stop(rb.status(401).build())
> } else if (suspendProcessing) {
> context.suspend(...);
> } else {
> RequestBuilder rb = context.getRequestBuilder();
> context.continue(rb.setSomething().build());
> }
> }
> }
>
> This may be possibly optimized/improved.
>
> Pros:
> Developers are familiar with filter signatures having no response type, example servlet filters.
> Cons:
> I believe there's a concern about possible bugs (someone forgets to do final else for example) and the possibility of
> having few empty returns.
Looks like wrapping style, but is non wrapping, which is confusing. May lead to errors with users trying to do some
post-processing logic after context.[actionMethod] is invoked.
>
> 3. Filter implementation only calls on FilterChain to get the information it needs in order to decide what to do with
> the current request: let it continue or block it or suspend it. It returns Response to indicate the request is blocked;
> this can be null meaning that the the request is OK to continue
>
> Example:
>
> public class MyFilter implements RequestFilter {
>
> public Response preFilter(FilterContext context) {
>
> if (stopProcessing) {
> return context.getResponseBuilder().status(401).build();
> } else if (suspendProcessing) {
> context.suspend(...);
> // possibly 'return null'
> } else {
> RequestBuilder rb = context.getRequestBuilder();
> context.setRequest(rb.setSomething().build());
> return null;
> }
> }
> }
>
> Pros:
> FilterContext represents the current request and the filter implementation only updates it when it has something to
> change on this request; if it does not then it returns a blocking Response
Simple to implement and sufficient for the current (basic) set of requirements.
> Cons:
> Returning 'null' is not the best pattern and some users may dislike it.
> Concerns about the extensibility - FilterChain may offer an option to query for specific extensions.
Returning a *response* from a *request* filter raises concerns about the design.
>
> 4. The current trunk version
>
> public class MyFilter implements RequestFilter {
>
> public FilterAction preFilter(FilterContext context) {
>
> if (stopProcessing) {
> ResponseBuilder rb = context.getResponseBuilder();
> context.setResponse(rb);
> return FilterAction.STOP;
> } else if (suspendProcessing) {
> context.suspend(...);
> return FilterAction.SUSPEND;
> } else {
> RequestBuilder rb = context.getRequestBuilder();
> context.setRequest(rb.setSomething().build());
> return FilterAction.NEXT;
> }
> }
> }
>
Addition from Bill Burke:
>
> 5. Any implementation (1-4), but its implemented as one java call stack, rather than a for-loop. FilterContext would
> have a proceed() method which a Request(or Response) filter would have to execute to proceed to the next filter in the
> chain.
>
> Pros:
> - Consistent with MBW/MBR handler
> - Previously executed filters can perform logic based on the state of the filter chain, i.e. If an exception is thrown,
> catch it and maybe even recover. If suspended or an exception thrown, maybe they need to do cleanup
> - You don't ever need to add another callback method to the RequestFilter interface if down the line users want to be
> notified of exceptions, suspensions, etc...
>
> Cons:
> - You really aren't wrapping around anything.
- large filter chains produce large stacks (no TCO in Java = performance penalty)
- filters are supposed to be stateless, so it's not clear what should be part of the clean-up or reconciliation tasks
- mixes filter and exception mapper concepts together, adds one more way of doing the same thing
- makes async, parallel and non-blocking execution strategies very hard to implement
>
> One extra consideration about the extensions. Ultimately, the job of the filter is to block or modify the request.
That's not complete. Observer is also a type of filter.
> Suspending the request is about getting the filter prepared for what it is supposed to do: block or modify the request.
> Perhaps that may let us decide that making suspending the requests possible from within filters should not affect
> FilterContext except for introducing an optional generic extension query handler
Let's assume you have a following request filter chain:
Observer filter F1 reads content
Observer filter F2 reads header H1
Observer filter F3 reads header H2, involves expensive calls (e.g. I/O to external systems)
Suppose you have a request like this that is coming on a slow connection:
GET /foo HTTP/1.1
... some headers ...
H2
... some headers ...
H1
... some headers ...
...content...
Without any extensions the whole filter chain needs to block on I/O operations in F1 until the content is fully read and
processed. This is a significant and obviously unnecessary performance penalty given the fact that all three filters
could have been executed in parallel, using non-blocking I/O and by the time the F1 and F2 are finished it is likely
that the expensive invocation of F3 has already finished too.
Now I am not saying that we should address such scenarios right away or that we should expose our users to any complex
implementation details around fork-join or any other execution strategy. But this is just to demonstrate what we may
expect to deal with in the future, esp. if we, as implementors, want to leverage the parallel/async execution to gain
performance boosts. That should help us to produce design flexible enough to support these types of scenarios. If all
that we have to do right now is to convert an existing enum into an opaque interface and add bunch of action methods
into the FilterContext, then I wonder why don't we just do it?
Marek
>
> Thanks, Sergey