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

@@ -512,8 +512,9 @@
     
     <bean id="lockStore" factory-bean="lockStoreFactory" factory-method="createLockStore"/>
     
-    <bean id="lockableAspectInterceptor" class="org.alfresco.repo.lock.mem.LockableAspectInterceptor">
+    <bean id="lockableAspectInterceptor" class="org.alfresco.repo.lock.mem.LockableAspectInterceptor" init-method="init">
         <property name="nodeService" ref="dbNodeService"/>
+        <property name="lockService" ref="lockService"/>
         <property name="lockStore" ref="lockStore"/>
         <property name="authenticationService" ref="authenticationService"/>
     </bean>

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.
  * <p>
- * There is no need for this interceptor to worry about setProperty, removeProperty, setProperties,
+ * This interceptor checks for EPHEMERAL lock directly - MNT-10477 fix.
- * 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.
  *  
  * @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<String> methodsToCheck = new HashSet<String>();
+    
     private final ThreadLocal<Boolean> threadEnabled;
 
     /**
@@ -67,6 +71,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
     public Object invoke(MethodInvocation invocation) throws Throwable
@@ -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<QName, Serializable> newProperties = (Map<QName, Serializable>) 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.
@@ -261,6 +287,15 @@ public class LockableAspectInterceptor implements MethodInterceptor
         return ephemeral;
     }
     
+    private void checkForLockIfEphemeral(NodeRef nodeRef)
+    {
+        LockState lockState = lockStore.get(nodeRef);
+        if (isEphemeralLock(lockState))
+        {
+            lockService.checkForLock(nodeRef);
+        }
+    }
+
     public void setLockStore(LockStore lockStore)
     {
         this.lockStore = lockStore;
@@ -275,4 +310,8 @@ public class LockableAspectInterceptor implements MethodInterceptor
     {
         this.nodeService = nodeService;
     }
+
+    public void setLockService(LockService lockService) {
+        this.lockService = lockService;
+    }
 }

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<QName, Serializable> props = new HashMap<QName, Serializable>();
+            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<QName, Serializable> props = new HashMap<QName, Serializable>();
+            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