From b6da5b01e2f66861a485ed7b48fa887410cecb69 Mon Sep 17 00:00:00 2001
From: gbellato <gbellato@localhost>
Date: Tue, 10 Oct 2006 12:57:52 +0000
Subject: [PATCH] Fix two null pointer Exception that can happen in the conflict resolution code when - replaying a modify operation on an entry that has already been deleted (778) - replaying a modify DN operation to rename an entry below a parent that does not exist anymore. (775)

---
 opends/src/server/org/opends/server/synchronization/ModifyDNMsg.java                                 |   20 +++
 opends/tests/unit-tests-testng/src/server/org/opends/server/synchronization/UpdateOperationTest.java |  158 +++++++++++++++++++++----
 opends/src/server/org/opends/server/synchronization/SynchronizationDomain.java                       |  129 +++++++++++---------
 3 files changed, 222 insertions(+), 85 deletions(-)

diff --git a/opends/src/server/org/opends/server/synchronization/ModifyDNMsg.java b/opends/src/server/org/opends/server/synchronization/ModifyDNMsg.java
index c794f4d..6436bc4 100644
--- a/opends/src/server/org/opends/server/synchronization/ModifyDNMsg.java
+++ b/opends/src/server/org/opends/server/synchronization/ModifyDNMsg.java
@@ -246,4 +246,24 @@
   {
     newSuperior = string;
   }
+
+  /**
+   * Get the new RDN of this operation.
+   *
+   * @return The new RDN of this operation.
+   */
+  public String getNewRDN()
+  {
+    return newRDN;
+  }
+
+  /**
+   * Set the new RDN of this operation.
+   * @param newRDN the new RDN of this operation.
+   */
+  public void setNewRDN(String newRDN)
+  {
+    this.newRDN = newRDN;
+  }
+
 }
diff --git a/opends/src/server/org/opends/server/synchronization/SynchronizationDomain.java b/opends/src/server/org/opends/server/synchronization/SynchronizationDomain.java
index d593969..a5da8cd 100644
--- a/opends/src/server/org/opends/server/synchronization/SynchronizationDomain.java
+++ b/opends/src/server/org/opends/server/synchronization/SynchronizationDomain.java
@@ -557,7 +557,8 @@
          * parent is the same as when the operation was performed.
          */
         String newParentId = findEntryId(modifyDNOperation.getNewSuperior());
-        if (!newParentId.equals(ctx.getNewParentId()))
+        if ((newParentId != null) &&
+            (!newParentId.equals(ctx.getNewParentId())))
         {
           modifyDNOperation.setResultCode(ResultCode.NO_SUCH_OBJECT);
           return new SynchronizationProviderResult(false);
@@ -1270,8 +1271,14 @@
        * search if the entry has been renamed, and return the new dn
        * of the entry.
        */
-      msg.setDn(findEntryDN(entryUid).toString());
-      return false;
+      DN newdn = findEntryDN(entryUid);
+      if (newdn != null)
+      {
+        msg.setDn(newdn.toString());
+        return false;
+      }
+      else
+        return true;
     }
     return true;
   }
@@ -1419,64 +1426,67 @@
     String entryUid = ctx.getEntryUid();
     String newSuperiorID = ctx.getNewParentId();
 
+    /*
+     * four possible cases :
+     * - the modified entry has been renamed
+     * - the new parent has been renamed
+     * - the operation is replayed for the second time.
+     * - the entry has been deleted
+     * action :
+     *  - change the target dn and the new parent dn and
+     *        restart the operation,
+     *  - don't do anything if the operation is replayed.
+     */
+
+    // Construct the new DN to use for the entry.
+    DN entryDN = op.getEntryDN();
+    DN newSuperior = findEntryDN(newSuperiorID);
+    RDN newRDN = op.getNewRDN();
+    DN parentDN;
+
+    if (newSuperior == null)
+    {
+      parentDN = entryDN.getParent();
+    }
+    else
+    {
+      parentDN = newSuperior;
+    }
+
+    if ((parentDN == null) || parentDN.isNullDN())
+    {
+      /* this should never happen
+       * can't solve any conflict in this case.
+       */
+      throw new Exception("operation parameters are invalid");
+    }
+
+    RDN[] parentComponents = parentDN.getRDNComponents();
+    RDN[] newComponents    = new RDN[parentComponents.length+1];
+    System.arraycopy(parentComponents, 0, newComponents, 1,
+        parentComponents.length);
+    newComponents[0] = newRDN;
+
+    DN newDN = new DN(newComponents);
+
+    // get the current DN of this entry in the database.
+    DN currentDN = findEntryDN(entryUid);
+
+    // if the newDN and the current DN match then the operation
+    // is a no-op (this was probably a second replay)
+    // don't do anything.
+    if (newDN.equals(currentDN))
+    {
+      return true;
+    }
+
     if (result == ResultCode.NO_SUCH_OBJECT)
     {
-      ModifyDNMsg modifyDnMsg = (ModifyDNMsg) msg;
-
       /*
-       * four possible cases :
-       * - the modified entry has been renamed
-       * - the new parent has been renamed
-       * - the operation is replayed for the second time.
-       * - the entry has been deleted
-       * action :
-       *  - change the target dn and the new parent dn and
-       *        restart the operation,
-       *  - don't do anything if the operation is replayed.
+       * The entry or it's new parent has not been found
+       * reconstruct the operation with the DN we just built
        */
-
-      // Construct the new DN to use for the entry.
-      DN entryDN = op.getEntryDN();
-      DN newSuperior = findEntryDN(newSuperiorID);
-      RDN newRDN = op.getNewRDN();
-      DN parentDN;
-
-      if (newSuperior == null)
-      {
-        parentDN = entryDN.getParent();
-      }
-      else
-      {
-        parentDN = newSuperior;
-      }
-
-      if ((parentDN == null) || parentDN.isNullDN())
-      {
-        /* this should never happen
-         * can't solve any conflict in this case.
-         */
-        throw new Exception("operation parameters are invalid");
-      }
-
-      RDN[] parentComponents = parentDN.getRDNComponents();
-      RDN[] newComponents    = new RDN[parentComponents.length+1];
-      System.arraycopy(parentComponents, 0, newComponents, 1,
-          parentComponents.length);
-      newComponents[0] = newRDN;
-
-      DN newDN = new DN(newComponents);
-
-      // get the current DN of this entry in the database.
-      DN currentDN = findEntryDN(entryUid);
-
-      // if the newDN and the current DN match then the operation
-      // is a no-op (this was probably a second replay)
-      // don't do anything.
-      if (newDN.equals(currentDN))
-      {
-        return true;
-      }
-
+      ModifyDNMsg modifyDnMsg = (ModifyDNMsg) msg;
       msg.setDn(currentDN.toString());
       modifyDnMsg.setNewSuperior(newSuperior.toString());
       return false;
@@ -1489,8 +1499,11 @@
        * add the conflict object class to the entry
        * and rename it using its entryuuid.
        */
+      ModifyDNMsg modifyDnMsg = (ModifyDNMsg) msg;
       generateAddConflictOp(op);
-      msg.setDn(generateConflictDn(entryUid, msg.getDn()));
+      modifyDnMsg.setNewRDN(generateConflictDn(entryUid,
+                            modifyDnMsg.getNewRDN()));
+      modifyDnMsg.setNewSuperior(newSuperior.toString());
       return false;
     }
     return true;
diff --git a/opends/tests/unit-tests-testng/src/server/org/opends/server/synchronization/UpdateOperationTest.java b/opends/tests/unit-tests-testng/src/server/org/opends/server/synchronization/UpdateOperationTest.java
index d90cab8..a9d6095 100644
--- a/opends/tests/unit-tests-testng/src/server/org/opends/server/synchronization/UpdateOperationTest.java
+++ b/opends/tests/unit-tests-testng/src/server/org/opends/server/synchronization/UpdateOperationTest.java
@@ -360,7 +360,7 @@
     broker.publish(addMsg);
 
     // Check that the entry has been created in the local DS.
-    Entry resultEntry = getEntry(personWithUUIDEntry.getDN(), 1000);
+    Entry resultEntry = getEntry(personWithUUIDEntry.getDN(), 1000, true);
     assertNotNull(resultEntry,
         "The send ADD synchronization message was not applied");
     entryList.add(resultEntry);
@@ -397,7 +397,7 @@
     broker.publish(addMsg);
 
     // Check that the entry has been created in the local DS.
-    resultEntry = getEntry(personWithUUIDEntry.getDN(), 1000);
+    resultEntry = getEntry(personWithUUIDEntry.getDN(), 1000, true);
     assertNotNull(resultEntry,
         "The ADD synchronization message was not applied");
     entryList.add(resultEntry);
@@ -405,8 +405,7 @@
     // send a modify operation with a wrong unique ID but the same DN
     mods = generatemods("telephonenumber", "02 01 03 05");
     modMsg = new ModifyMsg(gen.NewChangeNumber(),
-        DN.decode("cn=something,ou=People,dc=example,dc=com"), mods,
-        "10000000-9abc-def0-1234-1234567890ab");
+        DN.decode(user1dn), mods, "10000000-9abc-def0-1234-1234567890ab");
     broker.publish(modMsg);
 
     // check that the modify has not been applied
@@ -423,7 +422,7 @@
      * the same UUID has the entry that has been used in the tests above.
      * Finally check that the delete operation has been applied.
      */
-    // send a delete operation with a wong dn but the unique ID of the entry
+    // send a delete operation with a wrong dn but the unique ID of the entry
     // used above
     DeleteMsg delMsg =
       new DeleteMsg("cn=anotherdn,ou=People,dc=example,dc=com",
@@ -431,7 +430,7 @@
     broker.publish(delMsg);
     
     // check that the delete operation has been applied
-    resultEntry = getEntry(personWithUUIDEntry.getDN(), 1000);
+    resultEntry = getEntry(personWithUUIDEntry.getDN(), 1000, false);
 
     assertNull(resultEntry,
         "The DELETE synchronization message was not replayed");
@@ -450,7 +449,7 @@
     broker.publish(addMsg);
 
     //  Check that the entry has been created in the local DS.
-    resultEntry = getEntry(personWithUUIDEntry.getDN(), 1000);
+    resultEntry = getEntry(personWithUUIDEntry.getDN(), 1000, true);
     assertNotNull(resultEntry,
         "The ADD synchronization message was not applied");
     entryList.add(resultEntry);
@@ -465,7 +464,8 @@
 
     //  Check that the entry has been renamed and created in the local DS.
     resultEntry = getEntry(
-        DN.decode("entryuuid=" + user1entrysecondUUID +" + " + user1dn), 1000);
+        DN.decode("entryuuid=" + user1entrysecondUUID +" + " + user1dn),
+        1000, true);
     assertNotNull(resultEntry,
         "The ADD synchronization message was not applied");
 
@@ -478,7 +478,7 @@
       new DeleteMsg(personWithSecondUniqueID.getDN().toString(),
           gen.NewChangeNumber(), user1entrysecondUUID);
     broker.publish(delMsg);
-    resultEntry = getEntry(personWithUUIDEntry.getDN(), 1000);
+    resultEntry = getEntry(personWithUUIDEntry.getDN(), 1000, false);
 
     // check that the delete operation has been applied
     assertNull(resultEntry,
@@ -500,7 +500,7 @@
 
     //  Check that the entry has been renamed and created in the local DS.
     resultEntry = getEntry(
-        DN.decode("uid=new person,ou=People,dc=example,dc=com"), 1000);
+        DN.decode("uid=new person,ou=People,dc=example,dc=com"), 1000, true);
     assertNotNull(resultEntry,
         "The ADD synchronization message was not applied");
 
@@ -512,31 +512,118 @@
      * To achieve this send a delete operation with a correct DN
      * but a wrong unique ID.
      */
-
-    //  delete the entry to clean the database
+    
     delMsg =
       new DeleteMsg("uid=new person,ou=People,dc=example,dc=com",
           gen.NewChangeNumber(), "11111111-9abc-def0-1234-1234567890ab");
     broker.publish(delMsg);
     resultEntry = getEntry(
-          DN.decode("uid=new person,ou=People,dc=example,dc=com"), 1000);
+          DN.decode("uid=new person,ou=People,dc=example,dc=com"), 1000, true);
 
     // check that the delete operation has not been applied
     assertNotNull(resultEntry,
         "The DELETE synchronization message was replayed when it should not");
 
-    // delete the entry to clean the database
+    
+    /*
+     * Check that when replaying modify dn operations, the conflict
+     * resolution code is able to find the new DN of the parent entry
+     * if it has been renamed on another master.
+     * 
+     * To simulate this try to rename an entry below an entry that does
+     * not exist but giving the unique ID of an existing entry.
+     */
+    
+    ModifyDNMsg  modDnMsg = new ModifyDNMsg(
+        "uid=new person,ou=People,dc=example,dc=com", gen.NewChangeNumber(),
+        user1entryUUID, baseUUID, false,
+        "uid=wrong, ou=people,dc=example,dc=com", 
+        "uid=newrdn");
+    broker.publish(modDnMsg);
+    
+    resultEntry = getEntry(
+        DN.decode("uid=newrdn,ou=People,dc=example,dc=com"), 1000, true);
+
+    // check that the operation has been correctly relayed
+    assertNotNull(resultEntry,
+      "The modify dn was not or badly replayed");
+    
+    /*
+     * same test but by giving a bad entry DN 
+     */
+    
+     modDnMsg = new ModifyDNMsg(
+        "uid=wrong,ou=People,dc=example,dc=com", gen.NewChangeNumber(),
+        user1entryUUID, baseUUID, false, null, "uid=reallynewrdn");
+    broker.publish(modDnMsg);
+    
+    resultEntry = getEntry(
+        DN.decode("uid=reallynewrdn,ou=People,dc=example,dc=com"), 1000, true);
+
+    // check that the operation has been correctly relayed
+    assertNotNull(resultEntry,
+      "The modify dn was not or badly replayed");
+    
+    /*
+     * Check that conflicting entries are renamed when a 
+     * modifyDN is done with the same DN as an entry added on another server.
+     */
+    
+    // add a second entry
+    addMsg = new AddMsg(gen.NewChangeNumber(),
+        user1dn,
+        user1entrysecondUUID,
+        baseUUID,
+        personWithSecondUniqueID.getObjectClassAttribute(),
+        personWithSecondUniqueID.getAttributes(), new ArrayList<Attribute>());   
+    broker.publish(addMsg);
+    
+    //  check that the second entry has been added
+    resultEntry = getEntry(DN.decode(user1dn), 1000, true);
+  
+    // check that the add operation has been applied
+    assertNotNull(resultEntry, "The add operation was not replayed");
+    
+    // try to rename the first entry
+    modDnMsg = new ModifyDNMsg(user1dn, gen.NewChangeNumber(),
+                               user1entrysecondUUID, baseUUID, false,
+                               baseDn.toString(), "uid=reallynewrdn");
+    broker.publish(modDnMsg);
+    
+   // check that the second entry has been renamed
+    resultEntry = getEntry(
+        DN.decode("entryUUID = " + user1entrysecondUUID + "+uid=reallynewrdn," +
+            "ou=People,dc=example,dc=com"), 1000, true);
+  
+    // check that the delete operation has been applied
+    assertNotNull(resultEntry, "The modifyDN was not or incorrectly replayed");
+    
+    // delete the entries to clean the database
     delMsg =
-      new DeleteMsg("uid=new person,ou=People,dc=example,dc=com",
+      new DeleteMsg("uid=reallynewrdn,ou=People,dc=example,dc=com",
           gen.NewChangeNumber(), user1entryUUID);
     broker.publish(delMsg);
     resultEntry = getEntry(
-          DN.decode("uid=new person,ou=People,dc=example,dc=com"), 1000);
-
+        DN.decode("uid=reallynewrdn,ou=People,dc=example,dc=com"), 1000, false);
+    
+    //  check that the delete operation has been applied
+    assertNull(resultEntry,
+        "The DELETE synchronization message was not replayed");
+    
+    delMsg =
+      new DeleteMsg("entryUUID = " + user1entrysecondUUID + "+" + 
+          DN.decode(user1dn).getRDN().toString() +
+          "ou=People,dc=example,dc=com",
+          gen.NewChangeNumber(), user1entrysecondUUID);
+    broker.publish(delMsg);
+    resultEntry = getEntry(
+          DN.decode("entryUUID = " + user1entrysecondUUID + "+" + 
+              DN.decode(user1dn).getRDN().toString() +
+              "ou=People,dc=example,dc=com"), 1000, false);
+    
     // check that the delete operation has been applied
     assertNull(resultEntry,
         "The DELETE synchronization message was not replayed");
-
     broker.stop();
   }
 
@@ -552,6 +639,22 @@
     ChangeNumberGenerator gen = new ChangeNumberGenerator((short) 3, 0);
 
     /*
+     * loop receiving update until there is nothing left
+     * to make sure that message from previous tests have been consumed.
+     */
+    broker.setSoTimeout(100);
+    try
+    {
+      while (true)
+      {
+        broker.receive();
+      }
+     }
+    catch (Exception e)
+    {
+      broker.setSoTimeout(1000);
+    }
+    /*
      * Test that operations done on this server are sent to the
      * changelog server and forwarded to our changelog broker session.
      */
@@ -660,7 +763,7 @@
     /*
      * Check that the entry has been created in the local DS.
      */
-    Entry resultEntry = getEntry(personWithUUIDEntry.getDN(), 1000);
+    Entry resultEntry = getEntry(personWithUUIDEntry.getDN(), 1000, true);
     assertNotNull(resultEntry,
         "The send ADD synchronization message was not applied");
     entryList.add(resultEntry);
@@ -688,7 +791,7 @@
     broker.publish(moddnMsg);
 
     resultEntry = getEntry(
-        DN.decode("uid= new person,ou=People,dc=example,dc=com"), 1000);
+        DN.decode("uid= new person,ou=People,dc=example,dc=com"), 1000, true);
 
     assertNotNull(resultEntry,
         "The modify DN synchronization message was not applied");
@@ -700,7 +803,7 @@
                            gen.NewChangeNumber(), user1entryUUID);
     broker.publish(delMsg);
     resultEntry = getEntry(
-          DN.decode("uid= new person,ou=People,dc=example,dc=com"), 1000);
+          DN.decode("uid= new person,ou=People,dc=example,dc=com"), 1000, false);
 
     assertNull(resultEntry,
         "The DELETE synchronization message was not replayed");
@@ -751,13 +854,13 @@
   {
     // Wait no more than 1 second (synchro operation has to be sent,
     // received and replay)
-    int i = timeout/100;
+    int i = timeout/50;
     if (i<1)
       i=1;
     boolean found = false;
     while ((i> 0) && (!found))
     {
-      Thread.sleep(100);
+      Thread.sleep(50);
       Entry newEntry = DirectoryServer.getEntry(personWithUUIDEntry.getDN());
       if (newEntry == null)
         fail("The entry " + personWithUUIDEntry.getDN() +
@@ -812,16 +915,17 @@
    * @throws InterruptedException
    * @throws DirectoryException
    */
-  private Entry getEntry(DN dn, int timeout)
+  private Entry getEntry(DN dn, int timeout, boolean exist)
                throws InterruptedException, DirectoryException
   {
     Entry newEntry = null ;
-    int i = timeout/100;
+    int i = timeout/50;
     if (i<1)
       i=1;
-    while ((i> 0) && (newEntry == null))
+    newEntry = DirectoryServer.getEntry(dn);
+    while ((i> 0) && ((newEntry == null) == exist))
     {
-      Thread.sleep(100);
+      Thread.sleep(50);
       newEntry = DirectoryServer.getEntry(dn);
       i--;
     }

--
Gitblit v1.10.0