jackrabbit-oak-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From mdue...@apache.org
Subject svn commit: r1477007 - in /jackrabbit/oak/trunk/oak-core/src: main/java/org/apache/jackrabbit/oak/api/ main/java/org/apache/jackrabbit/oak/core/ main/java/org/apache/jackrabbit/oak/security/user/ test/java/org/apache/jackrabbit/oak/core/
Date Mon, 29 Apr 2013 12:40:13 GMT
Author: mduerig
Date: Mon Apr 29 12:40:13 2013
New Revision: 1477007

URL: http://svn.apache.org/r1477007
Log:
OAK-798: Review / refactor TreeImpl and related classes
OAK-709: Consider moving permission evaluation to the node state level
- Introduce @NonNull variants of Root.getTree, Tree.getParent and Tree.getChild
- Implement @CheckForNull variants in term of @NonNull variants
- Clean up TreeImpl
- Rename Tree.isConnected to Tree.exists

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/api/TreeLocation.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/AbstractNodeLocation.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/AbstractPropertyLocation.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/ImmutableRoot.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/ImmutableTree.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/ReadOnlyTree.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/main/java/org/apache/jackrabbit/oak/security/user/AuthorizableImpl.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=1477007&r1=1477006&r2=1477007&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
Apr 29 12:40:13 2013
@@ -34,8 +34,9 @@ import javax.annotation.Nonnull;
  * {@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#isConnected()} - will cause an
+ * {@link Tree#getParent()} and {@link Tree#exists()} - will cause an
  * {@code InvalidStateException}.
+ * TODO document iterability / existence (OAK-798)
  */
 public interface Root {
 
@@ -81,6 +82,16 @@ public interface Root {
     boolean copy(String sourcePath, String destPath);
 
     /**
+     * Retrieve the possible non existing {@code Tree} at the given absolute {@code path}.
+     * The path must resolve to a tree in this root.
+     *
+     * @param path absolute path to the tree
+     * @return tree at the given path.
+     */
+    @Nonnull
+    Tree getTreeNonNull(@Nonnull String path);
+
+    /**
      * Retrieve the {@code Tree} at the given absolute {@code path}. The path
      * must resolve to a tree in this 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=1477007&r1=1477006&r2=1477007&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
Apr 29 12:40:13 2013
@@ -60,8 +60,9 @@ import javax.annotation.Nullable;
  * {@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#isConnected()} - will cause an
+ * {@link Tree#getParent()} and {@link Tree#exists()} - will cause an
  * {@code InvalidStateException}.
+ * TODO document iterability / existence (OAK-798)
  */
 public interface Tree {
 
@@ -112,11 +113,12 @@ public interface Tree {
     Status getStatus();
 
     /**
-     * Determine whether this tree has been removed or has become disconnected otherwise
-     * (e.g. caused by a refresh, rebase or commit).
-     * @return {@code false} if this tree has been disconnected and {@code true} otherwise.
+     * Determine whether this tree has been removed or does not exist otherwise (e.g. caused
+     * by a refresh, rebase or commit) or is not visible due to access control restriction
+     * or does not exist at all.
+     * @return {@code true} if this tree exists, {@code false} otherwise.
      */
-    boolean isConnected();
+    boolean exists();
 
     /**
      * @return the current location
@@ -125,6 +127,13 @@ public interface Tree {
     TreeLocation getLocation();
 
     /**
+     * @return the possibly non existent parent of this {@code Tree}.
+     * @throws IllegalStateException if called on the root tree.
+     */
+    @Nonnull
+    Tree getParentNonNull();
+
+    /**
      * @return the parent of this {@code Tree} instance. This method returns
      *         {@code null} if the parent is not accessible or if no parent exists (root
      *         node).
@@ -181,6 +190,14 @@ public interface Tree {
     Iterable<? extends PropertyState> getProperties();
 
     /**
+     * Get a possibly non existing child of this {@code Tree}.
+     * @param name The name of the child to retrieve.
+     * @return The child with the given {@code name}.
+     */
+    @Nonnull
+    Tree getChildNonNull(@Nonnull String name);
+
+    /**
      * Get a child of this {@code Tree} instance.
      *
      * @param name The name of the child to retrieve.

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/api/TreeLocation.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/api/TreeLocation.java?rev=1477007&r1=1477006&r2=1477007&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/api/TreeLocation.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/api/TreeLocation.java
Mon Apr 29 12:40:13 2013
@@ -57,7 +57,7 @@ public interface TreeLocation {
      * {@link org.apache.jackrabbit.oak.api.PropertyState} for this {@code TreeLocation}
      * is available.
      * @return  {@code true} if the underlying item is available and has not been disconnected.
-     * @see org.apache.jackrabbit.oak.api.Tree#isConnected()
+     * @see org.apache.jackrabbit.oak.api.Tree#exists()
      */
     boolean exists();
 

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/AbstractNodeLocation.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/AbstractNodeLocation.java?rev=1477007&r1=1477006&r2=1477007&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/AbstractNodeLocation.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/AbstractNodeLocation.java
Mon Apr 29 12:40:13 2013
@@ -71,7 +71,7 @@ abstract class AbstractNodeLocation<T ex
 
     @Override
     public boolean exists() {
-        return tree.isConnected() && getTree() != null;
+        return tree.exists() && getTree() != null;
     }
 
     @Override

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/AbstractPropertyLocation.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/AbstractPropertyLocation.java?rev=1477007&r1=1477006&r2=1477007&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/AbstractPropertyLocation.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/AbstractPropertyLocation.java
Mon Apr 29 12:40:13 2013
@@ -48,13 +48,13 @@ abstract class AbstractPropertyLocation<
 
     @Override
     public boolean exists() {
-        return parentLocation.tree.isConnected() && getProperty() != null;
+        return parentLocation.tree.exists() && getProperty() != null;
     }
 
     @Override
     public PropertyState getProperty() {
         PropertyState property = parentLocation.getPropertyState(name);
-        return canRead(property)
+        return property != null && canRead(property)
             ? property
             : null;
     }

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/ImmutableRoot.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/ImmutableRoot.java?rev=1477007&r1=1477006&r2=1477007&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/ImmutableRoot.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/ImmutableRoot.java
Mon Apr 29 12:40:13 2013
@@ -18,6 +18,9 @@
  */
 package org.apache.jackrabbit.oak.core;
 
+import static com.google.common.base.Preconditions.checkArgument;
+import static org.apache.jackrabbit.oak.commons.PathUtils.elements;
+
 import javax.annotation.Nonnull;
 
 import org.apache.jackrabbit.oak.api.BlobFactory;
@@ -28,9 +31,6 @@ import org.apache.jackrabbit.oak.api.Tre
 import org.apache.jackrabbit.oak.commons.PathUtils;
 import org.apache.jackrabbit.oak.spi.state.NodeState;
 
-import static com.google.common.base.Preconditions.checkArgument;
-import static org.apache.jackrabbit.oak.commons.PathUtils.elements;
-
 /**
  * Simple implementation of the Root interface that only supports simple read
  * operations (excluding query) based on the {@code NodeState} (or {@code ImmutableTree})
@@ -54,9 +54,23 @@ public final class ImmutableRoot impleme
     }
 
     //---------------------------------------------------------------< Root >---
+
+
+    @Nonnull
+    @Override
+    public ImmutableTree getTreeNonNull(@Nonnull String path) {
+        checkArgument(PathUtils.isAbsolute(path));
+        ImmutableTree child = rootTree;
+        for (String name : elements(path)) {
+            child = child.getChildNonNull(name);
+        }
+        return child;
+    }
+
     @Override
     public ImmutableTree getTree(String path) {
-        return (ImmutableTree) getLocation(path).getTree();
+        ImmutableTree tree = getTreeNonNull(path);
+        return tree.exists() ? tree : null;
     }
 
     @Nonnull

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/ImmutableTree.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/ImmutableTree.java?rev=1477007&r1=1477006&r2=1477007&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/ImmutableTree.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/ImmutableTree.java
Mon Apr 29 12:40:13 2013
@@ -16,7 +16,11 @@
  */
 package org.apache.jackrabbit.oak.core;
 
+import static com.google.common.base.Preconditions.checkNotNull;
+import static org.apache.jackrabbit.oak.api.Type.STRING;
+
 import java.util.Iterator;
+
 import javax.annotation.CheckForNull;
 import javax.annotation.Nonnull;
 
@@ -29,9 +33,6 @@ import org.apache.jackrabbit.oak.commons
 import org.apache.jackrabbit.oak.spi.state.ChildNodeEntry;
 import org.apache.jackrabbit.oak.spi.state.NodeState;
 
-import static com.google.common.base.Preconditions.checkNotNull;
-import static org.apache.jackrabbit.oak.api.Type.STRING;
-
 /**
  * Immutable implementation of the {@code Tree} interface in order to provide
  * the much feature rich API functionality for a given {@code NodeState}.
@@ -165,14 +166,17 @@ public final class ImmutableTree extends
         return parentProvider.getParent();
     }
 
+    @Nonnull
     @Override
-    public ImmutableTree getChild(@Nonnull String name) {
+    public ImmutableTree getChildNonNull(@Nonnull String name) {
         NodeState child = state.getChildNode(name);
-        if (child.exists()) {
-            return new ImmutableTree(this, name, child);
-        } else {
-            return null;
-        }
+        return new ImmutableTree(this, name, child);
+    }
+
+    @Override
+    public ImmutableTree getChild(@Nonnull String name) {
+        ImmutableTree child = getChildNonNull(name);
+        return child.exists() ? child : null;
     }
 
     /**

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/ReadOnlyTree.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/ReadOnlyTree.java?rev=1477007&r1=1477006&r2=1477007&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/ReadOnlyTree.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/ReadOnlyTree.java
Mon Apr 29 12:40:13 2013
@@ -20,6 +20,7 @@ package org.apache.jackrabbit.oak.core;
 
 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 java.util.Iterator;
@@ -93,6 +94,13 @@ public class ReadOnlyTree implements Tre
         }
     }
 
+    @Nonnull
+    @Override
+    public Tree getParentNonNull() {
+        checkState(parent != null, "root tree does not have a parent");
+        return parent;
+    }
+
     @Override
     public Tree getParent() {
         return parent;
@@ -127,14 +135,16 @@ public class ReadOnlyTree implements Tre
         return state.getProperties();
     }
 
+    @Nonnull
+    @Override
+    public ReadOnlyTree getChildNonNull(@Nonnull String name) {
+        return new ReadOnlyTree(this, name, state.getChildNode(name));
+    }
+
     @Override
     public ReadOnlyTree getChild(@Nonnull String name) {
-        NodeState child = state.getChildNode(name);
-        if (child.exists()) {
-            return new ReadOnlyTree(this, name, child);
-        } else {
-            return null;
-        }
+        ReadOnlyTree child = getChildNonNull(name);
+        return child.exists() ? child : null;
     }
 
     @Override
@@ -143,7 +153,7 @@ public class ReadOnlyTree implements Tre
     }
 
     @Override
-    public boolean isConnected() {
+    public boolean exists() {
         return true;
     }
 

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=1477007&r1=1477006&r2=1477007&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 Apr 29 12:40:13 2013
@@ -177,7 +177,7 @@ public class RootImpl implements Root {
     @Override
     public boolean move(String sourcePath, String destPath) {
         checkLive();
-        TreeImpl destParent = rootTree.getTree(getParentPath(destPath));
+        TreeImpl destParent = rootTree.getTreeOrNull(getParentPath(destPath));
         if (destParent == null) {
             return false;
         }
@@ -205,12 +205,18 @@ public class RootImpl implements Root {
     }
 
     @Override
-    public TreeImpl getTree(String path) {
+    public TreeImpl getTreeNonNull(@Nonnull String path) {
         checkLive();
         return rootTree.getTree(path);
     }
 
     @Override
+    public TreeImpl getTree(String path) {
+        checkLive();
+        return rootTree.getTreeOrNull(path);
+    }
+
+    @Override
     public TreeLocation getLocation(String path) {
         checkArgument(PathUtils.isAbsolute(path));
         checkLive();

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=1477007&r1=1477006&r2=1477007&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 Apr 29 12:40:13 2013
@@ -21,8 +21,17 @@ package org.apache.jackrabbit.oak.core;
 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 com.google.common.collect.Iterables.filter;
+import static com.google.common.collect.Iterables.indexOf;
+import static com.google.common.collect.Iterables.size;
+import static com.google.common.collect.Iterables.transform;
+import static org.apache.jackrabbit.JcrConstants.JCR_UUID;
+import static org.apache.jackrabbit.oak.api.Tree.Status.EXISTING;
+import static org.apache.jackrabbit.oak.api.Tree.Status.MODIFIED;
+import static org.apache.jackrabbit.oak.api.Tree.Status.NEW;
 import static org.apache.jackrabbit.oak.api.Type.STRING;
 import static org.apache.jackrabbit.oak.commons.PathUtils.elements;
+import static org.apache.jackrabbit.oak.commons.PathUtils.isAbsolute;
 
 import java.util.Collections;
 import java.util.Iterator;
@@ -36,7 +45,6 @@ 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.JcrConstants;
 import org.apache.jackrabbit.oak.api.PropertyState;
 import org.apache.jackrabbit.oak.api.Tree;
 import org.apache.jackrabbit.oak.api.TreeLocation;
@@ -63,11 +71,6 @@ public class TreeImpl implements Tree {
     private final RootImpl root;
 
     /**
-     * The {@code NodeBuilder} for the underlying node state
-     */
-    private NodeBuilder nodeBuilder;
-
-    /**
      * Parent of this tree. Null for the root.
      */
     private TreeImpl parent;
@@ -77,6 +80,11 @@ public class TreeImpl implements Tree {
      */
     private String name;
 
+    /**
+     * The {@code NodeBuilder} for the underlying node state
+     */
+    private NodeBuilder nodeBuilder;
+
     /** Pointer into the list of pending moves */
     private Move pendingMoves;
 
@@ -114,9 +122,40 @@ public class TreeImpl implements Tree {
     }
 
     @Override
+    public Status getStatus() {
+        enter();
+
+        if (nodeBuilder.isNew()) {
+            return NEW;
+        } else if (nodeBuilder.isModified()) {
+            return MODIFIED;
+        } else {
+            return EXISTING;
+        }
+    }
+
+    @Override
+    public TreeLocation getLocation() {
+        enterNoStateCheck();
+        return new NodeLocation(this);
+    }
+
+    @Override
+    public boolean exists() {
+        return enterNoStateCheck();
+    }
+
+    @Override
+    public Tree getParentNonNull() {
+        checkState(parent != null, "root tree does not have a parent");
+        root.checkLive();
+        return parent;
+    }
+
+    @Override
     public Tree getParent() {
         enterNoStateCheck();
-        if (parent != null && canRead(parent)) {
+        if (parent != null && parent.nodeBuilder.exists()) {
             return parent;
         } else {
             return null;
@@ -126,108 +165,77 @@ public class TreeImpl implements Tree {
     @Override
     public PropertyState getProperty(String name) {
         enter();
-        PropertyState property = internalGetProperty(name);
-        if (canRead(property)) {
-            return property;
-        } else {
-            return null;
-        }
+        return getVisibleProperty(name);
     }
 
     @Override
     public Status getPropertyStatus(String name) {
         // TODO: see OAK-212
         Status nodeStatus = getStatus();
-        if (nodeStatus == Status.NEW) {
-            return (hasProperty(name)) ? Status.NEW : null;
+        if (nodeStatus == NEW) {
+            return (hasProperty(name)) ? NEW : null;
         }
-        PropertyState head = internalGetProperty(name);
-        if (head == null || !canRead(head)) {
-            // no permission to read status information for existing property
+        PropertyState head = getVisibleProperty(name);
+        if (head == null) {
             return null;
         }
 
         PropertyState base = getSecureBase().getProperty(name);
 
         if (base == null) {
-            return Status.NEW;
+            return NEW;
         } else if (head.equals(base)) {
-            return Status.EXISTING;
+            return EXISTING;
         } else {
-            return Status.MODIFIED;
+            return MODIFIED;
         }
     }
 
     @Override
     public boolean hasProperty(String name) {
-        enter();
         return getProperty(name) != null;
     }
 
     @Override
     public long getPropertyCount() {
-        enter();
-        return Iterables.size(getProperties());
+        return size(getProperties());
     }
 
     @Override
     public Iterable<? extends PropertyState> getProperties() {
         enter();
-        return Iterables.filter(nodeBuilder.getProperties(),
+        return filter(nodeBuilder.getProperties(),
                 new Predicate<PropertyState>() {
                     @Override
                     public boolean apply(PropertyState propertyState) {
-                        return canRead(propertyState);
+                        return !isHidden(propertyState.getName());
                     }
                 });
     }
 
     @Override
-    public TreeImpl getChild(@Nonnull String name) {
+    public TreeImpl getChildNonNull(@Nonnull String name) {
         checkNotNull(name);
-        enter();
-        TreeImpl child = internalGetChild(name);
-        return canRead(child) ? child : null;
-    }
-
-    @Override
-    public boolean isConnected() {
         enterNoStateCheck();
-        if (parent == null || nodeBuilder.exists()) {
-            return true;
-        }
-
-        return reconnect();
-    }
-
-    private boolean reconnect() {
-        if (parent != null && parent.reconnect()) {
-            nodeBuilder = parent.nodeBuilder.getChildNode(name);
-        }
-        return nodeBuilder.exists();
+        return new TreeImpl(root, this, name, pendingMoves);
     }
 
     @Override
-    public Status getStatus() {
-        enter();
-
-        if (nodeBuilder.isNew()) {
-            return Status.NEW;
-        } else if (nodeBuilder.isModified()) {
-            return Status.MODIFIED;
-        } else {
-            return Status.EXISTING;
-        }
+    public TreeImpl getChild(@Nonnull String name) {
+        TreeImpl child = getChildNonNull(name);
+        return child.nodeBuilder.exists() ? child : null;
     }
 
     @Override
     public boolean hasChild(@Nonnull String name) {
-        return getChild(name) != null;
+        checkNotNull(name);
+        enter();
+        TreeImpl child = new TreeImpl(root, this, name, pendingMoves);
+        return child.nodeBuilder.exists();
     }
 
     @Override
     public long getChildrenCount() {
-        // TODO: make sure cnt respects access control
         enter();
         return nodeBuilder.getChildNodeCount();
     }
@@ -241,41 +249,50 @@ public class TreeImpl implements Tree {
         } else {
             childNames = nodeBuilder.getChildNodeNames();
         }
-        return Iterables.filter(Iterables.transform(
+        return transform(
                 childNames,
                 new Function<String, Tree>() {
                     @Override
                     public Tree apply(String input) {
                         return new TreeImpl(root, TreeImpl.this, input, pendingMoves);
                     }
-                }),
-                new Predicate<Tree>() {
-                    @Override
-                    public boolean apply(Tree tree) {
-                        return tree != null && canRead((TreeImpl) tree);
-                    }
                 });
     }
 
     @Override
+    public boolean remove() {
+        enter();
+        if (parent != null && parent.hasChild(name)) {
+            NodeBuilder parentBuilder = parent.nodeBuilder;
+            parentBuilder.removeChildNode(name);
+            if (parent.hasOrderableChildren()) {
+                parentBuilder.setProperty(
+                        MemoryPropertyBuilder.copy(STRING, parent.nodeBuilder.getProperty(OAK_CHILD_ORDER))
+                                .removeValue(name)
+                                .getPropertyState()
+                );
+            }
+            root.updated();
+            return true;
+        } else {
+            return false;
+        }
+    }
+
+    @Override
     public Tree addChild(String name) {
         enter();
         if (!hasChild(name)) {
-            nodeBuilder.child(name);
+            nodeBuilder.setChildNode(name);
             if (hasOrderableChildren()) {
                 nodeBuilder.setProperty(
-                        MemoryPropertyBuilder.copy(Type.STRING, internalGetProperty(OAK_CHILD_ORDER))
+                        MemoryPropertyBuilder.copy(STRING, nodeBuilder.getProperty(OAK_CHILD_ORDER))
                                 .addValue(name)
                                 .getPropertyState());
             }
             root.updated();
         }
-
-        TreeImpl child = new TreeImpl(root, this, name, pendingMoves);
-
-        // Make sure to allocate the node builder for new nodes in order to correctly
-        // track removes and moves. See OAK-621
-        return child;
+        return new TreeImpl(root, this, name, pendingMoves);
     }
 
     @Override
@@ -289,30 +306,9 @@ public class TreeImpl implements Tree {
     }
 
     @Override
-    public boolean remove() {
-        enter();
-
-        if (!isRoot() && parent.hasChild(name)) {
-            NodeBuilder parentBuilder = parent.nodeBuilder;
-            parentBuilder.removeChildNode(name);
-            if (parent.hasOrderableChildren()) {
-                parentBuilder.setProperty(
-                        MemoryPropertyBuilder.copy(Type.STRING, parent.internalGetProperty(OAK_CHILD_ORDER))
-                                .removeValue(name)
-                                .getPropertyState()
-                );
-            }
-            root.updated();
-            return true;
-        } else {
-            return false;
-        }
-    }
-
-    @Override
     public boolean orderBefore(final String name) {
         enter();
-        if (isRoot()) {
+        if (parent == null) {
             // root does not have siblings
             return false;
         }
@@ -323,32 +319,34 @@ public class TreeImpl implements Tree {
         // perform the reorder
         parent.ensureChildOrderProperty();
         // all siblings but not this one
-        Iterable<String> filtered = Iterables.filter(
+        Iterable<String> siblings = filter(
                 parent.getOrderedChildNames(),
                 new Predicate<String>() {
                     @Override
-                    public boolean apply(@Nullable String input) {
-                        return !TreeImpl.this.getName().equals(input);
+                    public boolean apply(@Nullable String name) {
+                        return !TreeImpl.this.name.equals(name);
                     }
                 });
         // create head and tail
         Iterable<String> head;
         Iterable<String> tail;
         if (name == null) {
-            head = filtered;
+            head = siblings;
             tail = Collections.emptyList();
         } else {
-            int idx = Iterables.indexOf(filtered, new Predicate<String>() {
+            int idx = indexOf(siblings, new Predicate<String>() {
                 @Override
-                public boolean apply(@Nullable String input) {
-                    return name.equals(input);
+                public boolean apply(@Nullable String sibling) {
+                    return name.equals(sibling);
                 }
             });
-            head = Iterables.limit(filtered, idx);
-            tail = Iterables.skip(filtered, idx);
+            head = Iterables.limit(siblings, idx);
+            tail = Iterables.skip(siblings, idx);
         }
         // concatenate head, this name and tail
-        parent.nodeBuilder.setProperty(MultiStringPropertyState.stringProperty(OAK_CHILD_ORDER,
Iterables.concat(head, Collections.singleton(getName()), tail))
+        parent.nodeBuilder.setProperty(
+                MultiStringPropertyState.stringProperty(
+                        OAK_CHILD_ORDER, Iterables.concat(head, Collections.singleton(getName()),
tail))
         );
         root.updated();
         return true;
@@ -383,12 +381,6 @@ public class TreeImpl implements Tree {
     }
 
     @Override
-    public TreeLocation getLocation() {
-        enter();
-        return new NodeLocation(this);
-    }
-
-    @Override
     public String toString() {
         return getPathInternal() + ": " + getNodeState();
     }
@@ -422,6 +414,21 @@ public class TreeImpl implements Tree {
     }
 
     /**
+     * Get a possibly non existing tree.
+     * @param path the path to the tree
+     * @return a {@link Tree} instance for the child at {@code path}.
+     */
+    @CheckForNull
+    TreeImpl getTree(@Nonnull String path) {
+        checkArgument(isAbsolute(checkNotNull(path)));
+        TreeImpl child = this;
+        for (String name : elements(path)) {
+            child = new TreeImpl(child.root, child, name, child.pendingMoves);
+        }
+        return child;
+    }
+
+    /**
      * Get a tree for the tree identified by {@code path}.
      *
      * @param path the path to the child
@@ -429,13 +436,10 @@ public class TreeImpl implements Tree {
      *         {@code null} if no such tree exits or if the tree is not accessible.
      */
     @CheckForNull
-    TreeImpl getTree(String path) {
-        checkArgument(PathUtils.isAbsolute(path));
-        TreeImpl child = this;
-        for (String name : elements(path)) {
-            child = child.internalGetChild(name);
-        }
-        return canRead(child) ? child : null;
+    @Deprecated
+    TreeImpl getTreeOrNull(String path) {
+        TreeImpl child = getTree(path);
+        return child.nodeBuilder.exists() ? child : null;
     }
 
     /**
@@ -457,14 +461,14 @@ public class TreeImpl implements Tree {
             names.add(name);
         }
         PropertyBuilder<String> builder = MemoryPropertyBuilder.array(
-                Type.STRING, OAK_CHILD_ORDER);
+                STRING, OAK_CHILD_ORDER);
         builder.setValues(names);
         nodeBuilder.setProperty(builder.getPropertyState());
     }
 
     @Nonnull
     String getIdentifier() {
-        PropertyState property = internalGetProperty(JcrConstants.JCR_UUID);
+        PropertyState property = nodeBuilder.getProperty(JCR_UUID);
         if (property != null) {
             return property.getValue(STRING);
         } else if (parent == null) {
@@ -486,15 +490,26 @@ public class TreeImpl implements Tree {
 
     //------------------------------------------------------------< private >---
 
+    private boolean reconnect() {
+        if (parent != null && parent.reconnect()) {
+            nodeBuilder = parent.nodeBuilder.getChildNode(name);
+        }
+        return nodeBuilder.exists();
+    }
+
     private void enter() {
-        root.checkLive();
-        checkState(isConnected());
-        applyPendingMoves();
+        checkState(enterNoStateCheck(), "This tree is not connected");
     }
 
-    private void enterNoStateCheck() {
+    private boolean enterNoStateCheck() {
         root.checkLive();
         applyPendingMoves();
+        return reconnect();
+    }
+
+    private static boolean isHidden(String name) {
+        // FIXME clarify handling of hidden items (OAK-753).
+        return NodeStateUtils.isHidden(name);
     }
 
     /**
@@ -518,12 +533,11 @@ public class TreeImpl implements Tree {
         pendingMoves = pendingMoves.apply(this);
     }
 
-    private TreeImpl internalGetChild(String childName) {
-        return new TreeImpl(root, this, childName, pendingMoves);
-    }
+    private PropertyState getVisibleProperty(String name) {
+        return !isHidden(name)
+            ? nodeBuilder.getProperty(name)
+            : null;
 
-    private PropertyState internalGetProperty(String propertyName) {
-        return nodeBuilder.getProperty(propertyName);
     }
 
     private void buildPath(StringBuilder sb) {
@@ -533,16 +547,6 @@ public class TreeImpl implements Tree {
         }
     }
 
-    private boolean canRead(TreeImpl tree) {
-        // TODO: OAK-753 TreeImpl exposes hidden child trees
-        // return tree.getNodeState().exists() && !NodeStateUtils.isHidden(tree.getName());
-        return tree.nodeBuilder.exists();
-    }
-
-    private boolean canRead(PropertyState property) {
-        return property != null && !NodeStateUtils.isHidden(property.getName());
-    }
-
     /**
      * @return {@code true} if this tree has orderable children;
      *         {@code false} otherwise.
@@ -563,7 +567,7 @@ public class TreeImpl implements Tree {
             @Override
             public Iterator<String> iterator() {
                 return new Iterator<String>() {
-                    final PropertyState childOrder = internalGetProperty(OAK_CHILD_ORDER);
+                    final PropertyState childOrder = nodeBuilder.getProperty(OAK_CHILD_ORDER);
                     int index = 0;
 
                     @Override
@@ -573,7 +577,7 @@ public class TreeImpl implements Tree {
 
                     @Override
                     public String next() {
-                        return childOrder.getValue(Type.STRING, index++);
+                        return childOrder.getValue(STRING, index++);
                     }
 
                     @Override
@@ -622,21 +626,21 @@ public class TreeImpl implements Tree {
 
         @Override
         protected TreeImpl getChildTree(String name) {
-            return tree.internalGetChild(name);
+            return new TreeImpl(tree.root, tree, name, tree.pendingMoves);
         }
 
         @Override
         protected PropertyState getPropertyState(String name) {
-            return tree.internalGetProperty(name);
+            return tree.getVisibleProperty(name);
         }
 
         @Override
         protected boolean canRead(TreeImpl tree) {
-            return TreeImpl.this.canRead(tree);
+            return tree.nodeBuilder.exists();
         }
     }
 
-    private final class PropertyLocation extends AbstractPropertyLocation<TreeImpl>
{
+    private static final class PropertyLocation extends AbstractPropertyLocation<TreeImpl>
{
 
         private PropertyLocation(AbstractNodeLocation<TreeImpl> parentLocation, String
name) {
             super(parentLocation, name);
@@ -644,7 +648,7 @@ public class TreeImpl implements Tree {
 
         @Override
         protected boolean canRead(PropertyState property) {
-            return TreeImpl.this.canRead(property);
+            return !isHidden(property.getName());
         }
 
     }

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/AuthorizableImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/AuthorizableImpl.java?rev=1477007&r1=1477006&r2=1477007&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/AuthorizableImpl.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/AuthorizableImpl.java
Mon Apr 29 12:40:13 2013
@@ -16,8 +16,11 @@
  */
 package org.apache.jackrabbit.oak.security.user;
 
+import static org.apache.jackrabbit.oak.api.Type.STRING;
+
 import java.util.Collections;
 import java.util.Iterator;
+
 import javax.annotation.Nonnull;
 import javax.jcr.RepositoryException;
 import javax.jcr.Value;
@@ -34,8 +37,6 @@ import org.apache.jackrabbit.oak.spi.sec
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import static org.apache.jackrabbit.oak.api.Type.STRING;
-
 /**
  * Base class for {@code User} and {@code Group} implementations.
  */
@@ -181,7 +182,7 @@ abstract class AuthorizableImpl implemen
     //--------------------------------------------------------------------------
     @Nonnull
     Tree getTree() {
-        if (tree.isConnected()) {
+        if (tree.exists()) {
             return tree;
         } else {
             throw new IllegalStateException("Authorizable " + id + ": underlying tree has
been disconnected.");

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=1477007&r1=1477006&r2=1477007&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 Apr 29 12:40:13 2013
@@ -18,6 +18,12 @@
  */
 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.assertNull;
+import static org.junit.Assert.assertTrue;
+
 import java.util.ArrayList;
 import java.util.List;
 
@@ -33,12 +39,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.assertNull;
-import static org.junit.Assert.assertTrue;
-
 public class RootImplTest extends OakBaseTest {
 
     private ContentSession session;
@@ -124,7 +124,7 @@ public class RootImplTest extends OakBas
         root.move("/z", "/x/z");
         root.getTree("/x/z").remove();
 
-        assertFalse(z.isConnected());
+        assertFalse(z.exists());
 
         x.addChild("z");
         assertEquals(Status.EXISTING, z.getStatus());
@@ -136,7 +136,7 @@ public class RootImplTest extends OakBas
     }
 
     @Test
-    public void moveNew() throws CommitFailedException {
+    public void moveNew() {
         Root root = session.getLatestRoot();
         Tree tree = root.getTree("/");
 

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=1477007&r1=1477006&r2=1477007&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 Apr 29 12:40:13 2013
@@ -29,7 +29,6 @@ import java.util.HashSet;
 import java.util.Set;
 
 import com.google.common.collect.Sets;
-import org.apache.jackrabbit.oak.Oak;
 import org.apache.jackrabbit.oak.OakBaseTest;
 import org.apache.jackrabbit.oak.api.CommitFailedException;
 import org.apache.jackrabbit.oak.api.ContentSession;
@@ -180,13 +179,13 @@ public class TreeImplTest extends OakBas
     }
 
     @Test
-    public void removeNew() throws CommitFailedException {
+    public void removeNew() {
         Tree tree = root.getTree("/");
 
         Tree t = tree.addChild("new");
 
         tree.getChild("new").remove();
-        assertFalse(t.isConnected());
+        assertFalse(t.exists());
 
         assertNull(tree.getChild("new"));
     }
@@ -299,8 +298,8 @@ public class TreeImplTest extends OakBas
         Tree y = x.addChild("y");
         x.remove();
 
-        assertFalse(x.isConnected());
-        assertFalse(y.isConnected());
+        assertFalse(x.exists());
+        assertFalse(y.exists());
     }
 
     @Test
@@ -412,7 +411,7 @@ public class TreeImplTest extends OakBas
 
         assertEquals(Status.EXISTING, x.getStatus());
         assertNull(x.getPropertyStatus("p"));
-        assertFalse(xx.isConnected());
+        assertFalse(xx.exists());
     }
 
     @Test
@@ -429,8 +428,8 @@ public class TreeImplTest extends OakBas
 
         root.getTree("/x").remove();
 
-        assertFalse(x.isConnected());
-        assertFalse(xx.isConnected());
+        assertFalse(x.exists());
+        assertFalse(xx.exists());
     }
 
 }
\ No newline at end of file



Mime
View raw message