Re: Some code suggestions...

[Emmanuel LÃcharny <elecharny@gmail.com>]

Le 5/30/13 4:19 AM, Shawn McKinney a Ãcrit :
>
> On 05/25/2013 04:24 PM, Emmanuel Lecharny wrote:
>> Hi Shawn,
>>
>> I will use this mailing list for comments, suggestions and questions
>> regarding the code base, so that it will benefit to everyone interested.
>>
>> I have a couple of suggestions I already mentionned in a chat, but it's
>> better to have them recorded here :
>>
> Hello Emmanuel.  Welcome to the fortress mailing list.
>
> On 05/25/2013 04:24 PM, Emmanuel Lecharny wrote:
>> 1) You may want ot use the LDAP connection pool to replace the pool you
>> are using. Mots of the LDAP API has an implementation of a pool, even
>> JNDI (but you don't want to suffer using it , do you ?)
>>
> Willing to consider.  Where can I find more information on the pool
> you recommend?

Depends on the API you are using.

- JNDI :
http://docs.oracle.com/javase/jndi/tutorial/ldap/connect/config.html.
Ok, don't get me started :)

- UnboundId :
https://www.unboundid.com/products/ldapsdk/docs/javadoc/com/unboundid/ldap/sdk/LDAPConnectionPool.html
Although using this API is a bit problematic, as its licence is not
exactly compatible with the Fortress licence : it offers three different
kind of licences - this is the WTF of the day...- GPL (and you don *not*
want to use this -, LGPL (same kind of problem, but to a lesser extend)
and a stupid UnboundID license (" TERMINATION. UnboundID reserves the
right to discontinue offering the SDK and to modify the SDK at any time
in its sole discretion. Notwithstanding anything contained in this
Agreement to the contrary, UnboundID may also, in its sole discretion,
terminate or suspend access to the SDK to You or any end user at any
time." In other words, get lost).

- OpenDJ LDAP SDK :
http://opendj.forgerock.org/opendj-ldap-sdk/apidocs/org/forgerock/opendj/ldap/ConnectionPool.html.
A decent alternative. I mean it. CDDL, so fully compatible with ASL 2.0,
as soon as you don't do too much with it (see
http://www.apache.org/legal/3party.html)

- Apache LDAP API :
http://directory.apache.org/api/gen-docs/latest/apidocs/org/apache/directory/ldap/client/api/LdapConnectionPool.html.
Ok, hee, I'm biased :-)

- Netscape API :
http://docs.oracle.com/cd/E19957-01/816-5618-10/netscape/ldap/util/ConnectionPool.html



>
> On 05/25/2013 04:24 PM, Emmanuel Lecharny wrote:
>> 2) It would be better to get the connection immediately before using it,
>> and to release it as soon as you are done with it. For instance, the
>> connection could be get just before calling the dataProvider and
>> released just after. Not really a huge improvment, but assuming that
>> you may have thousands of requests per second, this may become an
>> issue.
>>
>
> The current pattern followed in the Fortress DAO's is as follows:
>
> // inside any DAO method:
> try
> {
>     1. getAdminConnection();
>     2. build up the ldap attribute set or search filter
>     3. perform the ldap operation
> }
> catch (LDAPException e)
> {
>    // perform error handling
> }
> finally
> {
>    4. closeAdminConnection(ld);
> }
>
> This change you are suggesting is to delay opening the connection
> until the beginning of step 3?
Yes. Basically :

try
{
    1. build up the ldap attribute set or search filter
    2. getAdminConnection();
    3. perform the ldap operation

Now, it's questionable if the connection should be grab in (2) instead
in the method that performs the operation. The only rationnal I see is
for a non-admin connection to be used, but this is not the case in teh
DAO, AFAICS.

-- 
Regards,
Cordialement,
Emmanuel LÃcharny
www.iktek.com