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 2014/02/25 10:49:15 UTC
svn commit: r1571634 - in /jackrabbit/oak/trunk/oak-core/src:
main/java/org/apache/jackrabbit/oak/plugins/document/
main/java/org/apache/jackrabbit/oak/plugins/document/memory/
main/java/org/apache/jackrabbit/oak/plugins/document/mongo/
test/java/org/a...
Author: mreutegg
Date: Tue Feb 25 09:49:14 2014
New Revision: 1571634
URL: http://svn.apache.org/r1571634
Log:
OAK-1466: Document _modCount not monotonic increasing
- do not modify passed UpdateOp and add test
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Commit.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/UpdateOp.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/memory/MemoryDocumentStore.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoDocumentStore.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/MongoDocumentStoreIT.java
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Commit.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Commit.java?rev=1571634&r1=1571633&r2=1571634&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Commit.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Commit.java Tue Feb 25 09:49:14 2014
@@ -325,7 +325,7 @@ public class Commit {
// to set isNew to false. If we get here the
// commitRoot document already exists and
// only needs an update
- UpdateOp commit = commitRoot.clone(commitRoot.getId());
+ UpdateOp commit = commitRoot.shallowCopy(commitRoot.getId());
commit.setNew(false);
// only set revision on commit root when there is
// no collision for this commit revision
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/UpdateOp.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/UpdateOp.java?rev=1571634&r1=1571633&r2=1571634&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/UpdateOp.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/UpdateOp.java Tue Feb 25 09:49:14 2014
@@ -73,9 +73,20 @@ public final class UpdateOp {
*
* @param id the primary key.
*/
- public UpdateOp clone(String id) {
+ public UpdateOp shallowCopy(String id) {
return new UpdateOp(id, isNew, isDelete, changes);
}
+
+ /**
+ * Creates a deep copy of this update operation. Changes to the returned
+ * {@code UpdateOp} do not affect this object.
+ *
+ * @return a copy of this operation.
+ */
+ public UpdateOp copy() {
+ return new UpdateOp(id, isNew, isDelete,
+ new HashMap<Key, Operation>(changes));
+ }
public String getId() {
return id;
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/memory/MemoryDocumentStore.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/memory/MemoryDocumentStore.java?rev=1571634&r1=1571633&r2=1571634&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/memory/MemoryDocumentStore.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/memory/MemoryDocumentStore.java Tue Feb 25 09:49:14 2014
@@ -242,7 +242,7 @@ public class MemoryDocumentStore impleme
if (!map.containsKey(key)) {
continue;
}
- internalCreateOrUpdate(collection, updateOp.clone(key), true);
+ internalCreateOrUpdate(collection, updateOp.shallowCopy(key), true);
}
} finally {
lock.unlock();
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoDocumentStore.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoDocumentStore.java?rev=1571634&r1=1571633&r2=1571634&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoDocumentStore.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoDocumentStore.java Tue Feb 25 09:49:14 2014
@@ -368,6 +368,8 @@ public class MongoDocumentStore implemen
boolean upsert,
boolean checkConditions) {
DBCollection dbCollection = getDBCollection(collection);
+ // make sure we don't modify the original updateOp
+ updateOp = updateOp.copy();
DBObject update = createUpdate(updateOp);
Lock lock = getAndLock(updateOp.getId());
@@ -453,7 +455,6 @@ public class MongoDocumentStore implemen
for (int i = 0; i < updateOps.size(); i++) {
inserts[i] = new BasicDBObject();
UpdateOp update = updateOps.get(i);
- update.increment(Document.MOD_COUNT, 1);
T target = collection.newDocument(this);
UpdateUtils.applyChanges(target, update, comparator);
docs.add(target);
@@ -484,6 +485,10 @@ public class MongoDocumentStore implemen
break;
}
}
+ if (!inserts[i].containsField(Document.MOD_COUNT)) {
+ inserts[i].put(Document.MOD_COUNT, 1L);
+ target.put(Document.MOD_COUNT, 1L);
+ }
}
DBCollection dbCollection = getDBCollection(collection);
@@ -519,6 +524,8 @@ public class MongoDocumentStore implemen
UpdateOp updateOp) {
DBCollection dbCollection = getDBCollection(collection);
QueryBuilder query = QueryBuilder.start(Document.ID).in(keys);
+ // make sure we don't modify the original updateOp
+ updateOp = updateOp.copy();
DBObject update = createUpdate(updateOp);
long start = start();
try {
@@ -543,7 +550,7 @@ public class MongoDocumentStore implemen
nodesCache.invalidate(new StringValue(entry.getKey()));
} else {
applyToCache(Collection.NODES, entry.getValue(),
- updateOp.clone(entry.getKey()));
+ updateOp.shallowCopy(entry.getKey()));
}
} finally {
lock.unlock();
@@ -559,7 +566,7 @@ public class MongoDocumentStore implemen
@CheckForNull
<T extends Document> T convertFromDBObject(@Nonnull Collection<T> collection,
- @Nullable DBObject n) {
+ @Nullable DBObject n) {
T copy = null;
if (n != null) {
copy = collection.newDocument(this);
Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/MongoDocumentStoreIT.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/MongoDocumentStoreIT.java?rev=1571634&r1=1571633&r2=1571634&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/MongoDocumentStoreIT.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/MongoDocumentStoreIT.java Tue Feb 25 09:49:14 2014
@@ -22,6 +22,7 @@ import java.util.Map;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
+import org.apache.jackrabbit.mk.api.MicroKernelException;
import org.apache.jackrabbit.oak.plugins.document.util.Utils;
import org.junit.Test;
@@ -30,6 +31,9 @@ import com.google.common.collect.Maps;
import static junit.framework.Assert.assertEquals;
import static junit.framework.Assert.assertNotNull;
import static junit.framework.Assert.assertNull;
+import static junit.framework.Assert.assertTrue;
+import static junit.framework.Assert.fail;
+import static org.apache.jackrabbit.oak.plugins.document.Collection.NODES;
/**
* Tests {@code MongoDocumentStore} with concurrent updates.
@@ -55,7 +59,7 @@ public class MongoDocumentStoreIT extend
for (int i = 0; i < UPDATES_PER_THREAD; i++) {
UpdateOp update = new UpdateOp(id, false);
update.setMapEntry("prop", r, String.valueOf(i));
- docStore.createOrUpdate(Collection.NODES, update);
+ docStore.createOrUpdate(NODES, update);
}
}
}));
@@ -68,7 +72,7 @@ public class MongoDocumentStoreIT extend
try {
Map<Revision, Integer> previous = Maps.newHashMap();
while (running.get()) {
- NodeDocument doc = docStore.find(Collection.NODES, id);
+ NodeDocument doc = docStore.find(NODES, id);
if (doc == null) {
throw new Exception("document is null");
}
@@ -105,7 +109,7 @@ public class MongoDocumentStoreIT extend
for (Exception e : exceptions) {
throw e;
}
- NodeDocument doc = docStore.find(Collection.NODES, id);
+ NodeDocument doc = docStore.find(NODES, id);
assertNotNull(doc);
Map<Revision, String> values = doc.getLocalMap("prop");
assertNotNull(values);
@@ -129,8 +133,33 @@ public class MongoDocumentStoreIT extend
public void negativeCache() throws Exception {
String id = Utils.getIdFromPath("/test");
DocumentStore docStore = mk.getDocumentStore();
- assertNull(docStore.find(Collection.NODES, id));
+ assertNull(docStore.find(NODES, id));
mk.commit("/", "+\"test\":{}", null, null);
- assertNotNull(docStore.find(Collection.NODES, id));
+ assertNotNull(docStore.find(NODES, id));
+ }
+
+ @Test
+ public void modCount() throws Exception {
+ DocumentStore docStore = mk.getDocumentStore();
+ String head = mk.commit("/", "+\"test\":{}", null, null);
+ mk.commit("/test", "^\"prop\":\"v1\"", head, null);
+ // make sure _lastRev is persisted and _modCount updated accordingly
+ mk.runBackgroundOperations();
+
+ NodeDocument doc = docStore.find(NODES, Utils.getIdFromPath("/test"));
+ assertNotNull(doc);
+ Number mc1 = doc.getModCount();
+ assertNotNull(mc1);
+ try {
+ mk.commit("/test", "^\"prop\":\"v2\"", head, null);
+ fail();
+ } catch (MicroKernelException e) {
+ // expected
+ }
+ doc = docStore.find(NODES, Utils.getIdFromPath("/test"));
+ assertNotNull(doc);
+ Number mc2 = doc.getModCount();
+ assertNotNull(mc2);
+ assertTrue(mc2.longValue() > mc1.longValue());
}
}