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/09/21 02:54:03 UTC

svn commit: r1525185 - in /jackrabbit/oak/trunk/oak-core/src: main/java/org/apache/jackrabbit/oak/core/ main/java/org/apache/jackrabbit/oak/kernel/ main/java/org/apache/jackrabbit/oak/plugins/memory/ main/java/org/apache/jackrabbit/oak/plugins/segment/...

Author: jukka
Date: Sat Sep 21 00:54:02 2013
New Revision: 1525185

URL: http://svn.apache.org/r1525185
Log:
OAK-659: Move purge logic for transient changes below the NodeBuilder interface

Remove NodeBuilder.reset() as the NodeStore.reset() method is better positioned

Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/SecureNodeBuilder.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeStore.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeBuilder.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeStore.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/SegmentNodeStore.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/SegmentNodeStoreService.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/AbstractNodeStore.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/NodeBuilder.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/NodeStore.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/ReadOnlyBuilder.java
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeBuilderTest.java

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/SecureNodeBuilder.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/SecureNodeBuilder.java?rev=1525185&r1=1525184&r2=1525185&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/SecureNodeBuilder.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/SecureNodeBuilder.java Sat Sep 21 00:54:02 2013
@@ -140,13 +140,6 @@ class SecureNodeBuilder implements NodeB
         return builder.isModified();
     }
 
-    @Override
-    public void reset(@Nonnull NodeState state) throws IllegalStateException {
-        builder.reset(state); // NOTE: can be dangerous with SecureNodeState
-        baseRevision++;
-        securityContext = null;
-    }
-
     public void baseChanged() {
         baseRevision++;
         securityContext = null;

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeStore.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeStore.java?rev=1525185&r1=1525184&r2=1525185&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeStore.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeStore.java Sat Sep 21 00:54:02 2013
@@ -34,10 +34,12 @@ import com.google.common.cache.LoadingCa
 import com.google.common.cache.Weigher;
 import com.google.common.util.concurrent.ListenableFuture;
 import com.google.common.util.concurrent.SettableFuture;
+
 import org.apache.jackrabbit.mk.api.MicroKernel;
 import org.apache.jackrabbit.mk.api.MicroKernelException;
 import org.apache.jackrabbit.oak.api.CommitFailedException;
 import org.apache.jackrabbit.oak.cache.CacheStats;
+import org.apache.jackrabbit.oak.plugins.memory.MemoryNodeBuilder;
 import org.apache.jackrabbit.oak.spi.commit.CommitHook;
 import org.apache.jackrabbit.oak.spi.commit.EmptyObserver;
 import org.apache.jackrabbit.oak.spi.commit.Observer;
@@ -135,6 +137,12 @@ public class KernelNodeStore extends Abs
         this.observer = checkNotNull(observer);
     }
 
+    @Override
+    protected void reset(NodeBuilder builder, NodeState state) {
+        checkArgument(builder instanceof MemoryNodeBuilder);
+        ((MemoryNodeBuilder) builder).reset(state);
+    }
+
     //----------------------------------------------------------< NodeStore >---
 
     @Override

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeBuilder.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeBuilder.java?rev=1525185&r1=1525184&r2=1525185&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeBuilder.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeBuilder.java Sat Sep 21 00:54:02 2013
@@ -213,6 +213,12 @@ public class MemoryNodeBuilder implement
         return name;
     }
 
+    public void reset(NodeState newBase) {
+        base = checkNotNull(newBase);
+        baseRevision++;
+        head().reset();
+    }
+
     //--------------------------------------------------------< NodeBuilder >---
 
     @Override
@@ -241,13 +247,6 @@ public class MemoryNodeBuilder implement
     }
 
     @Override
-    public void reset(NodeState newBase) {
-        base = checkNotNull(newBase);
-        baseRevision++;
-        head().reset();
-    }
-
-    @Override
     public long getChildNodeCount(long max) {
         return head().getCurrentNodeState().getChildNodeCount(max);
     }

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeStore.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeStore.java?rev=1525185&r1=1525184&r2=1525185&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeStore.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeStore.java Sat Sep 21 00:54:02 2013
@@ -31,11 +31,13 @@ import javax.annotation.CheckForNull;
 import javax.annotation.Nonnull;
 
 import com.google.common.io.ByteStreams;
+
 import org.apache.jackrabbit.oak.api.CommitFailedException;
 import org.apache.jackrabbit.oak.spi.commit.CommitHook;
 import org.apache.jackrabbit.oak.spi.commit.PostCommitHook;
 import org.apache.jackrabbit.oak.spi.state.AbstractNodeStore;
 import org.apache.jackrabbit.oak.spi.state.AbstractNodeStoreBranch;
+import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
 import org.apache.jackrabbit.oak.spi.state.NodeState;
 import org.apache.jackrabbit.oak.spi.state.NodeStoreBranch;
 
@@ -51,6 +53,12 @@ public class MemoryNodeStore extends Abs
     private final Map<String, NodeState> checkpoints = newHashMap();
 
     @Override
+    protected void reset(NodeBuilder builder, NodeState state) {
+        checkArgument(builder instanceof MemoryNodeBuilder);
+        ((MemoryNodeBuilder) builder).reset(state);
+    }
+
+    @Override
     public NodeState getRoot() {
         return root.get();
     }

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/SegmentNodeStore.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/SegmentNodeStore.java?rev=1525185&r1=1525184&r2=1525185&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/SegmentNodeStore.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/SegmentNodeStore.java Sat Sep 21 00:54:02 2013
@@ -31,12 +31,12 @@ import org.apache.jackrabbit.oak.spi.com
 import org.apache.jackrabbit.oak.spi.commit.EmptyObserver;
 import org.apache.jackrabbit.oak.spi.commit.Observer;
 import org.apache.jackrabbit.oak.spi.commit.PostCommitHook;
-import org.apache.jackrabbit.oak.spi.state.AbstractNodeStore;
 import org.apache.jackrabbit.oak.spi.state.ConflictAnnotatingRebaseDiff;
 import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
 import org.apache.jackrabbit.oak.spi.state.NodeState;
+import org.apache.jackrabbit.oak.spi.state.NodeStore;
 
-public class SegmentNodeStore extends AbstractNodeStore {
+public class SegmentNodeStore implements NodeStore {
 
     static final String ROOT = "root";
 
@@ -84,6 +84,7 @@ public class SegmentNodeStore extends Ab
             @Nonnull NodeBuilder builder,
             @Nonnull CommitHook commitHook, PostCommitHook committed)
             throws CommitFailedException {
+        checkArgument(builder instanceof SegmentNodeBuilder);
         checkNotNull(commitHook);
         SegmentNodeState head = getHead();
         rebase(builder, head.getChildNode(ROOT)); // TODO: can we avoid this?
@@ -91,7 +92,7 @@ public class SegmentNodeStore extends Ab
                 this, new SegmentWriter(store), head);
         branch.setRoot(builder.getNodeState());
         NodeState merged = branch.merge(commitHook, committed);
-        builder.reset(merged);
+        ((SegmentNodeBuilder) builder).reset(merged);
         return merged;
     }
 
@@ -101,10 +102,11 @@ public class SegmentNodeStore extends Ab
     }
 
     private NodeState rebase(@Nonnull NodeBuilder builder, NodeState newBase) {
+        checkArgument(builder instanceof SegmentNodeBuilder);
         NodeState oldBase = builder.getBaseState();
         if (!SegmentNodeState.fastEquals(oldBase, newBase)) {
             NodeState head = builder.getNodeState();
-            builder.reset(newBase);
+            ((SegmentNodeBuilder) builder).reset(newBase);
             head.compareAgainstBaseState(oldBase, new ConflictAnnotatingRebaseDiff(builder));
         }
         return builder.getNodeState();
@@ -112,8 +114,9 @@ public class SegmentNodeStore extends Ab
 
     @Override @Nonnull
     public NodeState reset(@Nonnull NodeBuilder builder) {
+        checkArgument(builder instanceof SegmentNodeBuilder);
         NodeState state = getRoot();
-        checkNotNull(builder).reset(state);
+        ((SegmentNodeBuilder) builder).reset(state);
         return state;
     }
 

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/SegmentNodeStoreService.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/SegmentNodeStoreService.java?rev=1525185&r1=1525184&r2=1525185&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/SegmentNodeStoreService.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/SegmentNodeStoreService.java Sat Sep 21 00:54:02 2013
@@ -40,7 +40,6 @@ import org.apache.jackrabbit.oak.plugins
 import org.apache.jackrabbit.oak.plugins.segment.mongo.MongoStore;
 import org.apache.jackrabbit.oak.spi.commit.CommitHook;
 import org.apache.jackrabbit.oak.spi.commit.PostCommitHook;
-import org.apache.jackrabbit.oak.spi.state.AbstractNodeStore;
 import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
 import org.apache.jackrabbit.oak.spi.state.NodeState;
 import org.apache.jackrabbit.oak.spi.state.NodeStore;
@@ -51,7 +50,7 @@ import org.osgi.service.component.Compon
 
 @Component(policy = ConfigurationPolicy.REQUIRE)
 @Service(NodeStore.class)
-public class SegmentNodeStoreService extends AbstractNodeStore {
+public class SegmentNodeStoreService implements NodeStore {
 
     @Property(description="The unique name of this instance")
     public static final String NAME = "name";

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/AbstractNodeStore.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/AbstractNodeStore.java?rev=1525185&r1=1525184&r2=1525185&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/AbstractNodeStore.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/AbstractNodeStore.java Sat Sep 21 00:54:02 2013
@@ -31,6 +31,18 @@ import org.apache.jackrabbit.oak.spi.com
 public abstract class AbstractNodeStore implements NodeStore {
 
     /**
+     * Replaces the base state of the given builder and throws away all
+     * changes in it. The effect of this method is equivalent to replacing
+     * the builder (and the connected subtree) with a new builder returned
+     * by {@code state.builder()}.
+     *
+     * @param state new base state
+     * @throws IllegalArgumentException if the builder is not acquired
+     *                                  from a root state of this store
+     */
+    protected abstract void reset(NodeBuilder builder, NodeState state);
+
+    /**
      * This default implementation is equal to first rebasing the builder
      * and then applying it to a new branch and immediately merging it back.
      * <p>
@@ -41,6 +53,8 @@ public abstract class AbstractNodeStore 
      * @param committed  the pos commit hook
      * @return the node state resulting from the merge.
      * @throws CommitFailedException
+     * @throws IllegalArgumentException if the builder is not acquired
+     *                                  from a root state of this store
      */
     @Override
     public NodeState merge(@Nonnull NodeBuilder builder, @Nonnull CommitHook commitHook,
@@ -50,7 +64,7 @@ public abstract class AbstractNodeStore 
         NodeStoreBranch branch = branch();
         branch.setRoot(builder.getNodeState());
         NodeState merged = branch.merge(commitHook, committed);
-        builder.reset(merged);
+        reset(builder, merged);
         return merged;
     }
 
@@ -61,14 +75,21 @@ public abstract class AbstractNodeStore 
      * conflicts.
      * @param builder  the builder to rebase
      * @return the node state resulting from the rebase.
+     * @throws IllegalArgumentException if the builder is not acquired
+     *                                  from a root state of this store
      */
     @Override
     public NodeState rebase(@Nonnull NodeBuilder builder) {
         NodeState head = checkNotNull(builder).getNodeState();
         NodeState base = builder.getBaseState();
-        builder.reset(getRoot());
-        head.compareAgainstBaseState(base, new ConflictAnnotatingRebaseDiff(builder));
-        return builder.getNodeState();
+        NodeState newBase = getRoot();
+        if (base != newBase) {
+            reset(builder, newBase);
+            head.compareAgainstBaseState(
+                    base, new ConflictAnnotatingRebaseDiff(builder));
+            head = builder.getNodeState();
+        }
+        return head;
     }
 
     /**
@@ -76,11 +97,14 @@ public abstract class AbstractNodeStore 
      * the store and returning the resulting node state from the builder.
      * @param builder the builder to reset
      * @return the node state resulting from the reset.
+     * @throws IllegalArgumentException if the builder is not acquired
+     *                                  from a root state of this store
      */
     @Override
     public NodeState reset(@Nonnull NodeBuilder builder) {
-        builder.reset(getRoot());
-        return builder.getNodeState();
+        NodeState head = getRoot();
+        reset(builder, head);
+        return head;
     }
 
 //------------------------------------------------------------< Object >--

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/NodeBuilder.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/NodeBuilder.java?rev=1525185&r1=1525184&r2=1525185&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/NodeBuilder.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/NodeBuilder.java Sat Sep 21 00:54:02 2013
@@ -113,23 +113,6 @@ public interface NodeBuilder {
     boolean isModified();
 
     /**
-     * Replaces the base state of this builder and throws away all changes.
-     * The effect of this method is equivalent to replacing this builder
-     * (and the connected subtree) with a new builder returned by
-     * {@code state.builder()}.
-     * <p>
-     * This method only works on builders acquired directly from a call
-     * to {@link NodeState#builder()}. Calling it on a builder returned
-     * by the {@link #child(String)} method will throw an
-     * {@link IllegalStateException}.
-     *
-     * @param state new base state
-     * @throws IllegalStateException if this is not a root builder
-     */
-    void reset(@Nonnull NodeState state)
-        throws IllegalStateException;
-
-    /**
      * Returns the current number of child nodes.
      * <p>
      * If an implementation does know the exact value, it returns it (even if

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/NodeStore.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/NodeStore.java?rev=1525185&r1=1525184&r2=1525185&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/NodeStore.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/NodeStore.java Sat Sep 21 00:54:02 2013
@@ -54,6 +54,8 @@ public interface NodeStore {
      * @param committed  the post commit hook
      * @return the node state resulting from the merge.
      * @throws CommitFailedException if the merge failed
+     * @throws IllegalArgumentException if the builder is not acquired
+     *                                  from a root state of this store
      */
     @Nonnull
     NodeState merge(@Nonnull NodeBuilder builder, @Nonnull CommitHook commitHook,
@@ -61,8 +63,11 @@ public interface NodeStore {
 
     /**
      * Rebase the changes in the passed {@code builder} on top of the current root state.
+     *
      * @param builder  the builder to rebase
      * @return the node state resulting from the rebase.
+     * @throws IllegalArgumentException if the builder is not acquired
+     *                                  from a root state of this store
      */
     @Nonnull
     NodeState rebase(@Nonnull NodeBuilder builder);
@@ -70,8 +75,11 @@ public interface NodeStore {
     /**
      * Reset the passed {@code builder} by throwing away all its changes and
      * setting its base state to the current root state.
+     *
      * @param builder the builder to reset
      * @return the node state resulting from the reset.
+     * @throws IllegalArgumentException if the builder is not acquired
+     *                                  from a root state of this store
      */
     NodeState reset(@Nonnull NodeBuilder builder);
 

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/ReadOnlyBuilder.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/ReadOnlyBuilder.java?rev=1525185&r1=1525184&r2=1525185&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/ReadOnlyBuilder.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/ReadOnlyBuilder.java Sat Sep 21 00:54:02 2013
@@ -68,11 +68,6 @@ public class ReadOnlyBuilder implements 
     }
 
     @Override
-    public void reset(NodeState state) {
-        throw unsupported();
-    }
-
-    @Override
     public long getChildNodeCount(long max) {
         return state.getChildNodeCount(max);
     }

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeBuilderTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeBuilderTest.java?rev=1525185&r1=1525184&r2=1525185&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeBuilderTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeBuilderTest.java Sat Sep 21 00:54:02 2013
@@ -144,7 +144,7 @@ public class MemoryNodeBuilderTest {
         assertTrue(child.hasChildNode("new"));
         assertTrue(root.child("x").hasChildNode("new"));
 
-        root.reset(base);
+        ((MemoryNodeBuilder) root).reset(base);
         assertFalse(child.hasChildNode("new"));
         assertFalse(root.child("x").hasChildNode("new"));
     }
@@ -155,7 +155,7 @@ public class MemoryNodeBuilderTest {
         NodeBuilder x = root.child("x");
         x.child("y");
 
-        root.reset(base);
+        ((MemoryNodeBuilder) root).reset(base);
         assertTrue(root.hasChildNode("x"));
         assertFalse(x.hasChildNode("y"));
     }