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/05/19 14:49:24 UTC
svn commit: r1595875 - in /jackrabbit/oak/trunk/oak-core/src:
main/java/org/apache/jackrabbit/oak/plugins/document/
main/java/org/apache/jackrabbit/oak/plugins/document/mongo/
test/java/org/apache/jackrabbit/oak/plugins/document/
Author: mreutegg
Date: Mon May 19 12:49:24 2014
New Revision: 1595875
URL: http://svn.apache.org/r1595875
Log:
OAK-1822: NodeDocument _modified may go back in time
Added:
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreIT.java
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/UpdateOp.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/UpdateUtils.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/AbstractDocumentStoreTest.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java?rev=1595875&r1=1595874&r2=1595875&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java Mon May 19 12:49:24 2014
@@ -1205,7 +1205,7 @@ public final class NodeDocument extends
public static void setModified(@Nonnull UpdateOp op,
@Nonnull Revision revision) {
- checkNotNull(op).set(MODIFIED_IN_SECS, getModifiedInSecs(checkNotNull(revision).getTimestamp()));
+ checkNotNull(op).max(MODIFIED_IN_SECS, getModifiedInSecs(checkNotNull(revision).getTimestamp()));
}
public static void setRevision(@Nonnull UpdateOp op,
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=1595875&r1=1595874&r2=1595875&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 Mon May 19 12:49:24 2014
@@ -131,9 +131,7 @@ public final class UpdateOp {
* @param value the 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;
+ Operation op = new Operation(Operation.Type.SET_MAP_ENTRY, value);
changes.put(new Key(property, checkNotNull(revision)), op);
}
@@ -145,8 +143,7 @@ public final class UpdateOp {
* @param revision the revision
*/
public void removeMapEntry(@Nonnull String property, @Nonnull Revision revision) {
- Operation op = new Operation();
- op.type = Operation.Type.REMOVE_MAP_ENTRY;
+ Operation op = new Operation(Operation.Type.REMOVE_MAP_ENTRY, null);
changes.put(new Key(property, checkNotNull(revision)), op);
}
@@ -157,9 +154,23 @@ public final class UpdateOp {
* @param value the value
*/
void set(String property, Object value) {
- Operation op = new Operation();
- op.type = Operation.Type.SET;
- op.value = value;
+ Operation op = new Operation(Operation.Type.SET, value);
+ changes.put(new Key(property, null), op);
+ }
+
+ /**
+ * Set the property to the given value if the new value is higher than the
+ * existing value. The property is also set to the given value if the
+ * property does not yet exist.
+ * <p>
+ * The result of a max operation with different types of values is
+ * undefined.
+ *
+ * @param property the name of the property to set.
+ * @param value the new value for the property.
+ */
+ <T> void max(String property, Comparable<T> value) {
+ Operation op = new Operation(Operation.Type.MAX, value);
changes.put(new Key(property, null), op);
}
@@ -187,9 +198,7 @@ public final class UpdateOp {
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;
+ Operation op = new Operation(Operation.Type.CONTAINS_MAP_ENTRY, exists);
changes.put(new Key(property, checkNotNull(revision)), op);
}
@@ -200,9 +209,7 @@ public final class UpdateOp {
* @param value the increment
*/
public void increment(@Nonnull String property, long value) {
- Operation op = new Operation();
- op.type = Operation.Type.INCREMENT;
- op.value = value;
+ Operation op = new Operation(Operation.Type.INCREMENT, value);
changes.put(new Key(property, null), op);
}
@@ -239,6 +246,14 @@ public final class UpdateOp {
SET,
/**
+ * Set the value if the new value is higher than the existing value.
+ * The new value is also considered higher, when there is no
+ * existing value.
+ * The sub-key is not used.
+ */
+ MAX,
+
+ /**
* Increment the Long value with the provided Long value.
* The sub-key is not used.
*/
@@ -267,12 +282,17 @@ public final class UpdateOp {
/**
* The operation type.
*/
- public Type type;
+ public final Type type;
/**
* The value, if any.
*/
- public Object value;
+ public final Object value;
+
+ Operation(Type type, Object value) {
+ this.type = checkNotNull(type);
+ this.value = value;
+ }
@Override
public String toString() {
@@ -283,18 +303,16 @@ public final class UpdateOp {
Operation reverse = null;
switch (type) {
case INCREMENT:
- reverse = new Operation();
- reverse.type = Type.INCREMENT;
- reverse.value = -(Long) value;
+ reverse = new Operation(Type.INCREMENT, -(Long) value);
break;
case SET:
+ case MAX:
case REMOVE_MAP_ENTRY:
case CONTAINS_MAP_ENTRY:
// nothing to do
break;
case SET_MAP_ENTRY:
- reverse = new Operation();
- reverse.type = Type.REMOVE_MAP_ENTRY;
+ reverse = new Operation(Type.REMOVE_MAP_ENTRY, null);
break;
}
return reverse;
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/UpdateUtils.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/UpdateUtils.java?rev=1595875&r1=1595874&r2=1595875&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/UpdateUtils.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/UpdateUtils.java Mon May 19 12:49:24 2014
@@ -44,7 +44,9 @@ public class UpdateUtils {
* @param comparator
* the revision comparator.
*/
- public static void applyChanges(@Nonnull Document doc, @Nonnull UpdateOp update, @Nonnull Comparator<Revision> comparator) {
+ public static void applyChanges(@Nonnull Document doc,
+ @Nonnull UpdateOp update,
+ @Nonnull Comparator<Revision> comparator) {
for (Entry<Key, Operation> e : checkNotNull(update).getChanges().entrySet()) {
Key k = e.getKey();
Operation op = e.getValue();
@@ -53,6 +55,15 @@ public class UpdateUtils {
doc.put(k.toString(), op.value);
break;
}
+ case MAX: {
+ Comparable newValue = (Comparable) op.value;
+ Object old = doc.get(k.toString());
+ //noinspection unchecked
+ if (old == null || newValue.compareTo(old) > 0) {
+ doc.put(k.toString(), op.value);
+ }
+ break;
+ }
case INCREMENT: {
Object old = doc.get(k.toString());
Long x = (Long) op.value;
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=1595875&r1=1595874&r2=1595875&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 Mon May 19 12:49:24 2014
@@ -570,6 +570,7 @@ public class MongoDocumentStore implemen
Operation op = entry.getValue();
switch (op.type) {
case SET:
+ case MAX:
case INCREMENT: {
inserts[i].put(k.toString(), op.value);
break;
@@ -965,6 +966,7 @@ public class MongoDocumentStore implemen
@Nonnull
private static DBObject createUpdate(UpdateOp updateOp) {
BasicDBObject setUpdates = new BasicDBObject();
+ BasicDBObject maxUpdates = new BasicDBObject();
BasicDBObject incUpdates = new BasicDBObject();
BasicDBObject unsetUpdates = new BasicDBObject();
@@ -980,16 +982,17 @@ public class MongoDocumentStore implemen
}
Operation op = entry.getValue();
switch (op.type) {
- case SET: {
+ case SET:
+ case SET_MAP_ENTRY: {
setUpdates.append(k.toString(), op.value);
break;
}
- case INCREMENT: {
- incUpdates.append(k.toString(), op.value);
+ case MAX: {
+ maxUpdates.append(k.toString(), op.value);
break;
}
- case SET_MAP_ENTRY: {
- setUpdates.append(k.toString(), op.value);
+ case INCREMENT: {
+ incUpdates.append(k.toString(), op.value);
break;
}
case REMOVE_MAP_ENTRY: {
@@ -1003,6 +1006,9 @@ public class MongoDocumentStore implemen
if (!setUpdates.isEmpty()) {
update.append("$set", setUpdates);
}
+ if (!maxUpdates.isEmpty()) {
+ update.append("$max", maxUpdates);
+ }
if (!incUpdates.isEmpty()) {
update.append("$inc", incUpdates);
}
Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/AbstractDocumentStoreTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/AbstractDocumentStoreTest.java?rev=1595875&r1=1595874&r2=1595875&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/AbstractDocumentStoreTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/AbstractDocumentStoreTest.java Mon May 19 12:49:24 2014
@@ -34,17 +34,19 @@ public abstract class AbstractDocumentSt
protected String dsname;
protected DocumentStore ds;
+ protected DocumentStoreFixture dsf;
protected List<String> removeMe = new ArrayList<String>();
static final Logger LOG = LoggerFactory.getLogger(AbstractDocumentStoreTest.class);
public AbstractDocumentStoreTest(DocumentStoreFixture dsf) {
+ this.dsf = dsf;
this.ds = dsf.createDocumentStore();
this.dsname = dsf.getName();
}
@After
- public void cleanUp() {
+ public void cleanUp() throws Exception {
if (!removeMe.isEmpty()) {
long start = System.nanoTime();
try {
@@ -65,6 +67,7 @@ public abstract class AbstractDocumentSt
LOG.info(removeMe.size() + " documents removed in " + elapsed + "ms (" + rate + "/ms)");
}
}
+ dsf.dispose();
}
@Parameterized.Parameters
Added: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreIT.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreIT.java?rev=1595875&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreIT.java (added)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreIT.java Mon May 19 12:49:24 2014
@@ -0,0 +1,116 @@
+/*
+ * 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.document;
+
+import org.apache.jackrabbit.oak.plugins.document.util.TimingDocumentStoreWrapper;
+import org.apache.jackrabbit.oak.spi.commit.CommitInfo;
+import org.apache.jackrabbit.oak.spi.commit.EmptyHook;
+import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
+import org.apache.jackrabbit.oak.stats.Clock;
+import org.junit.After;
+import org.junit.Test;
+
+import static java.util.concurrent.TimeUnit.SECONDS;
+import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.MODIFIED_IN_SECS_RESOLUTION;
+import static org.apache.jackrabbit.oak.plugins.document.util.Utils.getIdFromPath;
+import static org.junit.Assert.assertEquals;
+
+/**
+ * Tests DocumentNodeStore on various DocumentStore back-ends.
+ */
+public class DocumentNodeStoreIT extends AbstractDocumentStoreTest {
+
+
+ public DocumentNodeStoreIT(DocumentStoreFixture dsf) {
+ super(dsf);
+ }
+
+ @After
+ public void tearDown() {
+ Revision.resetClockToDefault();
+ }
+
+
+ @Test
+ public void modifiedResetWithDiff() throws Exception {
+ Clock clock = new Clock.Virtual();
+ clock.waitUntil(System.currentTimeMillis());
+ Revision.setClock(clock);
+ DocumentStore docStore = new TimingDocumentStoreWrapper(ds) {
+ @Override
+ public void dispose() {
+ // do not dispose yet
+ }
+ };
+ DocumentNodeStore ns1 = new DocumentMK.Builder()
+ .setDocumentStore(docStore).setClusterId(1)
+ .setAsyncDelay(0).clock(clock)
+ // use a no-op diff cache to simulate a cache miss
+ // when the diff is made later in the test
+ .setDiffCache(AmnesiaDiffCache.INSTANCE)
+ .getNodeStore();
+ NodeBuilder builder1 = ns1.getRoot().builder();
+ builder1.child("node");
+ removeMe.add(getIdFromPath("/node"));
+ for (int i = 0; i < DocumentMK.MANY_CHILDREN_THRESHOLD; i++) {
+ builder1.child("node-" + i);
+ removeMe.add(getIdFromPath("/node/node-" + i));
+ }
+ ns1.merge(builder1, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+ // make sure commit is visible to other node store instance
+ ns1.runBackgroundOperations();
+
+ DocumentNodeStore ns2 = new DocumentMK.Builder()
+ .setDocumentStore(docStore).setClusterId(2)
+ .setAsyncDelay(0).clock(clock).getNodeStore();
+
+ NodeBuilder builder2 = ns2.getRoot().builder();
+ builder2.child("node").child("child-a");
+ removeMe.add(getIdFromPath("/node/child-a"));
+ ns2.merge(builder2, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+
+ // wait at least _modified resolution. in reality the wait may
+ // not be necessary. e.g. when the clock passes the resolution boundary
+ // exactly at this time
+ clock.waitUntil(System.currentTimeMillis() +
+ SECONDS.toMillis(MODIFIED_IN_SECS_RESOLUTION + 1));
+
+ builder1 = ns1.getRoot().builder();
+ builder1.child("node").child("child-b");
+ removeMe.add(getIdFromPath("/node/child-b"));
+ ns1.merge(builder1, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+ // remember root for diff
+ DocumentNodeState root1 = ns1.getRoot();
+
+ builder1 = root1.builder();
+ builder1.child("node").child("child-c");
+ removeMe.add(getIdFromPath("/node/child-c"));
+ ns1.merge(builder1, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+ // remember root for diff
+ DocumentNodeState root2 = ns1.getRoot();
+
+ ns1.runBackgroundOperations();
+ ns2.runBackgroundOperations();
+
+ String diff = ns1.diffChildren(root2, root1);
+ // must report /node as changed
+ assertEquals("^\"node\":{}", diff.trim());
+
+ ns1.dispose();
+ ns2.dispose();
+ }
+}
Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java?rev=1595875&r1=1595874&r2=1595875&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java Mon May 19 12:49:24 2014
@@ -51,7 +51,6 @@ import org.apache.jackrabbit.oak.spi.sta
import org.apache.jackrabbit.oak.spi.state.NodeState;
import org.apache.jackrabbit.oak.stats.Clock;
import org.junit.After;
-import org.junit.Ignore;
import org.junit.Test;
import com.google.common.collect.Iterables;
@@ -419,7 +418,6 @@ public class DocumentNodeStoreTest {
ns.dispose();
}
- @Ignore("OAK-1822")
@Test
public void modifiedReset() throws Exception {
Clock clock = new Clock.Virtual();
@@ -469,66 +467,6 @@ public class DocumentNodeStoreTest {
ns2.dispose();
}
- @Ignore("OAK-1822")
- @Test
- public void modifiedResetWithDiff() throws Exception {
- Clock clock = new Clock.Virtual();
- clock.waitUntil(System.currentTimeMillis());
- Revision.setClock(clock);
- MemoryDocumentStore docStore = new MemoryDocumentStore();
- DocumentNodeStore ns1 = new DocumentMK.Builder()
- .setDocumentStore(docStore).setClusterId(1)
- .setAsyncDelay(0).clock(clock)
- // use a no-op diff cache to simulate a cache miss
- // when the diff is made later in the test
- .setDiffCache(AmnesiaDiffCache.INSTANCE)
- .getNodeStore();
- NodeBuilder builder1 = ns1.getRoot().builder();
- builder1.child("node");
- for (int i = 0; i < DocumentMK.MANY_CHILDREN_THRESHOLD; i++) {
- builder1.child("node-" + i);
- }
- ns1.merge(builder1, EmptyHook.INSTANCE, CommitInfo.EMPTY);
- // make sure commit is visible to other node store instance
- ns1.runBackgroundOperations();
-
- DocumentNodeStore ns2 = new DocumentMK.Builder()
- .setDocumentStore(docStore).setClusterId(2)
- .setAsyncDelay(0).clock(clock).getNodeStore();
-
- NodeBuilder builder2 = ns2.getRoot().builder();
- builder2.child("node").child("child-a");
- ns2.merge(builder2, EmptyHook.INSTANCE, CommitInfo.EMPTY);
-
- // wait at least _modified resolution. in reality the wait may
- // not be necessary. e.g. when the clock passes the resolution boundary
- // exactly at this time
- clock.waitUntil(System.currentTimeMillis() +
- SECONDS.toMillis(MODIFIED_IN_SECS_RESOLUTION + 1));
-
- builder1 = ns1.getRoot().builder();
- builder1.child("node").child("child-b");
- ns1.merge(builder1, EmptyHook.INSTANCE, CommitInfo.EMPTY);
- // remember root for diff
- DocumentNodeState root1 = ns1.getRoot();
-
- builder1 = root1.builder();
- builder1.child("node").child("child-c");
- ns1.merge(builder1, EmptyHook.INSTANCE, CommitInfo.EMPTY);
- // remember root for diff
- DocumentNodeState root2 = ns1.getRoot();
-
- ns1.runBackgroundOperations();
- ns2.runBackgroundOperations();
-
- String diff = ns1.diffChildren(root2, root1);
- // must report /node as changed
- assertEquals("^\"node\":{}", diff);
-
- ns1.dispose();
- ns2.dispose();
- }
-
private static class TestHook extends EditorHook {
TestHook(final String prefix) {