Merged V3.2 to HEAD

16939: Merged V3.1 to V3.2
      16938: ETHREEOH-622: AuthorityServiceImpl uses userNameMatcher to check for admin users according to case sensitivity settings
      16934: ETHREEOH-2584: Coding error in BaseSSOAuthenticationFilter
   16924: LDAP Performance
      - Created NodeService addChild variants that can add associations to multiple parents (groups/zones) at the same time with a single path check.
      - Created AuthorityService addAuthority variant that can add an authority to multiple groups at the same time, using the above
      - Optimized group association creation strategy. Groups and Persons created in 'depth first' order (root groups first, parents last). Prevents the nodes having to be reindexed.


git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@17070 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261

config/alfresco/authority-services-context.xml

@@ -25,6 +25,9 @@
         <property name="authorityDAO">
             <ref bean="authorityDAO" />
         </property>
+        <property name="userNameMatcher">
+            <ref bean="userNameMatcher" />
+        </property>
         <property name="authenticationService">
             <ref bean="authenticationService" />
         </property>

source/java/org/alfresco/repo/avm/AVMNodeService.java

@@ -949,6 +949,28 @@ public class AVMNodeService extends AbstractNodeServiceImpl implements NodeServi
             NodeRef childRef,
             QName assocTypeQName,
             QName qname) throws InvalidNodeRefException
+    {
+        return addChild(Collections.singletonList(parentRef), childRef, assocTypeQName, qname).get(0);
+    }
+
+    /**
+     * Associates a given child node with a given collection of parents.  All nodes must belong to the same store.
+     * <p>
+     * 
+     * 
+     * @param parentRefs
+     * @param childRef 
+     * @param assocTypeQName the qualified name of the association type as defined in the datadictionary
+     * @param qname the qualified name of the association
+     * @return Returns a reference to the newly created child association
+     * @throws InvalidNodeRefException if the parent or child nodes could not be found
+     * @throws CyclicChildRelationshipException if the child partakes in a cyclic relationship after the add
+     */
+    public List<ChildAssociationRef> addChild(
+            Collection<NodeRef> parentRefs,
+            NodeRef childRef,
+            QName assocTypeQName,
+            QName qname) throws InvalidNodeRefException
     {
         Pair<Integer, String> childVersionPath = AVMNodeConverter.ToAVMVersionPath(childRef);
         AVMNodeDescriptor child = fAVMService.lookup(childVersionPath.getFirst(), 
@@ -957,6 +979,10 @@ public class AVMNodeService extends AbstractNodeServiceImpl implements NodeServi
         {
             throw new InvalidNodeRefException(childVersionPath.getSecond() + " not found.", childRef);
         }
+        
+        List<ChildAssociationRef> childAssociationRefs = new ArrayList<ChildAssociationRef>(parentRefs.size());
+        for (NodeRef parentRef : parentRefs)
+        {
             Pair<Integer, String> parentVersionPath = AVMNodeConverter.ToAVMVersionPath(parentRef);
             if (parentVersionPath.getFirst() >= 0)
             {
@@ -965,17 +991,18 @@ public class AVMNodeService extends AbstractNodeServiceImpl implements NodeServi
             try
             {
                 fAVMService.link(parentVersionPath.getSecond(), qname.getLocalName(), child);
-            ChildAssociationRef newChild =
+                ChildAssociationRef newChild = new ChildAssociationRef(assocTypeQName, parentRef, qname,
-                new ChildAssociationRef(assocTypeQName, parentRef, qname, 
+                        AVMNodeConverter.ToNodeRef(-1, AVMNodeConverter.ExtendAVMPath(parentVersionPath.getSecond(),
-                    AVMNodeConverter.ToNodeRef(-1,
+                                qname.getLocalName())));
-                       AVMNodeConverter.ExtendAVMPath(parentVersionPath.getSecond(), qname.getLocalName())));
+                childAssociationRefs.add(newChild);
-            return newChild;
             }
             catch (AVMException e)
             {
                 throw new InvalidNodeRefException("Could not link.", childRef);
             }
         }
+        return childAssociationRefs;
+    }
     
     /**
      * Severs all parent-child relationships between two nodes.

source/java/org/alfresco/repo/node/db/DbNodeServiceImpl.java

@@ -855,45 +855,60 @@ public class DbNodeServiceImpl extends AbstractNodeServiceImpl
 
     public ChildAssociationRef addChild(NodeRef parentRef, NodeRef childRef, QName assocTypeQName, QName assocQName)
     {
-        Pair<Long, NodeRef> parentNodePair = getNodePairNotNull(parentRef);
+        return addChild(Collections.singletonList(parentRef), childRef, assocTypeQName, assocQName).get(0);
-        Long parentNodeId = parentNodePair.getFirst();
+    }
+
+    public List<ChildAssociationRef> addChild(Collection<NodeRef> parentRefs, NodeRef childRef, QName assocTypeQName, QName assocQName)
+    {
+        // Get the node's name, if present
         Pair<Long, NodeRef> childNodePair = getNodePairNotNull(childRef);
         Long childNodeId = childNodePair.getFirst();
+        Map<QName, Serializable> childNodeProperties = nodeDaoService.getNodeProperties(childNodePair.getFirst());
+        String childNodeName = extractNameProperty(childNodeProperties);
+
+        List <ChildAssociationRef> childAssociationRefs = new ArrayList<ChildAssociationRef>(parentRefs.size());
+        List<Pair<Long, NodeRef>> parentNodePairs = new ArrayList<Pair<Long, NodeRef>>(parentRefs.size());
+        for (NodeRef parentRef : parentRefs)
+        {
+            Pair<Long, NodeRef> parentNodePair = getNodePairNotNull(parentRef);
+            Long parentNodeId = parentNodePair.getFirst();
+            parentNodePairs.add(parentNodePair);
 
             // Invoke policy behaviours
             invokeBeforeCreateChildAssociation(parentRef, childRef, assocTypeQName, assocQName, false);
 
-        // Get the node's name, if present
-        Map<QName, Serializable> childNodeProperties = nodeDaoService.getNodeProperties(childNodePair.getFirst());
-        String childNodeName = extractNameProperty(childNodeProperties);
-        
             // make the association
-        Pair<Long, ChildAssociationRef> childAssocPair =  nodeDaoService.newChildAssoc(
+            Pair<Long, ChildAssociationRef> childAssocPair = nodeDaoService.newChildAssoc(parentNodeId, childNodeId,
-                parentNodeId,
+                    false, assocTypeQName, assocQName, childNodeName);
-                childNodeId,
-                false,
-                assocTypeQName,
-                assocQName,
-                childNodeName);
-        ChildAssociationRef childAssocRef = childAssocPair.getSecond();
             // ensure name uniqueness
             setChildNameUnique(childAssocPair, childNodePair);
-        NodeRef childNodeRef = childAssocRef.getChildRef();
+
+            childAssociationRefs.add(childAssocPair.getSecond());
+        }
         
         // check that the child addition of the child has not created a cyclic relationship
         // this functionality is provided for free in getPath
-        getPaths(childNodeRef, false);
+        getPaths(childRef, false);
 
         // Invoke policy behaviours
+        for (ChildAssociationRef childAssocRef : childAssociationRefs)
+        {
             invokeOnCreateChildAssociation(childAssocRef, false);
+        }
         
         // Add missing aspects
+        for (Pair<Long, NodeRef> parentNodePair : parentNodePairs)
+        {
             addMissingAspects(parentNodePair, assocTypeQName);
+        }
 
         // Index
+        for (ChildAssociationRef childAssocRef : childAssociationRefs)
+        {
             nodeIndexer.indexCreateChildAssociation(childAssocRef);
+        }
 
-        return childAssocRef;
+        return childAssociationRefs;
     }
 
     public void removeChild(NodeRef parentRef, NodeRef childRef) throws InvalidNodeRefException

source/java/org/alfresco/repo/security/authority/AuthorityDAO.java

@@ -24,21 +24,21 @@
  */
 package org.alfresco.repo.security.authority;
 
+import java.util.Collection;
 import java.util.Set;
 
 import org.alfresco.service.cmr.repository.NodeRef;
-import org.alfresco.service.cmr.security.AuthorityService;
 import org.alfresco.service.cmr.security.AuthorityType;
 
 public interface AuthorityDAO
 {
     /**
-     * Add an authority to another.
+     * Add a child authority to the given parent authorities
      * 
-     * @param parentName
+     * @param parentNames
      * @param childName
      */
-    void addAuthority(String parentName, String childName);
+    void addAuthority(Collection<String> parentNames, String childName);
 
     /**
      * Create an authority.

source/java/org/alfresco/repo/security/authority/AuthorityDAOImpl.java

@@ -149,26 +149,34 @@ public class AuthorityDAOImpl implements AuthorityDAO, NodeServicePolicies.Befor
         return ref != null;
     }
 
-    public void addAuthority(String parentName, String childName)
+    public void addAuthority(Collection<String> parentNames, String childName)
+    {
+        Set<NodeRef> parentRefs = new HashSet<NodeRef>(parentNames.size() * 2);
+        AuthorityType authorityType = AuthorityType.getAuthorityType(childName);
+        boolean notUserOrGroup = !authorityType.equals(AuthorityType.USER) && !authorityType.equals(AuthorityType.GROUP);
+        for (String parentName : parentNames)
         {
             NodeRef parentRef = getAuthorityOrNull(parentName);
             if (parentRef == null)
             {
                 throw new UnknownAuthorityException("An authority was not found for " + parentName);
             }
-        AuthorityType authorityType = AuthorityType.getAuthorityType(childName);
+            if (notUserOrGroup
-        if (!authorityType.equals(AuthorityType.USER)
+                    && !(authorityType.equals(AuthorityType.ROLE) && AuthorityType.getAuthorityType(parentName).equals(
-                && !authorityType.equals(AuthorityType.GROUP)
+                            AuthorityType.ROLE)))
-                && !(authorityType.equals(AuthorityType.ROLE) && AuthorityType.getAuthorityType(parentName).equals(AuthorityType.ROLE)))
             {
-            throw new AlfrescoRuntimeException("Authorities of the type " + authorityType + " may not be added to other authorities");
+                throw new AlfrescoRuntimeException("Authorities of the type " + authorityType
+                        + " may not be added to other authorities");
+            }
+            parentRefs.add(parentRef);
         }
         NodeRef childRef = getAuthorityOrNull(childName);
         if (childRef == null)
         {
             throw new UnknownAuthorityException("An authority was not found for " + childName);
         }
-        nodeService.addChild(parentRef, childRef, ContentModel.ASSOC_MEMBER, QName.createQName("cm", childName, namespacePrefixResolver));
+        nodeService.addChild(parentRefs, childRef, ContentModel.ASSOC_MEMBER, QName.createQName("cm", childName,
+                namespacePrefixResolver));
         authorityLookupCache.clear();
     }
 
@@ -183,10 +191,12 @@ public class AuthorityDAOImpl implements AuthorityDAO, NodeServicePolicies.Befor
                 ContentModel.TYPE_AUTHORITY_CONTAINER, props).getChildRef();
         if (authorityZones != null)
         {
+            Set<NodeRef> zoneRefs = new HashSet<NodeRef>(authorityZones.size() * 2);
             for (String authorityZone : authorityZones)
             {
-                nodeService.addChild(getOrCreateZone(authorityZone), childRef, ContentModel.ASSOC_IN_ZONE, QName.createQName("cm", name, namespacePrefixResolver));
+                zoneRefs.add(getOrCreateZone(authorityZone));
             }
+            nodeService.addChild(zoneRefs, childRef, ContentModel.ASSOC_IN_ZONE, QName.createQName("cm", name, namespacePrefixResolver));
         }
         authorityLookupCache.clear();
     }
@@ -607,15 +617,15 @@ public class AuthorityDAOImpl implements AuthorityDAO, NodeServicePolicies.Befor
     {
         if ((zones != null) && (zones.size() > 0))
         {
+            Set<NodeRef> zoneRefs = new HashSet<NodeRef>(zones.size() * 2);
+            for (String authorityZone : zones)
+            {
+                zoneRefs.add(getOrCreateZone(authorityZone));
+            }
             NodeRef authRef = getAuthorityOrNull(authorityName);
             if (authRef != null)
             {
-                for (String zone : zones)
+                nodeService.addChild(zoneRefs, authRef, ContentModel.ASSOC_IN_ZONE, QName.createQName("cm", authorityName, namespacePrefixResolver));
-                {
-                    // Add the person to an authentication zone (corresponding to an external user registry)
-                    // Let's preserve case on this child association
-                    nodeService.addChild(getOrCreateZone(zone), authRef, ContentModel.ASSOC_IN_ZONE, QName.createQName("cm", authorityName, namespacePrefixResolver));
-                }
             }
         }
     }

source/java/org/alfresco/repo/security/authority/AuthorityServiceImpl.java

@@ -24,6 +24,7 @@
  */
 package org.alfresco.repo.security.authority;
 
+import java.util.Collection;
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.Set;
@@ -31,6 +32,7 @@ import java.util.Set;
 import org.alfresco.model.ContentModel;
 import org.alfresco.repo.security.authentication.AuthenticationUtil;
 import org.alfresco.repo.security.permissions.PermissionServiceSPI;
+import org.alfresco.repo.security.person.UserNameMatcher;
 import org.alfresco.repo.tenant.TenantService;
 import org.alfresco.service.cmr.repository.NodeRef;
 import org.alfresco.service.cmr.repository.NodeService;
@@ -59,6 +61,8 @@ public class AuthorityServiceImpl implements AuthorityService, InitializingBean
 
     private AuthorityDAO authorityDAO;
     
+    private UserNameMatcher userNameMatcher;
+	
     private AuthenticationService authenticationService;
     
     private PermissionServiceSPI permissionServiceSPI;
@@ -104,6 +108,11 @@ public class AuthorityServiceImpl implements AuthorityService, InitializingBean
         this.authorityDAO = authorityDAO;
     }        
         
+    public void setUserNameMatcher(UserNameMatcher userNameMatcher)
+    {
+        this.userNameMatcher = userNameMatcher;
+    }
+
     public void setAuthenticationService(AuthenticationService authenticationService)
     {
         this.authenticationService = authenticationService;
@@ -206,44 +215,9 @@ public class AuthorityServiceImpl implements AuthorityService, InitializingBean
         Set<String> adminUsers = this.authenticationService.getDefaultAdministratorUserNames();
         Set<String> guestUsers = this.authenticationService.getDefaultGuestUserNames();
 
-        // note: for multi-tenancy, this currently relies on a naming convention which assumes that all tenant admins will 
+        // Check for name matches using MT + case sensitivity rules
-        // have the same base name as the default non-tenant specific admin. Typically "admin" is the default required admin user, 
+        boolean isAdminUser = containsMatch(adminUsers, currentUserName);
-        // although, if for example "bob" is also listed as an admin then all tenant-specific bob's will also have admin authority
+        boolean isGuestUser = containsMatch(guestUsers, currentUserName);
-        String currentUserBaseName = tenantService.getBaseNameUser(currentUserName);
-        
-        boolean isAdminUser = false;
-        boolean isGuestUser = false;
-        if (tenantService.isEnabled())
-        {
-            // note: for multi-tenancy, this currently relies on a naming convention which assumes that all tenant admins will 
-            // have the same base name as the default non-tenant specific admin. Typically "admin" is the default required admin user, 
-            // although, if for example "bob" is also listed as an admin then all tenant-specific bob's will also have admin authority
-            
-            for (String adminUser : adminUsers)
-            {
-                if (adminUser.equals(currentUserName) || tenantService.getBaseNameUser(adminUser).equals(currentUserBaseName))
-                {
-                    isAdminUser = true;
-                    break;
-                }
-            }
-            if (!isAdminUser)
-            {
-                for (String guestUser : guestUsers)
-                {
-                    if (guestUser.equals(currentUserName) || tenantService.getBaseNameUser(guestUser).equals(currentUserBaseName))
-                    {
-                        isGuestUser = true;
-                        break;
-                    }
-                }
-            }
-        }
-        else
-        {
-            isAdminUser = (adminUsers.contains(currentUserName) || adminUsers.contains(currentUserBaseName));
-            isGuestUser = (guestUsers.contains(currentUserName) || guestUsers.contains(currentUserBaseName));
-        }
         
         // Check if any of the user's groups are listed as admin groups
         if (!isAdminUser && !adminGroups.isEmpty())
@@ -338,7 +312,43 @@ public class AuthorityServiceImpl implements AuthorityService, InitializingBean
 
     public void addAuthority(String parentName, String childName)
     {
-        authorityDAO.addAuthority(parentName, childName);
+        addAuthority(Collections.singleton(parentName), childName);
+    }
+
+    public void addAuthority(Collection<String> parentNames, String childName)
+    {
+        authorityDAO.addAuthority(parentNames, childName);
+    }
+
+    private boolean containsMatch(Set<String> names, String name)
+    {
+        String baseName = this.tenantService.getBaseNameUser(name);
+        if (this.tenantService.isEnabled())
+        {
+            // note: for multi-tenancy, this currently relies on a naming convention which assumes that all tenant
+            // admins will have the same base name as the default non-tenant specific admin. Typically "admin" is the
+            // default required admin user, although, if for example "bob" is also listed as an admin then all
+            // tenant-specific bob's will also have admin authority
+            for (String candidate : names)
+            {
+                if (this.userNameMatcher.matches(candidate, name)
+                        || this.userNameMatcher.matches(this.tenantService.getBaseNameUser(candidate), baseName))
+                {
+                    return true;
+                }
+            }
+        }
+        else
+        {
+            for (String candidate : names)
+            {
+                if (this.userNameMatcher.matches(candidate, name) || this.userNameMatcher.matches(candidate, baseName))
+                {
+                    return true;
+                }
+            }
+        }
+        return false;
     }
 
     private void checkTypeIsMutable(AuthorityType type)

source/java/org/alfresco/repo/security/authority/SimpleAuthorityServiceImpl.java

@@ -24,6 +24,7 @@
  */
 package org.alfresco.repo.security.authority;
 
+import java.util.Collection;
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.Set;
@@ -205,6 +206,11 @@ public class SimpleAuthorityServiceImpl implements AuthorityService
         
     }
 
+    public void addAuthority(Collection<String> parentNames, String childName)
+    {
+
+    }
+
     public String createAuthority(AuthorityType type, String shortName)
     {
        return "";

source/java/org/alfresco/repo/security/sync/ChainingUserRegistrySynchronizer.java

@@ -25,12 +25,16 @@
 package org.alfresco.repo.security.sync;
 
 import java.text.DateFormat;
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.Comparator;
 import java.util.Date;
 import java.util.HashSet;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
 import java.util.Set;
+import java.util.TreeMap;
 import java.util.TreeSet;
 
 import org.alfresco.model.ContentModel;
@@ -55,7 +59,6 @@ import org.alfresco.service.cmr.security.PersonService;
 import org.alfresco.service.namespace.NamespaceService;
 import org.alfresco.service.namespace.QName;
 import org.alfresco.util.AbstractLifecycleBean;
-import org.alfresco.util.Pair;
 import org.alfresco.util.PropertyMap;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -365,17 +368,7 @@ public class ChainingUserRegistrySynchronizer extends AbstractLifecycleBean impl
                         boolean requiresNew = splitTxns
                                 || AlfrescoTransactionSupport.getTransactionReadState() == TxnReadState.TXN_READ_ONLY;
 
-                        int personsProcessed = syncPersonsWithPlugin(id, plugin, requiresNew, visitedZoneIds,
+                        syncWithPlugin(id, plugin, force, requiresNew, visitedZoneIds, allZoneIds);
-                                allZoneIds);
-                        int groupsProcessed = syncGroupsWithPlugin(id, plugin, force, requiresNew, visitedZoneIds,
-                                allZoneIds);
-                        if (ChainingUserRegistrySynchronizer.logger.isInfoEnabled())
-                        {
-                            ChainingUserRegistrySynchronizer.logger
-                                    .info("Finished synchronizing users and groups with user registry '" + id + "'");
-                            ChainingUserRegistrySynchronizer.logger.info(personsProcessed + " user(s) and "
-                                    + groupsProcessed + " group(s) processed");
-                        }
                     }
                 }
                 catch (NoSuchBeanDefinitionException e)
@@ -431,29 +424,31 @@ public class ChainingUserRegistrySynchronizer extends AbstractLifecycleBean impl
     }
 
     /**
-     * Synchronizes changes only to local users (persons) with a {@link UserRegistry} for a particular zone.
+     * Synchronizes local groups and users with a {@link UserRegistry} for a particular zone, optionally handling
+     * deletions.
      * 
      * @param zone
-     *            the zone id. This identifier is used to tag all created users, so that in the future we can tell those
+     *            the zone id. This identifier is used to tag all created groups and users, so that in the future we can
-     *            that have been deleted from the registry.
+     *            tell those that have been deleted from the registry.
      * @param userRegistry
      *            the user registry for the zone.
+     * @param force
+     *            <code>true</code> if user and group deletions are to be processed.
      * @param splitTxns
      *            Can the modifications to Alfresco be split across multiple transactions for maximum performance? If
-     *            <code>true</code>, users and groups are created/updated in batches of 10 for increased performance. If
+     *            <code>true</code>, users and groups are created/updated in batches for increased performance. If
      *            <code>false</code>, all users and groups are processed in the current transaction. This is required if
      *            calling synchronously (e.g. in response to an authentication event in the same transaction).
      * @param visitedZoneIds
      *            the set of zone ids already processed. These zones have precedence over the current zone when it comes
-     *            to user name 'collisions'. If a user is queried that already exists locally but is tagged with one of
+     *            to group name 'collisions'. If a user or group is queried that already exists locally but is tagged
-     *            the zones in this set, then it will be ignored as this zone has lower priority.
+     *            with one of the zones in this set, then it will be ignored as this zone has lower priority.
      * @param allZoneIds
      *            the set of all zone ids in the authentication chain. Helps us work out whether the zone information
-     *            recorded against a user is invalid for the current authentication chain and whether the user needs to
+     *            recorded against a user or group is invalid for the current authentication chain and whether the user
-     *            be 're-zoned'.
+     *            or group needs to be 're-zoned'.
-     * @return the number of users processed
      */
-    private int syncPersonsWithPlugin(final String zone, UserRegistry userRegistry, boolean splitTxns,
+    private void syncWithPlugin(final String zone, UserRegistry userRegistry, boolean force, boolean splitTxns,
             final Set<String> visitedZoneIds, final Set<String> allZoneIds)
     {
         // Create a prefixed zone ID for use with the authority service
@@ -462,9 +457,314 @@ public class ChainingUserRegistrySynchronizer extends AbstractLifecycleBean impl
         // The set of zones we associate with new objects (default plus registry specific)
         final Set<String> zoneSet = getZones(zoneId);
 
-        final long lastModifiedMillis = getMostRecentUpdateTime(
+        long lastModifiedMillis = getMostRecentUpdateTime(
-                ChainingUserRegistrySynchronizer.PERSON_LAST_MODIFIED_ATTRIBUTE, zoneId);
+                ChainingUserRegistrySynchronizer.GROUP_LAST_MODIFIED_ATTRIBUTE, zoneId);
-        final Date lastModified = lastModifiedMillis == -1 ? null : new Date(lastModifiedMillis);
+        Date lastModified = lastModifiedMillis == -1 ? null : new Date(lastModifiedMillis);
+
+        if (ChainingUserRegistrySynchronizer.logger.isInfoEnabled())
+        {
+            if (lastModified == null)
+            {
+                ChainingUserRegistrySynchronizer.logger.info("Retrieving all groups from user registry '" + zone + "'");
+            }
+            else
+            {
+                ChainingUserRegistrySynchronizer.logger.info("Retrieving groups changed since "
+                        + DateFormat.getDateTimeInstance().format(lastModified) + " from user registry '" + zone + "'");
+            }
+        }
+
+        // Get current set of known authorities
+        Set<String> allZoneAuthorities = this.retryingTransactionHelper.doInTransaction(
+                new RetryingTransactionCallback<Set<String>>()
+                {
+                    public Set<String> execute() throws Throwable
+                    {
+                        return ChainingUserRegistrySynchronizer.this.authorityService.getAllAuthoritiesInZone(zoneId,
+                                null);
+                    }
+                }, false, splitTxns);
+
+        // First, analyze the group structure. Create maps of authorities to their parents for associations to create
+        // and delete. Also deal with 'overlaps' with other zones in the authentication chain.
+        final BatchProcessor<NodeDescription> groupProcessor = new BatchProcessor<NodeDescription>(
+                this.retryingTransactionHelper, this.applicationEventPublisher, userRegistry.getGroups(lastModified),
+                zone + " Group Analysis", this.loggingInterval, this.workerThreads, 20);
+        class Analyzer implements Worker<NodeDescription>
+        {
+            private final Set<String> allZoneAuthorities;
+            private final Set<String> groupsToCreate = new TreeSet<String>();
+            private final Map<String, Set<String>> groupAssocsToCreate = new TreeMap<String, Set<String>>();
+            private final Map<String, Set<String>> groupAssocsToDelete = new TreeMap<String, Set<String>>();
+            private long latestTime;
+
+            public Analyzer(final Set<String> allZoneAuthorities, final long latestTime)
+            {
+                this.allZoneAuthorities = allZoneAuthorities;
+                this.latestTime = latestTime;
+            }
+
+            public long getLatestTime()
+            {
+                return this.latestTime;
+            }
+
+            public Set<String> getGroupsToCreate()
+            {
+                return this.groupsToCreate;
+            }
+
+            public Map<String, Set<String>> getGroupAssocsToCreate()
+            {
+                return this.groupAssocsToCreate;
+            }
+
+            public Map<String, Set<String>> getGroupAssocsToDelete()
+            {
+                return this.groupAssocsToDelete;
+            }
+
+            /*
+             * (non-Javadoc)
+             * @see org.alfresco.repo.security.sync.BatchProcessor.Worker#getIdentifier(java.lang.Object)
+             */
+            public String getIdentifier(NodeDescription entry)
+            {
+                return entry.getSourceId();
+            }
+
+            /*
+             * (non-Javadoc)
+             * @see org.alfresco.repo.security.sync.BatchProcessor.Worker#process(java.lang.Object)
+             */
+            public void process(NodeDescription group) throws Throwable
+            {
+                PropertyMap groupProperties = group.getProperties();
+                String groupName = (String) groupProperties.get(ContentModel.PROP_AUTHORITY_NAME);
+                String groupShortName = ChainingUserRegistrySynchronizer.this.authorityService.getShortName(groupName);
+                Set<String> groupZones = ChainingUserRegistrySynchronizer.this.authorityService
+                        .getAuthorityZones(groupName);
+
+                if (groupZones == null)
+                {
+                    // The group did not exist at all
+                    addAssociations(groupName, group.getChildAssociations(), false);
+                }
+                else
+                {
+                    // Check whether the group is in any of the authentication chain zones
+                    Set<String> intersection = new TreeSet<String>(groupZones);
+                    intersection.retainAll(allZoneIds);
+                    if (intersection.isEmpty())
+                    {
+                        // The group exists, but not in a zone that's in the authentication chain. May be due to
+                        // upgrade or zone changes. Let's re-zone them
+                        if (ChainingUserRegistrySynchronizer.logger.isWarnEnabled())
+                        {
+                            ChainingUserRegistrySynchronizer.logger.warn("Updating group '" + groupShortName
+                                    + "'. This group will in future be assumed to originate from user registry '"
+                                    + zone + "'.");
+                        }
+                        ChainingUserRegistrySynchronizer.this.authorityService.removeAuthorityFromZones(groupName,
+                                groupZones);
+                        ChainingUserRegistrySynchronizer.this.authorityService.addAuthorityToZones(groupName, zoneSet);
+                    }
+                    if (groupZones.contains(zoneId) || intersection.isEmpty())
+                    {
+                        // The group already existed in this zone or no valid zone: update the group
+                        updateAssociations(group, groupName);
+                    }
+                    else
+                    {
+                        // Check whether the group is in any of the higher priority authentication chain zones
+                        intersection.retainAll(visitedZoneIds);
+                        if (!intersection.isEmpty())
+                        {
+                            // A group that exists in a different zone with higher precedence
+                            return;
+                        }
+                        // The group existed, but in a zone with lower precedence
+                        if (ChainingUserRegistrySynchronizer.logger.isWarnEnabled())
+                        {
+                            ChainingUserRegistrySynchronizer.logger
+                                    .warn("Recreating occluded group '"
+                                            + groupShortName
+                                            + "'. This group was previously created through synchronization with a lower priority user registry.");
+                        }
+                        ChainingUserRegistrySynchronizer.this.authorityService.deleteAuthority(groupName);
+                        // create the group
+                        addAssociations(groupName, group.getChildAssociations(), false);
+                    }
+                }
+
+                synchronized (this)
+                {
+                    // Maintain the last modified date
+                    Date groupLastModified = group.getLastModified();
+                    if (groupLastModified != null)
+                    {
+                        this.latestTime = Math.max(this.latestTime, groupLastModified.getTime());
+                    }
+                }
+            }
+
+            private synchronized void updateAssociations(NodeDescription group, String groupName)
+            {
+                Set<String> oldChildren = ChainingUserRegistrySynchronizer.this.authorityService
+                        .getContainedAuthorities(null, groupName, true);
+                Set<String> newChildren = group.getChildAssociations();
+                Set<String> toDelete = new TreeSet<String>(oldChildren);
+                Set<String> toAdd = new TreeSet<String>(newChildren);
+                toDelete.removeAll(newChildren);
+                toAdd.removeAll(oldChildren);
+                addAssociations(groupName, toAdd, true);
+                deleteAssociations(groupName, toDelete);
+            }
+
+            private synchronized void addAssociations(String groupName, Set<String> children, boolean existed)
+            {
+                this.allZoneAuthorities.add(groupName);
+                if (!existed)
+                {
+                    this.groupsToCreate.add(groupName);
+                }
+                // Add an entry for the parent itself, in case it is a root group
+                Set<String> parents = this.groupAssocsToCreate.get(groupName);
+                if (parents == null)
+                {
+                    parents = new TreeSet<String>();
+                    this.groupAssocsToCreate.put(groupName, parents);
+                }
+                for (String child : children)
+                {
+                    parents = this.groupAssocsToCreate.get(child);
+                    if (parents == null)
+                    {
+                        parents = new TreeSet<String>();
+                        this.groupAssocsToCreate.put(child, parents);
+                    }
+                    parents.add(groupName);
+                }
+            }
+
+            private synchronized void deleteAssociations(String groupName, Set<String> children)
+            {
+                for (String child : children)
+                {
+                    // Make sure each child features as a key in the creation map
+                    addAssociations(child, Collections.<String> emptySet(), true);
+
+                    Set<String> parents = this.groupAssocsToDelete.get(child);
+                    if (parents == null)
+                    {
+                        parents = new TreeSet<String>();
+                        this.groupAssocsToDelete.put(child, parents);
+                    }
+                    parents.add(groupName);
+                }
+            }
+        }
+
+        Analyzer groupAnalyzer = new Analyzer(allZoneAuthorities, lastModifiedMillis);
+        int groupProcessedCount = groupProcessor.process(groupAnalyzer, splitTxns);
+        final Map<String, Set<String>> groupAssocsToCreate = groupAnalyzer.getGroupAssocsToCreate();
+        final Map<String, Set<String>> groupAssocsToDelete = groupAnalyzer.getGroupAssocsToDelete();
+
+        // Prune our set of authorities according to deletions
+        Set<String> deletionCandidates = null;
+        if (force)
+        {
+            deletionCandidates = new TreeSet<String>(allZoneAuthorities);
+            userRegistry.processDeletions(deletionCandidates);
+            allZoneAuthorities.removeAll(deletionCandidates);
+            groupAssocsToCreate.keySet().removeAll(deletionCandidates);
+            groupAssocsToDelete.keySet().removeAll(deletionCandidates);
+        }
+
+        // Sort the group associations in depth-first order (root groups first)
+        Map<String, Set<String>> sortedGroupAssociations = new LinkedHashMap<String, Set<String>>(groupAssocsToCreate
+                .size() * 2);
+        List<String> authorityPath = new ArrayList<String>(5);
+        for (String authority : groupAssocsToCreate.keySet())
+        {
+            if (allZoneAuthorities.contains(authority))
+            {
+                authorityPath.add(authority);
+                visitGroupAssociations(authorityPath, allZoneAuthorities, groupAssocsToCreate, sortedGroupAssociations);
+                authorityPath.clear();
+            }
+        }
+
+        // Add the groups and their parent associations in depth-first order
+        final Set<String> groupsToCreate = groupAnalyzer.getGroupsToCreate();
+        BatchProcessor<Map.Entry<String, Set<String>>> groupCreator = new BatchProcessor<Map.Entry<String, Set<String>>>(
+                this.retryingTransactionHelper, this.applicationEventPublisher, sortedGroupAssociations.entrySet(),
+                zone + " Group Creation and Association", this.loggingInterval, 1, 20);
+        groupCreator.process(new Worker<Map.Entry<String, Set<String>>>()
+        {
+
+            public String getIdentifier(Map.Entry<String, Set<String>> entry)
+            {
+                return entry.getKey() + " " + entry.getValue();
+            }
+
+            public void process(Map.Entry<String, Set<String>> entry) throws Throwable
+            {
+                Set<String> parents = entry.getValue();
+                String child = entry.getKey();
+
+                if (groupsToCreate.contains(child))
+                {
+                    String groupShortName = ChainingUserRegistrySynchronizer.this.authorityService.getShortName(child);
+                    if (ChainingUserRegistrySynchronizer.logger.isDebugEnabled())
+                    {
+                        ChainingUserRegistrySynchronizer.logger.debug("Creating group '" + groupShortName + "'");
+                    }
+                    // create the group
+                    ChainingUserRegistrySynchronizer.this.authorityService.createAuthority(AuthorityType
+                            .getAuthorityType(child), groupShortName, groupShortName, zoneSet);
+                }
+                if (!parents.isEmpty())
+                {
+                    if (ChainingUserRegistrySynchronizer.logger.isDebugEnabled())
+                    {
+                        for (String groupName : parents)
+                        {
+                            ChainingUserRegistrySynchronizer.logger.debug("Adding '"
+                                    + ChainingUserRegistrySynchronizer.this.authorityService.getShortName(child)
+                                    + "' to group '"
+                                    + ChainingUserRegistrySynchronizer.this.authorityService.getShortName(groupName)
+                                    + "'");
+                        }
+                    }
+                    ChainingUserRegistrySynchronizer.this.authorityService.addAuthority(parents, child);
+                }
+                Set<String> parentsToDelete = groupAssocsToDelete.get(child);
+                if (parentsToDelete != null && !parentsToDelete.isEmpty())
+                {
+                    for (String parent : parentsToDelete)
+                    {
+                        if (ChainingUserRegistrySynchronizer.logger.isDebugEnabled())
+                        {
+                            ChainingUserRegistrySynchronizer.logger
+                                    .debug("Removing '"
+                                            + ChainingUserRegistrySynchronizer.this.authorityService
+                                                    .getShortName(child)
+                                            + "' from group '"
+                                            + ChainingUserRegistrySynchronizer.this.authorityService
+                                                    .getShortName(parent) + "'");
+                        }
+                        ChainingUserRegistrySynchronizer.this.authorityService.removeAuthority(parent, child);
+                    }
+                }
+            }
+        }, splitTxns);
+
+        // Process persons and their parent associations
+
+        lastModifiedMillis = getMostRecentUpdateTime(ChainingUserRegistrySynchronizer.PERSON_LAST_MODIFIED_ATTRIBUTE,
+                zoneId);
+        lastModified = lastModifiedMillis == -1 ? null : new Date(lastModifiedMillis);
         if (ChainingUserRegistrySynchronizer.logger.isInfoEnabled())
         {
             if (lastModified == null)
@@ -479,10 +779,15 @@ public class ChainingUserRegistrySynchronizer extends AbstractLifecycleBean impl
         }
         final BatchProcessor<NodeDescription> personProcessor = new BatchProcessor<NodeDescription>(
                 this.retryingTransactionHelper, this.applicationEventPublisher, userRegistry.getPersons(lastModified),
-                zone + " User Creation", this.loggingInterval, this.workerThreads, 10);
+                zone + " User Creation and Association", this.loggingInterval, this.workerThreads, 10);
-        class CreationWorker implements Worker<NodeDescription>
+        class PersonWorker implements Worker<NodeDescription>
         {
-            private long latestTime = lastModifiedMillis;
+            private long latestTime;
+
+            public PersonWorker(final long latestTime)
+            {
+                this.latestTime = latestTime;
+            }
 
             public long getLatestTime()
             {
@@ -562,6 +867,42 @@ public class ChainingUserRegistrySynchronizer extends AbstractLifecycleBean impl
                         ChainingUserRegistrySynchronizer.this.personService.createPerson(personProperties, zoneSet);
                     }
                 }
+                // Maintain associations
+                Set<String> parents = groupAssocsToCreate.get(personName);
+                if (parents != null && !parents.isEmpty())
+                {
+                    if (ChainingUserRegistrySynchronizer.logger.isDebugEnabled())
+                    {
+                        for (String groupName : parents)
+                        {
+                            ChainingUserRegistrySynchronizer.logger.debug("Adding '"
+                                    + ChainingUserRegistrySynchronizer.this.authorityService.getShortName(personName)
+                                    + "' to group '"
+                                    + ChainingUserRegistrySynchronizer.this.authorityService.getShortName(groupName)
+                                    + "'");
+                        }
+                    }
+                    ChainingUserRegistrySynchronizer.this.authorityService.addAuthority(parents, personName);
+                }
+                Set<String> parentsToDelete = groupAssocsToDelete.get(personName);
+                if (parentsToDelete != null && !parentsToDelete.isEmpty())
+                {
+                    for (String parent : parentsToDelete)
+                    {
+                        if (ChainingUserRegistrySynchronizer.logger.isDebugEnabled())
+                        {
+                            ChainingUserRegistrySynchronizer.logger
+                                    .debug("Removing '"
+                                            + ChainingUserRegistrySynchronizer.this.authorityService
+                                                    .getShortName(personName)
+                                            + "' from group '"
+                                            + ChainingUserRegistrySynchronizer.this.authorityService
+                                                    .getShortName(parent) + "'");
+                        }
+                        ChainingUserRegistrySynchronizer.this.authorityService.removeAuthority(parent, personName);
+                    }
+                }
+
                 synchronized (this)
                 {
                     // Maintain the last modified date
@@ -574,302 +915,43 @@ public class ChainingUserRegistrySynchronizer extends AbstractLifecycleBean impl
             }
         }
 
-        CreationWorker creations = new CreationWorker();
+        PersonWorker persons = new PersonWorker(lastModifiedMillis);
-        int processedCount = personProcessor.process(creations, splitTxns);
+        int personProcessedCount = personProcessor.process(persons, splitTxns);
-        long latestTime = creations.getLatestTime();
-        if (latestTime != -1)
-        {
-            setMostRecentUpdateTime(ChainingUserRegistrySynchronizer.PERSON_LAST_MODIFIED_ATTRIBUTE, zoneId,
-                    latestTime, splitTxns);
-        }
 
-        // Remember we have visited this zone
+        // Only now that the whole tree has been processed is it safe to persist the last modified dates
-        visitedZoneIds.add(zoneId);
+        long latestTime = groupAnalyzer.getLatestTime();
-
-        return processedCount;
-    }
-
-    /**
-     * Synchronizes local groups with a {@link UserRegistry} for a particular zone and also handles deletions of local
-     * groups and users.
-     * 
-     * @param zone
-     *            the zone id. This identifier is used to tag all created groups, so that in the future we can tell
-     *            those that have been deleted from the registry.
-     * @param userRegistry
-     *            the user registry for the zone.
-     * @param force
-     *            <code>true</code> if user and group deletions are to be processed.
-     * @param splitTxns
-     *            Can the modifications to Alfresco be split across multiple transactions for maximum performance? If
-     *            <code>true</code>, users and groups are created/updated in batches of 10 for increased performance. If
-     *            <code>false</code>, all users and groups are processed in the current transaction. This is required if
-     *            calling synchronously (e.g. in response to an authentication event in the same transaction).
-     * @param visitedZoneIds
-     *            the set of zone ids already processed. These zones have precedence over the current zone when it comes
-     *            to group name 'collisions'. If a group is queried that already exists locally but is tagged with one
-     *            of the zones in this set, then it will be ignored as this zone has lower priority.
-     * @param allZoneIds
-     *            the set of all zone ids in the authentication chain. Helps us work out whether the zone information
-     *            recorded against a group is invalid for the current authentication chain and whether the group needs
-     *            to be 're-zoned'.
-     * @return the number of groups processed
-     */
-    private int syncGroupsWithPlugin(final String zone, UserRegistry userRegistry, boolean force, boolean splitTxns,
-            final Set<String> visitedZoneIds, final Set<String> allZoneIds)
-    {
-        // Create a prefixed zone ID for use with the authority service
-        final String zoneId = AuthorityService.ZONE_AUTH_EXT_PREFIX + zone;
-
-        // The set of zones we associate with new objects (default plus registry specific)
-        final Set<String> zoneSet = getZones(zoneId);
-
-        final long lastModifiedMillis = getMostRecentUpdateTime(
-                ChainingUserRegistrySynchronizer.GROUP_LAST_MODIFIED_ATTRIBUTE, zoneId);
-        final Date lastModified = lastModifiedMillis == -1 ? null : new Date(lastModifiedMillis);
-
-        if (ChainingUserRegistrySynchronizer.logger.isInfoEnabled())
-        {
-            if (lastModified == null)
-            {
-                ChainingUserRegistrySynchronizer.logger.info("Retrieving all groups from user registry '" + zone + "'");
-            }
-            else
-            {
-                ChainingUserRegistrySynchronizer.logger.info("Retrieving groups changed since "
-                        + DateFormat.getDateTimeInstance().format(lastModified) + " from user registry '" + zone + "'");
-            }
-        }
-
-        // Get current set of known authorities
-        Set<String> deletionCandidates = this.retryingTransactionHelper.doInTransaction(
-                new RetryingTransactionCallback<Set<String>>()
-                {
-                    public Set<String> execute() throws Throwable
-                    {
-                        return ChainingUserRegistrySynchronizer.this.authorityService.getAllAuthoritiesInZone(zoneId,
-                                null);
-                    }
-                }, false, splitTxns);
-
-        final BatchProcessor<NodeDescription> groupProcessor = new BatchProcessor<NodeDescription>(
-                this.retryingTransactionHelper, this.applicationEventPublisher, userRegistry.getGroups(lastModified,
-                        deletionCandidates, force), zone + " Group Creation", this.loggingInterval, this.workerThreads,
-                20);
-        class CreationWorker implements Worker<NodeDescription>
-        {
-            private final Set<Pair<String, String>> groupAssocsToCreate = new TreeSet<Pair<String, String>>(
-                    new Comparator<Pair<String, String>>()
-                    {
-
-                        public int compare(Pair<String, String> o1, Pair<String, String> o2)
-                        {
-                            int result = o1.getFirst().compareTo(o2.getFirst());
-                            if (result == 0)
-                            {
-                                return o1.getSecond().compareTo(o2.getSecond());
-                            }
-                            return result;
-                        }
-                    });
-            private long latestTime = lastModifiedMillis;
-
-            public long getLatestTime()
-            {
-                return this.latestTime;
-            }
-
-            public Set<Pair<String, String>> getGroupAssocsToCreate()
-            {
-                return this.groupAssocsToCreate;
-            }
-
-            /*
-             * (non-Javadoc)
-             * @see org.alfresco.repo.security.sync.BatchProcessor.Worker#getIdentifier(java.lang.Object)
-             */
-            public String getIdentifier(NodeDescription entry)
-            {
-                return entry.getSourceId();
-            }
-
-            /*
-             * (non-Javadoc)
-             * @see org.alfresco.repo.security.sync.BatchProcessor.Worker#process(java.lang.Object)
-             */
-            public void process(NodeDescription group) throws Throwable
-            {
-                PropertyMap groupProperties = group.getProperties();
-                String groupName = (String) groupProperties.get(ContentModel.PROP_AUTHORITY_NAME);
-                String groupShortName = ChainingUserRegistrySynchronizer.this.authorityService.getShortName(groupName);
-                Set<String> groupZones = ChainingUserRegistrySynchronizer.this.authorityService
-                        .getAuthorityZones(groupName);
-
-                if (groupZones == null)
-                {
-                    // The group did not exist at all
-                    if (ChainingUserRegistrySynchronizer.logger.isDebugEnabled())
-                    {
-                        ChainingUserRegistrySynchronizer.logger.debug("Creating group '" + groupShortName + "'");
-                    }
-                    // create the group
-                    ChainingUserRegistrySynchronizer.this.authorityService.createAuthority(AuthorityType
-                            .getAuthorityType(groupName), groupShortName, (String) groupProperties
-                            .get(ContentModel.PROP_AUTHORITY_DISPLAY_NAME), zoneSet);
-                    Set<String> children = group.getChildAssociations();
-                    if (!children.isEmpty())
-                    {
-                        synchronized (this)
-                        {
-                            for (String child : children)
-                            {
-                                this.groupAssocsToCreate.add(new Pair<String, String>(groupName, child));
-                            }
-                        }
-                    }
-                }
-                else
-                {
-                    // Check whether the group is in any of the authentication chain zones
-                    Set<String> intersection = new TreeSet<String>(groupZones);
-                    intersection.retainAll(allZoneIds);
-                    if (intersection.isEmpty())
-                    {
-                        // The group exists, but not in a zone that's in the authentication chain. May be due to
-                        // upgrade or zone changes. Let's re-zone them
-                        if (ChainingUserRegistrySynchronizer.logger.isWarnEnabled())
-                        {
-                            ChainingUserRegistrySynchronizer.logger.warn("Updating group '" + groupShortName
-                                    + "'. This group will in future be assumed to originate from user registry '"
-                                    + zone + "'.");
-                        }
-                        ChainingUserRegistrySynchronizer.this.authorityService.removeAuthorityFromZones(groupName,
-                                groupZones);
-                        ChainingUserRegistrySynchronizer.this.authorityService.addAuthorityToZones(groupName, zoneSet);
-                    }
-                    if (groupZones.contains(zoneId) || intersection.isEmpty())
-                    {
-                        // The group already existed in this zone or no valid zone: update the group
-                        Set<String> oldChildren = ChainingUserRegistrySynchronizer.this.authorityService
-                                .getContainedAuthorities(null, groupName, true);
-                        Set<String> newChildren = group.getChildAssociations();
-                        Set<String> toDelete = new TreeSet<String>(oldChildren);
-                        Set<String> toAdd = new TreeSet<String>(newChildren);
-                        toDelete.removeAll(newChildren);
-                        toAdd.removeAll(oldChildren);
-                        if (!toAdd.isEmpty())
-                        {
-                            synchronized (this)
-                            {
-                                for (String child : toAdd)
-                                {
-                                    this.groupAssocsToCreate.add(new Pair<String, String>(groupName, child));
-                                }
-                            }
-                        }
-                        for (String child : toDelete)
-                        {
-                            if (ChainingUserRegistrySynchronizer.logger.isDebugEnabled())
-                            {
-                                ChainingUserRegistrySynchronizer.logger.debug("Removing '"
-                                        + ChainingUserRegistrySynchronizer.this.authorityService.getShortName(child)
-                                        + "' from group '" + groupShortName + "'");
-                            }
-                            ChainingUserRegistrySynchronizer.this.authorityService.removeAuthority(groupName, child);
-                        }
-                    }
-                    else
-                    {
-                        // Check whether the group is in any of the higher priority authentication chain zones
-                        intersection.retainAll(visitedZoneIds);
-                        if (!intersection.isEmpty())
-                        {
-                            // A group that exists in a different zone with higher precedence
-                            return;
-                        }
-                        // The group existed, but in a zone with lower precedence
-                        if (ChainingUserRegistrySynchronizer.logger.isWarnEnabled())
-                        {
-                            ChainingUserRegistrySynchronizer.logger
-                                    .warn("Recreating occluded group '"
-                                            + groupShortName
-                                            + "'. This group was previously created through synchronization with a lower priority user registry.");
-                        }
-                        ChainingUserRegistrySynchronizer.this.authorityService.deleteAuthority(groupName);
-                        // create the group
-                        ChainingUserRegistrySynchronizer.this.authorityService.createAuthority(AuthorityType
-                                .getAuthorityType(groupName), groupShortName, (String) groupProperties
-                                .get(ContentModel.PROP_AUTHORITY_DISPLAY_NAME), zoneSet);
-                        Set<String> children = group.getChildAssociations();
-                        if (!children.isEmpty())
-                        {
-                            synchronized (this)
-                            {
-                                for (String child : children)
-                                {
-                                    this.groupAssocsToCreate.add(new Pair<String, String>(groupName, child));
-                                }
-                            }
-                        }
-                    }
-                }
-
-                synchronized (this)
-                {
-                    // Maintain the last modified date
-                    Date groupLastModified = group.getLastModified();
-                    if (groupLastModified != null)
-                    {
-                        this.latestTime = Math.max(this.latestTime, groupLastModified.getTime());
-                    }
-                }
-            }
-        }
-
-        CreationWorker creations = new CreationWorker();
-        int processedCount = groupProcessor.process(creations, splitTxns);
-        long latestTime = creations.getLatestTime();
         if (latestTime != -1)
         {
             setMostRecentUpdateTime(ChainingUserRegistrySynchronizer.GROUP_LAST_MODIFIED_ATTRIBUTE, zoneId, latestTime,
                     splitTxns);
         }
-
+        latestTime = persons.getLatestTime();
-        // Add the new associations, now that we have created everything
+        if (latestTime != -1)
-        BatchProcessor<Pair<String, String>> groupAssocProcessor = new BatchProcessor<Pair<String, String>>(
-                this.retryingTransactionHelper, this.applicationEventPublisher, creations.getGroupAssocsToCreate(),
-                zone + " Group Association Creation", this.loggingInterval, this.workerThreads, 20);
-        groupAssocProcessor.process(new Worker<Pair<String, String>>()
         {
-
+            setMostRecentUpdateTime(ChainingUserRegistrySynchronizer.PERSON_LAST_MODIFIED_ATTRIBUTE, zoneId,
-            public String getIdentifier(Pair<String, String> entry)
+                    latestTime, splitTxns);
-            {
-                return entry.toString();
         }
 
-            public void process(Pair<String, String> entry) throws Throwable
-            {
-                String groupName = entry.getFirst();
-                String child = entry.getSecond();
-                if (ChainingUserRegistrySynchronizer.logger.isDebugEnabled())
-                {
-                    ChainingUserRegistrySynchronizer.logger.debug("Adding '"
-                            + ChainingUserRegistrySynchronizer.this.authorityService.getShortName(child)
-                            + "' to group '"
-                            + ChainingUserRegistrySynchronizer.this.authorityService.getShortName(groupName) + "'");
-                }
-                ChainingUserRegistrySynchronizer.this.authorityService.addAuthority(groupName, child);
-            }
-        }, splitTxns);
-
         // Delete authorities if we have complete information for the zone
         if (force)
         {
             BatchProcessor<String> authorityDeletionProcessor = new BatchProcessor<String>(
                     this.retryingTransactionHelper, this.applicationEventPublisher, deletionCandidates, zone
                             + " Authority Deletion", this.loggingInterval, this.workerThreads, 10);
-            processedCount += authorityDeletionProcessor.process(new Worker<String>()
+            class AuthorityDeleter implements Worker<String>
             {
+                private int personProcessedCount;
+                private int groupProcessedCount;
+
+                public int getPersonProcessedCount()
+                {
+                    return this.personProcessedCount;
+                }
+
+                public int getGroupProcessedCount()
+                {
+                    return this.groupProcessedCount;
+                }
 
                 public String getIdentifier(String entry)
                 {
@@ -885,6 +967,10 @@ public class ChainingUserRegistrySynchronizer extends AbstractLifecycleBean impl
                             ChainingUserRegistrySynchronizer.logger.debug("Deleting user '" + authority + "'");
                         }
                         ChainingUserRegistrySynchronizer.this.personService.deletePerson(authority);
+                        synchronized (this)
+                        {
+                            this.personProcessedCount++;
+                        }
                     }
                     else
                     {
@@ -895,15 +981,76 @@ public class ChainingUserRegistrySynchronizer extends AbstractLifecycleBean impl
                                     + "'");
                         }
                         ChainingUserRegistrySynchronizer.this.authorityService.deleteAuthority(authority);
+                        synchronized (this)
+                        {
+                            this.groupProcessedCount++;
                         }
                     }
-            }, splitTxns);
+                }
+            }
+            AuthorityDeleter authorityDeleter = new AuthorityDeleter();
+            authorityDeletionProcessor.process(authorityDeleter, splitTxns);
+            groupProcessedCount += authorityDeleter.getGroupProcessedCount();
+            personProcessedCount += authorityDeleter.getPersonProcessedCount();
         }
 
         // Remember we have visited this zone
         visitedZoneIds.add(zoneId);
 
-        return processedCount;
+        if (ChainingUserRegistrySynchronizer.logger.isInfoEnabled())
+        {
+            ChainingUserRegistrySynchronizer.logger.info("Finished synchronizing users and groups with user registry '"
+                    + zone + "'");
+            ChainingUserRegistrySynchronizer.logger.info(personProcessedCount + " user(s) and " + groupProcessedCount
+                    + " group(s) processed");
+        }
+    }
+
+    /**
+     * Visits the last authority in the given list by recursively visiting its parents in associationsOld and then
+     * adding the authority to associationsNew. Used to sort associationsOld into 'depth-first' order.
+     * 
+     * @param authorityPath
+     *            The authority to visit, preceeded by all its descendants. Allows detection of cyclic child
+     *            associations.
+     * @param allAuthorities
+     *            the set of all known authorities
+     * @param associationsOld
+     *            the association map to sort
+     * @param associationsNew
+     *            the association map to add to in depth first order
+     */
+    private void visitGroupAssociations(List<String> authorityPath, Set<String> allAuthorities,
+            Map<String, Set<String>> associationsOld, Map<String, Set<String>> associationsNew)
+    {
+        String authorityName = authorityPath.get(authorityPath.size() - 1);
+        if (!associationsNew.containsKey(authorityName))
+        {
+            Set<String> associations = associationsOld.get(authorityName);
+
+            if (!associations.isEmpty())
+            {
+                // Filter out associations to unknown parent authorities
+                associations.retainAll(allAuthorities);
+                int insertIndex = authorityPath.size();
+                for (String parentAuthority : associations)
+                {
+                    // Prevent cyclic paths
+                    if (!authorityPath.contains(parentAuthority))
+                    {
+                        authorityPath.add(parentAuthority);
+                        visitGroupAssociations(authorityPath, allAuthorities, associationsOld, associationsNew);
+                        authorityPath.remove(insertIndex);
+                    }
+                }
+            }
+
+            // Omit associations from users from this map, as they will be processed separately
+            if (AuthorityType.getAuthorityType(authorityName) != AuthorityType.USER)
+            {
+                associationsNew.put(authorityName, associations);
+            }
+        }
     }
 
     /**

source/java/org/alfresco/repo/security/sync/ChainingUserRegistrySynchronizerTest.java

@@ -632,14 +632,12 @@ public class ChainingUserRegistrySynchronizerTest extends TestCase
             return this.zoneId;
         }
         
-        /*
+        
-         * (non-Javadoc)
+
-         * @see org.alfresco.repo.security.sync.UserRegistry#getGroups(java.util.Date, java.util.Set, boolean)
+        /* (non-Javadoc)
+         * @see org.alfresco.repo.security.sync.UserRegistry#processDeletions(java.util.Set)
          */
-        public Collection<NodeDescription> getGroups(Date modifiedSince, Set<String> candidateAuthoritiesForDeletion,
+        public void processDeletions(Set<String> candidateAuthoritiesForDeletion)
-                boolean prune)
-        {
-            if (prune)
         {
             for (NodeDescription person : this.persons)
             {
@@ -650,6 +648,12 @@ public class ChainingUserRegistrySynchronizerTest extends TestCase
                 candidateAuthoritiesForDeletion.remove(group.getProperties().get(ContentModel.PROP_AUTHORITY_NAME));
             }
         }
+
+        /* (non-Javadoc)
+         * @see org.alfresco.repo.security.sync.UserRegistry#getGroups(java.util.Date)
+         */
+        public Collection<NodeDescription> getGroups(Date modifiedSince)
+        {
             return this.groups;
         }
 

source/java/org/alfresco/repo/security/sync/UserRegistry.java

@@ -50,22 +50,23 @@ public interface UserRegistry
     public Collection<NodeDescription> getPersons(Date modifiedSince);
 
     /**
-     * Gets descriptions of all the groups in the user registry or all those changed since a certain date. Group
+     * Gets descriptions of all the groups in the user registry or all those changed since a certain date.
-     * associations should be restricted to those in the given set of known authorities. Optionally this set is 'pruned'
-     * to contain only those authorities that no longer exist in the user registry, i.e. the deletion candidates.
      * 
      * @param modifiedSince
      *            if non-null, then only descriptions of groups modified since this date should be returned; if
      *            <code>null</code> then descriptions of all groups should be returned.
-     * @param knownAuthorities
-     *            the current set of known authorities
-     * @param prune
-     *            should this set be 'pruned' so that it contains only those authorities that do not exist in the
-     *            registry, i.e. the deletion candidates?
      * @return a {@link Collection} of {@link NodeDescription}s of all the groups in the user registry or all those
      *         changed since a certain date. The description properties should correspond to those of an Alfresco
      *         authority node.
      */
-    public Collection<NodeDescription> getGroups(Date modifiedSince, Set<String> knownAuthorities, boolean prune);
+    public Collection<NodeDescription> getGroups(Date modifiedSince);
 
+    /**
+     * Retrieves the complete set of known users and groups from the user registry and removes them from the set of
+     * candidate local authorities to be deleted.
+     * 
+     * @param candidateAuthoritiesForDeletion
+     *            the candidate authorities for deletion
+     */
+    public void processDeletions(final Set<String> candidateAuthoritiesForDeletion);
 }

source/java/org/alfresco/repo/security/sync/ldap/LDAPUserRegistry.java

@@ -480,14 +480,11 @@ public class LDAPUserRegistry implements UserRegistry, LDAPNameResolver, Initial
         return new PersonCollection(modifiedSince);
     }
 
-    /**
+    /*
-     * Retrieves the complete set of known users and groups from the LDAP directory and removes them from the set of
+     * (non-Javadoc)
-     * candidate local authorities to be deleted.
+     * @see org.alfresco.repo.security.sync.UserRegistry#processDeletions(java.util.Set)
-     * 
-     * @param candidateAuthoritiesForDeletion
-     *            the candidate authorities for deletion
      */
-    private void processDeletions(final Set<String> candidateAuthoritiesForDeletion)
+    public void processDeletions(final Set<String> candidateAuthoritiesForDeletion)
     {
         processQuery(new SearchCallback()
         {
@@ -562,20 +559,8 @@ public class LDAPUserRegistry implements UserRegistry, LDAPNameResolver, Initial
      * (non-Javadoc)
      * @see org.alfresco.repo.security.sync.UserRegistry#getGroups(java.util.Date)
      */
-    public Collection<NodeDescription> getGroups(Date modifiedSince, final Set<String> candidateAuthoritiesForDeletion,
+    public Collection<NodeDescription> getGroups(Date modifiedSince)
-            boolean prune)
     {
-        // Take the given set of authorities as a starting point for the set of all authorities
-        final Set<String> allAuthorities = new TreeSet<String>(candidateAuthoritiesForDeletion);
-
-        // If required, work out what authority deletions are required by pruning down the deletion set and the set of
-        // all authorities
-        if (prune)
-        {
-            processDeletions(candidateAuthoritiesForDeletion);
-            allAuthorities.removeAll(candidateAuthoritiesForDeletion);
-        }
-
         // Work out whether the user and group trees are disjoint. This may allow us to optimize reverse DN
         // resolution.
         final LdapName groupDistinguishedNamePrefix;
@@ -636,7 +621,6 @@ public class LDAPUserRegistry implements UserRegistry, LDAPNameResolver, Initial
                     group = new NodeDescription(result.getNameInNamespace());
                     group.getProperties().put(ContentModel.PROP_AUTHORITY_NAME, gid);
                     lookup.put(gid, group);
-                    allAuthorities.add(gid);
                 }
                 else if (LDAPUserRegistry.this.errorOnDuplicateGID)
                 {
@@ -799,12 +783,6 @@ public class LDAPUserRegistry implements UserRegistry, LDAPNameResolver, Initial
             LDAPUserRegistry.logger.debug("Found " + lookup.size());
         }
 
-        // Post-process the group associations to filter out those that point to excluded users or groups (now that we
-        // know the full set of groups)
-        for (NodeDescription group : lookup.values())
-        {
-            group.getChildAssociations().retainAll(allAuthorities);
-        }
         return lookup.values();
     }
 

source/java/org/alfresco/repo/version/NodeServiceImpl.java

@@ -234,6 +234,18 @@ public class NodeServiceImpl implements NodeService, VersionModel
         throw new UnsupportedOperationException(MSG_UNSUPPORTED);
     }
 
+    /**
+     * @throws UnsupportedOperationException always
+     */
+    public List<ChildAssociationRef> addChild(Collection<NodeRef> parentRefs,
+            NodeRef childRef,
+            QName assocTypeQName,
+            QName qname) throws InvalidNodeRefException
+    {
+        // This operation is not supported for a version store
+        throw new UnsupportedOperationException(MSG_UNSUPPORTED);
+    }
+
     /**
      * @throws UnsupportedOperationException always
      */

source/java/org/alfresco/service/cmr/repository/NodeService.java

@@ -324,6 +324,26 @@ public interface NodeService
             QName assocTypeQName,
             QName qname) throws InvalidNodeRefException;
     
+    /**
+     * Associates a given child node with a given collection of parents.  All nodes must belong to the same store.
+     * <p>
+     * 
+     * 
+     * @param parentRefs
+     * @param childRef 
+     * @param assocTypeQName the qualified name of the association type as defined in the datadictionary
+     * @param qname the qualified name of the association
+     * @return Returns a reference to the newly created child association
+     * @throws InvalidNodeRefException if the parent or child nodes could not be found
+     * @throws CyclicChildRelationshipException if the child partakes in a cyclic relationship after the add
+     */
+    @Auditable(key = Auditable.Key.ARG_0 ,parameters = {"parentRefs", "childRef", "assocTypeQName", "qname"})
+    public List<ChildAssociationRef> addChild(
+            Collection<NodeRef> parentRefs,
+            NodeRef childRef,
+            QName assocTypeQName,
+            QName qname) throws InvalidNodeRefException;
+
     /**
      * Severs all parent-child relationships between two nodes.
      * <p>

source/java/org/alfresco/service/cmr/security/AuthorityService.java

@@ -24,6 +24,7 @@
  */
 package org.alfresco.service.cmr.security;
 
+import java.util.Collection;
 import java.util.Set;
 
 import org.alfresco.service.Auditable;
@@ -215,6 +216,18 @@ public interface AuthorityService
     @Auditable(parameters = {"parentName", "childName"})
     public void addAuthority(String parentName, String childName);
 
+    /**
+     * Set a given child authority to be included by the given parent authorities. For example, adding a
+     * group to groups or adding a user to groups.
+     * 
+     * @param parentNames -
+     *            the full name string identifier for the parents.
+     * @param childName -
+     *            the string identifier for the child.
+     */
+    @Auditable(parameters = {"parentNames", "childName"})
+    public void addAuthority(Collection<String> parentNames, String childName);
+
     /**
      * Remove an authority as a member of another authority. The child authority
      * will still exist. If the child authority was not created as a root