diff --git a/config/alfresco/cache-context.xml b/config/alfresco/cache-context.xml
index 2b4a8d34cf..64ba00b281 100644
--- a/config/alfresco/cache-context.xml
+++ b/config/alfresco/cache-context.xml
@@ -43,6 +43,7 @@
+
diff --git a/source/java/org/alfresco/repo/security/authority/AuthorityBridgeTableAsynchronouslyRefreshedCache.java b/source/java/org/alfresco/repo/security/authority/AuthorityBridgeTableAsynchronouslyRefreshedCache.java
index 86025b8c61..6a1e324222 100644
--- a/source/java/org/alfresco/repo/security/authority/AuthorityBridgeTableAsynchronouslyRefreshedCache.java
+++ b/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 links = authorityBridgeDAO.getAuthorityBridgeLinks();
BridgeTable bridgeTable = new BridgeTable();
- for (AuthorityBridgeLink link : links)
+ try
{
- bridgeTable.addLink(link.getParentName(), link.getChildName());
+ 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 links)
+ {
+ Map> parentsToChildren = new HashMap>();
+ for (AuthorityBridgeLink link : links)
+ {
+ addToMap(parentsToChildren, link.getParentName(), link.getChildName());
+ }
+
+ Map> removed = new HashMap>();
+ for (String parent : parentsToChildren.keySet())
+ {
+ if (logger.isDebugEnabled())
+ {
+ logger.debug("Start checking from '" + parent + "'");
+ }
+ Set authorities = new HashSet();
+ 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> parentsToChildren, Set authorities,
+ Map> removed)
+ {
+ Set 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> removed, String parent, String child)
+ {
+ Set remChildren = removed.get(parent);
+ return (remChildren != null && remChildren.contains(child));
+ }
+
+ private void addToMap(Map> map, String parent, String child)
+ {
+ Set children = map.get(parent);
+ if (children == null)
+ {
+ children = new HashSet();
+ children.add(child);
+ map.put(parent, children);
+ }
+ else
+ {
+ children.add(child);
+ }
+ }
+
+ private void fixCyclic(final Map> removed)
+ {
+ // delete cyclic links in new transaction because
+ // current cache refresh will be interrupted with AlfrescoRuntimeException
+ retryingTransactionHelper.doInTransaction(new RetryingTransactionCallback()
+ {
+ @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();
}
}
diff --git a/source/java/org/alfresco/repo/security/authority/AuthorityDAO.java b/source/java/org/alfresco/repo/security/authority/AuthorityDAO.java
index 581502d501..7ba055bc1e 100644
--- a/source/java/org/alfresco/repo/security/authority/AuthorityDAO.java
+++ b/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.
*/
diff --git a/source/java/org/alfresco/repo/security/authority/AuthorityDAOImpl.java b/source/java/org/alfresco/repo/security/authority/AuthorityDAOImpl.java
index 3b228fc2c6..5e908cb4d9 100644
--- a/source/java/org/alfresco/repo/security/authority/AuthorityDAOImpl.java
+++ b/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,7 +797,10 @@ public class AuthorityDAOImpl implements AuthorityDAO, NodeServicePolicies.Befor
else
{
userAuthorityCache.clear();
- authorityBridgeTableCache.refresh();
+ if (cacheRefresh)
+ {
+ authorityBridgeTableCache.refresh();
+ }
}
}
diff --git a/source/test-java/org/alfresco/repo/security/authority/AuthorityBridgeTableAsynchronouslyRefreshedCacheTest.java b/source/test-java/org/alfresco/repo/security/authority/AuthorityBridgeTableAsynchronouslyRefreshedCacheTest.java
index 7288be4d50..3978301e82 100644
--- a/source/test-java/org/alfresco/repo/security/authority/AuthorityBridgeTableAsynchronouslyRefreshedCacheTest.java
+++ b/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 cyclicLinks = new LinkedList();
+ // 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