Author: bcarothers Date: 2009-12-07 15:56:18 -0500 (Mon, 07 Dec 2009) New Revision: 1414 Modified: trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/AbstractJcrNode.java trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/DnaLexicon.java trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrEngine.java trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrI18n.java trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrRepository.java trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrSession.java trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/WorkspaceLockManager.java trunk/dna-jcr/src/main/resources/org/jboss/dna/jcr/JcrI18n.properties trunk/dna-jcr/src/main/resources/org/jboss/dna/jcr/dna_builtins.cnd trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/JcrRepositoryTest.java trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/WorkspaceLockManagerTest.java Log: DNA-541 Locking Implementation Does Not Support Timeouts Committed a patch that implements lock expiration based on the algorithm above modified by Randall's comments. The patch includes two tests to confirm that the functionality works, but one is @Ignored because it depends on garbage collector behavior to test the results and the other is @Ignored because it has to sleep for 30 seconds to test the timeout. The patch also addresses specific feedback from the JIRA by integrating the ScheduledExecutionService in the JcrEngine into the DnaEngine's lifecycle methods (start, shutdown, awaitTermination) and synchronizing access to JcrRepository.activeSessions Modified: trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/AbstractJcrNode.java =================================================================== --- trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/AbstractJcrNode.java 2009-12-07 15:19:53 UTC (rev 1413) +++ trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/AbstractJcrNode.java 2009-12-07 20:56:18 UTC (rev 1414) @@ -1409,7 +1409,7 @@ } } - session().workspace().lockManager().unlock(session(), lock); + session().workspace().lockManager().unlock(session().getExecutionContext(), lock); session().removeLockToken(lock.getLockToken()); } Modified: trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/DnaLexicon.java =================================================================== --- trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/DnaLexicon.java 2009-12-07 15:19:53 UTC (rev 1413) +++ trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/DnaLexicon.java 2009-12-07 20:56:18 UTC (rev 1414) @@ -34,10 +34,12 @@ public class DnaLexicon extends org.jboss.dna.repository.DnaLexicon { public static final Name BASE = new BasicName(Namespace.URI, "base"); + public static final Name EXPIRATION_DATE = new BasicName(Namespace.URI, "expirationDate"); public static final Name IS_HELD_BY_SESSION = new BasicName(Namespace.URI, "isHeldBySession"); public static final Name IS_SESSION_SCOPED = new BasicName(Namespace.URI, "isSessionScoped"); public static final Name LOCK = new BasicName(Namespace.URI, "lock"); public static final Name LOCKED_UUID = new BasicName(Namespace.URI, "lockedUuid"); + public static final Name LOCKING_SESSION = new BasicName(Namespace.URI, "lockingSession"); public static final Name LOCKS = new BasicName(Namespace.URI, "locks"); public static final Name NAMESPACE = new BasicName(Namespace.URI, "namespace"); public static final Name NODE_TYPES = new BasicName(Namespace.URI, "nodeTypes"); Modified: trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrEngine.java =================================================================== --- trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrEngine.java 2009-12-07 15:19:53 UTC (rev 1413) +++ trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrEngine.java 2009-12-07 20:56:18 UTC (rev 1414) @@ -23,17 +23,23 @@ */ package org.jboss.dna.jcr; +import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.Map; import java.util.Set; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledThreadPoolExecutor; +import java.util.concurrent.TimeUnit; import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReentrantLock; import javax.jcr.Repository; import javax.jcr.RepositoryException; import net.jcip.annotations.ThreadSafe; import org.jboss.dna.common.util.CheckArg; +import org.jboss.dna.common.util.Logger; import org.jboss.dna.graph.ExecutionContext; import org.jboss.dna.graph.Graph; import org.jboss.dna.graph.Location; @@ -57,9 +63,19 @@ @ThreadSafe public class JcrEngine extends DnaEngine { + final static int LOCK_SWEEP_INTERVAL_IN_MILLIS = 30000; + final static int LOCK_EXTENSION_INTERVAL_IN_MILLIS = LOCK_SWEEP_INTERVAL_IN_MILLIS * 2; + + private final Logger log = Logger.getLogger(DnaEngine.class); + private final Map<String, JcrRepository> repositories; private final Lock repositoriesLock; + /** + * Provides the ability to schedule lock clean-up + */ + private final ScheduledExecutorService scheduler = new ScheduledThreadPoolExecutor(2); + JcrEngine( ExecutionContext context, DnaConfiguration.ConfigurationDefinition configuration ) { super(context, configuration); @@ -68,6 +84,65 @@ } /** + * Clean up session-scoped locks created by session that are no longer active by iterating over the {@link JcrRepository + * repositories} and calling their {@link JcrRepository#cleanUpLocks() clean-up method}. + * <p> + * It should not be possible for a session to be terminated without cleaning up its locks, but this method will help clean-up + * dangling locks should a session terminate abnormally. + * </p> + */ + void cleanUpLocks() { + Collection<JcrRepository> repos; + + try { + // Make a copy of the repositories to minimize the time that the lock needs to be held + repositoriesLock.lock(); + repos = new ArrayList<JcrRepository>(repositories.values()); + } finally { + repositoriesLock.unlock(); + } + + for (JcrRepository repository : repos) { + try { + repository.cleanUpLocks(); + } catch (Throwable t) { + log.error(t, JcrI18n.errorCleaningUpLocks, repository.getRepositorySourceName()); + } + } + } + + @Override + public void shutdown() { + scheduler.shutdown(); + + super.shutdown(); + } + + @Override + public boolean awaitTermination( long timeout, + TimeUnit unit ) throws InterruptedException { + if (!scheduler.awaitTermination(timeout, unit)) return false; + + return super.awaitTermination(timeout, unit); + } + + @Override + public void start() { + super.start(); + + final JcrEngine engine = this; + Runnable cleanUpTask = new Runnable() { + + @Override + public void run() { + engine.cleanUpLocks(); + } + + }; + scheduler.scheduleAtFixedRate(cleanUpTask, 0, LOCK_SWEEP_INTERVAL_IN_MILLIS, TimeUnit.MILLISECONDS); + } + + /** * Get the {@link Repository} implementation for the named repository. * * @param repositoryName the name of the repository, which corresponds to the name of a configured {@link RepositorySource} Modified: trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrI18n.java =================================================================== --- trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrI18n.java 2009-12-07 15:19:53 UTC (rev 1413) +++ trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrI18n.java 2009-12-07 20:56:18 UTC (rev 1414) @@ -66,6 +66,9 @@ public static I18n unableToRemapUriUsingPrefixUsedInNamespaceRegistry; public static I18n errorWhileInitializingTheNamespaceRegistry; + public static I18n errorCleaningUpLocks; + public static I18n cleaningUpLocks; + public static I18n cleanedUpLocks; public static I18n invalidRelativePath; public static I18n invalidPathParameter; public static I18n invalidNamePattern; Modified: trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrRepository.java =================================================================== --- trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrRepository.java 2009-12-07 15:19:53 UTC (rev 1413) +++ trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrRepository.java 2009-12-07 20:56:18 UTC (rev 1414) @@ -33,10 +33,12 @@ import java.util.Collections; import java.util.EnumMap; import java.util.HashMap; +import java.util.HashSet; import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.WeakHashMap; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.CopyOnWriteArrayList; @@ -61,6 +63,8 @@ import org.jboss.dna.graph.ExecutionContext; import org.jboss.dna.graph.Graph; import org.jboss.dna.graph.JaasSecurityContext; +import org.jboss.dna.graph.Location; +import org.jboss.dna.graph.Node; import org.jboss.dna.graph.SecurityContext; import org.jboss.dna.graph.Subgraph; import org.jboss.dna.graph.connector.RepositoryConnection; @@ -75,12 +79,16 @@ import org.jboss.dna.graph.observe.Changes; import org.jboss.dna.graph.observe.Observable; import org.jboss.dna.graph.observe.Observer; +import org.jboss.dna.graph.property.DateTime; +import org.jboss.dna.graph.property.DateTimeFactory; import org.jboss.dna.graph.property.Name; import org.jboss.dna.graph.property.NamespaceRegistry; import org.jboss.dna.graph.property.Path; import org.jboss.dna.graph.property.PathFactory; +import org.jboss.dna.graph.property.PathNotFoundException; import org.jboss.dna.graph.property.Property; import org.jboss.dna.graph.property.PropertyFactory; +import org.jboss.dna.graph.property.ValueFactory; import org.jboss.dna.graph.property.basic.GraphNamespaceRegistry; import org.jboss.dna.graph.query.parse.QueryParsers; import org.jboss.dna.graph.query.parse.SqlQueryParser; @@ -111,6 +119,8 @@ @ThreadSafe public class JcrRepository implements Repository { + private static final Logger log = Logger.getLogger(JcrRepository.class); + /** * A flag that controls whether the repository uses a shared repository (or workspace) for the "/jcr:system" content in all of * the workspaces. In production, this needs to be "true" for proper JCR functionality, but in some debugging cases it can be @@ -122,6 +132,13 @@ static final boolean WORKSPACES_SHARE_SYSTEM_BRANCH = true; /** + * The user name for anonymous sessions + * + * @see Option#ANONYMOUS_USER_ROLES + */ + static final String ANONYMOUS_USER_NAME = "<anonymous>"; + + /** * The available options for the {@code JcrRepository}. */ public enum Option { @@ -270,6 +287,9 @@ private final QueryParsers queryParsers = new QueryParsers(new SqlQueryParser(), new XPathQueryParser(), new FullTextSearchParser()); + // package-scoped to facilitate testing + final WeakHashMap<JcrSession, Object> activeSessions = new WeakHashMap<JcrSession, Object>(); + /** * Creates a JCR repository that uses the supplied {@link RepositoryConnectionFactory repository connection factory} to * establish {@link Session sessions} to the underlying repository source upon {@link #login() login}. @@ -471,7 +491,7 @@ anonymousUserContext = new SecurityContext() { public String getUserName() { - return null; + return ANONYMOUS_USER_NAME; } public boolean hasRole( String roleName ) { @@ -788,6 +808,11 @@ } catch (AccessControlException ace) { throw new NoSuchWorkspaceException(JcrI18n.workspaceNameIsInvalid.text(sourceName, workspaceName)); } + + synchronized (this.activeSessions) { + activeSessions.put(session, null); + } + return session; } @@ -809,6 +834,101 @@ return lockManager; } + /** + * Marks the given session as inactive (by removing it from the {@link #activeSessions active sessions map}. + * + * @param session the session to be marked as inactive + */ + void sessionLoggedOut( JcrSession session ) { + synchronized (this.activeSessions) { + this.activeSessions.remove(session); + } + } + + /** + * Returns the set of active sessions in this repository + * + * @return the set of active sessions in this repository + */ + Set<JcrSession> activeSessions() { + Set<JcrSession> activeSessions; + synchronized (this.activeSessions) { + activeSessions = new HashSet<JcrSession>(this.activeSessions.keySet()); + } + // There can and will be elements in this set that are no longer live but haven't yet been gc'ed. + // Filter those out + for (Iterator<JcrSession> iter = activeSessions.iterator(); iter.hasNext();) { + JcrSession session = iter.next(); + if (session != null && !session.isLive()) { + iter.remove(); + } + } + + return activeSessions; + } + + /** + * Iterates through the list of session-scoped locks in this repository, deleting any session-scoped locks that were created + * by a session that is no longer active. + */ + void cleanUpLocks() { + if (log.isTraceEnabled()) { + log.trace(JcrI18n.cleaningUpLocks.text()); + } + + Set<JcrSession> activeSessions = activeSessions(); + Set<String> activeSessionIds = new HashSet<String>(activeSessions.size()); + + for (JcrSession activeSession : activeSessions) { + activeSessionIds.add(activeSession.sessionId()); + } + + Graph systemGraph = createSystemGraph(executionContext); + PathFactory pathFactory = executionContext.getValueFactories().getPathFactory(); + ValueFactory<Boolean> booleanFactory = executionContext.getValueFactories().getBooleanFactory(); + ValueFactory<String> stringFactory = executionContext.getValueFactories().getStringFactory(); + + DateTimeFactory dateFactory = executionContext.getValueFactories().getDateFactory(); + DateTime now = dateFactory.create(); + DateTime newExpirationDate = now.plusMillis(JcrEngine.LOCK_EXTENSION_INTERVAL_IN_MILLIS); + + Path locksPath = pathFactory.createAbsolutePath(JcrLexicon.SYSTEM, DnaLexicon.LOCKS); + + Subgraph locksGraph = null; + try { + locksGraph = systemGraph.getSubgraphOfDepth(2).at(locksPath); + } catch (PathNotFoundException pnfe) { + // It's possible for this to run before the dna:locks child node gets added to the /jcr:system node. + return; + } + + for (Location lockLocation : locksGraph.getRoot().getChildren()) { + Node lockNode = locksGraph.getNode(lockLocation); + + Boolean isSessionScoped = booleanFactory.create(lockNode.getProperty(DnaLexicon.IS_SESSION_SCOPED).getFirstValue()); + + if (!isSessionScoped) continue; + String lockingSession = stringFactory.create(lockNode.getProperty(DnaLexicon.LOCKING_SESSION).getFirstValue()); + + // Extend locks held by active sessions + if (activeSessionIds.contains(lockingSession)) { + systemGraph.set(DnaLexicon.EXPIRATION_DATE).on(lockLocation).to(newExpirationDate); + } else { + DateTime expirationDate = dateFactory.create(lockNode.getProperty(DnaLexicon.EXPIRATION_DATE).getFirstValue()); + // Destroy expired locks (if it was still held by an active session, it would have been extended by now) + if (expirationDate.isBefore(now)) { + String workspaceName = stringFactory.create(lockNode.getProperty(DnaLexicon.WORKSPACE).getFirstValue()); + WorkspaceLockManager lockManager = lockManagers.get(workspaceName); + lockManager.unlock(executionContext, lockManager.createLock(lockNode)); + } + } + } + + if (log.isTraceEnabled()) { + log.trace(JcrI18n.cleanedUpLocks.text()); + } + } + protected class FederatedRepositoryContext implements RepositoryContext { private final RepositoryConnectionFactory connectionFactory; Modified: trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrSession.java =================================================================== --- trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrSession.java 2009-12-07 15:19:53 UTC (rev 1413) +++ trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrSession.java 2009-12-07 20:56:18 UTC (rev 1414) @@ -785,6 +785,7 @@ isLive = false; this.workspace().observationManager().removeAllEventListeners(); this.workspace().lockManager().cleanLocks(this); + this.repository.sessionLoggedOut(this); this.executionContext.getSecurityContext().logout(); } Modified: trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/WorkspaceLockManager.java =================================================================== --- trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/WorkspaceLockManager.java 2009-12-07 15:19:53 UTC (rev 1413) +++ trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/WorkspaceLockManager.java 2009-12-07 20:56:18 UTC (rev 1414) @@ -16,6 +16,8 @@ import org.jboss.dna.graph.Graph; import org.jboss.dna.graph.Location; import org.jboss.dna.graph.connector.LockFailedException; +import org.jboss.dna.graph.property.DateTime; +import org.jboss.dna.graph.property.DateTimeFactory; import org.jboss.dna.graph.property.Path; import org.jboss.dna.graph.property.PathFactory; import org.jboss.dna.graph.property.PathNotFoundException; @@ -31,6 +33,7 @@ @ThreadSafe class WorkspaceLockManager { + private final ExecutionContext context; private final Path locksPath; private final JcrRepository repository; private final String workspaceName; @@ -40,6 +43,7 @@ JcrRepository repository, String workspaceName, Path locksPath ) { + this.context = context; this.repository = repository; this.workspaceName = workspaceName; this.locksPath = locksPath; @@ -79,7 +83,7 @@ } ExecutionContext sessionContext = session.getExecutionContext(); - String lockOwner = sessionContext.getSecurityContext().getUserName(); + String lockOwner = session.getUserID(); DnaLock lock = createLock(lockOwner, lockUuid, nodeUuid, isDeep, isSessionScoped); Graph.Batch batch = repository.createSystemGraph(sessionContext).batch(); @@ -89,11 +93,17 @@ Property lockOwnerProp = propFactory.create(JcrLexicon.LOCK_OWNER, lockOwner); Property lockIsDeepProp = propFactory.create(JcrLexicon.LOCK_IS_DEEP, isDeep); + DateTimeFactory dateFactory = sessionContext.getValueFactories().getDateFactory(); + DateTime expirationDate = dateFactory.create(); + expirationDate = expirationDate.plusMillis(JcrEngine.LOCK_EXTENSION_INTERVAL_IN_MILLIS); + batch.create(pathFactory.create(locksPath, pathFactory.createSegment(lockUuid.toString())), propFactory.create(JcrLexicon.PRIMARY_TYPE, DnaLexicon.LOCK), propFactory.create(DnaLexicon.WORKSPACE, workspaceName), propFactory.create(DnaLexicon.LOCKED_UUID, nodeUuid.toString()), propFactory.create(DnaLexicon.IS_SESSION_SCOPED, isSessionScoped), + propFactory.create(DnaLexicon.LOCKING_SESSION, session.sessionId()), + propFactory.create(DnaLexicon.EXPIRATION_DATE, expirationDate), // This gets set after the lock succeeds and the lock token gets added to the session propFactory.create(DnaLexicon.IS_HELD_BY_SESSION, false), lockOwnerProp, @@ -116,6 +126,10 @@ return lock; } + DnaLock createLock( org.jboss.dna.graph.Node lockNode ) { + return new DnaLock(lockNode); + } + /* Factory method added to facilitate mocked testing */ DnaLock createLock( String lockOwner, UUID lockUuid, @@ -171,7 +185,7 @@ workspaceBatch.execute(); } catch (LockFailedException lfe) { // Attempt to lock node at the repo level failed - cancel lock - unlock(session, lock); + unlock(session.getExecutionContext(), lock); throw new RepositoryException(lfe); } @@ -180,22 +194,21 @@ /** * Removes the provided lock, effectively unlocking the node to which the lock is associated. * - * @param session the session in which the node is being unlocked + * @param sessionExecutionContext the execution context of the session in which the node is being unlocked * @param lock the lock to be removed */ - void unlock( JcrSession session, + void unlock( ExecutionContext sessionExecutionContext, DnaLock lock ) { try { - ExecutionContext context = session.getExecutionContext(); - PathFactory pathFactory = context.getValueFactories().getPathFactory(); + PathFactory pathFactory = sessionExecutionContext.getValueFactories().getPathFactory(); // Remove the lock node under the /jcr:system branch ... - Graph.Batch batch = repository.createSystemGraph(context).batch(); + Graph.Batch batch = repository.createSystemGraph(sessionExecutionContext).batch(); batch.delete(pathFactory.create(locksPath, pathFactory.createSegment(lock.getUuid().toString()))); batch.execute(); // Unlock the node in the repository graph ... - unlockNodeInRepository(session, lock); + unlockNodeInRepository(sessionExecutionContext, lock); workspaceLocksByNodeUuid.remove(lock.nodeUuid); } catch (PathNotFoundException pnfe) { @@ -220,12 +233,12 @@ * /jcr:system/dna:locks} subgraph. * </p> * - * @param session the session in which the node is being unlocked + * @param sessionExecutionContext the execution context of the session in which the node is being unlocked * @param lock */ - void unlockNodeInRepository( JcrSession session, + void unlockNodeInRepository( ExecutionContext sessionExecutionContext, DnaLock lock ) { - Graph.Batch workspaceBatch = repository.createWorkspaceGraph(this.workspaceName, session.getExecutionContext()).batch(); + Graph.Batch workspaceBatch = repository.createWorkspaceGraph(this.workspaceName, sessionExecutionContext).batch(); workspaceBatch.remove(JcrLexicon.LOCK_OWNER, JcrLexicon.LOCK_IS_DEEP).on(lock.nodeUuid); workspaceBatch.unlock(lock.nodeUuid); @@ -249,9 +262,8 @@ ValueFactory<Boolean> booleanFactory = context.getValueFactories().getBooleanFactory(); PathFactory pathFactory = context.getValueFactories().getPathFactory(); - org.jboss.dna.graph.Node lockNode = repository.createSystemGraph(context) - .getNodeAt(pathFactory.create(locksPath, - pathFactory.createSegment(lockToken))); + org.jboss.dna.graph.Node lockNode = repository.createSystemGraph(context).getNodeAt(pathFactory.create(locksPath, + pathFactory.createSegment(lockToken))); return booleanFactory.create(lockNode.getProperty(DnaLexicon.IS_HELD_BY_SESSION).getFirstValue()); @@ -275,9 +287,8 @@ PropertyFactory propFactory = context.getPropertyFactory(); PathFactory pathFactory = context.getValueFactories().getPathFactory(); - repository.createSystemGraph(context) - .set(propFactory.create(DnaLexicon.IS_HELD_BY_SESSION, value)) - .on(pathFactory.create(locksPath, pathFactory.createSegment(lockToken))); + repository.createSystemGraph(context).set(propFactory.create(DnaLexicon.IS_HELD_BY_SESSION, value)).on(pathFactory.create(locksPath, + pathFactory.createSegment(lockToken))); } /** @@ -340,11 +351,12 @@ * @param session the session on behalf of which the lock operation is being performed */ void cleanLocks( JcrSession session ) { + ExecutionContext context = session.getExecutionContext(); Collection<String> lockTokens = session.lockTokens(); for (String lockToken : lockTokens) { DnaLock lock = lockFor(lockToken); if (lock != null && lock.isSessionScoped()) { - unlock(session, lock); + unlock(context, lock); } } } @@ -361,6 +373,32 @@ private final boolean deep; private final boolean sessionScoped; + DnaLock( org.jboss.dna.graph.Node lockNode ) { + ValueFactory<String> stringFactory = context.getValueFactories().getStringFactory(); + ValueFactory<UUID> uuidFactory = context.getValueFactories().getUuidFactory(); + ValueFactory<Boolean> booleanFactory = context.getValueFactories().getBooleanFactory(); + + assert lockNode.getLocation().getPath() != null; + + String lockUuidAsString = lockNode.getLocation().getPath().getLastSegment().getName().getLocalName(); + Property lockOwnerProperty = lockNode.getProperty(JcrLexicon.LOCK_OWNER); + Property nodeUuidProperty = lockNode.getProperty(DnaLexicon.LOCKED_UUID); + Property lockIsDeepProperty = lockNode.getProperty(JcrLexicon.LOCK_IS_DEEP); + Property isSessionScopedProperty = lockNode.getProperty(DnaLexicon.IS_SESSION_SCOPED); + + assert lockUuidAsString != null; + assert lockOwnerProperty != null; + assert nodeUuidProperty != null; + assert lockIsDeepProperty != null; + assert isSessionScopedProperty != null; + + this.lockOwner = stringFactory.create(lockOwnerProperty.getFirstValue()); + this.lockUuid = UUID.fromString(lockUuidAsString); + this.nodeUuid = uuidFactory.create(nodeUuidProperty.getFirstValue()); + this.deep = booleanFactory.create(lockIsDeepProperty.getFirstValue()); + this.sessionScoped = booleanFactory.create(isSessionScopedProperty.getFirstValue()); + } + DnaLock( String lockOwner, UUID lockUuid, UUID nodeUuid, Modified: trunk/dna-jcr/src/main/resources/org/jboss/dna/jcr/JcrI18n.properties =================================================================== --- trunk/dna-jcr/src/main/resources/org/jboss/dna/jcr/JcrI18n.properties 2009-12-07 15:19:53 UTC (rev 1413) +++ trunk/dna-jcr/src/main/resources/org/jboss/dna/jcr/JcrI18n.properties 2009-12-07 20:56:18 UTC (rev 1414) @@ -54,6 +54,9 @@ unableToRemapUriNotRegisteredInNamespaceRegistry = Unable to remap the namespace "{1}" to prefix "{0}" because the URI is not already registered in the workspace's namespace registry unableToRemapUriUsingPrefixUsedInNamespaceRegistry = Unable to remap the namespace "{1}" to prefix "{0}" because the prefix is already used as the prefix for the namespace "{2}" in the workspace's namespace registry +errorCleaningUpLocks = Error while cleaning up locks for JCR repository "{0}" +cleaningUpLocks = Lock clean up process begun +cleanedUpLocks = Lock clean up process completed errorWhileInitializingTheNamespaceRegistry = Error while initializing the namespace registry for workspace "{0}" invalidRelativePath = "{0}" is not a valid relative path invalidPathParameter = The "{1}" parameter value "{0}" was not a valid path Modified: trunk/dna-jcr/src/main/resources/org/jboss/dna/jcr/dna_builtins.cnd =================================================================== --- trunk/dna-jcr/src/main/resources/org/jboss/dna/jcr/dna_builtins.cnd 2009-12-07 15:19:53 UTC (rev 1413) +++ trunk/dna-jcr/src/main/resources/org/jboss/dna/jcr/dna_builtins.cnd 2009-12-07 20:56:18 UTC (rev 1414) @@ -46,6 +46,8 @@ [dna:lock] > nt:base - dna:lockedUuid (string) protected ignore - jcr:lockOwner (string) protected ignore +- dna:lockingSession (string) protected ignore +- dna:expirationDate (date) protected ignore - dna:sessionScope (boolean) protected ignore - jcr:isDeep (boolean) protected ignore - dna:isHeldBySession (boolean) protected ignore Modified: trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/JcrRepositoryTest.java =================================================================== --- trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/JcrRepositoryTest.java 2009-12-07 15:19:53 UTC (rev 1413) +++ trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/JcrRepositoryTest.java 2009-12-07 20:56:18 UTC (rev 1414) @@ -26,7 +26,6 @@ import static org.hamcrest.collection.IsArrayContaining.hasItemInArray; import static org.hamcrest.core.Is.is; import static org.hamcrest.core.IsNull.notNullValue; -import static org.hamcrest.core.IsNull.nullValue; import static org.junit.Assert.assertThat; import java.security.AccessControlContext; import java.security.AccessController; @@ -51,10 +50,12 @@ import org.jboss.dna.graph.connector.RepositorySourceException; import org.jboss.dna.graph.connector.inmemory.InMemoryRepositorySource; import org.jboss.dna.graph.observe.MockObservable; +import org.jboss.dna.jcr.JcrRepository.Option; import org.jboss.security.config.IDTrustConfiguration; import org.junit.After; import org.junit.Before; import org.junit.BeforeClass; +import org.junit.Ignore; import org.junit.Test; import org.mockito.Mockito; import org.mockito.MockitoAnnotations; @@ -192,12 +193,12 @@ testDescriptorKeys(repository); testDescriptorValues(repository); } - + @Test public void shouldProvideObserver() { assertThat(this.repository.getObserver(), is(notNullValue())); } - + @Test public void shouldProvideRepositoryObservable() { assertThat(this.repository.getRepositoryObservable(), is(notNullValue())); @@ -205,7 +206,8 @@ @Test public void shouldHaveDefaultOptionsWhenNotOverridden() { - JcrRepository repository = new JcrRepository(context, connectionFactory, sourceName, new MockObservable(), descriptors, null); + JcrRepository repository = new JcrRepository(context, connectionFactory, sourceName, new MockObservable(), descriptors, + null); assertThat(repository.getOptions().get(JcrRepository.Option.PROJECT_NODE_TYPES), is(JcrRepository.DefaultOption.PROJECT_NODE_TYPES)); } @@ -258,7 +260,7 @@ session = (JcrSession)repository.login(); assertThat(session, is(notNullValue())); - assertThat(session.getUserID(), is(nullValue())); + assertThat(session.getUserID(), is(JcrRepository.ANONYMOUS_USER_NAME)); } @@ -483,4 +485,76 @@ assertThat(repository.getDescriptor(Repository.SPEC_NAME_DESC), is(JcrI18n.SPEC_NAME_DESC.text())); assertThat(repository.getDescriptor(Repository.SPEC_VERSION_DESC), is("1.0")); } + + @Ignore( "GC behavior is non-deterministic from the application's POV - this test _will_ occasionally fail" ) + @Test + public void shouldAllowManySessionLoginsAndLogouts() throws Exception { + // Use a different repository that supports anonymous logins to make this test cleaner + Map<Option, String> options = new HashMap<Option, String>(); + options.put(JcrRepository.Option.ANONYMOUS_USER_ROLES, JcrSession.DNA_ADMIN_PERMISSION); + JcrRepository repository = new JcrRepository(context, connectionFactory, sourceName, new MockObservable(), descriptors, + options); + + Session session; + + for (int i = 0; i < 10000; i++) { + session = repository.login(); + session.logout(); + } + + session = repository.login(); + session = null; + + // Give the gc a chance to run + System.gc(); + Thread.sleep(100); + + assertThat(repository.activeSessions().size(), is(0)); + } + + @Ignore( "This test normally sleeps for 30 seconds" ) + @Test + public void shouldCleanUpLocksFromDeadSessions() throws Exception { + // Use a different repository that supports anonymous logins to make this test cleaner + Map<Option, String> options = new HashMap<Option, String>(); + options.put(JcrRepository.Option.ANONYMOUS_USER_ROLES, JcrSession.DNA_ADMIN_PERMISSION); + JcrRepository repository = new JcrRepository(context, connectionFactory, sourceName, new MockObservable(), descriptors, + options); + + String lockedNodeName = "lockedNode"; + JcrSession locker = (JcrSession)repository.login(); + + // Create a node to lock + javax.jcr.Node lockedNode = locker.getRootNode().addNode(lockedNodeName); + lockedNode.addMixin("mix:lockable"); + locker.save(); + + // Create a session-scoped lock (not deep) + lockedNode.lock(false, true); + assertThat(lockedNode.isLocked(), is(true)); + + Session reader = repository.login(); + javax.jcr.Node readerNode = (javax.jcr.Node)reader.getItem("/" + lockedNodeName); + assertThat(readerNode.isLocked(), is(true)); + + // No locks should have changed yet. + repository.cleanUpLocks(); + assertThat(lockedNode.isLocked(), is(true)); + assertThat(readerNode.isLocked(), is(true)); + + /* + * Simulate the GC cleaning up the session and it being purged from the activeSessions() map. + * This can't really be tested in a consistent way due to a lack of specificity around when + * the garbage collector runs. The @Ignored test above does cause a GC sweep on by computer and + * confirms that the code works in principle. A different chicken dance may be required to + * fully test this on a different computer. + */ + repository.activeSessions.remove(locker); + Thread.sleep(JcrEngine.LOCK_EXTENSION_INTERVAL_IN_MILLIS + 100); + + // The locker thread should be inactive and the lock cleaned up + repository.cleanUpLocks(); + assertThat(readerNode.isLocked(), is(false)); + } + } Modified: trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/WorkspaceLockManagerTest.java =================================================================== --- trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/WorkspaceLockManagerTest.java 2009-12-07 15:19:53 UTC (rev 1413) +++ trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/WorkspaceLockManagerTest.java 2009-12-07 20:56:18 UTC (rev 1414) @@ -151,9 +151,7 @@ @Test public void shouldCreateLockRequestWhenUnlockingNode() { DnaLock lock = workspaceLockManager.createLock("testOwner", UUID.randomUUID(), validUuid, false, false); - JcrSession session = mock(JcrSession.class); - stub(session.getExecutionContext()).toReturn(context); - workspaceLockManager.unlockNodeInRepository(session, lock); + workspaceLockManager.unlockNodeInRepository(context, lock); assertNextRequestIsUnlock(validLocation); }