On 29/11/11 13:43, Marek Potociar wrote:
> 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).
>
returning null to indicate the chain can continue is not cool, agreed,
but is easy to grasp IMHO
> 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);
> }
> }
>
Why should users suffer like this ? Just for them to anticipate some
future non-main-stream enhancements like the support for the suspended
invocations from filters ?
> 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.
>
I'd propose introducing a SuspendedInvocation exception for filter
developers to be able to indicate it instead of making it really complex
Sergey
> 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
>>
>
>
--
Sergey Beryozkin
Talend Community Coders
http://coders.talend.com/
Blog: http://sberyozkin.blogspot.com