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 ju...@apache.org on 2013/12/19 20:54:00 UTC

svn commit: r1552409 - in /jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak: core/ plugins/commit/ plugins/observation/ security/authorization/permission/

Author: jukka
Date: Thu Dec 19 19:54:00 2013
New Revision: 1552409

URL: http://svn.apache.org/r1552409
Log:
OAK-1296: Use TypePredicate instead of NodeType.isNodeType() for NodeState type checks

Use TypePredicate in PermissionHook
Also make the child order property type to be NAMES instead of STRINGS

Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/AbstractTree.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/MutableTree.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/commit/MergingNodeStateDiff.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/JcrListener.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionHook.java

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/AbstractTree.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/AbstractTree.java?rev=1552409&r1=1552408&r2=1552409&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/AbstractTree.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/AbstractTree.java Thu Dec 19 19:54:00 2013
@@ -26,7 +26,7 @@ import static com.google.common.collect.
 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.api.Type.NAME;
 import static org.apache.jackrabbit.oak.spi.state.NodeStateUtils.isHidden;
 
 import java.util.Iterator;
@@ -35,6 +35,7 @@ import javax.annotation.Nonnull;
 
 import com.google.common.base.Function;
 import com.google.common.base.Predicate;
+
 import org.apache.jackrabbit.mk.api.MicroKernel;
 import org.apache.jackrabbit.oak.api.PropertyState;
 import org.apache.jackrabbit.oak.api.Tree;
@@ -144,7 +145,7 @@ public abstract class AbstractTree imple
 
                         @Override
                         public String next() {
-                            return childOrder.getValue(STRING, index++);
+                            return childOrder.getValue(NAME, index++);
                         }
 
                         @Override

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/MutableTree.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/MutableTree.java?rev=1552409&r1=1552408&r2=1552409&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/MutableTree.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/MutableTree.java Thu Dec 19 19:54:00 2013
@@ -23,7 +23,7 @@ import static com.google.common.base.Pre
 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 org.apache.jackrabbit.oak.api.Type.STRING;
+import static org.apache.jackrabbit.oak.api.Type.NAME;
 import static org.apache.jackrabbit.oak.commons.PathUtils.elements;
 import static org.apache.jackrabbit.oak.commons.PathUtils.isAbsolute;
 import static org.apache.jackrabbit.oak.spi.state.NodeStateUtils.isHidden;
@@ -37,11 +37,12 @@ import javax.annotation.Nonnull;
 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.Type;
 import org.apache.jackrabbit.oak.core.AbstractRoot.Move;
-import org.apache.jackrabbit.oak.plugins.memory.MultiStringPropertyState;
+import org.apache.jackrabbit.oak.plugins.memory.MultiGenericPropertyState;
 import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
 import org.apache.jackrabbit.oak.spi.state.NodeState;
 import org.apache.jackrabbit.oak.util.PropertyBuilder;
@@ -176,7 +177,7 @@ public class MutableTree extends Abstrac
             if (parent.hasOrderableChildren()) {
                 // FIXME (OAK-842) child order not updated when parent is not accessible
                 parent.nodeBuilder.setProperty(
-                        PropertyBuilder.copy(STRING, parent.nodeBuilder.getProperty(OAK_CHILD_ORDER))
+                        PropertyBuilder.copy(NAME, parent.nodeBuilder.getProperty(OAK_CHILD_ORDER))
                                 .removeValue(name)
                                 .getPropertyState()
                 );
@@ -195,7 +196,7 @@ public class MutableTree extends Abstrac
             nodeBuilder.setChildNode(name);
             if (hasOrderableChildren()) {
                 nodeBuilder.setProperty(
-                        PropertyBuilder.copy(STRING, nodeBuilder.getProperty(OAK_CHILD_ORDER))
+                        PropertyBuilder.copy(NAME, nodeBuilder.getProperty(OAK_CHILD_ORDER))
                                 .addValue(name)
                                 .getPropertyState());
             }
@@ -256,7 +257,7 @@ public class MutableTree extends Abstrac
         }
         // concatenate head, this name and tail
         parent.nodeBuilder.setProperty(
-                MultiStringPropertyState.stringProperty(
+                MultiGenericPropertyState.nameProperty(
                         OAK_CHILD_ORDER, Iterables.concat(head, Collections.singleton(getName()), tail))
         );
         root.updated();
@@ -365,7 +366,7 @@ public class MutableTree extends Abstrac
         for (String name : nodeBuilder.getChildNodeNames()) {
             names.add(name);
         }
-        PropertyBuilder builder = PropertyBuilder.array(STRING, OAK_CHILD_ORDER);
+        PropertyBuilder<String> builder = PropertyBuilder.array(NAME, OAK_CHILD_ORDER);
         builder.setValues(names);
         nodeBuilder.setProperty(builder.getPropertyState());
     }
@@ -474,7 +475,7 @@ public class MutableTree extends Abstrac
     private void ensureChildOrderProperty() {
         if (!nodeBuilder.hasProperty(OAK_CHILD_ORDER)) {
             nodeBuilder.setProperty(
-                    MultiStringPropertyState.stringProperty(OAK_CHILD_ORDER, nodeBuilder.getChildNodeNames()));
+                    MultiGenericPropertyState.nameProperty(OAK_CHILD_ORDER, nodeBuilder.getChildNodeNames()));
         }
     }
 

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/commit/MergingNodeStateDiff.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/commit/MergingNodeStateDiff.java?rev=1552409&r1=1552408&r2=1552409&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/commit/MergingNodeStateDiff.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/commit/MergingNodeStateDiff.java Thu Dec 19 19:54:00 2013
@@ -19,8 +19,8 @@ package org.apache.jackrabbit.oak.plugin
 import java.util.Map;
 
 import com.google.common.collect.ImmutableMap;
+
 import org.apache.jackrabbit.oak.api.PropertyState;
-import org.apache.jackrabbit.oak.api.Type;
 import org.apache.jackrabbit.oak.core.AbstractTree;
 import org.apache.jackrabbit.oak.spi.commit.ConflictHandler;
 import org.apache.jackrabbit.oak.spi.commit.ConflictHandler.Resolution;
@@ -33,6 +33,7 @@ import org.apache.jackrabbit.oak.util.Pr
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import static org.apache.jackrabbit.oak.api.Type.NAME;
 import static org.apache.jackrabbit.oak.spi.state.ConflictAnnotatingRebaseDiff.CONFLICT;
 import static org.apache.jackrabbit.oak.spi.state.ConflictType.ADD_EXISTING_NODE;
 import static org.apache.jackrabbit.oak.spi.state.ConflictType.ADD_EXISTING_PROPERTY;
@@ -237,7 +238,7 @@ public final class MergingNodeStateDiff 
         target.setChildNode(name, state);
         PropertyState childOrder = target.getProperty(AbstractTree.OAK_CHILD_ORDER);
         if (childOrder != null) {
-            PropertyBuilder builder = PropertyBuilder.copy(Type.STRING, childOrder);
+            PropertyBuilder<String> builder = PropertyBuilder.copy(NAME, childOrder);
             builder.addValue(name);
             target.setProperty(builder.getPropertyState());
         }
@@ -247,7 +248,7 @@ public final class MergingNodeStateDiff 
         target.getChildNode(name).remove();
         PropertyState childOrder = target.getProperty(AbstractTree.OAK_CHILD_ORDER);
         if (childOrder != null) {
-            PropertyBuilder builder = PropertyBuilder.copy(Type.STRING, childOrder);
+            PropertyBuilder<String> builder = PropertyBuilder.copy(NAME, childOrder);
             builder.removeValue(name);
             target.setProperty(builder.getPropertyState());
         }

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/JcrListener.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/JcrListener.java?rev=1552409&r1=1552408&r2=1552409&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/JcrListener.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/JcrListener.java Thu Dec 19 19:54:00 2013
@@ -16,7 +16,6 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-
 package org.apache.jackrabbit.oak.plugins.observation;
 
 import static com.google.common.collect.Lists.newArrayList;
@@ -26,7 +25,6 @@ import static javax.jcr.observation.Even
 import static javax.jcr.observation.Event.NODE_REMOVED;
 import static javax.jcr.observation.Event.PROPERTY_ADDED;
 import static javax.jcr.observation.Event.PROPERTY_REMOVED;
-import static org.apache.jackrabbit.oak.api.Type.STRING;
 import static org.apache.jackrabbit.oak.core.AbstractTree.OAK_CHILD_ORDER;
 import static org.apache.jackrabbit.oak.plugins.identifier.IdentifierManager.getIdentifier;
 
@@ -37,7 +35,7 @@ import java.util.Map;
 import javax.jcr.observation.Event;
 
 import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.Lists;
+
 import org.apache.jackrabbit.oak.api.PropertyState;
 import org.apache.jackrabbit.oak.api.Tree;
 import org.apache.jackrabbit.oak.commons.PathUtils;
@@ -142,14 +140,8 @@ class JcrListener implements IterableLis
     //------------------------------------------------------------< private >---
 
     private void detectReorder(String name, NodeState before, NodeState after) {
-        PropertyState afterOrder = after.getProperty(OAK_CHILD_ORDER);
-        PropertyState beforeOrder = before.getProperty(OAK_CHILD_ORDER);
-        if (afterOrder == null || beforeOrder == null) {
-            return;
-        }
-
-        List<String> afterNames = getNames(afterOrder);
-        List<String> beforeNames = getNames(beforeOrder);
+        List<String> afterNames = newArrayList(after.getNames(OAK_CHILD_ORDER));
+        List<String> beforeNames = newArrayList(before.getNames(OAK_CHILD_ORDER));
 
         afterNames.retainAll(beforeNames);
         beforeNames.retainAll(afterNames);
@@ -171,14 +163,6 @@ class JcrListener implements IterableLis
         }
     }
 
-    private static List<String> getNames(PropertyState propertyState) {
-        List<String> names = Lists.newArrayList();
-        for (int k = 0; k < propertyState.count(); k++) {
-            names.add(propertyState.getValue(STRING, k));
-        }
-        return names;
-    }
-
     private Event createEvent(int eventType, Tree tree) {
         return createEvent(eventType, tree.getPath(), getIdentifier(tree), emptyMap());
     }

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionHook.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionHook.java?rev=1552409&r1=1552408&r2=1552409&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionHook.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionHook.java Thu Dec 19 19:54:00 2013
@@ -27,13 +27,12 @@ import javax.annotation.Nonnull;
 
 import com.google.common.base.Objects;
 import com.google.common.base.Strings;
+
 import org.apache.jackrabbit.oak.api.CommitFailedException;
-import org.apache.jackrabbit.oak.api.Tree;
 import org.apache.jackrabbit.oak.api.Type;
 import org.apache.jackrabbit.oak.core.ImmutableRoot;
 import org.apache.jackrabbit.oak.core.ImmutableTree;
-import org.apache.jackrabbit.oak.core.TreeTypeProvider;
-import org.apache.jackrabbit.oak.plugins.nodetype.ReadOnlyNodeTypeManager;
+import org.apache.jackrabbit.oak.plugins.nodetype.TypePredicate;
 import org.apache.jackrabbit.oak.spi.commit.PostValidationHook;
 import org.apache.jackrabbit.oak.spi.security.authorization.accesscontrol.AccessControlConstants;
 import org.apache.jackrabbit.oak.spi.security.authorization.permission.PermissionConstants;
@@ -51,8 +50,11 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.collect.Iterables.addAll;
+import static com.google.common.collect.Sets.newLinkedHashSet;
 import static org.apache.jackrabbit.JcrConstants.JCR_PRIMARYTYPE;
 import static org.apache.jackrabbit.JcrConstants.JCR_SYSTEM;
+import static org.apache.jackrabbit.oak.core.AbstractTree.OAK_CHILD_ORDER;
 import static org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState.EMPTY_NODE;
 
 /**
@@ -87,9 +89,12 @@ public class PermissionHook implements P
     private final PermissionEntryCache cache;
 
     private NodeBuilder permissionRoot;
-    private ReadOnlyNodeTypeManager ntMgr;
     private PrivilegeBitsProvider bitsProvider;
 
+    private TypePredicate isACL;
+    private TypePredicate isACE;
+    private TypePredicate isGrantACE;
+
     private Map<String, Acl> modified = new HashMap<String, Acl>();
     private Map<String, Acl> deleted = new HashMap<String, Acl>();
 
@@ -105,9 +110,12 @@ public class PermissionHook implements P
         NodeBuilder rootAfter = after.builder();
 
         permissionRoot = getPermissionRoot(rootAfter);
-        ntMgr = ReadOnlyNodeTypeManager.getInstance(before);
         bitsProvider = new PrivilegeBitsProvider(new ImmutableRoot(before));
 
+        isACL = new TypePredicate(after, NT_REP_ACL);
+        isACE = new TypePredicate(after, NT_REP_ACE);
+        isGrantACE = new TypePredicate(after, NT_REP_GRANT_ACE);
+
         Diff diff = new Diff("");
         after.compareAgainstBaseState(before, diff);
         apply();
@@ -125,24 +133,12 @@ public class PermissionHook implements P
         cache.flush(principalNames);
     }
 
-    private boolean isACL(@Nonnull Tree tree) {
-        return ntMgr.isNodeType(tree, NT_REP_ACL);
-    }
-
-    private boolean isACE(@Nonnull Tree tree) {
-        return ntMgr.isNodeType(tree, NT_REP_ACE);
-    }
-
     @Nonnull
     private NodeBuilder getPermissionRoot(NodeBuilder rootBuilder) {
         // permission root has been created during workspace initialization
         return rootBuilder.getChildNode(JCR_SYSTEM).getChildNode(REP_PERMISSION_STORE).getChildNode(workspaceName);
     }
 
-    private static Tree getTree(String name, NodeState nodeState) {
-        return new ImmutableTree(ImmutableTree.ParentProvider.UNSUPPORTED, name, nodeState, TreeTypeProvider.EMPTY);
-    }
-
     private class Diff extends DefaultNodeStateDiff {
 
         private final String parentPath;
@@ -158,8 +154,7 @@ public class PermissionHook implements P
                 return true;
             }
             String path = parentPath + '/' + name;
-            Tree tree = getTree(name, after);
-            if (isACL(tree)) {
+            if (isACL.apply(after)) {
                 Acl acl = new Acl(parentPath, name, new AfterNode(path, after));
                 modified.put(acl.accessControlledPath, acl);
             } else {
@@ -175,10 +170,8 @@ public class PermissionHook implements P
                 return true;
             }
             String path = parentPath + '/' + name;
-            Tree beforeTree = getTree(name, before);
-            Tree afterTree = getTree(name, after);
-            if (isACL(beforeTree)) {
-                if (isACL(afterTree)) {
+            if (isACL.apply(before)) {
+                if (isACL.apply(after)) {
                     Acl acl = new Acl(parentPath, name, new AfterNode(path, after));
                     modified.put(acl.accessControlledPath, acl);
 
@@ -193,7 +186,7 @@ public class PermissionHook implements P
                     Acl acl = new Acl(parentPath, name, new BeforeNode(path, before));
                     deleted.put(acl.accessControlledPath, acl);
                 }
-            } else if (isACL(afterTree)) {
+            } else if (isACL.apply(after)) {
                 Acl acl = new Acl(parentPath, name, new AfterNode(path, after));
                 modified.put(acl.accessControlledPath, acl);
             } else {
@@ -209,8 +202,7 @@ public class PermissionHook implements P
                 return true;
             }
             String path = parentPath + '/' + name;
-            Tree tree = getTree(name, before);
-            if (isACL(tree)) {
+            if (isACL.apply(before)) {
                 Acl acl = new Acl(parentPath, name, new BeforeNode(path, before));
                 deleted.put(acl.accessControlledPath, acl);
             } else {
@@ -280,11 +272,20 @@ public class PermissionHook implements P
                 this.accessControlledPath = aclPath.length() == 0 ? "/" : aclPath;
             }
             nodeName = PermissionUtil.getEntryName(accessControlledPath);
+
+            NodeState acl = node.getNodeState();
+            Set<String> orderedChildNames =
+                    newLinkedHashSet(acl.getNames(OAK_CHILD_ORDER));
+            long n = orderedChildNames.size();
+            if (acl.getChildNodeCount(n + 1) > n) {
+                addAll(orderedChildNames, acl.getChildNodeNames());
+            }
+
             int index = 0;
-            Tree aclTree = getTree(node.getName(), node.getNodeState());
-            for (Tree child : aclTree.getChildren()) {
-                if (isACE(child)) {
-                    AcEntry entry = new AcEntry(child, accessControlledPath, index);
+            for (String childName : orderedChildNames) {
+                NodeState ace = acl.getChildNode(childName);
+                if (isACE.apply(ace)) {
+                    AcEntry entry = new AcEntry(new ImmutableTree(ace), accessControlledPath, index);
                     List<AcEntry> list = entries.get(entry.principalName);
                     if (list == null) {
                         list = new ArrayList<AcEntry>();
@@ -436,13 +437,13 @@ public class PermissionHook implements P
         private final int index;
         private int hashCode = -1;
 
-        private AcEntry(@Nonnull Tree aceTree, @Nonnull String accessControlledPath, int index) {
+        private AcEntry(@Nonnull ImmutableTree aceTree, @Nonnull String accessControlledPath, int index) {
             this.accessControlledPath = accessControlledPath;
             this.index = index;
 
             principalName = Text.escapeIllegalJcrChars(checkNotNull(TreeUtil.getString(aceTree, REP_PRINCIPAL_NAME)));
             privilegeBits = bitsProvider.getBits(TreeUtil.getStrings(aceTree, REP_PRIVILEGES));
-            isAllow = NT_REP_GRANT_ACE.equals(TreeUtil.getPrimaryTypeName(aceTree));
+            isAllow = isGrantACE.apply(aceTree.getNodeState());
             restrictions = restrictionProvider.readRestrictions(Strings.emptyToNull(accessControlledPath), aceTree);
         }