jackrabbit-oak-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From chet...@apache.org
Subject svn commit: r1517503 - in /jackrabbit/oak/trunk/oak-jcr/src: main/java/org/apache/jackrabbit/oak/jcr/ main/java/org/apache/jackrabbit/oak/jcr/delegate/ main/java/org/apache/jackrabbit/oak/jcr/operation/ test/java/org/apache/jackrabbit/oak/jcr/
Date Mon, 26 Aug 2013 11:39:24 GMT
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();



Mime
View raw message