From d11926a41394b9bc2f3669078947db3ddc9d0e2c Mon Sep 17 00:00:00 2001
From: Jean-Noël Rouvignac <jn.rouvignac@gmail.com>
Date: Mon, 31 Aug 2015 08:49:43 +0000
Subject: [PATCH] Preliminary OPENDJ-1192 (PR #8) Modify request replay failures

---
 opendj-server-legacy/src/test/java/org/opends/server/replication/plugin/AttrHistoricalMultipleTest.java |   83 +++++++++++----------------
 opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/AttrHistorical.java             |    6 -
 opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/AttrHistoricalMultiple.java     |   48 +++++++++-------
 3 files changed, 63 insertions(+), 74 deletions(-)

diff --git a/opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/AttrHistorical.java b/opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/AttrHistorical.java
index 4711415..b7054a2 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/AttrHistorical.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/AttrHistorical.java
@@ -44,13 +44,11 @@
    * It should use whatever historical information is stored in this class
    * to solve the conflict and modify the mod and the mods iterator accordingly
    *
-   * @param modsIterator  The iterator on the mods from which the mod is
-   *                      extracted.
+   * @param modsIterator  The iterator on the mods from which the mod is extracted.
    * @param csn  The CSN associated to the operation.
    * @param modifiedEntry The entry modified by this operation.
    * @param mod           The modification.
-   *
-   * @return a boolean indicating if a conflict was detected.
+   * @return {@code true} if a conflict was detected, {@code false} otherwise.
    */
   public abstract boolean replayOperation(
       Iterator<Modification> modsIterator, CSN csn, Entry modifiedEntry, Modification mod);
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/AttrHistoricalMultiple.java b/opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/AttrHistoricalMultiple.java
index 036ea9e..5c9b609 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/AttrHistoricalMultiple.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/AttrHistoricalMultiple.java
@@ -241,18 +241,18 @@
            * skip this mod
            */
           modsIterator.remove();
-          break;
+          return true;
         }
 
         if (!conflictDelete(csn, m, modifiedEntry))
         {
           modsIterator.remove();
+          return true;
         }
-        break;
+        return false;
 
       case ADD:
-        conflictAdd(csn, m, modsIterator);
-        break;
+        return conflictAdd(csn, m, modsIterator);
 
       case REPLACE:
         if (csn.isOlderThan(getDeleteTime()))
@@ -261,15 +261,14 @@
            * skip this mod
            */
           modsIterator.remove();
-          break;
+          return true;
         }
 
-        /* save the values that are added by the replace operation
-         * into addedValues
-         * first process the replace as a delete operation -> this generate
-         * a list of values that should be kept
-         * then process the addedValues as if they were coming from a add
-         * -> this generate the list of values that needs to be added
+        /* save the values that are added by the replace operation into addedValues
+         * first process the replace as a delete operation
+         * -> this generates a list of values that should be kept
+         * then process the addedValues as if they were coming from an add
+         * -> this generates the list of values that needs to be added
          * concatenate the 2 generated lists into a replace
          */
         Attribute addedValues = m.getAttribute();
@@ -284,13 +283,15 @@
         AttributeBuilder builder = new AttributeBuilder(keptValues);
         builder.addAll(m.getAttribute());
         m.setAttribute(builder.toAttribute());
-        break;
+        return false;
 
       case INCREMENT:
         // TODO : FILL ME
-        break;
+        return false;
+
+      default:
+        return false;
       }
-      return true;
     }
     else
     {
@@ -485,14 +486,17 @@
   }
 
   /**
-   * Process a add attribute values that is conflicting with a previous
-   * modification.
+   * Process a add attribute values that is conflicting with a previous modification.
    *
-   * @param csn  the historical info associated to the entry
-   * @param m the modification that is being processed
-   * @param modsIterator iterator on the list of modification
+   * @param csn
+   *          the historical info associated to the entry
+   * @param m
+   *          the modification that is being processed
+   * @param modsIterator
+   *          iterator on the list of modification
+   * @return {@code true} if a conflict was detected, {@code false} otherwise.
    */
-  private void conflictAdd(CSN csn, Modification m, Iterator<Modification> modsIterator)
+  private boolean conflictAdd(CSN csn, Modification m, Iterator<Modification> modsIterator)
   {
     /*
      * if historicalattributedelete is newer forget this mod else find
@@ -510,7 +514,7 @@
        * forget this MOD ADD
        */
       modsIterator.remove();
-      return;
+      return true;
     }
 
     AttributeBuilder builder = new AttributeBuilder(m.getAttribute());
@@ -576,12 +580,14 @@
     if (attr.isEmpty())
     {
       modsIterator.remove();
+      return true;
     }
 
     if (csn.isNewerThan(getLastUpdateTime()))
     {
       lastUpdateTime = csn;
     }
+    return false;
   }
 
   @Override
diff --git a/opendj-server-legacy/src/test/java/org/opends/server/replication/plugin/AttrHistoricalMultipleTest.java b/opendj-server-legacy/src/test/java/org/opends/server/replication/plugin/AttrHistoricalMultipleTest.java
index 61f3918..8d503c4 100644
--- a/opendj-server-legacy/src/test/java/org/opends/server/replication/plugin/AttrHistoricalMultipleTest.java
+++ b/opendj-server-legacy/src/test/java/org/opends/server/replication/plugin/AttrHistoricalMultipleTest.java
@@ -58,24 +58,9 @@
 @SuppressWarnings("javadoc")
 public class AttrHistoricalMultipleTest extends ReplicationTestCase
 {
-  private static enum E
-  {
-    CONFLICT(true), CONFLICT_BUT_SHOULD_NOT_BE(true), SUCCESS(false);
-
-    private final boolean expectedConflictStatus;
-
-    private E(boolean expectedResultForReplay)
-    {
-      this.expectedConflictStatus = expectedResultForReplay;
-    }
-
-    private boolean getExpectedResult()
-    {
-      return this.expectedConflictStatus;
-    }
-  }
-
   private static final String ATTRIBUTE_NAME = "description";
+  private static final boolean CONFLICT = true;
+  private static final boolean SUCCESS = false;
 
   private CSNGenerator csnGen = new CSNGenerator(1025, System.currentTimeMillis());
   private AttrHistoricalMultiple attrHist;
@@ -177,11 +162,11 @@
   public void replay_addDeleteSameTime() throws Exception
   {
     mod = newModification(ADD, "X");
-    replayOperation(csn, entry, mod, E.CONFLICT_BUT_SHOULD_NOT_BE);
+    replayOperation(csn, entry, mod, SUCCESS);
     assertAttributeValues(entry, "X");
 
     mod = newModification(DELETE, "X");
-    replayOperation(csn, entry, mod, E.CONFLICT_BUT_SHOULD_NOT_BE);
+    replayOperation(csn, entry, mod, SUCCESS);
     assertNoAttributeValue(entry);
   }
 
@@ -191,15 +176,15 @@
     CSN[] t = newCSNs(3);
 
     mod = newModification(ADD, "X");
-    replayOperation(t[0], entry, mod, E.CONFLICT_BUT_SHOULD_NOT_BE);
+    replayOperation(t[0], entry, mod, SUCCESS);
     assertAttributeValues(entry, "X");
 
     mod = newModification(ADD, "Y");
-    replayOperation(t[2], entry, mod, E.CONFLICT_BUT_SHOULD_NOT_BE);
+    replayOperation(t[2], entry, mod, SUCCESS);
     assertAttributeValues(entry, "X", "Y");
 
     mod = newModification(DELETE, "Y");
-    replayOperationSuppressMod(t[1], entry, mod, E.CONFLICT);
+    replayOperationSuppressMod(t[1], entry, mod, CONFLICT);
     assertAttributeValues(entry, "X", "Y");
   }
 
@@ -211,7 +196,7 @@
     replay_addDeleteNoValue(t[0], t[2]);
 
     mod = newModification(ADD, "Y");
-    replayOperationSuppressMod(t[1], entry, mod, E.CONFLICT);
+    replayOperationSuppressMod(t[1], entry, mod, CONFLICT);
     assertNoAttributeValue(entry);
   }
 
@@ -221,15 +206,15 @@
     CSN[] t = newCSNs(3);
 
     mod = newModification(ADD, "X");
-    replayOperation(t[0], entry, mod, E.CONFLICT_BUT_SHOULD_NOT_BE);
+    replayOperation(t[0], entry, mod, SUCCESS);
     assertAttributeValues(entry, "X");
 
     mod = newModification(DELETE, "X");
-    replayOperation(t[2], entry, mod, E.CONFLICT_BUT_SHOULD_NOT_BE);
+    replayOperation(t[2], entry, mod, SUCCESS);
     assertNoAttributeValue(entry);
 
     mod = newModification(ADD, "X");
-    replayOperationSuppressMod(t[1], entry, mod, E.CONFLICT);
+    replayOperationSuppressMod(t[1], entry, mod, CONFLICT);
     assertNoAttributeValue(entry);
   }
 
@@ -239,11 +224,11 @@
     CSN[] t = newCSNs(2);
 
     mod = newModification(ADD, "X");
-    replayOperation(t[0], entry, mod, E.CONFLICT_BUT_SHOULD_NOT_BE);
+    replayOperation(t[0], entry, mod, SUCCESS);
     assertAttributeValues(entry, "X");
 
     mod = newModification(ADD, "X");
-    replayOperationSuppressMod(t[1], entry, mod, E.CONFLICT);
+    replayOperationSuppressMod(t[1], entry, mod, CONFLICT);
     assertAttributeValues(entry, "X");
   }
 
@@ -253,15 +238,15 @@
     CSN[] t = newCSNs(3);
 
     mod = newModification(ADD, "X");
-    replayOperation(t[0], entry, mod, E.CONFLICT_BUT_SHOULD_NOT_BE);
+    replayOperation(t[0], entry, mod, SUCCESS);
     assertAttributeValues(entry, "X");
 
     mod = newModification(DELETE, "X");
-    replayOperation(t[1], entry, mod, E.CONFLICT_BUT_SHOULD_NOT_BE);
+    replayOperation(t[1], entry, mod, SUCCESS);
     assertNoAttributeValue(entry);
 
     mod = newModification(ADD, "X");
-    replayOperation(t[1], entry, mod, E.CONFLICT_BUT_SHOULD_NOT_BE);
+    replayOperation(t[1], entry, mod, SUCCESS);
     assertAttributeValues(entry, "X");
   }
 
@@ -269,7 +254,7 @@
   public void replay_deleteNoPreviousHistory() throws Exception
   {
     mod = newModification(DELETE, "Y");
-    replayOperationSuppressMod(csn, entry, mod, E.CONFLICT);
+    replayOperationSuppressMod(csn, entry, mod, CONFLICT);
     assertNoAttributeValue(entry);
   }
 
@@ -279,11 +264,11 @@
     CSN[] t = newCSNs(2);
 
     mod = newModification(ADD, "X");
-    replayOperation(t[0], entry, mod, E.CONFLICT_BUT_SHOULD_NOT_BE);
+    replayOperation(t[0], entry, mod, SUCCESS);
     assertAttributeValues(entry, "X");
 
     mod = newModification(DELETE, "X");
-    replayOperation(t[1], entry, mod, E.CONFLICT_BUT_SHOULD_NOT_BE);
+    replayOperation(t[1], entry, mod, SUCCESS);
     assertNoAttributeValue(entry);
   }
 
@@ -295,7 +280,7 @@
     replay_addDeleteNoValue(t[0], t[2]);
 
     mod = newModification(DELETE, "X");
-    replayOperationSuppressMod(t[1], entry, mod, E.CONFLICT);
+    replayOperationSuppressMod(t[1], entry, mod, CONFLICT);
     assertNoAttributeValue(entry);
   }
 
@@ -315,11 +300,11 @@
   private void replay_addDeleteNoValue(CSN tAdd, CSN tDel) throws Exception
   {
     mod = newModification(ADD, "X");
-    replayOperation(tAdd, entry, mod, E.CONFLICT_BUT_SHOULD_NOT_BE);
+    replayOperation(tAdd, entry, mod, SUCCESS);
     assertAttributeValues(entry, "X");
 
     mod = newModification(DELETE);
-    replayOperation(tDel, entry, mod, E.CONFLICT_BUT_SHOULD_NOT_BE);
+    replayOperation(tDel, entry, mod, SUCCESS);
     assertNoAttributeValue(entry);
   }
 
@@ -327,7 +312,7 @@
   public void replay_replace() throws Exception
   {
     mod = newModification(REPLACE, "X");
-    replayOperation(csn, entry, mod, E.SUCCESS);
+    replayOperation(csn, entry, mod, SUCCESS);
     assertAttributeValues(entry, "X");
   }
 
@@ -337,11 +322,11 @@
     CSN[] t = newCSNs(2);
 
     mod = newModification(ADD, "X");
-    replayOperation(t[1], entry, mod, E.CONFLICT_BUT_SHOULD_NOT_BE);
+    replayOperation(t[1], entry, mod, SUCCESS);
     assertAttributeValues(entry, "X");
 
     mod = newModification(REPLACE, "Y");
-    replayOperation(t[0], entry, mod, E.CONFLICT_BUT_SHOULD_NOT_BE);
+    replayOperation(t[0], entry, mod, SUCCESS);
     assertAttributeValues(entry, "X", "Y");
   }
 
@@ -353,7 +338,7 @@
     replay_addDeleteNoValue(t[0], t[2]);
 
     mod = newModification(REPLACE, "Y");
-    replayOperationSuppressMod(t[1], entry, mod, E.CONFLICT);
+    replayOperationSuppressMod(t[1], entry, mod, CONFLICT);
     assertNoAttributeValue(entry);
   }
 
@@ -377,28 +362,28 @@
     return new Modification(modType, Attributes.empty(ATTRIBUTE_NAME));
   }
 
-  private void replayOperationSuppressMod(CSN csn, Entry entry, Modification mod, E conflictStatus)
+  private void replayOperationSuppressMod(CSN csn, Entry entry, Modification mod, boolean shouldConflict)
       throws Exception
   {
     Iterator<Modification> itMod = mock(Iterator.class);
-    replayOperation(itMod, csn, entry, mod, conflictStatus);
+    replayOperation(itMod, csn, entry, mod, shouldConflict);
     verifyModSuppressed(itMod);
   }
 
-  private void replayOperation(CSN csn, Entry entry, Modification mod, E conflictStatus) throws Exception
+  private void replayOperation(CSN csn, Entry entry, Modification mod, boolean shouldConflict) throws Exception
   {
     Iterator<Modification> itMod = mock(Iterator.class);
-    replayOperation(itMod, csn, entry, mod, conflictStatus);
+    replayOperation(itMod, csn, entry, mod, shouldConflict);
     verifyZeroInteractions(itMod);
   }
 
   private void replayOperation(Iterator<Modification> modsIterator, CSN csn, Entry entry, Modification mod,
-      E conflictStatus) throws Exception
+      boolean shouldConflict) throws Exception
   {
     boolean result = attrHist.replayOperation(modsIterator, csn, entry, mod);
-    assertEquals(result, conflictStatus.getExpectedResult(),
-        "Expected " + (conflictStatus == E.CONFLICT ? "a" : "no") + " conflict when applying " + mod + " to " + entry);
-    if (entry != null && conflictStatus != E.CONFLICT)
+    assertEquals(result, shouldConflict,
+        "Expected " + (shouldConflict ? "a" : "no") + " conflict when applying " + mod + " to " + entry);
+    if (entry != null && !shouldConflict)
     {
       entry.applyModification(mod);
       assertAttributeValues(entry, mod);

--
Gitblit v1.10.0