From 063f72a2c5db7cc5b47e9d9d2c2c6ccef9b8f31e Mon Sep 17 00:00:00 2001
From: Ludovic Poitou <ludovic.poitou@forgerock.com>
Date: Fri, 09 Sep 2011 13:24:02 +0000
Subject: [PATCH] Fix for OPENDJ-274. Added detection of modifications with identical ChangeNumber to a Single-Value Attribute. Unit tests added for all cases that were identified as not resulting in the same value in the attribute after replication.
---
opends/tests/unit-tests-testng/src/server/org/opends/server/replication/plugin/ModifyConflictTest.java | 413 ++++++++++++++++++++++++++++++++++++++++++++-
opends/src/server/org/opends/server/replication/plugin/AttrHistoricalSingle.java | 86 +++++++-
2 files changed, 475 insertions(+), 24 deletions(-)
diff --git a/opends/src/server/org/opends/server/replication/plugin/AttrHistoricalSingle.java b/opends/src/server/org/opends/server/replication/plugin/AttrHistoricalSingle.java
index c61c6e9..826bacc 100644
--- a/opends/src/server/org/opends/server/replication/plugin/AttrHistoricalSingle.java
+++ b/opends/src/server/org/opends/server/replication/plugin/AttrHistoricalSingle.java
@@ -23,6 +23,7 @@
*
*
* Copyright 2008-2010 Sun Microsystems, Inc.
+ * Portions Copyright 2011 ForgeRock AS
*/
package org.opends.server.replication.plugin;
@@ -52,6 +53,10 @@
private ChangeNumber addTime = null; // last time when a value was added
private AttributeValue value = null; // last added value
+ // last operation applied. This is only used for multiple mods on the same
+ // single valued attribute in the same modification.
+ private HistAttrModificationKey lastMod = null;
+
/**
* {@inheritDoc}
*/
@@ -99,11 +104,13 @@
case DELETE:
this.deleteTime = changeNumber;
this.value = newValue;
+ lastMod = HistAttrModificationKey.DEL;
break;
case ADD:
this.addTime = changeNumber;
this.value = newValue;
+ lastMod = HistAttrModificationKey.ADD;
break;
case REPLACE:
@@ -111,10 +118,12 @@
{
// REPLACE with null value is actually a DELETE
this.deleteTime = changeNumber;
+ lastMod = HistAttrModificationKey.DEL;
}
else
{
this.deleteTime = addTime = changeNumber;
+ lastMod = HistAttrModificationKey.REPL;
}
this.value = newValue;
break;
@@ -144,21 +153,51 @@
switch (mod.getModificationType())
{
case DELETE:
- if ((changeNumber.newer(addTime)) &&
- ((newValue == null) ||
- ((newValue != null) && (newValue.equals(value))) ||
- (value == null)))
+ if (changeNumber.newer(addTime))
{
- if (changeNumber.newer(deleteTime))
- deleteTime = changeNumber;
- AttributeType type = modAttr.getAttributeType();
- if (!modifiedEntry.hasAttribute(type))
+ if ((newValue == null) ||
+ ((newValue != null) && (newValue.equals(value))) ||
+ (value == null))
+ {
+ if (changeNumber.newer(deleteTime))
+ {
+ deleteTime = changeNumber;
+ }
+ AttributeType type = modAttr.getAttributeType();
+ if (!modifiedEntry.hasAttribute(type))
+ {
+ conflict = true;
+ modsIterator.remove();
+ }
+ else if ((newValue != null) &&
+ (!modifiedEntry.hasValue(type, modAttr.getOptions(), newValue)))
+ {
+ conflict = true;
+ modsIterator.remove();
+ }
+ else
+ {
+ lastMod = HistAttrModificationKey.DEL;
+ }
+ }
+ else
{
conflict = true;
modsIterator.remove();
}
- else if ((newValue != null) &&
- (!modifiedEntry.hasValue(type, modAttr.getOptions(), newValue)))
+ }
+ else if (changeNumber.equals(addTime))
+ {
+ if ((lastMod == HistAttrModificationKey.ADD)
+ || (lastMod == HistAttrModificationKey.REPL))
+ {
+ if (changeNumber.newer(deleteTime))
+ {
+ deleteTime = changeNumber;
+ }
+ lastMod = HistAttrModificationKey.DEL;
+ }
+ else
{
conflict = true;
modsIterator.remove();
@@ -166,8 +205,8 @@
}
else
{
- conflict = true;
- modsIterator.remove();
+ conflict = true;
+ modsIterator.remove();
}
break;
@@ -178,6 +217,7 @@
mod.setModificationType(ModificationType.REPLACE);
addTime = changeNumber;
value = newValue;
+ lastMod = HistAttrModificationKey.REPL;
}
else
{
@@ -187,18 +227,32 @@
// no conflict : don't do anything beside setting the addTime
addTime = changeNumber;
value = newValue;
+ lastMod = HistAttrModificationKey.ADD;
}
else
{
- conflict = true;
- modsIterator.remove();
+ // Case where changeNumber = addTime = deleteTime
+ if (changeNumber.equals(deleteTime)
+ && changeNumber.equals(addTime)
+ && (lastMod == HistAttrModificationKey.DEL))
+ {
+ // No conflict, record the new value.
+ value = newValue;
+ lastMod = HistAttrModificationKey.ADD;
+ }
+ else
+ {
+ conflict = true;
+ modsIterator.remove();
+ }
}
}
break;
case REPLACE:
- if ((changeNumber.older(deleteTime)) && (changeNumber.older(deleteTime)))
+ if ((changeNumber.older(deleteTime))
+ && ((addTime == null) || (changeNumber.older(addTime))))
{
conflict = true;
modsIterator.remove();
@@ -209,12 +263,14 @@
{
value = newValue;
deleteTime = changeNumber;
+ lastMod = HistAttrModificationKey.DEL;
}
else
{
addTime = changeNumber;
value = newValue;
deleteTime = changeNumber;
+ lastMod = HistAttrModificationKey.REPL;
}
}
break;
diff --git a/opends/tests/unit-tests-testng/src/server/org/opends/server/replication/plugin/ModifyConflictTest.java b/opends/tests/unit-tests-testng/src/server/org/opends/server/replication/plugin/ModifyConflictTest.java
index f4b5afc..c8703d4 100644
--- a/opends/tests/unit-tests-testng/src/server/org/opends/server/replication/plugin/ModifyConflictTest.java
+++ b/opends/tests/unit-tests-testng/src/server/org/opends/server/replication/plugin/ModifyConflictTest.java
@@ -61,7 +61,8 @@
* attributes - conflict with single-valued attributes - conflict with
* options - conflict with binary attributes - Replace, add, delete
* attribute, delete attribute value - conflict on the objectclass
- * attribute
+ * attribute.
+ * Added tests for multiple mods on same attribute in the same modify operation.
*/
public class ModifyConflictTest
@@ -915,7 +916,7 @@
Attribute attr = Attributes.create(DESCRIPTION, "init value");
Modification mod1 = new Modification(ModificationType.DELETE, attr);
-
+
Attribute attr2 = Attributes.empty(DESCRIPTION);
Modification mod2 = new Modification(ModificationType.REPLACE, attr2);
@@ -931,12 +932,12 @@
"DEL one value, del by Replace of the same attribute was not correct");
assertEquals(mods.get(1), mod2,
"DEL one value, del by Replace of the same attribute was not correct");
-
+
// Replay the same modifs again
replayModifies(entry, hist, mods2, 12);
assertEquals(mods2.size(), 2,
"DEL one value, del by Replace of the same attribute was not correct");
-
+
}
/**
@@ -1331,12 +1332,12 @@
private void replayModifies(
Entry entry, EntryHistorical hist, List<Modification> mods, int date)
{
- InternalClientConnection connection =
+ InternalClientConnection aConnection =
InternalClientConnection.getRootConnection();
ChangeNumber t = new ChangeNumber(date, 0, 0);
ModifyOperationBasis modOpBasis =
- new ModifyOperationBasis(connection, 1, 1, null, entry.getDN(), mods);
+ new ModifyOperationBasis(aConnection, 1, 1, null, entry.getDN(), mods);
LocalBackendModifyOperation modOp = new LocalBackendModifyOperation(modOpBasis);
ModifyContext ctx = new ModifyContext(t, "uniqueId");
modOp.setAttachment(SYNCHROCONTEXT, ctx);
@@ -1351,7 +1352,7 @@
DirectoryServer.getSchema().getAttributeType(
EntryHistorical.HISTORICALATTRIBUTENAME);
- InternalClientConnection connection =
+ InternalClientConnection aConnection =
InternalClientConnection.getRootConnection();
ChangeNumber t = new ChangeNumber(date, 0, 0);
@@ -1359,7 +1360,7 @@
mods.add(mod);
ModifyOperationBasis modOpBasis =
- new ModifyOperationBasis(connection, 1, 1, null, entry.getDN(), mods);
+ new ModifyOperationBasis(aConnection, 1, 1, null, entry.getDN(), mods);
LocalBackendModifyOperation modOp = new LocalBackendModifyOperation(modOpBasis);
ModifyContext ctx = new ModifyContext(t, "uniqueId");
modOp.setAttachment(SYNCHROCONTEXT, ctx);
@@ -1368,7 +1369,7 @@
if (mod.getModificationType() == ModificationType.ADD)
{
AddOperationBasis addOpBasis =
- new AddOperationBasis(connection, 1, 1, null, entry
+ new AddOperationBasis(aConnection, 1, 1, null, entry
.getDN(), entry.getObjectClasses(), entry.getUserAttributes(),
entry.getOperationalAttributes());
LocalBackendAddOperation addOp = new LocalBackendAddOperation(addOpBasis);
@@ -1448,4 +1449,398 @@
}
}
}
+
+ /**
+ * Test that a single replicated modify operation, that contains a
+ * modify-add of a value followed by modify-delete of that value
+ * is handled properly.
+ */
+ @Test()
+ public void addDeleteSameOpSingle() throws Exception
+ {
+ Entry entry = initializeEntry();
+
+ // load historical from the entry
+ EntryHistorical hist = EntryHistorical.newInstanceFromEntry(entry);
+
+ /*
+ * Add at time t1 that the previous delete. The
+ * conflict resolution should detect that this add must be ignored.
+ */
+ testModify(entry, hist, DISPLAYNAME, ModificationType.ADD,
+ "aValue", 1, true);
+
+ /*
+ * simulate a delete of same value in the same operation done at time
+ * t1
+ */
+ testModify(entry, hist, DISPLAYNAME, ModificationType.DELETE, "aValue", 1,
+ true);
+
+ /* The entry should have no value */
+ List<Attribute> attrs = entry.getAttribute(DISPLAYNAME);
+ assertNull(attrs);
+ }
+
+
+ /**
+ * Test that a single replicated modify operation, that contains a
+ * modify-add of a value followed by modify-delete of the attribute
+ * is handled properly.
+ */
+ @Test()
+ public void addDeleteAttrSameOpSingle() throws Exception
+ {
+ Entry entry = initializeEntry();
+
+ // load historical from the entry
+ EntryHistorical hist = EntryHistorical.newInstanceFromEntry(entry);
+
+ /*
+ * Add at time t1 that the previous delete. The
+ * conflict resolution should detect that this add must be ignored.
+ */
+ testModify(entry, hist, DISPLAYNAME, ModificationType.ADD,
+ "aValue", 1, true);
+
+ /*
+ * simulate a delete of the attribute in the same operation done at time
+ * t1
+ */
+ testModify(entry, hist, DISPLAYNAME, ModificationType.DELETE, null, 1,
+ true);
+
+ /* The entry should have no value */
+ List<Attribute> attrs = entry.getAttribute(DISPLAYNAME);
+ assertNull(attrs);
+ }
+
+ /**
+ * Test that the replay of a single replicated modify operation,
+ * that contains a modify-add of a value followed by modify-delete
+ * of the same value is handled properly.
+ */
+ @Test()
+ public void replayAddDeleteSameOpSingle() throws Exception
+ {
+ Entry entry = initializeEntry();
+
+ // load historical from the entry
+ EntryHistorical hist = EntryHistorical.newInstanceFromEntry(entry);
+
+ /*
+ * Add at time t1 that the previous delete. The
+ * conflict resolution should detect that this add must be ignored.
+ */
+ testModify(entry, hist, DISPLAYNAME, ModificationType.ADD,
+ "aValue", 2, true);
+
+ /*
+ * simulate a delete of the attribute in the same operation done at time
+ * t1
+ */
+ testModify(entry, hist, DISPLAYNAME, ModificationType.DELETE, "aValue", 2,
+ true);
+
+ /*
+ * Redo the same operations. This time, we expect them not to be applied.
+ */
+ testModify(entry, hist, DISPLAYNAME, ModificationType.ADD,
+ "aValue", 2, true);
+
+ /*
+ * simulate a delete of the attribute in the same operation done at time
+ * t1
+ */
+ testModify(entry, hist, DISPLAYNAME, ModificationType.DELETE, "aValue", 2,
+ true);
+
+ /* The entry should have no value */
+ List<Attribute> attrs = entry.getAttribute(DISPLAYNAME);
+ assertNull(attrs);
+ }
+
+ /**
+ * Test that the replay of a single replicated modify operation,
+ * that contains a modify-add of a value followed by modify-delete
+ * of the attribute is handled properly.
+ */
+ @Test()
+ public void replayAddDeleteAttrSameOpSingle() throws Exception
+ {
+ Entry entry = initializeEntry();
+
+ // load historical from the entry
+ EntryHistorical hist = EntryHistorical.newInstanceFromEntry(entry);
+
+ /*
+ * Add at time t1 that the previous delete. The
+ * conflict resolution should detect that this add must be ignored.
+ */
+ testModify(entry, hist, DISPLAYNAME, ModificationType.ADD,
+ "aValue", 1, true);
+
+ /*
+ * simulate a delete of the attribute in the same operation done at time
+ * t1
+ */
+ testModify(entry, hist, DISPLAYNAME, ModificationType.DELETE, null, 1,
+ true);
+
+ /*
+ * Redo the same operations. This time, we expect them not to be applied.
+ */
+ testModify(entry, hist, DISPLAYNAME, ModificationType.ADD,
+ "aValue", 1, true);
+
+ /*
+ * simulate a delete of the attribute in the same operation done at time
+ * t1
+ */
+ testModify(entry, hist, DISPLAYNAME, ModificationType.DELETE, null, 1,
+ true);
+
+ /* The entry should have no value */
+ List<Attribute> attrs = entry.getAttribute(DISPLAYNAME);
+ assertNull(attrs);
+ }
+
+
+ /**
+ * Test that a single replicated modify operation, that contains a
+ * modify-replace of a value followed by modify-delete of that value
+ * is handled properly.
+ */
+ @Test()
+ public void replaceDeleteSameOpSingle() throws Exception
+ {
+ Entry entry = initializeEntry();
+
+ // load historical from the entry
+ EntryHistorical hist = EntryHistorical.newInstanceFromEntry(entry);
+
+ /*
+ * Add at time t1 that the previous delete. The
+ * conflict resolution should detect that this add must be ignored.
+ */
+ testModify(entry, hist, DISPLAYNAME, ModificationType.REPLACE,
+ "aValue", 1, true);
+
+ /*
+ * simulate a delete of same value in the same operation done at time
+ * t1
+ */
+ testModify(entry, hist, DISPLAYNAME, ModificationType.DELETE, "aValue", 1,
+ true);
+
+ /* The entry should have no value */
+ List<Attribute> attrs = entry.getAttribute(DISPLAYNAME);
+ assertNull(attrs);
+ }
+
+
+ /**
+ * Test that a single replicated modify operation, that contains a
+ * modify-replace of a value followed by modify-delete of the attribute
+ * is handled properly.
+ */
+ @Test()
+ public void replaceDeleteAttrSameOpSingle() throws Exception
+ {
+ Entry entry = initializeEntry();
+
+ // load historical from the entry
+ EntryHistorical hist = EntryHistorical.newInstanceFromEntry(entry);
+
+ /*
+ * Add at time t1 that the previous delete. The
+ * conflict resolution should detect that this add must be ignored.
+ */
+ testModify(entry, hist, DISPLAYNAME, ModificationType.REPLACE,
+ "aValue", 1, true);
+
+ /*
+ * simulate a delete of the attribute in the same operation done at time
+ * t1
+ */
+ testModify(entry, hist, DISPLAYNAME, ModificationType.DELETE, null, 1,
+ true);
+
+ /* The entry should have no value */
+ List<Attribute> attrs = entry.getAttribute(DISPLAYNAME);
+ assertNull(attrs);
+ }
+
+ /**
+ * Test that the replay of a single replicated modify operation,
+ * that contains a modify-replace of a value followed by modify-delete
+ * of the same value is handled properly.
+ */
+ @Test()
+ public void replayReplaceDeleteSameOpSingle() throws Exception
+ {
+ Entry entry = initializeEntry();
+
+ // load historical from the entry
+ EntryHistorical hist = EntryHistorical.newInstanceFromEntry(entry);
+
+ /*
+ * Add at time t1 that the previous delete. The
+ * conflict resolution should detect that this add must be ignored.
+ */
+ testModify(entry, hist, DISPLAYNAME, ModificationType.REPLACE,
+ "aValue", 2, true);
+
+ /*
+ * simulate a delete of the attribute in the same operation done at time
+ * t1
+ */
+ testModify(entry, hist, DISPLAYNAME, ModificationType.DELETE, "aValue", 2,
+ true);
+
+ /*
+ * Redo the same operations. This time, we expect them not to be applied.
+ */
+ testModify(entry, hist, DISPLAYNAME, ModificationType.REPLACE,
+ "aValue", 2, true);
+
+ /*
+ * simulate a delete of the attribute in the same operation done at time
+ * t1
+ */
+ testModify(entry, hist, DISPLAYNAME, ModificationType.DELETE, "aValue", 2,
+ true);
+
+ /* The entry should have no value */
+ List<Attribute> attrs = entry.getAttribute(DISPLAYNAME);
+ assertNull(attrs);
+ }
+
+ /**
+ * Test that the replay of a single replicated modify operation,
+ * that contains a modify-replace of a value followed by modify-delete
+ * of the attribute is handled properly.
+ */
+ @Test()
+ public void replayReplaceDeleteAttrSameOpSingle() throws Exception
+ {
+ Entry entry = initializeEntry();
+
+ // load historical from the entry
+ EntryHistorical hist = EntryHistorical.newInstanceFromEntry(entry);
+
+ /*
+ * Add at time t1 that the previous delete. The
+ * conflict resolution should detect that this add must be ignored.
+ */
+ testModify(entry, hist, DISPLAYNAME, ModificationType.REPLACE,
+ "aValue", 1, true);
+
+ /*
+ * simulate a delete of the attribute in the same operation done at time
+ * t1
+ */
+ testModify(entry, hist, DISPLAYNAME, ModificationType.DELETE, null, 1,
+ true);
+
+ /*
+ * Redo the same operations. This time, we expect them not to be applied.
+ */
+ testModify(entry, hist, DISPLAYNAME, ModificationType.REPLACE,
+ "aValue", 1, true);
+
+ /*
+ * simulate a delete of the attribute in the same operation done at time
+ * t1
+ */
+ testModify(entry, hist, DISPLAYNAME, ModificationType.DELETE, null, 1,
+ true);
+
+ /* The entry should have no value */
+ List<Attribute> attrs = entry.getAttribute(DISPLAYNAME);
+ assertNull(attrs);
+ }
+
+
+ /**
+ * Test that a single replicated modify operation, that contains a
+ * modify-replace of a value followed by modify-delete of that value,
+ * followed by a modify-add of a new value is handled properly.
+ */
+ @Test()
+ public void replaceDeleteAddSameOpSingle() throws Exception
+ {
+ Entry entry = initializeEntry();
+
+ // load historical from the entry
+ EntryHistorical hist = EntryHistorical.newInstanceFromEntry(entry);
+
+ /*
+ * Add at time t1 that the previous delete. The
+ * conflict resolution should detect that this add must be ignored.
+ */
+ testModify(entry, hist, DISPLAYNAME, ModificationType.REPLACE,
+ "aValue", 1, true);
+
+ /*
+ * simulate a delete of same value in the same operation done at time
+ * t1
+ */
+ testModify(entry, hist, DISPLAYNAME, ModificationType.DELETE, "aValue", 1,
+ true);
+ /*
+ * simulate an add of new value in the same operation done at time
+ * t1
+ */
+ testModify(entry, hist, DISPLAYNAME, ModificationType.ADD, "NewValue", 1,
+ true);
+
+ /* The entry should have one value */
+ List<Attribute> attrs = entry.getAttribute(DISPLAYNAME);
+ Attribute attr = attrs.get(0);
+ assertEquals(1, attr.size());
+ attr.contains(AttributeValues.create(attr.getAttributeType(), "NewValue"));
+ }
+
+ /**
+ * Test that a single replicated modify operation, that contains a
+ * modify-replace of a value followed by modify-delete of the attribute,
+ * followed by a modify-add of a new value is handled properly.
+ */
+ @Test()
+ public void replaceDeleteAttrAddSameOpSingle() throws Exception
+ {
+ Entry entry = initializeEntry();
+
+ // load historical from the entry
+ EntryHistorical hist = EntryHistorical.newInstanceFromEntry(entry);
+
+ /*
+ * Add at time t1 that the previous delete. The
+ * conflict resolution should detect that this add must be ignored.
+ */
+ testModify(entry, hist, DISPLAYNAME, ModificationType.REPLACE,
+ "aValue", 1, true);
+
+ /*
+ * simulate a delete of same value in the same operation done at time
+ * t1
+ */
+ testModify(entry, hist, DISPLAYNAME, ModificationType.DELETE, null, 1,
+ true);
+ /*
+ * simulate an add of new value in the same operation done at time
+ * t1
+ */
+ testModify(entry, hist, DISPLAYNAME, ModificationType.ADD, "NewValue", 1,
+ true);
+
+ /* The entry should have no value */
+ List<Attribute> attrs = entry.getAttribute(DISPLAYNAME);
+ Attribute attr = attrs.get(0);
+ assertEquals(1, attr.size());
+ attr.contains(AttributeValues.create(attr.getAttributeType(), "NewValue"));
+ }
+
+
}
--
Gitblit v1.10.0