From f7b86ad94b7970468345fdc93ccb0cc861c12b9d Mon Sep 17 00:00:00 2001 From: Alex Mukha Date: Fri, 14 Aug 2015 09:16:26 +0000 Subject: [PATCH] Merged DEV to HEAD (5.1) 110000: MNT-13836: Custom behaviours cannot be disabled (using disableBehaviour(QName)) - Changed class policies enabled/disabled checks. Now each behaviour at a given level must be explicitly disabled & re-enabled (unless all behaviours are disabled in the transaction). - Added JUnit tests. git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@110119 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261 --- .../repo/policy/ClassBehaviourIndex.java | 40 +-- .../PolicyComponentTransactionTest.java | 259 ++++++++++++++++++ .../repo/policy/policycomponenttest_model.xml | 2 + 3 files changed, 282 insertions(+), 19 deletions(-) diff --git a/source/java/org/alfresco/repo/policy/ClassBehaviourIndex.java b/source/java/org/alfresco/repo/policy/ClassBehaviourIndex.java index 07d797d65d..2b45cd54d6 100644 --- a/source/java/org/alfresco/repo/policy/ClassBehaviourIndex.java +++ b/source/java/org/alfresco/repo/policy/ClassBehaviourIndex.java @@ -20,6 +20,7 @@ package org.alfresco.repo.policy; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.List; import java.util.concurrent.locks.ReentrantReadWriteLock; @@ -161,28 +162,16 @@ import org.alfresco.util.LockHelper; { List behaviours = new ArrayList(); - // Determine if behaviour has been disabled - boolean isEnabled = true; - if (filter != null) + // Find class behaviour by scanning up the class hierarchy + List> behaviour = null; + while (binding != null) { - NodeRef nodeRef = binding.getNodeRef(); - QName className = binding.getClassQName(); - isEnabled = (nodeRef == null) ? filter.isEnabled(className) : filter.isEnabled(nodeRef, className); - } - - if (isEnabled) - { - // Find class behaviour by scanning up the class hierarchy - List> behaviour = null; - while (binding != null) + behaviour = classMap.get(binding); + if (behaviour != null && isEnabled(binding)) { - behaviour = classMap.get(binding); - if (behaviour != null) - { - behaviours.addAll(0, behaviour); // note: list base/generalised before extended/specific - } - binding = (B)binding.generaliseBinding(); + behaviours.addAll(0, behaviour); // note: list base/generalised before extended/specific } + binding = (B)binding.generaliseBinding(); } // Append all service-level behaviours @@ -266,4 +255,17 @@ import org.alfresco.util.LockHelper; } } + private boolean isEnabled(B binding) + { + // Determine if behaviour has been disabled + boolean isEnabled = true; + if (filter != null) + { + NodeRef nodeRef = binding.getNodeRef(); + QName className = binding.getClassQName(); + isEnabled = (nodeRef == null) ? filter.isEnabled(className) : filter.isEnabled(nodeRef, className); + } + + return isEnabled; + } } diff --git a/source/test-java/org/alfresco/repo/policy/PolicyComponentTransactionTest.java b/source/test-java/org/alfresco/repo/policy/PolicyComponentTransactionTest.java index 35afc12501..ea39622177 100644 --- a/source/test-java/org/alfresco/repo/policy/PolicyComponentTransactionTest.java +++ b/source/test-java/org/alfresco/repo/policy/PolicyComponentTransactionTest.java @@ -18,21 +18,31 @@ */ package org.alfresco.repo.policy; +import java.io.Serializable; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; +import java.util.Map; import javax.transaction.UserTransaction; import junit.framework.TestCase; +import org.alfresco.model.ContentModel; import org.alfresco.repo.dictionary.DictionaryBootstrap; import org.alfresco.repo.dictionary.DictionaryDAO; +import org.alfresco.repo.node.NodeServicePolicies.OnCreateNodePolicy; +import org.alfresco.repo.nodelocator.CompanyHomeNodeLocator; +import org.alfresco.repo.nodelocator.NodeLocatorService; import org.alfresco.repo.policy.Behaviour.NotificationFrequency; import org.alfresco.repo.policy.Policy.Arg; import org.alfresco.repo.security.authentication.AuthenticationComponent; import org.alfresco.repo.tenant.TenantService; import org.alfresco.repo.transaction.AlfrescoTransactionSupport; +import org.alfresco.service.cmr.repository.ChildAssociationRef; import org.alfresco.service.cmr.repository.NodeRef; +import org.alfresco.service.cmr.repository.NodeService; +import org.alfresco.service.namespace.NamespaceService; import org.alfresco.service.namespace.QName; import org.alfresco.service.transaction.TransactionService; import org.alfresco.test_category.OwnJVMTestsCategory; @@ -50,6 +60,7 @@ public class PolicyComponentTransactionTest extends TestCase private static final String TEST_MODEL = "org/alfresco/repo/policy/policycomponenttest_model.xml"; private static final String TEST_NAMESPACE = "http://www.alfresco.org/test/policycomponenttest/1.0"; private static QName BASE_TYPE = QName.createQName(TEST_NAMESPACE, "base"); + private static QName FILE_TYPE = QName.createQName(TEST_NAMESPACE, "file"); private static ApplicationContext applicationContext = ApplicationContextHelper.getApplicationContext(); private static ClassPolicyDelegate sideEffectDelegate = null; @@ -57,6 +68,9 @@ public class PolicyComponentTransactionTest extends TestCase private BehaviourFilter behaviourFilter; private TransactionService trxService; private AuthenticationComponent authenticationComponent; + private NodeService nodeService; + private NodeLocatorService nodeLocatorService; + private NodeRef companyHome; @Override @@ -75,6 +89,8 @@ public class PolicyComponentTransactionTest extends TestCase this.policyComponent = (PolicyComponent)applicationContext.getBean("policyComponent"); this.behaviourFilter = (BehaviourFilter) applicationContext.getBean("policyBehaviourFilter"); this.trxService = (TransactionService) applicationContext.getBean("transactionComponent"); + this.nodeService = (NodeService) applicationContext.getBean("nodeService"); + this.nodeLocatorService = (NodeLocatorService) applicationContext.getBean("nodeLocatorService"); this.authenticationComponent = (AuthenticationComponent)applicationContext.getBean("authenticationComponent"); this.authenticationComponent.setSystemUserAsCurrentUser(); @@ -88,6 +104,8 @@ public class PolicyComponentTransactionTest extends TestCase Behaviour baseBehaviour = new JavaBehaviour(this, "sideEffectTest", NotificationFrequency.TRANSACTION_COMMIT); this.policyComponent.bindClassBehaviour(policyName, BASE_TYPE, baseBehaviour); } + + this.companyHome = nodeLocatorService.getNode(CompanyHomeNodeLocator.NAME, null, null); } @@ -359,6 +377,224 @@ public class PolicyComponentTransactionTest extends TestCase } } + /** + * Test for MNT_13836 + *

first show that both behaviours are enabled and triggered for a sub- (child) instance

+ * @throws Exception + */ + public void testChildParentBehaviours1() throws Exception + { + TestOnCreateNodePolicy baseTypeBehavior = new TestOnCreateNodePolicy(); + TestOnCreateNodePolicy fileTypeBehavior = new TestOnCreateNodePolicy(); + + // bind custom behavior for parent type + policyComponent.bindClassBehaviour(OnCreateNodePolicy.QNAME, BASE_TYPE, new JavaBehaviour(baseTypeBehavior, "onCreateNode")); + // bind custom behavior for child type + policyComponent.bindClassBehaviour(OnCreateNodePolicy.QNAME, FILE_TYPE, new JavaBehaviour(fileTypeBehavior, "onCreateNode")); + + UserTransaction transaction = trxService.getUserTransaction(); + try + { + transaction.begin(); + + final String name = "Test (" + System.currentTimeMillis() + ").docx"; + final Map contentProps = new HashMap(); + contentProps.put(ContentModel.PROP_NAME, name); + + // create node of child type + nodeService.createNode(companyHome, + ContentModel.ASSOC_CONTAINS, + QName.createQName(NamespaceService.CONTENT_MODEL_PREFIX, name), + FILE_TYPE, + contentProps); + transaction.commit(); + } + catch(Exception e) + { + try { transaction.rollback(); } catch (IllegalStateException ee) {} + throw e; + } + + assertTrue("Behavior should be executed for parent type.", baseTypeBehavior.isExecuted()); + assertEquals(1, baseTypeBehavior.getExecutionCount()); + assertTrue("Behavior should be executed for child type.", fileTypeBehavior.isExecuted()); + assertEquals(1, fileTypeBehavior.getExecutionCount()); + } + + /** + * Test for MNT_13836 + *

then disable the super- behaviour only and show that sub behaviour is still enabled and triggered

+ * @throws Exception + */ + public void testChildParentBehaviours2() throws Exception + { + TestOnCreateNodePolicy baseTypeBehavior = new TestOnCreateNodePolicy(); + TestOnCreateNodePolicy fileTypeBehavior = new TestOnCreateNodePolicy(); + + // bind custom behavior for parent type + policyComponent.bindClassBehaviour(OnCreateNodePolicy.QNAME, BASE_TYPE, new JavaBehaviour(baseTypeBehavior, "onCreateNode")); + // bind custom behavior for child type + policyComponent.bindClassBehaviour(OnCreateNodePolicy.QNAME, FILE_TYPE, new JavaBehaviour(fileTypeBehavior, "onCreateNode")); + + UserTransaction transaction = trxService.getUserTransaction(); + try + { + transaction.begin(); + // disable behavior for parent type + behaviourFilter.disableBehaviour(BASE_TYPE); + // check that behavior is disabled correctly + try + { + checkBehaviour(BASE_TYPE, companyHome, true, false, false, true); + + final String name = "Test (" + System.currentTimeMillis() + ").docx"; + final Map contentProps = new HashMap(); + contentProps.put(ContentModel.PROP_NAME, name); + + // create node of child type + nodeService.createNode(companyHome, + ContentModel.ASSOC_CONTAINS, + QName.createQName(NamespaceService.CONTENT_MODEL_PREFIX, name), + FILE_TYPE, + contentProps); + } + finally + { + behaviourFilter.enableBehaviour(BASE_TYPE); + checkBehaviour(BASE_TYPE, companyHome, true, true, true, true); + } + transaction.commit(); + } + catch(Exception e) + { + try { transaction.rollback(); } catch (IllegalStateException ee) {} + throw e; + } + + assertFalse("Behavior should not be executed for parent type.", baseTypeBehavior.isExecuted()); + assertEquals(0, baseTypeBehavior.getExecutionCount()); + assertTrue("Behavior should be executed for child type.", fileTypeBehavior.isExecuted()); + assertEquals(1, fileTypeBehavior.getExecutionCount()); + } + + /** + * Test for MNT_13836 + *

then also disable the sub- behaviour and show that neither behaviour is triggered

+ * @throws Exception + */ + public void testChildParentBehaviours3() throws Exception + { + TestOnCreateNodePolicy baseTypeBehavior = new TestOnCreateNodePolicy(); + TestOnCreateNodePolicy fileTypeBehavior = new TestOnCreateNodePolicy(); + + // bind custom behavior for parent type + policyComponent.bindClassBehaviour(OnCreateNodePolicy.QNAME, BASE_TYPE, new JavaBehaviour(baseTypeBehavior, "onCreateNode")); + // bind custom behavior for child type + policyComponent.bindClassBehaviour(OnCreateNodePolicy.QNAME, FILE_TYPE, new JavaBehaviour(fileTypeBehavior, "onCreateNode")); + + UserTransaction transaction = trxService.getUserTransaction(); + try + { + transaction.begin(); + // disable behavior for parent type + behaviourFilter.disableBehaviour(BASE_TYPE); + // check that behavior is disabled correctly + checkBehaviour(BASE_TYPE, companyHome, true, false, false, true); + + // disable behavior for child type + behaviourFilter.disableBehaviour(FILE_TYPE); + // check that behavior is disabled correctly + checkBehaviour(FILE_TYPE, companyHome, true, false, false, true); + try + { + final String name = "Test (" + System.currentTimeMillis() + ").docx"; + final Map contentProps = new HashMap(); + contentProps.put(ContentModel.PROP_NAME, name); + + // create node of child type + nodeService.createNode(companyHome, + ContentModel.ASSOC_CONTAINS, + QName.createQName(NamespaceService.CONTENT_MODEL_PREFIX, name), + FILE_TYPE, + contentProps); + } + finally + { + behaviourFilter.enableBehaviour(BASE_TYPE); + behaviourFilter.enableBehaviour(FILE_TYPE); + checkBehaviour(BASE_TYPE, companyHome, true, true, true, true); + checkBehaviour(FILE_TYPE, companyHome, true, true, true, true); + } + transaction.commit(); + } + catch(Exception e) + { + try { transaction.rollback(); } catch (IllegalStateException ee) {} + throw e; + } + + assertFalse("Behavior should not be executed for parent type.", baseTypeBehavior.isExecuted()); + assertEquals(0, baseTypeBehavior.getExecutionCount()); + assertFalse("Behavior should not be executed for child type.", fileTypeBehavior.isExecuted()); + assertEquals(0, fileTypeBehavior.getExecutionCount()); + } + + /** + * Test for MNT_13836 + *

then vice-versa, ie. disabling sub- behaviour does not disable inherited super- behaviours

+ * @throws Exception + */ + public void testChildParentBehaviours4() throws Exception + { + TestOnCreateNodePolicy baseTypeBehavior = new TestOnCreateNodePolicy(); + TestOnCreateNodePolicy fileTypeBehavior = new TestOnCreateNodePolicy(); + + // bind custom behavior for parent type + policyComponent.bindClassBehaviour(OnCreateNodePolicy.QNAME, BASE_TYPE, new JavaBehaviour(baseTypeBehavior, "onCreateNode")); + // bind custom behavior for child type + policyComponent.bindClassBehaviour(OnCreateNodePolicy.QNAME, FILE_TYPE, new JavaBehaviour(fileTypeBehavior, "onCreateNode")); + + UserTransaction transaction = trxService.getUserTransaction(); + try + { + transaction.begin(); + // disable behavior for child type + behaviourFilter.disableBehaviour(FILE_TYPE); + // check that behavior is disabled correctly + checkBehaviour(FILE_TYPE, companyHome, true, false, false, true); + + try + { + final String name = "Test (" + System.currentTimeMillis() + ").docx"; + final Map contentProps = new HashMap(); + contentProps.put(ContentModel.PROP_NAME, name); + + // create node of child type + nodeService.createNode(companyHome, + ContentModel.ASSOC_CONTAINS, + QName.createQName(NamespaceService.CONTENT_MODEL_PREFIX, name), + FILE_TYPE, + contentProps); + } + finally + { + behaviourFilter.enableBehaviour(FILE_TYPE); + checkBehaviour(FILE_TYPE, companyHome, true, true, true, true); + } + transaction.commit(); + } + catch(Exception e) + { + try { transaction.rollback(); } catch (IllegalStateException ee) {} + throw e; + } + + assertTrue("Behavior should be executed for parent type.", baseTypeBehavior.isExecuted()); + assertEquals(1, baseTypeBehavior.getExecutionCount()); + assertFalse("Behavior should not be executed for child type.", fileTypeBehavior.isExecuted()); + assertEquals(0, fileTypeBehavior.getExecutionCount()); + } + /** * @param className the class to check * @param nodeRef the node instance to check @@ -517,5 +753,28 @@ public class PolicyComponentTransactionTest extends TestCase } } + private class TestOnCreateNodePolicy implements OnCreateNodePolicy + { + private boolean executed; + private int executionCount; + + @Override + public void onCreateNode(ChildAssociationRef childAssocRef) + { + executed = true; + executionCount++; + } + + public boolean isExecuted() + { + return executed; + } + + public int getExecutionCount() + { + return executionCount; + } + } + } diff --git a/source/test-resources/org/alfresco/repo/policy/policycomponenttest_model.xml b/source/test-resources/org/alfresco/repo/policy/policycomponenttest_model.xml index f0268dda82..a5927e9617 100644 --- a/source/test-resources/org/alfresco/repo/policy/policycomponenttest_model.xml +++ b/source/test-resources/org/alfresco/repo/policy/policycomponenttest_model.xml @@ -2,6 +2,7 @@ + @@ -11,6 +12,7 @@ + sys:base d:text