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/28 14:37:20 UTC
svn commit: r1462088 - in /jackrabbit/oak/trunk:
oak-core/src/main/java/org/apache/jackrabbit/oak/api/
oak-core/src/main/java/org/apache/jackrabbit/oak/core/
oak-core/src/test/java/org/apache/jackrabbit/oak/api/
oak-core/src/test/java/org/apache/jackra...
Author: mduerig
Date: Thu Mar 28 13:37:19 2013
New Revision: 1462088
URL: http://svn.apache.org/r1462088
Log:
OAK-690: Enforce and clarify Root contract wrt. invalid Tree instances
Change Tree/Root contract such that Tree instances stay valid across save/refresh/rebase operations
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/api/RootTest.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/api/TreeTest.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
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserManagerImplTest.java
jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/ItemDelegate.java
jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/SessionDelegate.java
jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/MoveRemoveTest.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=1462088&r1=1462087&r2=1462088&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 Thu Mar 28 13:37:19 2013
@@ -31,9 +31,10 @@ import javax.annotation.Nonnull;
* 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
+ * {@link Tree} instances may become disconnected after a call to {@link #refresh()},
+ * {@link #rebase()} or {@link #commit()}. Any access to disconnected tree instances
+ * - except for {@link Tree#getName()}, {@link Tree#isRoot()}, {@link Tree#getPath()},
+ * {@link Tree#getParent()} and {@link Tree#getStatus()} - will cause an
* {@code InvalidStateException}.
*/
public interface Root {
@@ -101,15 +102,14 @@ public interface Root {
/**
* Rebase this root instance to the latest revision. After a call to this method,
- * all trees obtained through {@link #getTree(String)} become invalid and fresh
- * instances must be obtained.
+ * trees obtained through {@link #getTree(String)} may become disconnected.
*/
void rebase();
/**
* Reverts all changes made to this root and refreshed to the latest trunk.
- * After a call to this method, all trees obtained through {@link #getTree(String)}
- * become invalid and fresh instances must be obtained.
+ * After a call to this method, trees obtained through {@link #getTree(String)}
+ * may become disconnected.
*/
void refresh();
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=1462088&r1=1462087&r2=1462088&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 Thu Mar 28 13:37:19 2013
@@ -57,11 +57,11 @@ import javax.annotation.Nullable;
* 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()}.
+ * {@link Tree} instances may become disconnected after a call to {@link Root#refresh()},
+ * {@link Root#rebase()} or {@link Root#commit()}. Any access to disconnected tree instances
+ * - except for {@link Tree#getName()}, {@link Tree#isRoot()}, {@link Tree#getPath()},
+ * {@link Tree#getParent()} and {@link Tree#getStatus()} - will cause an
+ * {@code InvalidStateException}.
*/
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=1462088&r1=1462087&r2=1462088&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 Thu Mar 28 13:37:19 2013
@@ -18,6 +18,11 @@
*/
package org.apache.jackrabbit.oak.core;
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkNotNull;
+import static org.apache.jackrabbit.oak.commons.PathUtils.getName;
+import static org.apache.jackrabbit.oak.commons.PathUtils.getParentPath;
+
import java.io.IOException;
import java.io.InputStream;
import java.security.PrivilegedAction;
@@ -60,21 +65,9 @@ import org.apache.jackrabbit.oak.spi.sta
import org.apache.jackrabbit.oak.spi.state.NodeStore;
import org.apache.jackrabbit.oak.spi.state.NodeStoreBranch;
-import static com.google.common.base.Preconditions.checkArgument;
-import static com.google.common.base.Preconditions.checkNotNull;
-import static org.apache.jackrabbit.oak.commons.PathUtils.getName;
-import static org.apache.jackrabbit.oak.commons.PathUtils.getParentPath;
-import static org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState.EMPTY_NODE;
-
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);
@@ -244,7 +237,7 @@ public class RootImpl implements Root {
if (!store.getRoot().equals(rootTree.getBaseState())) {
purgePendingChanges();
branch.rebase();
- rootTree = new TreeImpl(this, lastMove);
+ rootTree.reset(branch.getHead());
permissionProvider = null;
}
}
@@ -253,12 +246,7 @@ 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(EMPTY_NODE);
- }
- rootTree = new TreeImpl(this, lastMove);
+ rootTree.reset(branch.getHead());
modCount = 0;
if (permissionProvider != null) {
permissionProvider.refresh();
@@ -500,5 +488,9 @@ public class RootImpl implements Root {
return move;
}
+ @Override
+ public String toString() {
+ return '>' + source + ':' + PathUtils.concat(destParent.getPathInternal(), destName);
+ }
}
}
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=1462088&r1=1462087&r2=1462088&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 Thu Mar 28 13:37:19 2013
@@ -18,6 +18,13 @@
*/
package org.apache.jackrabbit.oak.core;
+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;
+
import java.util.Collections;
import java.util.Iterator;
import java.util.Set;
@@ -45,13 +52,6 @@ import org.apache.jackrabbit.oak.spi.sta
import org.apache.jackrabbit.oak.spi.state.NodeStateUtils;
import org.apache.jackrabbit.oak.spi.state.PropertyBuilder;
-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;
-
public class TreeImpl implements Tree {
/**
@@ -67,7 +67,7 @@ public class TreeImpl implements Tree {
/**
* The node state this tree is based on. {@code null} if this is a newly added tree.
*/
- private final NodeState baseState;
+ private NodeState baseState;
/**
* The {@code NodeBuilder} for the underlying node state
@@ -154,32 +154,32 @@ public class TreeImpl implements Tree {
@Override
public Status getPropertyStatus(String name) {
// TODO: see OAK-212
- enter();
Status nodeStatus = getStatus();
+ if (nodeStatus == Status.DISCONNECTED) {
+ return Status.DISCONNECTED;
+ }
if (nodeStatus == Status.NEW) {
return (hasProperty(name)) ? Status.NEW : null;
- } else if (nodeStatus == Status.DISCONNECTED) {
- return Status.DISCONNECTED;
- } else {
- PropertyState head = internalGetProperty(name);
- if (head != null && !canRead(head)) {
- // no permission to read status information for existing property
- return null;
- }
+ }
+ PropertyState head = internalGetProperty(name);
+ if (head != null && !canRead(head)) {
+ // no permission to read status information for existing property
+ return null;
+ }
- NodeState parentBase = getBaseState();
- PropertyState base = parentBase == null ? null : parentBase.getProperty(name);
- if (head == null) {
- return (base == null) ? null : Status.DISCONNECTED;
- } else {
- if (base == null) {
- return Status.NEW;
- } else if (head.equals(base)) {
- return Status.EXISTING;
- } else {
- return Status.MODIFIED;
- }
- }
+ NodeState parentBase = nodeBuilder.getBaseState();
+ PropertyState base = parentBase == null ? null : parentBase.getProperty(name);
+
+ if (head == null) {
+ return (base == null) ? null : Status.DISCONNECTED;
+ }
+
+ if (base == null) {
+ return Status.NEW;
+ } else if (head.equals(base)) {
+ return Status.EXISTING;
+ } else {
+ return Status.MODIFIED;
}
}
@@ -220,16 +220,25 @@ public class TreeImpl implements Tree {
}
private boolean isDisconnected() {
- if (isRoot()) {
- return false;
- }
- if (parent.nodeBuilder == null) {
+ if (parent == null || nodeBuilder.isConnected()) {
return false;
}
- if (!parent.nodeBuilder.isConnected()) {
- return true;
+
+ return !reconnect();
+ }
+
+ private boolean reconnect() {
+ if (parent != null && parent.reconnect()) {
+ if (parent.nodeBuilder.hasChildNode(name)) {
+ nodeBuilder = parent.nodeBuilder.child(name);
+ }
+ else {
+ // make this builder disconnected from its new parent
+ nodeBuilder = parent.nodeBuilder.child(name);
+ parent.nodeBuilder.removeNode(name);
+ }
}
- return !nodeBuilder.isConnected();
+ return nodeBuilder.isConnected();
}
@Override
@@ -451,15 +460,17 @@ public class TreeImpl implements Tree {
void moveTo(TreeImpl destParent, String destName) {
name = destName;
parent = destParent;
- if (!parent.isDisconnected()) {
- if (parent.nodeBuilder.hasChildNode(name)) {
- nodeBuilder = parent.nodeBuilder.child(name);
- } else {
- // make this builder disconnected from its new parent
- nodeBuilder = parent.nodeBuilder.child(name);
- parent.nodeBuilder.removeNode(name);
- }
- }
+ }
+
+ /**
+ * Reset this (root) tree instance's underlying node state to the passed {@code state}.
+ * @param state
+ * @throws IllegalStateException if {@code isRoot()} is {@code false}.
+ */
+ void reset(NodeState state) {
+ checkState(parent == null);
+ nodeBuilder.reset(state);
+ baseState = state;
}
/**
@@ -532,9 +543,7 @@ 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");
- }
+ checkState(!isDisconnected());
applyPendingMoves();
}
Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/api/RootTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/api/RootTest.java?rev=1462088&r1=1462087&r2=1462088&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/api/RootTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/api/RootTest.java Thu Mar 28 13:37:19 2013
@@ -18,6 +18,8 @@
*/
package org.apache.jackrabbit.oak.api;
+import static org.apache.jackrabbit.oak.OakAssert.assertSequence;
+
import org.apache.jackrabbit.oak.Oak;
import org.apache.jackrabbit.oak.plugins.commit.ConflictValidator;
import org.apache.jackrabbit.oak.plugins.commit.JcrConflictHandler;
@@ -25,8 +27,6 @@ import org.junit.After;
import org.junit.Before;
import org.junit.Test;
-import static org.apache.jackrabbit.oak.OakAssert.assertSequence;
-
/**
* Contains tests related to {@link Root}
*/
@@ -60,10 +60,8 @@ public class RootTest {
r.commit();
r.copy("/node3", "/c/node3");
- c = r.getTree("/").getChild("c");
assertSequence(c.getChildren(), "node1", "node2", "node3");
r.commit();
- c = r.getTree("/").getChild("c");
assertSequence(c.getChildren(), "node1", "node2", "node3");
} finally {
s.close();
@@ -83,10 +81,8 @@ public class RootTest {
r.commit();
r.move("/node3", "/c/node3");
- c = r.getTree("/").getChild("c");
assertSequence(c.getChildren(), "node1", "node2", "node3");
r.commit();
- c = r.getTree("/").getChild("c");
assertSequence(c.getChildren(), "node1", "node2", "node3");
} finally {
s.close();
Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/api/TreeTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/api/TreeTest.java?rev=1462088&r1=1462087&r2=1462088&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/api/TreeTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/api/TreeTest.java Thu Mar 28 13:37:19 2013
@@ -18,6 +18,11 @@
*/
package org.apache.jackrabbit.oak.api;
+import static org.apache.jackrabbit.oak.OakAssert.assertSequence;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
import java.util.Set;
import com.google.common.collect.Sets;
@@ -31,11 +36,6 @@ import org.junit.After;
import org.junit.Before;
import org.junit.Test;
-import static org.apache.jackrabbit.oak.OakAssert.assertSequence;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
-
/**
* Contains tests related to {@link Tree}
*/
@@ -78,25 +78,22 @@ public class TreeTest {
t.addChild("node2");
t.addChild("node3");
r.commit();
- t = r.getTree("/");
+
t.getChild("node1").orderBefore("node2");
t.getChild("node3").orderBefore(null);
assertSequence(t.getChildren(), "node1", "node2", "node3");
r.commit();
// check again after commit
- t = r.getTree("/");
assertSequence(t.getChildren(), "node1", "node2", "node3");
t.getChild("node3").orderBefore("node2");
assertSequence(t.getChildren(), "node1", "node3", "node2");
r.commit();
- t = r.getTree("/");
assertSequence(t.getChildren(), "node1", "node3", "node2");
t.getChild("node1").orderBefore(null);
assertSequence(t.getChildren(), "node3", "node2", "node1");
r.commit();
- t = r.getTree("/");
assertSequence(t.getChildren(), "node3", "node2", "node1");
// :childOrder property invisible?
@@ -117,7 +114,6 @@ public class TreeTest {
t1.addChild("node2");
t1.addChild("node3");
r1.commit();
- t1 = r1.getTree("/");
ContentSession s2 = repository.login(null, null);
try {
@@ -127,13 +123,11 @@ public class TreeTest {
t1.getChild("node2").orderBefore("node1");
t1.getChild("node3").orderBefore(null);
r1.commit();
- t1 = r1.getTree("/");
assertSequence(t1.getChildren(), "node2", "node1", "node3");
t2.getChild("node3").orderBefore("node1");
t2.getChild("node2").orderBefore(null);
r2.commit();
- t2 = r2.getTree("/");
// other session wins
assertSequence(t2.getChildren(), "node2", "node1", "node3");
@@ -141,7 +135,6 @@ public class TreeTest {
t2.getChild("node3").orderBefore("node1");
t2.getChild("node2").orderBefore(null);
r2.commit();
- t2 = r2.getTree("/");
assertSequence(t2.getChildren(), "node3", "node1", "node2");
} finally {
@@ -162,7 +155,6 @@ public class TreeTest {
t1.addChild("node2");
t1.addChild("node3");
r1.commit();
- t1 = r1.getTree("/");
ContentSession s2 = repository.login(null, null);
try {
@@ -173,19 +165,16 @@ public class TreeTest {
t1.getChild("node3").orderBefore(null);
t1.addChild("node4");
r1.commit();
- t1 = r1.getTree("/");
assertSequence(t1.getChildren(), "node2", "node1", "node3", "node4");
t2.getChild("node3").orderBefore("node1");
r2.commit();
- t2 = r2.getTree("/");
// other session wins
assertSequence(t2.getChildren(), "node2", "node1", "node3", "node4");
// try again on current root
t2.getChild("node3").orderBefore("node1");
r2.commit();
- t2 = r2.getTree("/");
assertSequence(t2.getChildren(), "node2", "node3", "node1", "node4");
} finally {
@@ -207,7 +196,6 @@ public class TreeTest {
t1.addChild("node3");
t1.addChild("node4");
r1.commit();
- t1 = r1.getTree("/");
ContentSession s2 = repository.login(null, null);
try {
@@ -218,19 +206,16 @@ public class TreeTest {
t1.getChild("node3").orderBefore(null);
t1.getChild("node4").remove();
r1.commit();
- t1 = r1.getTree("/");
assertSequence(t1.getChildren(), "node2", "node1", "node3");
t2.getChild("node3").orderBefore("node1");
r2.commit();
- t2 = r2.getTree("/");
// other session wins
assertSequence(t2.getChildren(), "node2", "node1", "node3");
// try again on current root
t2.getChild("node3").orderBefore("node1");
r2.commit();
- t2 = r2.getTree("/");
assertSequence(t2.getChildren(), "node2", "node3", "node1");
} finally {
@@ -252,7 +237,6 @@ public class TreeTest {
t1.addChild("node3");
t1.addChild("node4");
r1.commit();
- t1 = r1.getTree("/");
ContentSession s2 = repository.login(null, null);
try {
@@ -262,20 +246,17 @@ public class TreeTest {
t1.getChild("node2").orderBefore("node1");
t1.getChild("node3").orderBefore(null);
r1.commit();
- t1 = r1.getTree("/");
assertSequence(t1.getChildren(), "node2", "node1", "node4", "node3");
t2.getChild("node3").orderBefore("node1");
t2.getChild("node4").remove();
r2.commit();
- t2 = r2.getTree("/");
// other session wins wrt ordering, but node4 is gone
assertSequence(t2.getChildren(), "node2", "node1", "node3");
// try reorder again on current root
t2.getChild("node3").orderBefore("node1");
r2.commit();
- t2 = r2.getTree("/");
assertSequence(t2.getChildren(), "node2", "node3", "node1");
} finally {
@@ -296,7 +277,6 @@ public class TreeTest {
t1.addChild("node2");
t1.addChild("node3");
r1.commit();
- t1 = r1.getTree("/");
ContentSession s2 = repository.login(null, null);
try {
@@ -306,12 +286,10 @@ public class TreeTest {
t1.getChild("node2").orderBefore("node1");
t1.getChild("node3").remove();
r1.commit();
- t1 = r1.getTree("/");
assertSequence(t1.getChildren(), "node2", "node1");
t2.getChild("node3").orderBefore("node1");
r2.commit();
- t2 = r2.getTree("/");
assertSequence(t2.getChildren(), "node2", "node1");
} finally {
@@ -332,7 +310,6 @@ public class TreeTest {
t1.addChild("node2");
t1.addChild("node3");
r1.commit();
- t1 = r1.getTree("/c");
ContentSession s2 = repository.login(null, null);
try {
@@ -343,12 +320,10 @@ public class TreeTest {
// now 'c' does not have ordered children anymore
r1.getTree("/").addChild("c");
r1.commit();
- t1 = r1.getTree("/c");
assertSequence(t1.getChildren());
t2.getChild("node3").orderBefore("node1");
r2.commit();
- t2 = r2.getTree("/c");
assertSequence(t2.getChildren());
} finally {
@@ -370,7 +345,6 @@ public class TreeTest {
t1.addChild("node3");
t1.addChild("node4");
r1.commit();
- t1 = r1.getTree("/");
ContentSession s2 = repository.login(null, null);
try {
@@ -380,12 +354,10 @@ public class TreeTest {
t1.getChild("node2").orderBefore("node1");
t1.getChild("node3").remove();
r1.commit();
- t1 = r1.getTree("/");
assertSequence(t1.getChildren(), "node2", "node1", "node4");
t2.getChild("node4").orderBefore("node3");
r2.commit();
- t2 = r2.getTree("/");
assertSequence(t2.getChildren(), "node2", "node1", "node4");
} finally {
@@ -487,7 +459,6 @@ public class TreeTest {
t1 = r1.getTree("/c");
t1.getChild("node2").orderBefore("node1");
r1.commit();
- t1 = r1.getTree("/c");
assertSequence(t1.getChildren(), "node2", "node1");
t2.remove();
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=1462088&r1=1462087&r2=1462088&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 Thu Mar 28 13:37:19 2013
@@ -18,6 +18,14 @@
*/
package org.apache.jackrabbit.oak.core;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNotSame;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
import java.util.ArrayList;
import java.util.List;
@@ -33,14 +41,6 @@ import org.junit.After;
import org.junit.Before;
import org.junit.Test;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNotSame;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
-
public class RootImplTest {
private ContentSession session;
@@ -109,7 +109,6 @@ public class RootImplTest {
assertEquals("/y/xx", x.getPath());
root.commit();
- tree = root.getTree("/");
assertFalse(tree.hasChild("x"));
assertTrue(tree.hasChild("y"));
@@ -151,6 +150,24 @@ public class RootImplTest {
assertNull(tree.getChild("new"));
}
+ @Test
+ public void moveExistingParent() throws CommitFailedException {
+ Root root = session.getLatestRoot();
+ root.getTree("/").addChild("parent").addChild("new");
+ root.commit();
+
+ Tree parent = root.getTree("/parent");
+ Tree n = root.getTree("/parent/new");
+
+ root.move("/parent", "/moved");
+
+ assertEquals(Status.EXISTING, parent.getStatus());
+ assertEquals(Status.EXISTING, n.getStatus());
+
+ assertEquals("/moved", parent.getPath());
+ assertEquals("/moved/new", n.getPath());
+ }
+
/**
* Regression test for OAK-208
*/
@@ -170,7 +187,6 @@ public class RootImplTest {
assertFalse(r.hasChild("b"));
root.commit();
- r = root.getTree("/");
assertFalse(r.hasChild("a"));
assertFalse(r.hasChild("b"));
}
@@ -188,7 +204,6 @@ public class RootImplTest {
assertEquals("/xx", x.getPath());
root.commit();
- tree = root.getTree("/");
assertFalse(tree.hasChild("x"));
assertTrue(tree.hasChild("xx"));
@@ -209,7 +224,6 @@ public class RootImplTest {
assertTrue(y.hasChild("xx"));
root.commit();
- tree = root.getTree("/");
assertTrue(tree.hasChild("x"));
assertTrue(tree.hasChild("y"));
@@ -229,7 +243,6 @@ public class RootImplTest {
assertTrue(y.getChild("xx").hasChild("x1"));
root.commit();
- tree = root.getTree("/");
assertTrue(tree.hasChild("x"));
assertTrue(tree.hasChild("y"));
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=1462088&r1=1462087&r2=1462088&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 Thu Mar 28 13:37:19 2013
@@ -16,6 +16,14 @@
*/
package org.apache.jackrabbit.oak.core;
+import static org.apache.jackrabbit.oak.api.Type.LONG;
+import static org.apache.jackrabbit.oak.api.Type.STRING;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+
import java.util.Collections;
import java.util.HashSet;
import java.util.Set;
@@ -31,18 +39,8 @@ 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;
-import static org.apache.jackrabbit.oak.api.Type.STRING;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-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...
*/
@@ -145,7 +143,6 @@ public class TreeImplTest {
assertTrue(tree.hasChild("new"));
root.commit();
- tree = root.getTree("/");
assertTrue(tree.hasChild("new"));
@@ -161,7 +158,6 @@ public class TreeImplTest {
tree.addChild("new");
root.commit();
- tree = root.getTree("/");
assertTrue(tree.hasChild("new"));
Tree added = tree.addChild("new");
@@ -178,7 +174,6 @@ public class TreeImplTest {
assertFalse(tree.hasChild("x"));
root.commit();
- tree = root.getTree("/");
assertFalse(tree.hasChild("x"));
}
@@ -207,7 +202,6 @@ public class TreeImplTest {
assertEquals("value", property.getValue(STRING));
root.commit();
- tree = root.getTree("/");
property = tree.getProperty("new");
assertNotNull(property);
@@ -224,7 +218,6 @@ public class TreeImplTest {
assertFalse(tree.hasProperty("a"));
root.commit();
- tree = root.getTree("/");
assertFalse(tree.hasProperty("a"));
}
@@ -270,12 +263,12 @@ public class TreeImplTest {
tree.setProperty("P0", "V1");
root.commit();
- tree = root.getTree("/");
+
assertTrue(tree.hasProperty("P0"));
tree.removeProperty("P0");
root.commit();
- tree = root.getTree("/");
+
assertFalse(tree.hasProperty("P0"));
}
@@ -287,26 +280,24 @@ public class TreeImplTest {
assertEquals(Tree.Status.NEW, tree.getChild("new").getStatus());
root.commit();
- tree = root.getTree("/");
assertEquals(Tree.Status.EXISTING, tree.getChild("new").getStatus());
Tree added = tree.getChild("new");
added.addChild("another");
assertEquals(Tree.Status.MODIFIED, tree.getChild("new").getStatus());
root.commit();
- tree = root.getTree("/");
assertEquals(Tree.Status.EXISTING, tree.getChild("new").getStatus());
tree.getChild("new").getChild("another").remove();
assertEquals(Tree.Status.MODIFIED, tree.getChild("new").getStatus());
root.commit();
- tree = root.getTree("/");
assertEquals(Tree.Status.EXISTING, tree.getChild("new").getStatus());
assertNull(tree.getChild("new").getChild("another"));
Tree x = root.getTree("/x");
Tree y = x.addChild("y");
x.remove();
+
assertEquals(Status.DISCONNECTED, x.getStatus());
assertEquals(Status.DISCONNECTED, y.getStatus());
}
@@ -319,24 +310,22 @@ public class TreeImplTest {
assertEquals(Tree.Status.NEW, tree.getPropertyStatus("new"));
root.commit();
- tree = root.getTree("/");
assertEquals(Tree.Status.EXISTING, tree.getPropertyStatus("new"));
tree.setProperty("new", "value2");
assertEquals(Tree.Status.MODIFIED, tree.getPropertyStatus("new"));
root.commit();
- tree = root.getTree("/");
assertEquals(Tree.Status.EXISTING, tree.getPropertyStatus("new"));
tree.removeProperty("new");
assertEquals(Tree.Status.DISCONNECTED, tree.getPropertyStatus("new"));
root.commit();
- tree = root.getTree("/");
assertNull(tree.getPropertyStatus("new"));
Tree x = root.getTree("/x");
x.setProperty("y", "value1");
x.remove();
+
assertEquals(Status.DISCONNECTED, x.getPropertyStatus("y"));
}
@@ -346,7 +335,6 @@ public class TreeImplTest {
tree.addChild("one").addChild("two");
root.commit();
- tree = root.getTree("/");
tree.getChild("one").getChild("two").addChild("three");
assertEquals(Tree.Status.EXISTING, tree.getChild("one").getStatus());
assertEquals(Tree.Status.MODIFIED, tree.getChild("one").getChild("two").getStatus());
@@ -355,22 +343,18 @@ public class TreeImplTest {
@Test
public void largeChildList() throws CommitFailedException {
Tree tree = root.getTree("/");
-
Set<String> added = new HashSet<String>();
- tree.addChild("large");
- tree = tree.getChild("large");
+ Tree large = tree.addChild("large");
for (int c = 0; c < 10000; c++) {
String name = "n" + c;
added.add(name);
- tree.addChild(name);
+ large.addChild(name);
}
root.commit();
- tree = root.getTree("/");
- tree = tree.getChild("large");
- for (Tree child : tree.getChildren()) {
+ for (Tree child : large.getChildren()) {
assertTrue(added.remove(child.getName()));
}
@@ -388,12 +372,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));
}
@@ -414,31 +398,43 @@ public class TreeImplTest {
}
@Test
- @Ignore("OAK-690") // FIXME enable once OAK-690 is fixed
- public void testInvalidTreeAccess() throws CommitFailedException {
- Tree r = root.getTree("/");
+ public void testDisconnectAfterRefresh() {
Tree x = root.getTree("/x");
+ x.setProperty("p", "any");
+ Tree xx = x.addChild("xx");
+ xx.setProperty("q", "any");
+
+ assertEquals(Status.MODIFIED, x.getStatus());
+ assertEquals(Status.NEW, x.getPropertyStatus("p"));
+ assertEquals(Status.NEW, xx.getStatus());
+ assertEquals(Status.NEW, xx.getPropertyStatus("q"));
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) {}
+ assertEquals(Status.EXISTING, x.getStatus());
+ assertNull(x.getPropertyStatus("p"));
+ assertEquals(Status.DISCONNECTED, xx.getStatus());
+ assertEquals(Status.DISCONNECTED, xx.getPropertyStatus("q"));
}
+
+ @Test
+ public void testDisconnectAfterRemove() {
+ Tree x = root.getTree("/x");
+ x.setProperty("p", "any");
+ Tree xx = x.addChild("xx");
+ xx.setProperty("q", "any");
+
+ assertEquals(Status.MODIFIED, x.getStatus());
+ assertEquals(Status.NEW, x.getPropertyStatus("p"));
+ assertEquals(Status.NEW, xx.getStatus());
+ assertEquals(Status.NEW, xx.getPropertyStatus("q"));
+
+ root.getTree("/x").remove();
+
+ assertEquals(Status.DISCONNECTED, x.getStatus());
+ assertEquals(Status.DISCONNECTED, x.getPropertyStatus("p"));
+ assertEquals(Status.DISCONNECTED, xx.getStatus());
+ assertEquals(Status.DISCONNECTED, xx.getPropertyStatus("q"));
+ }
+
}
\ No newline at end of file
Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserManagerImplTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserManagerImplTest.java?rev=1462088&r1=1462087&r2=1462088&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserManagerImplTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserManagerImplTest.java Thu Mar 28 13:37:19 2013
@@ -16,10 +16,18 @@
*/
package org.apache.jackrabbit.oak.security.user;
+import static junit.framework.Assert.assertEquals;
+import static junit.framework.Assert.assertFalse;
+import static junit.framework.Assert.assertNotNull;
+import static junit.framework.Assert.assertTrue;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.fail;
+
import java.security.Principal;
import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
+
import javax.jcr.RepositoryException;
import javax.jcr.UnsupportedRepositoryOperationException;
@@ -38,13 +46,6 @@ import org.junit.After;
import org.junit.Before;
import org.junit.Test;
-import static junit.framework.Assert.assertEquals;
-import static junit.framework.Assert.assertFalse;
-import static junit.framework.Assert.assertNotNull;
-import static junit.framework.Assert.assertTrue;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.fail;
-
/**
* UserManagerImplTest...
*/
@@ -158,28 +159,22 @@ public class UserManagerImplTest extends
NodeUtil folder = userNode.addChild("folder", UserConstants.NT_REP_AUTHORIZABLE_FOLDER);
String path = folder.getTree().getPath();
+
+ // authNode - authFolder -> create User
try {
- // authNode - authFolder -> create User
- try {
- Principal p = new PrincipalImpl("test2");
- Authorizable a = userMgr.createUser(p.getName(), p.getName(), p, path);
- root.commit();
+ Principal p = new PrincipalImpl("test2");
+ Authorizable a = userMgr.createUser(p.getName(), p.getName(), p, path);
+ root.commit();
- fail("Users may not be nested.");
- } catch (CommitFailedException e) {
- // success
- } finally {
- root.refresh();
- Authorizable a = userMgr.getAuthorizable("test2");
- if (a != null) {
- a.remove();
- root.commit();
- }
- }
+ fail("Users may not be nested.");
+ } catch (CommitFailedException e) {
+ // success
} finally {
- root.refresh();
- folder.getTree().remove();
- root.commit();
+ Authorizable a = userMgr.getAuthorizable("test2");
+ if (a != null) {
+ a.remove();
+ root.commit();
+ }
}
NodeUtil someContent = userNode.addChild("mystuff", JcrConstants.NT_UNSTRUCTURED);
@@ -195,7 +190,6 @@ public class UserManagerImplTest extends
} catch (CommitFailedException e) {
// success
} finally {
- root.refresh();
Authorizable a = userMgr.getAuthorizable("test3");
if (a != null) {
a.remove();
Modified: jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/ItemDelegate.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/ItemDelegate.java?rev=1462088&r1=1462087&r2=1462088&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/ItemDelegate.java (original)
+++ jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/ItemDelegate.java Thu Mar 28 13:37:19 2013
@@ -36,19 +36,11 @@ public abstract class ItemDelegate {
protected final SessionDelegate sessionDelegate;
/** The underlying {@link org.apache.jackrabbit.oak.api.TreeLocation} of this item. */
- private TreeLocation location;
-
- /**
- * Revision on which this item is based. The underlying state of the item
- * is re-resolved whenever the revision of the session does not match this
- * revision.
- */
- private int revision;
+ private final TreeLocation location;
ItemDelegate(SessionDelegate sessionDelegate, TreeLocation location) {
this.sessionDelegate = checkNotNull(sessionDelegate);
this.location = checkNotNull(location);
- this.revision = sessionDelegate.getRevision();
}
/**
@@ -90,7 +82,7 @@ public abstract class ItemDelegate {
* @return {@code true} iff stale
*/
public boolean isStale() {
- return !loadLocation().exists();
+ return !location.exists();
}
/**
@@ -99,7 +91,7 @@ public abstract class ItemDelegate {
*/
@CheckForNull
public Status getStatus() {
- return loadLocation().getStatus();
+ return location.getStatus();
}
/**
@@ -109,7 +101,6 @@ public abstract class ItemDelegate {
*/
@Nonnull // FIXME this should be package private. OAK-672
public TreeLocation getLocation() throws InvalidItemStateException {
- TreeLocation location = loadLocation();
if (!location.exists()) {
throw new InvalidItemStateException("Item is stale");
}
@@ -121,21 +112,4 @@ public abstract class ItemDelegate {
return toStringHelper(this).add("location", location).toString();
}
- //------------------------------------------------------------< private >---
-
- /**
- * The underlying {@link org.apache.jackrabbit.oak.api.TreeLocation} of this item.
- * The location is only loaded when the revision of this item does not match
- * the revision of the session or when the location does not exist (anymore).
- * @return tree location of the underlying item.
- */
- @Nonnull
- private synchronized TreeLocation loadLocation() {
- if (sessionDelegate.getRevision() != revision || !location.exists()) {
- location = sessionDelegate.getLocation(location.getPath());
- revision = sessionDelegate.getRevision();
- }
- return location;
- }
-
}
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=1462088&r1=1462087&r2=1462088&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 Thu Mar 28 13:37:19 2013
@@ -50,7 +50,6 @@ public class SessionDelegate {
private boolean isAlive = true;
private int sessionOpCount;
- private int revision;
public SessionDelegate(@Nonnull ContentSession contentSession) {
this.contentSession = checkNotNull(contentSession);
@@ -186,7 +185,6 @@ public class SessionDelegate {
public void save() throws RepositoryException {
try {
root.commit();
- revision++;
} catch (CommitFailedException e) {
e.throwRepositoryException();
}
@@ -198,7 +196,6 @@ public class SessionDelegate {
} else {
root.refresh();
}
- revision++;
}
//----------------------------------------------------------< Workspace >---
@@ -303,16 +300,6 @@ public class SessionDelegate {
}
/**
- * Revision of this session. The revision is incremented each time a session is refreshed or saved.
- * This allows items to determine whether they need to re-resolve their underlying state when the
- * revision on which an item is based does not match the revision of the session any more.
- * @return the current revision of this session
- */
- int getRevision() {
- return revision;
- }
-
- /**
* Get the {@code Tree} with the given path
* @param path oak path
* @return tree at the given path or {@code null} if no such tree exists or
Modified: jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/MoveRemoveTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/MoveRemoveTest.java?rev=1462088&r1=1462087&r2=1462088&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/MoveRemoveTest.java (original)
+++ jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/MoveRemoveTest.java Thu Mar 28 13:37:19 2013
@@ -16,6 +16,9 @@
*/
package org.apache.jackrabbit.oak.jcr;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+
import javax.jcr.InvalidItemStateException;
import javax.jcr.Node;
import javax.jcr.RepositoryException;
@@ -23,9 +26,6 @@ import javax.jcr.Session;
import org.junit.Test;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.fail;
-
public class MoveRemoveTest extends AbstractRepositoryTest {
@Test
@@ -229,9 +229,11 @@ public class MoveRemoveTest extends Abst
session.getRootNode().addNode("parent").addNode("new");
session.save();
+ Node p = session.getNode("/parent");
Node n = session.getNode("/parent/new");
session.move("/parent", "/moved");
+ assertEquals("/moved", p.getPath());
assertEquals("/moved/new", n.getPath());
}