directory-api mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Matthew Swift <Matthew.Sw...@Sun.COM>
Subject Re: [DN] Existing API review
Date Wed, 13 Jan 2010 11:24:30 GMT
Hi Emmanuel,


Thanks for doing this (happy new year by the way!). I've put my 
responses inline...

On 12/01/10 14:29, Emmanuel Lecharny wrote:
> Hi,
>
> yesturday, I conducted a review on the existing APIs for the DN class :
> - Name/LdapName (JNDI) (I will call it JNDI)
> - jLdap/LdapSdk (I will call it LSD)
> - ApacheDS (ADS)
> - OpenDS (ODS)
> - UnbounID (UID)
>
> There are some important differences. There are two sets of API,
> depending on the existence of a constructor, or not.
>
> - API having a constructor :
> JNDI,
> ADS,
> UID
>
> - API not having a constructor :
> LSD,
> ODS (ODS uses a static method valueof() to produce a DN)
>
> IMHO, I really think that users prefer having a constructor over none or
> a static factory. There are three rasons for that :
> 1) Most of LDAP users are using the JNDI API, which has a LdapNAME()
> constructor

Agreed.

> 2) In Java, it's pretty natural to create objects through constructor.


This may be true for constructors which compose the constructor's 
parameters (e.g. list of RDNs or RDN+DN) into a new object (DN), but it 
is definitely not true for constructors which perform parsing or type 
conversions. There are very many examples in J2SE:

    * String.valueOf(...) -converting objects to Strings

    * Integer.valueOf(int) - converting an int to an Integer

    * Integer.valueOf(String) - parsing a String as an Integer

    * plus other primitive Objects (Boolean, Byte, Char, Float, ...)


In fact, the use of static factory methods is a very common design idiom 
and is strongly recommended in many texts including Joshua Bloch's 
"Effective Java" (item #1). By using static factories with well known 
names (e.g. valueOf) we are able to avoid creating duplicate objects 
(item 4) if this proves useful (i.e. through the use of a cache) and 
also enforce the singleton property for the root DN (item #2).


> 3) I'm not sure that handling a cache for DN is a valuable trick, as it
> leads to some contention and the need to manage this cache in a
> muli-threaded environement (this has to be carefully evaluated)
>

I have done already and it results in a 30-40% parsing performance 
improvement as well as a significant reduction in garbage. Having said 
that, I'm not totally convinced that this gain is really worth the extra 
complexity and old generation memory usage - especially considering 
large server based applications may have 1000s of threads - e.g using 
LDAP from within a servlet - all those thread local caches could use a 
significant amount of memory!

Our implementation uses a small thread/schema local cache mapping String 
values to DNs. In most applications I think that it's reasonable to 
assume that 99% (perhaps more) DNs share the same parent or grand 
parent. When parsing a DN we first check if the string is in the cache 
and, if not, parse the RDN and recursively repeat for the parent DN (in 
the SDK a DN is implemented recursively as an RDN+DN).


> I think that we should have some basic constructors, and if the cache is
> proven to be valuable, then we can extend the API by adding the valueof(
> ... ) method.
>


Even if you decide that caching is not required then that's no reason to 
develop an API which prevents you from implementing one in future. Using 
a normal constructor prevents the use of a cache (or forces you to use 
the pimpl idiom). If you extend the API later by adding a valueOf method 
then no existing applications will be able to take advantage of the perf 
improvement unless they are modified to use the new static factory method.

Incidentally, I'm not sure how representative of a typical LDAP 
application OpenDS is, but I found that the String based constructor 
(valueOf) is by far the most heavily used (2000 references) compared 
with <20 references for the others. The most references occur in the 
unit tests which are probably the most "client like" part of the server.


> The base constructor we can have are probably something like:
> DN()
> DN(String dnStr)
> DN( RDN... rdns)
> DN( RDN rdn, DN parent)
>
> Thoughts ?
>


I like the DN( RDN rdn, DN parent) constructor - we support this via an 
instance method called DN.child(RDN). I think I prefer the constructor 
approach since it is not clear from our "child" method whether or not it 
modifies this DN or creates a new DN.  A constructor is more obvious. 
You may want to have a concatenation constructor DN(DN child, DN parent) 
for constructing DNs of entries within a subtree using a relative DN (or 
"local name").

One thing that is a bit tricky is whether or not the API should order 
RDN parameters in little-endian (LDAP World) order or big-endian 
(everyone else outside of LDAP) order. I think first time users may be 
surprised by LDAP's unnatural little endian ordering.

Also, I strongly believe that DNs and RDNs and AVAs should be immutable 
objects (as well as any other low level API type). What do you think?

Also, on the subject of AVAs - we have the AVA type as an inner class in 
RDN. I'm not particularly happy with this this, but less happy with it 
being a standalone class since AVAs are only used in RDNs and may 
introduce confusion elsewhere. For example, filters also use attribute 
value assertions but these are not the same type of object as an AVA 
even if they have the same name. For example, AVAs (in RDNs) do not 
allow attribute options or matching rules to be specified.

What do you think? Inner class or standalone?

Matt


Mime
View raw message