From 3caceb54e34935d2214338183167b82202b606b1 Mon Sep 17 00:00:00 2001 From: Alan Davis Date: Wed, 2 Apr 2014 22:13:54 +0000 Subject: [PATCH] Merged HEAD-BUG-FIX (4.3/Cloud) to HEAD (4.3/Cloud) 65738: Merged V4.2-BUG-FIX (4.2.2) to HEAD-BUG-FIX (4.3/Cloud) 65639: Merged DEV to V4.2-BUG-FIX (4.2.2) 65624 : MNT-10477 : Lock on document being edited online persists when updating name of document - LockableAspectInterceptor checks for EPHEMERAL locks if they exists. Fix related test git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@66268 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261 --- config/alfresco/core-services-context.xml | 3 +- .../lock/mem/LockableAspectInterceptor.java | 45 ++++++- .../repo/lock/LockServiceImplTest.java | 118 ++++++++++++++++++ 3 files changed, 162 insertions(+), 4 deletions(-) diff --git a/config/alfresco/core-services-context.xml b/config/alfresco/core-services-context.xml index 6270d63e22..63c931f97a 100644 --- a/config/alfresco/core-services-context.xml +++ b/config/alfresco/core-services-context.xml @@ -512,8 +512,9 @@ - + + diff --git a/source/java/org/alfresco/repo/lock/mem/LockableAspectInterceptor.java b/source/java/org/alfresco/repo/lock/mem/LockableAspectInterceptor.java index addf5e8706..01dbc96406 100644 --- a/source/java/org/alfresco/repo/lock/mem/LockableAspectInterceptor.java +++ b/source/java/org/alfresco/repo/lock/mem/LockableAspectInterceptor.java @@ -20,10 +20,12 @@ package org.alfresco.repo.lock.mem; import java.io.Serializable; import java.util.HashMap; +import java.util.HashSet; import java.util.Map; import java.util.Set; import org.alfresco.model.ContentModel; +import org.alfresco.service.cmr.lock.LockService; import org.alfresco.service.cmr.repository.NodeRef; import org.alfresco.service.cmr.repository.NodeService; import org.alfresco.service.cmr.security.AuthenticationService; @@ -38,9 +40,7 @@ import org.aopalliance.intercept.MethodInvocation; * they are reported to have the cm:lockable aspect on them. As ephemeral locks are only held in memory * the nodes have not been marked with this aspect, so the aspect must be spoofed. *

- * There is no need for this interceptor to worry about setProperty, removeProperty, setProperties, - * addProperties since if a service other than the lock service attempts to set cm:lockable properties when it - * is already locked, the beforeUpdateNode node service policy will disallow the update anyway. + * This interceptor checks for EPHEMERAL lock directly - MNT-10477 fix. * * @author Matt Ward */ @@ -49,6 +49,10 @@ public class LockableAspectInterceptor implements MethodInterceptor private LockStore lockStore; private AuthenticationService authenticationService; private NodeService nodeService; + private LockService lockService; + + private Set methodsToCheck = new HashSet(); + private final ThreadLocal threadEnabled; /** @@ -66,6 +70,18 @@ public class LockableAspectInterceptor implements MethodInterceptor } }; } + + public void init() + { + /* check for lock before following methods proceed - MNT-10477 */ + methodsToCheck.add("addAspect"); + methodsToCheck.add("addProperties"); + methodsToCheck.add("removeAspect"); + methodsToCheck.add("removeProperty"); + methodsToCheck.add("setProperties"); + methodsToCheck.add("setProperty"); + methodsToCheck.add("setType"); + } @SuppressWarnings("unchecked") @Override @@ -174,6 +190,8 @@ public class LockableAspectInterceptor implements MethodInterceptor // TODO: This is potentially creating an ephemeral lock here, put it in the lockstore? NodeRef nodeRef = (NodeRef) args[0]; Map newProperties = (Map) args[1]; + /* MNT-10477 fix */ + checkForLockIfEphemeral(nodeRef); if (newProperties.get(ContentModel.PROP_LOCK_LIFETIME) == Lifetime.EPHEMERAL) { @@ -194,6 +212,14 @@ public class LockableAspectInterceptor implements MethodInterceptor return invocation.proceed(); } } + else if (methodsToCheck.contains(methodName)) + { + NodeRef nodeRef = (NodeRef) args[0]; + /* MNT-10477 fix */ + checkForLockIfEphemeral(nodeRef); + + return invocation.proceed(); + } else { // If not a special case, invoke the original method. @@ -260,6 +286,15 @@ public class LockableAspectInterceptor implements MethodInterceptor lockState.getLifetime() == Lifetime.EPHEMERAL; return ephemeral; } + + private void checkForLockIfEphemeral(NodeRef nodeRef) + { + LockState lockState = lockStore.get(nodeRef); + if (isEphemeralLock(lockState)) + { + lockService.checkForLock(nodeRef); + } + } public void setLockStore(LockStore lockStore) { @@ -275,4 +310,8 @@ public class LockableAspectInterceptor implements MethodInterceptor { this.nodeService = nodeService; } + + public void setLockService(LockService lockService) { + this.lockService = lockService; + } } diff --git a/source/test-java/org/alfresco/repo/lock/LockServiceImplTest.java b/source/test-java/org/alfresco/repo/lock/LockServiceImplTest.java index cff32db3cc..59eccc7fec 100644 --- a/source/test-java/org/alfresco/repo/lock/LockServiceImplTest.java +++ b/source/test-java/org/alfresco/repo/lock/LockServiceImplTest.java @@ -367,6 +367,124 @@ public class LockServiceImplTest extends BaseSpringTest assertFalse(rs.getNodeRefs().contains(noAspectNode)); } + /* MNT-10477 related test */ + @Test + public void testEphemeralLockModifyNode() + { + TestWithUserUtils.authenticateUser(GOOD_USER_NAME, PWD, rootNodeRef, this.authenticationService); + + // Check that the node is not currently locked + assertEquals(LockStatus.NO_LOCK, lockService.getLockStatus(noAspectNode)); + + // Check that there really is no lockable aspect + assertEquals(false, nodeService.hasAspect(noAspectNode, ContentModel.ASPECT_LOCKABLE)); + + // Lock the node + lockService.lock(noAspectNode, LockType.WRITE_LOCK, 86400, Lifetime.EPHEMERAL, "some extra data"); + + // get bad user + TestWithUserUtils.authenticateUser(BAD_USER_NAME, PWD, rootNodeRef, this.authenticationService); + + assertEquals(LockStatus.LOCKED, lockService.getLockStatus(noAspectNode)); + + NodeService fullNodeService = (NodeService) applicationContext.getBean("nodeService"); + + /* addProperties test */ + try + { + Map props = new HashMap(); + props.put(ContentModel.PROP_DESCRIPTION, "descr" + System.currentTimeMillis()); + props.put(ContentModel.PROP_TITLE, "title" + System.currentTimeMillis()); + fullNodeService.addProperties(noAspectNode, props); + + fail(); + } + catch(NodeLockedException e) + { + // it's ok - node supposed to be locked + } + + /* setProperty test */ + try + { + fullNodeService.setProperty(noAspectNode, ContentModel.PROP_DESCRIPTION, "descr" + System.currentTimeMillis()); + + fail(); + } + catch(NodeLockedException e) + { + // it's ok - node supposed to be locked + } + + /* setProperties test */ + try + { + Map props = new HashMap(); + props.put(ContentModel.PROP_DESCRIPTION, "descr" + System.currentTimeMillis()); + props.put(ContentModel.PROP_TITLE, "title" + System.currentTimeMillis()); + fullNodeService.setProperties(noAspectNode, props); + + fail(); + } + catch(NodeLockedException e) + { + // it's ok - node supposed to be locked + } + + /* removeProperty test */ + try + { + fullNodeService.removeProperty(noAspectNode, ContentModel.PROP_DESCRIPTION); + + fail(); + } + catch(NodeLockedException e) + { + // it's ok - node supposed to be locked + } + + /* addAspect test */ + try + { + fullNodeService.addAspect(noAspectNode, ContentModel.ASPECT_AUTHOR , null); + + fail(); + } + catch(NodeLockedException e) + { + // it's ok - node supposed to be locked + } + + /* removeAspect test */ + try + { + fullNodeService.removeAspect(noAspectNode, ContentModel.ASPECT_AUTHOR); + + fail(); + } + catch(NodeLockedException e) + { + // it's ok - node supposed to be locked + } + + /* setType test */ + try + { + fullNodeService.setType(noAspectNode, ContentModel.TYPE_CMOBJECT); + + fail(); + } + catch(NodeLockedException e) + { + // it's ok - node supposed to be locked + } + + TestWithUserUtils.authenticateUser(GOOD_USER_NAME, PWD, rootNodeRef, this.authenticationService); + + lockService.unlock(noAspectNode); + assertEquals(LockStatus.NO_LOCK, lockService.getLockStatus(noAspectNode)); + } + public void testLockRevertedOnRollback() throws NotSupportedException, SystemException { // Preconditions of test