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

88886: Merged V4.2-BUG-FIX (4.2.4) to HEAD-BUG-FIX (5.0/Cloud)
      88846: MNT-12473: Merged V4.2.2 (4.2.2.16) to V4.2-BUG-FIX (4.2.4)
         88606: MNT-12574: CLONE - cyclic groups can be created bringing the server down
         During build of the AuthorityBridgeTableAsynchronouslyRefreshedCache check and fix all cyclic groups. Add unit test.
      88847: MNT-12473: Merged V4.2.2 (4.2.2.16) to V4.2-BUG-FIX (4.2.4)
         88650: MNT-12574: CLONE - cyclic groups can be created bringing the server down
         Do the cyclic check only in case of ConcurrentModificationException.


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

config/alfresco/cache-context.xml

@@ -43,6 +43,7 @@
       <property name="authorityBridgeDAO" ref="authorityBridgeDAO" />
       <property name="retryingTransactionHelper" ref="retryingTransactionHelper" />
       <property name="tenantAdminService" ref="tenantAdminService" />
+      <property name="authorityDAO" ref="authorityDAO" />
    </bean>
    
    <!-- ===================================== -->

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2005-2013 Alfresco Software Limited.
+ * Copyright (C) 2005-2014 Alfresco Software Limited.
  *
  * This file is part of Alfresco
  *
@@ -18,8 +18,14 @@
  */
 package org.alfresco.repo.security.authority;
 
+import java.util.ConcurrentModificationException;
+import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Map;
+import java.util.Set;
 
+import org.alfresco.error.AlfrescoRuntimeException;
 import org.alfresco.repo.cache.AbstractMTAsynchronouslyRefreshedCache;
 import org.alfresco.repo.security.authentication.AuthenticationUtil;
 import org.alfresco.repo.security.authentication.AuthenticationUtil.RunAsWork;
@@ -28,6 +34,8 @@ import org.alfresco.repo.transaction.RetryingTransactionHelper;
 import org.alfresco.repo.transaction.RetryingTransactionHelper.RetryingTransactionCallback;
 import org.alfresco.util.BridgeTable;
 import org.alfresco.util.PropertyCheck;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
 import org.springframework.beans.factory.InitializingBean;
 
 /**
@@ -39,6 +47,18 @@ public class AuthorityBridgeTableAsynchronouslyRefreshedCache extends  AbstractM
     private AuthorityBridgeDAO authorityBridgeDAO;
     private RetryingTransactionHelper retryingTransactionHelper;
     private TenantAdminService tenantAdminService;
+    private AuthorityDAO authorityDAO;
+
+    private Log logger = LogFactory.getLog(getClass());
+
+    /**
+     * @param authorityDAO
+     *            the authorityDAO to set
+     */
+    public void setAuthorityDAO(AuthorityDAO authorityDAO)
+    {
+        this.authorityDAO = authorityDAO;
+    }
 
     /**
      * @param authorityBridgeDAO
@@ -87,18 +107,134 @@ public class AuthorityBridgeTableAsynchronouslyRefreshedCache extends  AbstractM
     {
         List<AuthorityBridgeLink> links = authorityBridgeDAO.getAuthorityBridgeLinks();
         BridgeTable<String> bridgeTable = new BridgeTable<String>();
+        try
+        {
             for (AuthorityBridgeLink link : links)
             {
                 bridgeTable.addLink(link.getParentName(), link.getChildName());
             }
+        }
+        catch (ConcurrentModificationException e)
+        {
+            // Explain exception
+            checkCyclic(links);
+            // If cyclic groups is not the cause then rethrow
+            throw e;
+        }
         return bridgeTable;
     }
 
+    private void checkCyclic(List<AuthorityBridgeLink> links)
+    {
+        Map<String, Set<String>> parentsToChildren = new HashMap<String, Set<String>>();
+        for (AuthorityBridgeLink link : links)
+        {
+            addToMap(parentsToChildren, link.getParentName(), link.getChildName());
+        }
+        
+        Map<String, Set<String>> removed = new HashMap<String, Set<String>>();
+        for (String parent : parentsToChildren.keySet())
+        {
+            if (logger.isDebugEnabled())
+            {
+                logger.debug("Start checking from '" + parent + "'");
+            }
+            Set<String> authorities = new HashSet<String>();
+            authorities.add(parent);
+            doCheck(parent, parentsToChildren, authorities, removed);
+        }
+        if (!removed.isEmpty())
+        {
+            fixCyclic(removed);
+            throw new AlfrescoRuntimeException("Cyclic links were detected and removed.");
+        }
+    }
+
+    private void doCheck(String parent, Map<String, Set<String>> parentsToChildren, Set<String> authorities,
+            Map<String, Set<String>> removed)
+    {
+        Set<String> children = parentsToChildren.get(parent);
+        if (children != null)
+        {
+            for (String child : children)
+            {
+                if (logger.isDebugEnabled())
+                {
+                    logger.debug("Check link from '" + parent + "' to '" + child + "'");
+                }
+                if (isRemoved(removed, parent, child))
+                {
+                    if (logger.isDebugEnabled())
+                    {
+                        logger.debug("Link from '" + parent + "' to '" + child + "' has been already removed");
+                    }
+                    continue;
+                }
+                if (!authorities.add(child))
+                {
+                    addToMap(removed, parent, child);
+                    continue;
+                }
+                doCheck(child, parentsToChildren, authorities, removed);
+                authorities.remove(child);
+            }
+            if (logger.isDebugEnabled())
+            {
+                logger.debug("Children of '" + parent + "' were processed");
+            }
+        }
+    }
+
+    private boolean isRemoved(Map<String, Set<String>> removed, String parent, String child)
+    {
+        Set<String> remChildren = removed.get(parent);
+        return (remChildren != null && remChildren.contains(child));
+    }
+
+    private void addToMap(Map<String, Set<String>> map, String parent, String child)
+    {
+        Set<String> children = map.get(parent);
+        if (children == null)
+        {
+            children = new HashSet<String>();
+            children.add(child);
+            map.put(parent, children);
+        }
+        else
+        {
+            children.add(child);
+        }
+    }
+
+    private void fixCyclic(final Map<String, Set<String>> removed)
+    {
+        // delete cyclic links in new transaction because
+        // current cache refresh will be interrupted with AlfrescoRuntimeException
+        retryingTransactionHelper.doInTransaction(new RetryingTransactionCallback<Void>()
+        {
+            @Override
+            public Void execute() throws Throwable
+            {
+                for (String parentName : removed.keySet())
+                {
+                    for (String childName : removed.get(parentName))
+                    {
+                        // do not refresh authorityBridgeTableCache
+                        authorityDAO.removeAuthority(parentName, childName, false);
+                        logger.error("Link from '" + parentName + "' to '" + childName +"' was removed to break cycle.");
+                    }
+                }
+                return null;
+            }
+        }, false, true);
+    }
+
     @Override
     public void afterPropertiesSet() throws Exception
     {
         PropertyCheck.mandatory(this, "authorityBridgeDAO", authorityBridgeDAO);
         PropertyCheck.mandatory(this, "retryingTransactionHelper", retryingTransactionHelper);
+        PropertyCheck.mandatory(this, "authorityDAO", authorityDAO);
         super.afterPropertiesSet();
     }
 }

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

@@ -73,6 +73,11 @@ public interface AuthorityDAO
      */
     void removeAuthority(String parentName, String childName);
 
+    /**
+     * Remove an authority without authorityBridgeTableCache refresh.
+     */
+    void removeAuthority(String parentName, String childName, boolean cacheRefresh);
+
     /**
      * Get the authorities that contain the one given.
      */

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

@@ -772,6 +772,11 @@ public class AuthorityDAOImpl implements AuthorityDAO, NodeServicePolicies.Befor
     }
 
     public void removeAuthority(String parentName, String childName)
+    {
+        removeAuthority(parentName, childName, true);
+    }
+
+    public void removeAuthority(String parentName, String childName, boolean cacheRefresh)
     {
         NodeRef parentRef = getAuthorityOrNull(parentName);
         if (parentRef == null)
@@ -792,9 +797,12 @@ public class AuthorityDAOImpl implements AuthorityDAO, NodeServicePolicies.Befor
         else
         {
             userAuthorityCache.clear();
+            if (cacheRefresh)
+            {
                 authorityBridgeTableCache.refresh();
             }
         }
+    }
 
     /**
      * Explicitly use the bridge table to list authorities.

source/test-java/org/alfresco/repo/security/authority/AuthorityBridgeTableAsynchronouslyRefreshedCacheTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2005-2013 Alfresco Software Limited.
+ * Copyright (C) 2005-2014 Alfresco Software Limited.
  *
  * This file is part of Alfresco
  *
@@ -18,6 +18,15 @@
  */
 package org.alfresco.repo.security.authority;
 
+import static org.mockito.Matchers.anyString;
+import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.anyBoolean;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.util.ConcurrentModificationException;
+import java.util.LinkedList;
+import java.util.List;
 import java.util.Set;
 
 import junit.framework.TestCase;
@@ -37,7 +46,10 @@ import org.alfresco.service.transaction.TransactionService;
 import org.alfresco.test_category.OwnJVMTestsCategory;
 import org.alfresco.util.ApplicationContextHelper;
 import org.alfresco.util.GUID;
+
 import org.junit.experimental.categories.Category;
+import org.mockito.invocation.InvocationOnMock;
+import org.mockito.stubbing.Answer;
 import org.springframework.context.ApplicationContext;
 
 @Category(OwnJVMTestsCategory.class)
@@ -154,4 +166,94 @@ public class AuthorityBridgeTableAsynchronouslyRefreshedCacheTest extends TestCa
             }
         }, false, true);
     }
+    
+    /**
+     * See MNT-12473
+     */
+    public void testCyclicGroups()
+    {
+        List<AuthorityBridgeLink> cyclicLinks = new LinkedList<AuthorityBridgeLink>();
+        // no cycle
+        cyclicLinks.add(createAuthorityBridgeLink("a1", "a2"));
+        
+        cyclicLinks.add(createAuthorityBridgeLink("g1", "g2"));
+        cyclicLinks.add(createAuthorityBridgeLink("g2", "g3"));
+        cyclicLinks.add(createAuthorityBridgeLink("g3", "g4"));
+        // 1st cycle
+        cyclicLinks.add(createAuthorityBridgeLink("g4", "g1"));
+        
+        cyclicLinks.add(createAuthorityBridgeLink("b1", "b2"));
+        // child with no cycle
+        cyclicLinks.add(createAuthorityBridgeLink("b2", "a1"));
+        cyclicLinks.add(createAuthorityBridgeLink("b2", "b3"));
+        // 2nd cycle
+        cyclicLinks.add(createAuthorityBridgeLink("b3", "b1"));
+        
+        cyclicLinks.add(createAuthorityBridgeLink("d1", "d2"));
+        cyclicLinks.add(createAuthorityBridgeLink("d2", "d3"));
+        // 3rd cycle
+        cyclicLinks.add(createAuthorityBridgeLink("d3", "d1"));
+        cyclicLinks.add(createAuthorityBridgeLink("d2", "d4"));
+        // 4th cycle
+        cyclicLinks.add(createAuthorityBridgeLink("d4", "d1"));
+        cyclicLinks.add(createAuthorityBridgeLink("d3", "d5"));
+        // 5th cycle
+        cyclicLinks.add(createAuthorityBridgeLink("d5", "d1"));
+        
+        AuthorityBridgeDAO authorityBridgeDAOMock = mock(AuthorityBridgeDAO.class);
+        when(authorityBridgeDAOMock.getAuthorityBridgeLinks()).thenReturn(cyclicLinks);
+        
+        AuthorityDAO authorityDAOMock = mock(AuthorityDAO.class);
+        class Counter
+        {
+            private int removed = 0;
+            public int getRemoved()
+            {
+                return removed;
+            }
+            public void increase()
+            {
+                this.removed++;
+            }
+        }
+        ;
+        final Counter cycles = new Counter();
+        doAnswer(new Answer<Object>()
+        {
+            public Object answer(InvocationOnMock invocation)
+            {
+                cycles.increase();
+                return null;
+            }
+        }).when(authorityDAOMock).removeAuthority(anyString(), anyString(), anyBoolean());
+        
+        AuthorityBridgeTableAsynchronouslyRefreshedCache cache = new AuthorityBridgeTableAsynchronouslyRefreshedCache();
+        cache.setAuthorityBridgeDAO(authorityBridgeDAOMock);
+        cache.setAuthorityDAO(authorityDAOMock);
+        cache.setTenantAdminService(tenantAdminService);
+        cache.setRetryingTransactionHelper(transactionService.getRetryingTransactionHelper());
+        
+        try
+        {
+            cache.buildCache(tenantAdminService.getCurrentUserDomain());
+        }
+        catch (AlfrescoRuntimeException e1)
+        {
+            assertTrue(e1.getMessage().contains("Cyclic links were detected"));
+            assertEquals(5, cycles.getRemoved());
+        }
+        catch (ConcurrentModificationException e2)
+        {
+            fail("Cyclic links were NOT detected and processed");
+        }
+    }
+    
+    private AuthorityBridgeLink createAuthorityBridgeLink(String parentName, String childName)
+    {
+        AuthorityBridgeLink link = new AuthorityBridgeLink();
+        link.setParentName(parentName);
+        link.setChildName(childName);
+        return link;
+    }
+
 }