+
+
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