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));
}