From 3a31430c1d80863bdee62d29e251bd41377e134d Mon Sep 17 00:00:00 2001
From: coulbeck <coulbeck@localhost>
Date: Tue, 17 Oct 2006 16:09:03 +0000
Subject: [PATCH] Fix for  [Issue 635]  NullPointerException when trying to access non existing entry.  Reviewed by gbellato.

---
 opendj-sdk/opends/src/server/org/opends/server/synchronization/OperationContext.java                            |   11 ++++-
 opendj-sdk/opends/src/server/org/opends/server/synchronization/SynchronizationDomain.java                       |   44 +++++++++++++++------
 opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/synchronization/UpdateOperationTest.java |   30 +++++++++-----
 3 files changed, 58 insertions(+), 27 deletions(-)

diff --git a/opendj-sdk/opends/src/server/org/opends/server/synchronization/OperationContext.java b/opendj-sdk/opends/src/server/org/opends/server/synchronization/OperationContext.java
index 89c7e5c..fb48525 100644
--- a/opendj-sdk/opends/src/server/org/opends/server/synchronization/OperationContext.java
+++ b/opendj-sdk/opends/src/server/org/opends/server/synchronization/OperationContext.java
@@ -71,9 +71,9 @@
   }
 
   /**
-   * Get the unique Identifier of the modiffied entry.
+   * Get the unique Identifier of the modified entry.
    *
-   * @return the unique Identifier of the modiffied entry.
+   * @return the unique Identifier of the modified entry.
    */
   public String getEntryUid()
   {
@@ -84,11 +84,16 @@
    * Get the change number of an operation.
    *
    * @param op The operation.
-   * @return the change number of the provided operation.
+   * @return The change number of the provided operation, or null if there is
+   * no change number associated with the operation.
    */
   public static ChangeNumber getChangeNumber(Operation op)
   {
     OperationContext ctx = (OperationContext)op.getAttachment(SYNCHROCONTEXT);
+    if (ctx == null)
+    {
+      return null;
+    }
     return ctx.changeNumber;
   }
 
diff --git a/opendj-sdk/opends/src/server/org/opends/server/synchronization/SynchronizationDomain.java b/opendj-sdk/opends/src/server/org/opends/server/synchronization/SynchronizationDomain.java
index a5da8cd..f02c35d 100644
--- a/opendj-sdk/opends/src/server/org/opends/server/synchronization/SynchronizationDomain.java
+++ b/opendj-sdk/opends/src/server/org/opends/server/synchronization/SynchronizationDomain.java
@@ -95,7 +95,7 @@
 
   private List<ListenerThread> synchroThreads =
     new ArrayList<ListenerThread>();
-  private SortedMap<ChangeNumber, PendingChange> pendingChanges =
+  private final SortedMap<ChangeNumber, PendingChange> pendingChanges =
     new TreeMap<ChangeNumber, PendingChange>();
   private SortedMap<ChangeNumber, UpdateMessage> waitingAckMsgs =
     new TreeMap<ChangeNumber, UpdateMessage>();
@@ -725,7 +725,7 @@
   }
 
   /**
-   * Do the necessary processing when and AckMessage was received.
+   * Do the necessary processing when an AckMessage is received.
    *
    * @param ack The AckMessage that was received.
    */
@@ -755,15 +755,23 @@
    */
   public void synchronize(Operation op)
   {
-    numReplayedPostOpCalled++;
+    ResultCode result = op.getResultCode();
+    if ((result == ResultCode.SUCCESS) && op.isSynchronizationOperation())
+    {
+      numReplayedPostOpCalled++;
+    }
     UpdateMessage msg = null;
+
+    // Note that a failed non-synchronization operation might not have a change
+    // number.
     ChangeNumber curChangeNumber = OperationContext.getChangeNumber(op);
 
-    ResultCode result = op.getResultCode();
     boolean isAssured = isAssured(op);
 
     if ((result == ResultCode.SUCCESS) && (!op.isSynchronizationOperation()))
     {
+      // Generate a synchronization message for a successful non-synchronization
+      // operation.
       msg = UpdateMessage.generateMsg(op, isAssured);
 
       if (msg == null)
@@ -808,19 +816,28 @@
         else
           curChange.setMsg(msg);
 
-        if (!op.isSynchronizationOperation() && isAssured && (msg != null))
+        if (msg != null && isAssured)
         {
+          // Add the assured message to the list of those whose acknowledgements
+          // we are awaiting.
           waitingAckMsgs.put(curChangeNumber, msg);
         }
       }
       else if (!op.isSynchronizationOperation())
-        pendingChanges.remove(curChangeNumber);
+      {
+        // Remove an unsuccessful non-synchronization operation from the pending
+        // changes list.
+        if (curChangeNumber != null)
+        {
+          pendingChanges.remove(curChangeNumber);
+        }
+      }
 
       pushCommittedChanges();
     }
 
-    if ((!op.isSynchronizationOperation()) && msg.isAssured() && (msg != null)
-        && (result == ResultCode.SUCCESS))
+    // Wait for acknowledgement of an assured message.
+    if (msg != null && isAssured)
     {
       synchronized (msg)
       {
@@ -1110,7 +1127,7 @@
       {
         /*
          * An Exception happened during the replay process.
-         * Continue with the next change but the servers will know start
+         * Continue with the next change but the servers will now start
          * to be inconsistent.
          * TODO : REPAIR : Should let the repair tool know about this
          */
@@ -1139,9 +1156,9 @@
   }
 
   /**
-   * This methods is called when an error happends while replaying
-   * and operation.
-   * It is necessary because the postOPeration does not always get
+   * This method is called when an error happens while replaying
+   * an operation.
+   * It is necessary because the postOperation does not always get
    * called when error or Exceptions happen during the operation replay.
    *
    * @param changeNumber the ChangeNumber of the operation with error.
@@ -1536,7 +1553,8 @@
   /**
    * Generate the Dn to use for a conflicting entry.
    *
-   * @param op Operation that generated the conflict
+   * @param entryUid The unique identifier of the entry involved in the
+   * conflict.
    * @param dn Original dn.
    * @return The generated Dn for a conflicting entry.
    */
diff --git a/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/synchronization/UpdateOperationTest.java b/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/synchronization/UpdateOperationTest.java
index 58563dc..b9da84a 100644
--- a/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/synchronization/UpdateOperationTest.java
+++ b/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/synchronization/UpdateOperationTest.java
@@ -44,16 +44,7 @@
 import org.opends.server.core.ModifyOperation;
 import org.opends.server.core.Operation;
 import org.opends.server.protocols.internal.InternalClientConnection;
-import org.opends.server.types.Attribute;
-import org.opends.server.types.AttributeType;
-import org.opends.server.types.AttributeValue;
-import org.opends.server.types.DN;
-import org.opends.server.types.DirectoryException;
-import org.opends.server.types.Entry;
-import org.opends.server.types.Modification;
-import org.opends.server.types.ModificationType;
-import org.opends.server.types.OperationType;
-import org.opends.server.types.RDN;
+import org.opends.server.types.*;
 import org.testng.annotations.AfterClass;
 import org.testng.annotations.BeforeClass;
 import org.testng.annotations.DataProvider;
@@ -642,7 +633,7 @@
     final DN baseDn = DN.decode("ou=People,dc=example,dc=com");
 
     cleanEntries();
-    
+
     ChangelogBroker broker = openChangelogSession(baseDn, (short) 3);
     ChangeNumberGenerator gen = new ChangeNumberGenerator((short) 3, 0);
 
@@ -992,4 +983,21 @@
         "Unable to add the syncrhonized server");
     entryList.add(synchroServerEntry);
   }
+
+  /**
+   * Test case for
+   * [Issue 635] NullPointerException when trying to access non existing entry.
+   */
+  @Test(enabled=true)
+  public void deleteNoSuchObject() throws Exception
+  {
+    DN dn = DN.decode("cn=No Such Object,ou=People,dc=example,dc=com");
+    Operation op =
+         new DeleteOperation(connection,
+                             InternalClientConnection.nextOperationID(),
+                             InternalClientConnection.nextMessageID(), null,
+                             dn);
+    op.run();
+    assertEquals(op.getResultCode(), ResultCode.NO_SUCH_OBJECT);
+  }
 }

--
Gitblit v1.10.0