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 2013/09/24 13:21:04 UTC

svn commit: r1525856 [1/2] - in /jackrabbit/oak/trunk/oak-core/src: main/java/org/apache/jackrabbit/oak/plugins/mongomk/ main/java/org/apache/jackrabbit/oak/plugins/mongomk/util/ test/java/org/apache/jackrabbit/oak/plugins/mongomk/ test/java/org/apache...

Author: mreutegg
Date: Tue Sep 24 11:21:03 2013
New Revision: 1525856

URL: http://svn.apache.org/r1525856
Log:
OAK-1025: Keep value histories in sorted map

Added:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/StableRevisionComparator.java   (with props)
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/util/MergeSortedIterators.java   (with props)
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/mongomk/util/MergeSortedIteratorsTest.java   (with props)
Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/Collision.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/Commit.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/MemoryDocumentStore.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/MongoDocumentStore.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/MongoMK.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/Node.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/NodeDocument.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/UpdateOp.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/ValueMap.java
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/mongomk/DocumentSplitTest.java
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/mongomk/MongoDocumentStoreTest.java
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/mongomk/SimpleTest.java

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/Collision.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/Collision.java?rev=1525856&r1=1525855&r2=1525856&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/Collision.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/Collision.java Tue Sep 24 11:21:03 2013
@@ -47,15 +47,18 @@ class Collision {
     private final Revision theirRev;
     private final UpdateOp ourOp;
     private final Revision ourRev;
+    private final RevisionContext context;
 
     Collision(@Nonnull NodeDocument document,
               @Nonnull Revision theirRev,
               @Nonnull UpdateOp ourOp,
-              @Nonnull Revision ourRev) {
+              @Nonnull Revision ourRev,
+              @Nonnull RevisionContext context) {
         this.document = checkNotNull(document);
         this.theirRev = checkNotNull(theirRev);
         this.ourOp = checkNotNull(ourOp);
         this.ourRev = checkNotNull(ourRev);
+        this.context = checkNotNull(context);
     }
 
     /**
@@ -74,7 +77,7 @@ class Collision {
         // their commit wins, we have to mark ourRev
         NodeDocument newDoc = Collection.NODES.newDocument(store);
         document.deepCopy(newDoc);
-        MemoryDocumentStore.applyChanges(newDoc, ourOp);
+        MemoryDocumentStore.applyChanges(newDoc, ourOp, context.getRevisionComparator());
         if (!markCommitRoot(newDoc, ourRev, store)) {
             throw new MicroKernelException("Unable to annotate our revision "
                     + "with collision marker. Our revision: " + ourRev
@@ -96,7 +99,6 @@ class Collision {
     private static boolean markCommitRoot(@Nonnull NodeDocument document,
                                           @Nonnull Revision revision,
                                           @Nonnull DocumentStore store) {
-        String rev = revision.toString();
         String p = Utils.getPathFromId(document.getId());
         String commitRootPath = null;
         // first check if we can mark the commit with the given revision
@@ -110,22 +112,22 @@ class Collision {
             commitRootPath = p;
         } else {
             // next look at commit root
-            commitRootPath = document.getCommitRootPath(rev);
+            commitRootPath = document.getCommitRootPath(revision);
             if (commitRootPath == null) {
-                throwNoCommitRootException(rev, document);
+                throwNoCommitRootException(revision, document);
             }
         }
         // at this point we have a commitRootPath
         UpdateOp op = new UpdateOp(Utils.getIdFromPath(commitRootPath), false);
-        document = store.find(Collection.NODES, op.getKey());
+        NodeDocument commitRoot = store.find(Collection.NODES, op.getId());
         // check commit status of revision
-        if (document.isCommitted(revision)) {
+        if (commitRoot.isCommitted(revision)) {
             return false;
         }
-        op.setMapEntry(NodeDocument.COLLISIONS, rev, true);
-        document = store.createOrUpdate(Collection.NODES, op);
+        op.setMapEntry(NodeDocument.COLLISIONS, revision, true);
+        commitRoot = store.createOrUpdate(Collection.NODES, op);
         // check again on old document right before our update was applied
-        if (document.isCommitted(revision)) {
+        if (commitRoot.isCommitted(revision)) {
             return false;
         }
         // otherwise collision marker was set successfully
@@ -134,7 +136,7 @@ class Collision {
         return true;
     }
     
-    private static void throwNoCommitRootException(@Nonnull String revision,
+    private static void throwNoCommitRootException(@Nonnull Revision revision,
                                                    @Nonnull Document document)
                                                            throws MicroKernelException {
         throw new MicroKernelException("No commit root for revision: "

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/Commit.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/Commit.java?rev=1525856&r1=1525855&r2=1525856&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/Commit.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/Commit.java Tue Sep 24 11:21:03 2013
@@ -120,7 +120,7 @@ public class Commit {
     void updateProperty(String path, String propertyName, String value) {
         UpdateOp op = getUpdateOperationForNode(path);
         String key = Utils.escapePropertyName(propertyName);
-        op.setMapEntry(key, revision.toString(), value);
+        op.setMapEntry(key, revision, value);
     }
 
     void addNode(Node n) {
@@ -277,7 +277,7 @@ public class Commit {
             store.createOrUpdate(Collection.NODES, reverse);
         }
         for (UpdateOp op : newDocuments) {
-            store.remove(Collection.NODES, op.key);
+            store.remove(Collection.NODES, op.id);
         }
     }
 
@@ -309,17 +309,17 @@ public class Commit {
             if (newestRev == null) {
                 if (op.isDelete || !op.isNew) {
                     conflictMessage = "The node " + 
-                            op.getKey() + " does not exist or is already deleted";
+                            op.getId() + " does not exist or is already deleted";
                 }
             } else {
                 if (op.isNew) {
                     conflictMessage = "The node " + 
-                            op.getKey() + " was already added in revision\n" +
+                            op.getId() + " was already added in revision\n" +
                             newestRev;
                 } else if (mk.isRevisionNewer(newestRev, baseRevision)
                         && (op.isDelete || isConflicting(doc, op))) {
                     conflictMessage = "The node " + 
-                            op.getKey() + " was changed in revision\n" + newestRev +
+                            op.getId() + " was changed in revision\n" + newestRev +
                             ", which was applied after the base revision\n" + 
                             baseRevision;
                 }
@@ -336,7 +336,7 @@ public class Commit {
             if (collisions.get() != null && isConflicting(doc, op)) {
                 for (Revision r : collisions.get()) {
                     // mark collisions on commit root
-                    new Collision(doc, r, op, revision).mark(store);
+                    new Collision(doc, r, op, revision, mk).mark(store);
                 }
             }
         }

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/MemoryDocumentStore.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/MemoryDocumentStore.java?rev=1525856&r1=1525855&r2=1525856&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/MemoryDocumentStore.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/MemoryDocumentStore.java Tue Sep 24 11:21:03 2013
@@ -17,6 +17,8 @@
 package org.apache.jackrabbit.oak.plugins.mongomk;
 
 import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
@@ -31,7 +33,11 @@ import javax.annotation.Nonnull;
 
 import org.apache.jackrabbit.mk.api.MicroKernelException;
 import org.apache.jackrabbit.oak.plugins.mongomk.UpdateOp.Operation;
-import org.apache.jackrabbit.oak.plugins.mongomk.util.Utils;
+
+import com.google.common.collect.Maps;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+import static org.apache.jackrabbit.oak.plugins.mongomk.UpdateOp.Key;
 
 /**
  * Emulates a MongoDB store (possibly consisting of multiple shards and
@@ -44,7 +50,7 @@ public class MemoryDocumentStore impleme
      */
     private ConcurrentSkipListMap<String, NodeDocument> nodes =
             new ConcurrentSkipListMap<String, NodeDocument>();
-    
+
     /**
      * The 'clusterNodes' collection.
      */
@@ -53,6 +59,12 @@ public class MemoryDocumentStore impleme
 
     private final ReadWriteLock rwLock = new ReentrantReadWriteLock();
 
+    /**
+     * Comparator for maps with {@link Revision} keys. The maps are ordered
+     * descending, newest revisions first!
+     */
+    private final Comparator<Revision> comparator = Collections.reverseOrder(new StableRevisionComparator());
+
     @Override
     public <T extends Document> T find(Collection<T> collection, String key, int maxCacheAge) {
         return find(collection, key);
@@ -164,12 +176,12 @@ public class MemoryDocumentStore impleme
         lock.lock();
         try {
             // get the node if it's there
-            oldDoc = map.get(update.key);
+            oldDoc = map.get(update.id);
 
             T doc = collection.newDocument(this);
             if (oldDoc == null) {
                 if (!update.isNew) {
-                    throw new MicroKernelException("Document does not exist: " + update.key);
+                    throw new MicroKernelException("Document does not exist: " + update.id);
                 }
             } else {
                 oldDoc.deepCopy(doc);
@@ -178,9 +190,9 @@ public class MemoryDocumentStore impleme
                 return null;
             }
             // update the document
-            applyChanges(doc, update);
+            applyChanges(doc, update, comparator);
             doc.seal();
-            map.put(update.key, doc);
+            map.put(update.id, doc);
             return oldDoc;
         } finally {
             lock.unlock();
@@ -189,12 +201,16 @@ public class MemoryDocumentStore impleme
 
     private static boolean checkConditions(Document doc,
                                            UpdateOp update) {
-        for (Map.Entry<String, Operation> change : update.changes.entrySet()) {
+        for (Map.Entry<Key, Operation> change : update.changes.entrySet()) {
             Operation op = change.getValue();
             if (op.type == Operation.Type.CONTAINS_MAP_ENTRY) {
-                String k = change.getKey();
-                String[] kv = k.split("\\.");
-                Object value = doc.get(kv[0]);
+                Key k = change.getKey();
+                Revision r = k.getRevision();
+                if (r == null) {
+                    throw new IllegalStateException(
+                            "CONTAINS_MAP_ENTRY must not contain null revision");
+                }
+                Object value = doc.get(k.getName());
                 if (value == null) {
                     if (Boolean.TRUE.equals(op.value)) {
                         return false;
@@ -203,11 +219,11 @@ public class MemoryDocumentStore impleme
                     if (value instanceof Map) {
                         Map<?, ?> map = (Map<?, ?>) value;
                         if (Boolean.TRUE.equals(op.value)) {
-                            if (!map.containsKey(kv[1])) {
+                            if (!map.containsKey(r)) {
                                 return false;
                             }
                         } else {
-                            if (map.containsKey(kv[1])) {
+                            if (map.containsKey(r)) {
                                 return false;
                             }
                         }
@@ -225,60 +241,49 @@ public class MemoryDocumentStore impleme
      * Apply the changes to the in-memory document.
      * 
      * @param doc the target document.
-     * @param update the changes to apply
+     * @param update the changes to apply.
+     * @param comparator the revision comparator.
      */
-    public static void applyChanges(Document doc, UpdateOp update) {
-        for (Entry<String, Operation> e : update.changes.entrySet()) {
-            String k = e.getKey();
+    public static void applyChanges(@Nonnull Document doc,
+                                    @Nonnull UpdateOp update,
+                                    @Nonnull Comparator<Revision> comparator) {
+        for (Entry<Key, Operation> e : checkNotNull(update).changes.entrySet()) {
+            Key k = e.getKey();
             Operation op = e.getValue();
             switch (op.type) {
             case SET: {
-                doc.put(k, op.value);
+                doc.put(k.toString(), op.value);
                 break;
             }
             case INCREMENT: {
-                Object old = doc.get(k);
+                Object old = doc.get(k.toString());
                 Long x = (Long) op.value;
                 if (old == null) {
                     old = 0L;
                 }
-                doc.put(k, ((Long) old) + x);
+                doc.put(k.toString(), ((Long) old) + x);
                 break;
             }
             case SET_MAP_ENTRY: {
-                String[] kv = splitInTwo(k, '.');
-                Object old = doc.get(kv[0]);
+                Object old = doc.get(k.getName());
                 @SuppressWarnings("unchecked")
-                Map<String, Object> m = (Map<String, Object>) old;
+                Map<Revision, Object> m = (Map<Revision, Object>) old;
                 if (m == null) {
-                    m = Utils.newMap();
-                    doc.put(kv[0], m);
+                    m = Maps.newTreeMap(comparator);
+                    doc.put(k.getName(), m);
                 }
-                m.put(kv[1], op.value);
+                m.put(k.getRevision(), op.value);
                 break;
             }
             case REMOVE_MAP_ENTRY: {
-                String[] kv = splitInTwo(k, '.');
-                Object old = doc.get(kv[0]);
+                Object old = doc.get(k.getName());
                 @SuppressWarnings("unchecked")
-                Map<String, Object> m = (Map<String, Object>) old;
+                Map<Revision, Object> m = (Map<Revision, Object>) old;
                 if (m != null) {
-                    m.remove(kv[1]);
+                    m.remove(k.getRevision());
                 }
                 break;
             }
-            case SET_MAP: {
-                String[] kv = splitInTwo(k, '.');
-                Object old = doc.get(kv[0]);
-                @SuppressWarnings("unchecked")
-                Map<String, Object> m = (Map<String, Object>) old;
-                if (m == null) {
-                    m = Utils.newMap();
-                    doc.put(kv[0], m);
-                }
-                m.put(kv[1], op.value);
-                break;
-            }
             case CONTAINS_MAP_ENTRY:
                 // no effect
                 break;
@@ -286,14 +291,6 @@ public class MemoryDocumentStore impleme
         }
     }
     
-    private static String[] splitInTwo(String s, char separator) {
-        int index = s.indexOf(separator);
-        if (index < 0) {
-            return new String[] { s };
-        }
-        return new String[] { s.substring(0, index), s.substring(index + 1) };
-    }
-
     @Override
     public <T extends Document> boolean create(Collection<T> collection,
                                                List<UpdateOp> updateOps) {
@@ -302,7 +299,7 @@ public class MemoryDocumentStore impleme
         try {
             ConcurrentSkipListMap<String, T> map = getMap(collection);
             for (UpdateOp op : updateOps) {
-                if (map.containsKey(op.key)) {
+                if (map.containsKey(op.id)) {
                     return false;
                 }
             }

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/MongoDocumentStore.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/MongoDocumentStore.java?rev=1525856&r1=1525855&r2=1525856&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/MongoDocumentStore.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/MongoDocumentStore.java Tue Sep 24 11:21:03 2013
@@ -18,8 +18,12 @@ package org.apache.jackrabbit.oak.plugin
 
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
+import java.util.Comparator;
 import java.util.List;
+import java.util.Map;
 import java.util.Map.Entry;
+import java.util.TreeMap;
 import java.util.concurrent.Callable;
 import java.util.concurrent.ExecutionException;
 
@@ -44,6 +48,8 @@ import com.mongodb.QueryBuilder;
 import com.mongodb.WriteConcern;
 import com.mongodb.WriteResult;
 
+import static org.apache.jackrabbit.oak.plugins.mongomk.UpdateOp.Key;
+
 /**
  * A document store that uses MongoDB as the backend.
  */
@@ -64,12 +70,18 @@ public class MongoDocumentStore implemen
     private final Cache<String, NodeDocument> nodesCache;
     private final CacheStats cacheStats;
 
+    /**
+     * Comparator for maps with {@link Revision} keys. The maps are ordered
+     * descending, newest revisions first!
+     */
+    private final Comparator<Revision> comparator = Collections.reverseOrder(new StableRevisionComparator());
+
     public MongoDocumentStore(DB db, MongoMK.Builder builder) {
         nodes = db.getCollection(
                 Collection.NODES.toString());
         clusterNodes = db.getCollection(
                 Collection.CLUSTER_NODES.toString());
-        
+
         // indexes:
         // the _id field is the primary key, so we don't need to define it
         DBObject index = new BasicDBObject();
@@ -257,46 +269,39 @@ public class MongoDocumentStore implemen
                                                  boolean upsert,
                                                  boolean checkConditions) {
         DBCollection dbCollection = getDBCollection(collection);
-        QueryBuilder query = getByKeyQuery(updateOp.key);
+        QueryBuilder query = getByKeyQuery(updateOp.id);
 
         BasicDBObject setUpdates = new BasicDBObject();
         BasicDBObject incUpdates = new BasicDBObject();
         BasicDBObject unsetUpdates = new BasicDBObject();
 
-        for (Entry<String, Operation> entry : updateOp.changes.entrySet()) {
-            String k = entry.getKey();
-            if (k.equals(Document.ID)) {
+        for (Entry<Key, Operation> entry : updateOp.changes.entrySet()) {
+            Key k = entry.getKey();
+            if (k.getName().equals(Document.ID)) {
                 // avoid exception "Mod on _id not allowed"
                 continue;
             }
             Operation op = entry.getValue();
             switch (op.type) {
                 case SET: {
-                    setUpdates.append(k, op.value);
+                    setUpdates.append(k.toString(), op.value);
                     break;
                 }
                 case INCREMENT: {
-                    incUpdates.append(k, op.value);
+                    incUpdates.append(k.toString(), op.value);
                     break;
                 }
                 case SET_MAP_ENTRY: {
-                    setUpdates.append(k, op.value);
+                    setUpdates.append(k.toString(), op.value);
                     break;
                 }
                 case REMOVE_MAP_ENTRY: {
-                    unsetUpdates.append(k, "1");
-                    break;
-                }
-                case SET_MAP: {
-                    String[] kv = k.split("\\.");
-                    BasicDBObject sub = new BasicDBObject();
-                    sub.put(kv[1], op.value);
-                    setUpdates.append(kv[0], sub);
+                    unsetUpdates.append(k.toString(), "1");
                     break;
                 }
                 case CONTAINS_MAP_ENTRY: {
                     if (checkConditions) {
-                        query.and(k).exists(op.value);
+                        query.and(k.toString()).exists(op.value);
                     }
                     break;
                 }
@@ -335,8 +340,8 @@ public class MongoDocumentStore implemen
                     oldDoc.deepCopy(newDoc);
                     oldDoc.seal();
                 }
-                String key = updateOp.getKey();
-                MemoryDocumentStore.applyChanges(newDoc, updateOp);
+                String key = updateOp.getId();
+                MemoryDocumentStore.applyChanges(newDoc, updateOp, comparator);
                 newDoc.seal();
                 nodesCache.put(key, (NodeDocument) newDoc);
             }
@@ -377,22 +382,26 @@ public class MongoDocumentStore implemen
             inserts[i] = new BasicDBObject();
             UpdateOp update = updateOps.get(i);
             T target = collection.newDocument(this);
-            MemoryDocumentStore.applyChanges(target, update);
+            MemoryDocumentStore.applyChanges(target, update, comparator);
             docs.add(target);
-            for (Entry<String, Operation> entry : update.changes.entrySet()) {
-                String k = entry.getKey();
+            for (Entry<Key, Operation> entry : update.changes.entrySet()) {
+                Key k = entry.getKey();
                 Operation op = entry.getValue();
                 switch (op.type) {
                     case SET:
                     case INCREMENT: {
-                        inserts[i].put(k, op.value);
+                        inserts[i].put(k.toString(), op.value);
                         break;
                     }
-                    case SET_MAP:
                     case SET_MAP_ENTRY: {
-                        String[] kv = k.split("\\.");
-                        DBObject value = new BasicDBObject(kv[1], op.value);
-                        inserts[i].put(kv[0], value);
+                        Revision r = k.getRevision();
+                        if (r == null) {
+                            throw new IllegalStateException(
+                                    "SET_MAP_ENTRY must not have null revision");
+                        }
+                        DBObject value = new BasicDBObject(
+                                k.getRevision().toString(), op.value);
+                        inserts[i].put(k.getName(), value);
                         break;
                     }
                     case REMOVE_MAP_ENTRY:
@@ -441,13 +450,22 @@ public class MongoDocumentStore implemen
                 } else if (o instanceof Long) {
                     copy.put(key, o);
                 } else if (o instanceof BasicDBObject) {
-                    copy.put(key, o);
+                    copy.put(key, convertMongoMap((BasicDBObject) o));
                 }
             }
         }
         return copy;
     }
 
+    @Nonnull
+    private Map<Revision, Object> convertMongoMap(@Nonnull BasicDBObject obj) {
+        Map<Revision, Object> map = new TreeMap<Revision, Object>(comparator);
+        for (Map.Entry<String, Object> entry : obj.entrySet()) {
+            map.put(Revision.fromString(entry.getKey()), entry.getValue());
+        }
+        return map;
+    }
+
     private <T extends Document> DBCollection getDBCollection(Collection<T> collection) {
         if (collection == Collection.NODES) {
             return nodes;

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/MongoMK.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/MongoMK.java?rev=1525856&r1=1525855&r2=1525856&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/MongoMK.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/MongoMK.java Tue Sep 24 11:21:03 2013
@@ -263,10 +263,10 @@ public class MongoMK implements MicroKer
             clusterNodeInfo = null;
         }
         this.clusterId = cid;
-        
+
         this.revisionComparator = new RevisionComparator(clusterId);
         this.asyncDelay = builder.getAsyncDelay();
-        this.branches = new UnmergedBranches(revisionComparator);
+        this.branches = new UnmergedBranches(getRevisionComparator());
 
         //TODO Make stats collection configurable as it add slight overhead
         //TODO Expose the stats as JMX beans
@@ -290,7 +290,7 @@ public class MongoMK implements MicroKer
         init();
         // initial reading of the revisions of other cluster nodes
         backgroundRead();
-        revisionComparator.add(headRevision, Revision.newRevision(0));
+        getRevisionComparator().add(headRevision, Revision.newRevision(0));
         headRevision = newRevision();
         LOG.info("Initialized MongoMK with clusterNodeId: {}", clusterId);
     }
@@ -364,8 +364,12 @@ public class MongoMK implements MicroKer
     void backgroundRead() {
         String id = Utils.getIdFromPath("/");
         NodeDocument doc = store.find(Collection.NODES, id, asyncDelay);
+        if (doc == null) {
+            return;
+        }
         Map<Integer, Revision> lastRevMap = doc.getLastRev();
         
+        RevisionComparator revisionComparator = getRevisionComparator();
         boolean hasNewRevisions = false;
         // the (old) head occurred first
         Revision headSeen = Revision.newRevision(0);
@@ -411,9 +415,11 @@ public class MongoMK implements MicroKer
             for (UpdateOp op : doc.split(this)) {
                 NodeDocument before = store.createOrUpdate(Collection.NODES, op);
                 if (before != null) {
-                    NodeDocument after = store.find(Collection.NODES, op.getKey());
-                    LOG.info("Split operation on {}. Size before: {}, after: {}",
-                            new Object[]{id, before.getMemory(), after.getMemory()});
+                    NodeDocument after = store.find(Collection.NODES, op.getId());
+                    if (after != null) {
+                        LOG.info("Split operation on {}. Size before: {}, after: {}",
+                                new Object[]{id, before.getMemory(), after.getMemory()});
+                    }
                 }
             }
             it.remove();
@@ -530,7 +536,7 @@ public class MongoMK implements MicroKer
      * @return true if x is newer
      */
     boolean isRevisionNewer(@Nonnull Revision x, @Nonnull Revision previous) {
-        return revisionComparator.compare(x, previous) > 0;
+        return getRevisionComparator().compare(x, previous) > 0;
     }
 
     public long getSplitDocumentAgeMillis() {
@@ -1014,6 +1020,7 @@ public class MongoMK implements MicroKer
 
     @Override
     public void publishRevision(Revision foreignRevision, Revision changeRevision) {
+        RevisionComparator revisionComparator = getRevisionComparator();
         if (revisionComparator.compare(headRevision, foreignRevision) >= 0) {
             // already visible
             return;
@@ -1163,7 +1170,7 @@ public class MongoMK implements MicroKer
             for (Revision rev : b.getCommits()) {
                 rev = rev.asTrunkRevision();
                 NodeDocument.setRevision(op, rev, "c-" + mergeCommit.toString());
-                op.containsMapEntry(NodeDocument.COLLISIONS, rev.toString(), false);
+                op.containsMapEntry(NodeDocument.COLLISIONS, rev, false);
             }
             if (store.findAndUpdate(Collection.NODES, op) != null) {
                 // remove from branchCommits map after successful update

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/Node.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/Node.java?rev=1525856&r1=1525855&r2=1525856&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/Node.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/Node.java Tue Sep 24 11:21:03 2013
@@ -81,7 +81,7 @@ public class Node implements CacheValue 
         NodeDocument.setDeleted(op, rev, false);
         for (String p : properties.keySet()) {
             String key = Utils.escapePropertyName(p);
-            op.setMapEntry(key, rev.toString(), properties.get(p));
+            op.setMapEntry(key, rev, properties.get(p));
         }
         return op;
     }

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/NodeDocument.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/NodeDocument.java?rev=1525856&r1=1525855&r2=1525856&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/NodeDocument.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/NodeDocument.java Tue Sep 24 11:21:03 2013
@@ -28,9 +28,7 @@ import java.util.Map;
 import java.util.NavigableMap;
 import java.util.Set;
 import java.util.SortedMap;
-import java.util.SortedSet;
 import java.util.TreeMap;
-import java.util.TreeSet;
 
 import javax.annotation.CheckForNull;
 import javax.annotation.Nonnull;
@@ -48,6 +46,8 @@ import com.google.common.collect.Iterabl
 import com.google.common.collect.Maps;
 
 import static com.google.common.base.Preconditions.checkNotNull;
+import static org.apache.jackrabbit.oak.plugins.mongomk.UpdateOp.Key;
+import static org.apache.jackrabbit.oak.plugins.mongomk.UpdateOp.Operation;
 
 /**
  * A document storing data about a node.
@@ -148,7 +148,7 @@ public class NodeDocument extends Docume
      * @return the map associated with the key.
      */
     @Nonnull
-    public Map<String, String> getValueMap(@Nonnull String key) {
+    public Map<Revision, String> getValueMap(@Nonnull String key) {
         Object value = super.get(key);
         if (IGNORE_ON_SPLIT.contains(key) || !(value instanceof Map)) {
             return Collections.emptyMap();
@@ -170,9 +170,9 @@ public class NodeDocument extends Docume
     @Nonnull
     public Map<Integer, Revision> getLastRev() {
         Map<Integer, Revision> map = Maps.newHashMap();
-        Map<String, String> valueMap = getLocalMap(LAST_REV);
-        for (Map.Entry<String, String> e : valueMap.entrySet()) {
-            int clusterId = Integer.parseInt(e.getKey());
+        Map<Revision, String> valueMap = getLocalMap(LAST_REV);
+        for (Map.Entry<Revision, String> e : valueMap.entrySet()) {
+            int clusterId = e.getKey().getClusterId();
             Revision rev = Revision.fromString(e.getValue());
             map.put(clusterId, rev);
         }
@@ -191,8 +191,7 @@ public class NodeDocument extends Docume
         if (commitRootDoc == null) {
             return false;
         }
-        String rev = checkNotNull(revision).toString();
-        String value = commitRootDoc.getLocalRevisions().get(rev);
+        String value = commitRootDoc.getLocalRevisions().get(revision);
         if (value != null) {
             return Utils.isCommitted(value);
         }
@@ -216,8 +215,7 @@ public class NodeDocument extends Docume
      * @return <code>true</code> if this document contains the given revision.
      */
     public boolean containsRevision(@Nonnull Revision revision) {
-        String rev = checkNotNull(revision).toString();
-        if (getLocalRevisions().containsKey(rev)) {
+        if (getLocalRevisions().containsKey(revision)) {
             return true;
         }
         for (NodeDocument prev : getPreviousDocs(revision, REVISIONS)) {
@@ -229,16 +227,6 @@ public class NodeDocument extends Docume
     }
 
     /**
-     * Same as {@link #containsRevision(Revision)}, but with a String parameter.
-     *
-     * @param revision the revision.
-     * @return <code>true</code> if this document contains the given revision.
-     */
-    public boolean containsRevision(@Nonnull String revision) {
-        return containsRevision(Revision.fromString(checkNotNull(revision)));
-    }
-
-    /**
      * Gets a sorted map of uncommitted revisions of this document with the
      * local cluster node id as returned by the {@link RevisionContext}. These
      * are the {@link #REVISIONS} entries where {@link Utils#isCommitted(String)}
@@ -250,12 +238,12 @@ public class NodeDocument extends Docume
     public SortedMap<Revision, Revision> getUncommittedRevisions(RevisionContext context) {
         // only look at revisions in this document.
         // uncommitted revisions are not split off
-        Map<String, String> valueMap = getLocalRevisions();
+        Map<Revision, String> valueMap = getLocalRevisions();
         SortedMap<Revision, Revision> revisions =
                 new TreeMap<Revision, Revision>(context.getRevisionComparator());
-        for (Map.Entry<String, String> commit : valueMap.entrySet()) {
+        for (Map.Entry<Revision, String> commit : valueMap.entrySet()) {
             if (!Utils.isCommitted(commit.getValue())) {
-                Revision r = Revision.fromString(commit.getKey());
+                Revision r = commit.getKey();
                 if (r.getClusterId() == context.getClusterId()) {
                     Revision b = Revision.fromString(commit.getValue());
                     revisions.put(r, b);
@@ -274,8 +262,8 @@ public class NodeDocument extends Docume
      * @return the commit root path or <code>null</code>.
      */
     @CheckForNull
-    public String getCommitRootPath(String revision) {
-        Map<String, String> valueMap = getCommitRoot();
+    public String getCommitRootPath(Revision revision) {
+        Map<Revision, String> valueMap = getCommitRoot();
         String depth = valueMap.get(revision);
         if (depth != null) {
             String p = Utils.getPathFromId(getId());
@@ -300,29 +288,30 @@ public class NodeDocument extends Docume
     public Revision getNewestRevision(RevisionContext context,
                                       Revision changeRev,
                                       CollisionHandler handler) {
-        SortedSet<String> revisions = new TreeSet<String>(Collections.reverseOrder());
         // no need to look at all commits. the primary document
         // always contains at least one commit, including all
         // branch commits which are not yet merged
-        revisions.addAll(getLocalRevisions().keySet());
-        revisions.addAll(getLocalCommitRoot().keySet());
+        SortedMap<Revision, String> revisions = getLocalRevisions();
+        SortedMap<Revision, String> commitRoots = getLocalCommitRoot();
         Revision newestRev = null;
-        for (String r : revisions) {
-            Revision propRev = Revision.fromString(r);
-            if (isRevisionNewer(context, propRev, changeRev)) {
+        for (Revision r : Iterables.mergeSorted(
+                Arrays.asList(revisions.keySet(), commitRoots.keySet()),
+                revisions.comparator())) {
+            if (isRevisionNewer(context, r, changeRev)) {
                 // we have seen a previous change from another cluster node
                 // (which might be conflicting or not) - we need to make
                 // sure this change is visible from now on
                 // TODO verify this is really needed
-                context.publishRevision(propRev, changeRev);
+                context.publishRevision(r, changeRev);
             }
-            if (newestRev == null || isRevisionNewer(context, propRev, newestRev)) {
-                if (!propRev.equals(changeRev)) {
-                    if (!isValidRevision(context, propRev, changeRev, new HashSet<Revision>())) {
-                        handler.concurrentModification(propRev);
-                    } else {
-                        newestRev = propRev;
-                    }
+            if (!r.equals(changeRev)) {
+                if (!isValidRevision(context, r, changeRev, new HashSet<Revision>())) {
+                    handler.concurrentModification(r);
+                } else {
+                    newestRev = r;
+                    // found newest revision, no need to check more revisions
+                    // revisions are sorted newest first
+                    break;
                 }
             }
         }
@@ -330,7 +319,7 @@ public class NodeDocument extends Docume
             return null;
         }
 
-        String value = getDeleted().get(newestRev.toString());
+        String value = getDeleted().get(newestRev);
         if ("true".equals(value)) {
             // deleted in the newest revision
             return null;
@@ -508,19 +497,18 @@ public class NodeDocument extends Docume
                                  @Nonnull Revision baseRevision,
                                  @Nonnull RevisionContext context) {
         // did existence of node change after baseRevision?
-        Map<String, String> deleted = getDeleted();
-        for (Map.Entry<String, String> entry : deleted.entrySet()) {
-            if (isRevisionNewer(context, Revision.fromString(entry.getKey()), baseRevision)) {
+        Map<Revision, String> deleted = getDeleted();
+        for (Map.Entry<Revision, String> entry : deleted.entrySet()) {
+            if (isRevisionNewer(context, entry.getKey(), baseRevision)) {
                 return true;
             }
         }
 
-        for (Map.Entry<String, UpdateOp.Operation> entry : op.changes.entrySet()) {
-            if (entry.getValue().type != UpdateOp.Operation.Type.SET_MAP_ENTRY) {
+        for (Map.Entry<Key, Operation> entry : op.changes.entrySet()) {
+            if (entry.getValue().type != Operation.Type.SET_MAP_ENTRY) {
                 continue;
             }
-            int idx = entry.getKey().indexOf('.');
-            String name = entry.getKey().substring(0, idx);
+            String name = entry.getKey().getName();
             if (DELETED.equals(name)) {
                 // existence of node changed, this always conflicts with
                 // any other concurrent change
@@ -530,8 +518,8 @@ public class NodeDocument extends Docume
                 continue;
             }
             // was this property touched after baseRevision?
-            for (String rev : getValueMap(name).keySet()) {
-                if (isRevisionNewer(context, Revision.fromString(rev), baseRevision)) {
+            for (Revision rev : getValueMap(name).keySet()) {
+                if (isRevisionNewer(context, rev, baseRevision)) {
                     return true;
                 }
             }
@@ -574,11 +562,11 @@ public class NodeDocument extends Docume
             NavigableMap<Revision, String> splitMap
                     = new TreeMap<Revision, String>(context.getRevisionComparator());
             splitValues.put(property, splitMap);
-            Map<String, String> valueMap = getLocalMap(property);
+            Map<Revision, String> valueMap = getLocalMap(property);
             // collect committed changes of this cluster node after the
             // most recent previous split revision
-            for (Map.Entry<String, String> entry : valueMap.entrySet()) {
-                Revision rev = Revision.fromString(entry.getKey());
+            for (Map.Entry<Revision, String> entry : valueMap.entrySet()) {
+                Revision rev = entry.getKey();
                 if (rev.getClusterId() != context.getClusterId()) {
                     continue;
                 }
@@ -617,13 +605,13 @@ public class NodeDocument extends Docume
             splitOps = new ArrayList<UpdateOp>(2);
             // move to another document
             UpdateOp main = new UpdateOp(id, false);
-            main.setMapEntry(PREVIOUS, high.toString(), low.toString());
+            main.setMapEntry(PREVIOUS, high, low.toString());
             UpdateOp old = new UpdateOp(Utils.getPreviousIdFor(id, high), true);
-            old.set(ID, old.getKey());
+            old.set(ID, old.getId());
             for (String property : splitValues.keySet()) {
                 NavigableMap<Revision, String> splitMap = splitValues.get(property);
                 for (Map.Entry<Revision, String> entry : splitMap.entrySet()) {
-                    String r = entry.getKey().toString();
+                    Revision r = entry.getKey();
                     main.removeMapEntry(property, r);
                     old.setMapEntry(property, r, entry.getValue());
                 }
@@ -643,7 +631,7 @@ public class NodeDocument extends Docume
     @Nonnull
     SortedMap<Revision, Range> getPreviousRanges() {
         if (previous == null) {
-            Map<String, String> map = getLocalMap(PREVIOUS);
+            Map<Revision, String> map = getLocalMap(PREVIOUS);
             if (map.isEmpty()) {
                 previous = EMPTY_RANGE_MAP;
             } else {
@@ -661,8 +649,8 @@ public class NodeDocument extends Docume
                                 return c;
                             }
                         });
-                for (Map.Entry<String, String> entry : map.entrySet()) {
-                    Revision high = Revision.fromString(entry.getKey());
+                for (Map.Entry<Revision, String> entry : map.entrySet()) {
+                    Revision high = entry.getKey();
                     Revision low = Revision.fromString(entry.getValue());
                     transformed.put(high, new Range(high, low));
                 }
@@ -676,7 +664,8 @@ public class NodeDocument extends Docume
      * Returns previous {@link NodeDocument}, which include entries for the
      * property in the given revision.
      * If the <code>revision</code> is <code>null</code>, then all previous
-     * documents are returned.
+     * documents are returned. The returned documents are returned in descending
+     * revision order (newest first).
      *
      * @param revision the revision to match or <code>null</code>.
      * @param property the name of a property.
@@ -714,7 +703,7 @@ public class NodeDocument extends Docume
                     return false;
                 }
                 return revision == null
-                        || input.getLocalMap(property).containsKey(revision.toString());
+                        || input.getLocalMap(property).containsKey(revision);
             }
         });
     }
@@ -727,11 +716,11 @@ public class NodeDocument extends Docume
      * @return local value map.
      */
     @Nonnull
-    Map<String, String> getLocalMap(String key) {
+    SortedMap<Revision, String> getLocalMap(String key) {
         @SuppressWarnings("unchecked")
-        Map<String, String> map = (Map<String, String>) super.get(key);
+        SortedMap<Revision, String> map = (SortedMap<Revision, String>) get(key);
         if (map == null) {
-            map = Collections.emptyMap();
+            map = ValueMap.EMPTY;
         }
         return map;
     }
@@ -740,17 +729,17 @@ public class NodeDocument extends Docume
      * @return the {@link #REVISIONS} stored on this document.
      */
     @Nonnull
-    Map<String, String> getLocalRevisions() {
+    SortedMap<Revision, String> getLocalRevisions() {
         return getLocalMap(REVISIONS);
     }
 
     @Nonnull
-    Map<String, String> getLocalCommitRoot() {
+    SortedMap<Revision, String> getLocalCommitRoot() {
         return getLocalMap(COMMIT_ROOT);
     }
 
     @Nonnull
-    Map<String, String> getLocalDeleted() {
+    SortedMap<Revision, String> getLocalDeleted() {
         return getLocalMap(DELETED);
     }
 
@@ -765,33 +754,33 @@ public class NodeDocument extends Docume
                                    @Nonnull Revision revision,
                                    @Nonnull String commitValue) {
         checkNotNull(op).setMapEntry(REVISIONS,
-                checkNotNull(revision).toString(), checkNotNull(commitValue));
+                checkNotNull(revision), checkNotNull(commitValue));
     }
 
     public static void unsetRevision(@Nonnull UpdateOp op,
                                      @Nonnull Revision revision) {
-        checkNotNull(op).unsetMapEntry(REVISIONS,
-                checkNotNull(revision).toString());
+        checkNotNull(op).unsetMapEntry(REVISIONS, checkNotNull(revision));
     }
 
     public static void setLastRev(@Nonnull UpdateOp op,
                                   @Nonnull Revision revision) {
         checkNotNull(op).setMapEntry(LAST_REV,
-                String.valueOf(checkNotNull(revision).getClusterId()),
+                new Revision(0, 0, revision.getClusterId()),
                 revision.toString());
     }
 
     public static void setCommitRoot(@Nonnull UpdateOp op,
                                      @Nonnull Revision revision,
                                      int commitRootDepth) {
-        checkNotNull(op).setMapEntry(COMMIT_ROOT,
-                checkNotNull(revision).toString(), String.valueOf(commitRootDepth));
+        checkNotNull(op).setMapEntry(COMMIT_ROOT, checkNotNull(revision),
+                String.valueOf(commitRootDepth));
     }
 
     public static void setDeleted(@Nonnull UpdateOp op,
                                   @Nonnull Revision revision,
                                   boolean deleted) {
-        checkNotNull(op).setMapEntry(DELETED, checkNotNull(revision).toString(), String.valueOf(deleted));
+        checkNotNull(op).setMapEntry(DELETED, checkNotNull(revision),
+                String.valueOf(deleted));
     }
 
     static final class Children implements CacheValue {
@@ -823,7 +812,7 @@ public class NodeDocument extends Docume
         if (containsRevision(rev)) {
             return this;
         }
-        String commitRootPath = getCommitRootPath(rev.toString());
+        String commitRootPath = getCommitRootPath(rev);
         if (commitRootPath == null) {
             // shouldn't happen, either node is commit root for a revision
             // or has a reference to the commit root
@@ -903,12 +892,11 @@ public class NodeDocument extends Docume
      */
     @CheckForNull
     private String getCommitValue(Revision revision) {
-        String r = revision.toString();
-        String value = getLocalRevisions().get(r);
+        String value = getLocalRevisions().get(revision);
         if (value == null) {
             // check previous
             for (NodeDocument prev : getPreviousDocs(revision, REVISIONS)) {
-                value = prev.getLocalRevisions().get(r);
+                value = prev.getLocalRevisions().get(revision);
                 if (value != null) {
                     break;
                 }
@@ -948,7 +936,7 @@ public class NodeDocument extends Docume
      * Get the latest property value that is larger or equal the min revision,
      * and smaller or equal the readRevision revision.
      *
-     * @param valueMap the revision-value map
+     * @param valueMap the sorted revision-value map
      * @param min the minimum revision (null meaning unlimited)
      * @param readRevision the maximum revision
      * @param validRevisions set of revision considered valid against the given
@@ -957,35 +945,33 @@ public class NodeDocument extends Docume
      */
     @CheckForNull
     private Value getLatestValue(@Nonnull RevisionContext context,
-                                 @Nonnull Map<String, String> valueMap,
+                                 @Nonnull Map<Revision, String> valueMap,
                                  @Nullable Revision min,
                                  @Nonnull Revision readRevision,
                                  @Nonnull Set<Revision> validRevisions) {
         String value = null;
         Revision latestRev = null;
-        for (Map.Entry<String, String> entry : valueMap.entrySet()) {
-            Revision propRev = Revision.fromString(entry.getKey());
+        for (Map.Entry<Revision, String> entry : valueMap.entrySet()) {
+            Revision propRev = entry.getKey();
             if (min != null && isRevisionNewer(context, min, propRev)) {
                 continue;
             }
-            if (latestRev != null && !isRevisionNewer(context, propRev, latestRev)) {
-                continue;
-            }
             if (isValidRevision(context, propRev, readRevision, validRevisions)) {
                 latestRev = propRev;
                 value = entry.getValue();
+                break;
             }
         }
         return value != null ? new Value(value, latestRev) : null;
     }
 
     @Nonnull
-    private Map<String, String> getDeleted() {
+    private Map<Revision, String> getDeleted() {
         return ValueMap.create(this, DELETED);
     }
 
     @Nonnull
-    private Map<String, String> getCommitRoot() {
+    private Map<Revision, String> getCommitRoot() {
         return ValueMap.create(this, COMMIT_ROOT);
     }
 

Added: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/StableRevisionComparator.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/StableRevisionComparator.java?rev=1525856&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/StableRevisionComparator.java (added)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/StableRevisionComparator.java Tue Sep 24 11:21:03 2013
@@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.oak.plugins.mongomk;
+
+import java.util.Comparator;
+
+/**
+ * <code>StableRevisionComparator</code> implements a revision comparator, which
+ * is only based on stable information available in the two revisions presented
+ * to this comparator. This is different from {@link Revision.RevisionComparator},
+ * which also takes the time into account when a foreign revision (from another
+ * cluster nodes) was first seen. This class is used in sorted collections where
+ * revision keys must have a stable ordering independent from the time when
+ * a revision was seen.
+ * <p>
+ * Revisions are first ordered by timestamp, then counter and finally cluster
+ * node id.
+ */
+class StableRevisionComparator implements Comparator<Revision> {
+
+    @Override
+    public int compare(Revision o1, Revision o2) {
+        int comp = o1.compareRevisionTime(o2);
+        if (comp != 0) {
+            return comp;
+        }
+        return Integer.signum(o1.getClusterId() - o2.getClusterId());
+    }
+}

Propchange: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/StableRevisionComparator.java
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/StableRevisionComparator.java
------------------------------------------------------------------------------
    svn:keywords = Author Date Id Revision Rev URL

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/UpdateOp.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/UpdateOp.java?rev=1525856&r1=1525855&r2=1525856&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/UpdateOp.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/UpdateOp.java Tue Sep 24 11:21:03 2013
@@ -16,36 +16,42 @@
  */
 package org.apache.jackrabbit.oak.plugins.mongomk;
 
+import java.util.HashMap;
 import java.util.Map;
 import java.util.Map.Entry;
-import java.util.TreeMap;
+
+import javax.annotation.CheckForNull;
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+
+import static com.google.common.base.Preconditions.checkNotNull;
 
 /**
  * A MongoDB "update" operation for one document.
  */
 public class UpdateOp {
 
-    final String key;
+    final String id;
     
     final boolean isNew;
     boolean isDelete;
     
-    final Map<String, Operation> changes = new TreeMap<String, Operation>();
+    final Map<Key, Operation> changes = new HashMap<Key, Operation>();
     
     /**
-     * Create an update operation for the given document. The commit root is assumed
-     * to be the path, unless this is changed later on.
+     * Create an update operation for the document with the given id. The commit
+     * root is assumed to be the path, unless this is changed later on.
      *
-     * @param key the primary key
+     * @param id the primary key
      * @param isNew whether this is a new document
      */
-    UpdateOp(String key, boolean isNew) {
-        this.key = key;
+    UpdateOp(String id, boolean isNew) {
+        this.id = id;
         this.isNew = isNew;
     }
     
-    String getKey() {
-        return key;
+    String getId() {
+        return id;
     }
     
     boolean isNew() {
@@ -58,48 +64,33 @@ public class UpdateOp {
     
     /**
      * Add a new or update an existing map entry.
-     * The property is a map of sub-names / values.
+     * The property is a map of revisions / values.
      * 
      * @param property the property
-     * @param subName the entry name
+     * @param revision the revision
      * @param value the value
      */
-    void setMapEntry(String property, String subName, Object value) {
+    void setMapEntry(@Nonnull String property, @Nonnull Revision revision, Object value) {
         Operation op = new Operation();
         op.type = Operation.Type.SET_MAP_ENTRY;
         op.value = value;
-        changes.put(property + "." + subName, op);
+        changes.put(new Key(property, checkNotNull(revision)), op);
     }
     
     /**
      * Remove a map entry.
-     * The property is a map of sub-names / values.
+     * The property is a map of revisions / values.
      * 
      * @param property the property
-     * @param subName the entry name
+     * @param revision the revision
      */
-    public void removeMapEntry(String property, String subName) {
+    public void removeMapEntry(@Nonnull String property, @Nonnull Revision revision) {
         Operation op = new Operation();
         op.type = Operation.Type.REMOVE_MAP_ENTRY;
-        changes.put(property + "." + subName, op);
+        changes.put(new Key(property, checkNotNull(revision)), op);
     }
     
     /**
-     * Set a map to a single key-value pair.
-     * The property is a map of sub-names / values.
-     * 
-     * @param property the property
-     * @param subName the entry name
-     * @param value the value
-     */
-    public void setMap(String property, String subName, Object value) {
-        Operation op = new Operation();
-        op.type = Operation.Type.SET_MAP;
-        op.value = value;
-        changes.put(property + "." + subName, op);
-    }
-
-    /**
      * Set the property to the given value.
      * 
      * @param property the property name
@@ -109,27 +100,18 @@ public class UpdateOp {
         Operation op = new Operation();
         op.type = Operation.Type.SET;
         op.value = value;
-        changes.put(property, op);
-    }
-    
-    /**
-     * Do not set the property (after it has been set).
-     * 
-     * @param property the property name
-     */
-    void unset(String property) {
-        changes.remove(property);
+        changes.put(new Key(property, null), op);
     }
     
     /**
      * Do not set the property entry (after it has been set).
-     * The property is a map of sub-names / values.
+     * The property is a map of revisions / values.
      * 
      * @param property the property name
-     * @param subName the entry name
+     * @param revision the revision
      */
-    void unsetMapEntry(String property, String subName) {
-        changes.remove(property + "." + subName);
+    void unsetMapEntry(@Nonnull String property, @Nonnull Revision revision) {
+        changes.remove(new Key(property, checkNotNull(revision)));
     }
 
     /**
@@ -137,16 +119,18 @@ public class UpdateOp {
      * method can be used to make a conditional update.
      *
      * @param property the property name
-     * @param subName the entry name
+     * @param revision the revision
      */
-    void containsMapEntry(String property, String subName, boolean exists) {
+    void containsMapEntry(@Nonnull String property,
+                          @Nonnull Revision revision,
+                          boolean exists) {
         if (isNew) {
             throw new IllegalStateException("Cannot use containsMapEntry() on new document");
         }
         Operation op = new Operation();
         op.type = Operation.Type.CONTAINS_MAP_ENTRY;
         op.value = exists;
-        changes.put(property + "." + subName, op);
+        changes.put(new Key(property, checkNotNull(revision)), op);
     }
 
     /**
@@ -155,27 +139,16 @@ public class UpdateOp {
      * @param property the key
      * @param value the increment
      */
-    void increment(String property, long value) {
+    void increment(@Nonnull String property, long value) {
         Operation op = new Operation();
         op.type = Operation.Type.INCREMENT;
         op.value = value;
-        changes.put(property, op);
-    }
-    
-    public Long getIncrement(String property) {
-        Operation op = changes.get(property);
-        if (op == null) {
-            return null;
-        }
-        if (op.type != Operation.Type.INCREMENT) {
-            throw new IllegalArgumentException("Not an increment operation");
-        }
-        return (Long) op.value;
+        changes.put(new Key(property, null), op);
     }
     
     public UpdateOp getReverseOperation() {
-        UpdateOp reverse = new UpdateOp(key, isNew);
-        for (Entry<String, Operation> e : changes.entrySet()) {
+        UpdateOp reverse = new UpdateOp(id, isNew);
+        for (Entry<Key, Operation> e : changes.entrySet()) {
             Operation r = e.getValue().getReverse();
             if (r != null) {
                 reverse.changes.put(e.getKey(), r);
@@ -186,7 +159,7 @@ public class UpdateOp {
 
     @Override
     public String toString() {
-        return "key: " + key + " " + (isNew ? "new" : "update") + " " + changes;
+        return "key: " + id + " " + (isNew ? "new" : "update") + " " + changes;
     }
     
     /**
@@ -226,14 +199,8 @@ public class UpdateOp {
             /**
              * Checks if the sub-key is present in a map or not.
              */
-            CONTAINS_MAP_ENTRY,
+            CONTAINS_MAP_ENTRY
              
-            /**
-             * Set the sub-key / value pair.
-             * The value in the stored node is a map.
-             */
-            SET_MAP,
-
          }
              
         
@@ -262,7 +229,6 @@ public class UpdateOp {
                 break;
             case SET:
             case REMOVE_MAP_ENTRY:
-            case SET_MAP:
             case CONTAINS_MAP_ENTRY:
                 // nothing to do
                 break;
@@ -276,4 +242,58 @@ public class UpdateOp {
         
     }
 
+    /**
+     * A key for an operation consists of a property name and an optional
+     * revision. The revision is only set if the value for the operation is
+     * set for a certain revision.
+     */
+    public static final class Key {
+
+        private final String name;
+        private final Revision revision;
+
+        public Key(@Nonnull String name, @Nullable Revision revision) {
+            this.name = checkNotNull(name);
+            this.revision = revision;
+        }
+
+        @Nonnull
+        public String getName() {
+            return name;
+        }
+
+        @CheckForNull
+        public Revision getRevision() {
+            return revision;
+        }
+
+        @Override
+        public String toString() {
+            String s = name;
+            if (revision != null) {
+                s += "." + revision.toString();
+            }
+            return s;
+        }
+
+        @Override
+        public int hashCode() {
+            int hash = name.hashCode();
+            if (revision != null) {
+                hash ^= revision.hashCode();
+            }
+            return hash;
+        }
+
+        @Override
+        public boolean equals(Object obj) {
+            if (obj instanceof Key) {
+                Key other = (Key) obj;
+                return name.equals(other.name) &&
+                        revision != null ? revision.equals(other.revision) : other.revision == null;
+            }
+            return false;
+        }
+    }
+
 }

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/ValueMap.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/ValueMap.java?rev=1525856&r1=1525855&r2=1525856&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/ValueMap.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/ValueMap.java Tue Sep 24 11:21:03 2013
@@ -18,12 +18,18 @@ package org.apache.jackrabbit.oak.plugin
 
 import java.util.AbstractMap;
 import java.util.AbstractSet;
+import java.util.Collections;
+import java.util.Comparator;
 import java.util.Iterator;
 import java.util.Map;
 import java.util.Set;
+import java.util.SortedMap;
+import java.util.TreeMap;
 
 import javax.annotation.Nonnull;
 
+import org.apache.jackrabbit.oak.plugins.mongomk.util.MergeSortedIterators;
+
 import com.google.common.collect.Iterators;
 
 /**
@@ -32,37 +38,40 @@ import com.google.common.collect.Iterato
  */
 class ValueMap {
 
+    static SortedMap<Revision, String> EMPTY = Collections.unmodifiableSortedMap(
+            new TreeMap<Revision, String>(new StableRevisionComparator()));
+
     @Nonnull
-    static Map<String, String> create(final @Nonnull NodeDocument doc,
-                                      final @Nonnull String property) {
-        final Map<String, String> map = doc.getLocalMap(property);
+    static Map<Revision, String> create(final @Nonnull NodeDocument doc,
+                                        final @Nonnull String property) {
+        final SortedMap<Revision, String> map = doc.getLocalMap(property);
         if (doc.getPreviousRanges().isEmpty()) {
             return map;
         }
-        final Set<Map.Entry<String, String>> values
-                = new AbstractSet<Map.Entry<String, String>>() {
+        final Comparator<? super Revision> c = map.comparator();
+        final Iterator<NodeDocument> docs = Iterators.concat(
+                Iterators.singletonIterator(doc),
+                doc.getPreviousDocs(null, property).iterator());
+        final Set<Map.Entry<Revision, String>> entrySet
+                = new AbstractSet<Map.Entry<Revision, String>>() {
 
             @Override
             @Nonnull
-            public Iterator<Map.Entry<String, String>> iterator() {
-                return Iterators.concat(map.entrySet().iterator(), Iterators.concat(new Iterator<Iterator<Map.Entry<String, String>>>() {
-                    private final Iterator<NodeDocument> previous = doc.getPreviousDocs(null, property).iterator();
-
-                    @Override
-                    public boolean hasNext() {
-                        return previous.hasNext();
-                    }
-
+            public Iterator<Map.Entry<Revision, String>> iterator() {
+                return new MergeSortedIterators<Map.Entry<Revision, String>>(
+                        new Comparator<Map.Entry<Revision, String>>() {
+                            @Override
+                            public int compare(Map.Entry<Revision, String> o1,
+                                               Map.Entry<Revision, String> o2) {
+                                return c.compare(o1.getKey(), o2.getKey());
+                            }
+                        }
+                ) {
                     @Override
-                    public Iterator<Map.Entry<String, String>> next() {
-                        return previous.next().getLocalMap(property).entrySet().iterator();
+                    public Iterator<Map.Entry<Revision, String>> nextIterator() {
+                        return docs.hasNext() ? docs.next().getLocalMap(property).entrySet().iterator() : null;
                     }
-
-                    @Override
-                    public void remove() {
-                        throw new UnsupportedOperationException();
-                    }
-                }));
+                };
             }
 
             @Override
@@ -74,14 +83,15 @@ class ValueMap {
                 return size;
             }
         };
-        return new AbstractMap<String, String>() {
 
-            private final Map<String, String> map = doc.getLocalMap(property);
+        return new AbstractMap<Revision, String>() {
+
+            private final Map<Revision, String> map = doc.getLocalMap(property);
 
             @Override
             @Nonnull
-            public Set<Entry<String, String>> entrySet() {
-                return values;
+            public Set<Entry<Revision, String>> entrySet() {
+                return entrySet;
             }
 
             @Override
@@ -91,7 +101,7 @@ class ValueMap {
                 if (value != null) {
                     return value;
                 }
-                Revision r = Revision.fromString(key.toString());
+                Revision r = (Revision) key;
                 for (NodeDocument prev : doc.getPreviousDocs(r, property)) {
                     value = prev.getLocalMap(property).get(key);
                     if (value != null) {

Added: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/util/MergeSortedIterators.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/util/MergeSortedIterators.java?rev=1525856&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/util/MergeSortedIterators.java (added)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/util/MergeSortedIterators.java Tue Sep 24 11:21:03 2013
@@ -0,0 +1,127 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.oak.plugins.mongomk.util;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.Iterator;
+import java.util.List;
+import java.util.NoSuchElementException;
+
+import com.google.common.collect.Iterators;
+import com.google.common.collect.PeekingIterator;
+
+/**
+ * <code>MergeSortedIterators</code> is a specialized implementation of a
+ * merge sort of already sorted iterators of some type of comparable elements.
+ * The input iterators must return the elements in sorted order according to
+ * the provided Comparator. In addition the sequence of iterators must also
+ * be sorted in a way that the first element of the next iterator is greater
+ * than the first element of the previous iterator.
+ */
+public abstract class MergeSortedIterators<T> implements Iterator<T> {
+
+    private final List<PeekingIterator<T>> iterators = new ArrayList<PeekingIterator<T>>();
+    private final Comparator<T> comparator;
+    private T lastPeek;
+
+    public MergeSortedIterators(final Comparator<T> comparator) {
+        this.comparator = comparator;
+        fetchNextIterator();
+    }
+
+    /**
+     * @return the next {@link Iterator} or <code>null</code> if there is none.
+     */
+    public abstract Iterator<T> nextIterator();
+
+    //----------------------------< Iterator >----------------------------------
+
+    @Override
+    public boolean hasNext() {
+        return !iterators.isEmpty();
+    }
+
+    @Override
+    public T next() {
+        if (!hasNext()) {
+            throw new NoSuchElementException();
+        }
+        PeekingIterator<T> it = iterators.get(0);
+        T next = it.next();
+        // more elements?
+        if (it.hasNext()) {
+            adjustFirst();
+        } else {
+            // remove from list of iterators
+            iterators.remove(0);
+        }
+        // fetch next iterator?
+        if (comparator.compare(next, lastPeek) >= 0) {
+            fetchNextIterator();
+        }
+        return next;
+    }
+
+    @Override
+    public void remove() {
+        throw new UnsupportedOperationException();
+    }
+
+    //----------------------------< internal >----------------------------------
+
+    private void fetchNextIterator() {
+        Iterator<T> it = nextIterator();
+        if (it != null && it.hasNext()) {
+            PeekingIterator<T> pIt = Iterators.peekingIterator(it);
+            if (!iterators.isEmpty()
+                    &&  comparator.compare(pIt.peek(), lastPeek) < 0) {
+                throw new IllegalStateException("First element of next iterator must be greater than previous iterator");
+            }
+            lastPeek = pIt.peek();
+            iterators.add(pIt);
+            adjustLast();
+        }
+    }
+
+    private void adjustFirst() {
+        // shift first iterator until peeked elements are sorted again
+        int i = 0;
+        while (i + 1 < iterators.size()) {
+            if (comparator.compare(iterators.get(i).peek(), iterators.get(i + 1).peek()) > 0) {
+                Collections.swap(iterators, i, i + 1);
+                i++;
+            } else {
+                break;
+            }
+        }
+    }
+
+    private void adjustLast() {
+        // shift last until sorted again
+        int i = iterators.size() - 1;
+        while (i - 1 >= 0) {
+            if (comparator.compare(iterators.get(i - 1).peek(), iterators.get(i).peek()) > 0) {
+                Collections.swap(iterators, i, i - 1);
+                i--;
+            } else {
+                break;
+            }
+        }
+    }
+}

Propchange: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/util/MergeSortedIterators.java
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/util/MergeSortedIterators.java
------------------------------------------------------------------------------
    svn:keywords = Author Date Id Revision Rev URL

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/mongomk/DocumentSplitTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/mongomk/DocumentSplitTest.java?rev=1525856&r1=1525855&r2=1525856&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/mongomk/DocumentSplitTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/mongomk/DocumentSplitTest.java Tue Sep 24 11:21:03 2013
@@ -37,25 +37,24 @@ public class DocumentSplitTest extends B
     @Test
     public void splitRevisions() throws Exception {
         DocumentStore store = mk.getDocumentStore();
-        Set<String> revisions = Sets.newHashSet();
+        Set<Revision> revisions = Sets.newHashSet();
         NodeDocument doc = store.find(Collection.NODES, Utils.getIdFromPath("/"));
         assertNotNull(doc);
         revisions.addAll(doc.getLocalRevisions().keySet());
-        revisions.add(mk.commit("/", "+\"foo\":{}+\"bar\":{}", null, null));
+        revisions.add(Revision.fromString(mk.commit("/", "+\"foo\":{}+\"bar\":{}", null, null)));
         // create nodes
         while (revisions.size() <= NodeDocument.REVISIONS_SPLIT_OFF_SIZE) {
-            revisions.add(mk.commit("/", "+\"foo/node-" + revisions.size() + "\":{}" +
-                    "+\"bar/node-" + revisions.size() + "\":{}", null, null));
+            revisions.add(Revision.fromString(mk.commit("/", "+\"foo/node-" + revisions.size() + "\":{}" +
+                    "+\"bar/node-" + revisions.size() + "\":{}", null, null)));
         }
         mk.runBackgroundOperations();
         String head = mk.getHeadRevision();
         doc = store.find(Collection.NODES, Utils.getIdFromPath("/"));
         assertNotNull(doc);
-        Map<String, String> revs = doc.getLocalRevisions();
+        Map<Revision, String> revs = doc.getLocalRevisions();
         // one remaining in the local revisions map
         assertEquals(1, revs.size());
-        for (String r : revisions) {
-            Revision rev = Revision.fromString(r);
+        for (Revision rev : revisions) {
             assertTrue(doc.containsRevision(rev));
             assertTrue(doc.isCommitted(rev));
         }
@@ -69,7 +68,7 @@ public class DocumentSplitTest extends B
     @Test
     public void splitDeleted() throws Exception {
         DocumentStore store = mk.getDocumentStore();
-        Set<String> revisions = Sets.newHashSet();
+        Set<Revision> revisions = Sets.newHashSet();
         mk.commit("/", "+\"foo\":{}", null, null);
         NodeDocument doc = store.find(Collection.NODES, Utils.getIdFromPath("/foo"));
         assertNotNull(doc);
@@ -77,9 +76,9 @@ public class DocumentSplitTest extends B
         boolean create = false;
         while (revisions.size() <= NodeDocument.REVISIONS_SPLIT_OFF_SIZE) {
             if (create) {
-                revisions.add(mk.commit("/", "+\"foo\":{}", null, null));
+                revisions.add(Revision.fromString(mk.commit("/", "+\"foo\":{}", null, null)));
             } else {
-                revisions.add(mk.commit("/", "-\"foo\"", null, null));
+                revisions.add(Revision.fromString(mk.commit("/", "-\"foo\"", null, null)));
             }
             create = !create;
         }
@@ -87,11 +86,10 @@ public class DocumentSplitTest extends B
         String head = mk.getHeadRevision();
         doc = store.find(Collection.NODES, Utils.getIdFromPath("/foo"));
         assertNotNull(doc);
-        Map<String, String> deleted = doc.getLocalDeleted();
+        Map<Revision, String> deleted = doc.getLocalDeleted();
         // one remaining in the local deleted map
         assertEquals(1, deleted.size());
-        for (String r : revisions) {
-            Revision rev = Revision.fromString(r);
+        for (Revision rev : revisions) {
             assertTrue(doc.containsRevision(rev));
             assertTrue(doc.isCommitted(rev));
         }
@@ -110,21 +108,20 @@ public class DocumentSplitTest extends B
         mk.commit("/", "+\"foo\":{}+\"bar\":{}", null, null);
         NodeDocument doc = store.find(Collection.NODES, Utils.getIdFromPath("/foo"));
         assertNotNull(doc);
-        Set<String> commitRoots = Sets.newHashSet();
+        Set<Revision> commitRoots = Sets.newHashSet();
         commitRoots.addAll(doc.getLocalCommitRoot().keySet());
         // create nodes
         while (commitRoots.size() <= NodeDocument.REVISIONS_SPLIT_OFF_SIZE) {
-            commitRoots.add(mk.commit("/", "^\"foo/prop\":" + commitRoots.size() +
-                    "^\"bar/prop\":" + commitRoots.size(), null, null));
+            commitRoots.add(Revision.fromString(mk.commit("/", "^\"foo/prop\":" +
+                    commitRoots.size() + "^\"bar/prop\":" + commitRoots.size(), null, null)));
         }
         mk.runBackgroundOperations();
         doc = store.find(Collection.NODES, Utils.getIdFromPath("/foo"));
         assertNotNull(doc);
-        Map<String, String> commits = doc.getLocalCommitRoot();
+        Map<Revision, String> commits = doc.getLocalCommitRoot();
         // one remaining in the local commit root map
         assertEquals(1, commits.size());
-        for (String r : commitRoots) {
-            Revision rev = Revision.fromString(r);
+        for (Revision rev : commitRoots) {
             assertTrue(doc.isCommitted(rev));
         }
     }
@@ -135,24 +132,23 @@ public class DocumentSplitTest extends B
         mk.commit("/", "+\"foo\":{}", null, null);
         NodeDocument doc = store.find(Collection.NODES, Utils.getIdFromPath("/foo"));
         assertNotNull(doc);
-        Set<String> revisions = Sets.newHashSet();
+        Set<Revision> revisions = Sets.newHashSet();
         // create nodes
         while (revisions.size() <= NodeDocument.REVISIONS_SPLIT_OFF_SIZE) {
-            revisions.add(mk.commit("/", "^\"foo/prop\":" + revisions.size(), null, null));
+            revisions.add(Revision.fromString(mk.commit("/", "^\"foo/prop\":" +
+                    revisions.size(), null, null)));
         }
         mk.runBackgroundOperations();
         doc = store.find(Collection.NODES, Utils.getIdFromPath("/foo"));
         assertNotNull(doc);
-        Map<String, String> localRevs = doc.getLocalRevisions();
+        Map<Revision, String> localRevs = doc.getLocalRevisions();
         // one remaining in the local revisions map
         assertEquals(1, localRevs.size());
-        for (String r : revisions) {
-            Revision rev = Revision.fromString(r);
+        for (Revision rev : revisions) {
             assertTrue(doc.isCommitted(rev));
         }
         // all revisions in the prop map
-        Map<String, String> valueMap = doc.getValueMap("prop");
-        assertNotNull(valueMap);
+        Map<Revision, String> valueMap = doc.getValueMap("prop");
         assertEquals((long) revisions.size(), valueMap.size());
         // one remaining revision in the local map
         valueMap = doc.getLocalMap("prop");

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/mongomk/MongoDocumentStoreTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/mongomk/MongoDocumentStoreTest.java?rev=1525856&r1=1525855&r2=1525856&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/mongomk/MongoDocumentStoreTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/mongomk/MongoDocumentStoreTest.java Tue Sep 24 11:21:03 2013
@@ -78,14 +78,17 @@ public class MongoDocumentStoreTest {
         DocumentStore docStore = openDocumentStore();
 
         UpdateOp updateOp = new UpdateOp("/", true);
-        updateOp.setMapEntry("property1", "key1", "value1");
+        Revision r1 = new Revision(0, 0, 0);
+        updateOp.setMapEntry("property1", r1, "value1");
         updateOp.increment("property2", 1);
         updateOp.set("property3", "value3");
         docStore.createOrUpdate(Collection.NODES, updateOp);
         NodeDocument doc = docStore.find(Collection.NODES, "/");
+        assertNotNull(doc);
 
         Map<?, ?> property1 = (Map<?, ?>) doc.get("property1");
-        String value1 = (String) property1.get("key1");
+        assertNotNull(property1);
+        String value1 = (String) property1.get(r1);
         assertEquals("value1", value1);
 
         Long value2 = (Long) doc.get("property2");
@@ -102,13 +105,14 @@ public class MongoDocumentStoreTest {
     @Test
     public void batchAdd() throws Exception {
         DocumentStore docStore = openDocumentStore();
+        Revision r1 = new Revision(0, 0, 0);
         int nUpdates = 10;
         List<UpdateOp> updateOps = new ArrayList<UpdateOp>();
         for (int i = 0; i < nUpdates; i++) {
             String path = "/node" + i;
             UpdateOp updateOp = new UpdateOp(path, true);
             updateOp.set(Document.ID, "/node" + i);
-            updateOp.setMapEntry("property1", "key1", "value1");
+            updateOp.setMapEntry("property1", r1, "value1");
             updateOp.increment("property2", 1);
             updateOp.set("property3", "value3");
             updateOps.add(updateOp);
@@ -143,36 +147,40 @@ public class MongoDocumentStoreTest {
 
     @Test
     public void containsMapEntry() {
+        Revision r = new Revision(0, 0, 0);
+        Revision unknown = new Revision(0, 1, 0);
         DocumentStore docStore = openDocumentStore();
         UpdateOp op = new UpdateOp("/node", true);
-        op.setMapEntry("map", "key", "value");
+        op.setMapEntry("map", r, "value");
         docStore.createOrUpdate(Collection.NODES, op);
 
         op = new UpdateOp("/node", false);
         op.set("prop", "value");
-        op.containsMapEntry("map", "unknown-key", true);
+        op.containsMapEntry("map", unknown, true);
         // update if unknown-key exists -> must not succeed
         assertNull(docStore.findAndUpdate(Collection.NODES, op));
 
         op = new UpdateOp("/node", false);
         op.set("prop", "value");
-        op.containsMapEntry("map", "key", true);
+        op.containsMapEntry("map", r, true);
         // update if key exists -> must succeed
         NodeDocument doc = docStore.findAndUpdate(Collection.NODES, op);
         assertNotNull(doc);
 
         doc = docStore.find(Collection.NODES, "/node");
+        assertNotNull(doc);
         assertNotNull(doc.get("prop"));
         assertEquals("value", doc.get("prop"));
 
         op = new UpdateOp("/node", false);
         op.set("prop", "other");
-        op.containsMapEntry("map", "key", false);
+        op.containsMapEntry("map", r, false);
         // update if key does not exist -> must not succeed
         assertNull(docStore.findAndUpdate(Collection.NODES, op));
 
         // value must still be the same
         doc = docStore.find(Collection.NODES, "/node");
+        assertNotNull(doc);
         assertNotNull(doc.get("prop"));
         assertEquals("value", doc.get("prop"));
     }
@@ -264,20 +272,22 @@ public class MongoDocumentStoreTest {
         }
 
         private void addNodes() {
+            Revision r1 = new Revision(0, 0, 0);
             for (int i = 0; i < nNodes; i++) {
                 String path = "/" + nodeName + i;
                 UpdateOp updateOp = new UpdateOp(path, true);
-                updateOp.setMapEntry("property1", "key1", "value1");
+                updateOp.setMapEntry("property1", r1, "value1");
                 updateOp.set("property3", "value3");
                 docStore.createOrUpdate(Collection.NODES, updateOp);
             }
         }
 
         private void updateNodes() {
+            Revision r2 = new Revision(0, 1, 0);
             for (int i = 0; i < nNodes; i++) {
                 String path = "/" + nodeName + i;
                 UpdateOp updateOp = new UpdateOp(path, false);
-                updateOp.setMapEntry("property1", "key2", "value2");
+                updateOp.setMapEntry("property1", r2, "value2");
                 updateOp.set("property4", "value4");
                 docStore.createOrUpdate(Collection.NODES, updateOp);
             }