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 mr...@apache.org on 2018/04/23 07:41:27 UTC

svn commit: r1829826 - in /jackrabbit/oak/trunk/oak-store-document/src: main/java/org/apache/jackrabbit/oak/plugins/document/ main/java/org/apache/jackrabbit/oak/plugins/document/util/ test/java/org/apache/jackrabbit/oak/plugins/document/

Author: mreutegg
Date: Mon Apr 23 07:41:26 2018
New Revision: 1829826

URL: http://svn.apache.org/viewvc?rev=1829826&view=rev
Log:
OAK-7401: Changes kept in memory when update limit is hit in commit hook

Added:
    jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentBranchRootNodeState.java   (with props)
    jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/ModifiedDocumentNodeState.java   (with props)
    jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranchTest.java   (with props)
Modified:
    jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeState.java
    jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
    jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranch.java
    jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentRootBuilder.java
    jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/CountingDiff.java
    jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/ClusterConflictTest.java
    jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStateTest.java
    jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranchInCommitHookTest.java
    jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java
    jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryRandomizedIT.java
    jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/OrphanedBranchTest.java
    jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/TestUtils.java
    jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/ValueMapTest.java
    jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VisibleChangesTest.java

Added: jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentBranchRootNodeState.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentBranchRootNodeState.java?rev=1829826&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentBranchRootNodeState.java (added)
+++ jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentBranchRootNodeState.java Mon Apr 23 07:41:26 2018
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.oak.plugins.document;
+
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+
+import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+
+/**
+ * The root node state of a persisted branch.
+ */
+class DocumentBranchRootNodeState extends DocumentNodeState {
+
+    private final DocumentNodeStore store;
+    private final DocumentNodeStoreBranch branch;
+
+    DocumentBranchRootNodeState(@Nonnull DocumentNodeStore store,
+                                @Nonnull DocumentNodeStoreBranch branch,
+                                @Nonnull String path,
+                                @Nonnull RevisionVector rootRevision,
+                                @Nullable RevisionVector lastRevision,
+                                @Nonnull BundlingContext bundlingContext) {
+        super(store, path, lastRevision, rootRevision, false, bundlingContext);
+        this.store = store;
+        this.branch = checkNotNull(branch);
+    }
+
+    @Nonnull
+    @Override
+    public NodeBuilder builder() {
+        return new DocumentRootBuilder(store.getRoot(getRootRevision()), store, branch);
+    }
+}

Propchange: jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentBranchRootNodeState.java
------------------------------------------------------------------------------
    svn:eol-style = native

Modified: jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeState.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeState.java?rev=1829826&r1=1829825&r2=1829826&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeState.java (original)
+++ jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeState.java Mon Apr 23 07:41:26 2018
@@ -61,6 +61,7 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.base.Preconditions.checkState;
 import static org.apache.jackrabbit.oak.commons.PathUtils.concat;
 import static org.apache.jackrabbit.oak.commons.StringUtils.estimateMemoryUsage;
 
@@ -121,7 +122,7 @@ public class DocumentNodeState extends A
                 fromExternalChange, createBundlingContext(checkNotNull(properties), hasChildren));
     }
 
-    private DocumentNodeState(@Nonnull DocumentNodeStore store,
+    protected DocumentNodeState(@Nonnull DocumentNodeStore store,
                               @Nonnull String path,
                               @Nullable RevisionVector lastRevision,
                               @Nullable RevisionVector rootRevision,
@@ -170,6 +171,23 @@ public class DocumentNodeState extends A
     }
 
     /**
+     * Returns this state as a branch root state connected to the given
+     * {@code branch}.
+     *
+     * @param branch the branch instance.
+     * @return a {@link DocumentBranchRootNodeState} connected to the given
+     *      {@code branch}.
+     * @throws IllegalStateException if this is not a root node state or does
+     *      not represent a branch state.
+     */
+    @Nonnull
+    DocumentNodeState asBranchRootState(@Nonnull DocumentNodeStoreBranch branch) {
+        checkState(PathUtils.denotesRoot(path));
+        checkState(getRootRevision().isBranch());
+        return new DocumentBranchRootNodeState(store, branch, path, rootRevision, lastRevision, bundlingContext);
+    }
+
+    /**
      * @return {@code true} if this node state was created as a result of an
      *          external change; {@code false} otherwise.
      */
@@ -333,27 +351,9 @@ public class DocumentNodeState extends A
     public NodeBuilder builder() {
         if ("/".equals(getPath())) {
             if (getRootRevision().isBranch()) {
-                // check if this node state is head of a branch
-                Branch b = store.getBranches().getBranch(getRootRevision());
-                if (b == null) {
-                    if (store.isDisableBranches()) {
-                        if (DocumentNodeStoreBranch.getCurrentBranch() != null) {
-                            return new DocumentRootBuilder(this, store);
-                        } else {
-                            return new MemoryNodeBuilder(this);
-                        }
-                    } else {
-                        throw new IllegalStateException("No branch for revision: " + getRootRevision());
-                    }
-                }
-                if (b.isHead(getRootRevision().getBranchRevision())
-                        && DocumentNodeStoreBranch.getCurrentBranch() != null) {
-                    return new DocumentRootBuilder(this, store);
-                } else {
-                    return new MemoryNodeBuilder(this);
-                }
+                throw new IllegalStateException("Cannot create builder from branched DocumentNodeState");
             } else {
-                return new DocumentRootBuilder(this, store);
+                return new DocumentRootBuilder(this, store, store.createBranch(this));
             }
         } else {
             return new MemoryNodeBuilder(this);
@@ -816,7 +816,7 @@ public class DocumentNodeState extends A
         return props.containsKey(key);
     }
 
-    private static class BundlingContext {
+    protected static class BundlingContext {
         final Matcher matcher;
         final Map<String, PropertyState> rootProperties;
         final boolean hasBundledChildren;

Modified: jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java?rev=1829826&r1=1829825&r2=1829826&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java (original)
+++ jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java Mon Apr 23 07:41:26 2018
@@ -1601,10 +1601,6 @@ public final class DocumentNodeStore
 
     @Nonnull
     DocumentNodeStoreBranch createBranch(DocumentNodeState base) {
-        DocumentNodeStoreBranch b = DocumentNodeStoreBranch.getCurrentBranch();
-        if (b != null) {
-            return b;
-        }
         return new DocumentNodeStoreBranch(this, base, mergeLock);
     }
 

Modified: jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranch.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranch.java?rev=1829826&r1=1829825&r2=1829826&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranch.java (original)
+++ jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranch.java Mon Apr 23 07:41:26 2018
@@ -27,8 +27,6 @@ import static org.apache.jackrabbit.oak.
 import java.util.HashSet;
 import java.util.Random;
 import java.util.Set;
-import java.util.concurrent.Callable;
-import java.util.concurrent.ConcurrentMap;
 import java.util.concurrent.locks.Lock;
 import java.util.concurrent.locks.ReadWriteLock;
 
@@ -37,11 +35,11 @@ import javax.annotation.Nonnull;
 
 import com.google.common.base.Function;
 import com.google.common.collect.Iterables;
-import com.google.common.collect.Maps;
 import com.google.common.collect.Sets;
 
 import org.apache.jackrabbit.oak.api.CommitFailedException;
 import org.apache.jackrabbit.oak.plugins.document.util.Utils;
+import org.apache.jackrabbit.oak.plugins.memory.MemoryNodeBuilder;
 import org.apache.jackrabbit.oak.spi.commit.CommitHook;
 import org.apache.jackrabbit.oak.spi.commit.CommitInfo;
 import org.apache.jackrabbit.oak.spi.state.ConflictAnnotatingRebaseDiff;
@@ -62,7 +60,6 @@ class DocumentNodeStoreBranch implements
                     + ".perf"));
     private static final int MAX_LOCK_TRY_TIME_MULTIPLIER = Integer.getInteger("oak.maxLockTryTimeMultiplier", 30);
 
-    private static final ConcurrentMap<Thread, DocumentNodeStoreBranch> BRANCHES = Maps.newConcurrentMap();
     private static final Random RANDOM = new Random();
     private static final long MIN_BACKOFF = 50;
 
@@ -151,6 +148,15 @@ class DocumentNodeStoreBranch implements
         return mergeLock;
     }
 
+    /**
+     * For test purposes only!
+     * <p>
+     * Forces the branch to persist the changes to the underlying store.
+     */
+    void persist() {
+        branchState.persist();
+    }
+
     @Nonnull
     private NodeState merge0(@Nonnull CommitHook hook,
                              @Nonnull CommitInfo info,
@@ -318,30 +324,6 @@ class DocumentNodeStoreBranch implements
         return store.getRoot(rev);
     }
 
-    private <T> T withCurrentBranch(Callable<T> callable) throws Exception {
-        Thread t = Thread.currentThread();
-        Object previous = BRANCHES.putIfAbsent(t, this);
-        try {
-            return callable.call();
-        } finally {
-            if (previous == null) {
-                BRANCHES.remove(t, this);
-            }
-        }
-    }
-
-    /**
-     * Returns the branch instance in use by the current thread or
-     * {@code null} if there is none.
-     * <p>
-     * See also {@link #withCurrentBranch(Callable)}.
-     *
-     */
-    @CheckForNull
-    static DocumentNodeStoreBranch getCurrentBranch() {
-        return BRANCHES.get(Thread.currentThread());
-    }
-
     private static CommitFailedException mergeFailed(Throwable t) {
         String msg = t.getMessage();
         if (msg == null) {
@@ -483,8 +465,6 @@ class DocumentNodeStoreBranch implements
     private class InMemory extends BranchState {
         /** Root state of the transient head. */
         private NodeState head;
-        /** Number of in-memory updates */
-        private int numUpdates;
 
         @Override
         public String toString() {
@@ -493,8 +473,7 @@ class DocumentNodeStoreBranch implements
 
         InMemory(DocumentNodeState base, NodeState head) {
             super(base);
-            this.head = head;
-            this.numUpdates = countChanges(base, head);
+            this.head = newModifiedDocumentNodeState(head);
         }
 
         @Override
@@ -507,10 +486,10 @@ class DocumentNodeStoreBranch implements
         void setRoot(NodeState root) {
             if (base.equals(root)) {
                 branchState = new Unmodified(base);
-            } else if (!head.equals(root)) {
-                numUpdates += countChanges(head, root);
-                head = root;
-                if (numUpdates > updateLimit) {
+            } else {
+                int numChanges = countChanges(base, root);
+                head = newModifiedDocumentNodeState(root);
+                if (numChanges > updateLimit) {
                     persist();
                 }
             }
@@ -519,10 +498,16 @@ class DocumentNodeStoreBranch implements
         @Override
         void rebase() {
             DocumentNodeState root = store.getRoot();
-            NodeBuilder builder = root.builder();
+            // is a rebase necessary?
+            if (root.getRootRevision().equals(base.getRootRevision())) {
+                // fresh root is still at the same revision as
+                // the base of the branch
+                return;
+            }
+            NodeBuilder builder = new MemoryNodeBuilder(root);
             head.compareAgainstBaseState(base, new ConflictAnnotatingRebaseDiff(builder));
-            head = builder.getNodeState();
             base = root;
+            head = newModifiedDocumentNodeState(builder.getNodeState());
         }
 
         @Override
@@ -536,15 +521,35 @@ class DocumentNodeStoreBranch implements
             Lock lock = acquireMergeLock(exclusive);
             try {
                 rebase();
-                NodeState toCommit = hook.processCommit(base, head, info);
+                boolean success = false;
+                NodeState previousHead = head;
                 try {
-                    NodeState newHead = DocumentNodeStoreBranch.this.persist(toCommit, base, info);
-                    branchState = new Merged(base);
-                    return newHead;
-                } catch (ConflictException e) {
-                    throw e.asCommitFailedException();
-                } catch (Throwable t) {
-                    throw mergeFailed(t);
+                    NodeState toCommit = hook.processCommit(base, head, info);
+                    try {
+                        NodeState newHead;
+                        if (this != branchState) {
+                            // branch state is not in-memory anymore
+                            Persisted p = branchState.persist();
+                            RevisionVector branchRev = p.getHead().getRootRevision();
+                            newHead = store.getRoot(store.merge(branchRev, info));
+                            store.getStatsCollector().doneMergeBranch(p.numCommits);
+                        } else {
+                            newHead = DocumentNodeStoreBranch.this.persist(toCommit, base, info);
+                        }
+                        branchState = new Merged(base);
+                        success = true;
+                        return newHead;
+                    } catch (ConflictException e) {
+                        throw e.asCommitFailedException();
+                    } catch (Throwable t) {
+                        throw mergeFailed(t);
+                    }
+                } finally {
+                    if (!success) {
+                        this.head = previousHead;
+                        // make sure branch state is reset
+                        branchState = this;
+                    }
                 }
             } finally {
                 if (lock != null) {
@@ -552,6 +557,10 @@ class DocumentNodeStoreBranch implements
                 }
             }
         }
+
+        private ModifiedDocumentNodeState newModifiedDocumentNodeState(NodeState modified) {
+            return new ModifiedDocumentNodeState(store, DocumentNodeStoreBranch.this, base, modified);
+        }
     }
 
     /**
@@ -580,7 +589,13 @@ class DocumentNodeStoreBranch implements
 
         Persisted(DocumentNodeState base) {
             super(base);
-            this.head = createBranch(base);
+            this.head = createBranch(base).asBranchRootState(DocumentNodeStoreBranch.this);
+        }
+
+        @Override
+        Persisted persist() {
+            // nothing to do, this branch state is already persisted
+            return this;
         }
 
         /**
@@ -595,7 +610,7 @@ class DocumentNodeStoreBranch implements
 
         @Override
         @Nonnull
-        NodeState getHead() {
+        DocumentNodeState getHead() {
             return head;
         }
 
@@ -610,7 +625,8 @@ class DocumentNodeStoreBranch implements
         void rebase() {
             DocumentNodeState root = store.getRoot();
             // perform rebase in store
-            head = store.getRoot(store.rebase(head.getRootRevision(), root.getRootRevision()));
+            head = store.getRoot(store.rebase(head.getRootRevision(), root.getRootRevision()))
+                    .asBranchRootState(DocumentNodeStoreBranch.this);
             base = root;
         }
 
@@ -626,18 +642,13 @@ class DocumentNodeStoreBranch implements
             try {
                 rebase();
                 previousHead = head;
-                DocumentNodeState newRoot = withCurrentBranch(new Callable<DocumentNodeState>() {
-                    @Override
-                    public DocumentNodeState call() throws Exception {
-                        checkForConflicts();
-                        NodeState toCommit = checkNotNull(hook).processCommit(base, head, info);
-                        persistTransientHead(toCommit);
-                        return store.getRoot(store.merge(head.getRootRevision(), info));
-                    }
-                });
+                checkForConflicts();
+                NodeState toCommit = checkNotNull(hook).processCommit(base, head, info);
+                persistTransientHead(toCommit);
+                DocumentNodeState newRoot = store.getRoot(store.merge(head.getRootRevision(), info));
+                success = true;
                 branchState = new Merged(base);
                 store.getStatsCollector().doneMergeBranch(numCommits);
-                success = true;
                 return newRoot;
             } catch (CommitFailedException e) {
                 throw e;
@@ -658,7 +669,8 @@ class DocumentNodeStoreBranch implements
         private void persistTransientHead(NodeState newHead)
                 throws DocumentStoreException {
             try {
-                head = DocumentNodeStoreBranch.this.persist(newHead, head, CommitInfo.EMPTY);
+                head = DocumentNodeStoreBranch.this.persist(newHead, head, CommitInfo.EMPTY)
+                        .asBranchRootState(DocumentNodeStoreBranch.this);
             } catch (ConflictException e) {
                 throw DocumentStoreException.convert(e);
             }
@@ -668,9 +680,9 @@ class DocumentNodeStoreBranch implements
 
         private void resetBranch(DocumentNodeState branchHead, DocumentNodeState ancestor) {
             try {
-                head = store.getRoot(
-                        store.reset(branchHead.getRootRevision(),
-                                ancestor.getRootRevision()));
+                head = store.getRoot(store.reset(branchHead.getRootRevision(),
+                            ancestor.getRootRevision()))
+                        .asBranchRootState(DocumentNodeStoreBranch.this);
             } catch (Exception e) {
                 CommitFailedException ex = new CommitFailedException(
                         OAK, 100, "Branch reset failed", e);

Modified: jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentRootBuilder.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentRootBuilder.java?rev=1829826&r1=1829825&r2=1829826&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentRootBuilder.java (original)
+++ jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentRootBuilder.java Mon Apr 23 07:41:26 2018
@@ -71,11 +71,12 @@ class DocumentRootBuilder extends Abstra
     private int updates;
 
     DocumentRootBuilder(@Nonnull DocumentNodeState base,
-                        @Nonnull DocumentNodeStore store) {
+                        @Nonnull DocumentNodeStore store,
+                        @Nonnull DocumentNodeStoreBranch branch) {
         super(checkNotNull(base));
         this.store = checkNotNull(store);
         this.base = base;
-        this.branch = store.createBranch(base);
+        this.branch = checkNotNull(branch);
         this.updateLimit = store.getUpdateLimit();
     }
 
@@ -108,12 +109,10 @@ class DocumentRootBuilder extends Abstra
     @Nonnull
     @Override
     public NodeState getNodeState() {
-        if (DocumentNodeStoreBranch.getCurrentBranch() != null) {
+        if (updates > 0) {
             purge();
-            return branch.getHead();
-        } else {
-            return super.getNodeState();
         }
+        return branch.getHead();
     }
 
     @Override
@@ -181,4 +180,11 @@ class DocumentRootBuilder extends Abstra
         updates = 0;
     }
 
+    /**
+     * For test purposes only!
+     */
+    void persist() {
+        purge();
+        branch.persist();
+    }
 }

Added: jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/ModifiedDocumentNodeState.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/ModifiedDocumentNodeState.java?rev=1829826&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/ModifiedDocumentNodeState.java (added)
+++ jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/ModifiedDocumentNodeState.java Mon Apr 23 07:41:26 2018
@@ -0,0 +1,157 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.oak.plugins.document;
+
+import javax.annotation.Nonnull;
+
+import org.apache.jackrabbit.oak.api.PropertyState;
+import org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState;
+import org.apache.jackrabbit.oak.plugins.memory.ModifiedNodeState;
+import org.apache.jackrabbit.oak.spi.state.AbstractNodeState;
+import org.apache.jackrabbit.oak.spi.state.ApplyDiff;
+import org.apache.jackrabbit.oak.spi.state.ChildNodeEntry;
+import org.apache.jackrabbit.oak.spi.state.EqualsDiff;
+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 static com.google.common.base.Preconditions.checkNotNull;
+import static org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState.EMPTY_NODE;
+
+/**
+ * A node state based on a {@link DocumentNodeState} with some modifications
+ * applied on top of it represented by {@link #modified}. This node state is
+ * a thin wrapper around {@link #modified} and creates a new {@link NodeBuilder}
+ * connected to the {@link #branch} on {@link #builder()}.
+ */
+class ModifiedDocumentNodeState extends AbstractNodeState {
+
+    private final DocumentNodeStore store;
+
+    private final DocumentNodeStoreBranch branch;
+
+    private final DocumentNodeState base;
+
+    private final NodeState modified;
+
+    ModifiedDocumentNodeState(@Nonnull DocumentNodeStore store,
+                              @Nonnull DocumentNodeStoreBranch branch,
+                              @Nonnull DocumentNodeState base,
+                              @Nonnull NodeState modified) {
+        this.store = checkNotNull(store);
+        this.branch = checkNotNull(branch);
+        this.base = checkNotNull(base);
+        this.modified = checkNotNull(modified);
+    }
+
+    @Override
+    public boolean exists() {
+        return modified.exists();
+    }
+
+    @Nonnull
+    @Override
+    public Iterable<? extends PropertyState> getProperties() {
+        return modified.getProperties();
+    }
+
+    @Override
+    public boolean hasChildNode(@Nonnull String name) {
+        return modified.hasChildNode(name);
+    }
+
+    @Nonnull
+    @Override
+    public NodeState getChildNode(@Nonnull String name)
+            throws IllegalArgumentException {
+        return modified.getChildNode(name);
+    }
+
+    @Nonnull
+    @Override
+    public Iterable<? extends ChildNodeEntry> getChildNodeEntries() {
+        return modified.getChildNodeEntries();
+    }
+
+    @Nonnull
+    @Override
+    public NodeBuilder builder() {
+        NodeBuilder builder = new DocumentRootBuilder(base, store, branch);
+        modified.compareAgainstBaseState(base, new ApplyDiff(builder));
+        return builder;
+    }
+
+    @Override
+    public boolean equals(Object that) {
+        if (this == that) {
+            return true;
+        } else if (that instanceof AbstractDocumentNodeState) {
+            AbstractDocumentNodeState other = (AbstractDocumentNodeState) that;
+            if (!base.getPath().equals(other.getPath())) {
+                // path does not match: not equals
+                // (even if the properties are equal)
+                return false;
+            }
+            if (revisionEquals(base, other)) {
+                // other is equal to our base state
+                // perform an equals diff with base and modified
+                return EqualsDiff.equals(base, modified);
+            }
+            // revision does not match: might still be equals
+        } else if (that instanceof ModifiedNodeState) {
+            ModifiedNodeState modified = (ModifiedNodeState) that;
+            if (modified.getBaseState() == base) {
+                // base states are the same, compare the modified
+                return EqualsDiff.equals(this.modified, modified);
+            }
+        }
+        if (that instanceof NodeState) {
+            return AbstractNodeState.equals(modified, (NodeState) that);
+        }
+        return false;
+    }
+
+    @Override
+    public boolean compareAgainstBaseState(NodeState base, NodeStateDiff diff) {
+        if (this == base) {
+            return true;
+        } else if (base == EMPTY_NODE || !base.exists()) {
+            // special case
+            return EmptyNodeState.compareAgainstEmptyState(this, diff);
+        } else if (this.base == base) {
+            return modified.compareAgainstBaseState(this.base, diff);
+        } else if (base instanceof AbstractDocumentNodeState) {
+            AbstractDocumentNodeState other = (AbstractDocumentNodeState) base;
+            if (this.base.getPath().equals(other.getPath())) {
+                if (revisionEquals(this.base, other)) {
+                    return modified.compareAgainstBaseState(this.base, diff);
+                }
+            }
+        }
+        // fall back to the generic node state diff algorithm
+        return super.compareAgainstBaseState(base, diff);
+    }
+
+    private boolean revisionEquals(AbstractDocumentNodeState a,
+                                   AbstractDocumentNodeState b) {
+        RevisionVector rv1 = a.getLastRevision();
+        rv1 = rv1 != null ? rv1.asTrunkRevision() : null;
+        RevisionVector rv2 = b.getLastRevision();
+        rv2 = rv2 != null ? rv2.asTrunkRevision() : null;
+        return rv1 != null && rv1.equals(rv2);
+    }
+}

Propchange: jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/ModifiedDocumentNodeState.java
------------------------------------------------------------------------------
    svn:eol-style = native

Modified: jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/CountingDiff.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/CountingDiff.java?rev=1829826&r1=1829825&r2=1829826&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/CountingDiff.java (original)
+++ jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/CountingDiff.java Mon Apr 23 07:41:26 2018
@@ -65,7 +65,6 @@ public class CountingDiff implements Nod
     public boolean childNodeChanged(String name,
                                     NodeState before,
                                     NodeState after) {
-        inc();
         return after.compareAgainstBaseState(before, this);
     }
 

Modified: jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/ClusterConflictTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/ClusterConflictTest.java?rev=1829826&r1=1829825&r2=1829826&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/ClusterConflictTest.java (original)
+++ jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/ClusterConflictTest.java Mon Apr 23 07:41:26 2018
@@ -43,6 +43,7 @@ import org.junit.Test;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import static org.apache.jackrabbit.oak.plugins.document.TestUtils.persistToBranch;
 import static org.apache.jackrabbit.oak.spi.commit.CommitInfo.EMPTY;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
@@ -100,7 +101,7 @@ public class ClusterConflictTest {
         final NodeBuilder b2 = ns2.getRoot().builder();
         b2.child("bar");
         if (withBranch) {
-            purge(b2);
+            persistToBranch(b2);
         }
         b2.child("baz");
 
@@ -246,8 +247,4 @@ public class ClusterConflictTest {
             throws CommitFailedException {
         return store.merge(root, EmptyHook.INSTANCE, EMPTY);
     }
-
-    private static void purge(NodeBuilder builder) {
-        ((DocumentRootBuilder) builder).purge();
-    }
 }

Modified: jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStateTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStateTest.java?rev=1829826&r1=1829825&r2=1829826&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStateTest.java (original)
+++ jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStateTest.java Mon Apr 23 07:41:26 2018
@@ -23,6 +23,7 @@ import org.apache.jackrabbit.oak.spi.sta
 import org.junit.Rule;
 import org.junit.Test;
 
+import static org.apache.jackrabbit.oak.plugins.document.TestUtils.asDocumentState;
 import static org.junit.Assert.assertEquals;
 
 public class DocumentNodeStateTest {
@@ -48,6 +49,34 @@ public class DocumentNodeStateTest {
 
         NodeState ns = store.getRoot().getChildNode("a");
         assertEquals(2, ns.getPropertyCount());
+    }
+
+    @Test
+    public void asBranchRootState() {
+        DocumentNodeStore store = builderProvider.newBuilder().getNodeStore();
+        DocumentNodeStoreBranch branch = store.createBranch(store.getRoot());
+        NodeBuilder builder = branch.getBase().builder();
+        builder.child("a");
+        branch.setRoot(builder.getNodeState());
+        branch.persist();
+        asDocumentState(branch.getHead()).asBranchRootState(branch);
+    }
+
+    @Test(expected = IllegalStateException.class)
+    public void asBranchRootStateNonBranch() {
+        DocumentNodeStore store = builderProvider.newBuilder().getNodeStore();
+        DocumentNodeStoreBranch branch = store.createBranch(store.getRoot());
+        store.getRoot().asBranchRootState(branch);
+    }
+
+    @Test(expected = IllegalStateException.class)
+    public void asBranchRootStateNonRootState() throws Exception {
+        DocumentNodeStore store = builderProvider.newBuilder().getNodeStore();
+        NodeBuilder builder = store.getRoot().builder();
+        builder.child("a");
+        store.merge(builder, EmptyHook.INSTANCE, CommitInfo.EMPTY);
 
+        DocumentNodeStoreBranch branch = store.createBranch(store.getRoot());
+        asDocumentState(store.getRoot().getChildNode("a")).asBranchRootState(branch);
     }
 }

Modified: jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranchInCommitHookTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranchInCommitHookTest.java?rev=1829826&r1=1829825&r2=1829826&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranchInCommitHookTest.java (original)
+++ jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranchInCommitHookTest.java Mon Apr 23 07:41:26 2018
@@ -17,21 +17,28 @@
 package org.apache.jackrabbit.oak.plugins.document;
 
 import java.util.Map;
+import java.util.concurrent.atomic.AtomicInteger;
 
+import javax.annotation.Nonnull;
+
+import org.apache.jackrabbit.oak.api.CommitFailedException;
 import org.apache.jackrabbit.oak.plugins.document.util.Utils;
+import org.apache.jackrabbit.oak.spi.commit.CommitHook;
 import org.apache.jackrabbit.oak.spi.commit.CommitInfo;
+import org.apache.jackrabbit.oak.spi.commit.CompositeHook;
 import org.apache.jackrabbit.oak.spi.commit.DefaultEditor;
 import org.apache.jackrabbit.oak.spi.commit.Editor;
 import org.apache.jackrabbit.oak.spi.commit.EditorHook;
 import org.apache.jackrabbit.oak.spi.commit.EditorProvider;
 import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
 import org.apache.jackrabbit.oak.spi.state.NodeState;
-import org.junit.Ignore;
 import org.junit.Rule;
 import org.junit.Test;
 
 import static org.apache.jackrabbit.oak.plugins.document.util.Utils.isCommitted;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 public class DocumentNodeStoreBranchInCommitHookTest {
 
@@ -39,7 +46,6 @@ public class DocumentNodeStoreBranchInCo
     public DocumentMKBuilderProvider builderProvider = new DocumentMKBuilderProvider();
 
     // OAK-7401
-    @Ignore
     @Test
     public void manyChangesInCommitHooks() throws Exception {
         DocumentNodeStore ns = builderProvider.newBuilder()
@@ -51,11 +57,90 @@ public class DocumentNodeStoreBranchInCo
 
         NodeBuilder builder = ns.getRoot().builder();
         builder.child("test");
-        ns.merge(builder, new TestHook(), CommitInfo.EMPTY);
+        ns.merge(builder, new TestHook(17), CommitInfo.EMPTY);
+
+        // must have created branch commits
+        root = Utils.getRootDocument(ns.getDocumentStore());
+        assertTrue(numBranchCommits(root) > initialNumBranchCommits);
+    }
+
+    // OAK-7401
+    @Test
+    public void manyChangesInMultipleCommitHooks() throws Exception {
+        DocumentNodeStore ns = builderProvider.newBuilder()
+                .setAsyncDelay(0).setUpdateLimit(10).build();
+
+        // get initial branch commits
+        NodeDocument root = Utils.getRootDocument(ns.getDocumentStore());
+        int initialNumBranchCommits = numBranchCommits(root);
+
+        NodeBuilder builder = ns.getRoot().builder();
+        builder.child("test");
+        TestHook h1 = new TestHook(17);
+        TestHook h2 = new TestHook(23);
+        ns.merge(builder, new CompositeHook(h1, h2), CommitInfo.EMPTY);
+
+        // must have created branch commits
+        root = Utils.getRootDocument(ns.getDocumentStore());
+        assertTrue(numBranchCommits(root) > initialNumBranchCommits);
+
+        h1.assertChanges(ns.getRoot());
+        h2.assertChanges(ns.getRoot());
+    }
+
+    // OAK-7401
+    @Test
+    public void manyChangesInCommitHooksMultipleBranchCommits() throws Exception {
+        DocumentNodeStore ns = builderProvider.newBuilder()
+                .setAsyncDelay(0).setUpdateLimit(10).build();
+
+        // get initial branch commits
+        NodeDocument root = Utils.getRootDocument(ns.getDocumentStore());
+        int initialNumBranchCommits = numBranchCommits(root);
+
+        NodeBuilder builder = ns.getRoot().builder();
+        builder.child("test");
+        TestHook hook = new TestHook(41);
+        ns.merge(builder, hook, CommitInfo.EMPTY);
 
         // must have created branch commits
         root = Utils.getRootDocument(ns.getDocumentStore());
         assertTrue(numBranchCommits(root) > initialNumBranchCommits);
+
+        hook.assertChanges(ns.getRoot());
+    }
+
+    // OAK-7401
+    @Test
+    public void commitHookTriggersBranchWithConflict() throws Exception {
+        DocumentNodeStore ns = builderProvider.newBuilder()
+                .setAsyncDelay(0).setUpdateLimit(10).build();
+
+        // get initial branch commits
+        NodeDocument root = Utils.getRootDocument(ns.getDocumentStore());
+        int initialNumBranchCommits = numBranchCommits(root);
+
+        NodeBuilder builder = ns.getRoot().builder();
+        builder.child("test");
+        TestHook hook = new TestHook(41);
+        ns.merge(builder, new CompositeHook(hook,
+                        new FailingHook(1)), // fail once
+                CommitInfo.EMPTY);
+
+        // must have created branch commits
+        root = Utils.getRootDocument(ns.getDocumentStore());
+        assertTrue(numBranchCommits(root) > initialNumBranchCommits);
+
+        hook.assertChanges(ns.getRoot());
+
+        // must have left behind unmerged branch commits
+        root = Utils.getRootDocument(ns.getDocumentStore());
+        for (String value : root.getLocalRevisions().values()) {
+            if (!Utils.isCommitted(value)) {
+                return;
+            }
+        }
+        fail("Must have created unmerged branch commits");
     }
 
     private int numBranchCommits(NodeDocument root) {
@@ -71,34 +156,91 @@ public class DocumentNodeStoreBranchInCo
         return numBranchCommits;
     }
 
+    private static class FailingHook implements CommitHook {
+
+        private int countDown;
+
+        FailingHook(int numFailures) {
+            this.countDown = numFailures;
+        }
+
+        @Nonnull
+        @Override
+        public NodeState processCommit(NodeState before,
+                                       NodeState after,
+                                       CommitInfo info)
+                throws CommitFailedException {
+            if (countDown-- > 0) {
+                throw new ConflictException("Conflict",
+                        new Revision(0, 0, 1))
+                        .asCommitFailedException();
+            }
+            return after;
+        }
+    }
+
     private static class TestHook extends EditorHook {
 
-        TestHook() {
-            super(new EditorProvider() {
-                @Override
-                public Editor getRootEditor(NodeState before,
-                                            NodeState after,
-                                            NodeBuilder builder,
-                                            CommitInfo info) {
-                    return new TestEditor(builder);
-                }
-            });
+        private static final AtomicInteger COUNTER = new AtomicInteger();
+
+        private final TestEditorProvider editorProvider;
+
+        TestHook(int numChanges) {
+            this(new TestEditorProvider("parent-" + COUNTER.getAndIncrement(), numChanges));
+        }
+
+        private TestHook(TestEditorProvider editorProvider) {
+            super(editorProvider);
+            this.editorProvider = editorProvider;
+        }
+
+        void assertChanges(NodeState root) {
+            NodeState parent = root.getChildNode(editorProvider.parentName);
+            for (int i = 0; i < editorProvider.numChanges; i++) {
+                String msg = "Missing node: /" + editorProvider.parentName + "/n-" + i;
+                assertTrue(msg, parent.hasChildNode("n-" + i));
+            }
+        }
+    }
+
+    private static class TestEditorProvider implements EditorProvider {
+
+        private final String parentName;
+        private final int numChanges;
+
+        TestEditorProvider(String parentName, int numChanges) {
+            this.parentName = parentName;
+            this.numChanges = numChanges;
+        }
+
+        @Override
+        public Editor getRootEditor(NodeState before,
+                                    NodeState after,
+                                    NodeBuilder builder,
+                                    CommitInfo info) {
+            return new TestEditor(parentName, numChanges, builder);
         }
     }
 
     private static class TestEditor extends DefaultEditor {
 
+        private final String parentName;
+        private final int numChanges;
         private final NodeBuilder builder;
 
-        TestEditor(NodeBuilder builder) {
+        TestEditor(String parentName, int numChanges, NodeBuilder builder) {
+            this.parentName = parentName;
+            this.numChanges = numChanges;
             this.builder = builder;
         }
 
         @Override
         public void enter(NodeState before, NodeState after) {
-            NodeBuilder stuff = builder.child("parent");
-            for (int i = 0; i < 17; i++) {
-                stuff.child("n-" + i);
+            NodeBuilder stuff = builder.child(parentName);
+            for (int i = 0; i < numChanges; i++) {
+                String name = "n-" + i;
+                assertFalse(stuff.hasChildNode(name));
+                stuff.child(name);
             }
         }
     }

Added: jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranchTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranchTest.java?rev=1829826&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranchTest.java (added)
+++ jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranchTest.java Mon Apr 23 07:41:26 2018
@@ -0,0 +1,106 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.oak.plugins.document;
+
+import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
+import org.junit.Rule;
+import org.junit.Test;
+
+import static org.apache.jackrabbit.oak.plugins.document.TestUtils.merge;
+import static org.apache.jackrabbit.oak.plugins.document.TestUtils.persistToBranch;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+public class DocumentNodeStoreBranchTest {
+
+    @Rule
+    public DocumentMKBuilderProvider builderProvider = new DocumentMKBuilderProvider();
+
+    @Test
+    public void branchedBranch() throws Exception {
+        DocumentNodeStore ns = builderProvider.newBuilder().getNodeStore();
+        NodeBuilder b1 = ns.getRoot().builder();
+        b1.child("a");
+        persistToBranch(b1);
+        NodeBuilder b2 = b1.getNodeState().builder();
+        b1.child("b");
+        persistToBranch(b1);
+
+        b2.child("c");
+        persistToBranch(b2);
+        assertTrue(b2.hasChildNode("a"));
+        assertFalse(b2.hasChildNode("b"));
+        assertTrue(b2.hasChildNode("c"));
+
+        // b1 must still see 'a' and 'b', but not 'c'
+        assertTrue(b1.hasChildNode("a"));
+        assertTrue(b1.hasChildNode("b"));
+        assertFalse(b1.hasChildNode("c"));
+
+        merge(ns, b1);
+
+        assertTrue(ns.getRoot().getChildNode("a").exists());
+        assertTrue(ns.getRoot().getChildNode("b").exists());
+        assertFalse(ns.getRoot().getChildNode("c").exists());
+
+        // b2 must not be able to merge
+        try {
+            merge(ns, b2);
+            fail("Merge must fail with IllegalStateException");
+        } catch (IllegalStateException e) {
+            // expected
+        }
+    }
+
+    /**
+     * Similar test as {@link #branchedBranch()} but without persistent branch.
+     */
+    @Test
+    public void builderFromStateFromBuilder() throws Exception {
+        DocumentNodeStore ns = builderProvider.newBuilder().getNodeStore();
+        NodeBuilder b1 = ns.getRoot().builder();
+        b1.child("a");
+
+        NodeBuilder b2 = b1.getNodeState().builder();
+        b1.child("b");
+
+        b2.child("c");
+        assertTrue(b2.hasChildNode("a"));
+        assertFalse(b2.hasChildNode("b"));
+        assertTrue(b2.hasChildNode("c"));
+
+        // b1 must still see 'a' and 'b', but not 'c'
+        assertTrue(b1.hasChildNode("a"));
+        assertTrue(b1.hasChildNode("b"));
+        assertFalse(b1.hasChildNode("c"));
+
+        merge(ns, b1);
+
+        assertTrue(ns.getRoot().getChildNode("a").exists());
+        assertTrue(ns.getRoot().getChildNode("b").exists());
+        assertFalse(ns.getRoot().getChildNode("c").exists());
+
+        // b2 must not be able to merge
+        try {
+            merge(ns, b2);
+            fail("Merge must fail with IllegalStateException");
+        } catch (IllegalStateException e) {
+            // expected
+        }
+    }
+}

Propchange: jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranchTest.java
------------------------------------------------------------------------------
    svn:eol-style = native

Modified: jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java?rev=1829826&r1=1829825&r2=1829826&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java (original)
+++ jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java Mon Apr 23 07:41:26 2018
@@ -697,7 +697,7 @@ public class DocumentNodeStoreTest {
         int i = 0;
         // force creation of a branch
         while (docStore.find(NODES, id) == null) {
-            node.setProperty("foo", i++);
+            node.child("foo-" + i++);
         }
 
         NodeDocument doc = docStore.find(NODES, id);
@@ -738,7 +738,7 @@ public class DocumentNodeStoreTest {
         int i = 0;
         // force creation of a branch
         while (docStore.find(NODES, id) == null) {
-            node.setProperty("foo", i++);
+            node.child("foo-"+ i++);
         }
         merge(store1, builder);
 
@@ -776,10 +776,10 @@ public class DocumentNodeStoreTest {
         String testId = Utils.getIdFromPath("/test");
         NodeBuilder test = builder.child("test");
         test.setProperty("p", "value");
+        test.setProperty("q", 0);
         // force creation of branch
-        int q = 0;
-        while (store.find(NODES, testId) == null) {
-            test.setProperty("q", q++);
+        for (int i = 0; store.find(NODES, testId) == null; i++) {
+            test.child("foo-" + i++);
         }
         merge(ns, builder);
 

Modified: jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryRandomizedIT.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryRandomizedIT.java?rev=1829826&r1=1829825&r2=1829826&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryRandomizedIT.java (original)
+++ jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryRandomizedIT.java Mon Apr 23 07:41:26 2018
@@ -150,7 +150,7 @@ public class LastRevRecoveryRandomizedIT
 
     private void purge() {
         ops.add("purge()");
-        builder.purge();
+        builder.persist();
     }
 
     private void merge() throws CommitFailedException {

Modified: jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/OrphanedBranchTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/OrphanedBranchTest.java?rev=1829826&r1=1829825&r2=1829826&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/OrphanedBranchTest.java (original)
+++ jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/OrphanedBranchTest.java Mon Apr 23 07:41:26 2018
@@ -40,6 +40,7 @@ import org.slf4j.LoggerFactory;
 
 import static org.apache.jackrabbit.oak.plugins.document.Collection.NODES;
 import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.COLLISIONS;
+import static org.apache.jackrabbit.oak.plugins.document.TestUtils.persistToBranch;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
@@ -98,6 +99,7 @@ public class OrphanedBranchTest {
             // perform changes until a branch is created
             while (store.getBranches().size() == numBranches) {
                 child.setProperty("prop", count++);
+                persistToBranch(builder);
             }
             // but do not merge!
             numCreated++;
@@ -177,6 +179,7 @@ public class OrphanedBranchTest {
         // perform changes until a branch is created
         while (store.getBranches().size() == numBranches) {
             child.setProperty("prop", count++);
+            persistToBranch(builder);
         }
         // perform some other change to update head revision
         builder = store.getRoot().builder();

Modified: jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/TestUtils.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/TestUtils.java?rev=1829826&r1=1829825&r2=1829826&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/TestUtils.java (original)
+++ jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/TestUtils.java Mon Apr 23 07:41:26 2018
@@ -18,6 +18,7 @@ package org.apache.jackrabbit.oak.plugin
 
 import java.util.Map;
 
+import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
 import com.google.common.base.Function;
@@ -86,6 +87,7 @@ public class TestUtils {
         return nb;
     }
 
+    @Nonnull
     public static DocumentNodeState asDocumentState(NodeState state){
         if (state instanceof DocumentNodeState){
             return (DocumentNodeState) state;
@@ -101,4 +103,12 @@ public class TestUtils {
     public static void resetRevisionClockToDefault() {
         Revision.resetClockToDefault();
     }
+
+    public static void persistToBranch(NodeBuilder builder) {
+        if (builder instanceof DocumentRootBuilder) {
+            ((DocumentRootBuilder) builder).persist();
+            return;
+        }
+        fail("Not of type DocumentRootBuilder: " + builder.getClass().getName());
+    }
 }

Modified: jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/ValueMapTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/ValueMapTest.java?rev=1829826&r1=1829825&r2=1829826&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/ValueMapTest.java (original)
+++ jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/ValueMapTest.java Mon Apr 23 07:41:26 2018
@@ -33,6 +33,7 @@ import com.google.common.collect.Iterato
 import com.google.common.collect.Lists;
 
 import static org.apache.jackrabbit.oak.plugins.document.Collection.NODES;
+import static org.apache.jackrabbit.oak.plugins.document.TestUtils.persistToBranch;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
@@ -157,6 +158,7 @@ public class ValueMapTest {
                 int p = 0;
                 while (numRevs == docStore.find(NODES, id).getLocalRevisions().size()) {
                     child.setProperty("prop", p++);
+                    persistToBranch(builder);
                 }
                 branches.add(builder);
             } else {

Modified: jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VisibleChangesTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VisibleChangesTest.java?rev=1829826&r1=1829825&r2=1829826&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VisibleChangesTest.java (original)
+++ jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VisibleChangesTest.java Mon Apr 23 07:41:26 2018
@@ -31,6 +31,7 @@ import org.apache.jackrabbit.oak.spi.com
 import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
 import org.junit.Test;
 
+import static org.apache.jackrabbit.oak.plugins.document.TestUtils.persistToBranch;
 import static org.apache.jackrabbit.oak.plugins.document.util.Utils.getIdFromPath;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
@@ -61,6 +62,7 @@ public class VisibleChangesTest {
         while (numRevs == getRevisionsSize(store, "/")) {
             foo.setProperty("p", "v");
             foo.removeProperty("p");
+            persistToBranch(builder);
         }
 
         store.paths.clear();