MNT-18308 - Add flag forceAsyncAclCreation

src/main/java/org/alfresco/repo/domain/permissions/ADMAccessControlListDAO.java

@@ -74,7 +74,10 @@ public class ADMAccessControlListDAO implements AccessControlListDAO
 
     /**maxim transaction time allowed for {@link #setFixedAcls(Long, Long, Long, Long, List, boolean, AsyncCallParameters, boolean)} */
     private long fixedAclMaxTransactionTime = 10 * 1000;
-
+
+    /** If time fixedAclMaxTransactionTime exceeds, force ACL creation to be async if flag is set to true */
+    private boolean forceAsyncAclCreation = false;
+
     public void setNodeDAO(NodeDAO nodeDAO)
     {
         this.nodeDAO = nodeDAO;
@@ -99,12 +102,22 @@ public void setPreserveAuditableData(boolean preserveAuditableData)
     {
         this.preserveAuditableData = preserveAuditableData;
     }
+
+    public void setForceAsyncAclCreation(boolean forceAsyncAclCreation)
+    {
+        this.forceAsyncAclCreation = forceAsyncAclCreation;
+    }
 
     public boolean isPreserveAuditableData()
     {
         return preserveAuditableData;
     }
-
+
+    public boolean isForceAsyncAclCreation()
+    {
+        return forceAsyncAclCreation;
+    }
+
     public void forceCopy(NodeRef nodeRef)
     {
         // Nothing to do
@@ -466,44 +479,54 @@ else if (dbAcl.getAclType() == ACLType.SHARED)
     private boolean setFixAclPending(Long nodeId, Long inheritFrom, Long mergeFrom, Long sharedAclToReplace, List<AclChange> changes,
             boolean set, boolean asyncCall, boolean propagateOnChildren)
     {
-        // check if async call is required
-        if (!asyncCall)
-        {
-            // make regular method call
-            setFixedAcls(nodeId, inheritFrom, mergeFrom, sharedAclToReplace, changes, set, asyncCall, propagateOnChildren);
-            return true;
-        }
-        else
-        {
-            // check transaction time
-            long transactionStartTime = AlfrescoTransactionSupport.getTransactionStartTime();
-            long transactionTime = System.currentTimeMillis() - transactionStartTime;
-
-            if (transactionTime < fixedAclMaxTransactionTime)
-            {
-                // make regular method call if time is under max transaction configured time
-                setFixedAcls(nodeId, inheritFrom, mergeFrom, sharedAclToReplace, changes, set, asyncCall, propagateOnChildren);
-                return true;
-            }
-            else
-            {
-                // time exceeded;
-                if (nodeDAO.getPrimaryChildrenAcls(nodeId).size() == 0)
-                {
-                    // if node is leaf in tree hierarchy call setFixedAcls now as processing with FixedAclUpdater would be more time consuming
-                    setFixedAcls(nodeId, inheritFrom, mergeFrom, sharedAclToReplace, changes, set, asyncCall, false);
-                }
-                else
-                {
-                    // set ASPECT_PENDING_FIX_ACL aspect on node to be later on processed with FixedAclUpdater
-                    addFixedAclPendingAspect(nodeId, sharedAclToReplace, inheritFrom);
-                    AlfrescoTransactionSupport.bindResource(FixedAclUpdater.FIXED_ACL_ASYNC_REQUIRED_KEY, true);
-                }
-                // stop propagating on children nodes
-                return false;
-            }
-        }
+     // If we exceeded the transaction time and either call is async or system set to force acl async creation, delegate to job
+        if(isSubjectToAsyncAclCreation(nodeId,asyncCall)) {
+            // set ASPECT_PENDING_FIX_ACL aspect on node to be later on processed with FixedAclUpdater
+            addFixedAclPendingAspect(nodeId, sharedAclToReplace, inheritFrom);
+            AlfrescoTransactionSupport.bindResource(FixedAclUpdater.FIXED_ACL_ASYNC_REQUIRED_KEY, true);      
+            return false;
+        }
+
+        // make regular method call
+        setFixedAcls(nodeId, inheritFrom, mergeFrom, sharedAclToReplace, changes, set, asyncCall, propagateOnChildren);
+        return true;
     }
+
+    /**
+     * MNT-18308 - Verify if we turn this ACL creation to an asynchronous process based on three parameters: if the call is already async and 
+     * transaction time was exceeded OR if system is set to force async ACL creation when max transaction time allowed is exceeded
+     * even on asynchronous calls
+     */
+    private boolean isSubjectToAsyncAclCreation(Long nodeId, boolean asyncCall) {
+
+        //If this is not an explicitly set or forced async call, it shall never go async
+        if (!asyncCall && !forceAsyncAclCreation) {
+            return false;
+        }
+
+        // This can be async so check the transaction time
+        long transactionStartTime = AlfrescoTransactionSupport.getTransactionStartTime();
+        long transactionTime = System.currentTimeMillis() - transactionStartTime;
+        boolean hasTimeExceeded = transactionTime < fixedAclMaxTransactionTime;
+
+        // Transaction time has not exceeded yet
+        if (!hasTimeExceeded) {
+            return false;
+        }
+
+        // It has exceeded the allowed time frame and async acl creation is set to be forced
+        if (forceAsyncAclCreation) {
+            return true;
+        }
+
+        // It has exceeded the allowed time frame, the call is already async and this node has children in it
+        if (asyncCall && nodeDAO.getPrimaryChildrenAcls(nodeId).size() > 0 ) {
+            return true;
+        }
+
+        // It is an async call and time was exceeded, but to maintain previous behaviour, we shall still process childless nodes synchronously
+        return false;
+    }
 
     private void addFixedAclPendingAspect(Long nodeId, Long sharedAclToReplace, Long inheritFrom)
     {

src/main/java/org/alfresco/repo/domain/permissions/FixedAclUpdater.java

@@ -321,7 +321,9 @@ public boolean handle(Pair<Long, NodeRef> nodePair)
             if (nodes.size() < maxItemBatchSize)
             {
                 nodes.add(nodePair.getSecond());
-                maxNodeId = nodePair.getFirst();
+                if(nodePair.getFirst() > maxNodeId) {
+                    maxNodeId = nodePair.getFirst();
+                }
                 return true;
             }
             return false;

src/main/resources/alfresco/dao/dao-context.xml

@@ -265,6 +265,7 @@
       <property name="behaviourFilter" ref="policyBehaviourFilter" />
       <property name="preserveAuditableData" value="${system.auditableData.ACLs}"></property>
       <property name="fixedAclMaxTransactionTime" value="${system.fixedACLs.maxTransactionTime}"/>
+      <property name="forceAsyncAclCreation" value="${system.fixedACLsUpdater.forceAsyncAclCreation}"></property>
    </bean>
 
    <bean id="aclCrudDAO" class="org.alfresco.repo.domain.permissions.ibatis.AclCrudDAOImpl">

src/main/resources/alfresco/repository.properties

@@ -1194,6 +1194,8 @@ system.fixedACLsUpdater.maxItemBatchSize=100
 system.fixedACLsUpdater.numThreads=4
 # fixedACLsUpdater cron expression - fire at midnight every day
 system.fixedACLsUpdater.cronExpression=0 0 0 * * ? 
+# forceAsyncAclCreation - If transaction exceeds maxTransactionTime, force all remaining nodes to be updated asynchronously
+system.fixedACLsUpdater.forceAsyncAclCreation=false
 
 cmis.disable.hidden.leading.period.files=false
 

src/test/java/org/alfresco/repo/domain/permissions/FixedAclUpdaterTest.java

@@ -69,6 +69,8 @@ public class FixedAclUpdaterTest extends TestCase
     private Repository repository;
     private FixedAclUpdater fixedAclUpdater;
     private NodeRef folderNodeRef;
+    private NodeRef mntFolder1;
+    private NodeRef mntFolder2;
     private PermissionsDaoComponent permissionsDaoComponent;
     private PermissionService permissionService;
     private NodeDAO nodeDAO;
@@ -97,6 +99,11 @@ public void setUp() throws Exception
         // change setFixedAclMaxTransactionTime to lower value so setInheritParentPermissions on created folder hierarchy require async call
         setFixedAclMaxTransactionTime(permissionsDaoComponent, home, 50);
 
+        RetryingTransactionCallback<NodeRef> cb2 = createFolderHierchyCallback(home, fileFolderService, "MNT18308_1", filesPerLevel);
+        mntFolder1 = txnHelper.doInTransaction(cb2);
+
+        RetryingTransactionCallback<NodeRef> cb3 = createFolderHierchyCallback(home, fileFolderService, "MNT18308_2", filesPerLevel);
+        mntFolder2 = txnHelper.doInTransaction(cb3);
     }
 
     private static void setFixedAclMaxTransactionTime(PermissionsDaoComponent permissionsDaoComponent, NodeRef folderNodeRef,
@@ -113,6 +120,19 @@ private static void setFixedAclMaxTransactionTime(PermissionsDaoComponent permis
         }
     }
 
+    private static void setForceAsyncAclCreation(PermissionsDaoComponent permissionsDaoComponent, NodeRef folderNodeRef, boolean forceAsyncAclCreation)
+    {
+        if (permissionsDaoComponent instanceof ADMPermissionsDaoComponentImpl)
+        {
+            AccessControlListDAO acldao = ((ADMPermissionsDaoComponentImpl) permissionsDaoComponent).getACLDAO(folderNodeRef);
+            if (acldao instanceof ADMAccessControlListDAO)
+            {
+                ADMAccessControlListDAO admAcLDao = (ADMAccessControlListDAO) acldao;
+                admAcLDao.setForceAsyncAclCreation(forceAsyncAclCreation);
+            }
+        }
+    }
+
     private static RetryingTransactionCallback<NodeRef> createFolderHierchyCallback(final NodeRef root,
             final FileFolderService fileFolderService, final String rootName, final int[] filesPerLevel)
     {
@@ -189,8 +209,7 @@ public Void execute() throws Throwable
             {
                 permissionService.setInheritParentPermissions(folderNodeRef, false, true);
 
-                Boolean asyncCallRequired = (Boolean) AlfrescoTransactionSupport.getResource(FixedAclUpdater.FIXED_ACL_ASYNC_REQUIRED_KEY);
-                assertTrue("asyncCallRequired should be true", asyncCallRequired);
+                assertTrue("asyncCallRequired should be true", isAsyncCallSetAsRequired());
 
                 return null;
             }
@@ -226,6 +245,111 @@ public Void execute() throws Throwable
         }, false, true);
     }
 
+    @Test
+    public void testMNT18308_flagOff()
+    {
+        // Test the default behaviour without setting forceAsyncAclCreation
+        txnHelper.doInTransaction(new RetryingTransactionCallback<Void>()
+        {
+            @Override
+            public Void execute() throws Throwable
+            {
+                assertFalse("asyncCallRequiredBefore should be false", isAsyncCallSetAsRequired());
+
+                //setInheritParentPermissions with sync call
+                permissionService.setInheritParentPermissions(mntFolder1, false, false);
+
+                assertFalse("asyncCallRequiredAfter should remain false", isAsyncCallSetAsRequired());
+
+                return null;
+            }
+        }, false, true);
+
+        // check that there are nodes to process by job
+        txnHelper.doInTransaction(new RetryingTransactionCallback<Void>()
+        {
+            @Override
+            public Void execute() throws Throwable
+            {
+                assertTrue("There should not be any nodes with ASPECT_PENDING_FIX_ACL", getNodesCountWithPendingFixedAclAspect() == 0);
+                return null;
+            }
+        }, false, true);
+    }
+
+    @Test
+    public void testMNT18308_flagOn()
+    {
+        // Set property forceAsyncAclCreation to true
+        setForceAsyncAclCreation(permissionsDaoComponent, repository.getCompanyHome(), true);
+
+        txnHelper.doInTransaction(new RetryingTransactionCallback<Void>()
+        {
+            @Override
+            public Void execute() throws Throwable
+            {
+                assertFalse("asyncCallRequiredBefore should be false", isAsyncCallSetAsRequired());
+
+                //setInheritParentPermissions with sync call
+                permissionService.setInheritParentPermissions(mntFolder2, false, false);
+
+                assertTrue("asyncCallRequiredAfter should now be true", isAsyncCallSetAsRequired());
+
+                return null;
+            }
+        }, false, true);
+
+        // check that there are nodes to process
+        txnHelper.doInTransaction(new RetryingTransactionCallback<Void>()
+        {
+            @Override
+            public Void execute() throws Throwable
+            {
+                assertTrue("No nodes are set to be processed by job", getNodesCountWithPendingFixedAclAspect() > 0);
+                return null;
+            }
+        }, false, true);
+
+        txnHelper.doInTransaction(new RetryingTransactionCallback<Void>()
+        {
+            @Override
+            public Void execute() throws Throwable
+            {
+                int count = 0;
+                do
+                {
+                    count = fixedAclUpdater.execute();
+                }
+                while(count > 0);
+
+                return null;
+            }
+        }, false, true);
+
+        // check if nodes with ASPECT_PENDING_FIX_ACL are processed
+        txnHelper.doInTransaction(new RetryingTransactionCallback<Void>()
+        {
+            @Override
+            public Void execute() throws Throwable
+            {
+                assertEquals("Not all nodes were processed", 0, getNodesCountWithPendingFixedAclAspect());
+                return null;
+            }
+        }, false, true);
+
+        setForceAsyncAclCreation(permissionsDaoComponent, repository.getCompanyHome(), false);
+    }
+
+    private boolean isAsyncCallSetAsRequired() {
+
+        if(AlfrescoTransactionSupport.getResource(FixedAclUpdater.FIXED_ACL_ASYNC_REQUIRED_KEY) == null) {
+            return false;
+        } else {
+            return (boolean) AlfrescoTransactionSupport.getResource(FixedAclUpdater.FIXED_ACL_ASYNC_REQUIRED_KEY);
+        }
+
+    }
+
     private static class GetNodesCountWithAspectCallback implements NodeRefQueryCallback
     {
         int nodesNumber = 0;