On Thu, Oct 13, 2011 at 5:01 PM, Emmanuel Lecharny <elecharny@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<akarasulu@apache.org>  wrote:


On Thu, Oct 13, 2011 at 3:05 PM, Emmanuel Lécharny<elecharny@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<elecharny@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