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.
Let me know if I misunderstood your proposal.
Marek
>
> Bill
>
>
>
>
>
>
>
>
>
>
>> Marek
>>
>>>
>>> Cheers, Sergey
>>>
>>>> BTW, We're starting to implement the spec next week. I'll probably have
>>>> a lot more comments when we start playing with things.
>>>>
>>>>
>>>
>>>
>