Author: mreutegg Date: Mon Nov 2 14:45:20 2015 New Revision: 1712018 URL: http://svn.apache.org/viewvc?rev=1712018&view=rev Log: OAK-3557: NodeDocument.isConflicting() reads complete revision history for changed property
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/PropertyHistory.java jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/NodeDocumentTest.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=1712018&r1=1712017&r2=1712018&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 Mon Nov 2 14:45:20 2015 @@ -1118,7 +1118,7 @@ public final class NodeDocument extends continue; } // was this property touched after baseRevision? - for (Revision rev : getValueMap(name).keySet()) { + for (Revision rev : getChanges(name, baseRevision, context)) { if (rev.equals(commitRevision)) { continue; } @@ -1536,6 +1536,59 @@ public final class NodeDocument extends } /** + * Returns all changes for the given property back to {@code min} revision + * (exclusive). The revisions include committed as well as uncommitted + * changes. + * + * @param property the name of the property. + * @param min the lower bound revision (exclusive). + * @param context the revision context. + * @return changes back to {@code min} revision. + */ + @Nonnull + Iterable<Revision> getChanges(@Nonnull final String property, + @Nonnull final Revision min, + @Nonnull final RevisionContext context) { + return new Iterable<Revision>() { + @Override + public Iterator<Revision> iterator() { + final Set<Revision> changes = getValueMap(property).keySet(); + final Set<Integer> clusterIds = Sets.newHashSet(); + for (Revision r : getLocalMap(property).keySet()) { + clusterIds.add(r.getClusterId()); + } + for (Range r : getPreviousRanges().values()) { + if (isRevisionNewer(context, r.high, min)) { + clusterIds.add(r.high.getClusterId()); + } + } + final Iterator<Revision> unfiltered = changes.iterator(); + return new AbstractIterator<Revision>() { + @Override + protected Revision computeNext() { + while (unfiltered.hasNext()) { + Revision next = unfiltered.next(); + if (isRevisionNewer(context, next, min)) { + return next; + } else { + // further revisions with this clusterId + // are older than min revision + clusterIds.remove(next.getClusterId()); + // no more revisions to check + if (clusterIds.isEmpty()) { + return endOfData(); + } + } + } + return endOfData(); + } + }; + } + }; + + } + + /** * Returns the local value map for the given key. * * @param key the key. Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/PropertyHistory.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/PropertyHistory.java?rev=1712018&r1=1712017&r2=1712018&view=diff ============================================================================== --- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/PropertyHistory.java (original) +++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/PropertyHistory.java Mon Nov 2 14:45:20 2015 @@ -23,7 +23,6 @@ import static java.util.AbstractMap.Simp import java.util.Iterator; import java.util.Map; -import java.util.NoSuchElementException; import java.util.TreeMap; import javax.annotation.Nonnull; @@ -31,6 +30,7 @@ import javax.annotation.Nullable; import com.google.common.base.Function; import com.google.common.base.Predicates; +import com.google.common.collect.AbstractIterator; import com.google.common.collect.Iterators; import com.google.common.collect.PeekingIterator; import org.apache.jackrabbit.oak.plugins.document.util.Utils; @@ -85,37 +85,17 @@ class PropertyHistory implements Iterabl * @return the docs in the correct order. */ private Iterator<NodeDocument> ensureOrder(final Iterable<Map.Entry<Revision, NodeDocument>> docs) { - return new Iterator<NodeDocument>() { + return new AbstractIterator<NodeDocument>() { PeekingIterator<Map.Entry<Revision, NodeDocument>> input = Iterators.peekingIterator(docs.iterator()); TreeMap<Revision, NodeDocument> queue = new TreeMap<Revision, NodeDocument>(StableRevisionComparator.INSTANCE); - NodeDocument next = fetchNext(); @Override - public boolean hasNext() { - return next != null; - } - - @Override - public NodeDocument next() { - if (!hasNext()) { - throw new NoSuchElementException(); - } - NodeDocument doc = next; - next = fetchNext(); - return doc; - } - - @Override - public void remove() { - throw new UnsupportedOperationException(); - } - - private NodeDocument fetchNext() { + protected NodeDocument computeNext() { refillQueue(); if (queue.isEmpty()) { - return null; + return endOfData(); } else { return queue.remove(queue.lastKey()); } Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/NodeDocumentTest.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/NodeDocumentTest.java?rev=1712018&r1=1712017&r2=1712018&view=diff ============================================================================== --- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/NodeDocumentTest.java (original) +++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/NodeDocumentTest.java Mon Nov 2 14:45:20 2015 @@ -21,6 +21,7 @@ import java.util.Comparator; import java.util.HashSet; import java.util.Iterator; import java.util.List; +import java.util.Map; import java.util.Random; import java.util.Set; @@ -499,6 +500,92 @@ public class NodeDocumentTest { ns.dispose(); } + @Test + public void getChanges() throws Exception { + final int numChanges = 200; + Random random = new Random(); + DocumentNodeStore ns = createTestStore(numChanges); + DocumentStore store = ns.getDocumentStore(); + NodeDocument doc = getRootDocument(store); + for (int i = 0; i < 10; i++) { + int idx = random.nextInt(numChanges); + Revision r = Iterables.get(doc.getValueMap("p").keySet(), idx); + Iterable<Revision> revs = doc.getChanges("p", r, ns); + assertEquals(idx, Iterables.size(revs)); + } + ns.dispose(); + } + + @Test + public void getChangesMixedClusterIds() throws Exception { + final int numChanges = 200; + Random random = new Random(); + MemoryDocumentStore store = new MemoryDocumentStore(); + DocumentNodeStore ns1 = createTestStore(store, 0); + DocumentNodeStore ns2 = createTestStore(store, 0); + List<DocumentNodeStore> nodeStores = Lists.newArrayList(ns1, ns2); + + for (int i = 0; i < numChanges; i++) { + DocumentNodeStore ns = nodeStores.get(random.nextInt(nodeStores.size())); + ns.runBackgroundOperations(); + NodeBuilder builder = ns.getRoot().builder(); + builder.setProperty("p", i); + merge(ns, builder); + ns.runBackgroundOperations(); + if (random.nextDouble() < 0.2) { + Revision head = ns.getHeadRevision(); + for (UpdateOp op : SplitOperations.forDocument( + getRootDocument(store), ns, head, 2)) { + store.createOrUpdate(NODES, op); + } + } + } + + NodeDocument doc = getRootDocument(store); + for (int i = 0; i < 10; i++) { + int idx = random.nextInt(numChanges); + Revision r = Iterables.get(doc.getValueMap("p").keySet(), idx); + Iterable<Revision> revs1 = doc.getChanges("p", r, ns1); + Iterable<Revision> revs2 = doc.getChanges("p", r, ns2); + assertEquals(Iterables.size(revs1), Iterables.size(revs2)); + assertEquals(idx, Iterables.size(revs1)); + } + + ns1.dispose(); + ns2.dispose(); + } + + // OAK-3557 + @Test + public void isConflicting() throws Exception { + final int numChanges = 200; + final Set<String> prevDocCalls = Sets.newHashSet(); + MemoryDocumentStore store = new MemoryDocumentStore() { + @Override + public <T extends Document> T find(Collection<T> collection, + String key) { + if (Utils.getPathFromId(key).startsWith("p")) { + prevDocCalls.add(key); + } + return super.find(collection, key); + } + }; + DocumentNodeStore ns = createTestStore(store, numChanges); + NodeDocument doc = getRootDocument(store); + Map<Revision, String> valueMap = doc.getValueMap("p"); + assertEquals(200, valueMap.size()); + Revision baseRev = valueMap.keySet().iterator().next(); + Revision commitRev = ns.newRevision(); + UpdateOp op = new UpdateOp(Utils.getIdFromPath("/"), false); + op.setMapEntry("p", commitRev, "v"); + + prevDocCalls.clear(); + assertFalse(doc.isConflicting(op, baseRev, commitRev, ns, false)); + assertTrue("too many calls for previous documents: " + prevDocCalls, + prevDocCalls.size() <= 6); + ns.dispose(); + } + private DocumentNodeStore createTestStore(int numChanges) throws Exception { return createTestStore(new MemoryDocumentStore(), numChanges); }