Mailing-List: contact oak-commits-help@jackrabbit.apache.org; run by ezmlm
Precedence: bulk
Reply-To: oak-dev@jackrabbit.apache.org
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r1455173 - in /jackrabbit/oak/trunk/oak-core/src:
 main/java/org/apache/jackrabbit/oak/api/
 main/java/org/apache/jackrabbit/oak/core/
 test/java/org/apache/jackrabbit/oak/core/
Date: Mon, 11 Mar 2013 15:15:02 -0000
To: oak-commits@jackrabbit.apache.org
From: mduerig@apache.org
Message-Id: <20130311151502.BD9DA2388A6C@eris.apache.org>

Author: mduerig
Date: Mon Mar 11 15:15:01 2013
New Revision: 1455173

URL: http://svn.apache.org/r1455173
Log:
OAK-690: Enforce and clarify Root contract wrt. invalid Tree instances
Initial implementation of enforcement. Currently disabled until all follow up issues this is causing are fixed. Use -DOAK-690=true to enable

Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/api/Root.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/api/Tree.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/RootImpl.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/TreeImpl.java
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/core/RootImplTest.java
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/core/TreeImplTest.java

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/api/Root.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/api/Root.java?rev=1455173&r1=1455172&r2=1455173&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/api/Root.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/api/Root.java Mon Mar 11 15:15:01 2013
@@ -23,13 +23,18 @@ import javax.annotation.Nonnull;
 
 /**
  * The root of a {@link Tree}.
- * <p>
+ * <p/>
  * The data returned by this class filtered for the access rights that are set
  * in the {@link ContentSession} that created this object.
- * <p>
+ * <p/>
  * All root instances created by a content session become invalid after the
  * content session is closed. Any method called on an invalid root instance
  * will throw an {@code InvalidStateException}.
+ * <p/>
+ * All {@link Tree} instances acquired through a root become invalid upon call of
+ * {@link #refresh()}, {@link #rebase()} or {@link #commit()}. Any access to invalid
+ * tree instances - except for hierarchy related methods - will cause an
+ * {@code InvalidStateException}.
  */
 public interface Root {
 

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/api/Tree.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/api/Tree.java?rev=1455173&r1=1455172&r2=1455173&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/api/Tree.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/api/Tree.java Mon Mar 11 15:15:01 2013
@@ -55,6 +55,12 @@ import javax.annotation.Nonnull;
  * All tree instances created in the context of a content session become invalid
  * after the content session is closed. Any method called on an invalid tree instance
  * will throw an {@code InvalidStateException}.
+ * <p/>
+ * All {@link Tree} instances acquired through a root become invalid upon call of
+ * {@link Root#refresh()}, {@link Root#rebase()} or {@link Root#commit()}. Any
+ * access to invalid tree instances - except for hierarchy related methods - will cause
+ * an {@code InvalidStateException}. The hierarchy related methods are {@link #getName()},
+ * {@link #isRoot()}, {@link #getPath()}, {@link #getParent()} and {@link #getStatus()}.
  */
 public interface Tree {
 

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/RootImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/RootImpl.java?rev=1455173&r1=1455172&r2=1455173&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/RootImpl.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/RootImpl.java Mon Mar 11 15:15:01 2013
@@ -68,6 +68,12 @@ import static org.apache.jackrabbit.oak.
 public class RootImpl implements Root {
 
     /**
+     * Disable checks for invalid trees.
+     * FIXME: remove once OAK-690 and dependencies are fixed
+     */
+    static final boolean OAK_690 = Boolean.getBoolean("OAK-690");
+
+    /**
      * Number of {@link #updated} calls for which changes are kept in memory.
      */
     private static final int PURGE_LIMIT = Integer.getInteger("oak.root.purgeLimit", 100);
@@ -242,6 +248,11 @@ public class RootImpl implements Root {
     public final void refresh() {
         checkLive();
         branch = store.branch();
+
+        // Disconnect all children -> access to now invalid trees fails fast
+        if (OAK_690) {
+            rootTree.getNodeBuilder().reset(MemoryNodeState.EMPTY_NODE);
+        }
         rootTree = new TreeImpl(this, lastMove);
         modCount = 0;
         if (permissionProvider != null) {

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/TreeImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/TreeImpl.java?rev=1455173&r1=1455172&r2=1455173&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/TreeImpl.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/TreeImpl.java Mon Mar 11 15:15:01 2013
@@ -47,6 +47,7 @@ import org.apache.jackrabbit.oak.spi.sta
 import static com.google.common.base.Objects.toStringHelper;
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.base.Preconditions.checkState;
 import static org.apache.jackrabbit.oak.api.Type.STRING;
 import static org.apache.jackrabbit.oak.commons.PathUtils.elements;
 
@@ -109,25 +110,25 @@ public class TreeImpl implements Tree {
 
     @Override
     public String getName() {
-        enter();
+        enterNoStateCheck();
         return name;
     }
 
     @Override
     public boolean isRoot() {
-        enter();
+        enterNoStateCheck();
         return parent == null;
     }
 
     @Override
     public String getPath() {
-        enter();
+        enterNoStateCheck();
         return getPathInternal();
     }
 
     @Override
     public Tree getParent() {
-        enter();
+        enterNoStateCheck();
         if (parent != null && canRead(parent)) {
             return parent;
         } else {
@@ -229,7 +230,7 @@ public class TreeImpl implements Tree {
 
     @Override
     public Status getStatus() {
-        enter();
+        enterNoStateCheck();
 
         if (isDisconnected()) {
             return Status.DISCONNECTED;
@@ -527,6 +528,14 @@ public class TreeImpl implements Tree {
 
     private void enter() {
         root.checkLive();
+        if (RootImpl.OAK_690) {
+            checkState(parent != null || this == root.getTree("/"), "Tree access after commit, refresh or rebase");
+        }
+        applyPendingMoves();
+    }
+
+    private void enterNoStateCheck() {
+        root.checkLive();
         applyPendingMoves();
     }
 

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/core/RootImplTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/core/RootImplTest.java?rev=1455173&r1=1455172&r2=1455173&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/core/RootImplTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/core/RootImplTest.java Mon Mar 11 15:15:01 2013
@@ -170,6 +170,7 @@ public class RootImplTest {
         assertFalse(r.hasChild("b"));
 
         root.commit();
+        r = root.getTree("/");
         assertFalse(r.hasChild("a"));
         assertFalse(r.hasChild("b"));
     }

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/core/TreeImplTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/core/TreeImplTest.java?rev=1455173&r1=1455172&r2=1455173&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/core/TreeImplTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/core/TreeImplTest.java Mon Mar 11 15:15:01 2013
@@ -31,6 +31,7 @@ import org.apache.jackrabbit.oak.api.Tre
 import org.apache.jackrabbit.oak.plugins.memory.LongPropertyState;
 import org.junit.After;
 import org.junit.Before;
+import org.junit.Ignore;
 import org.junit.Test;
 
 import static org.apache.jackrabbit.oak.api.Type.LONG;
@@ -40,6 +41,7 @@ import static org.junit.Assert.assertFal
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 /**
  * TreeImplTest...
@@ -386,10 +388,12 @@ public class TreeImplTest {
 
         tree.setOrderableChildren(true);
         root.commit();
+        tree = root.getTree("/").addChild("test");
         assertNotNull(((TreeImpl) tree).getNodeState().getProperty(TreeImpl.OAK_CHILD_ORDER));
 
         tree.setOrderableChildren(false);
         root.commit();
+        tree = root.getTree("/").addChild("test");
         assertNull(((TreeImpl) tree).getNodeState().getProperty(TreeImpl.OAK_CHILD_ORDER));
     }
 
@@ -408,4 +412,33 @@ public class TreeImplTest {
             assertEquals(childNames[index++], child.getName());
         }
     }
+
+    @Test
+    @Ignore("OAK-690")  // FIXME enable once OAK-690 is fixed
+    public void testInvalidTreeAccess() throws CommitFailedException {
+        Tree r = root.getTree("/");
+        Tree x = root.getTree("/x");
+
+        root.refresh();
+
+        try {
+            r.getChild("any");
+            fail("Expected IllegalStateException");
+        } catch (IllegalStateException expected) {}
+
+        try {
+            x.getChild("any");
+            fail("Expected IllegalStateException");
+        } catch (IllegalStateException expected) {}
+
+        try {
+            r.addChild("any");
+            fail("Expected IllegalStateException");
+        } catch (IllegalStateException expected) {}
+
+        try {
+            x.addChild("any");
+            fail("Expected IllegalStateException");
+        } catch (IllegalStateException expected) {}
+    }
 }
\ No newline at end of file