Commit da8f42dd authored by Brian Major's avatar Brian Major
Browse files

ac2 - Changed thread safety in LdapPersistence. Removed the returning of... 

ac2 - Changed thread safety in LdapPersistence.  Removed the returning of groups from the add and modify group persistence APIs
parent b12c2e12

projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/GroupPersistence.java

+2 −6
Original line number Diff line number Diff line
@@ -116,8 +116,6 @@ public interface GroupPersistence<T extends Principal> 
     *

     * @param group The group to create

     *

     * @return created group

     *

     * @throws GroupAlreadyExistsException If a group with the same ID already

     *                                     exists.

     * @throws TransientException If an temporary, unexpected problem occurred.
@@ -126,7 +124,7 @@ public interface GroupPersistence<T extends Principal> 
     * @throws GroupNotFoundException if one of the groups in group members or

     * group admins does not exist in the server.

     */

    Group addGroup(Group group)

    void addGroup(Group group)

        throws GroupAlreadyExistsException, TransientException,

               AccessControlException, UserNotFoundException,

               GroupNotFoundException;
@@ -149,14 +147,12 @@ public interface GroupPersistence<T extends Principal> 
     *

     * @param group The group to update.

     *

     * @return The newly updated group.

     *

     * @throws GroupNotFoundException If the group was not found.

     * @throws TransientException If an temporary, unexpected problem occurred.

     * @throws AccessControlException If the operation is not permitted.

     * @throws UserNotFoundException If owner or group members not valid users.

     */

    Group modifyGroup(Group group)

    void modifyGroup(Group group)

        throws GroupNotFoundException, TransientException,

               AccessControlException, UserNotFoundException;

projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/ConnectionPools.java

+33 −0
Original line number Diff line number Diff line
@@ -69,6 +69,8 @@ 


package ca.nrc.cadc.ac.server.ldap;



import java.util.Collection;

import java.util.Iterator;

import java.util.Map;



/**
@@ -81,10 +83,14 @@ public class ConnectionPools 


    private Map<String,LdapConnectionPool> pools;



    private long lastPoolCheck = System.currentTimeMillis();

    private boolean isClosed;



    public ConnectionPools(Map<String,LdapConnectionPool> pools, LdapConfig config)

    {

        this.pools = pools;

        this.config = config;

        isClosed = false;

    }



    public Map<String,LdapConnectionPool> getPools()
@@ -97,4 +103,31 @@ public class ConnectionPools 
        return config;

    }



    public long getLastPoolCheck()

    {

        return lastPoolCheck;

    }



    public void setLastPoolCheck(long lastPoolCheck)

    {

        this.lastPoolCheck = lastPoolCheck;

    }



    public void close()

    {

        Collection<LdapConnectionPool> allPools = pools.values();

        Iterator<LdapConnectionPool> i = allPools.iterator();

        while (i.hasNext())

        {

            LdapConnectionPool next = i.next();

            next.shutdown();

        }

        isClosed = true;

    }



    public boolean isClosed()

    {

        return isClosed;

    }



}

projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapConnections.java

+18 −8
Original line number Diff line number Diff line
@@ -75,8 +75,6 @@ import ca.nrc.cadc.net.TransientException; 
import ca.nrc.cadc.profiler.Profiler;



import com.unboundid.ldap.sdk.LDAPConnection;

import com.unboundid.ldap.sdk.LDAPConnectionPool;

import com.unboundid.ldap.sdk.LDAPException;



/**

 * This class in the means by which the DAO classes obtain
@@ -129,10 +127,14 @@ class LdapConnections 
    {

        if (persistence != null)

        {

            if (readOnlyPool == null)

            {

                readOnlyPool = persistence.getPool(LdapPersistence.POOL_READONLY);

            }

            if (autoConfigReadOnlyConn == null)

            {

                log.debug("Getting new auto config read only connection.");

                autoConfigReadOnlyConn = persistence.getConnection(LdapPersistence.POOL_READONLY);

                autoConfigReadOnlyConn = readOnlyPool.getConnection();

                profiler.checkpoint("Get read only connection");

            }

            else
@@ -164,10 +166,14 @@ class LdapConnections 
    {

        if (persistence != null)

        {

            if (readWritePool == null)

            {

                readWritePool = persistence.getPool(LdapPersistence.POOL_READWRITE);

            }

            if (autoConfigReadWriteConn == null)

            {

                log.debug("Getting new auto config read write connection.");

                autoConfigReadWriteConn = persistence.getConnection(LdapPersistence.POOL_READWRITE);

                autoConfigReadWriteConn = readWritePool.getConnection();

                profiler.checkpoint("Get read write connection");

            }

            else
@@ -199,10 +205,14 @@ class LdapConnections 
    {

        if (persistence != null)

        {

            if (unboundReadOnlyPool == null)

            {

                unboundReadOnlyPool = persistence.getPool(LdapPersistence.POOL_UNBOUNDREADONLY);

            }

            if (autoConfigUnboundReadOnlyConn == null)

            {

                log.debug("Getting new auto config unbound read only connection.");

                autoConfigUnboundReadOnlyConn = persistence.getConnection(LdapPersistence.POOL_UNBOUNDREADONLY);

                autoConfigUnboundReadOnlyConn = unboundReadOnlyPool.getConnection();

                profiler.checkpoint("Get read write connection");

            }

            else
@@ -237,19 +247,19 @@ class LdapConnections 
            if (autoConfigReadOnlyConn != null)

            {

                log.debug("Releasing read only auto config connection.");

                persistence.releaseConnection(LdapPersistence.POOL_READONLY, autoConfigReadOnlyConn);

                readOnlyPool.releaseConnection(autoConfigReadOnlyConn);

                profiler.checkpoint("Release read only connection");

            }

            if (autoConfigReadWriteConn != null)

            {

                log.debug("Releasing read write auto config connection.");

                persistence.releaseConnection(LdapPersistence.POOL_READWRITE, autoConfigReadWriteConn);

                readWritePool.releaseConnection(autoConfigReadWriteConn);

                profiler.checkpoint("Release read write connection");

            }

            if (autoConfigUnboundReadOnlyConn != null)

            {

                log.debug("Releasing unbound read only auto config connection.");

                persistence.releaseConnection(LdapPersistence.POOL_UNBOUNDREADONLY, autoConfigUnboundReadOnlyConn);

                unboundReadOnlyPool.releaseConnection(autoConfigUnboundReadOnlyConn);

                profiler.checkpoint("Release read only connection");

            }

        }

projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapGroupDAO.java

+8 −27
Original line number Diff line number Diff line
@@ -73,6 +73,7 @@ import java.security.Principal; 
import java.util.ArrayList;

import java.util.Collection;

import java.util.HashSet;

import java.util.LinkedList;

import java.util.List;

import java.util.Set;
@@ -96,8 +97,6 @@ import com.unboundid.ldap.sdk.AddRequest; 
import com.unboundid.ldap.sdk.Attribute;

import com.unboundid.ldap.sdk.DN;

import com.unboundid.ldap.sdk.Filter;

import com.unboundid.ldap.sdk.LDAPConnection;

import com.unboundid.ldap.sdk.LDAPConnectionPool;

import com.unboundid.ldap.sdk.LDAPException;

import com.unboundid.ldap.sdk.LDAPInterface;

import com.unboundid.ldap.sdk.LDAPResult;
@@ -113,7 +112,6 @@ import com.unboundid.ldap.sdk.SearchResultListener; 
import com.unboundid.ldap.sdk.SearchResultReference;

import com.unboundid.ldap.sdk.SearchScope;

import com.unboundid.ldap.sdk.controls.ProxiedAuthorizationV2RequestControl;

import java.util.LinkedList;



public class LdapGroupDAO<T extends Principal> extends LdapDAO

{
@@ -133,7 +131,6 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO 
    };



    private Profiler profiler = new Profiler(LdapDAO.class);

    LdapConnections connections;



    private LdapUserDAO<T> userPersist;
@@ -164,7 +161,7 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO 
     * @throws UserNotFoundException If owner or a member not valid user.

     * @throws GroupNotFoundException

     */

    public Group addGroup(final Group group)

    public void addGroup(final Group group)

        throws GroupAlreadyExistsException, TransientException,

               UserNotFoundException, AccessControlException,

               GroupNotFoundException
@@ -187,10 +184,9 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO 


        try

        {

            Group newGroup = reactivateGroup(group);

            if ( newGroup != null)

            if (reactivateGroup(group))

            {

                return newGroup;

                return;

            }

            else

            {
@@ -213,22 +209,6 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO 
                                  group.getGroupAdmins());

                LdapDAO.checkLdapResult(result.getResultCode());



                // AD: Search results sometimes come incomplete if

                // connection is not reset - not sure why.



                // BM: commented-out this workout with introduction

                // of connection pools.  Reconnecting within a pool

                // causes an error.



                //getReadWriteConnection().reconnect();

                try

                {

                    return getGroup(group.getID());

                }

                catch (GroupNotFoundException e)

                {

                    throw new RuntimeException("BUG: new group not found");

                }

            }

        }

        catch (LDAPException e)
@@ -301,7 +281,7 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO 
     * @throws TransientException

     * @throws GroupAlreadyExistsException

     */

    private Group reactivateGroup(final Group group)

    private boolean reactivateGroup(final Group group)

        throws AccessControlException, UserNotFoundException,

        TransientException, GroupAlreadyExistsException

    {
@@ -323,7 +303,7 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO 


            if (searchResult == null)

            {

                return null;

                return false;

            }



            if (searchResult.getAttributeValue("nsaccountlock") == null)
@@ -334,7 +314,8 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO 
            // activate group

            try

            {

                return modifyGroup(null, group, true);

                modifyGroup(null, group, true);

                return true;

            }

            catch (GroupNotFoundException e)

            {

projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapGroupPersistence.java

+4 −8
Original line number Diff line number Diff line
@@ -74,8 +74,6 @@ import java.util.Collection; 


import org.apache.log4j.Logger;



import com.unboundid.ldap.sdk.LDAPException;



import ca.nrc.cadc.ac.Group;

import ca.nrc.cadc.ac.GroupAlreadyExistsException;

import ca.nrc.cadc.ac.GroupNotFoundException;
@@ -149,7 +147,7 @@ public class LdapGroupPersistence<T extends Principal> extends LdapPersistence i 
        }

    }



    public Group addGroup(Group group)

    public void addGroup(Group group)

        throws GroupAlreadyExistsException, TransientException,

               AccessControlException, UserNotFoundException,

               GroupNotFoundException
@@ -161,8 +159,7 @@ public class LdapGroupPersistence<T extends Principal> extends LdapPersistence i 
        {

            userDAO = new LdapUserDAO<T>(conns);

            groupDAO = new LdapGroupDAO<T>(conns, userDAO);

            Group ret = groupDAO.addGroup(group);

            return ret;

            groupDAO.addGroup(group);

        }

        finally

        {
@@ -189,7 +186,7 @@ public class LdapGroupPersistence<T extends Principal> extends LdapPersistence i 
        }

    }



    public Group modifyGroup(Group group)

    public void modifyGroup(Group group)

        throws GroupNotFoundException, TransientException,

               AccessControlException, UserNotFoundException

    {
@@ -200,8 +197,7 @@ public class LdapGroupPersistence<T extends Principal> extends LdapPersistence i 
        {

            userDAO = new LdapUserDAO<T>(conns);

            groupDAO = new LdapGroupDAO<T>(conns, userDAO);

            Group ret = groupDAO.modifyGroup(group);

            return ret;

            groupDAO.modifyGroup(group);

        }

        finally

        {