Index: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Commit.java =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Commit.java (revision 1784930) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Commit.java (working copy) @@ -102,6 +102,9 @@ String id = Utils.getIdFromPath(path); op = new UpdateOp(id, false); NodeDocument.setModified(op, revision); + if (getBranch() != null) { + NodeDocument.setBranchCommit(op, revision); + } operations.put(path, op); } return op; Index: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java (revision 1784930) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java (working copy) @@ -2154,6 +2154,7 @@ for (Revision r : b.getCommits()) { r = r.asTrunkRevision(); NodeDocument.removeRevision(op, r); + NodeDocument.removeBranchCommit(op, r); } store.findAndUpdate(NODES, op); } Index: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java (revision 1784930) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java (working copy) @@ -225,6 +225,11 @@ */ private static final String STALE_PREV = "_stalePrev"; + /** + * Contains revision entries for changes done by branch commits. + */ + private static final String BRANCH_COMMITS = "_bc"; + //~----------------------------< Split Document Types > /** @@ -300,6 +305,11 @@ * A split document which does not contain REVISIONS history. */ COMMIT_ROOT_ONLY(60), + /** + * A split document which contains all types of data, but no branch + * commits. + */ + DEFAULT_NO_BRANCH(70), ; final int type; @@ -1678,6 +1688,16 @@ } /** + * Returns the branch commit entries on this document + * ({@link #BRANCH_COMMITS}). This method does not consider previous + * documents, but only returns the entries on this document. + */ + @Nonnull + public Set getBranchCommits() { + return getLocalMap(BRANCH_COMMITS).keySet(); + } + + /** * Resolves the commit value for the change with the given revision on this * document. If necessary, this method will lookup the commit value on the * referenced commit root document. @@ -1829,6 +1849,17 @@ checkNotNull(op).set(HAS_BINARY_FLAG, HAS_BINARY_VAL); } + public static void setBranchCommit(@Nonnull UpdateOp op, + @Nonnull Revision revision) { + checkNotNull(op).setMapEntry(BRANCH_COMMITS, + revision, String.valueOf(true)); + } + + public static void removeBranchCommit(@Nonnull UpdateOp op, + @Nonnull Revision revision) { + checkNotNull(op).removeMapEntry(BRANCH_COMMITS, revision); + } + //----------------------------< internal >---------------------------------- private void previousDocumentNotFound(String prevId, Revision rev) { Index: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/SplitOperations.java =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/SplitOperations.java (revision 1784930) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/SplitOperations.java (working copy) @@ -366,11 +366,15 @@ // referenced anymore from from most recent changes if (!mostRecentRevs.contains(r)) { main.removeMapEntry(property, r); + NodeDocument.removeBranchCommit(main, r); } } else { main.removeMapEntry(property, r); } old.setMapEntry(property, r, entry.getValue()); + if (doc.getBranchCommits().contains(r)) { + NodeDocument.setBranchCommit(old, r); + } } } // check size of old document @@ -520,6 +524,7 @@ if (PROPERTY_OR_DELETED.apply(entry.getKey())) { NodeDocument.removeCommitRoot(main, r); NodeDocument.removeRevision(main, r); + NodeDocument.removeBranchCommit(main, r); } } } @@ -554,6 +559,8 @@ type = SplitDocType.DEFAULT_LEAF; } else if (oldDoc.getLocalRevisions().isEmpty()) { type = SplitDocType.COMMIT_ROOT_ONLY; + } else if (oldDoc.getBranchCommits().isEmpty()) { + type = SplitDocType.DEFAULT_NO_BRANCH; } // Copy over the hasBinary flag Index: oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentSplitTest.java =================================================================== --- oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentSplitTest.java (revision 1784930) +++ oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentSplitTest.java (working copy) @@ -19,6 +19,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.HashSet; +import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Random; @@ -62,10 +63,16 @@ import static org.apache.jackrabbit.oak.plugins.document.UpdateOp.Operation.Type.SET_MAP_ENTRY; import static org.apache.jackrabbit.oak.plugins.document.util.Utils.isCommitted; import static org.apache.jackrabbit.oak.plugins.memory.BinaryPropertyState.binaryProperty; +import static org.hamcrest.Matchers.either; +import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.hasSize; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -103,7 +110,7 @@ assertNotNull(ns.getNode("/", RevisionVector.fromString(head))); NodeDocument prevDoc = Iterators.getOnlyElement(doc.getAllPreviousDocs()); - assertEquals(SplitDocType.DEFAULT, prevDoc.getSplitDocType()); + assertThat(prevDoc.getSplitDocType(), either(is(SplitDocType.DEFAULT)).or(is(SplitDocType.DEFAULT_NO_BRANCH))); mk.commit("/", "+\"baz\":{}", null, null); ns.setAsyncDelay(0); @@ -1001,6 +1008,71 @@ assertEquals(0, Iterables.size(splitOps)); } + @Test + public void splitWithBranchCommit() throws Exception { + DocumentStore store = mk.getDocumentStore(); + DocumentNodeStore ns = mk.getNodeStore(); + NodeBuilder builder = ns.getRoot().builder(); + builder.child("foo"); + merge(ns, builder); + + String branch = mk.branch(null); + branch = mk.commit("/foo", "^\"p\":\"value\"", branch, null); + mk.merge(branch, null); + + String id = Utils.getIdFromPath("/foo"); + NodeDocument doc = store.find(NODES, id); + assertNotNull(doc); + assertThat(doc.getBranchCommits(), is(not(empty()))); + + for (int i = 0; i < 5; i++) { + builder = ns.getRoot().builder(); + builder.child("foo").setProperty("p", "value-" + i); + merge(ns, builder); + } + ns.runBackgroundOperations(); + doc = store.find(NODES, id); + for (UpdateOp op : SplitOperations.forDocument(doc, ns, ns.getHeadRevision(), NO_BINARY, 5)) { + store.createOrUpdate(NODES, op); + } + doc = store.find(NODES, id); + // must have a previous document now + assertThat(doc.getPreviousRanges().keySet(), hasSize(1)); + // branch commit entry moved to previous document + assertThat(doc.getBranchCommits(), is(empty())); + NodeDocument prev = doc.getAllPreviousDocs().next(); + assertThat(prev.getBranchCommits(), is(not(empty()))); + } + + @Test + public void splitDefaultNoBranch() throws Exception { + DocumentStore store = mk.getDocumentStore(); + DocumentNodeStore ns = mk.getNodeStore(); + NodeBuilder builder = ns.getRoot().builder(); + builder.child("foo").child("bar"); + merge(ns, builder); + + for (int i = 0; i < 5; i++) { + builder = ns.getRoot().builder(); + builder.child("foo").setProperty("p", "value-" + i); + merge(ns, builder); + } + ns.runBackgroundOperations(); + String id = Utils.getIdFromPath("/foo"); + NodeDocument doc = store.find(NODES, id); + assertNotNull(doc); + for (UpdateOp op : SplitOperations.forDocument(doc, ns, ns.getHeadRevision(), NO_BINARY, 5)) { + store.createOrUpdate(NODES, op); + } + doc = store.find(NODES, id); + // must have a previous document now + assertThat(doc.getPreviousRanges().keySet(), is(not(empty()))); + Iterator it = doc.getAllPreviousDocs(); + while (it.hasNext()) { + assertEquals(SplitDocType.DEFAULT_NO_BRANCH, it.next().getSplitDocType()); + } + } + private static class TestRevisionContext implements RevisionContext { private final RevisionContext rc; Index: oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/OrphanedBranchTest.java =================================================================== --- oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/OrphanedBranchTest.java (revision 1784930) +++ oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/OrphanedBranchTest.java (working copy) @@ -21,6 +21,7 @@ import java.io.IOException; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.TimeUnit; import com.google.common.collect.Lists; @@ -131,6 +132,9 @@ map = doc.getLocalCommitRoot(); assertTrue("too many orphaned commit root entries: " + map.size() + " > " + limit, map.size() <= limit); + Set branchCommits = doc.getBranchCommits(); + assertTrue("too many orphaned branch commit entries: " + branchCommits.size() + " > " + limit, + branchCommits.size() <= limit); // run garbage collector once in a while for changes on /bar if (numCreated % NodeDocument.NUM_REVS_THRESHOLD == 0) { @@ -182,6 +186,7 @@ // force remove branch NodeDocument doc = store.getDocumentStore().find(NODES, id); assertNotNull(doc); + assertFalse(doc.getBranchCommits().isEmpty()); Map valueMap = doc.getLocalMap("prop"); assertFalse(valueMap.isEmpty()); UnmergedBranches branches = store.getBranches(); @@ -201,6 +206,7 @@ } doc = store.getDocumentStore().find(NODES, id); + assertTrue(doc.getBranchCommits().isEmpty()); doc.getNodeAtRevision(store, store.getHeadRevision(), null); store.dispose();