On Thu, Oct 13, 2011 at 5:01 PM, Emmanuel Lecharny <elecha...@gmail.com>wrote:
> On 10/13/11 3:16 PM, Göktürk Gezer wrote:
>
>> Hi Emmanuel,Alex
>>
>> I would like to add something to discussion.
>> More inline...
>>
>> On Thu, Oct 13, 2011 at 3:23 PM, Alex Karasulu<akaras...@apache.org>
>> wrote:
>>
>>
>>> On Thu, Oct 13, 2011 at 3:05 PM, Emmanuel Lécharny<elecha...@apache.org>
>>> **wrote:
>>>
>>> On 10/13/11 1:44 PM, Alex Karasulu wrote:
>>>>
>>>> On Thu, Oct 13, 2011 at 2:20 PM, Emmanuel Lécharny<
>>>>> elecha...@apache.org>
>>>>> **wrote:
>>>>>
>>>>>
>>>>> Hi,
>>>>>
>>>>>> Göktük asked if there is a way to transform Interceptors to be bundles
>>>>>> instead of being statically loaded in core.
>>>>>>
>>>>>> This can be case yes, but i actually wanted them for further
>>>>>
>>>> interceptors. We can keep core interceptors in place. Don't need to
>> separate
>> them all. Lets keep core interceptors in apacheds-core, and protocol
>> specific interceptors in their protocol implementation. And then any other
>> interceptor that comes and registers itself through OSGI can be attached
>> to
>> DirectoryService's interceptor list.
>>
>
> I'd rather decouple the whole thing. There is no reason for core to depend
> on Interceptors.
>
I'm in agreement here as well. Even if this is done as an exercise it will
clean up a lot of ugly coupling crust resident in the code. It's a bit
extreme but it has value and maybe will allow replacement of core
functionality easily down the line. It gives us more options in the end.
>
>
>> I tried to play around the idea yesterday in the train, and I faced some
>>>>>> interesting challenges.
>>>>>>
>>>>>> o First, many interecptors are doing calls to the chain again, but
>>>>>> with
>>>>>> a
>>>>>> restricted set of interceptors. For instance, in the
>>>>>> SchemaInterceptor,
>>>>>> we
>>>>>> go through the chain again when modifying the schema itself. In order
>>>>>> to
>>>>>> speedup the operation, we declare a BYPASS sets of interceptors (I'm
>>>>>> not
>>>>>> sure it's a good idea, but right now, this is how we proceed). At the
>>>>>> end,
>>>>>> this BYPASS set is declared this way :
>>>>>>
>>>>>> private static final Collection<String> BYPASS;
>>>>>>
>>>>>> static
>>>>>> {
>>>>>> Set<String> c = new HashSet<String>();
>>>>>> c.add( AuthenticationInterceptor.******class.getName() );
>>>>>> c.add( AciAuthorizationInterceptor.******class.getName() );
>>>>>> c.add( DefaultAuthorizationIntercepto******r.class.getName()
>>>>>> );
>>>>>> c.add( ExceptionInterceptor.class.******getName() );
>>>>>> c.add( SchemaInterceptor.class.******getName() );
>>>>>> BYPASS = Collections.******unmodifiableCollection( c );
>>>>>>
>>>>>>
>>>>>> }
>>>>>>
>>>>>> As we can see, it creates a static dependency on interceptors. It
>>>>>> might
>>>>>> be
>>>>>> a better idea to use logical names instead of class names, and let the
>>>>>> OSGi
>>>>>> container retrieve the classes itself.
>>>>>>
>>>>>>
>>>>>> This is a good idea. How about going a little further and having a
>>>>>> set
>>>>>>
>>>>> of
>>>>> interceptor chain re-entry constants or set of enum values like:
>>>>>
>>>>> ReEntry.NO_AUTHENTICATION
>>>>> ReEntry.NO_AUTHORIZATION
>>>>> ReEntry.NO_ERROR_CHECKING
>>>>> ReEntry.NO_SCHEMA_CHECKING
>>>>>
>>>>> etc ...
>>>>>
>>>>> This is like saying we do not need authentication, authorization,
>>>>> additional
>>>>> exception handling and checks or schema checking on re-entry instead of
>>>>> having a direct list of interceptors to avoid.
>>>>>
>>>>> That's a good idea.
>>>>
>>>> One thing that might be problematic though is that we have no idea which
>>>> interceptors are going to be present in the chain, so we may be unable
>>>> to
>>>> tell the chain not to use the interceptors added on the fly (for
>>>> instance,
>>>> the logger interceptor).
>>>>
>>>>
>>>> Good point. Perhaps this is where we can have some kind of generic
>>> property
>>> that states whether or not by default on re-entry the interceptor should
>>> be
>>> included or excluded. There's a system default, say exclude by default
>>> always. Then the interceptor might override this with some class property
>>> like excludeOnReentry?
>>>
>>> This way even though the IC does not know which interceptors are present
>>> it
>>> can react accordingly on reentry. So for this logger interceptor example
>>> it
>>> might have excludeOnReentry set to false in which case it will always be
>>> included when present which makes send. We would not add the interceptor
>>> if
>>> we did not want to log reentrant invocations.
>>>
>>
> IMO, the best would be to declare sets of (I) we should go through, instead
> of sets of (I) we should bypass. This way, we will be able to know what is
> being executed, and we won't provide a way for users to pollute the internal
> executions of operations (keep in mind that those internal operation are
> themselves called by other operations).
>
> We can also declare those sets in the configuration, for each operation, so
> if we want to allow someone to modify the execution order, it's still
> possible to do so. (it can be done later though).
>
Perhaps we should break this out and discuss this in a separate thread. The
reason historically for listing what you should not execute was a poor
attempt to decouple and well you just don't know what extra interceptors you
have.
I have some ideas here that I also want to think through as well then post.
> <snip/>
>>>
>>> The Interceptors themselves each have a configuration. In this
>>> configuration the Interceptor should expose what aspects it participates
>>> in.
>>> For example FooInterceptor might expose that it participates in the
>>> authentication aspect. This way the IC knows for example in a schema
>>> modification operation that causes re-entry to occur, this aspect is
>>> excluded say during a modify operation since the session is already
>>> authenticated (no need to perform this twice or on each re-entry into the
>>> IC). The FooInterceptor will be bypassed in this case.
>>>
>>> For all the discussions above i may suggest using OSGI service
>> properties.
>> So we don't statically reference the specific interceptor class, we just
>> get
>> them using OSGI with some filter like
>> (interceptor.type="**AuthenticationInterceptor"). By this way we don't
>> change
>> a code much but we handle the decoupling.
>>
>
> I like that.
>
>
+1
>
>
>>
>>>
>>> There can also be other hint mechanisms given to the interceptor chain
>>>>> so
>>>>> it
>>>>> can correctly asses which interceptors to include or exclude on
>>>>> re-entry.
>>>>> For example there could be properties exposed for defaults on the
>>>>> interceptor telling the chain always exclude on re-entry etc. There
>>>>> should
>>>>> be some more thought put on this but the present situation as you state
>>>>> sucks where OSGi and pluggability is concerned.
>>>>>
>>>>> Right. We will try to get OSGi implemented anyway, and once it's done,
>>>> we
>>>> can start thinking about a better mechanism.
>>>>
>>> FYI, i just implemented dynamic Interceptor loading using IPojo. Its a
>>>
>> starter implementation. We can improve it using the ideas those come out
>> from that mail. But you must now handling interceptor dynamism does not
>> solved the problems. Now i've to solve problems about concurrency.
>> java.util.concurrent classses are acting weird under OSGI.
>>
>
> Let's think about it when we are done with the decoupling. We first have to
> clean up the place before starting building up something new, otherwise we
> might build some castle on sand...
>
Sounds good.
--
Best Regards,
-- Alex
