directory-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alex Karasulu" <akaras...@apache.org>
Subject Re: Interceptors optimizations
Date Wed, 04 Apr 2007 18:23:15 GMT
Hi Emmanuel,

On 4/4/07, Emmanuel Lecharny <elecharny@gmail.com> wrote:
>
> Hi,
>
> just a few questions while I'm browsing the code :
>
> 1) Bind Operation :
> when we do a Bind, we call the ServerContext() constructor, where the code
> looks like :
> ...
>     protected ServerContext(DirectoryService service, Hashtable env)
> throws NamingException
>     {
>         this.service = service;
>
>         // set references to cloned env and the proxy
>         this.nexusProxy = new PartitionNexusProxy( this, service );
>
>         DirectoryServiceConfiguration cfg = service.getConfiguration();
>         this.env = ( Hashtable ) cfg.getEnvironment().clone();
>         this.env.putAll( env );
>         LdapJndiProperties props =
> LdapJndiProperties.getLdapJndiProperties( this.env );
>         dn = props.getProviderDn();
>
>         // need to issue a bind operation here
>         this.nexusProxy.bind( dn );
>
>         if ( ! nexusProxy.hasEntry( dn ) )
>         {
>             throw new NameNotFoundException( dn + " does not exist" );
>         }
>
>         if ( dn.size() == 0 )
>         {
>             return;
>         }
> }
>
> The last test is totally useless,


Yeah looks like it was a vestigial structure and something below it might
have been removed.

but I also think that the previous one is not needed : if we successfully
> bound, then the dn must exist in the server.


No not necessarily.  The binding is done using the principalDN which must be
present to bind. However for JNDI sake you need to
check and see if the DN to bind to in the PROVIDER_URL exists.  If you don't
do this check here there may be some problems encountered in processing to
follow.

My guess is that the check is just because we didn't updated the Authent
> cache after a modification, leading to pb when binding on a modified user.
> Am I wrong ?


Yeah I think you're confusing the DN from the provider URL with the
principal DN.

2) In SimpleAuthenticator (and it might be true in all the authenticators),
> we call the lookupPassword method, to grab the password from the backend.
> This is correct, but why don't we simply avoid going through all the
> interceptors? We are still passing through referralService and
> exceptionService, and I think this is not necessary. Is there something I'm
> missing ?


Yeah this can be avoided using a collection of bypass instructions on the
proxy.  As you may already know you can each proxy method has two overloads:
one that does not bypass and one with an additional argument for the
interceptors to bypass.

However be careful when bypassing interceptors because you have to think of
all the possibilities.  Like for example future functionality for
vitualization might map users to somewhere else or call stored procedures or
trigger some event.  So select what you do bypass carefully.  I say this
because I've been screwed several times where I bypassed a couple
interceptors thinking they were unnecessary and had to deal with a
nightmarish debugging sessions to figure out why my interceptor was not
getting called until I discovered that I bypassed it.

In general the rule I use for myself is use bypass instructions very
sparingly.  If you must bypass something make sure you know definitely and
if there is some question about it don't bypass at all.  This helps me a
lot.

3) In ExceptionService, lookup operation, we  have this code :
> ...
>        // check if entry to lookup exists
>         String msg = "Attempt to lookup non-existant entry: ";
>         assertHasEntry( nextInterceptor, msg, ctx.getDn() );
>
>         return nextInterceptor.lookup( lookupContext );
> }
>
> Is there a reason why we simply don't catch the exception from the last
> line to generate a LdapNameNotFoundException (the assertHasEntry is throwing
> this exception if the entry does not exist)


Hmmmm I think I did this to protect lower layers and centralize where we do
the check for non-existant entries.  The idea way back in my head for
creating this ExceptionService was to centralize a number of common checks
performed so they were not duplicated in every interceptor downstream and
only done once in one place.

More generally, for internal calls, like lookup( password ), why don't we
> just directly send them to the backend ? Am I missing something ?


Well other interceptors may modify things and bypassing the interceptor
chain would prevent their influence on the operation.  Let me give a clear
example WRT the lookup() operation.  Let's say you have replication
enabled.  As you know mitosis does not delete deleted entries but marks them
as deleted.  This btw is called a tomestoned entry in LDAP replication
jargon.  So the mitosis interceptor basically responds to an attempt to
lookup an entry that is tomestoned with a entry does not exist response.  If
it did not do this then tomestoned entries would still be visible.  So you
cannot simply bypass all interceptors.  Even if you knew bypassing the
interceptors would be OK (which I unfortunately did several times and have
to fix) you need to keep in mind that users can add their own application
specific interceptors outside of the default set which you are not aware
of.  And you cannot begin to presume what they can do.  This btw is why the
bypass collection on proxy operations lists the interceptors to bypass
instead of listing the interceptors to actually includes.

HTH,
Alex

Mime
View raw message