Return-Path: X-Original-To: apmail-jackrabbit-oak-commits-archive@minotaur.apache.org Delivered-To: apmail-jackrabbit-oak-commits-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 75CBE10D96 for ; Mon, 15 Jul 2013 13:40:08 +0000 (UTC) Received: (qmail 79306 invoked by uid 500); 15 Jul 2013 13:40:06 -0000 Delivered-To: apmail-jackrabbit-oak-commits-archive@jackrabbit.apache.org Received: (qmail 78386 invoked by uid 500); 15 Jul 2013 13:40:05 -0000 Mailing-List: contact oak-commits-help@jackrabbit.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: oak-dev@jackrabbit.apache.org Delivered-To: mailing list oak-commits@jackrabbit.apache.org Received: (qmail 78083 invoked by uid 99); 15 Jul 2013 13:40:04 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 15 Jul 2013 13:40:04 +0000 X-ASF-Spam-Status: No, hits=-2000.0 required=5.0 tests=ALL_TRUSTED X-Spam-Check-By: apache.org Received: from [140.211.11.4] (HELO eris.apache.org) (140.211.11.4) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 15 Jul 2013 13:39:50 +0000 Received: from eris.apache.org (localhost [127.0.0.1]) by eris.apache.org (Postfix) with ESMTP id 6686323889EA; Mon, 15 Jul 2013 13:39:30 +0000 (UTC) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: svn commit: r1503239 - in /jackrabbit/oak/trunk: oak-core/src/main/java/org/apache/jackrabbit/oak/core/ oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/ oak-core/src/test/java/org/apache/jackrabbit/oak/ oak-jcr/src/main/java/org/ap... Date: Mon, 15 Jul 2013 13:39:30 -0000 To: oak-commits@jackrabbit.apache.org From: mduerig@apache.org X-Mailer: svnmailer-1.0.9 Message-Id: <20130715133930.6686323889EA@eris.apache.org> X-Virus-Checked: Checked by ClamAV on apache.org Author: mduerig Date: Mon Jul 15 13:39:29 2013 New Revision: 1503239 URL: http://svn.apache.org/r1503239 Log: OAK-144 Implement observation Base SecurableNodeStateDiff on Tree in order to have the hierarchy information available for evaluating access control restrictions 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/plugins/observation/SecurableNodeStateDiff.java jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/SecureNodeStateDiff.java jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/SecureNodeStateDiffTest.java jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/observation/ChangeProcessor.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=1503239&r1=1503238&r2=1503239&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 Mon Jul 15 13:39:29 2013 @@ -81,6 +81,14 @@ public abstract class AbstractTree imple } /** + * @return the underlying {@code NodeState} of this tree + */ + @Nonnull + public NodeState getNodeState() { + return nodeBuilder.getNodeState(); + } + + /** * Factory method for creating child trees * @param name name of the child tree * @return child tree of this tree with the given {@code name} @@ -247,14 +255,6 @@ public abstract class AbstractTree imple //------------------------------------------------------------< internal >--- /** - * @return the underlying {@code NodeState} of this tree - */ - @Nonnull - NodeState getNodeState() { - return nodeBuilder.getNodeState(); - } - - /** * The identifier of a tree is the value of its {@code jcr:uuid} property. * If no such property exists the identifier is a slash ({@code/}) if the * tree is the root. Otherwise the identifier is the tree's {@code name} appended Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/SecurableNodeStateDiff.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/SecurableNodeStateDiff.java?rev=1503239&r1=1503238&r2=1503239&view=diff ============================================================================== --- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/SecurableNodeStateDiff.java (original) +++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/SecurableNodeStateDiff.java Mon Jul 15 13:39:29 2013 @@ -23,6 +23,7 @@ import javax.annotation.CheckForNull; import javax.annotation.Nonnull; import org.apache.jackrabbit.oak.api.PropertyState; +import org.apache.jackrabbit.oak.api.Tree; import org.apache.jackrabbit.oak.spi.state.NodeState; import org.apache.jackrabbit.oak.spi.state.NodeStateDiff; import org.apache.jackrabbit.oak.spi.state.RecursingNodeStateDiff; @@ -34,8 +35,8 @@ import org.apache.jackrabbit.oak.spi.sta *

* Implementors must implement the {@link #create(SecurableNodeStateDiff, String, NodeState, NodeState)} * factory method for creating {@code SecurableNodeStateDiff} instances for child nodes. - * Further implementors should override {@link #canRead(PropertyState, PropertyState)} and - * {@link #canRead(String, NodeState, NodeState)} and determine whether the passed states are + * Further implementors should override {@link #canRead(Tree, PropertyState, Tree, PropertyState)} and + * {@link #canRead(Tree, Tree)} and determine whether the passed states are * accessible and the respective callbacks should thus be invoked. Finally implementors should override, * {@link #secureBefore(String, NodeState)}, and {@link #secureAfter(String, NodeState)}} wrapping the * passed node state into a node state that restricts access to accessible child nodes and properties. @@ -48,6 +49,16 @@ public abstract class SecurableNodeState private final SecurableNodeStateDiff parent; /** + * Tree before the changes + */ + protected final Tree beforeTree; + + /** + * Tree after the changes + */ + protected final Tree afterTree; + + /** * Unsecured diff this secured diff delegates to after it has determined * that the items pertaining to a call back are accessible. */ @@ -61,25 +72,32 @@ public abstract class SecurableNodeState */ private Deferred deferred = Deferred.EMPTY; - private SecurableNodeStateDiff(SecurableNodeStateDiff parent, RecursingNodeStateDiff diff) { + private SecurableNodeStateDiff(SecurableNodeStateDiff parent, Tree beforeTree, Tree afterTree, RecursingNodeStateDiff diff) { this.parent = parent; + this.beforeTree = beforeTree; + this.afterTree = afterTree; this.diff = diff; } /** * Create a new child instance * @param parent parent of this instance + * @param beforeParent parent tree before the changes + * @param afterParent parent tree after the changes + * @param name name of the child node */ - protected SecurableNodeStateDiff(SecurableNodeStateDiff parent) { - this(parent, RecursingNodeStateDiff.EMPTY); + protected SecurableNodeStateDiff(SecurableNodeStateDiff parent, Tree beforeParent, Tree afterParent, String name) { + this(parent, beforeParent.getChild(name), afterParent.getChild(name), RecursingNodeStateDiff.EMPTY); } /** * Create a new instance wrapping a unsecured diff. * @param diff unsecured diff + * @param beforeTree parent tree before the changes + * @param afterTree parent tree after the changes */ - protected SecurableNodeStateDiff(RecursingNodeStateDiff diff) { - this(null, diff); + protected SecurableNodeStateDiff(RecursingNodeStateDiff diff, Tree beforeTree, Tree afterTree) { + this(null, beforeTree, afterTree, diff); } /** @@ -96,11 +114,13 @@ public abstract class SecurableNodeState /** * Determine whether a property is accessible + * @param beforeParent parent before the changes * @param before before state of the property + * @param afterParent parent after the changes * @param after after state of the property * @return {@code true} if accessible, {@code false} otherwise. */ - protected boolean canRead(PropertyState before, PropertyState after) { + protected boolean canRead(Tree beforeParent, PropertyState before, Tree afterParent, PropertyState after) { return true; } @@ -110,7 +130,7 @@ public abstract class SecurableNodeState * @param after after state of the node * @return {@code true} if accessible, {@code false} otherwise. */ - protected boolean canRead(String name, NodeState before, NodeState after) { + protected boolean canRead(Tree before, Tree after) { return true; } @@ -138,9 +158,11 @@ public abstract class SecurableNodeState return nodeState; } + //------------------------------------------------------------< NodeStateDiff >--- + @Override public boolean propertyAdded(PropertyState after) { - if (canRead(null, after)) { + if (canRead(beforeTree, null, afterTree, after)) { return applyDeferred() && diff.propertyAdded(after); } else { @@ -150,7 +172,7 @@ public abstract class SecurableNodeState @Override public boolean propertyChanged(PropertyState before, PropertyState after) { - if (canRead(before, after)) { + if (canRead(beforeTree, before, afterTree, after)) { return applyDeferred() && diff.propertyChanged(before, after); } else { @@ -160,7 +182,7 @@ public abstract class SecurableNodeState @Override public boolean propertyDeleted(PropertyState before) { - if (canRead(before, null)) { + if (canRead(beforeTree, before, afterTree, null)) { return applyDeferred() && diff.propertyDeleted(before); } else { @@ -170,7 +192,7 @@ public abstract class SecurableNodeState @Override public boolean childNodeAdded(String name, NodeState after) { - if (canRead(name, null, after)) { + if (canRead(beforeTree.getChild(name), afterTree.getChild(name))) { return applyDeferred() && diff.childNodeAdded(name, secureAfter(name, after)); } else { return true; @@ -202,7 +224,7 @@ public abstract class SecurableNodeState @Override public boolean childNodeDeleted(String name, NodeState before) { - if (canRead(name, before, null)) { + if (canRead(beforeTree.getChild(name), afterTree.getChild(name))) { return applyDeferred() && diff.childNodeDeleted(name, secureBefore(name, before)); } else { return true; Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/SecureNodeStateDiff.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/SecureNodeStateDiff.java?rev=1503239&r1=1503238&r2=1503239&view=diff ============================================================================== --- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/SecureNodeStateDiff.java (original) +++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/SecureNodeStateDiff.java Mon Jul 15 13:39:29 2013 @@ -22,38 +22,40 @@ package org.apache.jackrabbit.oak.plugin import static org.apache.jackrabbit.oak.spi.state.NodeStateUtils.isHidden; import org.apache.jackrabbit.oak.api.PropertyState; +import org.apache.jackrabbit.oak.api.Tree; +import org.apache.jackrabbit.oak.core.ImmutableTree; import org.apache.jackrabbit.oak.spi.state.NodeState; -import org.apache.jackrabbit.oak.spi.state.NodeStateDiff; import org.apache.jackrabbit.oak.spi.state.RecursingNodeStateDiff; public class SecureNodeStateDiff extends SecurableNodeStateDiff { - private SecureNodeStateDiff(RecursingNodeStateDiff diff) { - super(diff); + private SecureNodeStateDiff(RecursingNodeStateDiff diff, Tree before, Tree after) { + super(diff, before, after); } - private SecureNodeStateDiff(SecurableNodeStateDiff parent) { - super(parent); + private SecureNodeStateDiff(SecurableNodeStateDiff parent, Tree beforeParent, Tree afterParent, String name) { + super(parent, beforeParent, afterParent, name); } - public static NodeStateDiff wrap(RecursingNodeStateDiff diff) { - return new SecureNodeStateDiff(diff); + public static void compare(RecursingNodeStateDiff diff, ImmutableTree before, ImmutableTree after) { + SecureNodeStateDiff secureDiff = new SecureNodeStateDiff(diff, before, after); + after.getNodeState().compareAgainstBaseState(before.getNodeState(), secureDiff); } @Override protected SecurableNodeStateDiff create(SecurableNodeStateDiff parent, String name, NodeState before, NodeState after) { - return isHidden(name) ? null : new SecureNodeStateDiff(parent); + return isHidden(name) ? null : new SecureNodeStateDiff(parent, beforeTree, afterTree, name); } @Override - protected boolean canRead(PropertyState before, PropertyState after) { + protected boolean canRead(Tree beforeParent, PropertyState before, Tree afterParent, PropertyState after) { // TODO implement canRead return true; } @Override - protected boolean canRead(String name, NodeState before, NodeState after) { + protected boolean canRead(Tree before, Tree after) { // TODO implement canRead return true; } Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/SecureNodeStateDiffTest.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/SecureNodeStateDiffTest.java?rev=1503239&r1=1503238&r2=1503239&view=diff ============================================================================== --- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/SecureNodeStateDiffTest.java (original) +++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/SecureNodeStateDiffTest.java Mon Jul 15 13:39:29 2013 @@ -23,11 +23,13 @@ import static org.apache.jackrabbit.oak. import static org.junit.Assert.assertEquals; import org.apache.jackrabbit.oak.api.PropertyState; -import org.apache.jackrabbit.oak.spi.state.RecursingNodeStateDiff; +import org.apache.jackrabbit.oak.api.Tree; +import org.apache.jackrabbit.oak.core.ImmutableTree; import org.apache.jackrabbit.oak.plugins.observation.SecurableNodeStateDiff; import org.apache.jackrabbit.oak.spi.state.NodeBuilder; import org.apache.jackrabbit.oak.spi.state.NodeState; import org.apache.jackrabbit.oak.spi.state.NodeStateDiff; +import org.apache.jackrabbit.oak.spi.state.RecursingNodeStateDiff; import org.junit.Before; import org.junit.Test; @@ -113,26 +115,26 @@ public class SecureNodeStateDiffTest { } private static class SecureNodeStateDiff extends SecurableNodeStateDiff { - protected SecureNodeStateDiff(SecurableNodeStateDiff parent) { - super(parent); + protected SecureNodeStateDiff(SecurableNodeStateDiff parent, Tree beforeParent, Tree afterParent, String name) { + super(parent, beforeParent, afterParent, name); } - public static NodeStateDiff wrap(RecursingNodeStateDiff diff) { - return new SecureNodeStateDiff(diff); + public static NodeStateDiff wrap(RecursingNodeStateDiff diff, Tree before, Tree after) { + return new SecureNodeStateDiff(diff, before, after); } - private SecureNodeStateDiff(RecursingNodeStateDiff diff) { - super(diff); + private SecureNodeStateDiff(RecursingNodeStateDiff diff, Tree before, Tree after) { + super(diff, before, after); } @Override protected SecurableNodeStateDiff create(SecurableNodeStateDiff parent, String name, NodeState before, NodeState after) { - return new SecureNodeStateDiff(parent); + return new SecureNodeStateDiff(parent, beforeTree, afterTree, name); } @Override - protected boolean canRead(PropertyState before, PropertyState after) { + protected boolean canRead(Tree beforeParent, PropertyState before, Tree afterParent, PropertyState after) { return canRead(before) && canRead(after); } @@ -141,8 +143,8 @@ public class SecureNodeStateDiffTest { } @Override - protected boolean canRead(String name, NodeState before, NodeState after) { - return canRead(name); + protected boolean canRead(Tree before, Tree after) { + return canRead(before.getName()); } private static boolean canRead(String name) { @@ -161,7 +163,8 @@ public class SecureNodeStateDiffTest { } public void expect(String expected) { - after.compareAgainstBaseState(before, SecureNodeStateDiff.wrap(this)); + NodeStateDiff secureDiff = SecureNodeStateDiff.wrap(this, new ImmutableTree(before), new ImmutableTree(after)); + after.compareAgainstBaseState(before, secureDiff); assertEquals(expected, actual.toString()); } Modified: jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/observation/ChangeProcessor.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/observation/ChangeProcessor.java?rev=1503239&r1=1503238&r2=1503239&view=diff ============================================================================== --- jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/observation/ChangeProcessor.java (original) +++ jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/observation/ChangeProcessor.java Mon Jul 15 13:39:29 2013 @@ -28,7 +28,6 @@ import static javax.jcr.observation.Even import static javax.jcr.observation.Event.PROPERTY_ADDED; import static javax.jcr.observation.Event.PROPERTY_REMOVED; import static org.apache.jackrabbit.oak.core.IdentifierManager.getIdentifier; -import static org.apache.jackrabbit.oak.spi.state.NodeStateUtils.getNode; import java.util.ArrayList; import java.util.Iterator; @@ -41,22 +40,22 @@ import javax.jcr.observation.EventListen import com.google.common.base.Function; import com.google.common.collect.Iterators; +import com.google.common.collect.Lists; import org.apache.jackrabbit.api.jmx.EventListenerMBean; import org.apache.jackrabbit.commons.iterator.EventIteratorAdapter; import org.apache.jackrabbit.commons.observation.ListenerTracker; import org.apache.jackrabbit.oak.api.ContentSession; import org.apache.jackrabbit.oak.api.PropertyState; -import org.apache.jackrabbit.oak.api.Root; import org.apache.jackrabbit.oak.api.Tree; import org.apache.jackrabbit.oak.commons.PathUtils; import org.apache.jackrabbit.oak.core.ImmutableRoot; +import org.apache.jackrabbit.oak.core.ImmutableTree; import org.apache.jackrabbit.oak.namepath.NamePathMapper; import org.apache.jackrabbit.oak.plugins.observation.ChangeDispatcher.ChangeSet; import org.apache.jackrabbit.oak.plugins.observation.ChangeDispatcher.Listener; import org.apache.jackrabbit.oak.plugins.observation.Observable; import org.apache.jackrabbit.oak.plugins.observation.SecureNodeStateDiff; import org.apache.jackrabbit.oak.spi.state.NodeState; -import org.apache.jackrabbit.oak.spi.state.NodeStateDiff; import org.apache.jackrabbit.oak.spi.state.RecursingNodeStateDiff; import org.apache.jackrabbit.oak.spi.state.VisibleDiff; import org.apache.jackrabbit.oak.spi.whiteboard.Registration; @@ -194,13 +193,11 @@ class ChangeProcessor implements Runnabl while (!stopping && changes != null) { EventFilter filter = filterRef.get(); if (!(filter.excludeLocal() && changes.isLocal(contentSession))) { - NodeState beforeState = changes.getBeforeState(); - NodeState afterState = changes.getAfterState(); String path = namePathMapper.getOakPath(filter.getPath()); - EventGeneratingNodeStateDiff diff = new EventGeneratingNodeStateDiff( - changes, new ImmutableRoot(beforeState), new ImmutableRoot(afterState), path); - NodeStateDiff secureDiff = SecureNodeStateDiff.wrap(VisibleDiff.wrap(diff)); - getNode(afterState, path).compareAgainstBaseState(getNode(beforeState, path), secureDiff); + ImmutableTree beforeTree = getTree(changes.getBeforeState(), path); + ImmutableTree afterTree = getTree(changes.getAfterState(), path); + EventGeneratingNodeStateDiff diff = new EventGeneratingNodeStateDiff(changes, beforeTree, afterTree); + SecureNodeStateDiff.compare(VisibleDiff.wrap(diff), beforeTree, afterTree); if (!stopping) { diff.sendEvents(); } @@ -218,6 +215,10 @@ class ChangeProcessor implements Runnabl } } + private static ImmutableTree getTree(NodeState beforeState, String path) { + return new ImmutableRoot(beforeState).getTree(path); + } + //------------------------------------------------------------< private >--- private class EventGeneratingNodeStateDiff extends RecursingNodeStateDiff { @@ -237,8 +238,8 @@ class ChangeProcessor implements Runnabl this.events = events; } - public EventGeneratingNodeStateDiff(ChangeSet changes, Root beforeRoot, Root afterRoot, String path) { - this(changes, beforeRoot.getTree(path), afterRoot.getTree(path), new ArrayList>(EVENT_LIMIT)); + public EventGeneratingNodeStateDiff(ChangeSet changes, Tree beforeTree, Tree afterTree) { + this(changes, beforeTree, afterTree, Lists.>newArrayList()); } public void sendEvents() {