From 9bce02d6f4ad4d8522bac50f5c603efbb57b0d7e Mon Sep 17 00:00:00 2001
From: ludovicp <ludovicp@localhost>
Date: Thu, 27 May 2010 13:08:02 +0000
Subject: [PATCH] fix issue #4529 - The conflict resolution code was not correctly handling the case when two mods (one add and one del) are done in the same modify operation. This code contains unit tests case for this problem and fix the conflict resolution code.

---
 opendj-sdk/opends/src/server/org/opends/server/replication/plugin/AttrInfoMultiple.java                           |   19 ++++++---
 opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/replication/plugin/ModifyConflictTest.java |  100 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 112 insertions(+), 7 deletions(-)

diff --git a/opendj-sdk/opends/src/server/org/opends/server/replication/plugin/AttrInfoMultiple.java b/opendj-sdk/opends/src/server/org/opends/server/replication/plugin/AttrInfoMultiple.java
index a65d036..d78bef1 100644
--- a/opendj-sdk/opends/src/server/org/opends/server/replication/plugin/AttrInfoMultiple.java
+++ b/opendj-sdk/opends/src/server/org/opends/server/replication/plugin/AttrInfoMultiple.java
@@ -22,7 +22,7 @@
  * CDDL HEADER END
  *
  *
- *      Copyright 2006-2008 Sun Microsystems, Inc.
+ *      Copyright 2006-2010 Sun Microsystems, Inc.
  */
 package org.opends.server.replication.plugin;
 
@@ -465,6 +465,7 @@
       for (AttributeValue val : modAttr)
       {
         Boolean deleteIt = true;  // true if the delete must be done
+        Boolean addedInCurrentOp = false;
 
         /* update historical information */
         ValueInfo valInfo = new ValueInfo(val, null, changeNumber);
@@ -473,8 +474,14 @@
         {
           /* this value already exist in the historical information */
           ValueInfo oldValInfo  = valuesInfo.get(index);
-          if (changeNumber.newer(oldValInfo.getValueDeleteTime()) &&
-              changeNumber.newer(oldValInfo.getValueUpdateTime()))
+          if (changeNumber.equals(oldValInfo.getValueUpdateTime()))
+          {
+            // This value was added earlier in the same operation
+            // we need to keep the delete.
+            addedInCurrentOp = true;
+          }
+          if (changeNumber.newerOrEquals(oldValInfo.getValueDeleteTime()) &&
+              changeNumber.newerOrEquals(oldValInfo.getValueUpdateTime()))
           {
             valuesInfo.remove(index);
             valuesInfo.add(valInfo);
@@ -494,8 +501,8 @@
          * MOD to make sure the delete is going to succeed
          */
         if (!deleteIt
-            || !modifiedEntry.hasValue(modAttr.getAttributeType(), modAttr
-                .getOptions(), val))
+            || (!modifiedEntry.hasValue(modAttr.getAttributeType(), modAttr
+                .getOptions(), val) && ! addedInCurrentOp))
         {
           // this value was already deleted before and therefore
           // this should not be replayed.
@@ -587,7 +594,7 @@
           /* this value is marked as a deleted value
            * check if this mod is more recent the this delete
            */
-          if (changeNumber.newer(oldValueInfo.getValueDeleteTime()))
+          if (changeNumber.newerOrEquals(oldValueInfo.getValueDeleteTime()))
           {
             /* this add is more recent,
              * remove the old delete historical information
diff --git a/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/replication/plugin/ModifyConflictTest.java b/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/replication/plugin/ModifyConflictTest.java
index 1362f56..1d61470 100644
--- a/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/replication/plugin/ModifyConflictTest.java
+++ b/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/replication/plugin/ModifyConflictTest.java
@@ -22,7 +22,7 @@
  * CDDL HEADER END
  *
  *
- *      Copyright 2006-2009 Sun Microsystems, Inc.
+ *      Copyright 2006-2010 Sun Microsystems, Inc.
  */
 package org.opends.server.replication.plugin;
 
@@ -843,6 +843,87 @@
   }
 
   /**
+   * Test that a DEL and ADD of the same attribute same value done
+   * in a single operation correctly results in the value being kept.
+   *
+   *  This is not a conflict in the strict definition but does exert
+   *  the conflict resolution code.
+   */
+  @Test()
+  public void delAndAddSameOp() throws Exception
+  {
+    Entry entry = initializeEntry();
+
+    // load historical from the entry
+    Historical hist = Historical.load(entry);
+
+    /*
+     * simulate a add of the description attribute done at time t10
+     */
+    testModify(entry, hist, DESCRIPTION, ModificationType.ADD,
+        "init value", 10, true);
+    /*
+     * Now simulate a del and a add in the same operation
+     */
+
+    Attribute attr = Attributes.create(DESCRIPTION, "init value");
+    Modification mod1 = new Modification(ModificationType.DELETE, attr);
+
+    attr = Attributes.create(DESCRIPTION, "Init Value");
+    Modification mod2 = new Modification(ModificationType.ADD, attr);
+
+    List<Modification> mods = new LinkedList<Modification>();
+    mods.add(mod1);
+    mods.add(mod2);
+
+    replayModifies(entry, hist, mods, 11);
+    assertEquals(mods.size(), 2,
+      "DEL and ADD of the same attribute same value was not correct");
+    assertEquals(mods.get(0), mod1,
+      "DEL and ADD of the same attribute same value was not correct");
+    assertEquals(mods.get(1), mod2,
+      "DEL and ADD of the same attribute same value was not correct");
+  }
+
+  /**
+   * Test that a ADD and DEL of the same attribute same value done
+   * in a single operation correctly results in the value being kept.
+   *
+   * This is not a conflict in the strict definition but does exert
+   *  the conflict resolution code.
+   */
+  @Test()
+  public void AddAndDelSameOp() throws Exception
+  {
+    Entry entry = initializeEntry();
+
+    // load historical from the entry
+    Historical hist = Historical.load(entry);
+
+    /*
+     * Now simulate a del and a add in the same operation
+     */
+
+    Attribute attr = Attributes.create(DESCRIPTION, "init value");
+    Modification mod1 = new Modification(ModificationType.ADD, attr);
+
+    attr = Attributes.create(DESCRIPTION, "Init Value");
+    Modification mod2 = new Modification(ModificationType.DELETE, attr);
+
+    List<Modification> mods = new LinkedList<Modification>();
+    mods.add(mod1);
+    mods.add(mod2);
+
+    replayModifies(entry, hist, mods, 11);
+    assertEquals(mods.size(), 2,
+      "DEL and ADD of the same attribute same value was not correct");
+    assertEquals(mods.get(0), mod1,
+      "DEL and ADD of the same attribute same value was not correct");
+    assertEquals(mods.get(1), mod2,
+      "DEL and ADD of the same attribute same value was not correct");
+  }
+
+  /**
    * Test that conflict between two modify-add with for
    * multi-valued attributes are handled correctly when some of the values
    * are the same :
@@ -1190,6 +1271,23 @@
   /**
    *
    */
+
+  private void replayModifies(
+      Entry entry, Historical hist, List<Modification> mods, int date)
+  {
+    InternalClientConnection connection =
+      InternalClientConnection.getRootConnection();
+    ChangeNumber t = new ChangeNumber(date, 0, 0);
+
+    ModifyOperationBasis modOpBasis =
+      new ModifyOperationBasis(connection, 1, 1, null, entry.getDN(), mods);
+    LocalBackendModifyOperation modOp = new LocalBackendModifyOperation(modOpBasis);
+    ModifyContext ctx = new ModifyContext(t, "uniqueId");
+    modOp.setAttachment(SYNCHROCONTEXT, ctx);
+
+    hist.replayOperation(modOp, entry);
+  }
+
   private List<Modification> replayModify(
       Entry entry, Historical hist, Modification mod, int date)
   {

--
Gitblit v1.10.0