fixes for: https://issues.apache.org/jira/browse/AMQ-4555

git-svn-id: https://svn.apache.org/repos/asf/activemq/trunk@1486063 13f79535-47bb-0310-9956-ffa450edef68

activemq-broker/src/main/java/org/apache/activemq/security/SimpleCachedLDAPAuthorizationMap.java

@@ -16,37 +16,57 @@
  */
 package org.apache.activemq.security;
 
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.Hashtable;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.LinkedBlockingQueue;
+import java.util.concurrent.ThreadFactory;
+import java.util.concurrent.ThreadPoolExecutor;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicReference;
+
+import javax.naming.Binding;
+import javax.naming.Context;
+import javax.naming.InvalidNameException;
+import javax.naming.NamingEnumeration;
+import javax.naming.NamingException;
+import javax.naming.directory.Attribute;
+import javax.naming.directory.Attributes;
+import javax.naming.directory.DirContext;
+import javax.naming.directory.InitialDirContext;
+import javax.naming.directory.SearchControls;
+import javax.naming.directory.SearchResult;
+import javax.naming.event.EventDirContext;
+import javax.naming.event.NamespaceChangeListener;
+import javax.naming.event.NamingEvent;
+import javax.naming.event.NamingExceptionEvent;
+import javax.naming.event.ObjectChangeListener;
+import javax.naming.ldap.LdapName;
+import javax.naming.ldap.Rdn;
+
 import org.apache.activemq.command.ActiveMQDestination;
 import org.apache.activemq.command.ActiveMQQueue;
 import org.apache.activemq.command.ActiveMQTopic;
 import org.apache.activemq.filter.DestinationMapEntry;
-import org.apache.activemq.jaas.GroupPrincipal;
 import org.apache.activemq.jaas.UserPrincipal;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import javax.naming.*;
+public class SimpleCachedLDAPAuthorizationMap implements AuthorizationMap {
-import javax.naming.directory.*;
-import javax.naming.event.*;
-import javax.naming.ldap.LdapName;
-import javax.naming.ldap.Rdn;
-import java.util.*;
-
-/**
- */
-public class SimpleCachedLDAPAuthorizationMap extends DefaultAuthorizationMap {
 
     private static final Logger LOG = LoggerFactory.getLogger(SimpleCachedLDAPAuthorizationMap.class);
 
     // Configuration Options
-    private String initialContextFactory = "com.sun.jndi.ldap.LdapCtxFactory";
+    private final String initialContextFactory = "com.sun.jndi.ldap.LdapCtxFactory";
     private String connectionURL = "ldap://localhost:1024";
     private String connectionUsername = "uid=admin,ou=system";
     private String connectionPassword = "secret";
     private String connectionProtocol = "s";
     private String authentication = "simple";
 
-    
     private int queuePrefixLength = 4;
     private int topicPrefixLength = 4;
     private int tempPrefixLength = 4;
@@ -55,7 +75,6 @@ public class SimpleCachedLDAPAuthorizationMap extends DefaultAuthorizationMap {
     private String topicSearchBase = "ou=Topic,ou=Destination,ou=ActiveMQ,ou=system";
     private String tempSearchBase = "ou=Temp,ou=Destination,ou=ActiveMQ,ou=system";
 
-    
     private String permissionGroupMemberAttribute = "member";
 
     private String adminPermissionGroupSearchFilter = "(cn=Admin)";
@@ -68,7 +87,6 @@ public class SimpleCachedLDAPAuthorizationMap extends DefaultAuthorizationMap {
     private String groupNameAttribute = "cn";
     private String userNameAttribute = "uid";
 
-    
     private int refreshInterval = -1;
     private boolean refreshDisabled = false;
 
@@ -80,8 +98,27 @@ public class SimpleCachedLDAPAuthorizationMap extends DefaultAuthorizationMap {
     protected DirContext context;
     private EventDirContext eventContext;
 
-    protected HashMap<ActiveMQDestination, AuthorizationEntry> entries = 
+    private final AtomicReference<DefaultAuthorizationMap> map =
-            new HashMap<ActiveMQDestination, AuthorizationEntry>();
+        new AtomicReference<DefaultAuthorizationMap>(new DefaultAuthorizationMap());
+    private final ThreadPoolExecutor updaterService;
+
+    protected Map<ActiveMQDestination, AuthorizationEntry> entries =
+        new ConcurrentHashMap<ActiveMQDestination, AuthorizationEntry>();
+
+    public SimpleCachedLDAPAuthorizationMap() {
+        // Allow for only a couple outstanding update requests, they can be slow so we
+        // don't want a bunch to pile up for no reason.
+        updaterService = new ThreadPoolExecutor(1, 1, 0, TimeUnit.SECONDS,
+            new LinkedBlockingQueue<Runnable>(2),
+            new ThreadFactory() {
+
+                @Override
+                public Thread newThread(Runnable r) {
+                    return new Thread(r, "SimpleCachedLDAPAuthorizationMap update thread");
+                }
+            });
+        updaterService.setRejectedExecutionHandler(new ThreadPoolExecutor.DiscardPolicy());
+    }
 
     protected DirContext createContext() throws NamingException {
         Hashtable<String, String> env = new Hashtable<String, String>();
@@ -104,19 +141,20 @@ public class SimpleCachedLDAPAuthorizationMap extends DefaultAuthorizationMap {
             try {
                 context.getAttributes("");
                 alive = true;
-            } catch (Exception e) {}
+            } catch (Exception e) {
+            }
         }
         return alive;
     }
 
     /**
-     * Returns the existing open context or creates a new one and registers listeners for
+     * Returns the existing open context or creates a new one and registers listeners for push notifications if such an
-     * push notifications if such an update style is enabled.  This implementation should not
+     * update style is enabled. This implementation should not be invoked concurrently.
-     * be invoked concurrently.
      *
      * @return the current context
      *
-     * @throws NamingException if there is an error setting things up
+     * @throws NamingException
+     *             if there is an error setting things up
      */
     protected DirContext open() throws NamingException {
         if (isContextAlive()) {
@@ -126,7 +164,7 @@ public class SimpleCachedLDAPAuthorizationMap extends DefaultAuthorizationMap {
         try {
             context = createContext();
             if (refreshInterval == -1 && !refreshDisabled) {
-                eventContext = ((EventDirContext)context.lookup(""));
+                eventContext = ((EventDirContext) context.lookup(""));
 
                 final SearchControls constraints = new SearchControls();
                 constraints.setSearchScope(SearchControls.SUBTREE_SCOPE);
@@ -139,8 +177,8 @@ public class SimpleCachedLDAPAuthorizationMap extends DefaultAuthorizationMap {
                         this.new CachedLDAPAuthorizationMapNamespaceChangeListener(DestinationType.QUEUE, permissionType));
                 }
                 // Listener for changes to the destination pattern entry itself and not a permission entry.
-                eventContext.addNamingListener(queueSearchBase, "cn=*", new SearchControls(),
+                eventContext.addNamingListener(queueSearchBase, "cn=*", new SearchControls(), this.new CachedLDAPAuthorizationMapNamespaceChangeListener(
-                        this.new CachedLDAPAuthorizationMapNamespaceChangeListener(DestinationType.QUEUE, null));
+                    DestinationType.QUEUE, null));
 
                 // Listeners for Topic policy //
 
@@ -150,8 +188,8 @@ public class SimpleCachedLDAPAuthorizationMap extends DefaultAuthorizationMap {
                         this.new CachedLDAPAuthorizationMapNamespaceChangeListener(DestinationType.TOPIC, permissionType));
                 }
                 // Listener for changes to the destination pattern entry itself and not a permission entry.
-                eventContext.addNamingListener(topicSearchBase, "cn=*", new SearchControls(),
+                eventContext.addNamingListener(topicSearchBase, "cn=*", new SearchControls(), this.new CachedLDAPAuthorizationMapNamespaceChangeListener(
-                        this.new CachedLDAPAuthorizationMapNamespaceChangeListener(DestinationType.TOPIC, null));
+                    DestinationType.TOPIC, null));
 
                 // Listeners for Temp policy //
 
@@ -171,10 +209,11 @@ public class SimpleCachedLDAPAuthorizationMap extends DefaultAuthorizationMap {
     }
 
     /**
-     * Queries the directory and initializes the policy based on the data in the directory.
+     * Queries the directory and initializes the policy based on the data in the directory. This implementation should
-     * This implementation should not be invoked concurrently.
+     * not be invoked concurrently.
      *
-     * @throws Exception if there is an unrecoverable error processing the directory contents
+     * @throws Exception
+     *             if there is an unrecoverable error processing the directory contents
      */
     @SuppressWarnings("rawtypes")
     protected void query() throws Exception {
@@ -183,65 +222,72 @@ public class SimpleCachedLDAPAuthorizationMap extends DefaultAuthorizationMap {
         final SearchControls constraints = new SearchControls();
         constraints.setSearchScope(SearchControls.SUBTREE_SCOPE);
 
+        DefaultAuthorizationMap newMap = new DefaultAuthorizationMap();
         for (PermissionType permissionType : PermissionType.values()) {
             try {
-                processQueryResults(
+                processQueryResults(newMap,
-                        currentContext.search(queueSearchBase, getFilterForPermissionType(permissionType), constraints),
+                    currentContext.search(queueSearchBase, getFilterForPermissionType(permissionType),
-                        DestinationType.QUEUE, permissionType);
+                    constraints), DestinationType.QUEUE, permissionType);
             } catch (Exception e) {
-                LOG.error("Policy not applied!.  Error processing policy under '" + queueSearchBase + "' with filter '" 
+                LOG.error("Policy not applied!.  Error processing policy under '" + queueSearchBase +
-                        + getFilterForPermissionType(permissionType) + "'", e);
+                          "' with filter '" + getFilterForPermissionType(permissionType) + "'", e);
             }
         }
 
         for (PermissionType permissionType : PermissionType.values()) {
             try {
-                processQueryResults(
+                processQueryResults(newMap,
-                        currentContext.search(topicSearchBase, getFilterForPermissionType(permissionType), constraints),
+                    currentContext.search(topicSearchBase, getFilterForPermissionType(permissionType),
-                        DestinationType.TOPIC, permissionType);
+                    constraints), DestinationType.TOPIC, permissionType);
             } catch (Exception e) {
-                LOG.error("Policy not applied!.  Error processing policy under '" + topicSearchBase + "' with filter '" 
+                LOG.error("Policy not applied!.  Error processing policy under '" + topicSearchBase +
-                        + getFilterForPermissionType(permissionType) + "'", e);
+                          "' with filter '" + getFilterForPermissionType(permissionType) + "'", e);
             }
         }
 
         for (PermissionType permissionType : PermissionType.values()) {
             try {
-                processQueryResults(
+                processQueryResults(newMap,
-                        currentContext.search(tempSearchBase, getFilterForPermissionType(permissionType), constraints),
+                    currentContext.search(tempSearchBase, getFilterForPermissionType(permissionType),
-                        DestinationType.TEMP, permissionType);
+                    constraints), DestinationType.TEMP, permissionType);
             } catch (Exception e) {
-                LOG.error("Policy not applied!.  Error processing policy under '" + tempSearchBase + "' with filter '" 
+                LOG.error("Policy not applied!.  Error processing policy under '" + tempSearchBase +
-                        + getFilterForPermissionType(permissionType) + "'", e);
+                          "' with filter '" + getFilterForPermissionType(permissionType) + "'", e);
             }
         }
 
-        setEntries(new ArrayList<DestinationMapEntry>(entries.values()));
+        // Create and swap in the new instance with updated LDAP data.
+        newMap.setAuthorizationEntries(new ArrayList<DestinationMapEntry>(entries.values()));
+        this.map.set(newMap);
+
         updated();
     }
 
     /**
-     * Processes results from a directory query in the context of a given destination type and permission type.
+     * Processes results from a directory query in the context of a given destination type and permission type. This
-     * This implementation should not be invoked concurrently.
+     * implementation should not be invoked concurrently.
      *
-     * @param results the results to process
+     * @param results
-     * @param destinationType the type of the destination for which the directory results apply
+     *            the results to process
-     * @param permissionType the type of the permission for which the directory results apply
+     * @param destinationType
+     *            the type of the destination for which the directory results apply
+     * @param permissionType
+     *            the type of the permission for which the directory results apply
      *
-     * @throws Exception if there is an error processing the results
+     * @throws Exception
+     *             if there is an error processing the results
      */
-    protected void processQueryResults(NamingEnumeration<SearchResult> results,
+    protected void processQueryResults(DefaultAuthorizationMap map, NamingEnumeration<SearchResult> results, DestinationType destinationType, PermissionType permissionType)
-            DestinationType destinationType, PermissionType permissionType) throws Exception {
+        throws Exception {
 
         while (results.hasMore()) {
             SearchResult result = results.next();
             AuthorizationEntry entry = null;
 
             try {
-                entry = getEntry(new LdapName(result.getNameInNamespace()), destinationType);
+                entry = getEntry(map, new LdapName(result.getNameInNamespace()), destinationType);
             } catch (Exception e) {
-                LOG.error("Policy not applied!  Error parsing authorization policy entry under "
+                LOG.error("Policy not applied!  Error parsing authorization policy entry under " + result.getNameInNamespace(), e);
-                        + result.getNameInNamespace(), e);
                 continue;
             }
 
@@ -250,20 +296,23 @@ public class SimpleCachedLDAPAuthorizationMap extends DefaultAuthorizationMap {
     }
 
     /**
-     * Marks the time at which the authorization state was last refreshed.  Relevant for synchronous policy updates.
+     * Marks the time at which the authorization state was last refreshed. Relevant for synchronous
-     * This implementation should not be invoked concurrently.
+     * policy updates. This implementation should not be invoked concurrently.
      */
     protected void updated() {
         lastUpdated = System.currentTimeMillis();
     }
 
     /**
-     * Retrieves or creates the {@link AuthorizationEntry} that corresponds to
+     * Retrieves or creates the {@link AuthorizationEntry} that corresponds to the DN in {@code dn}. This implementation
-     * the DN in {@code dn}.  This implementation should not be invoked concurrently.
+     * should not be invoked concurrently.
      *
+     * @param map
+     *            the DefaultAuthorizationMap to operate on.
      * @param dn
      *            the DN representing the policy entry in the directory
-     * @param destinationType the type of the destination to get/create the entry for
+     * @param destinationType
+     *            the type of the destination to get/create the entry for
      *
      * @return the corresponding authorization entry for the DN
      *
@@ -271,7 +320,7 @@ public class SimpleCachedLDAPAuthorizationMap extends DefaultAuthorizationMap {
      *             if destination type is not one of {@link DestinationType#QUEUE}, {@link DestinationType#TOPIC},
      *             {@link DestinationType#TEMP} or if the policy entry DN is malformed
      */
-    protected AuthorizationEntry getEntry(LdapName dn, DestinationType destinationType) {
+    protected AuthorizationEntry getEntry(DefaultAuthorizationMap map, LdapName dn, DestinationType destinationType) {
         AuthorizationEntry entry = null;
         switch (destinationType) {
             case TEMP:
@@ -279,13 +328,12 @@ public class SimpleCachedLDAPAuthorizationMap extends DefaultAuthorizationMap {
                 if (dn.size() != getPrefixLengthForDestinationType(destinationType) + 1) {
                     // handle unknown entry
                     throw new IllegalArgumentException("Malformed policy structure for a temporary destination "
-                            + "policy entry.  The permission group entries should be immediately below the "
+                        + "policy entry.  The permission group entries should be immediately below the " + "temporary policy base DN.");
-                            + "temporary policy base DN.");
                 }
-                entry = getTempDestinationAuthorizationEntry();
+                entry = map.getTempDestinationAuthorizationEntry();
                 if (entry == null) {
                     entry = new TempDestinationAuthorizationEntry();
-                    setTempDestinationAuthorizationEntry((TempDestinationAuthorizationEntry) entry);
+                    map.setTempDestinationAuthorizationEntry((TempDestinationAuthorizationEntry) entry);
                 }
 
                 break;
@@ -295,8 +343,7 @@ public class SimpleCachedLDAPAuthorizationMap extends DefaultAuthorizationMap {
                 // handle regular destinations
                 if (dn.size() != getPrefixLengthForDestinationType(destinationType) + 2) {
                     throw new IllegalArgumentException("Malformed policy structure for a queue or topic destination "
-                            + "policy entry.  The destination pattern and permission group entries should be "
+                        + "policy entry.  The destination pattern and permission group entries should be " + "nested below the queue or topic policy base DN.");
-                            + "nested below the queue or topic policy base DN.");
                 }
 
                 ActiveMQDestination dest = formatDestination(dn, destinationType);
@@ -320,17 +367,19 @@ public class SimpleCachedLDAPAuthorizationMap extends DefaultAuthorizationMap {
     }
 
     /**
-     * Applies the policy from the directory to the given entry within the context of the provided
+     * Applies the policy from the directory to the given entry within the context of the provided permission type.
-     * permission type.
      *
-     * @param entry the policy entry to apply the policy to
+     * @param entry
-     * @param result the results from the directory to apply to the policy entry
+     *            the policy entry to apply the policy to
-     * @param permissionType the permission type of the data in the directory
+     * @param result
+     *            the results from the directory to apply to the policy entry
+     * @param permissionType
+     *            the permission type of the data in the directory
      *
-     * @throws NamingException if there is an error applying the ACL
+     * @throws NamingException
+     *             if there is an error applying the ACL
      */
-    protected void applyACL(AuthorizationEntry entry, SearchResult result,
+    protected void applyACL(AuthorizationEntry entry, SearchResult result, PermissionType permissionType) throws NamingException {
-            PermissionType permissionType) throws NamingException {
 
         // Find members
         Attribute memberAttribute = result.getAttributes().get(permissionGroupMemberAttribute);
@@ -348,13 +397,9 @@ public class SimpleCachedLDAPAuthorizationMap extends DefaultAuthorizationMap {
                 // Lookup of member to determine principal type (group or user) and name.
                 Attributes memberAttributes;
                 try {
-                    memberAttributes = context.getAttributes(memberDn, 
+                    memberAttributes = context.getAttributes(memberDn, new String[] { "objectClass", groupNameAttribute, userNameAttribute });
-                            new String[] {"objectClass", groupNameAttribute, userNameAttribute});
                 } catch (NamingException e) {
-                    LOG.error(
+                    LOG.error("Policy not applied! Unknown member " + memberDn + " in policy entry " + result.getNameInNamespace(), e);
-                            "Policy not applied! Unknown member " + memberDn
-                                    + " in policy entry "
-                                    + result.getNameInNamespace(), e);
                     continue;
                 }
 
@@ -368,10 +413,8 @@ public class SimpleCachedLDAPAuthorizationMap extends DefaultAuthorizationMap {
                         group = true;
                         Attribute name = memberAttributes.get(groupNameAttribute);
                         if (name == null) {
-                            LOG.error("Policy not applied! Group "
+                            LOG.error("Policy not applied! Group " + memberDn + "does not have name attribute " + groupNameAttribute + " under entry "
-                                    + memberDn
+                                + result.getNameInNamespace());
-                                    + "does not have name attribute "
-                                    + groupNameAttribute + " under entry " + result.getNameInNamespace());
                             break;
                         }
 
@@ -382,9 +425,8 @@ public class SimpleCachedLDAPAuthorizationMap extends DefaultAuthorizationMap {
                         user = true;
                         Attribute name = memberAttributes.get(userNameAttribute);
                         if (name == null) {
-                            LOG.error("Policy not applied! User "
+                            LOG.error("Policy not applied! User " + memberDn + " does not have name attribute " + userNameAttribute + " under entry "
-                                    + memberDn + " does not have name attribute "
+                                + result.getNameInNamespace());
-                                    + userNameAttribute + " under entry " + result.getNameInNamespace());
                             break;
                         }
 
@@ -398,14 +440,15 @@ public class SimpleCachedLDAPAuthorizationMap extends DefaultAuthorizationMap {
             }
 
             if ((!group && !user) || (group && user)) {
-                LOG.error("Policy not applied! Can't determine type of member "
+                LOG.error("Policy not applied! Can't determine type of member " + memberDn + " under entry " + result.getNameInNamespace());
-                        + memberDn + " under entry " + result.getNameInNamespace());
+            } else if (principalName != null) {
-            } else if (principalName != null){
+                DefaultAuthorizationMap map = this.map.get();
                 if (group && !user) {
                     try {
-                        members.add(createGroupPrincipal(principalName, getGroupClass()));
+                        members.add(DefaultAuthorizationMap.createGroupPrincipal(principalName, map.getGroupClass()));
                     } catch (Exception e) {
-                        NamingException ne = new NamingException("Can't create a group " + principalName + " of class " + getGroupClass());
+                        NamingException ne = new NamingException(
+                            "Can't create a group " + principalName + " of class " + map.getGroupClass());
                         ne.initCause(e);
                         throw ne;
                     }
@@ -418,18 +461,19 @@ public class SimpleCachedLDAPAuthorizationMap extends DefaultAuthorizationMap {
         try {
             applyAcl(entry, permissionType, members);
         } catch (Exception e) {
-            LOG.error(
+            LOG.error("Policy not applied! Error adding principals to ACL under " + result.getNameInNamespace(), e);
-                    "Policy not applied! Error adding principals to ACL under "
-                            + result.getNameInNamespace(), e);
         }
     }
 
     /**
      * Applies policy to the entry given the actual principals that will be applied to the policy entry.
      *
-     * @param entry the policy entry to which the policy should be applied
+     * @param entry
-     * @param permissionType the type of the permission that the policy will be applied to
+     *            the policy entry to which the policy should be applied
-     * @param acls the principals that represent the actual policy
+     * @param permissionType
+     *            the type of the permission that the policy will be applied to
+     * @param acls
+     *            the principals that represent the actual policy
      *
      * @throw IllegalArgumentException if {@code permissionType} is unsupported
      */
@@ -451,18 +495,20 @@ public class SimpleCachedLDAPAuthorizationMap extends DefaultAuthorizationMap {
     }
 
     /**
-     * Parses a DN into the equivalent {@link ActiveMQDestination}.  The default implementation
+     * Parses a DN into the equivalent {@link ActiveMQDestination}. The default implementation expects a format of
-     * expects a format of cn=<PERMISSION_NAME>,ou=<DESTINATION_PATTERN>,.... or 
+     * cn=<PERMISSION_NAME>,ou=<DESTINATION_PATTERN>,.... or ou=<DESTINATION_PATTERN>,.... for permission and
-     * ou=<DESTINATION_PATTERN>,.... for permission and destination entries, respectively.
+     * destination entries, respectively. For example {@code cn=admin,ou=$,ou=...} or {@code ou=$,ou=...}.
-     * For example {@code cn=admin,ou=$,ou=...} or {@code ou=$,ou=...}. 
      *
-     * @param dn the DN to parse
+     * @param dn
-     * @param destinationType the type of the destination that we are parsing
+     *            the DN to parse
+     * @param destinationType
+     *            the type of the destination that we are parsing
      *
      * @return the destination that the DN represents
      *
-     * @throws IllegalArgumentException if {@code destinationType} is {@link DestinationType#TEMP} or
+     * @throws IllegalArgumentException
-     * if the format of {@code dn} is incorrect for for a topic or queue
+     *             if {@code destinationType} is {@link DestinationType#TEMP} or if the format of {@code dn} is
+     *             incorrect for for a topic or queue
      *
      * @see #formatDestination(Rdn, DestinationType)
      */
@@ -477,24 +523,22 @@ public class SimpleCachedLDAPAuthorizationMap extends DefaultAuthorizationMap {
                 // on the destination type and the DN size.
                 if (dn.size() == (getPrefixLengthForDestinationType(destinationType) + 2)) {
                     destination = formatDestination(dn.getRdn(dn.size() - 2), destinationType);
-                } else if (dn.size() == (getPrefixLengthForDestinationType(destinationType) + 1)){
+                } else if (dn.size() == (getPrefixLengthForDestinationType(destinationType) + 1)) {
                     destination = formatDestination(dn.getRdn(dn.size() - 1), destinationType);
                 } else {
-                    throw new IllegalArgumentException(
+                    throw new IllegalArgumentException("Malformed DN for representing a permission or destination entry.");
-                            "Malformed DN for representing a permission or destination entry.");
                 }
                 break;
             default:
-                throw new IllegalArgumentException(
+                throw new IllegalArgumentException("Cannot format destination for destination type " + destinationType);
-                        "Cannot format destination for destination type " + destinationType);
         }
 
         return destination;
     }
 
     /**
-     * Parses RDN values representing the destination name/pattern and
+     * Parses RDN values representing the destination name/pattern and destination type into the equivalent
-     * destination type into the equivalent {@link ActiveMQDestination}.
+     * {@link ActiveMQDestination}.
      *
      * @param destinationName
      *            the RDN representing the name or pattern for the destination
@@ -521,19 +565,18 @@ public class SimpleCachedLDAPAuthorizationMap extends DefaultAuthorizationMap {
                 dest = new ActiveMQTopic(formatDestinationName(destinationName));
                 break;
             default:
-                throw new IllegalArgumentException("Unknown destination type: "
+                throw new IllegalArgumentException("Unknown destination type: " + destinationType);
-                        + destinationType);
         }
 
         return dest;
     }
 
     /**
-     * Parses the RDN representing a destination name/pattern into the standard string representation
+     * Parses the RDN representing a destination name/pattern into the standard string representation of the
-     * of the name/pattern.  This implementation does not care about the type of the RDN such that the RDN could
+     * name/pattern. This implementation does not care about the type of the RDN such that the RDN could be a CN or OU.
-     * be a CN or OU.
      *
-     * @param destinationName the RDN representing the name or pattern for the destination
+     * @param destinationName
+     *            the RDN representing the name or pattern for the destination
      *
      * @see #formatDestination(Rdn, Rdn)
      */
@@ -542,14 +585,13 @@ public class SimpleCachedLDAPAuthorizationMap extends DefaultAuthorizationMap {
     }
 
     /**
-     * Transcribes an existing set into a new set. Used to make defensive copies
+     * Transcribes an existing set into a new set. Used to make defensive copies for concurrent access.
-     * for concurrent access.
      *
      * @param source
      *            the source set or {@code null}
      *
-     * @return a new set containing the same elements as {@code source} or
+     * @return a new set containing the same elements as {@code source} or {@code null} if {@code source} is
-     *         {@code null} if {@code source} is {@code null}
+     *         {@code null}
      */
     protected <T> Set<T> transcribeSet(Set<T> source) {
         if (source != null) {
@@ -562,7 +604,8 @@ public class SimpleCachedLDAPAuthorizationMap extends DefaultAuthorizationMap {
     /**
      * Returns the filter string for the given permission type.
      *
-     * @throws IllegalArgumentException if {@code permissionType} is not supported
+     * @throws IllegalArgumentException
+     *             if {@code permissionType} is not supported
      *
      * @see #setAdminPermissionGroupSearchFilter(String)
      * @see #setReadPermissionGroupSearchFilter(String)
@@ -591,7 +634,8 @@ public class SimpleCachedLDAPAuthorizationMap extends DefaultAuthorizationMap {
     /**
      * Returns the DN prefix size based on the given destination type.
      *
-     * @throws IllegalArgumentException if {@code destinationType} is not supported
+     * @throws IllegalArgumentException
+     *             if {@code destinationType} is not supported
      *
      * @see #setQueueSearchBase(String)
      * @see #setTopicSearchBase(String)
@@ -618,11 +662,24 @@ public class SimpleCachedLDAPAuthorizationMap extends DefaultAuthorizationMap {
     }
 
     /**
-     * Performs a check for updates from the server in the event that synchronous updates are enabled 
+     * Performs a check for updates from the server in the event that synchronous updates are enabled and are the
-     * and are the refresh interval has elapsed.
+     * refresh interval has elapsed.
      */
     protected void checkForUpdates() {
+
+        if (context != null && refreshDisabled) {
+            return;
+        }
+
         if (context == null || (!refreshDisabled && (refreshInterval != -1 && System.currentTimeMillis() >= lastUpdated + refreshInterval))) {
+            this.updaterService.execute(new Runnable() {
+                @Override
+                public void run() {
+
+                    // Check again in case of stacked update request.
+                    if (context == null || (!refreshDisabled &&
+                        (refreshInterval != -1 && System.currentTimeMillis() >= lastUpdated + refreshInterval))) {
+
                         if (!isContextAlive()) {
                             try {
                                 context = createContext();
@@ -631,159 +688,160 @@ public class SimpleCachedLDAPAuthorizationMap extends DefaultAuthorizationMap {
                                 return;
                             }
                         }
-            reset();
+
-            setTempDestinationAuthorizationEntry(null);
                         entries.clear();
 
                         LOG.debug("Updating authorization map!");
                         try {
                             query();
                         } catch (Exception e) {
-                LOG.error("Error updating authorization map.  Partial policy "
+                            LOG.error("Error updating authorization map.  Partial policy " +
-                        + "may be applied until the next successful update.", e);
+                                      "may be applied until the next successful update.", e);
                         }
                     }
                 }
+            });
+        }
+    }
 
     // Authorization Map
 
     /**
-     * Provides synchronous refresh capabilities if so configured before delegating to the super implementation,
+     * Provides synchronized and defensive access to the admin ACLs for temp destinations as the super implementation
-     * and otherwise simply delegates to the super implementation.
+     * returns live copies of the ACLs and {@link AuthorizationEntry} is not setup for concurrent access.
      */
     @Override
-    protected synchronized Set<AuthorizationEntry> getAllEntries(ActiveMQDestination destination) {
+    public Set<Object> getTempDestinationAdminACLs() {
         checkForUpdates();
-        return super.getAllEntries(destination);
+        DefaultAuthorizationMap map = this.map.get();
+        return transcribeSet(map.getTempDestinationAdminACLs());
     }
 
     /**
-     * Provides synchronized and defensive access to the admin ACLs for temp destinations as the super
+     * Provides synchronized and defensive access to the read ACLs for temp destinations as the super implementation
-     * implementation returns live copies of the ACLs and {@link AuthorizationEntry} is not
+     * returns live copies of the ACLs and {@link AuthorizationEntry} is not setup for concurrent access.
-     * setup for concurrent access.
      */
     @Override
-    public synchronized Set<Object> getTempDestinationAdminACLs() {
+    public Set<Object> getTempDestinationReadACLs() {
         checkForUpdates();
-        return transcribeSet(super.getTempDestinationAdminACLs());
+        DefaultAuthorizationMap map = this.map.get();
+        return transcribeSet(map.getTempDestinationReadACLs());
     }
 
     /**
-     * Provides synchronized and defensive access to the read ACLs for temp destinations as the super
+     * Provides synchronized and defensive access to the write ACLs for temp destinations as the super implementation
-     * implementation returns live copies of the ACLs and {@link AuthorizationEntry} is not
+     * returns live copies of the ACLs and {@link AuthorizationEntry} is not setup for concurrent access.
-     * setup for concurrent access.
      */
-    public synchronized Set<Object> getTempDestinationReadACLs() {
+    @Override
+    public Set<Object> getTempDestinationWriteACLs() {
         checkForUpdates();
-        return transcribeSet(super.getTempDestinationReadACLs());
+        DefaultAuthorizationMap map = this.map.get();
+        return transcribeSet(map.getTempDestinationWriteACLs());
     }
 
     /**
-     * Provides synchronized and defensive access to the write ACLs for temp destinations as the super
+     * Provides synchronized access to the admin ACLs for the destinations as {@link AuthorizationEntry}
-     * implementation returns live copies of the ACLs and {@link AuthorizationEntry} is not
+     * is not setup for concurrent access.
-     * setup for concurrent access.
      */
-    public synchronized Set<Object> getTempDestinationWriteACLs() {
+    @Override
+    public Set<Object> getAdminACLs(ActiveMQDestination destination) {
         checkForUpdates();
-        return transcribeSet(super.getTempDestinationWriteACLs());
+        DefaultAuthorizationMap map = this.map.get();
+        return map.getAdminACLs(destination);
     }
 
     /**
-     * Provides synchronized access to the admin ACLs for the destinations as 
+     * Provides synchronized access to the read ACLs for the destinations as {@link AuthorizationEntry} is not setup for
-     * {@link AuthorizationEntry} is not setup for concurrent access.
+     * concurrent access.
      */
-    public synchronized Set<Object> getAdminACLs(ActiveMQDestination destination) {
+    @Override
-        return super.getAdminACLs(destination);
+    public Set<Object> getReadACLs(ActiveMQDestination destination) {
+        checkForUpdates();
+        DefaultAuthorizationMap map = this.map.get();
+        return map.getReadACLs(destination);
     }
 
     /**
-     * Provides synchronized access to the read ACLs for the destinations as 
+     * Provides synchronized access to the write ACLs for the destinations as {@link AuthorizationEntry} is not setup
-     * {@link AuthorizationEntry} is not setup for concurrent access.
+     * for concurrent access.
      */
-    public synchronized Set<Object> getReadACLs(ActiveMQDestination destination) {
+    @Override
+    public Set<Object> getWriteACLs(ActiveMQDestination destination) {
         checkForUpdates();
-        return super.getReadACLs(destination);
+        DefaultAuthorizationMap map = this.map.get();
-    }
+        return map.getWriteACLs(destination);
-
-    /**
-     * Provides synchronized access to the write ACLs for the destinations as 
-     * {@link AuthorizationEntry} is not setup for concurrent access.
-     */
-    public synchronized Set<Object> getWriteACLs(ActiveMQDestination destination) {
-        checkForUpdates();
-        return super.getWriteACLs(destination);
     }
 
     /**
      * Handler for new policy entries in the directory.
      *
-     * @param namingEvent the new entry event that occurred 
+     * @param namingEvent
-     * @param destinationType the type of the destination to which the event applies
+     *            the new entry event that occurred
-     * @param permissionType the permission type to which the event applies
+     * @param destinationType
+     *            the type of the destination to which the event applies
+     * @param permissionType
+     *            the permission type to which the event applies
      */
-    public synchronized void objectAdded(NamingEvent namingEvent, DestinationType destinationType,
+    public void objectAdded(NamingEvent namingEvent, DestinationType destinationType, PermissionType permissionType) {
-            PermissionType permissionType) {
         LOG.debug("Adding object: " + namingEvent.getNewBinding());
         SearchResult result = (SearchResult) namingEvent.getNewBinding();
 
         try {
+            DefaultAuthorizationMap map = this.map.get();
             LdapName name = new LdapName(result.getName());
-            
+            AuthorizationEntry entry = getEntry(map, name, destinationType);
-            AuthorizationEntry entry = getEntry(name, destinationType);
 
             applyACL(entry, result, permissionType);
             if (!(entry instanceof TempDestinationAuthorizationEntry)) {
-                put(entry.getDestination(), entry);
+                map.put(entry.getDestination(), entry);
             }
-            
         } catch (InvalidNameException e) {
-            LOG.error("Policy not applied!  Error parsing DN for addition of "
+            LOG.error("Policy not applied!  Error parsing DN for addition of " + result.getName(), e);
-                    + result.getName(), e);
         } catch (Exception e) {
-            LOG.error("Policy not applied!  Error processing object addition for addition of "
+            LOG.error("Policy not applied!  Error processing object addition for addition of " + result.getName(), e);
-                    + result.getName(), e);
         }
     }
 
     /**
      * Handler for removed policy entries in the directory.
      *
-     * @param namingEvent the removed entry event that occurred 
+     * @param namingEvent
-     * @param destinationType the type of the destination to which the event applies
+     *            the removed entry event that occurred
-     * @param permissionType the permission type to which the event applies
+     * @param destinationType
+     *            the type of the destination to which the event applies
+     * @param permissionType
+     *            the permission type to which the event applies
      */
-    public synchronized void objectRemoved(NamingEvent namingEvent, DestinationType destinationType,
+    public void objectRemoved(NamingEvent namingEvent, DestinationType destinationType, PermissionType permissionType) {
-            PermissionType permissionType) {
         LOG.debug("Removing object: " + namingEvent.getOldBinding());
         Binding result = namingEvent.getOldBinding();
 
         try {
+            DefaultAuthorizationMap map = this.map.get();
             LdapName name = new LdapName(result.getName());
-            
+            AuthorizationEntry entry = getEntry(map, name, destinationType);
-            AuthorizationEntry entry = getEntry(name, destinationType);
-
             applyAcl(entry, permissionType, new HashSet<Object>());
         } catch (InvalidNameException e) {
-            LOG.error("Policy not applied!  Error parsing DN for object removal for removal of "
+            LOG.error("Policy not applied!  Error parsing DN for object removal for removal of " + result.getName(), e);
-                    + result.getName(), e);
         } catch (Exception e) {
-            LOG.error("Policy not applied!  Error processing object removal for removal of "
+            LOG.error("Policy not applied!  Error processing object removal for removal of " + result.getName(), e);
-                    + result.getName(), e);
         }
     }
 
     /**
-     * Handler for renamed policy entries in the directory.  This handler deals with the renaming
+     * Handler for renamed policy entries in the directory. This handler deals with the renaming of destination entries
-     * of destination entries as well as permission entries.  If the permission type is not null, it is
+     * as well as permission entries. If the permission type is not null, it is assumed that we are dealing with the
-     * assumed that we are dealing with the renaming of a permission entry.  Otherwise, it is assumed
+     * renaming of a permission entry. Otherwise, it is assumed that we are dealing with the renaming of a destination
-     * that we are dealing with the renaming of a destination entry.
+     * entry.
      *
-     * @param namingEvent the renaming entry event that occurred 
+     * @param namingEvent
-     * @param destinationType the type of the destination to which the event applies
+     *            the renaming entry event that occurred
-     * @param permissionType the permission type to which the event applies
+     * @param destinationType
+     *            the type of the destination to which the event applies
+     * @param permissionType
+     *            the permission type to which the event applies
      */
-    public synchronized void objectRenamed(NamingEvent namingEvent, DestinationType destinationType,
+    public void objectRenamed(NamingEvent namingEvent, DestinationType destinationType, PermissionType permissionType) {
-            PermissionType permissionType) {
         Binding oldBinding = namingEvent.getOldBinding();
         Binding newBinding = namingEvent.getNewBinding();
         LOG.debug("Renaming object: " + oldBinding + " to " + newBinding);
@@ -805,9 +863,7 @@ public class SimpleCachedLDAPAuthorizationMap extends DefaultAuthorizationMap {
                 boolean matchedToType = false;
 
                 for (PermissionType newPermissionType : PermissionType.values()) {
-                    NamingEnumeration<SearchResult> results = context.search(
+                    NamingEnumeration<SearchResult> results = context.search(newName, getFilterForPermissionType(newPermissionType), controls);
-                            newName,
-                            getFilterForPermissionType(newPermissionType), controls);
 
                     if (results.hasMore()) {
                         objectAdded(namingEvent, destinationType, newPermissionType);
@@ -817,19 +873,18 @@ public class SimpleCachedLDAPAuthorizationMap extends DefaultAuthorizationMap {
                 }
 
                 if (!matchedToType) {
-                    LOG.error("Policy not applied!  Error processing object rename for rename of "
+                    LOG.error("Policy not applied!  Error processing object rename for rename of " + oldBinding.getName() + " to " + newBinding.getName()
-                            + oldBinding.getName() + " to " + newBinding.getName()
                         + ".  Could not determine permission type of new object.");
                 }
-                
             } else {
                 // Handle the case where a destination entry is being renamed.
                 if (oldDest != null && newDest != null) {
                     AuthorizationEntry entry = entries.remove(oldDest);
                     if (entry != null) {
                         entry.setDestination(newDest);
-                        put(newDest, entry);
+                        DefaultAuthorizationMap map = this.map.get();
-                        remove(oldDest, entry);
+                        map.put(newDest, entry);
+                        map.remove(oldDest, entry);
                         entries.put(newDest, entry);
                     } else {
                         LOG.warn("No authorization entry for " + oldDest);
@@ -837,23 +892,23 @@ public class SimpleCachedLDAPAuthorizationMap extends DefaultAuthorizationMap {
                 }
             }
         } catch (InvalidNameException e) {
-            LOG.error("Policy not applied!  Error parsing DN for object rename for rename of "
+            LOG.error("Policy not applied!  Error parsing DN for object rename for rename of " + oldBinding.getName() + " to " + newBinding.getName(), e);
-                    + oldBinding.getName() + " to " + newBinding.getName(), e);
         } catch (Exception e) {
-            LOG.error("Policy not applied!  Error processing object rename for rename of "
+            LOG.error("Policy not applied!  Error processing object rename for rename of " + oldBinding.getName() + " to " + newBinding.getName(), e);
-                    + oldBinding.getName() + " to " + newBinding.getName(), e);
         }
     }
 
     /**
      * Handler for changed policy entries in the directory.
      *
-     * @param namingEvent the changed entry event that occurred 
+     * @param namingEvent
-     * @param destinationType the type of the destination to which the event applies
+     *            the changed entry event that occurred
-     * @param permissionType the permission type to which the event applies
+     * @param destinationType
+     *            the type of the destination to which the event applies
+     * @param permissionType
+     *            the permission type to which the event applies
      */
-    public synchronized void objectChanged(NamingEvent namingEvent,
+    public void objectChanged(NamingEvent namingEvent, DestinationType destinationType, PermissionType permissionType) {
-            DestinationType destinationType, PermissionType permissionType) {
         LOG.debug("Changing object " + namingEvent.getOldBinding() + " to " + namingEvent.getNewBinding());
         objectRemoved(namingEvent, destinationType, permissionType);
         objectAdded(namingEvent, destinationType, permissionType);
@@ -862,7 +917,8 @@ public class SimpleCachedLDAPAuthorizationMap extends DefaultAuthorizationMap {
     /**
      * Handler for exception events from the registry.
      *
-     * @param namingExceptionEvent the exception event
+     * @param namingExceptionEvent
+     *            the exception event
      */
     public void namingExceptionThrown(NamingExceptionEvent namingExceptionEvent) {
         context = null;
@@ -974,8 +1030,7 @@ public class SimpleCachedLDAPAuthorizationMap extends DefaultAuthorizationMap {
         return permissionGroupMemberAttribute;
     }
 
-    public void setPermissionGroupMemberAttribute(
+    public void setPermissionGroupMemberAttribute(String permissionGroupMemberAttribute) {
-            String permissionGroupMemberAttribute) {
         this.permissionGroupMemberAttribute = permissionGroupMemberAttribute;
     }
 
@@ -983,8 +1038,7 @@ public class SimpleCachedLDAPAuthorizationMap extends DefaultAuthorizationMap {
         return adminPermissionGroupSearchFilter;
     }
 
-    public void setAdminPermissionGroupSearchFilter(
+    public void setAdminPermissionGroupSearchFilter(String adminPermissionGroupSearchFilter) {
-            String adminPermissionGroupSearchFilter) {
         this.adminPermissionGroupSearchFilter = adminPermissionGroupSearchFilter;
     }
 
@@ -992,8 +1046,7 @@ public class SimpleCachedLDAPAuthorizationMap extends DefaultAuthorizationMap {
         return readPermissionGroupSearchFilter;
     }
 
-    public void setReadPermissionGroupSearchFilter(
+    public void setReadPermissionGroupSearchFilter(String readPermissionGroupSearchFilter) {
-            String readPermissionGroupSearchFilter) {
         this.readPermissionGroupSearchFilter = readPermissionGroupSearchFilter;
     }
 
@@ -1001,8 +1054,7 @@ public class SimpleCachedLDAPAuthorizationMap extends DefaultAuthorizationMap {
         return writePermissionGroupSearchFilter;
     }
 
-    public void setWritePermissionGroupSearchFilter(
+    public void setWritePermissionGroupSearchFilter(String writePermissionGroupSearchFilter) {
-            String writePermissionGroupSearchFilter) {
         this.writePermissionGroupSearchFilter = writePermissionGroupSearchFilter;
     }
 
@@ -1063,38 +1115,32 @@ public class SimpleCachedLDAPAuthorizationMap extends DefaultAuthorizationMap {
     }
 
     protected static enum DestinationType {
-        QUEUE,
+        QUEUE, TOPIC, TEMP;
-        TOPIC,
-        TEMP;
     }
 
     protected static enum PermissionType {
-        READ,
+        READ, WRITE, ADMIN;
-        WRITE,
-        ADMIN;
     }
 
     /**
-     * Listener implementation for directory changes that maps change events to
+     * Listener implementation for directory changes that maps change events to destination types.
-     * destination types.
      */
-    protected class CachedLDAPAuthorizationMapNamespaceChangeListener implements
+    protected class CachedLDAPAuthorizationMapNamespaceChangeListener implements NamespaceChangeListener, ObjectChangeListener {
-            NamespaceChangeListener, ObjectChangeListener {
 
         private final DestinationType destinationType;
         private final PermissionType permissionType;
 
         /**
-         * Creates a new listener.  If {@code permissionType} is {@code null}, add
+         * Creates a new listener. If {@code permissionType} is {@code null}, add and remove events are ignored as they
-         * and remove events are ignored as they do not directly affect policy state.
+         * do not directly affect policy state. This configuration is used when listening for changes on entries that
-         * This configuration is used when listening for changes on entries that represent
+         * represent destination patterns and not for entries that represent permissions.
-         * destination patterns and not for entries that represent permissions.
          *
-         * @param destinationType the type of the destination being listened for
+         * @param destinationType
-         * @param permissionType the optional permission type being listened for
+         *            the type of the destination being listened for
+         * @param permissionType
+         *            the optional permission type being listened for
          */
-        public CachedLDAPAuthorizationMapNamespaceChangeListener(
+        public CachedLDAPAuthorizationMapNamespaceChangeListener(DestinationType destinationType, PermissionType permissionType) {
-                DestinationType destinationType, PermissionType permissionType) {
             this.destinationType = destinationType;
             this.permissionType = permissionType;
         }

activemq-unit-tests/src/test/java/org/apache/activemq/security/AbstractCachedLDAPAuthorizationMapLegacyTest.java

@@ -112,8 +112,13 @@ public abstract class AbstractCachedLDAPAuthorizationMapLegacyTest extends Abstr
 
         reader.close();
 
-        failedACLs = map.getReadACLs(new ActiveMQQueue("TEST.FOO"));
+        assertTrue("did not get expected size. ", Wait.waitFor(new Wait.Condition() {
-        assertEquals("set size: " + failedACLs, 0, failedACLs.size());
+
+            @Override
+            public boolean isSatisified() throws Exception {
+                return map.getReadACLs(new ActiveMQQueue("TEST.FOO")).size() == 0;
+            }
+        }));
 
         assertNull(map.getTempDestinationReadACLs());
         assertNull(map.getTempDestinationWriteACLs());
@@ -335,10 +340,14 @@ public abstract class AbstractCachedLDAPAuthorizationMapLegacyTest extends Abstr
         }
 
         reader.close();
-        Thread.sleep(2000);
 
-        failedACLs = map.getReadACLs(new ActiveMQQueue("FAILED"));
+        assertTrue("did not get expected size. ", Wait.waitFor(new Wait.Condition() {
-        assertEquals("set size: " + failedACLs, 2, failedACLs.size());
+
+            @Override
+            public boolean isSatisified() throws Exception {
+                return map.getReadACLs(new ActiveMQQueue("FAILED")).size() == 2;
+            }
+        }));
     }
 
     protected SimpleCachedLDAPAuthorizationMap createMap() {