jackrabbit-oak-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From mdue...@apache.org
Subject 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...
Date Thu, 01 Nov 2012 15:41:16 GMT
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"));



Mime
View raw message