On 12/01/2011 10:08 PM, Bill Burke wrote:
>
>
> 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.
Do you have a specific use case in mind? Can it be solved by some combination of request filter, response filter,
(wrapping) handler and exception mapper implementation?
>
> 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...
With one call stack you loose control over the threading model which makes any truly high-performance non-blocking or
concurrent processing support close to impossible.
Marek
>
> Bill
>