My opinion on the topic is following:
Filters should only stop the filter chain processing when returning a response (or throwing an exception). I agree with
Bill on this one.
Making a filter to return a response as suggested by Sergey does not seem like a good design to me. I don't like the
idea that we would be making decisions based on whether a returned response is null or not. Also, as a request filter
developer, I would be very unhappy about the code that needs to return a null after updating the request. It just reads
plain ugly. But most importantly such design would be difficult to extend to support other types of actions in the
future (e.g. non-blocking filters that may require to programmatically return a "suspend" action and a resume callback
somehow).
I do still agree with Sergey that currently used FilterAction enum provides only very limited functionality. I would
propose to convert FilterAction to an opaque interface that is returned from the filter. Instances of the interface
would be produced by a call to the provided FilterContext instance (which would be made immutable), e.g.:
public class MyFilter implements RequestFilter {
public FilterAction preFilter(FilterContext context) {
// ... do some request processing
if (stopProcessing) {
return context.stop(response);
}
return context.continue(request);
}
}
This way we would solve the issue with the disconnected return enum value from the actual filter continuation.
Continuation (FilterAction) would be constructed all at once including the relevant continuation values. Definition of
continuation factory methods on the FilterContext would also enforce the proper set of the output data required for each
particular continuation. This solution is also easily extensible in the future - we would just need to make sure to
properly document that FilterContext must not be extended by the API users as it can be subject to extension in the future.
Marek
On 11/18/2011 05:36 PM, Santiago Pericas-Geertsen wrote:
>
> On Nov 18, 2011, at 11:11 AM, Bill Burke wrote:
>
>>
>>
>> On 11/18/11 10:30 AM, Santiago Pericas-Geertsen wrote:
>>>
>>> On Nov 18, 2011, at 10:21 AM, Bill Burke wrote:
>>>
>>>>
>>>>
>>>> On 11/18/11 10:14 AM, Santiago Pericas-Geertsen wrote:
>>>>> Hi Sergey,
>>>>>
>>>>> It's an interesting observation. However, I think there may be use cases in which you want to stop the chain _without_ producing a response.
>>>>>
>>>>> Suppose an app has 2 authentication filters (e.g. for different authentication mechanisms it supports): if the first filter authenticates the user, then there's no need to run the next filter but there's also no response to be set. This is why I think there's value in separating continuations from responses.
>>>>>
>>>>
>>>> A filter should not be able to stop the chain unless it is providing a response. How would a filter know whether stopping the chain was correct behavior without full knowledge of the whole filter stack?
>>>
>>> User defined filters have the least priority in the chain (by default). As a developer, I know which filters I installed, why shouldn't I be able to control the execution of my chain?
>>>
>>
>> Because its really bad programming practice and prone to serious head-scratching errors?
>
> I'm not sure about the bad programming part (this pattern has been uses successfully in other frameworks), but I do agree it can lead to errors. It's extra power with extra responsibility. Having said that, there's always value in simplifying things too, and Sergey's suggestion does get rid of FilterAction.
>
> Any other votes/opinions on the topic?
>
> -- Santiago
>