svn commit: r1853054 - in /jackrabbit/oak/trunk/oak-store-document/src: main/java/org/apache/jackrabbit/oak/plugins/document/AbstractDocumentNodeState.java test/java/org/apache/jackrabbit/oak/plugins/document/NodeStoreDiffTest.java

[mr...@apache.org]

Author: mreutegg
Date: Wed Feb  6 08:32:07 2019
New Revision: 1853054

URL: http://svn.apache.org/viewvc?rev=1853054&view=rev
Log:
OAK-8025: Improve branch state comparison

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

Modified: jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/AbstractDocumentNodeState.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/AbstractDocumentNodeState.java?rev=1853054&r1=1853053&r2=1853054&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/AbstractDocumentNodeState.java (original)
+++ jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/AbstractDocumentNodeState.java Wed Feb  6 08:32:07 2019
@@ -131,13 +131,22 @@ public abstract class AbstractDocumentNo
     //------------------------------< internal >--------------------------------
 
     /**
-     * Returns {@code true} if this state has the same last revision as the
-     * {@code other} state.
+     * Returns {@code true} if this state is equal to the {@code other} state
+     * by inspecting the root and last revision. Two node states are guaranteed
+     * to be equal if their root revisions are equal (even if the two revisions
+     * have different branch flags) or their last revisions are equal. This
+     * method may return {@code false} even if the actual states are in fact
+     * equal!
      *
      * @param other the other state to compare with.
-     * @return {@code true} if the last revisions are equal, {@code false} otherwise.
+     * @return {@code true} if this state is equal to the {@code other} state
+     *      based on the root and last revisions.
      */
     private boolean revisionEquals(AbstractDocumentNodeState other) {
+        if (this.getRootRevision().asTrunkRevision()
+                .equals(other.getRootRevision().asTrunkRevision())) {
+            return true;
+        }
         return this.getLastRevision() != null
                 && this.getLastRevision().equals(other.getLastRevision());
     }

Modified: jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/NodeStoreDiffTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/NodeStoreDiffTest.java?rev=1853054&r1=1853053&r2=1853054&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/NodeStoreDiffTest.java (original)
+++ jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/NodeStoreDiffTest.java Wed Feb  6 08:32:07 2019
@@ -26,6 +26,7 @@ import java.util.Set;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Sets;
 import org.apache.jackrabbit.oak.api.CommitFailedException;
+import org.apache.jackrabbit.oak.cache.CacheStats;
 import org.apache.jackrabbit.oak.commons.PathUtils;
 import org.apache.jackrabbit.oak.plugins.commit.AnnotatingConflictHandler;
 import org.apache.jackrabbit.oak.plugins.commit.ConflictHook;
@@ -47,9 +48,13 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import static org.apache.jackrabbit.oak.plugins.document.DocumentMK.UPDATE_LIMIT;
+import static org.apache.jackrabbit.oak.plugins.document.TestUtils.asDocumentState;
+import static org.apache.jackrabbit.oak.plugins.document.TestUtils.persistToBranch;
 import static org.hamcrest.CoreMatchers.hasItem;
 import static org.hamcrest.CoreMatchers.not;
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertThat;
 
 public class NodeStoreDiffTest {
@@ -193,6 +198,43 @@ public class NodeStoreDiffTest {
         assertThat(tds.paths, not(hasItem("/var/x")));
     }
 
+    @Test
+    public void diffBranchBase() throws Exception {
+        createNodes("/foo", "/bar");
+
+        NodeBuilder b = ns.getRoot().builder();
+        b.child("n");
+        persistToBranch(b);
+
+        DocumentNodeState branchState = asDocumentState(b.getNodeState());
+        Branch branch = ns.getBranches().getBranch(branchState.getRootRevision());
+        assertNotNull(branch);
+        DocumentNodeState headState = ns.getRoot();
+
+        createNodes("/baz");
+
+        DocumentNodeState branchBase = ns.getRoot(branch.getBase().asBranchRevision(ns.getClusterId()));
+
+        tds.reset();
+        long diffCacheRequests = diffCacheRequests(ns);
+
+        branchBase.compareAgainstBaseState(headState, new TrackingDiff());
+        diffCacheRequests = diffCacheRequests(ns) - diffCacheRequests;
+
+        assertThat(tds.paths, not(hasItem("/foo")));
+        assertThat(tds.paths, not(hasItem("/bar")));
+        assertThat(tds.paths, not(hasItem("/baz")));
+        assertEquals(0L, diffCacheRequests);
+    }
+
+    private long diffCacheRequests(DocumentNodeStore ns) {
+        long num = 0;
+        for (CacheStats stats : ns.getDiffCacheStats()) {
+            num += stats.getRequestCount();
+        }
+        return num;
+    }
+
     private NodeState merge(NodeBuilder nb) throws CommitFailedException {
         NodeState result = ns.merge(nb, EmptyHook.INSTANCE, CommitInfo.EMPTY);
         prRev(result);