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 2019/01/30 07:53:41 UTC

svn commit: r1852493 - in /jackrabbit/oak/trunk/oak-store-document/src: main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java test/java/org/apache/jackrabbit/oak/plugins/document/BranchTest.java

Author: mreutegg
Date: Wed Jan 30 07:53:41 2019
New Revision: 1852493

URL: http://svn.apache.org/viewvc?rev=1852493&view=rev
Log:
OAK-8012: Unmerged branch changes visible after restart

Modified:
    jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java
    jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/BranchTest.java

Modified: jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java?rev=1852493&r1=1852492&r2=1852493&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java (original)
+++ jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java Wed Jan 30 07:53:41 2019
@@ -2063,20 +2063,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)
@@ -2087,9 +2092,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);
     }
 
     /**
@@ -2113,37 +2132,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/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/BranchTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/BranchTest.java?rev=1852493&r1=1852492&r2=1852493&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/BranchTest.java (original)
+++ jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/BranchTest.java Wed Jan 30 07:53:41 2019
@@ -28,7 +28,6 @@ import org.apache.jackrabbit.oak.spi.sta
 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.Ignore;
 import org.junit.Rule;
 import org.junit.Test;
 
@@ -88,7 +87,6 @@ public class BranchTest {
         assertModifiedPaths(b.getModifiedPathsUntil(c5));
     }
 
-    @Ignore("OAK-8012")
     @Test
     public void orphanedBranchTest() {
         MemoryDocumentStore store = new MemoryDocumentStore();