You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by md...@apache.org on 2013/03/11 16:15:02 UTC
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/
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