Author: mreutegg
Date: Wed Jan 30 08:05:09 2019
New Revision: 1852495
URL: http://svn.apache.org/viewvc?rev=1852495&view=rev
Log:
OAK-8012: Unmerged branch changes visible after restart
Merge revisions 1852492 and 1852493 from trunk
Modified:
jackrabbit/oak/branches/1.10/ (props changed)
jackrabbit/oak/branches/1.10/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java
jackrabbit/oak/branches/1.10/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/BranchTest.java
Propchange: jackrabbit/oak/branches/1.10/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Wed Jan 30 08:05:09 2019
@@ -1,3 +1,3 @@
/jackrabbit/oak/branches/1.0:1665962
-/jackrabbit/oak/trunk:1851451,1852052,1852084,1852451
+/jackrabbit/oak/trunk:1851451,1852052,1852084,1852451,1852492-1852493
/jackrabbit/trunk:1345480
Modified:
jackrabbit/oak/branches/1.10/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.10/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java?rev=1852495&r1=1852494&r2=1852495&view=diff
==============================================================================
---
jackrabbit/oak/branches/1.10/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java
(original)
+++
jackrabbit/oak/branches/1.10/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java
Wed Jan 30 08:05:09 2019
@@ -2064,20 +2064,25 @@ public final class NodeDocument extends
return false;
}
if (Utils.isCommitted(commitValue)) {
- if (context.getBranches().getBranch(readRevision) == null
- && !readRevision.isBranch()) {
+ Branch b = context.getBranches().getBranch(readRevision);
+ if (b == null) {
+ // readRevision is not from a branch commit, though it may
+ // still be a branch revision when it references the base
+ // of a new branch that has not yet been created. In that
+ // case the branch revision is equivalent with the trunk
+ // revision.
+
// resolve commit revision
revision = resolveCommitRevision(revision, commitValue);
- // readRevision is not from a branch
// compare resolved revision as is
return !readRevision.isRevisionNewer(revision);
} else {
- // on same merged branch?
- Revision tr =
readRevision.getBranchRevision().asTrunkRevision();
- if (commitValue.equals(context.getCommitValue(tr, this))) {
- // compare unresolved revision
- return !readRevision.isRevisionNewer(revision);
- }
+ // read revision is on a branch and the change is committed
+ // get the base revision of the branch and check
+ // if change is visible from there
+ RevisionVector baseRev =
b.getBase(readRevision.getBranchRevision());
+ revision = resolveCommitRevision(revision, commitValue);
+ return !baseRev.isRevisionNewer(revision);
}
} else {
// branch commit (not merged)
@@ -2088,9 +2093,23 @@ public final class NodeDocument extends
// this is an unmerged branch commit from another cluster node,
// hence never visible to us
return false;
+ } else {
+ // unmerged branch change with local clusterId
+ Branch b = context.getBranches().getBranch(readRevision);
+ if (b == null) {
+ // reading on trunk never sees changes on an unmerged
branch
+ return false;
+ } else if (b.containsCommit(revision)) {
+ // read revision is on the same branch as the
+ // unmerged branch changes -> compare revisions as is
+ return !readRevision.isRevisionNewer(revision);
+ } else {
+ // read revision is on a different branch than the
+ // unmerged branch changes -> never visible
+ return false;
+ }
}
}
- return includeRevision(context, resolveCommitRevision(revision,
commitValue), readRevision);
}
/**
@@ -2114,37 +2133,6 @@ public final class NodeDocument extends
return value;
}
- private static boolean includeRevision(RevisionContext context,
- Revision x,
- RevisionVector readRevision) {
- Branch b = null;
- if (x.getClusterId() == context.getClusterId()) {
- RevisionVector branchRev = new
RevisionVector(x).asBranchRevision(context.getClusterId());
- b = context.getBranches().getBranch(branchRev);
- }
- if (b != null) {
- // only include if read revision is also a branch revision
- // with a history including x
- if (readRevision.isBranch()
- && b.containsCommit(readRevision.getBranchRevision())) {
- // in same branch, include if the same revision or
- // readRevision is newer
- return !readRevision.isRevisionNewer(x);
- }
- // not part of branch identified by requestedRevision
- return false;
- }
- // assert: x is not a branch commit
- b = context.getBranches().getBranch(readRevision);
- if (b != null) {
- // reset readRevision to branch base revision to make
- // sure we don't include revisions committed after branch
- // was created
- readRevision = b.getBase(readRevision.getBranchRevision());
- }
- return !readRevision.isRevisionNewer(x);
- }
-
/**
* Get the latest property value smaller or equal the readRevision
revision.
*
Modified:
jackrabbit/oak/branches/1.10/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/BranchTest.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.10/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/BranchTest.java?rev=1852495&r1=1852494&r2=1852495&view=diff
==============================================================================
---
jackrabbit/oak/branches/1.10/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/BranchTest.java
(original)
+++
jackrabbit/oak/branches/1.10/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/BranchTest.java
Wed Jan 30 08:05:09 2019
@@ -16,16 +16,37 @@
*/
package org.apache.jackrabbit.oak.plugins.document;
+import java.util.HashSet;
+import java.util.Set;
+
import com.google.common.collect.Sets;
+import org.apache.jackrabbit.oak.api.PropertyState;
import org.apache.jackrabbit.oak.plugins.document.Branch.BranchCommit;
+import org.apache.jackrabbit.oak.plugins.document.memory.MemoryDocumentStore;
+import org.apache.jackrabbit.oak.spi.state.DefaultNodeStateDiff;
+import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
+import org.apache.jackrabbit.oak.spi.state.NodeState;
+import org.apache.jackrabbit.oak.spi.state.NodeStateDiff;
+import org.junit.Rule;
import org.junit.Test;
+import static
org.apache.jackrabbit.oak.plugins.document.TestUtils.asDocumentState;
+import static
org.apache.jackrabbit.oak.plugins.document.TestUtils.persistToBranch;
+import static
org.apache.jackrabbit.oak.plugins.document.util.Utils.getRootDocument;
+import static org.hamcrest.Matchers.contains;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
public class BranchTest {
+ @Rule
+ public DocumentMKBuilderProvider builderProvider = new
DocumentMKBuilderProvider();
+
@Test
public void getModifiedPathsUntil() {
UnmergedBranches branches = new UnmergedBranches();
@@ -66,6 +87,57 @@ public class BranchTest {
assertModifiedPaths(b.getModifiedPathsUntil(c5));
}
+ @Test
+ public void orphanedBranchTest() {
+ MemoryDocumentStore store = new MemoryDocumentStore();
+ DocumentNodeStore ns = builderProvider.newBuilder()
+ .setDocumentStore(store).build();
+ NodeBuilder builder = ns.getRoot().builder();
+ builder.setProperty("p", "v");
+ persistToBranch(builder);
+ ns.dispose();
+
+ // start again
+ ns = builderProvider.newBuilder()
+ .setDocumentStore(store).build();
+ assertFalse(ns.getRoot().hasProperty("p"));
+ }
+
+ @Test
+ public void compareBranchStates() {
+ DocumentNodeStore ns = builderProvider.newBuilder()
+ .setAsyncDelay(0).build();
+ ns.runBackgroundOperations();
+ DocumentStore store = ns.getDocumentStore();
+
+ NodeBuilder builder = ns.getRoot().builder();
+ builder.setProperty("p", "a");
+ persistToBranch(builder);
+ NodeState s1 = builder.getNodeState();
+ builder.setProperty("p", "b");
+ persistToBranch(builder);
+ NodeState s2 = builder.getNodeState();
+
+ Set<String> changes = new HashSet<>();
+ NodeStateDiff diff = new DefaultNodeStateDiff() {
+ @Override
+ public boolean propertyChanged(PropertyState before,
+ PropertyState after) {
+ changes.add(before.getName());
+ return super.propertyChanged(before, after);
+ }
+ };
+ s2.compareAgainstBaseState(s1, diff);
+ assertThat(changes, contains("p"));
+
+ NodeDocument root = getRootDocument(store);
+ RevisionVector br = asDocumentState(s1).getRootRevision();
+ assertTrue(br.isBranch());
+ DocumentNodeState state = root.getNodeAtRevision(ns, br, null);
+ assertNotNull(state);
+ assertEquals("a", state.getString("p"));
+ }
+
private void assertModifiedPaths(Iterable<String> actual, String...
expected) {
assertEquals(Sets.newHashSet(expected), Sets.newHashSet(actual));
}
