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 2015/06/24 11:40:15 UTC
svn commit: r1687220 - in /jackrabbit/oak/trunk/oak-core/src:
main/java/org/apache/jackrabbit/oak/plugins/document/
test/java/org/apache/jackrabbit/oak/plugins/document/
Author: mreutegg
Date: Wed Jun 24 09:40:15 2015
New Revision: 1687220
URL: http://svn.apache.org/r1687220
Log:
OAK-3028: Hierarchy conflict detection broken
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/DocumentNodeStore.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/UpdateOp.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/UpdateOpTest.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=1687220&r1=1687219&r2=1687220&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 Wed Jun 24 09:40:15 2015
@@ -383,7 +383,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.shallowCopy(commitRoot.getId());
+ UpdateOp commit = commitRoot.copy();
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/DocumentNodeStore.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java?rev=1687220&r1=1687219&r2=1687220&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java Wed Jun 24 09:40:15 2015
@@ -1170,13 +1170,11 @@ public final class DocumentNodeStore
@CheckForNull
NodeDocument updateCommitRoot(UpdateOp commit) throws DocumentStoreException {
// use batch commit when there are only revision and modified updates
- // and collision checks
boolean batch = true;
for (Map.Entry<Key, Operation> op : commit.getChanges().entrySet()) {
String name = op.getKey().getName();
if (NodeDocument.isRevisionsEntry(name)
- || NodeDocument.MODIFIED_IN_SECS.equals(name)
- || NodeDocument.COLLISIONS.equals(name)) {
+ || NodeDocument.MODIFIED_IN_SECS.equals(name)) {
continue;
}
batch = false;
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=1687220&r1=1687219&r2=1687220&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 Wed Jun 24 09:40:15 2015
@@ -50,23 +50,32 @@ public final class UpdateOp {
* @param isNew whether this is a new document
*/
UpdateOp(String id, boolean isNew) {
- this(id, isNew, false, new HashMap<Key, Operation>());
+ this(id, isNew, false, new HashMap<Key, Operation>(), null);
}
- private UpdateOp(String id, boolean isNew, boolean isDelete,
- Map<Key, Operation> changes) {
- this.id = id;
+ private UpdateOp(@Nonnull String id, boolean isNew, boolean isDelete,
+ @Nonnull Map<Key, Operation> changes,
+ @Nullable Map<Key, Condition> conditions) {
+ this.id = checkNotNull(id);
this.isNew = isNew;
this.isDelete = isDelete;
- this.changes = changes;
+ this.changes = checkNotNull(changes);
+ this.conditions = conditions;
}
static UpdateOp combine(String id, Iterable<UpdateOp> ops) {
Map<Key, Operation> changes = Maps.newHashMap();
+ Map<Key, Condition> conditions = Maps.newHashMap();
for (UpdateOp op : ops) {
changes.putAll(op.getChanges());
+ if (op.conditions != null) {
+ conditions.putAll(op.conditions);
+ }
+ }
+ if (conditions.isEmpty()) {
+ conditions = null;
}
- return new UpdateOp(id, false, false, changes);
+ return new UpdateOp(id, false, false, changes, conditions);
}
/**
@@ -76,7 +85,7 @@ public final class UpdateOp {
* @param id the primary key.
*/
public UpdateOp shallowCopy(String id) {
- return new UpdateOp(id, isNew, isDelete, changes);
+ return new UpdateOp(id, isNew, isDelete, changes, conditions);
}
/**
@@ -86,8 +95,12 @@ public final class UpdateOp {
* @return a copy of this operation.
*/
public UpdateOp copy() {
+ Map<Key, Condition> conditionMap = null;
+ if (conditions != null) {
+ conditionMap = new HashMap<Key, Condition>(conditions);
+ }
return new UpdateOp(id, isNew, isDelete,
- new HashMap<Key, Operation>(changes));
+ new HashMap<Key, Operation>(changes), conditionMap);
}
public String getId() {
@@ -254,7 +267,11 @@ public final class UpdateOp {
@Override
public String toString() {
- return "key: " + id + " " + (isNew ? "new" : "update") + " " + changes;
+ String s = "key: " + id + " " + (isNew ? "new" : "update") + " " + changes;
+ if (conditions != null) {
+ s += " conditions " + conditions;
+ }
+ return s;
}
private Map<Key, Condition> getOrCreateConditions() {
@@ -462,5 +479,4 @@ public final class UpdateOp {
return false;
}
}
-
}
Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/UpdateOpTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/UpdateOpTest.java?rev=1687220&r1=1687219&r2=1687220&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/UpdateOpTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/UpdateOpTest.java Wed Jun 24 09:40:15 2015
@@ -16,10 +16,15 @@
*/
package org.apache.jackrabbit.oak.plugins.document;
+import com.google.common.collect.Lists;
+
import org.junit.Test;
+import static com.google.common.collect.Lists.newArrayList;
+import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
/**
* Tests for UpdateOp
@@ -57,5 +62,111 @@ public class UpdateOpTest {
assertFalse(k6.equals(k7));
}
-
+ @Test
+ public void combine() {
+ UpdateOp op1 = new UpdateOp("id", false);
+ op1.set("p", "value");
+ UpdateOp op2 = new UpdateOp("id", false);
+ Revision r = Revision.newRevision(1);
+ op2.containsMapEntry("e", r, true);
+
+ UpdateOp combined = UpdateOp.combine("id", newArrayList(op1, op2));
+ assertTrue(combined.hasChanges());
+ assertEquals(1, combined.getChanges().size());
+ assertEquals(1, combined.getConditions().size());
+ }
+
+ @Test
+ public void containsMapEntry() {
+ Revision r = Revision.newRevision(1);
+ UpdateOp op = new UpdateOp("id", true);
+ try {
+ op.containsMapEntry("p", r, true);
+ fail("expected " + IllegalStateException.class.getName());
+ } catch (IllegalStateException e) {
+ // expected
+ }
+ op = new UpdateOp("id", false);
+ op.containsMapEntry("p", r, true);
+ assertEquals(1, op.getConditions().size());
+ UpdateOp.Key key = op.getConditions().keySet().iterator().next();
+ assertEquals(r, key.getRevision());
+ assertEquals("p", key.getName());
+ UpdateOp.Condition c = op.getConditions().get(key);
+ assertEquals(UpdateOp.Condition.EXISTS, c);
+
+ op = new UpdateOp("id", false);
+ op.containsMapEntry("p", r, false);
+ assertEquals(1, op.getConditions().size());
+ key = op.getConditions().keySet().iterator().next();
+ assertEquals(r, key.getRevision());
+ assertEquals("p", key.getName());
+ c = op.getConditions().get(key);
+ assertEquals(UpdateOp.Condition.MISSING, c);
+ }
+
+ @Test
+ public void copy() {
+ UpdateOp op1 = new UpdateOp("id", false);
+ op1.set("p", "value");
+
+ UpdateOp op2 = op1.copy();
+ assertTrue(op2.hasChanges());
+ assertEquals(1, op2.getChanges().size());
+ assertEquals(0, op2.getConditions().size());
+
+ Revision r = Revision.newRevision(1);
+ op1.containsMapEntry("e", r, true);
+
+ op2 = op1.copy();
+ assertEquals(1, op2.getConditions().size());
+ }
+
+ @Test
+ public void equalsTest() {
+ Revision r = Revision.newRevision(1);
+ UpdateOp op = new UpdateOp("id", true);
+ try {
+ op.equals("p", r, "v");
+ fail("expected " + IllegalStateException.class.getName());
+ } catch (IllegalStateException e) {
+ // expected
+ }
+ op = new UpdateOp("id", false);
+ op.equals("p", r, "v");
+ assertEquals(1, op.getConditions().size());
+ UpdateOp.Key key = op.getConditions().keySet().iterator().next();
+ assertEquals(r, key.getRevision());
+ assertEquals("p", key.getName());
+ UpdateOp.Condition c = op.getConditions().get(key);
+ assertEquals(UpdateOp.Condition.Type.EQUALS, c.type);
+ assertEquals("v", c.value);
+ }
+
+ @Test
+ public void getChanges() {
+ UpdateOp op = new UpdateOp("id", false);
+ assertEquals(0, op.getChanges().size());
+ assertTrue(!op.hasChanges());
+ op.set("p", "value");
+ assertEquals(1, op.getChanges().size());
+ assertTrue(op.hasChanges());
+ }
+
+ @Test
+ public void shallowCopy() {
+ UpdateOp op1 = new UpdateOp("id", false);
+ op1.set("p", "value");
+
+ UpdateOp op2 = op1.shallowCopy("id");
+ assertTrue(op2.hasChanges());
+ assertEquals(1, op2.getChanges().size());
+ assertEquals(0, op2.getConditions().size());
+
+ Revision r = Revision.newRevision(1);
+ op1.containsMapEntry("e", r, true);
+
+ op2 = op1.shallowCopy("id");
+ assertEquals(1, op2.getConditions().size());
+ }
}