Merged HEAD-BUG-FIX (5.0/Cloud) to HEAD (5.0/Cloud)

76911: Merged PLATFORM1 (5.0/Cloud) to HEAD-BUG-FIX (5.0/Cloud)
      66893: Clean up use of deprecated 'UserRegistrySynchronizer.synchronize' and general code clean up


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

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

@@ -125,9 +125,6 @@ public class ChainingUserRegistrySynchronizer extends AbstractLifecycleBean
     /** The label under which the status is stored for each zone. */
     private static final String LAST_ERROR_ATTRIBUTE = "LAST_ERROR";
 
-    /** The label under which the status is stored for each zone. */
-    private static final String LAST_HOST_ATTRIBUTE = "LAST_HOST";
-    
     /** The label under which the status is stored for each zone. */
     private static final String START_TIME_ATTRIBUTE = "START_TIME";
     
@@ -426,16 +423,6 @@ public class ChainingUserRegistrySynchronizer extends AbstractLifecycleBean
         throw new AuthenticationException("authentication.err.validation.authenticator.notfound", params);
     }
 
-    /*
-     * (non-Javadoc)
-     * @see org.alfresco.repo.security.sync.UserRegistrySynchronizer#synchronize(boolean, boolean, boolean)
-     */
-    @Override
-    public void synchronize(boolean forceUpdate, boolean isFullSync, final boolean splitTxns)
-    {
-        synchronizeInternal(forceUpdate, isFullSync, splitTxns);
-    }
-
     @Override
     public void synchronize(boolean forceUpdate, boolean isFullSync)
     {
@@ -813,26 +800,22 @@ public class ChainingUserRegistrySynchronizer extends AbstractLifecycleBean
         return Collections.emptySet();
     }
 
-    /*
+    @Override
-     * (non-Javadoc)
-     * @see org.alfresco.repo.security.sync.UserRegistrySynchronizer#createMissingPerson(java.lang.String)
-     */
     public boolean createMissingPerson(String userName)
     {
-        // synchronize or auto-create the missing person if we are allowed
+        // synchronise or auto-create the missing person if we are allowed
         if (userName != null && !userName.equals(AuthenticationUtil.getSystemUserName()))
         {
             if (this.syncWhenMissingPeopleLogIn)
             {
                 try
                 {
-                    synchronize(false, false, false);
+                    synchronizeInternal(false, false, false);
                 }
                 catch (Exception e)
                 {
                     // We don't want to fail the whole login if we can help it
-                    ChainingUserRegistrySynchronizer.logger.warn(
+                    ChainingUserRegistrySynchronizer.logger.warn("User authenticated but failed to sync with user registry", e);
-                            "User authenticated but failed to sync with user registry", e);
                 }
                 if (this.personService.personExists(userName))
                 {
@@ -968,8 +951,7 @@ public class ChainingUserRegistrySynchronizer extends AbstractLifecycleBean
         final BatchProcessor<NodeDescription> groupProcessor = new BatchProcessor<NodeDescription>(
                 SyncProcess.GROUP_ANALYSIS.getTitle(zone),
                 this.transactionService.getRetryingTransactionHelper(), 
-                userRegistry
+                userRegistry.getGroups(lastModified), 
-                .getGroups(lastModified), 
                 this.workerThreads, 
                 20, 
                 this.applicationEventPublisher,
@@ -1565,8 +1547,8 @@ public class ChainingUserRegistrySynchronizer extends AbstractLifecycleBean
                     // Batch 5 Group Association Creation
                     BatchProcessor<Map.Entry<String, Set<String>>> groupCreator = new BatchProcessor<Map.Entry<String, Set<String>>>(
                             SyncProcess.GROUP_ASSOCIATION_CREATION.getTitle(zone),
-                            ChainingUserRegistrySynchronizer.this.transactionService
+                            ChainingUserRegistrySynchronizer.this.transactionService.getRetryingTransactionHelper(),
-                                    .getRetryingTransactionHelper(), this.groupParentAssocsToCreate.entrySet(),
+                            this.groupParentAssocsToCreate.entrySet(),
                             ChainingUserRegistrySynchronizer.this.workerThreads, 20,
                             ChainingUserRegistrySynchronizer.this.applicationEventPublisher,
                             ChainingUserRegistrySynchronizer.logger,
@@ -1597,8 +1579,8 @@ public class ChainingUserRegistrySynchronizer extends AbstractLifecycleBean
                     // Batch 6 Person Association
                     BatchProcessor<Map.Entry<String, Set<String>>> groupCreator = new BatchProcessor<Map.Entry<String, Set<String>>>(
                             SyncProcess.PERSON_ASSOCIATION.getTitle(zone),
-                            ChainingUserRegistrySynchronizer.this.transactionService
+                            ChainingUserRegistrySynchronizer.this.transactionService.getRetryingTransactionHelper(),
-                                    .getRetryingTransactionHelper(), this.personParentAssocsToDelete.entrySet(),
+                            this.personParentAssocsToDelete.entrySet(),
                             ChainingUserRegistrySynchronizer.this.workerThreads, 20,
                             ChainingUserRegistrySynchronizer.this.applicationEventPublisher,
                             ChainingUserRegistrySynchronizer.logger,
@@ -1719,8 +1701,12 @@ public class ChainingUserRegistrySynchronizer extends AbstractLifecycleBean
         final BatchProcessor<NodeDescription> personProcessor = new BatchProcessor<NodeDescription>(
                 SyncProcess.USER_CREATION.getTitle(zone),
                 this.transactionService.getRetryingTransactionHelper(),
-                userRegistry.getPersons(lastModified), this.workerThreads, 10, this.applicationEventPublisher,
+                userRegistry.getPersons(lastModified),
-                ChainingUserRegistrySynchronizer.logger, this.loggingInterval);
+                this.workerThreads,
+                10,
+                this.applicationEventPublisher,
+                ChainingUserRegistrySynchronizer.logger,
+                this.loggingInterval);
         class PersonWorker extends BaseBatchProcessWorker<NodeDescription>
         {
             private long latestTime;
@@ -1856,8 +1842,12 @@ public class ChainingUserRegistrySynchronizer extends AbstractLifecycleBean
             BatchProcessor<String> authorityDeletionProcessor = new BatchProcessor<String>(
                     SyncProcess.AUTHORITY_DELETION.getTitle(zone), 
                     this.transactionService.getRetryingTransactionHelper(),
-                    deletionCandidates, this.workerThreads, 10, this.applicationEventPublisher,
+                    deletionCandidates,
-                    ChainingUserRegistrySynchronizer.logger, this.loggingInterval);
+                    this.workerThreads,
+                    10,
+                    this.applicationEventPublisher,
+                    ChainingUserRegistrySynchronizer.logger,
+                    this.loggingInterval);
             class AuthorityDeleter extends BaseBatchProcessWorker<String>
             {
                 private int personProcessedCount;
@@ -2007,10 +1997,6 @@ public class ChainingUserRegistrySynchronizer extends AbstractLifecycleBean
     /**
      * Modifies an authority's zone set from oldZones to newZones in the most efficient manner (avoiding unnecessary
      * reindexing cost).
-     * 
-     * @param authorityName
-     * @param oldZones
-     * @param newZones
      */
     private void updateAuthorityZones(String authorityName, Set<String> oldZones, final Set<String> newZones)
     {
@@ -2031,11 +2017,6 @@ public class ChainingUserRegistrySynchronizer extends AbstractLifecycleBean
         }
     }
     
-    /*
-     * (non-Javadoc)
-     * @seeorg.springframework.extensions.surf.util.AbstractLifecycleBean#onBootstrap(org.springframework.context.
-     * ApplicationEvent)
-     */
     @Override
     protected void onBootstrap(ApplicationEvent event)
     {
@@ -2049,7 +2030,7 @@ public class ChainingUserRegistrySynchronizer extends AbstractLifecycleBean
                 {
                     try
                     {
-                        synchronize(false, false, true);
+                        synchronizeInternal(false, false, true);
                     }
                     catch (Exception e)
                     {
@@ -2409,5 +2390,4 @@ public class ChainingUserRegistrySynchronizer extends AbstractLifecycleBean
     		 super.onApplicationEvent(event);
     	 }
     }
-      
 }

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

@@ -44,35 +44,7 @@ public interface UserRegistrySynchronizer
     /**
      * Retrieves timestamped user and group information from configured external sources and compares it with the local
      * users and groups last retrieved from the same sources. Any updates and additions made to those users and groups
-     * are applied to the local Alfresco copies.
+     * are applied to the local Alfresco copies. This process is always run in different transactions and threads.
-     * <p>
-     *     This method is <b>deprecated</b>, use {@link #synchronize(boolean forceUpdate, boolean isFullSync)} instead.
-     * </p>
-     * 
-     * @param forceUpdate
-     *            Should the complete set of users and groups be updated / created locally or just those known to have
-     *            changed since the last sync? When <code>true</code> then <i>all</i> users and groups are queried from
-     *            the user registry and updated locally. When <code>false</code> then each source is only queried for
-     *            those users and groups modified since the most recent modification date of all the objects last
-     *            queried from that same source.
-     * @param isFullSync
-     *            Should a complete set of user and group IDs be queried from the user registries in order to determine
-     *            deletions? This parameter is independent of <code>force</code> as a separate query is run to process
-     *            updates.
-     * @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).
-     */
-    @Deprecated
-    public void synchronize(boolean forceUpdate, boolean isFullSync, boolean splitTxns);
-
-    /**
-     * Retrieves timestamped user and group information from configured external sources and compares it with the local
-     * users and groups last retrieved from the same sources. Any updates and additions made to those users and groups
-     * are applied to the local Alfresco copies. Unlike deprecated {@link #synchronize(boolean, boolean, boolean)},
-     * this process is always run in different transactions and threads.
      *
      * @param forceUpdate
      *            Should the complete set of users and groups be updated / created locally or just those known to have

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2005-2010 Alfresco Software Limited.
+ * Copyright (C) 2005-2014 Alfresco Software Limited.
  *
  * This file is part of Alfresco
  *
@@ -34,25 +34,18 @@ import org.quartz.JobExecutionException;
  */
 public class UserRegistrySynchronizerJob implements Job
 {
-    /*
-     * (non-Javadoc)
-     * @see org.quartz.Job#execute(org.quartz.JobExecutionContext)
-     */
     public void execute(JobExecutionContext executionContext) throws JobExecutionException
     {
         final UserRegistrySynchronizer userRegistrySynchronizer = (UserRegistrySynchronizer) executionContext
                 .getJobDetail().getJobDataMap().get("userRegistrySynchronizer");
-        final String synchronizeChangesOnly = (String) executionContext.getJobDetail().getJobDataMap().get(
+        final String synchronizeChangesOnly = (String) executionContext.getJobDetail().getJobDataMap().get("synchronizeChangesOnly");
-                "synchronizeChangesOnly");
         AuthenticationUtil.runAs(new RunAsWork<Object>()
         {
             public Object doWork() throws Exception
             {
-                userRegistrySynchronizer.synchronize(synchronizeChangesOnly == null
+                userRegistrySynchronizer.synchronize(synchronizeChangesOnly == null || !Boolean.parseBoolean(synchronizeChangesOnly), true);
-                        || !Boolean.parseBoolean(synchronizeChangesOnly), true, true);
                 return null;
             }
         }, AuthenticationUtil.getSystemUserName());
     }
-
 }

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

@@ -173,7 +173,7 @@ public class ChainingUserRegistrySynchronizerTest extends TestCase
         {
             newGroup("G2", "U1", "U3", "U4"), newGroup("G6", "U3", "U4", "G7"), newGroup("G7", "U5")
         }));
-        this.synchronizer.synchronize(true, true, true);
+        this.synchronizer.synchronize(true, true);
         this.retryingTransactionHelper.doInTransaction(new RetryingTransactionCallback<Object>()
         {
 
@@ -216,13 +216,13 @@ public class ChainingUserRegistrySynchronizerTest extends TestCase
             newGroup("G7")
         }), new MockUserRegistry("Z1", new NodeDescription[] {}, new NodeDescription[] {}), new MockUserRegistry("Z2",
                 new NodeDescription[] {}, new NodeDescription[] {}));
-        this.synchronizer.synchronize(true, true, true);
+        this.synchronizer.synchronize(true, true);
         // Wipe out everything that was in Z0 - Z2
         this.applicationContextManager.setUserRegistries(new MockUserRegistry("Z0", new NodeDescription[] {},
                 new NodeDescription[] {}), new MockUserRegistry("Z1", new NodeDescription[] {},
                 new NodeDescription[] {}), new MockUserRegistry("Z2", new NodeDescription[] {},
                 new NodeDescription[] {}));
-        this.synchronizer.synchronize(true, true, true);
+        this.synchronizer.synchronize(true, true);
         this.retryingTransactionHelper.doInTransaction(new RetryingTransactionCallback<Object>()
         {
 
@@ -371,7 +371,7 @@ public class ChainingUserRegistrySynchronizerTest extends TestCase
             newGroup("G2", "U1", "U3", "U4", "U6"), newGroup("G6", "U3", "U4", "G7"),
             newGroupWithDisplayName("G7", "Late Arrival", "U4", "U5"), newGroup("G8", "U1", "U8")
         }));
-        this.synchronizer.synchronize(true, true, true);
+        this.synchronizer.synchronize(true, true);
         this.retryingTransactionHelper.doInTransaction(new RetryingTransactionCallback<Object>()
         {
 
@@ -426,7 +426,7 @@ public class ChainingUserRegistrySynchronizerTest extends TestCase
         {
             newPerson("U6"),
         }, new NodeDescription[] {}));
-        synchronizer.synchronize(true, true, true);
+        synchronizer.synchronize(true, true);
         this.retryingTransactionHelper.doInTransaction(new RetryingTransactionCallback<Object>()
         {
 
@@ -482,7 +482,7 @@ public class ChainingUserRegistrySynchronizerTest extends TestCase
         {
             newGroup("G2", "U1", "U3", "U4"), newGroup("G6", "U3", "U4", "G7"), newGroup("G7", "U5")
         }));
-        this.synchronizer.synchronize(true, true, true);
+        this.synchronizer.synchronize(true, true);
         this.retryingTransactionHelper.doInTransaction(new RetryingTransactionCallback<Object>()
         {
 
@@ -546,7 +546,7 @@ public class ChainingUserRegistrySynchronizerTest extends TestCase
         List<NodeDescription> persons = new ArrayList<NodeDescription>(new RandomPersonCollection(100));
         List<NodeDescription> groups = new ArrayList<NodeDescription>(new RandomGroupCollection(50, persons));
         this.applicationContextManager.setUserRegistries(new MockUserRegistry("Z0", persons, groups));
-        this.synchronizer.synchronize(true, true, true);
+        this.synchronizer.synchronize(true, true);
         tearDownTestUsersAndGroups();
     }
     
@@ -623,7 +623,7 @@ public class ChainingUserRegistrySynchronizerTest extends TestCase
             List<NodeDescription> groups = new ArrayList<NodeDescription>(new RandomGroupCollection(4, persons));
             MockUserRegistry testRegistry = new MockUserRegistry("Z0", persons, groups);
             this.applicationContextManager.setUserRegistries(testRegistry);
-            this.synchronizer.synchronize(true, true, true);
+            this.synchronizer.synchronize(true, true);
             
             if(this.synchronizer instanceof ChainingUserRegistrySynchronizerStatus)
             {
@@ -651,7 +651,7 @@ public class ChainingUserRegistrySynchronizerTest extends TestCase
             testStart = new Date();
             try
             {
-                this.synchronizer.synchronize(true, true, true);
+                this.synchronizer.synchronize(true, true);
                 fail("error not thrown");
             }
             catch (AlfrescoRuntimeException e)
@@ -697,7 +697,7 @@ public class ChainingUserRegistrySynchronizerTest extends TestCase
                 }, true, true);
         ChainingUserRegistrySynchronizerTest.this.applicationContextManager.setUserRegistries(new MockUserRegistry(
                 "Z0", Collections.<NodeDescription> emptyList(), groups));
-        ChainingUserRegistrySynchronizerTest.this.synchronizer.synchronize(true, true, true);
+        ChainingUserRegistrySynchronizerTest.this.synchronizer.synchronize(true, true);
         tearDownTestUsersAndGroups();
     }