From 4055228bdb36275e74f3476da584e9079bbedd5e Mon Sep 17 00:00:00 2001 From: Matt Ward Date: Tue, 24 Sep 2013 12:14:46 +0000 Subject: [PATCH] Merged BRANCHES/DEV/mward/head_locktry_fix to HEAD: 55423: ALF-20031: LockStore is no longer used as a cache for persistent locks - it only holds Lifetime.EPHEMERAL locks. 55458: ALF-20031: added repeatable read within transactions to LockStores 55461: ALF-20031: moved transactional test code into separate test classes to avoid clutter. 55470: ALF-20031: added test for LockStore reads when no transaction is available. 55473: Generated LockState.toString() to aid debugging, stack trace clarity etc. 55481: ALF-20031: added tests for null value behaviour (transactional repeatable reads). 55663: ALF-20031: (work in progress, broken tests) removing concurrency locks from AbstractLockStore. 55664: ALF-20031: moved inner Thread classes to anonymous inner classes as there will be more of these coming. 55675: ALF-20031: fixed AbstractLockStoreTxTest 55681: ALF-20031: added more test cases to AbstractLockStoreTxTest 55683: Added missing tests to enterprise.repo.cluster package's BuildSafeTestSuite. 55684: ALF-20031: Fix HazelcastLockStoreTest to clear lock store backing map in createLockStore() fixture setup. 55688: Commented LockStore.clear() as a DO NOT USE method. 55694: ALF-20031: removed LockStore.contains() as this is not required, and was currently unsafe (no repeatable reads). 55696: ALF-20031: Fix AbstractLockStore.clear(): was not clearing transactionally scoped map. 55700: ALF-20031: removed concurrency locks from LockServiceImpl.getLockState(NodeRef) 55712: ALF-20031: removed concurrency lock from LockServiceImpl.lock() 55716: ALF-20031: removed lockstore locking from LockableAspectInterceptor. 55718: ALF-20031: renamed method to isEphemeralLock() 55719: ALF-20031: removed concurrency lock from LockServiceImpl.unlock() 55728: ALF-20031: added cm:lockLifetime property to content model, and added persistence within lockservice. 55732: ALF-20031: LockableAspectInterceptor no longer uses ephemeral lockstore for setProperties() handling. 55753: ALF-20031: upon tx rollback, ephemeral locks are now rolled back to the last known value, rather than always being unlocked. 55759: ALF-20031: temporary fix, AbstractLockStoreTxTest needs further attention for thread coordination. 55760: ALF-20091: disabled spawning of new transactions in LockKeeperImpl.refreshAllLocks() 55767: ALF-20031: changed AbstractLockStore to use getFullyAuthenticatedUser() instead of getRunAsUser(). 55833: ALF-20091: reverted LockKeeperImpl to previous (create new tx) behaviour. Altered test to use separate transactions per lock phase. 55855: ALF-20031: fixed ReplicationServiceIntegrationTest, LockableAspectInterceptor's handling of cm:lockLifetime. git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@55893 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261 --- .../messages/content-model.properties | 2 + config/alfresco/model/contentModel.xml | 4 + .../alfresco/repo/lock/LockServiceImpl.java | 253 +++----- .../org/alfresco/repo/lock/LockUtils.java | 37 ++ .../repo/lock/mem/AbstractLockStore.java | 130 +++- .../org/alfresco/repo/lock/mem/LockState.java | 8 + .../org/alfresco/repo/lock/mem/LockStore.java | 9 +- .../alfresco/repo/lock/mem/LockStoreImpl.java | 6 - .../lock/mem/LockableAspectInterceptor.java | 130 ++-- .../filesys/repo/LockKeeperImplTest.java | 69 ++- .../repo/lock/LockServiceImplTest.java | 40 +- .../lock/mem/AbstractLockStoreTestBase.java | 22 +- .../lock/mem/AbstractLockStoreTxTest.java | 585 ++++++++++++++++++ .../repo/lock/mem/LockStoreImplTxTest.java | 33 + .../mem/LockableAspectInterceptorTest.java | 88 ++- 15 files changed, 1063 insertions(+), 353 deletions(-) create mode 100644 source/test-java/org/alfresco/repo/lock/mem/AbstractLockStoreTxTest.java create mode 100644 source/test-java/org/alfresco/repo/lock/mem/LockStoreImplTxTest.java diff --git a/config/alfresco/messages/content-model.properties b/config/alfresco/messages/content-model.properties index 454cfb9a77..35acc61fe5 100644 --- a/config/alfresco/messages/content-model.properties +++ b/config/alfresco/messages/content-model.properties @@ -246,6 +246,8 @@ cm_contentmodel.property.cm_lockOwner.title=Lock Owner cm_contentmodel.property.cm_lockOwner.description=Lock Owner cm_contentmodel.property.cm_lockType.title=Lock Type cm_contentmodel.property.cm_lockType.description=Lock Type +cm_contentmodel.property.cm_lockLifetime.title=Lock Lifetime +cm_contentmodel.property.cm_lockLifetime.description=Lock Lifetime cm_contentmodel.property.cm_expiryDate.title=Expiry Date cm_contentmodel.property.cm_expiryDate.description=Expiry Date cm_contentmodel.property.cm_lockIsDeep.title=Deep Lock diff --git a/config/alfresco/model/contentModel.xml b/config/alfresco/model/contentModel.xml index 9c921bd702..524bc25f5f 100644 --- a/config/alfresco/model/contentModel.xml +++ b/config/alfresco/model/contentModel.xml @@ -991,6 +991,10 @@ d:text true + + d:text + true + d:date true diff --git a/source/java/org/alfresco/repo/lock/LockServiceImpl.java b/source/java/org/alfresco/repo/lock/LockServiceImpl.java index 5707658c00..19603db044 100644 --- a/source/java/org/alfresco/repo/lock/LockServiceImpl.java +++ b/source/java/org/alfresco/repo/lock/LockServiceImpl.java @@ -68,6 +68,7 @@ import org.alfresco.service.cmr.search.ResultSet; import org.alfresco.service.cmr.search.SearchService; import org.alfresco.service.cmr.security.AuthenticationService; import org.alfresco.service.namespace.QName; +import org.alfresco.util.Pair; import org.alfresco.util.PropertyCheck; import org.springframework.util.Assert; @@ -88,7 +89,7 @@ public class LockServiceImpl implements LockService, /** Key to the nodes ref's to ignore when checking for locks */ private static final String KEY_IGNORE_NODES = "lockService.ignoreNodes"; - private static final Object KEY_LOCKED_NODES = "lockService.lockedNode"; + private static final Object KEY_MODIFIED_NODES = "lockService.lockedNode"; private NodeService nodeService; private TenantService tenantService; @@ -318,7 +319,11 @@ public class LockServiceImpl implements LockService, lockType = LockType.WRITE_LOCK; } - LockStatus currentLockStatus = getLockStatus(nodeRef, userName); + // Get the current lock info and status for the node ref. + Pair statusAndState = getLockStateAndStatus(nodeRef, userName); + LockState currentLockInfo = statusAndState.getFirst(); + LockStatus currentLockStatus = statusAndState.getSecond(); + if (LockStatus.LOCKED.equals(currentLockStatus) == true) { // Error since we are trying to lock a locked node @@ -328,50 +333,43 @@ public class LockServiceImpl implements LockService, LockStatus.LOCK_EXPIRED.equals(currentLockStatus) == true || LockStatus.LOCK_OWNER.equals(currentLockStatus) == true) { - lockStore.acquireConcurrencyLock(nodeRef); - try + final Date expiryDate = makeExpiryDate(timeToExpire); + + // Store the lock in the appropriate place. + if (lifetime == Lifetime.PERSISTENT) { - // Double check lock status now that the lockStore has been locked by NodeRef - if (getLockStatus(nodeRef, userName) == LockStatus.LOCKED) - { - throw new UnableToAquireLockException(nodeRef); - } - - final Date expiryDate = makeExpiryDate(timeToExpire); - - // Only persist the lock if required. - if (lifetime.equals(Lifetime.PERSISTENT)) + lockableAspectInterceptor.disableForThread(); + try { // Add lock aspect if not already present - lockableAspectInterceptor.disableForThread(); - try - { - ensureLockAspect(nodeRef); - persistLockProps(nodeRef, lockType, userName, expiryDate); - } - finally - { - lockableAspectInterceptor.enableForThread(); - } + ensureLockAspect(nodeRef); + persistLockProps(nodeRef, lockType, lifetime, userName, expiryDate); } - - // Always store the lock in memory. - lockStore.set( - nodeRef, - LockState.createLock(nodeRef, lockType, userName, expiryDate, lifetime, additionalInfo)); - - // Record the NodeRef being locked, so that it can be removed on rollback. - TransactionalResourceHelper.getSet(KEY_LOCKED_NODES).add(nodeRef); + finally + { + lockableAspectInterceptor.enableForThread(); + } + } + else if (lifetime == Lifetime.EPHEMERAL) + { + // Store the lock only in memory. + LockState lock = LockState.createLock(nodeRef, lockType, userName, + expiryDate, lifetime, additionalInfo); + lockStore.set(nodeRef, lock); + // Record the NodeRef being locked and its last known lockstate. This allows + // it to be reverted to this state on rollback. + TransactionalResourceHelper.getMap(KEY_MODIFIED_NODES).put(nodeRef, currentLockInfo); AlfrescoTransactionSupport.bindListener(this); } - finally + else { - lockStore.releaseConcurrencyLock(nodeRef); + throw new IllegalStateException(lifetime.getClass().getSimpleName() + + " is not a valid value: " + lifetime.toString()); } } } - private void persistLockProps(NodeRef nodeRef, LockType lockType, String userName, Date expiryDate) + private void persistLockProps(NodeRef nodeRef, LockType lockType, Lifetime lifetime, String userName, Date expiryDate) { addToIgnoreSet(nodeRef); try @@ -379,6 +377,7 @@ public class LockServiceImpl implements LockService, // Set the current user as the lock owner this.nodeService.setProperty(nodeRef, ContentModel.PROP_LOCK_OWNER, userName); 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); } finally @@ -467,48 +466,47 @@ public class LockServiceImpl implements LockService, // Unlock the parent nodeRef = tenantService.getName(nodeRef); - lockStore.acquireConcurrencyLock(nodeRef); - try + LockState lockState = getLockState(nodeRef); + + if (lockState.isLockInfo()) { - LockState lockState = getLockState(nodeRef); - - if (lockState.isLockInfo()) + // MNT-231: forbidden to unlock a checked out node + if (!allowCheckedOut && nodeService.hasAspect(nodeRef, ContentModel.ASPECT_CHECKED_OUT)) { - // MNT-231: forbidden to unlock a checked out node - if (!allowCheckedOut && nodeService.hasAspect(nodeRef, ContentModel.ASPECT_CHECKED_OUT)) - { - throw new UnableToReleaseLockException(nodeRef, CAUSE.CHECKED_OUT); - } + throw new UnableToReleaseLockException(nodeRef, CAUSE.CHECKED_OUT); + } - // Always unlock in memory regardless of lifetime - lockStore.set(nodeRef, LockState.createUnlocked(nodeRef)); - - // Remove the lock from persistent storage. - if (lockState.getLifetime().equals(Lifetime.PERSISTENT)) + // Remove the lock from persistent storage. + Lifetime lifetime = lockState.getLifetime(); + if (lifetime == Lifetime.PERSISTENT) + { + addToIgnoreSet(nodeRef); + behaviourFilter.disableBehaviour(nodeRef, ContentModel.ASPECT_VERSIONABLE); + lockableAspectInterceptor.disableForThread(); + try { - addToIgnoreSet(nodeRef); - behaviourFilter.disableBehaviour(nodeRef, ContentModel.ASPECT_VERSIONABLE); - lockableAspectInterceptor.disableForThread(); - try + // Clear the lock + if (nodeService.hasAspect(nodeRef, ContentModel.ASPECT_LOCKABLE)) { - // Clear the lock - if (nodeService.hasAspect(nodeRef, ContentModel.ASPECT_LOCKABLE)) - { - nodeService.removeAspect(nodeRef, ContentModel.ASPECT_LOCKABLE); - } - } - finally - { - behaviourFilter.enableBehaviour(nodeRef, ContentModel.ASPECT_VERSIONABLE); - lockableAspectInterceptor.enableForThread(); - removeFromIgnoreSet(nodeRef); + nodeService.removeAspect(nodeRef, ContentModel.ASPECT_LOCKABLE); } } + finally + { + behaviourFilter.enableBehaviour(nodeRef, ContentModel.ASPECT_VERSIONABLE); + lockableAspectInterceptor.enableForThread(); + removeFromIgnoreSet(nodeRef); + } + } + else if (lifetime == Lifetime.EPHEMERAL) + { + // Remove the ephemeral lock. + lockStore.set(nodeRef, LockState.createUnlocked(nodeRef)); + } + else + { + throw new IllegalStateException("Unhandled Lifetime value: " + lifetime); } - } - finally - { - lockStore.releaseConcurrencyLock(nodeRef); } if (unlockChildren) @@ -552,50 +550,22 @@ public class LockServiceImpl implements LockService, * @return the lock status */ public LockStatus getLockStatus(NodeRef nodeRef, String userName) + { + Pair stateAndStatus = getLockStateAndStatus(nodeRef, userName); + LockStatus lockStatus = stateAndStatus.getSecond(); + return lockStatus; + } + + private Pair getLockStateAndStatus(NodeRef nodeRef, String userName) { final LockState lockState = getLockState(nodeRef); String lockOwner = lockState.getOwner(); Date expiryDate = lockState.getExpires(); - LockStatus status = lockStatus(userName, lockOwner, expiryDate); - return status; + LockStatus status = LockUtils.lockStatus(userName, lockOwner, expiryDate); + return new Pair(lockState, status); } - /** - * Given the lock owner and expiry date of a lock calculates the lock status with respect - * to the user name supplied, e.g. the current user. - * - * @param userName User name to evaluate the lock against. - * @param lockOwner Owner of the lock. - * @param expiryDate Expiry date of the lock. - * @return LockStatus - */ - private LockStatus lockStatus(String userName, String lockOwner, Date expiryDate) - { - LockStatus result = LockStatus.NO_LOCK; - - if (lockOwner != null) - { - if (expiryDate != null && expiryDate.before(new Date()) == true) - { - // Indicate that the lock has expired - result = LockStatus.LOCK_EXPIRED; - } - else - { - if (lockOwner.equals(userName) == true) - { - result = LockStatus.LOCK_OWNER; - } - else - { - result = LockStatus.LOCKED; - } - } - } - return result; - } - /** * @see LockService#getLockType(NodeRef) */ @@ -850,52 +820,35 @@ public class LockServiceImpl implements LockService, @Override public LockState getLockState(NodeRef nodeRef) { + // Check in-memory for ephemeral locks first. LockState lockState = lockStore.get(nodeRef); + if (lockState == null) { - lockStore.acquireConcurrencyLock(nodeRef); - try + // No in-memory state, so get from the DB. + if (nodeService.hasAspect(nodeRef, ContentModel.ASPECT_LOCKABLE)) { - // Double check there is still no lock - if (lockStore.contains(nodeRef)) - { - // In-memory lock state has appeared since last check, so use it. - lockState = lockStore.get(nodeRef); - } - else - { - // Still no in-memory state, so get from the DB and cache it also. - lockableAspectInterceptor.disableForThread(); - if (nodeService.hasAspect(nodeRef, ContentModel.ASPECT_LOCKABLE)) - { - String lockOwner = (String) nodeService.getProperty(nodeRef, ContentModel.PROP_LOCK_OWNER); - - Date expiryDate = (Date) nodeService.getProperty(nodeRef, ContentModel.PROP_EXPIRY_DATE); - String lockTypeStr = (String) nodeService.getProperty(nodeRef, ContentModel.PROP_LOCK_TYPE); - LockType lockType = lockTypeStr != null ? LockType.valueOf(lockTypeStr) : null; - - // Add to memory store, we mark it as PERSISTENT as it was in the persistent storage! - lockState = LockState.createLock( - nodeRef, - lockType, - lockOwner, - expiryDate, - Lifetime.PERSISTENT, - null); - } - else - { - // There is no lock information - lockState = LockState.createUnlocked(nodeRef); - } - // Cache the lock state - lockStore.set(nodeRef, lockState); - } + String lockOwner = (String) nodeService.getProperty(nodeRef, ContentModel.PROP_LOCK_OWNER); + + Date expiryDate = (Date) nodeService.getProperty(nodeRef, ContentModel.PROP_EXPIRY_DATE); + String lockTypeStr = (String) nodeService.getProperty(nodeRef, ContentModel.PROP_LOCK_TYPE); + 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; + + // Mark lockstate as PERSISTENT as it was in the persistent storage! + lockState = LockState.createLock( + nodeRef, + lockType, + lockOwner, + expiryDate, + lifetime, + null); } - finally + else { - lockableAspectInterceptor.enableForThread(); - lockStore.releaseConcurrencyLock(nodeRef); + // There is no lock information + lockState = LockState.createUnlocked(nodeRef); } } @@ -937,11 +890,11 @@ public class LockServiceImpl implements LockService, @Override public void afterRollback() { - // As rollback has occurred we are unable to keep hold of any locks set during this transaction. - Set lockedNodes = TransactionalResourceHelper.getSet(KEY_LOCKED_NODES); - for (NodeRef nodeRef : lockedNodes) + // As rollback has occurred we are unable to keep hold of any ephemeral locks set during this transaction. + Map lockedNodes = TransactionalResourceHelper.getMap(KEY_MODIFIED_NODES); + for (LockState lockInfo : lockedNodes.values()) { - lockStore.set(nodeRef, LockState.createUnlocked(nodeRef)); + lockStore.set(lockInfo.getNodeRef(), lockInfo); } } } diff --git a/source/java/org/alfresco/repo/lock/LockUtils.java b/source/java/org/alfresco/repo/lock/LockUtils.java index d8408616ae..3b0a976a6c 100644 --- a/source/java/org/alfresco/repo/lock/LockUtils.java +++ b/source/java/org/alfresco/repo/lock/LockUtils.java @@ -18,6 +18,8 @@ */ package org.alfresco.repo.lock; +import java.util.Date; + import org.alfresco.service.cmr.lock.LockService; import org.alfresco.service.cmr.lock.LockStatus; import org.alfresco.service.cmr.lock.LockType; @@ -67,4 +69,39 @@ public class LockUtils return true; } } + + /** + * Given the lock owner and expiry date of a lock calculates the lock status with respect + * to the user name supplied, e.g. the current user. + * + * @param userName User name to evaluate the lock against. + * @param lockOwner Owner of the lock. + * @param expiryDate Expiry date of the lock. + * @return LockStatus + */ + public static LockStatus lockStatus(String userName, String lockOwner, Date expiryDate) + { + LockStatus result = LockStatus.NO_LOCK; + + if (lockOwner != null) + { + if (expiryDate != null && expiryDate.before(new Date()) == true) + { + // Indicate that the lock has expired + result = LockStatus.LOCK_EXPIRED; + } + else + { + if (lockOwner.equals(userName) == true) + { + result = LockStatus.LOCK_OWNER; + } + else + { + result = LockStatus.LOCKED; + } + } + } + return result; + } } diff --git a/source/java/org/alfresco/repo/lock/mem/AbstractLockStore.java b/source/java/org/alfresco/repo/lock/mem/AbstractLockStore.java index d4dacf2b18..119c2a8442 100644 --- a/source/java/org/alfresco/repo/lock/mem/AbstractLockStore.java +++ b/source/java/org/alfresco/repo/lock/mem/AbstractLockStore.java @@ -18,10 +18,19 @@ */ package org.alfresco.repo.lock.mem; +import java.util.Date; +import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentMap; +import org.alfresco.repo.lock.LockUtils; +import org.alfresco.repo.security.authentication.AuthenticationUtil; +import org.alfresco.repo.transaction.TransactionalResourceHelper; +import org.alfresco.service.cmr.lock.LockStatus; +import org.alfresco.service.cmr.lock.UnableToAquireLockException; import org.alfresco.service.cmr.repository.NodeRef; +import org.springframework.dao.ConcurrencyFailureException; +import org.springframework.transaction.support.TransactionSynchronizationManager; /** * Base class for LockStore implementations that use a ConcurrentMap as storage. @@ -55,54 +64,101 @@ public abstract class AbstractLockStore txMap = getTxMap(); + if (txMap != null && txMap.containsKey(nodeRef)) { - return map.get(nodeRef); + // The transactional map is able to provide the LockState + lockState = txMap.get(nodeRef); } - finally + else { - releaseConcurrencyLock(nodeRef); - } - } - - @Override - public boolean contains(NodeRef nodeRef) - { - acquireConcurrencyLock(nodeRef); - try - { - return map.containsKey(nodeRef); - } - finally - { - releaseConcurrencyLock(nodeRef); + lockState = map.get(nodeRef); + if (txMap != null) + { + // As the txMap doesn't have the LockState, cache it for later. + txMap.put(nodeRef, lockState); + } } + return lockState; } @Override public void set(NodeRef nodeRef, LockState lockState) { - acquireConcurrencyLock(nodeRef); - try + Map txMap = getTxMap(); + LockState previousLockState = null; + if (txMap != null) { - doSet(nodeRef, lockState); + if (txMap.containsKey(nodeRef)) + { + // There is known previous state. + previousLockState = txMap.get(nodeRef); + } + else + { + // No previous known state - get the current state, this becomes + // the previous known state. + previousLockState = get(nodeRef); + } } - finally + else { - releaseConcurrencyLock(nodeRef); + // No transaction, but we still need to know the previous state, before attempting + // to set new state. + previousLockState = get(nodeRef); + } + + // Has the lock been succesfully placed into the lock store? + boolean updated = false; + + if (previousLockState != null) + { + String userName = AuthenticationUtil.getFullyAuthenticatedUser(); + String owner = previousLockState.getOwner(); + Date expires = previousLockState.getExpires(); + if (LockUtils.lockStatus(userName, owner, expires) == LockStatus.LOCKED) + { + throw new UnableToAquireLockException(nodeRef); + } + // Use ConcurrentMap.replace(key, old, new) so that we can ensure we don't encounter a + // 'lost update' (i.e. someone else has locked a node while we were thinking about it). + updated = map.replace(nodeRef, previousLockState, lockState); + } + else + { + if (map.putIfAbsent(nodeRef, lockState) == null) + { + updated = true; + } + } + + if (!updated) + { + String msg = String.format("Attempt to update lock state failed, old=%s, new=%s, noderef=%s", + previousLockState, lockState, nodeRef); + throw new ConcurrencyFailureException(msg); + } + else + { + // Keep the new value for future reads within this TX. + if (txMap != null) + { + txMap.put(nodeRef, lockState); + } } } - protected abstract void doSet(NodeRef nodeRef, LockState lockState); @Override public void clear() { - // TODO: lock whole map? map.clear(); + Map txMap = getTxMap(); + if (txMap != null) + { + txMap.clear(); + } } @Override @@ -111,10 +167,26 @@ public abstract class AbstractLockStore getTxMap() + { + if (!TransactionSynchronizationManager.isSynchronizationActive()) + { + return null; + } + Map map = TransactionalResourceHelper.getMap(getClass().getName()+".repeatableReadMap"); + return map; + } + @Override public Set getNodes() { - // TODO: lock whole map? return map.keySet(); } } diff --git a/source/java/org/alfresco/repo/lock/mem/LockState.java b/source/java/org/alfresco/repo/lock/mem/LockState.java index 0ee5354ce9..c7ee3a81ba 100644 --- a/source/java/org/alfresco/repo/lock/mem/LockState.java +++ b/source/java/org/alfresco/repo/lock/mem/LockState.java @@ -215,4 +215,12 @@ public final class LockState implements Serializable else if (!this.owner.equals(other.owner)) return false; return true; } + + @Override + public String toString() + { + return "LockState [nodeRef=" + this.nodeRef + ", lockType=" + this.lockType + ", owner=" + + this.owner + ", expires=" + this.expires + ", lifetime=" + this.lifetime + + ", additionalInfo=" + this.additionalInfo + "]"; + } } diff --git a/source/java/org/alfresco/repo/lock/mem/LockStore.java b/source/java/org/alfresco/repo/lock/mem/LockStore.java index f20fdd3912..44718c6948 100644 --- a/source/java/org/alfresco/repo/lock/mem/LockStore.java +++ b/source/java/org/alfresco/repo/lock/mem/LockStore.java @@ -51,11 +51,16 @@ import org.alfresco.service.cmr.repository.NodeRef; public interface LockStore { LockState get(NodeRef nodeRef); - boolean contains(NodeRef nodeRef); void set(NodeRef nodeRef, LockState lockState); - void clear(); void acquireConcurrencyLock(NodeRef nodeRef); void releaseConcurrencyLock(NodeRef nodeRef); void setMaxTryLockMillis(long maxTryLockMillis); public Set getNodes(); + + /** + * WARNING: only use in test code - unsafe method for production use. + * + * TODO: remove this method? + */ + void clear(); } diff --git a/source/java/org/alfresco/repo/lock/mem/LockStoreImpl.java b/source/java/org/alfresco/repo/lock/mem/LockStoreImpl.java index 938eaed78e..3026ab191d 100644 --- a/source/java/org/alfresco/repo/lock/mem/LockStoreImpl.java +++ b/source/java/org/alfresco/repo/lock/mem/LockStoreImpl.java @@ -106,10 +106,4 @@ public class LockStoreImpl extends AbstractLockStore aspects = (Set) invocation.proceed(); - if (hasEphemeralLock(nodeRef) && !aspects.contains(ContentModel.ASPECT_LOCKABLE)) + LockState lockState = lockStore.get(nodeRef); + if (isEphemeralLock(lockState) && !aspects.contains(ContentModel.ASPECT_LOCKABLE)) { aspects.add(ContentModel.ASPECT_LOCKABLE); } @@ -106,24 +108,26 @@ public class LockableAspectInterceptor implements MethodInterceptor else if (methodName.equals("getProperties")) { NodeRef nodeRef = (NodeRef) args[0]; - lockStore.acquireConcurrencyLock(nodeRef); - try + + Map properties = (Map) invocation.proceed(); + LockState lockState = lockStore.get(nodeRef); + if (isEphemeralLock(lockState)) { - Map properties = (Map) invocation.proceed(); - if (hasEphemeralLock(nodeRef)) + String userName = lockState.getOwner(); + properties.put(ContentModel.PROP_LOCK_OWNER, userName); + properties.put(ContentModel.PROP_LOCK_TYPE, lockState.getLockType().toString()); + properties.put(ContentModel.PROP_EXPIRY_DATE, lockState.getExpires()); + properties.put(ContentModel.PROP_LOCK_LIFETIME, Lifetime.EPHEMERAL); + } + else if (nodeService.hasAspect(nodeRef, ContentModel.ASPECT_LOCKABLE)) + { + // Persistent lock, ensure lifetime property is present. + if (!properties.containsKey(ContentModel.PROP_LOCK_LIFETIME)) { - LockState lockState = lockStore.get(nodeRef); - String userName = lockState.getOwner(); - properties.put(ContentModel.PROP_LOCK_OWNER, userName); - properties.put(ContentModel.PROP_LOCK_TYPE, lockState.getLockType().toString()); - properties.put(ContentModel.PROP_EXPIRY_DATE, lockState.getExpires()); + properties.put(ContentModel.PROP_LOCK_LIFETIME, Lifetime.PERSISTENT); } - return properties; - } - finally - { - lockStore.releaseConcurrencyLock(nodeRef); } + return properties; } else if (methodName.equals("getProperty")) { @@ -133,60 +137,61 @@ public class LockableAspectInterceptor implements MethodInterceptor // Avoid locking unless it is an interesting property. if (isLockProperty(propQName)) { - lockStore.acquireConcurrencyLock(nodeRef); - try + LockState lockState = lockStore.get(nodeRef); + if (isEphemeralLock(lockState)) { - if (hasEphemeralLock(nodeRef)) + if (ContentModel.PROP_LOCK_OWNER.equals(propQName)) { - LockState lockState = lockStore.get(nodeRef); - if (ContentModel.PROP_LOCK_OWNER.equals(propQName)) - { - return lockState.getOwner(); - } - else if (ContentModel.PROP_LOCK_TYPE.equals(propQName)) - { - return lockState.getLockType().toString(); - } - else if (ContentModel.PROP_EXPIRY_DATE.equals(propQName)) - { - return lockState.getExpires(); - } + return lockState.getOwner(); + } + else if (ContentModel.PROP_LOCK_TYPE.equals(propQName)) + { + return lockState.getLockType().toString(); + } + else if (ContentModel.PROP_EXPIRY_DATE.equals(propQName)) + { + return lockState.getExpires(); + } + else if (ContentModel.PROP_LOCK_LIFETIME.equals(propQName)) + { + return lockState.getLifetime().toString(); } } - finally + else if (ContentModel.PROP_LOCK_LIFETIME.equals(propQName)) { - lockStore.releaseConcurrencyLock(nodeRef); + // Is there a persistent lock? + if (nodeService.hasAspect(nodeRef, ContentModel.ASPECT_LOCKABLE)) + { + return Lifetime.PERSISTENT.toString(); + } } - } + } return invocation.proceed(); } else if (methodName.equals("setProperties")) { - // If a client has retrieved the node's properties using getProperties and is saving them - // back using setProperties then it is important that lock properties (e.g. cm:lockType, cm:lockOwner) - // are not persisted if there is an ephemeral lock present - otherwise the ephemeral lock will - // be effectively converted into a peristent lock. + // Ephemeral locks must not be persisted to the database. + // TODO: This is potentially creating an ephemeral lock here, put it in the lockstore? NodeRef nodeRef = (NodeRef) args[0]; Map newProperties = (Map) args[1]; - lockStore.acquireConcurrencyLock(nodeRef); - try + if (newProperties.get(ContentModel.PROP_LOCK_LIFETIME) == Lifetime.EPHEMERAL) { - if (hasEphemeralLock(nodeRef) && containsLockProperty(newProperties)) - { - Map convertedProperties = filterLockProperties(newProperties); - // Now complete the call by passing the converted properties - nodeService.setProperties(nodeRef, convertedProperties); - return null; - } - else - { - return invocation.proceed(); - } + Map convertedProperties = filterLockProperties(newProperties); + // Now complete the call by passing the converted properties + nodeService.setProperties(nodeRef, convertedProperties); + return null; } - finally + else if (newProperties.containsKey(ContentModel.PROP_LOCK_LIFETIME)) { - lockStore.releaseConcurrencyLock(nodeRef); + // Always remove this property, even for persistent locks. + newProperties.remove(ContentModel.PROP_LOCK_LIFETIME); + nodeService.setProperties(nodeRef, newProperties); + return null; + } + else + { + return invocation.proceed(); } } else @@ -235,19 +240,6 @@ public class LockableAspectInterceptor implements MethodInterceptor return filteredProps; } - /** - * Does the collection contain a lock related property? - */ - private boolean containsLockProperty(Map properties) - { - boolean containsLockProperty = ( - properties.containsKey(ContentModel.PROP_LOCK_OWNER) || - properties.containsKey(ContentModel.PROP_LOCK_TYPE) || - properties.containsKey(ContentModel.PROP_EXPIRY_DATE) - ); - return containsLockProperty; - } - /** * Return true if the specified property QName is for a lock-related property. */ @@ -256,13 +248,13 @@ public class LockableAspectInterceptor implements MethodInterceptor boolean isLockProp = propQName.equals(ContentModel.PROP_LOCK_OWNER) || propQName.equals(ContentModel.PROP_LOCK_TYPE) || + propQName.equals(ContentModel.PROP_LOCK_LIFETIME) || propQName.equals(ContentModel.PROP_EXPIRY_DATE); return isLockProp; } - private boolean hasEphemeralLock(NodeRef nodeRef) + private boolean isEphemeralLock(LockState lockState) { - LockState lockState = lockStore.get(nodeRef); boolean ephemeral = lockState != null && lockState.isLockInfo() && lockState.getLifetime() == Lifetime.EPHEMERAL; diff --git a/source/test-java/org/alfresco/filesys/repo/LockKeeperImplTest.java b/source/test-java/org/alfresco/filesys/repo/LockKeeperImplTest.java index 614ba574fe..7975472d06 100644 --- a/source/test-java/org/alfresco/filesys/repo/LockKeeperImplTest.java +++ b/source/test-java/org/alfresco/filesys/repo/LockKeeperImplTest.java @@ -80,23 +80,20 @@ public class LockKeeperImplTest extends TestCase } /* - * Tests a basic sequence of lock, refresh, remove, refresh in a single transaction + * Tests a basic sequence of lock, refresh, remove, refresh in separate transactions */ public void testBasicLockUnlock() throws Exception { logger.debug("testBasicLockUnlock"); final RetryingTransactionHelper tran = transactionService.getRetryingTransactionHelper(); + final String FILE_NAME = "LockKeeperImplTestNode"; - RetryingTransactionCallback testITCB = new RetryingTransactionCallback() { - + RetryingTransactionCallback lockCB = new RetryingTransactionCallback() { @Override - public Void execute() throws Throwable - { - String FILE_NAME = "LockKeeperImplTestNode"; - - NodeRef companyHome = repositoryHelper.getCompanyHome(); - + public Boolean execute() throws Throwable + { + NodeRef companyHome = repositoryHelper.getCompanyHome(); NodeRef nodeRef = nodeService.getChildByName(companyHome, ContentModel.ASSOC_CONTAINS, FILE_NAME); if(nodeRef == null) @@ -108,24 +105,48 @@ public class LockKeeperImplTest extends TestCase logger.debug("first lock"); lockKeeper.addLock(nodeRef); - assertTrue("node not locked", nodeService.hasAspect(nodeRef, ContentModel.ASPECT_LOCKABLE)); - - logger.debug("first refresh"); - lockKeeper.refreshAllLocks(); - - lockKeeper.removeLock(nodeRef); - - assertFalse("node not unlocked", nodeService.hasAspect(nodeRef, ContentModel.ASPECT_LOCKABLE)); - - logger.debug("second refresh"); - lockKeeper.refreshAllLocks(); - + boolean locked = nodeService.hasAspect(nodeRef, ContentModel.ASPECT_LOCKABLE); + return locked; + } + }; + boolean locked = tran.doInTransaction(lockCB); + assertTrue("node not locked", locked); + + RetryingTransactionCallback refreshCB = new RetryingTransactionCallback() { + @Override + public Void execute() throws Throwable + { + logger.debug("first refresh"); + lockKeeper.refreshAllLocks(); return null; } }; - tran.doInTransaction(testITCB); - - + tran.doInTransaction(refreshCB); + + RetryingTransactionCallback removeCB = new RetryingTransactionCallback() { + @Override + public Boolean execute() throws Throwable + { + NodeRef companyHome = repositoryHelper.getCompanyHome(); + NodeRef nodeRef = nodeService.getChildByName(companyHome, ContentModel.ASSOC_CONTAINS, FILE_NAME); + lockKeeper.removeLock(nodeRef); + boolean locked = nodeService.hasAspect(nodeRef, ContentModel.ASPECT_LOCKABLE); + return locked; + } + }; + locked = tran.doInTransaction(removeCB); + assertFalse("node not unlocked", locked); + + RetryingTransactionCallback refreshAgainCB = new RetryingTransactionCallback() { + @Override + public Void execute() throws Throwable + { + logger.debug("second refresh"); + lockKeeper.refreshAllLocks(); + return null; + } + }; + tran.doInTransaction(refreshAgainCB); } /* diff --git a/source/test-java/org/alfresco/repo/lock/LockServiceImplTest.java b/source/test-java/org/alfresco/repo/lock/LockServiceImplTest.java index 1f81cf69ee..c515e6fcbb 100644 --- a/source/test-java/org/alfresco/repo/lock/LockServiceImplTest.java +++ b/source/test-java/org/alfresco/repo/lock/LockServiceImplTest.java @@ -21,6 +21,7 @@ package org.alfresco.repo.lock; import java.io.Serializable; import java.util.HashMap; import java.util.List; +import java.util.Map; import javax.transaction.NotSupportedException; import javax.transaction.SystemException; @@ -198,6 +199,13 @@ public class LockServiceImplTest extends BaseSpringTest assertEquals(null, lockState.getExpires()); assertEquals(null, lockState.getAdditionalInfo()); + // Check the correct properties have been set + Map props = nodeService.getProperties(parentNode); + assertEquals(GOOD_USER_NAME, props.get(ContentModel.PROP_LOCK_OWNER)); + assertEquals(LockType.WRITE_LOCK.toString(), props.get(ContentModel.PROP_LOCK_TYPE)); + assertEquals(Lifetime.PERSISTENT.toString(), props.get(ContentModel.PROP_LOCK_LIFETIME)); + assertEquals(null, props.get(ContentModel.PROP_EXPIRY_DATE)); + TestWithUserUtils.authenticateUser(BAD_USER_NAME, PWD, rootNodeRef, this.authenticationService); assertEquals( @@ -329,21 +337,29 @@ public class LockServiceImplTest extends BaseSpringTest assertEquals(LockStatus.NO_LOCK, lockService.getLockStatus(noAspectNode)); } - public void testLockReleasedOnRollback() throws NotSupportedException, SystemException + public void testLockRevertedOnRollback() throws NotSupportedException, SystemException { // Preconditions of test assertEquals(LockStatus.NO_LOCK, lockService.getLockStatus(noAspectNode)); assertEquals(LockStatus.NO_LOCK, lockService.getLockStatus(rootNodeRef)); - lockService.lock(noAspectNode, LockType.WRITE_LOCK); - lockService.lock(rootNodeRef, LockType.NODE_LOCK); + // Lock noAspectNode + lockService.lock(noAspectNode, LockType.WRITE_LOCK, 0, Lifetime.EPHEMERAL); + + // Lock rootNodeRef + lockService.lock(rootNodeRef, LockType.NODE_LOCK, 0, Lifetime.EPHEMERAL); + + // Sometime later, a refresh occurs (so this should not be reverted to unlocked, but to this state) + lockService.lock(rootNodeRef, LockType.NODE_LOCK, 3600, Lifetime.EPHEMERAL); // Rollback endTransaction(); - // The locks should not present. + // This lock should not be present. assertEquals(LockStatus.NO_LOCK, lockService.getLockStatus(noAspectNode)); - assertEquals(LockStatus.NO_LOCK, lockService.getLockStatus(rootNodeRef)); + + // This lock should still be present. + assertEquals(LockStatus.LOCK_OWNER, lockService.getLockStatus(rootNodeRef)); } /** @@ -445,22 +461,18 @@ public class LockServiceImplTest extends BaseSpringTest LockStatus lockStatus2 = this.lockService.getLockStatus(this.parentNode); assertEquals(LockStatus.LOCKED, lockStatus2); - // Check lockstore is populated during status read + // Check lockstore is not used for persistent locks + // Previously LockStore was doubling up as a cache - the change in requirements means a test + // is necessary to ensure the work has been implemented correctly (despite being an odd test) LockStore lockStore = (LockStore) applicationContext.getBean("lockStore"); lockStore.clear(); LockState lockState = lockStore.get(parentNode); // Nothing stored against node ref assertNull(lockState); lockService.getLockStatus(parentNode); - // In-memory store populated during getLockStatus + // In-memory store still empty - only used for ephemeral locks lockState = lockStore.get(parentNode); - // Check details are correct - assertEquals(Lifetime.PERSISTENT, lockState.getLifetime()); - assertEquals(null, lockState.getExpires()); - assertEquals(GOOD_USER_NAME, lockState.getOwner()); - assertEquals(LockType.WRITE_LOCK, lockState.getLockType()); - assertEquals(parentNode, lockState.getNodeRef()); - assertEquals(null, lockState.getAdditionalInfo()); + assertNull(lockState); TestWithUserUtils.authenticateUser(GOOD_USER_NAME, PWD, rootNodeRef, this.authenticationService); diff --git a/source/test-java/org/alfresco/repo/lock/mem/AbstractLockStoreTestBase.java b/source/test-java/org/alfresco/repo/lock/mem/AbstractLockStoreTestBase.java index 4ad6343c53..b71d5e6ef4 100644 --- a/source/test-java/org/alfresco/repo/lock/mem/AbstractLockStoreTestBase.java +++ b/source/test-java/org/alfresco/repo/lock/mem/AbstractLockStoreTestBase.java @@ -18,10 +18,7 @@ */ package org.alfresco.repo.lock.mem; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; +import static org.junit.Assert.*; import java.util.Comparator; import java.util.Iterator; @@ -41,7 +38,7 @@ import org.junit.Test; * @author Matt Ward */ public abstract class AbstractLockStoreTestBase -{ +{ /** * Instance of the Class Under Test. */ @@ -54,7 +51,6 @@ public abstract class AbstractLockStoreTestBase */ protected abstract T createLockStore(); - @Before public void setUpLockStore() { @@ -96,9 +92,9 @@ public abstract class AbstractLockStoreTestBase lockStore.set(nodeRef1, lock1); lockStore.set(nodeRef2, lock2); - assertTrue(lockStore.contains(nodeRef1)); - assertTrue(lockStore.contains(nodeRef2)); - assertFalse(lockStore.contains(nodeRef3)); + assertNotNull(lockStore.get(nodeRef1)); + assertNotNull(lockStore.get(nodeRef2)); + assertNull(lockStore.get(nodeRef3)); } @Test @@ -113,13 +109,13 @@ public abstract class AbstractLockStoreTestBase lockStore.set(nodeRef1, lock1); lockStore.set(nodeRef2, lock2); - assertTrue(lockStore.contains(nodeRef1)); - assertTrue(lockStore.contains(nodeRef2)); + assertNotNull(lockStore.get(nodeRef1)); + assertNotNull(lockStore.get(nodeRef2)); lockStore.clear(); - assertFalse(lockStore.contains(nodeRef1)); - assertFalse(lockStore.contains(nodeRef2)); + assertNull(lockStore.get(nodeRef1)); + assertNull(lockStore.get(nodeRef2)); } @Test diff --git a/source/test-java/org/alfresco/repo/lock/mem/AbstractLockStoreTxTest.java b/source/test-java/org/alfresco/repo/lock/mem/AbstractLockStoreTxTest.java new file mode 100644 index 0000000000..b0994c6ef8 --- /dev/null +++ b/source/test-java/org/alfresco/repo/lock/mem/AbstractLockStoreTxTest.java @@ -0,0 +1,585 @@ +/* + * Copyright (C) 2005-2013 Alfresco Software Limited. + * + * This file is part of Alfresco + * + * Alfresco is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Alfresco is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with Alfresco. If not, see . + */ +package org.alfresco.repo.lock.mem; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.fail; + +import java.util.Date; + +import javax.transaction.NotSupportedException; +import javax.transaction.SystemException; +import javax.transaction.UserTransaction; + +import org.alfresco.repo.security.authentication.AuthenticationUtil; +import org.alfresco.service.cmr.lock.LockType; +import org.alfresco.service.cmr.lock.UnableToAquireLockException; +import org.alfresco.service.cmr.repository.NodeRef; +import org.alfresco.service.transaction.TransactionService; +import org.alfresco.util.ApplicationContextHelper; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Test; +import org.springframework.context.ApplicationContext; +import org.springframework.dao.ConcurrencyFailureException; + +/** + * Integration tests that check transaction related functionality of {@link LockStore} implementations. + * @author Matt Ward + */ +public abstract class AbstractLockStoreTxTest +{ + /** + * Instance of the Class Under Test. + */ + protected T lockStore; + + protected static ApplicationContext ctx; + + /** + * Concrete subclasses must implement this method to provide the tests with a LockStore instance. + * + * @return LockStore to test + */ + protected abstract T createLockStore(); + + @BeforeClass + public static void setUpSpringContext() + { + ctx = ApplicationContextHelper.getApplicationContext(); + } + + @Before + public void setUpLockStore() + { + lockStore = createLockStore(); + } + + @Test + public void testRepeatableReadsInTransaction() throws NotSupportedException, SystemException + { + final TransactionService txService = (TransactionService) ctx.getBean("TransactionService"); + UserTransaction txA = txService.getUserTransaction(); + + final NodeRef nodeRef = new NodeRef("workspace://SpacesStore/UUID-1"); + final NodeRef nodeRef2 = new NodeRef("workspace://SpacesStore/UUID-2"); + Date now = new Date(); + Date expires = new Date(now.getTime() + 180000); + final LockState lockState1 = LockState.createLock(nodeRef, LockType.WRITE_LOCK, + "jbloggs", expires, Lifetime.EPHEMERAL, null); + + + Thread txB = new Thread("TxB") + { + @Override + public void run() + { + Object main = AbstractLockStoreTxTest.this; + UserTransaction tx = txService.getUserTransaction(); + try + { + tx.begin(); + try + { + // txB read lock state + LockState lockState = lockStore.get(nodeRef); + assertEquals("jbloggs", lockState.getOwner()); + assertEquals(Lifetime.EPHEMERAL, lockState.getLifetime()); + + // Wait, while txA changes the lock state + passControl(this, main); + + // assert txB still sees state A + lockState = lockStore.get(nodeRef); + assertEquals("jbloggs", lockState.getOwner()); + + // Wait, while txA checks whether it can see lock for nodeRef2 (though it doesn't exist yet) + passControl(this, main); + + // txB sets a value, already seen as non-existent lock by txA + lockStore.set(nodeRef2, LockState.createLock(nodeRef2, LockType.WRITE_LOCK, + "csmith", null, Lifetime.EPHEMERAL, null)); + } + finally + { + tx.rollback(); + } + } + catch (Throwable e) + { + throw new RuntimeException("Error in transaction B", e); + } + finally + { + // Stop 'main' from waiting + synchronized(main) + { + main.notifyAll(); + } + } + } + }; + + txA.begin(); + try + { + // txA set lock state 1 + lockStore.set(nodeRef, lockState1); + + // Wait while txB reads and checks the LockState + txB.setDaemon(true); + txB.start(); + passControl(this, txB); + + // txA set different lock state + AuthenticationUtil.setFullyAuthenticatedUser("jbloggs"); // Current lock owner needed to change lock. + final LockState lockState2 = LockState.createWithOwner(lockState1, "another"); + lockStore.set(nodeRef, lockState2); + + // Wait while txB reads/checks the LockState again for nodeRef + passControl(this, txB); + + // Another update + AuthenticationUtil.setFullyAuthenticatedUser("another"); // Current lock owner needed to change lock. + final LockState lockState3 = LockState.createWithOwner(lockState2, "bsmith"); + lockStore.set(nodeRef, lockState3); + // Check we can see the update. + assertEquals("bsmith", lockStore.get(nodeRef).getOwner()); + + // Perform a read, that we know will retrieve a null value + assertNull("nodeRef2 LockState", lockStore.get(nodeRef2)); + + // Wait while txB populates the store with a value for nodeRef2 + passControl(this, txB); + + // Perform the read again - update should not be visible in this transaction + assertNull("nodeRef2 LockState", lockStore.get(nodeRef2)); + } + finally + { + txA.rollback(); + } + } + + protected void passControl(Object from, Object to) + { + synchronized(to) + { + to.notifyAll(); + } + synchronized(from) + { + try + { + // TODO: wait should be called in a loop with repeated wait condition check, + // but what's the condition we're waiting on? + from.wait(10000); + } + catch (InterruptedException error) + { + throw new RuntimeException(error); + } + } + } + + @Test + public void testReadsWhenNoTransaction() throws NotSupportedException, SystemException + { + final NodeRef nodeRef = new NodeRef("workspace://SpacesStore/UUID-1"); + final NodeRef nodeRef2 = new NodeRef("workspace://SpacesStore/UUID-2"); + Date now = new Date(); + Date expires = new Date(now.getTime() + 180000); + final LockState lockState1 = LockState.createLock(nodeRef, LockType.WRITE_LOCK, + "jbloggs", expires, Lifetime.EPHEMERAL, null); + + + Thread thread2 = new Thread("Thread2") + { + @Override + public void run() + { + Object main = AbstractLockStoreTxTest.this; + try + { + // Thread2 read lock state + LockState lockState = lockStore.get(nodeRef); + assertEquals("jbloggs", lockState.getOwner()); + assertEquals(Lifetime.EPHEMERAL, lockState.getLifetime()); + + // Wait, while Thread1 changes the lock state + passControl(this, main); + + // assert Thread2 sees the updated state + lockState = lockStore.get(nodeRef); + assertEquals("another", lockState.getOwner()); + + // Thread2 sets lock state B + AuthenticationUtil.setFullyAuthenticatedUser("another"); // Current lock owner + lockStore.set(nodeRef, LockState.createUnlocked(nodeRef)); + + // Check it is still visible for this thread + lockState = lockStore.get(nodeRef); + assertFalse(lockState.isLockInfo()); + assertNull(lockState.getOwner()); + + // Wait, while Thread1 checks initial LockState value for nodeRef2 (null) + passControl(this, main); + + // Thread2 sets a value, already seen as null by Thread1 - the update will be seen by Thread1 + lockStore.set(nodeRef2, LockState.createLock(nodeRef2, LockType.WRITE_LOCK, + "not-null-lockstate", null, Lifetime.EPHEMERAL, null)); + } + finally + { + // Stop 'main' from waiting + synchronized(main) + { + main.notifyAll(); + } + } + } + }; + + + // Thread1 set lock state 1 + lockStore.set(nodeRef, lockState1); + + // Wait while Thread2 reads and checks the LockState + thread2.setDaemon(true); + thread2.start(); + passControl(this, thread2); + + // Thread1 sets different lock state + AuthenticationUtil.setFullyAuthenticatedUser("jbloggs"); // Current lock owner needed to change lock + final LockState lockState2 = LockState.createWithOwner(lockState1, "another"); + lockStore.set(nodeRef, lockState2); + + // Wait while Thread2 reads the LockState again for nodeRef + passControl(this, thread2); + + // Thread2 has unlocked the node, we should see the result + assertFalse("Node still locked, but shouldn't be", lockStore.get(nodeRef).isLockInfo()); + assertNull(lockStore.get(nodeRef).getOwner()); + assertNull(lockStore.get(nodeRef).getExpires()); + + // Another update + AuthenticationUtil.setFullyAuthenticatedUser("jbloggs"); // Current lock owner + final LockState lockState3 = LockState.createWithOwner(lockState2, "bsmith"); + lockStore.set(nodeRef, lockState3); + // Check we can see the update. + assertEquals("bsmith", lockStore.get(nodeRef).getOwner()); + + // Perform a read, that we know will retrieve a null value + assertNull("Lock state should be null.", lockStore.get(nodeRef2)); + + // Wait while Thread2 populates the store with a value for nodeRef2 + passControl(this, thread2); + + // Perform the read again - we should see Thread2's update + assertNotNull("Lock state should NOT be null.", lockStore.get(nodeRef2)); + assertEquals("not-null-lockstate", lockStore.get(nodeRef2).getOwner()); + } + + @Test + public void testCannotSetLockWhenChangedByAnotherTx() throws NotSupportedException, SystemException + { + final TransactionService txService = (TransactionService) ctx.getBean("TransactionService"); + UserTransaction txA = txService.getUserTransaction(); + final NodeRef nodeRef = new NodeRef("workspace://SpacesStore/UUID-1"); + Date now = new Date(); + Date expires = new Date(now.getTime() + 180000); + final LockState lockState1 = LockState.createLock(nodeRef, LockType.WRITE_LOCK, + "jbloggs", expires, Lifetime.EPHEMERAL, null); + + + Thread txB = new Thread("TxB") + { + @Override + public void run() + { + Object main = AbstractLockStoreTxTest.this; + UserTransaction tx = txService.getUserTransaction(); + try + { + tx.begin(); + try + { + // txB read lock state + LockState lockState = lockStore.get(nodeRef); + assertEquals("jbloggs", lockState.getOwner()); + assertEquals(Lifetime.EPHEMERAL, lockState.getLifetime()); + + // Wait, while txA changes the lock state + passControl(this, main); + + try + { + // Attempt to change the lock state for a NodeRef should fail + // when it has been modified by another tx since this tx last inspected it. + AuthenticationUtil.setFullyAuthenticatedUser("jbloggs"); // Current lock owner + lockStore.set(nodeRef, LockState.createLock(nodeRef, LockType.WRITE_LOCK, + "csmith", null, Lifetime.EPHEMERAL, null)); + fail("Exception should have been thrown but was not."); + } + catch (ConcurrencyFailureException e) + { + // Good! + } + } + finally + { + tx.rollback(); + } + } + catch (Throwable e) + { + throw new RuntimeException("Error in transaction B", e); + } + finally + { + // Stop 'main' from waiting + synchronized(main) + { + main.notifyAll(); + } + } + } + }; + + txA.begin(); + try + { + // txA set lock state 1 + lockStore.set(nodeRef, lockState1); + + // Wait while txB reads and checks the LockState + txB.setDaemon(true); + txB.start(); + passControl(this, txB); + + // txA set different lock state + AuthenticationUtil.setFullyAuthenticatedUser("jbloggs"); // Current lock owner needed to change lock. + final LockState lockState2 = LockState.createWithOwner(lockState1, "another"); + lockStore.set(nodeRef, lockState2); + + // Wait while txB attempts to modify the lock info + passControl(this, txB); + + // Lock shouldn't have changed since this tx updated it. + assertEquals(lockState2, lockStore.get(nodeRef)); + } + finally + { + txA.rollback(); + } + } + + @Test + public void testCanChangeLockIfLatestValueIsHeldEvenIfAlreadyChangedByAnotherTx() throws NotSupportedException, SystemException + { + final TransactionService txService = (TransactionService) ctx.getBean("TransactionService"); + UserTransaction txA = txService.getUserTransaction(); + final NodeRef nodeRef = new NodeRef("workspace://SpacesStore/UUID-1"); + final Date now = new Date(); + Date expired = new Date(now.getTime() - 180000); + final LockState lockState1 = LockState.createLock(nodeRef, LockType.WRITE_LOCK, + "jbloggs", expired, Lifetime.EPHEMERAL, null); + + final LockState lockState2 = LockState.createWithOwner(lockState1, "another"); + + Thread txB = new Thread("TxB") + { + @Override + public void run() + { + Object main = AbstractLockStoreTxTest.this; + UserTransaction tx = txService.getUserTransaction(); + try + { + tx.begin(); + try + { + AuthenticationUtil.setFullyAuthenticatedUser("new-user"); + + // txB read lock state + LockState readLockState = lockStore.get(nodeRef); + assertEquals(lockState2, readLockState); + + // Set new value, even though txA has already set new values + // (but not since this tx's initial read) + Date expiresFuture = new Date(now.getTime() + 180000); + final LockState newUserLockState = LockState.createLock(nodeRef, LockType.WRITE_LOCK, + "new-user", expiresFuture, Lifetime.EPHEMERAL, null); + lockStore.set(nodeRef, newUserLockState); + + // Read + assertEquals(newUserLockState, lockStore.get(nodeRef)); + } + finally + { + tx.rollback(); + } + } + catch (Throwable e) + { + throw new RuntimeException("Error in transaction B", e); + } + finally + { + // Stop 'main' from waiting + synchronized(main) + { + main.notifyAll(); + } + } + } + }; + + txA.begin(); + try + { + AuthenticationUtil.setFullyAuthenticatedUser("jbloggs"); // Current lock owner needed to change lock. + + // txA set lock state 1 + lockStore.set(nodeRef, lockState1); + assertEquals(lockState1, lockStore.get(nodeRef)); + + // txA set different lock state + lockStore.set(nodeRef, lockState2); + assertEquals(lockState2, lockStore.get(nodeRef)); + + // Wait while txB modifies the lock info + txB.setDaemon(true); + txB.start(); + passControl(this, txB); + + // This tx should still see the same state, though it has been changed by txB. + assertEquals(lockState2, lockStore.get(nodeRef)); + } + finally + { + txA.rollback(); + } + } + + + @Test + public void testOnlyCurrentLockOwnerCanChangeInfo() throws NotSupportedException, SystemException + { + final TransactionService txService = (TransactionService) ctx.getBean("TransactionService"); + UserTransaction txA = txService.getUserTransaction(); + final NodeRef nodeRef = new NodeRef("workspace://SpacesStore/UUID-1"); + Date now = new Date(); + Date expires = new Date(now.getTime() + 180000); + final LockState lockState1 = LockState.createLock(nodeRef, LockType.WRITE_LOCK, + "jbloggs", expires, Lifetime.EPHEMERAL, null); + + txA.begin(); + try + { + AuthenticationUtil.setFullyAuthenticatedUser("jbloggs"); + + // Set initial lock state + lockStore.set(nodeRef, lockState1); + + // Set different lock state + // Current lock owner is still authenticated (jbloggs) + final LockState lockState2 = LockState.createWithOwner(lockState1, "csmith"); + lockStore.set(nodeRef, lockState2); + + // Check update + assertEquals(lockState2, lockStore.get(nodeRef)); + + // Incorrect lock owner - this should fail. + final LockState lockState3 = LockState.createWithOwner(lockState1, "dsmithers"); + try + { + lockStore.set(nodeRef, lockState3); + fail("Exception should have been thrown, but was not."); + } + catch(UnableToAquireLockException e) + { + // Good + } + + // Check no update. + assertEquals(lockState2, lockStore.get(nodeRef)); + } + finally + { + txA.rollback(); + } + } + + @Test + public void testOtherUserCanChangeLockInfoOnceExpired() throws NotSupportedException, SystemException + { + final TransactionService txService = (TransactionService) ctx.getBean("TransactionService"); + UserTransaction txA = txService.getUserTransaction(); + final NodeRef nodeRef = new NodeRef("workspace://SpacesStore/UUID-1"); + Date now = new Date(); + Date expired = new Date(now.getTime() - 900); + final LockState lockState1 = LockState.createLock(nodeRef, LockType.WRITE_LOCK, + "jbloggs", expired, Lifetime.EPHEMERAL, null); + + txA.begin(); + try + { + AuthenticationUtil.setFullyAuthenticatedUser("jbloggs"); + + // Set initial lock state + lockStore.set(nodeRef, lockState1); + + // Set different lock state + AuthenticationUtil.setFullyAuthenticatedUser("csmith"); + Date expiresFuture = new Date(now.getTime() + 180000); + final LockState lockState2 = LockState.createLock(nodeRef, LockType.WRITE_LOCK, + "csmith", expiresFuture, Lifetime.EPHEMERAL, null); + lockStore.set(nodeRef, lockState2); + + // Updated, since lock had expired. + assertEquals(lockState2, lockStore.get(nodeRef)); + + + // Incorrect lock owner - this should fail, current lock has not expired + // and is owned by csmith. + AuthenticationUtil.setFullyAuthenticatedUser("dsmithers"); + final LockState lockState3 = LockState.createWithOwner(lockState2, "dsmithers"); + try + { + lockStore.set(nodeRef, lockState3); + fail("Exception should have been thrown, but was not."); + } + catch(UnableToAquireLockException e) + { + // Good + } + + // Check no update. + assertEquals(lockState2, lockStore.get(nodeRef)); + } + finally + { + txA.rollback(); + } + } +} diff --git a/source/test-java/org/alfresco/repo/lock/mem/LockStoreImplTxTest.java b/source/test-java/org/alfresco/repo/lock/mem/LockStoreImplTxTest.java new file mode 100644 index 0000000000..95792a9608 --- /dev/null +++ b/source/test-java/org/alfresco/repo/lock/mem/LockStoreImplTxTest.java @@ -0,0 +1,33 @@ +/* + * Copyright (C) 2005-2013 Alfresco Software Limited. + * + * This file is part of Alfresco + * + * Alfresco is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Alfresco is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with Alfresco. If not, see . + */ +package org.alfresco.repo.lock.mem; + +/** + * Test transaction related functions of {@link LockStoreImpl}. + * + * @author Matt Ward + */ +public class LockStoreImplTxTest extends AbstractLockStoreTxTest +{ + @Override + protected LockStoreImpl createLockStore() + { + return new LockStoreImpl(20); + } +} 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 c4ff75a4b1..8f91b1c08d 100644 --- a/source/test-java/org/alfresco/repo/lock/mem/LockableAspectInterceptorTest.java +++ b/source/test-java/org/alfresco/repo/lock/mem/LockableAspectInterceptorTest.java @@ -187,22 +187,22 @@ public class LockableAspectInterceptorTest // adding cm:auditable will result in cm:created property being added nodeService.addAspect(nodeRef, ContentModel.ASPECT_AUDITABLE, new HashMap()); - Map properties = nodeService.getProperties(nodeRef); + Date now = new Date(); + Map lockProps = new HashMap(); + 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); + nodeService.addAspect(nodeRef, ContentModel.ASPECT_LOCKABLE, lockProps); - assertFalse("Node should not have lockOwner property", - properties.containsKey(ContentModel.PROP_LOCK_OWNER)); - assertTrue("Node should have created property", - properties.containsKey(ContentModel.PROP_CREATED)); + Map readProps = nodeService.getProperties(nodeRef); - lockStore.set(nodeRef, - LockState.createLock(nodeRef, LockType.WRITE_LOCK, lockOwner, null, Lifetime.PERSISTENT, null)); - properties = nodeService.getProperties(nodeRef); - - // Nothing should have changed since the persistent lock was added. - assertFalse("Node should not have lockOwner property", - properties.containsKey(ContentModel.PROP_LOCK_OWNER)); - assertTrue("Node should have created property", - properties.containsKey(ContentModel.PROP_CREATED)); + assertEquals("jbloggs", readProps.get(ContentModel.PROP_LOCK_OWNER)); + assertEquals(LockType.READ_ONLY_LOCK.toString(), readProps.get(ContentModel.PROP_LOCK_TYPE)); + assertEquals(now, readProps.get(ContentModel.PROP_EXPIRY_DATE)); + // Spoofed - wasn't explicitly added. + assertEquals(Lifetime.PERSISTENT, readProps.get(ContentModel.PROP_LOCK_LIFETIME)); + // Double check - not really present + assertFalse(rawNodeService.getProperties(nodeRef).containsKey(ContentModel.PROP_LOCK_LIFETIME)); } @Test @@ -254,7 +254,7 @@ 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 lockStore.set(nodeRef, @@ -264,6 +264,7 @@ public class LockableAspectInterceptorTest 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(Lifetime.EPHEMERAL.toString(), nodeService.getProperty(nodeRef, ContentModel.PROP_LOCK_LIFETIME)); } @Test @@ -281,14 +282,22 @@ public class LockableAspectInterceptorTest assertEquals(null, nodeService.getProperty(nodeRef, ContentModel.PROP_EXPIRY_DATE)); Date now = new Date(); + // Set a lock on the node - lockStore.set(nodeRef, - LockState.createLock(nodeRef, LockType.WRITE_LOCK, lockOwner, now, Lifetime.PERSISTENT, null)); + Map lockProps = new HashMap(); + lockProps.put(ContentModel.PROP_LOCK_OWNER, "another"); + lockProps.put(ContentModel.PROP_LOCK_TYPE, LockType.WRITE_LOCK.toString()); + lockProps.put(ContentModel.PROP_EXPIRY_DATE, now); + nodeService.addAspect(nodeRef, ContentModel.ASPECT_LOCKABLE, lockProps); // cm:lockable properties should be unaffected - 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("another", 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)); + // Spoofed property + assertEquals(Lifetime.PERSISTENT.toString(), nodeService.getProperty(nodeRef, ContentModel.PROP_LOCK_LIFETIME)); + // Double check, not really present + assertNull(rawNodeService.getProperty(nodeRef, ContentModel.PROP_LOCK_LIFETIME)); } @Test @@ -356,26 +365,18 @@ public class LockableAspectInterceptorTest nodeName, ContentModel.TYPE_BASE).getChildRef(); - Map properties = nodeService.getProperties(nodeRef); - // Properties that should be unaffected + Map properties = rawNodeService.getProperties(nodeRef); + + // With the exception of cm:lockLifetime, lock properties should be unaffected after setProperties() properties.put(ContentModel.PROP_AUTHOR, "Joe Bloggs"); properties.put(ContentModel.PROP_NAME, "A Name"); properties.put(ContentModel.PROP_LOCK_TYPE, LockType.NODE_LOCK); + properties.put(ContentModel.PROP_LOCK_LIFETIME, Lifetime.PERSISTENT); properties.put(ContentModel.PROP_LOCK_OWNER, "Alison Bloggs"); Date expiryDate = new Date(); properties.put(ContentModel.PROP_EXPIRY_DATE, expiryDate); - lockStore.set( - nodeRef, - LockState.createLock( - nodeRef, - LockType.NODE_LOCK, - "Alison Bloggs", - expiryDate, - Lifetime.PERSISTENT, - null)); - - // Set the properties + // Set the properties (intercepted) nodeService.setProperties(nodeRef, properties); // Check the persisted properties @@ -385,6 +386,9 @@ public class LockableAspectInterceptorTest assertEquals(LockType.NODE_LOCK.toString(), properties.get(ContentModel.PROP_LOCK_TYPE)); assertEquals("Alison Bloggs", properties.get(ContentModel.PROP_LOCK_OWNER)); assertEquals(expiryDate, properties.get(ContentModel.PROP_EXPIRY_DATE)); + + // cm:lockLifetime is not persisted. + assertFalse(properties.containsKey(ContentModel.PROP_LOCK_LIFETIME)); } @Test @@ -397,26 +401,17 @@ public class LockableAspectInterceptorTest nodeName, ContentModel.TYPE_BASE).getChildRef(); - Map properties = nodeService.getProperties(nodeRef); - // Properties that should be unaffected + Map properties = rawNodeService.getProperties(nodeRef); + // Non-lock properties should be unaffected after setProperties() properties.put(ContentModel.PROP_AUTHOR, "Joe Bloggs"); properties.put(ContentModel.PROP_NAME, "A Name"); - // Properties that should not be persisted + // Lock properties should not be persisted (filtered out by interceptor) properties.put(ContentModel.PROP_LOCK_TYPE, LockType.NODE_LOCK); + properties.put(ContentModel.PROP_LOCK_LIFETIME, Lifetime.EPHEMERAL); properties.put(ContentModel.PROP_LOCK_OWNER, "Alison Bloggs"); Date expiryDate = new Date(); properties.put(ContentModel.PROP_EXPIRY_DATE, expiryDate); - lockStore.set( - nodeRef, - LockState.createLock( - nodeRef, - LockType.NODE_LOCK, - "Alison Bloggs", - expiryDate, - Lifetime.EPHEMERAL, - null)); - // Set the properties nodeService.setProperties(nodeRef, properties); @@ -426,6 +421,7 @@ public class LockableAspectInterceptorTest assertEquals("A Name", properties.get(ContentModel.PROP_NAME)); // Check the filtered properties assertNull(properties.get(ContentModel.PROP_LOCK_TYPE)); + assertNull(properties.get(ContentModel.PROP_LOCK_LIFETIME)); assertNull(properties.get(ContentModel.PROP_LOCK_OWNER)); assertNull(properties.get(ContentModel.PROP_EXPIRY_DATE)); }