On 12/1/11 11:36 AM, Marek Potociar wrote:
>
>
> On 12/01/2011 04:38 PM, Bill Burke wrote:
>>
>>
>> On 12/1/11 10:01 AM, Marek Potociar wrote:
>>>
>>>
>>> On 11/30/2011 04:12 PM, Sergey Beryozkin wrote:
>>>> On 30/11/11 13:20, Bill Burke wrote:
>>>>>
>>>>>
>>>>> On 11/30/11 5:52 AM, Marek Potociar wrote:
>>>>>>
>>>>>> public interface NextAction { }
>>>>>>
>>>>>> IMO it should be defined as an inner interface of FilterContext. We
>>>>>> can add some methods if we find it useful. Otherwise
>>>>>> it should be up to the implementation how to implement the interface
>>>>>> and what methods should be internally provided.
>>>>>
>>>>> Yuck. -1 on a special interface. What about my proposal with filter
>>>>> returning void, and FilterContext having a state or a set of boolean
>>>>> methods that describe the state:
>>>>>
>>>>> void preFilter(FilterContext ctx) {
>>>>> context.stop();
>>>>> }
>>>>>
>>>>> ctx.isStopped();
>>>>>
>>>> +1 to having 'void' only for this filter api option. I'd prefer
>>>> FilterContext having no stop/continue/suspend at all.
>>>> FilterContext.setResponse is a good enough way to indicate to the runtime that this has provided a response of its own
>>>> or not
>>>
>>> IMHO, FilterContext.setResponse(...) is a very cryptic way of saying that the request filter chain execution is stopped
>>> and the response processing should be resumed. It's certainly not immediately clear that this operation breaks the
>>> filter chain.
>>>
>>
>> BTW, I think it might be a bit cleaner if instead of:
>>
>> for (RequestFilter filter : filters)
>> {
>> if (filter.preFilter(ctx) == STOP) break;
>> }
>>
>> You did one big call stack for each pre/post filter chain. (As you do with MBR/MBW).
>>
>> FilterContext {
>> void proceed();
>> }
>>
>> Otherwise you're gonna end up adding a bunch of callbacks like
>>
>> Filter {
>> void aborted(FilterContext);
>> void suspended(FilterContext);
>>
>> ...
>>
>> }
>>
>> Following me? With this model its easier for a filter to clean up after itself if the request was aborted. Exception
>> handling is simpler. A filter can control whether or not the chain is continued by calling proceed(). Overall there's
>> a lot less you need to add to the interface.
>
> If I read you correctly, what you propose is to turn non-wrapping filters into wrapping ones, right? I wonder why would
> we want to do this. It would make some sense if the filters were at the same time request and response filters. Not to
> mention complications of supporting asynchronous client/server processing and potential performance related issues. I've
> seen this approach fail miserably in Metro few years back when we decided to switch from wrapping Pipes to non-wrapping
> Tubes.
>
> Also I don't fully follow the clean-up part. Aren't we assuming filters are stateless singleton providers? I hope we are
> (at least by default).
>
> As for exception processing, let's keep the concerns separated here - we already have ExceptionMapper in place to deal
> with exceptions.
>
Again, if a previously invoked RequestFilter wants to know if a request
is aborted/stopped, you would need to have a callback added to the
API....OR.... just have one callstack. If a previously invoked
RequestFilter wants to know about an exception thrown by the current
RequestFilter, then this is another thing you'd have to add to the
interface...OR...just ahve one callstack.
With one callstack, you can go to the Response return type model that
Sergey wants and still have full control over flow. API is simpler, etc...
Bill
--
Bill Burke
JBoss, a division of Red Hat
http://bill.burkecentral.com