From 68f7ce770ea39f4fff25b802fa5c401d181f2aa1 Mon Sep 17 00:00:00 2001 From: Alan Davis Date: Sat, 31 Jan 2015 11:11:51 +0000 Subject: [PATCH] Merged HEAD-BUG-FIX (5.1/Cloud) to HEAD (5.1/Cloud) 91057: Merged V4.2-BUG-FIX (4.2.5) to HEAD-BUG-FIX (5.0/Cloud) 90986: MNT-11732: ephemeral locks have configurable threshold over which locks will be automatically made persistent instead. By setting the new property, administrators may control how ephemeral locks are created, for example: property alfresco.ephemeralLock.expiryThresh=30 This will mean that when a LockService client requests that an ephemeral lock is created with a timeout of greater than 30 seconds, the lock will be created as a persistent lock instead (the ephemeral lifetime request is vetoed). By setting this property to -1, ALL locks will be created as persistent locks (giving the old, pre-ephemeral locking behaviour). By leaving this at the default (48 hours) the newer locking behaviour is completely unaffected, e.g. ephemeral locks may be created with up to 48 hour expiry time and over this limit an exception is thrown (and the lock is not created). By setting the value to something in between these settings it is possible to have quite a nice balance of behaviour. Using the example of 30 seconds, as above, will mean that using Share's "Edit Online" to edit a document in MS Word will result in a persistent lock (as MS Word requests approx 1 hour for its locks). After Solr has performed its next incremental index, then the Word document may be seen in the "Document's I'm Editing" filter in Share, since this is a persistent (in-DB) lock. git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@94765 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261 --- config/alfresco/core-services-context.xml | 1 + config/alfresco/model/contentModel.xml | 4 ++ config/alfresco/repository.properties | 7 ++ .../alfresco/repo/lock/LockServiceImpl.java | 30 ++++++-- .../service/cmr/lock/LockService.java | 11 ++- .../repo/lock/LockServiceImplTest.java | 70 ++++++++++++++++--- .../mem/LockableAspectInterceptorTest.java | 2 + 7 files changed, 106 insertions(+), 19 deletions(-) diff --git a/config/alfresco/core-services-context.xml b/config/alfresco/core-services-context.xml index 196149832c..5a7e99d2aa 100644 --- a/config/alfresco/core-services-context.xml +++ b/config/alfresco/core-services-context.xml @@ -431,6 +431,7 @@ + diff --git a/config/alfresco/model/contentModel.xml b/config/alfresco/model/contentModel.xml index 2f5c65c3db..53f30c333a 100644 --- a/config/alfresco/model/contentModel.xml +++ b/config/alfresco/model/contentModel.xml @@ -1076,6 +1076,10 @@ d:boolean true + + d:text + true + diff --git a/config/alfresco/repository.properties b/config/alfresco/repository.properties index f671759f27..8ca51d9042 100644 --- a/config/alfresco/repository.properties +++ b/config/alfresco/repository.properties @@ -1072,6 +1072,13 @@ alfresco.jmx.connector.enabled=true # Please, see MNT-11895 for details. system.propval.uniquenessCheck.enabled=true +# Requests for ephemeral (in-memory) locks with expiry times (in seconds) greater +# than this value will result in persistent locks being created instead. By default +# this value is equal to the maximum allowed expiry for ephemeral locks, therefore +# this feature is disabled by default. Setting this to -1 would mean that ALL +# requests for ephemeral locks would result in persistent locks being created. +alfresco.ephemeralLock.expiryThresh=172800 + # SurfConfigFolder Patch # # Do we defer running the surf-config folder patch? diff --git a/source/java/org/alfresco/repo/lock/LockServiceImpl.java b/source/java/org/alfresco/repo/lock/LockServiceImpl.java index 9ae7dea6fb..af30667b7c 100644 --- a/source/java/org/alfresco/repo/lock/LockServiceImpl.java +++ b/source/java/org/alfresco/repo/lock/LockServiceImpl.java @@ -104,6 +104,8 @@ public class LockServiceImpl implements LockService, private NodeIndexer nodeIndexer; + private int ephemeralExpiryThreshold; + public void setNodeService(NodeService nodeService) { this.nodeService = nodeService; @@ -299,15 +301,16 @@ public class LockServiceImpl implements LockService, public void lock(NodeRef nodeRef, LockType lockType, int timeToExpire, Lifetime lifetime, String additionalInfo) { invokeBeforeLock(nodeRef, lockType); - if (additionalInfo != null && !lifetime.equals(Lifetime.EPHEMERAL)) - { - throw new IllegalArgumentException("additionalInfo may only be provided for ephemeral locks."); - } if (lifetime.equals(Lifetime.EPHEMERAL) && (timeToExpire > MAX_EPHEMERAL_LOCK_SECONDS)) { throw new IllegalArgumentException("Attempt to create ephemeral lock for " + timeToExpire + " seconds - exceeds maximum allowed time."); } + if (lifetime.equals(Lifetime.EPHEMERAL) && (timeToExpire > ephemeralExpiryThreshold)) + { + lifetime = Lifetime.PERSISTENT; + } + nodeRef = tenantService.getName(nodeRef); @@ -344,7 +347,7 @@ public class LockServiceImpl implements LockService, { // Add lock aspect if not already present ensureLockAspect(nodeRef); - persistLockProps(nodeRef, lockType, lifetime, userName, expiryDate); + persistLockProps(nodeRef, lockType, lifetime, userName, expiryDate, additionalInfo); } finally { @@ -372,7 +375,7 @@ public class LockServiceImpl implements LockService, } } - private void persistLockProps(NodeRef nodeRef, LockType lockType, Lifetime lifetime, String userName, Date expiryDate) + private void persistLockProps(NodeRef nodeRef, LockType lockType, Lifetime lifetime, String userName, Date expiryDate, String additionalInfo) { addToIgnoreSet(nodeRef); try @@ -382,6 +385,7 @@ public class LockServiceImpl implements LockService, this.nodeService.setProperty(nodeRef, ContentModel.PROP_LOCK_TYPE, lockType.toString()); this.nodeService.setProperty(nodeRef, ContentModel.PROP_LOCK_LIFETIME, lifetime.toString()); this.nodeService.setProperty(nodeRef, ContentModel.PROP_EXPIRY_DATE, expiryDate); + this.nodeService.setProperty(nodeRef, ContentModel.PROP_LOCK_ADDITIONAL_INFO, additionalInfo); } finally { @@ -845,6 +849,7 @@ public class LockServiceImpl implements LockService, LockType lockType = lockTypeStr != null ? LockType.valueOf(lockTypeStr) : null; String lifetimeStr = (String) nodeService.getProperty(nodeRef, ContentModel.PROP_LOCK_LIFETIME); Lifetime lifetime = lifetimeStr != null ? Lifetime.valueOf(lifetimeStr) : null; + String additionalInfo = (String) nodeService.getProperty(nodeRef, ContentModel.PROP_LOCK_ADDITIONAL_INFO); // Mark lockstate as PERSISTENT as it was in the persistent storage! lockState = LockState.createLock( @@ -853,7 +858,7 @@ public class LockServiceImpl implements LockService, lockOwner, expiryDate, lifetime, - null); + additionalInfo); } else { @@ -912,4 +917,15 @@ public class LockServiceImpl implements LockService, lockStore.set(lockInfo.getNodeRef(), lockInfo); } } + + @Override + public void setEphemeralExpiryThreshold(int threshSecs) + { + ephemeralExpiryThreshold = threshSecs; + } + + public int getEphemeralExpiryThreshold() + { + return ephemeralExpiryThreshold; + } } diff --git a/source/java/org/alfresco/service/cmr/lock/LockService.java b/source/java/org/alfresco/service/cmr/lock/LockService.java index 997dc5f07c..6dee4739a2 100644 --- a/source/java/org/alfresco/service/cmr/lock/LockService.java +++ b/source/java/org/alfresco/service/cmr/lock/LockService.java @@ -19,14 +19,12 @@ package org.alfresco.service.cmr.lock; import java.util.Collection; -import java.util.List; import org.alfresco.api.AlfrescoPublicApi; import org.alfresco.repo.lock.mem.Lifetime; import org.alfresco.repo.lock.mem.LockState; import org.alfresco.service.Auditable; import org.alfresco.service.cmr.repository.NodeRef; -import org.alfresco.service.cmr.repository.StoreRef; /** @@ -363,4 +361,13 @@ public interface LockService * @return LockState */ public LockState getLockState(NodeRef nodeRef); + + /** + * Specifies the maximum expiry time for which a request for an ephemeral lock + * will be honoured. Requests for ephemeral locks with expiry times greater than + * this value will be automatically converted to a request for a persistent lock. + * + * @param threshSecs + */ + public void setEphemeralExpiryThreshold(int threshSecs); } diff --git a/source/test-java/org/alfresco/repo/lock/LockServiceImplTest.java b/source/test-java/org/alfresco/repo/lock/LockServiceImplTest.java index 6cda7f55f7..709dd6829c 100644 --- a/source/test-java/org/alfresco/repo/lock/LockServiceImplTest.java +++ b/source/test-java/org/alfresco/repo/lock/LockServiceImplTest.java @@ -18,6 +18,8 @@ */ package org.alfresco.repo.lock; +import static org.junit.Assert.assertNotEquals; + import java.io.Serializable; import java.util.HashMap; import java.util.List; @@ -257,17 +259,12 @@ public class LockServiceImplTest extends BaseSpringTest this.lockService.lock(this.noAspectNode, LockType.WRITE_LOCK); } - public void testPersistentLockDisallowsAdditionalInfo() + public void testPersistentLockMayStoreAdditionalInfo() { - try - { - lockService.lock(noAspectNode, LockType.NODE_LOCK, 0, Lifetime.PERSISTENT, "additional info"); - fail("additionalInfo must be null for persistent lock, expected IllegalArgumentException."); - } - catch (IllegalArgumentException e) - { - // Good, exception was thrown. - } + lockService.lock(noAspectNode, LockType.NODE_LOCK, 0, Lifetime.PERSISTENT, "additional info"); + + LockState lockState = lockService.getLockState(noAspectNode); + assertEquals("additional info", lockState.getAdditionalInfo()); } public void testEphemeralLock() @@ -818,6 +815,59 @@ public class LockServiceImplTest extends BaseSpringTest assertEquals(LockStatus.LOCK_EXPIRED, this.lockService.getLockStatus(this.parentNode)); } + public void testEphemeralExpiryThreshold() + { + TestWithUserUtils.authenticateUser(GOOD_USER_NAME, PWD, rootNodeRef, this.authenticationService); + final int origThresh = ((LockServiceImpl)lockService).getEphemeralExpiryThreshold(); + // Check the default situation is that the threshold does not apply. + assertEquals(LockServiceImpl.MAX_EPHEMERAL_LOCK_SECONDS, origThresh); + try + { + // Set the ephemeral expiry threshold to a much smaller value than the default + // so that it takes effect. + lockService.setEphemeralExpiryThreshold(300); + + // Check for an expiry time that should be unaffected by the threshold. + checkLifetimeForExpiry(Lifetime.EPHEMERAL, 0, Lifetime.EPHEMERAL); + checkLifetimeForExpiry(Lifetime.EPHEMERAL, 150, Lifetime.EPHEMERAL); + + // Check the largest allowed ephemeral expiry time. + checkLifetimeForExpiry(Lifetime.EPHEMERAL, 300, Lifetime.EPHEMERAL); + + // When the expiry is greater than the threshold, then the lock should be + // applied as a persistent lock. + checkLifetimeForExpiry(Lifetime.PERSISTENT, 301, Lifetime.EPHEMERAL); + + // Switch off ephemeral locks entirely + lockService.setEphemeralExpiryThreshold(-1); + // Always persistent... + checkLifetimeForExpiry(Lifetime.PERSISTENT, 0, Lifetime.EPHEMERAL); + checkLifetimeForExpiry(Lifetime.PERSISTENT, 150, Lifetime.EPHEMERAL); + checkLifetimeForExpiry(Lifetime.PERSISTENT, 300, Lifetime.EPHEMERAL); + checkLifetimeForExpiry(Lifetime.PERSISTENT, 301, Lifetime.EPHEMERAL); + } + finally + { + lockService.setEphemeralExpiryThreshold(origThresh); + } + } + + private void checkLifetimeForExpiry(Lifetime expectedLifetime, int expirySecs, Lifetime requestedLifetime) + { + lockService.unlock(parentNode); + assertNotEquals(LockStatus.LOCKED ,lockService.getLockStatus(parentNode)); + lockService.lock(parentNode, LockType.WRITE_LOCK, expirySecs, requestedLifetime); + LockState lock = lockService.getLockState(parentNode); + assertEquals(expectedLifetime, lock.getLifetime()); + + // Check that for any timeouts we test, a request for a persistent lock always yields a persistent lock. + lockService.unlock(parentNode); + assertNotEquals(LockStatus.LOCKED ,lockService.getLockStatus(parentNode)); + lockService.lock(parentNode, LockType.WRITE_LOCK, expirySecs, Lifetime.PERSISTENT); + lock = lockService.getLockState(parentNode); + assertEquals(Lifetime.PERSISTENT, lock.getLifetime()); + } + /** * Unit test to validate the behaviour of creating children of locked nodes. * No lock - can create children 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 19e474715a..ff8d9444ff 100644 --- a/source/test-java/org/alfresco/repo/lock/mem/LockableAspectInterceptorTest.java +++ b/source/test-java/org/alfresco/repo/lock/mem/LockableAspectInterceptorTest.java @@ -210,6 +210,7 @@ public class LockableAspectInterceptorTest lockProps.put(ContentModel.PROP_LOCK_OWNER, "jbloggs"); lockProps.put(ContentModel.PROP_LOCK_TYPE, LockType.READ_ONLY_LOCK.toString()); lockProps.put(ContentModel.PROP_EXPIRY_DATE, now); + lockProps.put(ContentModel.PROP_LOCK_ADDITIONAL_INFO, "{ \"fieldName\": \"extra lock info\" }"); nodeService.addAspect(nodeRef, ContentModel.ASPECT_LOCKABLE, lockProps); Map readProps = nodeService.getProperties(nodeRef); @@ -221,6 +222,7 @@ public class LockableAspectInterceptorTest assertEquals(Lifetime.PERSISTENT.toString(), readProps.get(ContentModel.PROP_LOCK_LIFETIME)); // Double check - not really present ensurePropertyNotPresent(nodeRef, ContentModel.PROP_LOCK_LIFETIME); + assertEquals("{ \"fieldName\": \"extra lock info\" }", readProps.get(ContentModel.PROP_LOCK_ADDITIONAL_INFO)); } /**