Author: mreutegg
Date: Wed Oct 7 10:07:17 2015
New Revision: 1707223
URL: http://svn.apache.org/viewvc?rev=1707223&view=rev
Log:
OAK-3474: NodeDocument.getNodeAtRevision can go into property history traversal
when latest rev on current doc isn't committed
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java?rev=1707223&r1=1707222&r2=1707223&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java
Wed Oct 7 10:07:17 2015
@@ -966,8 +966,7 @@ public final class NodeDocument extends
// check if there may be more recent values in a previous document
if (!getPreviousRanges().isEmpty()) {
- Revision newest = local.firstKey();
- if (isRevisionNewer(nodeStore, newest, value.revision)) {
+ if (!isMostRecentCommitted(nodeStore, local, value.revision)) {
// not reading the most recent value, we may need to
// consider previous documents as well
Revision newestPrev = getPreviousRanges().firstKey();
@@ -1709,6 +1708,39 @@ public final class NodeDocument extends
//----------------------------< internal
>----------------------------------
/**
+ * Returns {@code true} if the given {@code revision} is more recent or
+ * equal to the committed revision in {@code valueMap}. This method assumes
+ * the given {@code revision} is committed.
+ *
+ * @param context the revision context.
+ * @param valueMap the value map sorted most recent first.
+ * @param revision a committed revision.
+ * @return if {@code revision} is the most recent committed revision in the
+ * {@code valueMap}.
+ */
+ private boolean isMostRecentCommitted(RevisionContext context,
+ SortedMap<Revision, String> valueMap,
+ Revision revision) {
+ if (valueMap.isEmpty()) {
+ return true;
+ }
+ // shortcut when revision is the first key
+ Revision first = valueMap.firstKey();
+ if (!isRevisionNewer(context, first, revision)) {
+ return true;
+ }
+ // need to check commit status
+ for (Revision r : valueMap.keySet()) {
+ Revision c = getCommitRevision(r);
+ if (c != null) {
+ return !isRevisionNewer(context, c, revision);
+ }
+ }
+ // no committed revision found in valueMap
+ return true;
+ }
+
+ /**
* Returns {@code true} if the two revisions are ambiguous. That is, they
* are from different cluster nodes and the comparison of the two revision
* depends on the seen at revision and is different when just comparing the
Modified:
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java?rev=1707223&r1=1707222&r2=1707223&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java
Wed Oct 7 10:07:17 2015
@@ -53,6 +53,7 @@ import java.util.concurrent.Semaphore;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.atomic.AtomicLong;
import java.util.concurrent.atomic.AtomicReference;
import java.util.concurrent.locks.ReentrantReadWriteLock;
@@ -2268,6 +2269,52 @@ public class DocumentNodeStoreTest {
merge(ns3, b3);
}
+ // OAK-3474
+ @Test
+ public void ignoreUncommitted() throws Exception {
+ final AtomicLong numPreviousFinds = new AtomicLong();
+ MemoryDocumentStore store = new MemoryDocumentStore() {
+ @Override
+ public <T extends Document> T find(Collection<T> collection,
+ String key) {
+ if (Utils.getPathFromId(key).startsWith("p")) {
+ numPreviousFinds.incrementAndGet();
+ }
+ return super.find(collection, key);
+ }
+ };
+ DocumentNodeStore ns = builderProvider.newBuilder()
+ .setDocumentStore(store).setAsyncDelay(0).getNodeStore();
+
+ String id = Utils.getIdFromPath("/test");
+ NodeBuilder b = ns.getRoot().builder();
+ b.child("test").setProperty("p", "a");
+ merge(ns, b);
+ NodeDocument doc;
+ int i = 0;
+ do {
+ b = ns.getRoot().builder();
+ b.child("test").setProperty("q", i++);
+ merge(ns, b);
+ doc = store.find(NODES, id);
+ assertNotNull(doc);
+ if (i % 100 == 0) {
+ ns.runBackgroundOperations();
+ }
+ } while (doc.getPreviousRanges().isEmpty());
+
+ Revision r = ns.newRevision();
+ UpdateOp op = new UpdateOp(id, false);
+ NodeDocument.setCommitRoot(op, r, 0);
+ op.setMapEntry("p", r, "b");
+ assertNotNull(store.findAndUpdate(NODES, op));
+
+ doc = store.find(NODES, id);
+ numPreviousFinds.set(0);
+ doc.getNodeAtRevision(ns, ns.getHeadRevision(), null);
+ assertEquals(0, numPreviousFinds.get());
+ }
+
private static DocumentNodeState asDocumentNodeState(NodeState state) {
if (!(state instanceof DocumentNodeState)) {
throw new IllegalArgumentException("Not a DocumentNodeState");
