Hi Emmanuel,

On 4/4/07, Emmanuel Lecharny <elecharny@gmail.com> wrote:

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 )

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.