diff --git a/source/java/org/alfresco/repo/lock/LockServiceImpl.java b/source/java/org/alfresco/repo/lock/LockServiceImpl.java index 834280c2cc..6f53c74483 100644 --- a/source/java/org/alfresco/repo/lock/LockServiceImpl.java +++ b/source/java/org/alfresco/repo/lock/LockServiceImpl.java @@ -2,7 +2,7 @@ * #%L * Alfresco Repository * %% - * Copyright (C) 2005 - 2016 Alfresco Software Limited + * Copyright (C) 2005 - 2017 Alfresco Software Limited * %% * This file is part of the Alfresco software. * If the software was purchased under a paid Alfresco license, the terms of @@ -540,7 +540,7 @@ public class LockServiceImpl implements LockService, } finally { - behaviourFilter.enableBehaviour(nodeRef, ContentModel.ASPECT_VERSIONABLE); + behaviourFilter.enableBehaviour(nodeRef, ContentModel.ASPECT_VERSIONABLE); lockableAspectInterceptor.enableForThread(); removeFromIgnoreSet(nodeRef); } @@ -919,6 +919,18 @@ public class LockServiceImpl implements LockService, nodeRef = tenantService.getName(nodeRef); LockState lockState = lockStore.get(nodeRef); + if (lockState != null) + { + String lockOwner = lockState.getOwner(); + Date expiryDate = lockState.getExpires(); + LockStatus status = LockUtils.lockStatus(lockOwner, lockOwner, expiryDate); + // in-memory ephemeral lock which is expired is irrelevant + if (status.equals(LockStatus.LOCK_EXPIRED)) + { + lockState = null; + } + } + //ALF-20361: It is possible that a rollback has resulted in a "non-lock" lock state being added to //the lock store. Because of that, we check both whether the retrieved lockState is null and, if it isn't, //whether it represents a real lock diff --git a/source/java/org/alfresco/repo/lock/mem/LockableAspectInterceptor.java b/source/java/org/alfresco/repo/lock/mem/LockableAspectInterceptor.java index 23f94fdee5..29021ea7f8 100644 --- a/source/java/org/alfresco/repo/lock/mem/LockableAspectInterceptor.java +++ b/source/java/org/alfresco/repo/lock/mem/LockableAspectInterceptor.java @@ -2,7 +2,7 @@ * #%L * Alfresco Repository * %% - * Copyright (C) 2005 - 2016 Alfresco Software Limited + * Copyright (C) 2005 - 2017 Alfresco Software Limited * %% * This file is part of the Alfresco software. * If the software was purchased under a paid Alfresco license, the terms of @@ -25,16 +25,19 @@ */ package org.alfresco.repo.lock.mem; -import java.io.Serializable; +import java.io.Serializable; +import java.util.Date; import java.util.HashMap; import java.util.HashSet; import java.util.Map; import java.util.Set; -import org.alfresco.model.ContentModel; +import org.alfresco.model.ContentModel; +import org.alfresco.repo.lock.LockUtils; import org.alfresco.repo.lock.traitextender.LockableAspectInterceptorExtension; import org.alfresco.repo.lock.traitextender.LockableAspectInterceptorTrait; -import org.alfresco.service.cmr.lock.LockService; +import org.alfresco.service.cmr.lock.LockService; +import org.alfresco.service.cmr.lock.LockStatus; import org.alfresco.service.cmr.repository.NodeRef; import org.alfresco.service.cmr.repository.NodeService; import org.alfresco.service.cmr.security.AuthenticationService; @@ -333,7 +336,18 @@ public class LockableAspectInterceptor implements MethodInterceptor, Extensible @Extend(traitAPI=LockableAspectInterceptorTrait.class,extensionAPI=LockableAspectInterceptorExtension.class) private LockState getLockState(NodeRef nodeRef) { - LockState lockState = lockStore.get(nodeRef); + LockState lockState = lockStore.get(nodeRef); + // Disregard in-memory lock if expired + if (lockState != null) + { + String lockOwner = lockState.getOwner(); + Date expiryDate = lockState.getExpires(); + LockStatus status = LockUtils.lockStatus(lockOwner, lockOwner, expiryDate); + if (status.equals(LockStatus.LOCK_EXPIRED)) + { + lockState = null; + } + } return lockState; } diff --git a/source/test-java/org/alfresco/repo/lock/LockServiceImplTest.java b/source/test-java/org/alfresco/repo/lock/LockServiceImplTest.java index 5bcda01df0..71ca0e76c3 100644 --- a/source/test-java/org/alfresco/repo/lock/LockServiceImplTest.java +++ b/source/test-java/org/alfresco/repo/lock/LockServiceImplTest.java @@ -2,7 +2,7 @@ * #%L * Alfresco Repository * %% - * Copyright (C) 2005 - 2016 Alfresco Software Limited + * Copyright (C) 2005 - 2017 Alfresco Software Limited * %% * This file is part of the Alfresco software. * If the software was purchased under a paid Alfresco license, the terms of @@ -520,6 +520,59 @@ public class LockServiceImplTest extends BaseSpringTest assertFalse(lockService.isLocked(noAspectNode)); } + /** + * Test that covers MNT-17612 - having an expired ephemeral lock and a persistent one on the same node. + */ + public void testExpiredEphemeralLockAndPersitentLock() + { + TestWithUserUtils.authenticateUser(GOOD_USER_NAME, PWD, rootNodeRef, this.authenticationService); + + // Check that the node is not currently locked + assertEquals(LockStatus.NO_LOCK, securedLockService.getLockStatus(noAspectNode)); + assertFalse(securedLockService.isLocked(noAspectNode)); + + // Check that there really is no lockable aspect + assertEquals(false, nodeService.hasAspect(noAspectNode, ContentModel.ASPECT_LOCKABLE)); + + // Lock the node + securedLockService.lock(noAspectNode, LockType.WRITE_LOCK, 1, Lifetime.EPHEMERAL); + + // Check that we can retrieve LockState + LockState lockState = securedLockService.getLockState(noAspectNode); + assertEquals(noAspectNode, lockState.getNodeRef()); + assertEquals(LockType.WRITE_LOCK, lockState.getLockType()); + assertEquals(GOOD_USER_NAME, lockState.getOwner()); + assertEquals(Lifetime.EPHEMERAL, lockState.getLifetime()); + assertNotNull(lockState.getExpires()); + + // Wait for 2 seconds to give the ephemeral lock time to expire + try {Thread.sleep(2*1000);} catch (Exception exception){}; + + assertFalse(securedLockService.isLocked(noAspectNode)); + + // Do a persistent lock with a different user (simulate an Edit Offline - MNT-17612) + TestWithUserUtils.authenticateUser(BAD_USER_NAME, PWD, rootNodeRef, this.authenticationService); + + // Lock the node + securedLockService.lock(noAspectNode, LockType.READ_ONLY_LOCK, 1000, Lifetime.PERSISTENT); + + assertTrue(securedLockService.isLocked(noAspectNode)); + assertEquals(true, nodeService.hasAspect(noAspectNode, ContentModel.ASPECT_LOCKABLE)); + assertEquals(LockType.READ_ONLY_LOCK, securedLockService.getLockType(noAspectNode)); + + // Check that we can retrieve LockState + lockState = securedLockService.getLockState(noAspectNode); + assertEquals(noAspectNode, lockState.getNodeRef()); + assertEquals(LockType.READ_ONLY_LOCK, lockState.getLockType()); + assertEquals(BAD_USER_NAME, lockState.getOwner()); + assertEquals(Lifetime.PERSISTENT, lockState.getLifetime()); + + // Check unlock + securedLockService.unlock(noAspectNode); + + assertFalse(securedLockService.isLocked(noAspectNode)); + } + public void testLockRevertedOnRollback() throws NotSupportedException, SystemException { // Preconditions of test diff --git a/source/test-java/org/alfresco/repo/lock/mem/LockableAspectInterceptorTest.java b/source/test-java/org/alfresco/repo/lock/mem/LockableAspectInterceptorTest.java index b7d6ed5eb5..9580e0b626 100644 --- a/source/test-java/org/alfresco/repo/lock/mem/LockableAspectInterceptorTest.java +++ b/source/test-java/org/alfresco/repo/lock/mem/LockableAspectInterceptorTest.java @@ -2,7 +2,7 @@ * #%L * Alfresco Repository * %% - * Copyright (C) 2005 - 2016 Alfresco Software Limited + * Copyright (C) 2005 - 2017 Alfresco Software Limited * %% * This file is part of the Alfresco software. * If the software was purchased under a paid Alfresco license, the terms of @@ -25,46 +25,47 @@ */ package org.alfresco.repo.lock.mem; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; - -import java.io.InputStream; -import java.io.Serializable; -import java.util.Date; -import java.util.HashMap; -import java.util.Map; -import java.util.Set; -import java.util.concurrent.Callable; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; -import java.util.concurrent.Future; - -import org.alfresco.model.ContentModel; -import org.alfresco.repo.action.executer.ContentMetadataExtracter; -import org.alfresco.repo.security.authentication.AuthenticationUtil; -import org.alfresco.repo.transaction.RetryingTransactionHelper.RetryingTransactionCallback; -import org.alfresco.service.cmr.action.Action; -import org.alfresco.service.cmr.action.ActionService; -import org.alfresco.service.cmr.lock.LockService; -import org.alfresco.service.cmr.lock.LockType; -import org.alfresco.service.cmr.model.FileFolderService; -import org.alfresco.service.cmr.repository.ContentWriter; -import org.alfresco.service.cmr.repository.NodeRef; -import org.alfresco.service.cmr.repository.NodeService; -import org.alfresco.service.cmr.repository.StoreRef; -import org.alfresco.service.namespace.QName; -import org.alfresco.service.transaction.TransactionService; -import org.alfresco.util.ApplicationContextHelper; -import org.alfresco.util.GUID; -import org.junit.AfterClass; -import org.junit.Before; -import org.junit.BeforeClass; -import org.junit.Test; -import org.springframework.context.ApplicationContext; - +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +import java.io.InputStream; +import java.io.Serializable; +import java.util.Calendar; +import java.util.Date; +import java.util.HashMap; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.Callable; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; + +import org.alfresco.model.ContentModel; +import org.alfresco.repo.action.executer.ContentMetadataExtracter; +import org.alfresco.repo.security.authentication.AuthenticationUtil; +import org.alfresco.repo.transaction.RetryingTransactionHelper.RetryingTransactionCallback; +import org.alfresco.service.cmr.action.Action; +import org.alfresco.service.cmr.action.ActionService; +import org.alfresco.service.cmr.lock.LockService; +import org.alfresco.service.cmr.lock.LockType; +import org.alfresco.service.cmr.model.FileFolderService; +import org.alfresco.service.cmr.repository.ContentWriter; +import org.alfresco.service.cmr.repository.NodeRef; +import org.alfresco.service.cmr.repository.NodeService; +import org.alfresco.service.cmr.repository.StoreRef; +import org.alfresco.service.namespace.QName; +import org.alfresco.service.transaction.TransactionService; +import org.alfresco.util.ApplicationContextHelper; +import org.alfresco.util.GUID; +import org.junit.AfterClass; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Test; +import org.springframework.context.ApplicationContext; + public class LockableAspectInterceptorTest { private static ApplicationContext appCtx; @@ -287,17 +288,18 @@ public class LockableAspectInterceptorTest properties.containsKey(ContentModel.PROP_LOCK_OWNER)); assertTrue("Node should have created property", properties.containsKey(ContentModel.PROP_CREATED)); - - Date now = new Date(); - // Set a lock on the node and reload the properties + + // Set a lock on the node and reload the properties + // Make sure the ephemeral lock won't expire before asserting cm:lockable properties (spoofed). + Date expire = makeExpiryDate(2); lockStore.set(nodeRef, - LockState.createLock(nodeRef, LockType.WRITE_LOCK, lockOwner, now, Lifetime.EPHEMERAL, null)); + LockState.createLock(nodeRef, LockType.WRITE_LOCK, lockOwner, expire, Lifetime.EPHEMERAL, null)); properties = nodeService.getProperties(nodeRef); // cm:lockable properties should be spoofed assertEquals(lockOwner, properties.get(ContentModel.PROP_LOCK_OWNER)); assertEquals(LockType.WRITE_LOCK.toString(), properties.get(ContentModel.PROP_LOCK_TYPE)); - assertEquals(now, properties.get(ContentModel.PROP_EXPIRY_DATE)); + assertEquals(expire, properties.get(ContentModel.PROP_EXPIRY_DATE)); // Spoofed - wasn't explicitly added. assertEquals(Lifetime.EPHEMERAL.toString(), properties.get(ContentModel.PROP_LOCK_LIFETIME)); @@ -319,16 +321,18 @@ public class LockableAspectInterceptorTest assertEquals(null, nodeService.getProperty(nodeRef, ContentModel.PROP_LOCK_OWNER)); assertEquals(null, nodeService.getProperty(nodeRef, ContentModel.PROP_LOCK_TYPE)); assertEquals(null, nodeService.getProperty(nodeRef, ContentModel.PROP_EXPIRY_DATE)); - assertEquals(null, nodeService.getProperty(nodeRef, ContentModel.PROP_LOCK_LIFETIME)); - Date now = new Date(); - // Set a lock on the node + assertEquals(null, nodeService.getProperty(nodeRef, ContentModel.PROP_LOCK_LIFETIME)); + + // Set a lock on the node + // Make sure the ephemeral lock won't expire before asserting cm:lockable properties (spoofed). + Date expire = makeExpiryDate(2); lockStore.set(nodeRef, - LockState.createLock(nodeRef, LockType.WRITE_LOCK, lockOwner, now, Lifetime.EPHEMERAL, null)); + LockState.createLock(nodeRef, LockType.WRITE_LOCK, lockOwner, expire, Lifetime.EPHEMERAL, null)); // cm:lockable properties should be spoofed assertEquals(lockOwner, nodeService.getProperty(nodeRef, ContentModel.PROP_LOCK_OWNER)); assertEquals(LockType.WRITE_LOCK.toString(), nodeService.getProperty(nodeRef, ContentModel.PROP_LOCK_TYPE)); - assertEquals(now, nodeService.getProperty(nodeRef, ContentModel.PROP_EXPIRY_DATE)); + assertEquals(expire, nodeService.getProperty(nodeRef, ContentModel.PROP_EXPIRY_DATE)); assertEquals(Lifetime.EPHEMERAL.toString(), nodeService.getProperty(nodeRef, ContentModel.PROP_LOCK_LIFETIME)); } @@ -583,5 +587,20 @@ public class LockableAspectInterceptorTest } }); AuthenticationUtil.popAuthentication(); + } + + private Date makeExpiryDate(int timeToExpireInSeconds) + { + // Set the expiry date + Date expiryDate = null; + if (timeToExpireInSeconds > 0) + { + expiryDate = new Date(); + Calendar calendar = Calendar.getInstance(); + calendar.setTime(expiryDate); + calendar.add(Calendar.SECOND, timeToExpireInSeconds); + expiryDate = calendar.getTime(); + } + return expiryDate; } }