From 5d0d4fb5edebb72339b5b5ed89ff19f0c6d92e75 Mon Sep 17 00:00:00 2001 From: Alan Davis Date: Wed, 12 Feb 2014 00:14:46 +0000 Subject: [PATCH] Merged HEAD-BUG-FIX (4.3/Cloud) to HEAD (4.3/Cloud) 58817: Merged V4.2-BUG-FIX (4.2.1) to HEAD-BUG-FIX (Cloud/4.3) 58784: Merged DEV to V4.2-BUG-FIX (4.2.1) 58617 : MNT-9711: ldap synchronization algorithm is not optimal - The verification for deleting permissions was added for the full synchronization. - The unit tests were modified according the new logic. 58667 : MNT-9711: ldap synchronization algorithm is not optimal - Refactoring code. Removed unreachable code. git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@62038 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261 --- .../ChainingUserRegistrySynchronizer.java | 122 +++--------------- .../ChainingUserRegistrySynchronizerTest.java | 34 ++--- 2 files changed, 25 insertions(+), 131 deletions(-) diff --git a/source/java/org/alfresco/repo/security/sync/ChainingUserRegistrySynchronizer.java b/source/java/org/alfresco/repo/security/sync/ChainingUserRegistrySynchronizer.java index 0717895e0e..938f622df9 100644 --- a/source/java/org/alfresco/repo/security/sync/ChainingUserRegistrySynchronizer.java +++ b/source/java/org/alfresco/repo/security/sync/ChainingUserRegistrySynchronizer.java @@ -18,40 +18,6 @@ */ package org.alfresco.repo.security.sync; -import java.io.IOException; -import java.io.Serializable; -import java.io.UnsupportedEncodingException; -import java.net.URLDecoder; -import java.text.DateFormat; -import java.util.Collection; -import java.util.Collections; -import java.util.Date; -import java.util.Deque; -import java.util.HashMap; -import java.util.HashSet; -import java.util.Iterator; -import java.util.LinkedHashMap; -import java.util.LinkedList; -import java.util.List; -import java.util.Map; -import java.util.Set; -import java.util.TreeMap; -import java.util.TreeSet; -import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.ScheduledThreadPoolExecutor; -import java.util.concurrent.TimeUnit; - -import javax.management.AttributeNotFoundException; -import javax.management.InstanceNotFoundException; -import javax.management.IntrospectionException; -import javax.management.MBeanAttributeInfo; -import javax.management.MBeanException; -import javax.management.MBeanInfo; -import javax.management.MBeanServerConnection; -import javax.management.MalformedObjectNameException; -import javax.management.ObjectName; -import javax.management.ReflectionException; - import org.alfresco.model.ContentModel; import org.alfresco.repo.admin.SysAdminParams; import org.alfresco.repo.batch.BatchProcessor; @@ -87,12 +53,21 @@ import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationEvent; import org.springframework.context.ApplicationEventPublisher; import org.springframework.context.ApplicationEventPublisherAware; -import org.springframework.context.ApplicationListener; -import org.springframework.context.event.ContextRefreshedEvent; import org.springframework.dao.ConcurrencyFailureException; import org.springframework.extensions.surf.util.AbstractLifecycleBean; import org.springframework.extensions.surf.util.I18NUtil; +import javax.management.*; +import java.io.IOException; +import java.io.Serializable; +import java.io.UnsupportedEncodingException; +import java.net.URLDecoder; +import java.text.DateFormat; +import java.util.*; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledThreadPoolExecutor; +import java.util.concurrent.TimeUnit; + /** * A ChainingUserRegistrySynchronizer is responsible for synchronizing Alfresco's local user (person) and * group (authority) information with the external subsystems in the authentication chain (most typically LDAP @@ -1469,8 +1444,8 @@ public class ChainingUserRegistrySynchronizer extends AbstractLifecycleBean private void processGroups(UserRegistry userRegistry, boolean isFullSync, boolean splitTxns) { // If we got back some groups, we have to cross reference them with the set of known authorities - if (isFullSync || !this.groupParentAssocsToDelete.isEmpty() - || !this.groupParentAssocsToDelete.isEmpty()) + // MNT-9711 fix. If allowDeletions is false, there is no need to pull all users and all groups from LDAP during the full synchronization. + if (allowDeletions && (isFullSync || !this.groupParentAssocsToDelete.isEmpty())) { final Set allZonePersons = newPersonSet(); final Set allZoneGroups = new TreeSet(); @@ -1515,76 +1490,9 @@ public class ChainingUserRegistrySynchronizer extends AbstractLifecycleBean this.deletionCandidates = new TreeSet(); this.deletionCandidates.addAll(personDeletionCandidates); this.deletionCandidates.addAll(groupDeletionCandidates); - if (allowDeletions) - { - allZonePersons.removeAll(personDeletionCandidates); - allZoneGroups.removeAll(groupDeletionCandidates); - } - else - { - if (!personDeletionCandidates.isEmpty()) - { - ChainingUserRegistrySynchronizer.logger.warn("The following missing users are not being deleted as allowDeletions == false"); - for (String person : personDeletionCandidates) - { - ChainingUserRegistrySynchronizer.logger.warn(" " + person); - } - } - if (!groupDeletionCandidates.isEmpty()) - { - ChainingUserRegistrySynchronizer.logger.warn("The following missing groups are not being deleted as allowDeletions == false"); - for (String group : groupDeletionCandidates) - { - ChainingUserRegistrySynchronizer.logger.warn(" " + group); - } - } - - // Complete association deletion information by scanning deleted groups - // Batch 3 - Missing Authority Scanning", - BatchProcessor groupScanner = new BatchProcessor( - SyncProcess.MISSING_AUTHORITY.getTitle(zone), - ChainingUserRegistrySynchronizer.this.transactionService - .getRetryingTransactionHelper(), this.deletionCandidates, - ChainingUserRegistrySynchronizer.this.workerThreads, 20, - ChainingUserRegistrySynchronizer.this.applicationEventPublisher, - ChainingUserRegistrySynchronizer.logger, - ChainingUserRegistrySynchronizer.this.loggingInterval); - groupScanner.process(new BaseBatchProcessWorker() - { - @Override - public String getIdentifier(String entry) - { - return entry; - } - - @Override - public void process(String authority) throws Throwable - { - // Disassociate it from this zone, allowing it to be reclaimed by something further down the chain - ChainingUserRegistrySynchronizer.this.authorityService.removeAuthorityFromZones(authority, - Collections.singleton(zoneId)); - - // For groups, remove all members - if (AuthorityType.getAuthorityType(authority) != AuthorityType.USER) - { - String groupShortName = ChainingUserRegistrySynchronizer.this.authorityService - .getShortName(authority); - String groupDisplayName = ChainingUserRegistrySynchronizer.this.authorityService - .getAuthorityDisplayName(authority); - NodeDescription dummy = new NodeDescription(groupShortName + " (Deleted)"); - PropertyMap dummyProperties = dummy.getProperties(); - dummyProperties.put(ContentModel.PROP_AUTHORITY_NAME, authority); - if (groupDisplayName != null) - { - dummyProperties.put(ContentModel.PROP_AUTHORITY_DISPLAY_NAME, groupDisplayName); - } - updateGroup(dummy, true); - } - } - }, splitTxns); - - } + allZonePersons.removeAll(personDeletionCandidates); + allZoneGroups.removeAll(groupDeletionCandidates); } // Prune the group associations now that we have complete information diff --git a/source/test-java/org/alfresco/repo/security/sync/ChainingUserRegistrySynchronizerTest.java b/source/test-java/org/alfresco/repo/security/sync/ChainingUserRegistrySynchronizerTest.java index abd08dcb18..47fffe907d 100644 --- a/source/test-java/org/alfresco/repo/security/sync/ChainingUserRegistrySynchronizerTest.java +++ b/source/test-java/org/alfresco/repo/security/sync/ChainingUserRegistrySynchronizerTest.java @@ -18,22 +18,6 @@ */ package org.alfresco.repo.security.sync; -import java.util.AbstractCollection; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; -import java.util.Collections; -import java.util.Date; -import java.util.HashSet; -import java.util.Iterator; -import java.util.LinkedHashMap; -import java.util.LinkedList; -import java.util.List; -import java.util.Map; -import java.util.Random; -import java.util.Set; -import java.util.TreeMap; - import junit.framework.TestCase; import org.alfresco.error.AlfrescoRuntimeException; @@ -60,6 +44,8 @@ import org.springframework.context.ApplicationContext; import org.springframework.context.support.ClassPathXmlApplicationContext; import org.springframework.context.support.StaticApplicationContext; +import java.util.*; + /** * Tests the {@link ChainingUserRegistrySynchronizer} using a simulated {@link UserRegistry}. * @@ -444,15 +430,15 @@ public class ChainingUserRegistrySynchronizerTest extends TestCase public Object execute() throws Throwable { - assertExists("Z0", "U2"); - assertExists("Z0", "U3"); - assertExists("Z0", "U4"); + // MNT-9711 fix. User U6 already exists in zone "Z0". According ChainingUserRegistrySynchronizercurrent + // implementation when allowDeletions==false person that exists in a different zone with higher + // precedence will be ignored + assertExists("Z0", "U6"); + assertExists("Z1", "U1"); + assertExists("Z1", "U7"); + assertExists("Z1", "G5"); + assertExists("Z2", "G6", "U3", "U4", "G7"); assertExists("Z1", "U5"); - assertExists("Z1", "u6"); - assertExists(null, "U1"); - assertExists(null, "U7"); - assertExists(null, "G5"); - assertExists(null, "G6"); return null; } }, false, true);