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 2012/11/01 16:41:16 UTC
svn commit: r1404646 - in /jackrabbit/oak/trunk:
oak-core/src/main/java/org/apache/jackrabbit/oak/core/
oak-core/src/test/java/org/apache/jackrabbit/oak/core/
oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/
oak-jcr/src/test/java/org/apache/jackrab...
Author: mduerig
Date: Thu Nov 1 15:41:16 2012
New Revision: 1404646
URL: http://svn.apache.org/viewvc?rev=1404646&view=rev
Log:
OAK-391: Avoid weak references in TreeImpl
OAK-14: Identify and document changes in behaviour wrt. Jackrabbit 2
- remove weak reference cache from TreeImpl
- adapt test expectations wrt. moved nodes
- add backward (in)compatibility tests
Modified:
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-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/ItemDelegate.java
jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/CompatibilityIssuesTest.java
jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/RepositoryTest.java
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=1404646&r1=1404645&r2=1404646&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 Nov 1 15:41:16 2012
@@ -20,13 +20,16 @@ package org.apache.jackrabbit.oak.core;
import java.util.Collections;
import java.util.Iterator;
-import java.util.Map;
import java.util.Set;
import javax.annotation.CheckForNull;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
+import com.google.common.base.Function;
+import com.google.common.base.Predicate;
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Sets;
import org.apache.jackrabbit.oak.api.PropertyState;
import org.apache.jackrabbit.oak.api.Tree;
import org.apache.jackrabbit.oak.api.TreeLocation;
@@ -41,12 +44,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 com.google.common.base.Function;
-import com.google.common.base.Predicate;
-import com.google.common.cache.CacheBuilder;
-import com.google.common.collect.Iterables;
-import com.google.common.collect.Sets;
-
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
import static org.apache.jackrabbit.oak.commons.PathUtils.elements;
@@ -71,12 +68,6 @@ public class TreeImpl implements Tree, P
/** Lazily initialised {@code NodeBuilder} for the underlying node state */
NodeBuilder nodeBuilder;
- /**
- * Cache for child trees that have been accessed before.
- */
- private final Map<String, TreeImpl> children =
- CacheBuilder.newBuilder().weakValues().<String, TreeImpl>build().asMap();
-
private TreeImpl(RootImpl root, TreeImpl parent, String name) {
this.root = checkNotNull(root);
this.parent = parent;
@@ -154,7 +145,7 @@ public class TreeImpl implements Tree, P
root.checkLive();
Status nodeStatus = getStatus();
if (nodeStatus == Status.NEW) {
- return (hasProperty(name)) ? Status.NEW : null;
+ return (hasProperty(name)) ? Status.NEW : null;
} else if (nodeStatus == Status.REMOVED) {
return Status.REMOVED; // FIXME not correct if no property existed with that name
} else {
@@ -264,12 +255,7 @@ public class TreeImpl implements Tree, P
new Function<String, Tree>() {
@Override
public Tree apply(String input) {
- TreeImpl child = children.get(input);
- if (child == null) {
- child = new TreeImpl(root, TreeImpl.this, input);
- children.put(input, child);
- }
- return child;
+ return new TreeImpl(root, TreeImpl.this, input);
}
}),
new Predicate<Tree>() {
@@ -309,7 +295,6 @@ public class TreeImpl implements Tree, P
if (!isRoot() && parent.hasChild(name)) {
NodeBuilder builder = parent.getNodeBuilder();
builder.removeNode(name);
- parent.children.remove(name);
removed = true;
if (parent.hasOrderableChildren()) {
builder.setProperty(
@@ -456,9 +441,6 @@ public class TreeImpl implements Tree, P
throw new IllegalStateException("Cannot move removed tree");
}
- parent.children.remove(name);
- destParent.children.put(destName, this);
-
name = destName;
parent = destParent;
}
@@ -515,12 +497,9 @@ public class TreeImpl implements Tree, P
//------------------------------------------------------------< private >---
private TreeImpl internalGetChild(String childName) {
- TreeImpl child = children.get(childName);
- if (child == null && getNodeBuilder().hasChildNode(childName)) {
- child = new TreeImpl(root, this, childName);
- children.put(childName, child);
- }
- return child;
+ return getNodeBuilder().hasChildNode(childName)
+ ? new TreeImpl(root, this, childName)
+ : null;
}
private PropertyState internalGetProperty(String propertyName) {
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=1404646&r1=1404645&r2=1404646&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 Nov 1 15:41:16 2012
@@ -110,20 +110,6 @@ public class RootImplTest {
assertTrue(tree.getChild("y").hasChild("xx"));
}
- @Test
- public void move2() {
- Root root = session.getLatestRoot();
- Tree r = root.getTree("/");
- Tree x = r.getChild("x");
- Tree y = r.getChild("y");
-
- assertFalse(y.hasChild("x"));
- assertEquals("", x.getParent().getName());
- root.move("/x", "/y/x");
- assertTrue(y.hasChild("x"));
- assertEquals("y", x.getParent().getName());
- }
-
/**
* Regression test for OAK-208
*/
Modified: jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/ItemDelegate.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/ItemDelegate.java?rev=1404646&r1=1404645&r2=1404646&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/ItemDelegate.java (original)
+++ jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/ItemDelegate.java Thu Nov 1 15:41:16 2012
@@ -133,7 +133,12 @@ public abstract class ItemDelegate {
}
private static boolean isStale(TreeLocation location) {
- return location.getStatus() == Status.REMOVED || location.getPath() == null;
+ try {
+ return location.getStatus() == Status.REMOVED || location.getPath() == null;
+ }
+ catch (IllegalStateException e) {
+ return true; // FIXME left over from OAK-391. Find better way to determine staleness
+ }
}
}
Modified: jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/CompatibilityIssuesTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/CompatibilityIssuesTest.java?rev=1404646&r1=1404645&r2=1404646&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/CompatibilityIssuesTest.java (original)
+++ jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/CompatibilityIssuesTest.java Thu Nov 1 15:41:16 2012
@@ -20,8 +20,16 @@ import javax.jcr.Node;
import javax.jcr.RepositoryException;
import javax.jcr.Session;
+import org.apache.jackrabbit.oak.Oak;
+import org.apache.jackrabbit.oak.api.CommitFailedException;
+import org.apache.jackrabbit.oak.api.ContentSession;
+import org.apache.jackrabbit.oak.api.Root;
+import org.apache.jackrabbit.oak.api.Tree;
import org.junit.Test;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
/**
@@ -60,12 +68,12 @@ public class CompatibilityIssuesTest ext
session1.save();
session2.getNode("/testNode").setProperty("p2", -1);
- check(session2); // Throws on JR2, not on JR3
+ check(session2); // Throws on JR2, not on Oak
session2.save();
Session session3 = createAnonymousSession();
try {
- check(session3); // Throws on JR3
+ check(session3); // Throws on Oak
fail();
} catch (AssertionError e) {
// expected
@@ -91,4 +99,40 @@ public class CompatibilityIssuesTest ext
}
}
+ @Test
+ public void move() throws RepositoryException {
+ Session session = getAdminSession();
+
+ Node node = session.getNode("/");
+ node.addNode("source").addNode("node");
+ node.addNode("target");
+ session.save();
+
+ session.refresh(true);
+ Node sourceNode = session.getNode("/source/node");
+ session.move("/source/node", "/target/moved");
+ // assertEquals("/target/moved", sourceNode.getPath()); // passes on JR2, fails on Oak
+ assertEquals("/source/node", sourceNode.getPath()); // fails on JR2, passed on Oak
+ }
+
+ @Test
+ public void move2() throws CommitFailedException {
+ ContentSession session = new Oak().createContentSession();
+ Root root = session.getLatestRoot();
+ root.getTree("/").addChild("x");
+ root.getTree("/").addChild("y");
+ root.commit();
+
+ Tree r = root.getTree("/");
+ Tree x = r.getChild("x");
+ Tree y = r.getChild("y");
+
+ assertFalse(y.hasChild("x"));
+ assertEquals("", x.getParent().getName());
+ root.move("/x", "/y/x");
+ assertTrue(y.hasChild("x"));
+ // assertEquals("y", x.getParent().getName()); // passed on JR2, fails on Oak
+ assertEquals("", x.getParent().getName()); // fails on JR2, passes on Oak
+ }
+
}
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=1404646&r1=1404645&r2=1404646&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 Thu Nov 1 15:41:16 2012
@@ -1356,10 +1356,8 @@ public class RepositoryTest extends Abst
node.addNode("target");
session.save();
- Node sourceNode = session.getNode(TEST_PATH + "/source/node");
session.refresh(true);
session.move(TEST_PATH + "/source/node", TEST_PATH + "/target/moved");
- assertEquals("/test_node/target/moved", sourceNode.getPath());
assertFalse(node.hasNode("source/node"));
assertTrue(node.hasNode("source"));
@@ -1381,10 +1379,8 @@ public class RepositoryTest extends Abst
node.addNode("target");
session.save();
- Node sourceNode = session.getNode(TEST_PATH + "/source/node");
session.refresh(true);
session.move(TEST_PATH + "/source/node", TEST_PATH + "/target/moved");
- assertEquals("/test_node/target/moved", sourceNode.getPath());
assertFalse(node.hasNode("source/node"));
assertTrue(node.hasNode("source"));