jackrabbit-oak-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ju...@apache.org
Subject svn commit: r1465664 - /jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/SecureNodeState.java
Date Mon, 08 Apr 2013 15:21:38 GMT
Author: jukka
Date: Mon Apr  8 15:21:37 2013
New Revision: 1465664

URL: http://svn.apache.org/r1465664
Log:
OAK-709: Consider moving permission evaluation to the node state level

Calculate the readState directly when a SecureNodeState is instantiated
(may need to make it lazy again later, TBD) so we can add the getChildNode()
optimization where we return a fully readable leaf node state directly
without a SecureNodeState wrapper.

Also streamline the property access methods to avoid duplicate
checks when possible.

Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/SecureNodeState.java

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/SecureNodeState.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/SecureNodeState.java?rev=1465664&r1=1465663&r2=1465664&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/SecureNodeState.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/SecureNodeState.java
Mon Apr  8 15:21:37 2013
@@ -21,7 +21,6 @@ import static com.google.common.base.Pre
 import java.util.Collections;
 import javax.annotation.CheckForNull;
 import javax.annotation.Nonnull;
-import javax.annotation.Nullable;
 
 import com.google.common.base.Function;
 import com.google.common.base.Predicate;
@@ -60,8 +59,23 @@ public class SecureNodeState extends Abs
 
     private final PermissionProvider permissionProvider;
 
-    private ReadStatus readStatus;
+    private final ReadStatus readStatus;
+
+    /**
+     * Predicate for testing whether a given property is readable.
+     */
+    private final Predicate<PropertyState> isPropertyReadable =
+            new Predicate<PropertyState>() {
+                @Override
+                public boolean apply(@Nonnull PropertyState property) {
+                    ReadStatus status =
+                            permissionProvider.getReadStatus(base, property);
+                    return status.isAllow();
+                }
+            };
+
     private long childNodeCount = -1;
+
     private long propertyCount = -1;
 
     public SecureNodeState(@Nonnull NodeState rootState,
@@ -70,6 +84,7 @@ public class SecureNodeState extends Abs
         this.state = checkNotNull(rootState);
         this.base = new ImmutableTree(rootState, typeProvider);
         this.permissionProvider = permissionProvider;
+        this.readStatus = permissionProvider.getReadStatus(base, null);
     }
 
     private SecureNodeState(
@@ -79,36 +94,41 @@ public class SecureNodeState extends Abs
         this.base = new ImmutableTree(parent.base, name, nodeState);
         this.permissionProvider = parent.permissionProvider;
         if (base.getType() == parent.base.getType()) {
-            this.readStatus = (ReadStatus.getChildStatus(parent.readStatus));
+            this.readStatus = ReadStatus.getChildStatus(parent.readStatus);
+        } else {
+            this.readStatus = permissionProvider.getReadStatus(base, null);
         }
     }
 
     @Override
     public boolean exists() {
-        return getReadStatus().includes(ReadStatus.ALLOW_THIS);
+        return readStatus.includes(ReadStatus.ALLOW_THIS);
     }
 
     @Override @CheckForNull
     public PropertyState getProperty(String name) {
         PropertyState property = state.getProperty(name);
-        if (property != null && canReadProperty(property)) {
-            return property;
-        } else {
-            return null;
+        if (property != null
+                && !readStatus.includes(ReadStatus.ALLOW_PROPERTIES)) {
+            if (!readStatus.appliesToThis()
+                    || isPropertyReadable.apply(property)) {
+                property = null;
+            }
         }
+        return property;
     }
 
     @Override
     public synchronized long getPropertyCount() {
         if (propertyCount == -1) {
-            ReadStatus rs = getReadStatus();
-            if (rs.includes(ReadStatus.ALLOW_PROPERTIES)) {
+            if (readStatus.includes(ReadStatus.ALLOW_PROPERTIES)) {
                 propertyCount = state.getPropertyCount();
-            } else if (rs.includes(ReadStatus.DENY_PROPERTIES)) {
+            } else if (readStatus.includes(ReadStatus.DENY_PROPERTIES)
+                    || !readStatus.appliesToThis()) {
                 propertyCount = 0;
             } else {
                 propertyCount = count(Iterables.filter(
-                        state.getProperties(), isPropertyReadable()));
+                        state.getProperties(), isPropertyReadable));
             }
         }
         return propertyCount;
@@ -117,14 +137,14 @@ public class SecureNodeState extends Abs
     @Nonnull
     @Override
     public Iterable<? extends PropertyState> getProperties() {
-        ReadStatus rs = getReadStatus();
-        if (rs.includes(ReadStatus.ALLOW_PROPERTIES)) {
+        if (readStatus.includes(ReadStatus.ALLOW_PROPERTIES)) {
             return state.getProperties();
-        } else if (rs.includes(ReadStatus.DENY_PROPERTIES)) {
+        } else if (readStatus.includes(ReadStatus.DENY_PROPERTIES)
+                || !readStatus.appliesToThis()) {
             return Collections.emptySet();
         } else {
             return Iterables.filter(
-                    state.getProperties(), isPropertyReadable());
+                    state.getProperties(), isPropertyReadable);
         }
     }
 
@@ -137,11 +157,16 @@ public class SecureNodeState extends Abs
     public NodeState getChildNode(@Nonnull String name) {
         NodeState child = state.getChildNode(name);
         if (child.exists()) {
-            return new SecureNodeState(this, name, child);
-        } else {
-            // a non-existing child node
-            return child;
+            SecureNodeState secure = new SecureNodeState(this, name, child);
+            if (!secure.readStatus.includes(ReadStatus.ALLOW_ALL)
+                    || child.getChildNodeCount() > 0) {
+                // The SecureNodeState wrapper is needed if the child node
+                // 1) exists, 2) has access restrictions, or 3) contains
+                // descendants whose access control status isn't yet known
+                child = secure;
+            }
         }
+        return child;
     }
 
     @Override
@@ -153,20 +178,9 @@ public class SecureNodeState extends Abs
     }
 
     @Override
-    public Iterable<String> getChildNodeNames() {
-        return Iterables.transform(getChildNodeEntries(), new Function<ChildNodeEntry,
String>() {
-            @Override
-            public String apply(@Nullable ChildNodeEntry cnEntry) {
-                return (cnEntry == null) ? null : cnEntry.getName();
-            }
-        });
-    }
-
-    @Override
     @Nonnull
     public Iterable<? extends ChildNodeEntry> getChildNodeEntries() {
-        ReadStatus rs = getReadStatus();
-        if (rs.includes(ReadStatus.DENY_CHILDREN)) {
+        if (readStatus.includes(ReadStatus.DENY_CHILDREN)) {
             return Collections.emptySet();
         } else {
             // TODO: review if ALLOW_CHILDREN could be used as well although we
@@ -192,40 +206,6 @@ public class SecureNodeState extends Abs
 
     //--------------------------------------------------------------------------
 
-    private ReadStatus getReadStatus() {
-        if (readStatus == null) {
-            readStatus = permissionProvider.getReadStatus(base, null);
-        }
-        return readStatus;
-    }
-
-    private boolean canReadProperty(@Nonnull PropertyState property) {
-        if (readStatus == null || readStatus.appliesToThis()) {
-            ReadStatus rs = permissionProvider.getReadStatus(this.base, property);
-            if (rs.appliesToThis()) {
-                // status applies to this property only -> recalc for others
-                return rs.isAllow();
-            } else {
-                readStatus = rs;
-            }
-        }
-        return readStatus.includes(ReadStatus.ALLOW_PROPERTIES);
-    }
-
-    /**
-     * Returns a predicate for testing whether a given property is readable.
-     *
-     * @return predicate
-     */
-    private Predicate<PropertyState> isPropertyReadable() {
-        return new Predicate<PropertyState>() {
-            @Override
-            public boolean apply(@Nonnull PropertyState property) {
-                return canReadProperty(property);
-            }
-        };
-    }
-
     private class ReadableChildNodeEntries implements Function<ChildNodeEntry, ChildNodeEntry>
{
         @Override
         public ChildNodeEntry apply(ChildNodeEntry input) {



Mime
View raw message