Author: chetanm
Date: Mon Aug 26 11:39:24 2013
New Revision: 1517503
URL: http://svn.apache.org/r1517503
Log:
OAK-960 - Provide an interceptor for SessionOperations
Replacing the use of interceptor with a simple threadLocal managed within the
SessionDelegate itself.
Thanks to Michale Duerig for simplified design!!
Removed:
jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/SessionOperationInterceptor.java
Modified:
jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/RepositoryImpl.java
jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/SessionImpl.java
jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/SessionDelegate.java
jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/operation/SessionOperation.java
jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/RepositoryTest.java
Modified:
jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/RepositoryImpl.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/RepositoryImpl.java?rev=1517503&r1=1517502&r2=1517503&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/RepositoryImpl.java
(original)
+++
jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/RepositoryImpl.java
Mon Aug 26 11:39:24 2013
@@ -205,7 +205,7 @@ public class RepositoryImpl implements J
ContentSession contentSession =
contentRepository.login(credentials, workspaceName);
SessionContext context = createSessionContext(
Collections.<String, Object>singletonMap(REFRESH_INTERVAL,
refreshInterval),
- createSessionDelegate(contentSession, securityProvider,
refreshInterval));
+ new
SessionDelegate(contentSession,securityProvider,refreshInterval));
return context.getSession();
} catch (LoginException e) {
throw new javax.jcr.LoginException(e.getMessage(), e);
@@ -233,18 +233,6 @@ public class RepositoryImpl implements J
return new SessionContext(this, whiteboard, attributes, delegate);
}
- /**
- * Factory method for creating a {@link SessionDelegate} instance for
- * a new session. Called by {@link #login()}. Can be overridden by
- * subclasses to customize the SessionDelegate implementation.
- *
- * @return session context
- */
- protected SessionDelegate createSessionDelegate(ContentSession
contentSession, SecurityProvider securityProvider,
- Long refreshInterval) {
- return new
SessionDelegate(contentSession,securityProvider,refreshInterval);
- }
-
SecurityProvider getSecurityProvider() {
return securityProvider;
}
@@ -272,15 +260,6 @@ public class RepositoryImpl implements J
return toLong(attributes.get(REFRESH_INTERVAL));
}
- private static String getString(Map<String, Object> attributes, String
name) {
- Object value = attributes.get(name);
- if (value instanceof String) {
- return (String) value;
- } else {
- return null;
- }
- }
-
private static Long toLong(Object value) {
if (value instanceof Long) {
return (Long) value;
Modified:
jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/SessionImpl.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/SessionImpl.java?rev=1517503&r1=1517502&r2=1517503&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/SessionImpl.java
(original)
+++
jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/SessionImpl.java
Mon Aug 26 11:39:24 2013
@@ -292,7 +292,7 @@ public class SessionImpl implements Jack
private Node getNodeById(final String id) throws RepositoryException {
return perform(new ReadOperation<Node>() {
@Override
- public Node perform() throws ItemNotFoundException,
RepositoryException {
+ public Node perform() throws RepositoryException {
NodeDelegate nd = sd.getNodeByIdentifier(id);
if (nd == null) {
throw new ItemNotFoundException("Node with id " + id + "
does not exist.");
@@ -394,6 +394,11 @@ public class SessionImpl implements Jack
sd.save();
return null;
}
+
+ @Override
+ public boolean isSave() {
+ return true;
+ }
});
}
@@ -692,4 +697,4 @@ public class SessionImpl implements Jack
public String toString() {
return sd.getContentSession().toString();
}
-}
\ No newline at end of file
+}
Modified:
jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/SessionDelegate.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/SessionDelegate.java?rev=1517503&r1=1517502&r2=1517503&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/SessionDelegate.java
(original)
+++
jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/SessionDelegate.java
Mon Aug 26 11:39:24 2013
@@ -53,13 +53,29 @@ import org.slf4j.LoggerFactory;
public class SessionDelegate {
static final Logger log = LoggerFactory.getLogger(SessionDelegate.class);
+ /**
+ * Threadlocal instance to keep track of the save operations performed in
the thread so far
+ * This is is then used to determine if the current session needs to be
refreshed to see the
+ * changes done by another session in current thread.
+ *
+ * <p><b>Note</b> - This thread local is never cleared. However we are
only storing java.lang.Integer
+ * in it thus it would not cause leaks typically associated with usage of
thread locals which are not
+ * cleared
+ * </p>
+ */
+ private static final ThreadLocal<Integer> SAVE_COUNT = new
ThreadLocal<Integer>() {
+ @Override
+ protected Integer initialValue() {
+ return 0;
+ }
+ };
+
private final ContentSession contentSession;
private final long refreshInterval;
private final Root root;
private final IdentifierManager idManager;
private final Exception initStackTrace;
private final PermissionProvider permissionProvider;
- private final SessionOperationInterceptor interceptor;
private boolean isAlive = true;
private int sessionOpCount;
@@ -69,11 +85,7 @@ public class SessionDelegate {
private boolean warnIfIdle = true;
private boolean refreshAtNextAccess = false;
-
- public SessionDelegate(@Nonnull ContentSession contentSession,
SecurityProvider securityProvider,
- long refreshInterval) {
- this(contentSession, securityProvider,
SessionOperationInterceptor.NOOP,refreshInterval);
- }
+ private int saveCount = SAVE_COUNT.get();
/**
* Create a new session delegate for a {@code ContentSession}. The refresh
behaviour of the
@@ -87,10 +99,8 @@ public class SessionDelegate {
* @param securityProvider the security provider
* @param refreshInterval refresh interval in seconds.
*/
- public SessionDelegate(@Nonnull ContentSession contentSession,
SecurityProvider securityProvider,
- SessionOperationInterceptor interceptor,long
refreshInterval) {
+ public SessionDelegate(@Nonnull ContentSession contentSession,
SecurityProvider securityProvider,long refreshInterval) {
this.contentSession = checkNotNull(contentSession);
- this.interceptor = checkNotNull(interceptor);
this.refreshInterval = MILLISECONDS.convert(refreshInterval, SECONDS);
this.root = contentSession.getLatestRoot();
this.idManager = new IdentifierManager(root);
@@ -120,7 +130,6 @@ public class SessionDelegate {
throws RepositoryException {
// Synchronize to avoid conflicting refreshes from concurrent JCR API
calls
if (sessionOpCount == 0) {
- interceptor.before(this,sessionOperation);
// Refresh and checks only for non re-entrant session operations
long now = System.currentTimeMillis();
long timeElapsed = now - lastAccessed;
@@ -134,9 +143,10 @@ public class SessionDelegate {
" refresh the session.", initStackTrace);
warnIfIdle = false;
}
- if (refreshAtNextAccess || timeElapsed >= refreshInterval) {
+ if (refreshAtNextAccess ||
commitDoneByOtherSessionInCurrentThread() || timeElapsed >= refreshInterval) {
// Refresh if forced or if the session has been idle too
long
refreshAtNextAccess = false;
+ saveCount = SAVE_COUNT.get();
refresh(true);
updateCount++;
}
@@ -152,7 +162,9 @@ public class SessionDelegate {
if (sessionOperation.isUpdate()) {
updateCount++;
}
- interceptor.after(this,sessionOperation);
+ if (sessionOperation.isSave()) {
+ SAVE_COUNT.set(saveCount = (SAVE_COUNT.get() + 1));
+ }
}
}
@@ -450,6 +462,14 @@ public class SessionDelegate {
//-----------------------------------------------------------< internal >---
+ private boolean commitDoneByOtherSessionInCurrentThread() {
+ //if the threadLocal counter differs from our seen saveCount so far
then
+ //some other session would have done a commit. If that is the case a
refresh would
+ //be required
+ return SAVE_COUNT.get() != saveCount;
+ }
+
+
/**
* Wraps the given {@link CommitFailedException} instance using the
* appropriate {@link RepositoryException} subclass based on the
Modified:
jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/operation/SessionOperation.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/operation/SessionOperation.java?rev=1517503&r1=1517502&r2=1517503&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/operation/SessionOperation.java
(original)
+++
jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/operation/SessionOperation.java
Mon Aug 26 11:39:24 2013
@@ -52,6 +52,10 @@ public abstract class SessionOperation<T
return false;
}
+ public boolean isSave() {
+ return false;
+ }
+
public void checkPreconditions() throws RepositoryException {
}
Modified:
jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/RepositoryTest.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/RepositoryTest.java?rev=1517503&r1=1517502&r2=1517503&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/RepositoryTest.java
(original)
+++
jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/RepositoryTest.java
Mon Aug 26 11:39:24 2013
@@ -1352,11 +1352,11 @@ public class RepositoryTest extends Abst
session1.save();
- // Make sure they are still not accessible through another session
- assertFalse(session2.itemExists("/node1"));
- assertFalse(session2.itemExists("/node1/node2"));
- assertFalse(session2.itemExists("/node1/node3"));
- assertFalse(session2.itemExists("/node1/node3/property1"));
+ // Make sure they are accessible through another session
+ assertTrue(session2.itemExists("/node1"));
+ assertTrue(session2.itemExists("/node1/node2"));
+ assertTrue(session2.itemExists("/node1/node3"));
+ assertTrue(session2.itemExists("/node1/node3/property1"));
session2.refresh(false);
@@ -1465,7 +1465,7 @@ public class RepositoryTest extends Abst
session1.save();
session2.save();
assertTrue(session1.getRootNode().hasNode("node1"));
- assertFalse(session1.getRootNode().hasNode("node2")); // was not
visible during save
+ assertTrue(session1.getRootNode().hasNode("node2"));
assertTrue(session2.getRootNode().hasNode("node1")); // save
refreshes
assertTrue(session2.getRootNode().hasNode("node2"));
} finally {
@@ -1475,6 +1475,23 @@ public class RepositoryTest extends Abst
}
@Test
+ public void inThreadSessionSynchronisation() throws RepositoryException {
+ Session session1 = createAdminSession();
+ Session session2 = createAdminSession();
+ Session session3 = createAdminSession();
+ try {
+ session1.getRootNode().addNode("newNode");
+ session1.save();
+ assertTrue(session2.nodeExists("/newNode"));
+ assertTrue(session3.nodeExists("/newNode"));
+ } finally {
+ session1.logout();
+ session2.logout();
+ session3.logout();
+ }
+ }
+
+ @Test
public void saveRefreshConflict() throws RepositoryException {
Session session1 = createAdminSession();
Session session2 = createAdminSession();
@@ -1519,7 +1536,7 @@ public class RepositoryTest extends Abst
session1.save();
assertFalse(session1.getRootNode().hasNode("node"));
- assertTrue(session2.getRootNode().hasNode("node"));
+ assertFalse(session2.getRootNode().hasNode("node"));
try {
session2.save();
