jackrabbit-oak-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From mreut...@apache.org
Subject svn commit: r1468326 - in /jackrabbit/oak/trunk/oak-mongomk/src: main/java/org/apache/jackrabbit/mongomk/ test/java/org/apache/jackrabbit/mongomk/
Date Tue, 16 Apr 2013 08:20:12 GMT
Author: mreutegg
Date: Tue Apr 16 08:20:11 2013
New Revision: 1468326

URL: http://svn.apache.org/r1468326
Log:
OAK-619 Lock-free MongoMK implementation
- Improved conflict handling with tests (one still failing and ignored)

Added:
    jackrabbit/oak/trunk/oak-mongomk/src/test/java/org/apache/jackrabbit/mongomk/ConflictTest.java
  (with props)
Modified:
    jackrabbit/oak/trunk/oak-mongomk/src/main/java/org/apache/jackrabbit/mongomk/Commit.java
    jackrabbit/oak/trunk/oak-mongomk/src/main/java/org/apache/jackrabbit/mongomk/MongoMK.java
    jackrabbit/oak/trunk/oak-mongomk/src/test/java/org/apache/jackrabbit/mongomk/MongoMKCommitAddTest.java

Modified: jackrabbit/oak/trunk/oak-mongomk/src/main/java/org/apache/jackrabbit/mongomk/Commit.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-mongomk/src/main/java/org/apache/jackrabbit/mongomk/Commit.java?rev=1468326&r1=1468325&r2=1468326&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-mongomk/src/main/java/org/apache/jackrabbit/mongomk/Commit.java
(original)
+++ jackrabbit/oak/trunk/oak-mongomk/src/main/java/org/apache/jackrabbit/mongomk/Commit.java
Tue Apr 16 08:20:11 2013
@@ -272,7 +272,8 @@ public class Commit {
                             op.path + " was already added in revision " + 
                             newestRev + "; before " + revision + "; document " + map);
                 }
-                if (mk.isRevisionNewer(newestRev, baseRevision)) {
+                if (mk.isRevisionNewer(newestRev, baseRevision)
+                        && isConflicting(map, op)) {
                     throw new MicroKernelException("The node " + 
                             op.path + " was changed in revision " + 
                             newestRev + 
@@ -302,7 +303,49 @@ public class Commit {
             }
         }
     }
-    
+
+    /**
+     * Checks whether the given <code>UpdateOp</code> conflicts with the
+     * existing content in <code>nodeMap</code>. The check is done based on the
+     * {@link #baseRevision} of this commit. An <code>UpdateOp</code> conflicts
+     * when there were changes after {@link #baseRevision} on properties also
+     * contained in <code>UpdateOp</code>.
+     *
+     * @param nodeMap the contents of the nodes before the update.
+     * @param op the update to perform.
+     * @return <code>true</code> if the update conflicts; <code>false</code>
+     *         otherwise.
+     */
+    private boolean isConflicting(Map<String, Object> nodeMap,
+                                  UpdateOp op) {
+        if (baseRevision == null) {
+            // no conflict is possible when there is no baseRevision
+            return false;
+        }
+        for (Map.Entry<String, UpdateOp.Operation> entry : op.changes.entrySet()) {
+            if (entry.getValue().type != UpdateOp.Operation.Type.SET_MAP_ENTRY) {
+                continue;
+            }
+            int idx = entry.getKey().indexOf('.');
+            String name = entry.getKey().substring(0, idx);
+            if (!Utils.isPropertyName(name)) {
+                continue;
+            }
+            // was this property touched after baseRevision?
+            @SuppressWarnings("unchecked")
+            Map<String, Object> changes = (Map<String, Object>) nodeMap.get(name);
+            if (changes == null) {
+                continue;
+            }
+            for (String rev : changes.keySet()) {
+                if (mk.isRevisionNewer(Revision.fromString(rev), baseRevision)) {
+                    return true;
+                }
+            }
+        }
+        return false;
+    }
+
     private UpdateOp[] splitDocument(Map<String, Object> map) {
         String id = (String) map.get(UpdateOp.ID);
         String path = Utils.getPathFromId(id);

Modified: jackrabbit/oak/trunk/oak-mongomk/src/main/java/org/apache/jackrabbit/mongomk/MongoMK.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-mongomk/src/main/java/org/apache/jackrabbit/mongomk/MongoMK.java?rev=1468326&r1=1468325&r2=1468326&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-mongomk/src/main/java/org/apache/jackrabbit/mongomk/MongoMK.java
(original)
+++ jackrabbit/oak/trunk/oak-mongomk/src/main/java/org/apache/jackrabbit/mongomk/MongoMK.java
Tue Apr 16 08:20:11 2013
@@ -51,6 +51,7 @@ import org.apache.jackrabbit.oak.commons
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.google.common.collect.Maps;
 import com.mongodb.DB;
 
 /**
@@ -1056,15 +1057,19 @@ public class MongoMK implements MicroKer
         if (nodeMap == null) {
             return null;
         }
+        Map<String, String> revisions = Maps.newHashMap();
+        if (nodeMap.containsKey(UpdateOp.REVISIONS)) {
+            //noinspection unchecked
+            revisions.putAll((Map<String, String>) nodeMap.get(UpdateOp.REVISIONS));
+        }
         @SuppressWarnings("unchecked")
-        Map<String, String> valueMap = (Map<String, String>) nodeMap
+        Map<String, String> deletedMap = (Map<String, String>) nodeMap
                 .get(UpdateOp.DELETED);
-        if (valueMap == null) {
-            return null;
+        if (deletedMap != null) {
+            revisions.putAll(deletedMap);
         }
         Revision newestRev = null;
-        String newestValue = null;
-        for (String r : valueMap.keySet()) {
+        for (String r : revisions.keySet()) {
             Revision propRev = Revision.fromString(r);
             if (newestRev == null || isRevisionNewer(propRev, newestRev)) {
                 if (isRevisionNewer(before, propRev)) {
@@ -1072,15 +1077,20 @@ public class MongoMK implements MicroKer
                             || isValidRevision(propRev, before,
                                 nodeMap, new HashSet<Revision>())) {
                         newestRev = propRev;
-                        newestValue = valueMap.get(r);
                     }
                 }
             }
         }
-        if ("true".equals(newestValue)) {
-            // deleted in the newest revision
+        if (newestRev == null) {
             return null;
         }
+        if (deletedMap != null) {
+            String value = deletedMap.get(newestRev.toString());
+            if ("true".equals(value)) {
+                // deleted in the newest revision
+                return null;
+            }
+        }
         return newestRev;
     }
     

Added: jackrabbit/oak/trunk/oak-mongomk/src/test/java/org/apache/jackrabbit/mongomk/ConflictTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-mongomk/src/test/java/org/apache/jackrabbit/mongomk/ConflictTest.java?rev=1468326&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-mongomk/src/test/java/org/apache/jackrabbit/mongomk/ConflictTest.java
(added)
+++ jackrabbit/oak/trunk/oak-mongomk/src/test/java/org/apache/jackrabbit/mongomk/ConflictTest.java
Tue Apr 16 08:20:11 2013
@@ -0,0 +1,148 @@
+/*
+ * 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.mongomk;
+
+import org.apache.jackrabbit.mk.api.MicroKernelException;
+import org.junit.Ignore;
+import org.junit.Test;
+
+import static org.junit.Assert.fail;
+
+/**
+ * <code>ConflictTest</code> checks
+ * <a href="http://wiki.apache.org/jackrabbit/Conflict%20handling%20through%20rebasing%20branches">conflict
handling</a>.
+ */
+public class ConflictTest extends BaseMongoMKTest {
+
+    @Test
+    public void addExistingProperty() {
+        String rev = mk.commit("/", "+\"foo\":{}", null, null);
+        mk.commit("/foo", "^\"prop\":\"value\"", rev, null);
+
+        try {
+            mk.commit("/foo", "^\"prop\":\"value\"", rev, null);
+            fail("Must fail with conflict for addExistingProperty");
+        } catch (MicroKernelException e) {
+            // expected
+        }
+    }
+
+    @Test
+    public void removeRemovedProperty() {
+        String rev = mk.commit("/", "+\"foo\":{\"prop\":\"value\"}", null, null);
+        mk.commit("/foo", "^\"prop\":null", rev, null);
+
+        try {
+            mk.commit("/foo", "^\"prop\":null", rev, null);
+            fail("Must fail with conflict for removeRemovedProperty");
+        } catch (MicroKernelException e) {
+            // expected
+        }
+    }
+
+    @Test
+    public void removeChangedProperty() {
+        String rev = mk.commit("/", "+\"foo\":{\"prop\":\"value\"}", null, null);
+        mk.commit("/foo", "^\"prop\":\"bar\"", rev, null);
+
+        try {
+            mk.commit("/foo", "^\"prop\":null", rev, null);
+            fail("Must fail with conflict for removeChangedProperty");
+        } catch (MicroKernelException e) {
+            // expected
+        }
+    }
+
+    @Test
+    public void changeRemovedProperty() {
+        String rev = mk.commit("/", "+\"foo\":{\"prop\":\"value\"}", null, null);
+        mk.commit("/foo", "^\"prop\":null", rev, null);
+
+        try {
+            mk.commit("/foo", "^\"prop\":\"bar\"", rev, null);
+            fail("Must fail with conflict for changeRemovedProperty");
+        } catch (MicroKernelException e) {
+            // expected
+        }
+    }
+
+    @Test
+    public void changeChangedProperty() {
+        String rev = mk.commit("/", "+\"foo\":{\"prop\":\"value\"}", null, null);
+        mk.commit("/foo", "^\"prop\":\"bar\"", rev, null);
+
+        try {
+            mk.commit("/foo", "^\"prop\":\"baz\"", rev, null);
+            fail("Must fail with conflict for changeChangedProperty");
+        } catch (MicroKernelException e) {
+            // expected
+        }
+    }
+
+    @Test
+    public void addExistingNode() {
+        String rev = mk.commit("/", "+\"foo\":{}", null, null);
+        mk.commit("/foo", "+\"bar\":{}", rev, null);
+
+        try {
+            mk.commit("/foo", "+\"bar\":{}", rev, null);
+            fail("Must fail with conflict for addExistingNode");
+        } catch (MicroKernelException e) {
+            // expected
+        }
+    }
+
+    @Test
+    public void removeRemovedNode() {
+        String rev = mk.commit("/", "+\"foo\":{}", null, null);
+        mk.commit("/", "-\"foo\"", rev, null);
+
+        try {
+            mk.commit("/", "-\"foo\"", rev, null);
+            fail("Must fail with conflict for removeRemovedNode");
+        } catch (MicroKernelException e) {
+            // expected
+        }
+    }
+
+    @Test
+    @Ignore
+    public void removeChangedNode() {
+        String rev = mk.commit("/", "+\"foo\":{}", null, null);
+        mk.commit("/foo", "^\"prop\":\"value\"", rev, null);
+
+        try {
+            mk.commit("/", "-\"foo\"", rev, null);
+            fail("Must fail with conflict for removeChangedNode");
+        } catch (MicroKernelException e) {
+            // expected
+        }
+    }
+
+    @Test
+    public void changeRemovedNode() {
+        String rev = mk.commit("/", "+\"foo\":{}", null, null);
+        mk.commit("/", "-\"foo\"", rev, null);
+
+        try {
+            mk.commit("/foo", "^\"prop\":\"value\"", rev, null);
+            fail("Must fail with conflict for changeRemovedNode");
+        } catch (MicroKernelException e) {
+            // expected
+        }
+    }
+}

Propchange: jackrabbit/oak/trunk/oak-mongomk/src/test/java/org/apache/jackrabbit/mongomk/ConflictTest.java
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: jackrabbit/oak/trunk/oak-mongomk/src/test/java/org/apache/jackrabbit/mongomk/ConflictTest.java
------------------------------------------------------------------------------
    svn:keywords = Author Date Id Revision Rev URL

Modified: jackrabbit/oak/trunk/oak-mongomk/src/test/java/org/apache/jackrabbit/mongomk/MongoMKCommitAddTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-mongomk/src/test/java/org/apache/jackrabbit/mongomk/MongoMKCommitAddTest.java?rev=1468326&r1=1468325&r2=1468326&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-mongomk/src/test/java/org/apache/jackrabbit/mongomk/MongoMKCommitAddTest.java
(original)
+++ jackrabbit/oak/trunk/oak-mongomk/src/test/java/org/apache/jackrabbit/mongomk/MongoMKCommitAddTest.java
Tue Apr 16 08:20:11 2013
@@ -20,6 +20,7 @@ import static org.junit.Assert.assertFal
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
+import org.apache.jackrabbit.mk.api.MicroKernelException;
 import org.json.simple.JSONObject;
 import org.junit.Ignore;
 import org.junit.Test;
@@ -143,9 +144,14 @@ public class MongoMKCommitAddTest extend
     public void setPropertyIllegalKey() throws Exception {
         mk.commit("/", "+\"a\" : {}", null, null);
 
-        mk.commit("/", "^\"a/ke.y1\" : \"value\"", null, null);
+        mk.commit("/", "^\"a/_id\" : \"value\"", null, null);
         String nodes = mk.getNodes("/a", null, 0 /*depth*/, 0 /*offset*/, -1 /*maxChildNodes*/,
null /*filter*/);
         JSONObject obj = parseJSONObject(nodes);
+        assertPropertyValue(obj, "_id", "value");
+
+        mk.commit("/", "^\"a/ke.y1\" : \"value\"", null, null);
+        nodes = mk.getNodes("/a", null, 0 /*depth*/, 0 /*offset*/, -1 /*maxChildNodes*/,
null /*filter*/);
+        obj = parseJSONObject(nodes);
         assertPropertyValue(obj, "ke.y1", "value");
 
         mk.commit("/", "^\"a/ke.y.1\" : \"value\"", null, null);
@@ -174,10 +180,18 @@ public class MongoMKCommitAddTest extend
         String rev1 = mk.commit("/", "+\"a\" : {} ^\"a/key1\" : \"value1\"", null, null);
 
         // Commit with rev1
-        mk.commit("/", "^\"a/key1\" : \"value2\"", rev1, null);
+        String rev2 = mk.commit("/", "^\"a/key1\" : \"value2\"", rev1, null);
+
+        // try to commit with rev1 again (to overwrite rev2)
+        try {
+            mk.commit("/", "^\"a/key1\" : \"value3\"", rev1, null);
+            fail("commit must fail with conflicting change");
+        } catch (MicroKernelException e) {
+            // expected
+        }
 
-        // Commit with rev1 again (to overwrite rev2)
-        mk.commit("/", "^\"a/key1\" : \"value3\"", rev1, null);
+        // now overwrite with correct base revision
+        mk.commit("/", "^\"a/key1\" : \"value3\"", rev2, null);
 
         String nodes = mk.getNodes("/a", null, 0 /*depth*/, 0 /*offset*/, -1 /*maxChildNodes*/,
null /*filter*/);
         JSONObject obj = parseJSONObject(nodes);



Mime
View raw message