From 43cd5f921108cfca9337ab5bdb8ff24b632ac54d Mon Sep 17 00:00:00 2001
From: Jean-Noel Rouvignac <jean-noel.rouvignac@forgerock.com>
Date: Mon, 15 Jul 2013 07:44:51 +0000
Subject: [PATCH] Refactoring on the LocalBackend*Operation classes: - Moved BooleanHolder out of LocalBackendAddOperation. - Extracted process*() methods from the processLocal*() methods for the LocalBackend*Operation classes. - Transformed plain comments into javadocs. - Used interfaces instead of concrete classes. - Made all protected members be private. - Used StatisUtils.collectionToString()

---
 opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendModifyOperation.java |  672 +++++++++++++-----------------
 opends/src/server/org/opends/server/workflowelement/localbackend/BooleanHolder.java               |   39 +
 opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendDeleteOperation.java |  530 +++++++++++------------
 opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendAddOperation.java    |   27 
 4 files changed, 585 insertions(+), 683 deletions(-)

diff --git a/opends/src/server/org/opends/server/workflowelement/localbackend/BooleanHolder.java b/opends/src/server/org/opends/server/workflowelement/localbackend/BooleanHolder.java
new file mode 100644
index 0000000..7ad70d1
--- /dev/null
+++ b/opends/src/server/org/opends/server/workflowelement/localbackend/BooleanHolder.java
@@ -0,0 +1,39 @@
+/*
+ * CDDL HEADER START
+ *
+ * The contents of this file are subject to the terms of the
+ * Common Development and Distribution License, Version 1.0 only
+ * (the "License").  You may not use this file except in compliance
+ * with the License.
+ *
+ * You can obtain a copy of the license at
+ * trunk/opends/resource/legal-notices/OpenDS.LICENSE
+ * or https://OpenDS.dev.java.net/OpenDS.LICENSE.
+ * See the License for the specific language governing permissions
+ * and limitations under the License.
+ *
+ * When distributing Covered Code, include this CDDL HEADER in each
+ * file and include the License file at
+ * trunk/opends/resource/legal-notices/OpenDS.LICENSE.  If applicable,
+ * add the following below this CDDL HEADER, with the fields enclosed
+ * by brackets "[]" replaced with your own identifying information:
+ *      Portions Copyright [yyyy] [name of copyright owner]
+ *
+ * CDDL HEADER END
+ *
+ *
+ *      Copyright 2013 ForgeRock AS
+ */
+package org.opends.server.workflowelement.localbackend;
+
+/**
+ * This class holds a boolean value. Allow to use in/out boolean parameters,
+ * which contain data to pass into the method like a regular parameter, but can
+ * also be used to return data from the method.
+ */
+class BooleanHolder
+{
+
+  /** The boolean value held in this class. */
+  boolean value;
+}
diff --git a/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendAddOperation.java b/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendAddOperation.java
index 90b6a6f..2d988ab 100644
--- a/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendAddOperation.java
+++ b/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendAddOperation.java
@@ -27,8 +27,6 @@
  */
 package org.opends.server.workflowelement.localbackend;
 
-
-
 import static org.opends.messages.CoreMessages.*;
 import static org.opends.server.config.ConfigConstants.*;
 import static org.opends.server.loggers.ErrorLogger.*;
@@ -58,8 +56,6 @@
 import org.opends.server.types.operation.PreOperationAddOperation;
 import org.opends.server.util.TimeThread;
 
-
-
 /**
  * This class defines an operation used to add an entry in a local backend
  * of the Directory Server.
@@ -75,14 +71,6 @@
   private static final DebugTracer TRACER = getTracer();
 
   /**
-   * Simple boolean helper class.
-   */
-  private static final class BooleanHolder
-  {
-    boolean value;
-  }
-
-  /**
    * The backend in which the entry is to be added.
    */
   private Backend backend;
@@ -169,17 +157,15 @@
     this.backend = wfe.getBackend();
     ClientConnection clientConnection = getClientConnection();
 
-    // Get the plugin config manager that will be used for invoking plugins.
-    PluginConfigManager pluginConfigManager =
-         DirectoryServer.getPluginConfigManager();
-
     // Check for a request to cancel this operation.
     checkIfCanceled(false);
 
 
     BooleanHolder executePostOpPlugins = new BooleanHolder();
-    processAdd(clientConnection, executePostOpPlugins, pluginConfigManager);
+    processAdd(clientConnection, executePostOpPlugins);
 
+    PluginConfigManager pluginConfigManager =
+        DirectoryServer.getPluginConfigManager();
 
     // Invoke the post-operation or post-synchronization add plugins.
     if (isSynchronizationOperation())
@@ -245,9 +231,7 @@
   }
 
   private void processAdd(ClientConnection clientConnection,
-      BooleanHolder executePostOpPlugins,
-      PluginConfigManager pluginConfigManager)
-      throws CanceledOperationException
+      BooleanHolder executePostOpPlugins) throws CanceledOperationException
   {
     // Process the entry DN and set of attributes to convert them from their
     // raw forms as provided by the client to the forms required for the rest
@@ -488,7 +472,8 @@
       {
         executePostOpPlugins.value = true;
         PluginResult.PreOperation preOpResult =
-            pluginConfigManager.invokePreOperationAddPlugins(this);
+            DirectoryServer.getPluginConfigManager()
+                .invokePreOperationAddPlugins(this);
         if (!preOpResult.continueProcessing())
         {
           setResultCode(preOpResult.getResultCode());
diff --git a/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendDeleteOperation.java b/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendDeleteOperation.java
index 380ee88..8057d19 100644
--- a/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendDeleteOperation.java
+++ b/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendDeleteOperation.java
@@ -71,29 +71,29 @@
   /**
    * The backend in which the operation is to be processed.
    */
-  protected Backend backend;
+  private Backend backend;
 
   /**
    * Indicates whether the LDAP no-op control has been requested.
    */
-  protected boolean noOp;
+  private boolean noOp;
 
   /**
    * The client connection on which this operation was requested.
    */
-  protected ClientConnection clientConnection;
+  private ClientConnection clientConnection;
 
   /**
    * The DN of the entry to be deleted.
    */
-  protected DN entryDN;
+  private DN entryDN;
 
   /**
    * The entry to be deleted.
    */
-  protected Entry entry;
+  private Entry entry;
 
-  // The pre-read request control included in the request, if applicable.
+  /** The pre-read request control included in the request, if applicable. */
   private LDAPPreReadRequestControl preReadRequest;
 
 
@@ -137,278 +137,19 @@
   public void processLocalDelete(final LocalBackendWorkflowElement wfe)
       throws CanceledOperationException
   {
-    boolean executePostOpPlugins = false;
     this.backend = wfe.getBackend();
 
     clientConnection = getClientConnection();
 
-    // Get the plugin config manager that will be used for invoking plugins.
-    PluginConfigManager pluginConfigManager =
-         DirectoryServer.getPluginConfigManager();
-
     // Check for a request to cancel this operation.
     checkIfCanceled(false);
 
-    // Create a labeled block of code that we can break out of if a problem is
-    // detected.
-deleteProcessing:
-    {
-      // Process the entry DN to convert it from its raw form as provided by the
-      // client to the form required for the rest of the delete processing.
-      entryDN = getEntryDN();
-      if (entryDN == null){
-        break deleteProcessing;
-      }
-
-      // Grab a write lock on the entry.
-      final Lock entryLock = LockManager.lockWrite(entryDN);
-      if (entryLock == null)
-      {
-        setResultCode(ResultCode.BUSY);
-        appendErrorMessage(ERR_DELETE_CANNOT_LOCK_ENTRY.get(
-                                String.valueOf(entryDN)));
-        break deleteProcessing;
-      }
-
-      try
-      {
-        // Get the entry to delete.  If it doesn't exist, then fail.
-        try
-        {
-          entry = backend.getEntry(entryDN);
-          if (entry == null)
-          {
-            setResultCode(ResultCode.NO_SUCH_OBJECT);
-            appendErrorMessage(ERR_DELETE_NO_SUCH_ENTRY.get(
-                                    String.valueOf(entryDN)));
-
-            try
-            {
-              DN parentDN = entryDN.getParentDNInSuffix();
-              while (parentDN != null)
-              {
-                if (DirectoryServer.entryExists(parentDN))
-                {
-                  setMatchedDN(parentDN);
-                  break;
-                }
-
-                parentDN = parentDN.getParentDNInSuffix();
-              }
-            }
-            catch (Exception e)
-            {
-              if (debugEnabled())
-              {
-                TRACER.debugCaught(DebugLogLevel.ERROR, e);
-              }
-            }
-
-            break deleteProcessing;
-          }
-        }
-        catch (DirectoryException de)
-        {
-          if (debugEnabled())
-          {
-            TRACER.debugCaught(DebugLogLevel.ERROR, de);
-          }
-
-          setResponseData(de);
-          break deleteProcessing;
-        }
-
-        if(!handleConflictResolution()) {
-            break deleteProcessing;
-        }
-
-        // Check to see if the client has permission to perform the
-        // delete.
-
-        // Check to see if there are any controls in the request.  If so, then
-        // see if there is any special processing required.
-        try
-        {
-          handleRequestControls();
-        }
-        catch (DirectoryException de)
-        {
-          if (debugEnabled())
-          {
-            TRACER.debugCaught(DebugLogLevel.ERROR, de);
-          }
-
-          setResponseData(de);
-          break deleteProcessing;
-        }
-
-
-        // FIXME: for now assume that this will check all permission
-        // pertinent to the operation. This includes proxy authorization
-        // and any other controls specified.
-
-        // FIXME: earlier checks to see if the entry already exists may
-        // have already exposed sensitive information to the client.
-        try
-        {
-          if (!AccessControlConfigManager.getInstance()
-              .getAccessControlHandler().isAllowed(this))
-          {
-            setResultCode(ResultCode.INSUFFICIENT_ACCESS_RIGHTS);
-            appendErrorMessage(ERR_DELETE_AUTHZ_INSUFFICIENT_ACCESS_RIGHTS
-                .get(String.valueOf(entryDN)));
-            break deleteProcessing;
-          }
-        }
-        catch (DirectoryException e)
-        {
-          setResultCode(e.getResultCode());
-          appendErrorMessage(e.getMessageObject());
-          break deleteProcessing;
-        }
-
-        // Check for a request to cancel this operation.
-        checkIfCanceled(false);
-
-
-        // If the operation is not a synchronization operation,
-        // invoke the pre-delete plugins.
-        if (! isSynchronizationOperation())
-        {
-          executePostOpPlugins = true;
-          PluginResult.PreOperation preOpResult =
-               pluginConfigManager.invokePreOperationDeletePlugins(this);
-          if (!preOpResult.continueProcessing())
-          {
-            setResultCode(preOpResult.getResultCode());
-            appendErrorMessage(preOpResult.getErrorMessage());
-            setMatchedDN(preOpResult.getMatchedDN());
-            setReferralURLs(preOpResult.getReferralURLs());
-            break deleteProcessing;
-          }
-        }
-
-
-        // Get the backend to use for the delete.  If there is none, then fail.
-        if (backend == null)
-        {
-          setResultCode(ResultCode.NO_SUCH_OBJECT);
-          appendErrorMessage(ERR_DELETE_NO_SUCH_ENTRY.get(
-                                  String.valueOf(entryDN)));
-          break deleteProcessing;
-        }
-
-
-        // If it is not a private backend, then check to see if the server or
-        // backend is operating in read-only mode.
-        if (! backend.isPrivateBackend())
-        {
-          switch (DirectoryServer.getWritabilityMode())
-          {
-            case DISABLED:
-              setResultCode(ResultCode.UNWILLING_TO_PERFORM);
-              appendErrorMessage(ERR_DELETE_SERVER_READONLY.get(
-                                      String.valueOf(entryDN)));
-              break deleteProcessing;
-
-            case INTERNAL_ONLY:
-              if (! (isInternalOperation() || isSynchronizationOperation()))
-              {
-                setResultCode(ResultCode.UNWILLING_TO_PERFORM);
-                appendErrorMessage(ERR_DELETE_SERVER_READONLY.get(
-                                        String.valueOf(entryDN)));
-                break deleteProcessing;
-              }
-          }
-
-          switch (backend.getWritabilityMode())
-          {
-            case DISABLED:
-              setResultCode(ResultCode.UNWILLING_TO_PERFORM);
-              appendErrorMessage(ERR_DELETE_BACKEND_READONLY.get(
-                                      String.valueOf(entryDN)));
-              break deleteProcessing;
-
-            case INTERNAL_ONLY:
-              if (! (isInternalOperation() || isSynchronizationOperation()))
-              {
-                setResultCode(ResultCode.UNWILLING_TO_PERFORM);
-                appendErrorMessage(ERR_DELETE_BACKEND_READONLY.get(
-                                        String.valueOf(entryDN)));
-                break deleteProcessing;
-              }
-          }
-        }
-
-
-        // The selected backend will have the responsibility of making sure that
-        // the entry actually exists and does not have any children (or possibly
-        // handling a subtree delete).  But we will need to check if there are
-        // any subordinate backends that should stop us from attempting the
-        // delete.
-        Backend[] subBackends = backend.getSubordinateBackends();
-        for (Backend b : subBackends)
-        {
-          DN[] baseDNs = b.getBaseDNs();
-          for (DN dn : baseDNs)
-          {
-            if (dn.isDescendantOf(entryDN))
-            {
-              setResultCode(ResultCode.NOT_ALLOWED_ON_NONLEAF);
-              appendErrorMessage(ERR_DELETE_HAS_SUB_BACKEND.get(
-                                      String.valueOf(entryDN),
-                                      String.valueOf(dn)));
-              break deleteProcessing;
-            }
-          }
-        }
-
-
-        // Actually perform the delete.
-        try
-        {
-          if (noOp)
-          {
-            setResultCode(ResultCode.NO_OPERATION);
-            appendErrorMessage(INFO_DELETE_NOOP.get());
-          }
-          else
-          {
-              if(!processPreOperation()) {
-                  break deleteProcessing;
-              }
-              backend.deleteEntry(entryDN, this);
-          }
-
-
-          LocalBackendWorkflowElement.addPreReadResponse(this,
-              preReadRequest, entry);
-
-
-          if (! noOp)
-          {
-            setResultCode(ResultCode.SUCCESS);
-          }
-        }
-        catch (DirectoryException de)
-        {
-          if (debugEnabled())
-          {
-            TRACER.debugCaught(DebugLogLevel.ERROR, de);
-          }
-
-          setResponseData(de);
-          break deleteProcessing;
-        }
-      }
-      finally
-      {
-        LockManager.unlock(entryDN, entryLock);
-        processSynchPostOperationPlugins();
-      }
-    }
+    BooleanHolder executePostOpPlugins = new BooleanHolder();
+    processDelete(executePostOpPlugins);
 
     // Invoke the post-operation or post-synchronization delete plugins.
+    PluginConfigManager pluginConfigManager =
+        DirectoryServer.getPluginConfigManager();
     if (isSynchronizationOperation())
     {
       if (getResultCode() == ResultCode.SUCCESS)
@@ -416,7 +157,7 @@
         pluginConfigManager.invokePostSynchronizationDeletePlugins(this);
       }
     }
-    else if (executePostOpPlugins)
+    else if (executePostOpPlugins.value)
     {
       PluginResult.PostOperation postOpResult =
           pluginConfigManager.invokePostOperationDeletePlugins(this);
@@ -430,7 +171,6 @@
       }
     }
 
-
     // Register a post-response call-back which will notify persistent
     // searches and change listeners.
     if (getResultCode() == ResultCode.SUCCESS)
@@ -442,7 +182,8 @@
         public void run()
         {
           // Notify persistent searches.
-          for (PersistentSearch psearch : wfe.getPersistentSearches()) {
+          for (PersistentSearch psearch : wfe.getPersistentSearches())
+          {
             psearch.processDelete(entry, getChangeNumber());
           }
 
@@ -462,8 +203,9 @@
                 TRACER.debugCaught(DebugLogLevel.ERROR, e);
               }
 
-              Message message = ERR_DELETE_ERROR_NOTIFYING_CHANGE_LISTENER
-                  .get(getExceptionMessage(e));
+              Message message =
+                  ERR_DELETE_ERROR_NOTIFYING_CHANGE_LISTENER
+                      .get(getExceptionMessage(e));
               logError(message);
             }
           }
@@ -472,6 +214,229 @@
     }
   }
 
+  private void processDelete(BooleanHolder executePostOpPlugins)
+      throws CanceledOperationException
+  {
+    // Process the entry DN to convert it from its raw form as provided by the
+    // client to the form required for the rest of the delete processing.
+    entryDN = getEntryDN();
+    if (entryDN == null)
+    {
+      return;
+    }
+
+    // Grab a write lock on the entry.
+    final Lock entryLock = LockManager.lockWrite(entryDN);
+    if (entryLock == null)
+    {
+      setResultCode(ResultCode.BUSY);
+      appendErrorMessage(ERR_DELETE_CANNOT_LOCK_ENTRY.get(String
+          .valueOf(entryDN)));
+      return;
+    }
+
+    try
+    {
+      // Get the entry to delete. If it doesn't exist, then fail.
+      entry = backend.getEntry(entryDN);
+      if (entry == null)
+      {
+        setResultCode(ResultCode.NO_SUCH_OBJECT);
+        appendErrorMessage(ERR_DELETE_NO_SUCH_ENTRY
+            .get(String.valueOf(entryDN)));
+
+        try
+        {
+          DN parentDN = entryDN.getParentDNInSuffix();
+          while (parentDN != null)
+          {
+            if (DirectoryServer.entryExists(parentDN))
+            {
+              setMatchedDN(parentDN);
+              break;
+            }
+
+            parentDN = parentDN.getParentDNInSuffix();
+          }
+        }
+        catch (Exception e)
+        {
+          if (debugEnabled())
+          {
+            TRACER.debugCaught(DebugLogLevel.ERROR, e);
+          }
+        }
+
+        return;
+      }
+
+      if (!handleConflictResolution())
+      {
+        return;
+      }
+
+      // Check to see if the client has permission to perform the
+      // delete.
+
+      // Check to see if there are any controls in the request. If so, then
+      // see if there is any special processing required.
+      handleRequestControls();
+
+      // FIXME: for now assume that this will check all permission
+      // pertinent to the operation. This includes proxy authorization
+      // and any other controls specified.
+
+      // FIXME: earlier checks to see if the entry already exists may
+      // have already exposed sensitive information to the client.
+      try
+      {
+        if (!AccessControlConfigManager.getInstance().getAccessControlHandler()
+            .isAllowed(this))
+        {
+          setResultCode(ResultCode.INSUFFICIENT_ACCESS_RIGHTS);
+          appendErrorMessage(ERR_DELETE_AUTHZ_INSUFFICIENT_ACCESS_RIGHTS
+              .get(String.valueOf(entryDN)));
+          return;
+        }
+      }
+      catch (DirectoryException e)
+      {
+        setResultCode(e.getResultCode());
+        appendErrorMessage(e.getMessageObject());
+        return;
+      }
+
+      // Check for a request to cancel this operation.
+      checkIfCanceled(false);
+
+      // If the operation is not a synchronization operation,
+      // invoke the pre-delete plugins.
+      if (!isSynchronizationOperation())
+      {
+        executePostOpPlugins.value = true;
+        PluginResult.PreOperation preOpResult =
+            DirectoryServer.getPluginConfigManager()
+                .invokePreOperationDeletePlugins(this);
+        if (!preOpResult.continueProcessing())
+        {
+          setResultCode(preOpResult.getResultCode());
+          appendErrorMessage(preOpResult.getErrorMessage());
+          setMatchedDN(preOpResult.getMatchedDN());
+          setReferralURLs(preOpResult.getReferralURLs());
+          return;
+        }
+      }
+
+      // Get the backend to use for the delete. If there is none, then fail.
+      if (backend == null)
+      {
+        setResultCode(ResultCode.NO_SUCH_OBJECT);
+        appendErrorMessage(ERR_DELETE_NO_SUCH_ENTRY
+            .get(String.valueOf(entryDN)));
+        return;
+      }
+
+      // If it is not a private backend, then check to see if the server or
+      // backend is operating in read-only mode.
+      if (!backend.isPrivateBackend())
+      {
+        switch (DirectoryServer.getWritabilityMode())
+        {
+        case DISABLED:
+          setResultCode(ResultCode.UNWILLING_TO_PERFORM);
+          appendErrorMessage(ERR_DELETE_SERVER_READONLY.get(String
+              .valueOf(entryDN)));
+          return;
+
+        case INTERNAL_ONLY:
+          if (!(isInternalOperation() || isSynchronizationOperation()))
+          {
+            setResultCode(ResultCode.UNWILLING_TO_PERFORM);
+            appendErrorMessage(ERR_DELETE_SERVER_READONLY.get(String
+                .valueOf(entryDN)));
+            return;
+          }
+        }
+
+        switch (backend.getWritabilityMode())
+        {
+        case DISABLED:
+          setResultCode(ResultCode.UNWILLING_TO_PERFORM);
+          appendErrorMessage(ERR_DELETE_BACKEND_READONLY.get(String
+              .valueOf(entryDN)));
+          return;
+
+        case INTERNAL_ONLY:
+          if (!(isInternalOperation() || isSynchronizationOperation()))
+          {
+            setResultCode(ResultCode.UNWILLING_TO_PERFORM);
+            appendErrorMessage(ERR_DELETE_BACKEND_READONLY.get(String
+                .valueOf(entryDN)));
+            return;
+          }
+        }
+      }
+
+      // The selected backend will have the responsibility of making sure that
+      // the entry actually exists and does not have any children (or possibly
+      // handling a subtree delete). But we will need to check if there are
+      // any subordinate backends that should stop us from attempting the
+      // delete.
+      Backend[] subBackends = backend.getSubordinateBackends();
+      for (Backend b : subBackends)
+      {
+        for (DN dn : b.getBaseDNs())
+        {
+          if (dn.isDescendantOf(entryDN))
+          {
+            setResultCode(ResultCode.NOT_ALLOWED_ON_NONLEAF);
+            appendErrorMessage(ERR_DELETE_HAS_SUB_BACKEND.get(String
+                .valueOf(entryDN), String.valueOf(dn)));
+            return;
+          }
+        }
+      }
+
+      // Actually perform the delete.
+      if (noOp)
+      {
+        setResultCode(ResultCode.NO_OPERATION);
+        appendErrorMessage(INFO_DELETE_NOOP.get());
+      }
+      else
+      {
+        if (!processPreOperation())
+        {
+          return;
+        }
+        backend.deleteEntry(entryDN, this);
+      }
+
+      LocalBackendWorkflowElement.addPreReadResponse(this, preReadRequest,
+          entry);
+
+      if (!noOp)
+      {
+        setResultCode(ResultCode.SUCCESS);
+      }
+    }
+    catch (DirectoryException de)
+    {
+      if (debugEnabled())
+      {
+        TRACER.debugCaught(DebugLogLevel.ERROR, de);
+      }
+
+      setResponseData(de);
+      return;
+    }
+    finally
+    {
+      LockManager.unlock(entryDN, entryLock);
+      processSynchPostOperationPlugins();
+    }
+  }
+
 
 
   /**
@@ -480,15 +445,14 @@
    * @throws  DirectoryException  If a problem occurs that should cause the
    *                              operation to fail.
    */
-  protected void handleRequestControls()
+  private void handleRequestControls()
           throws DirectoryException
   {
     List<Control> requestControls = getRequestControls();
     if ((requestControls != null) && (! requestControls.isEmpty()))
     {
-      for (int i=0; i < requestControls.size(); i++)
+      for (Control c : requestControls)
       {
-        Control c   = requestControls.get(i);
         String  oid = c.getOID();
 
         if (!LocalBackendWorkflowElement.isControlAllowed(entryDN, this, c))
@@ -572,7 +536,7 @@
           addAdditionalLogItem(AdditionalLogItem.keyOnly(getClass(),
               "obsoleteProxiedAuthzV1Control"));
 
-          // The requester must have the PROXIED_AUTH privilige in order to
+          // The requester must have the PROXIED_AUTH privilege in order to
           // be able to use this control.
           if (! clientConnection.hasPrivilege(Privilege.PROXIED_AUTH, this))
           {
@@ -596,7 +560,7 @@
         }
         else if (oid.equals(OID_PROXIED_AUTH_V2))
         {
-          // The requester must have the PROXIED_AUTH privilige in order to
+          // The requester must have the PROXIED_AUTH privilege in order to
           // be able to use this control.
           if (! clientConnection.hasPrivilege(Privilege.PROXIED_AUTH, this))
           {
@@ -642,7 +606,7 @@
    * @return  {@code true} if processing should continue for the operation, or
    *          {@code false} if not.
    */
-  protected boolean handleConflictResolution() {
+  private boolean handleConflictResolution() {
       boolean returnVal = true;
 
       for (SynchronizationProvider<?> provider :
@@ -676,8 +640,7 @@
   /**
    * Invoke post operation synchronization providers.
    */
-  protected void processSynchPostOperationPlugins() {
-
+  private void processSynchPostOperationPlugins() {
       for (SynchronizationProvider<?> provider :
           DirectoryServer.getSynchronizationProviders()) {
           try {
@@ -700,7 +663,7 @@
    * @return  {@code true} if processing should continue for the operation, or
    *          {@code false} if not.
    */
-  protected boolean processPreOperation() {
+  private boolean processPreOperation() {
       boolean returnVal = true;
 
       for (SynchronizationProvider<?> provider :
@@ -731,4 +694,3 @@
       return returnVal;
   }
 }
-
diff --git a/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendModifyOperation.java b/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendModifyOperation.java
index 7d9607f..e4aa570 100644
--- a/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendModifyOperation.java
+++ b/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendModifyOperation.java
@@ -27,8 +27,6 @@
  */
 package org.opends.server.workflowelement.localbackend;
 
-
-
 import static org.opends.messages.CoreMessages.*;
 import static org.opends.server.config.ConfigConstants.*;
 import static org.opends.server.loggers.ErrorLogger.*;
@@ -37,7 +35,6 @@
 import static org.opends.server.util.StaticUtils.*;
 
 import java.util.HashSet;
-import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.concurrent.locks.Lock;
@@ -58,8 +55,6 @@
 import org.opends.server.types.operation.PreOperationModifyOperation;
 import org.opends.server.util.Validator;
 
-
-
 /**
  * This class defines an operation used to modify an entry in a local backend
  * of the Directory Server.
@@ -82,7 +77,7 @@
    */
   protected Backend backend;
 
-  // Indicates whether the request included the user's current password.
+  /** Indicates whether the request included the user's current password. */
   private boolean currentPasswordProvided;
 
   /**
@@ -91,7 +86,7 @@
    */
   protected boolean enabledStateChanged;
 
-  // Indicates whether the user's account is currently enabled.
+  /** Indicates whether the user's account is currently enabled. */
   private boolean isEnabled;
 
   /**
@@ -105,7 +100,7 @@
   protected boolean permissiveModify = false;
 
   /**
-   * Indicates whether this modify operation includees a password change.
+   * Indicates whether this modify operation includes a password change.
    */
   protected boolean passwordChanged;
 
@@ -144,19 +139,19 @@
    */
   protected Entry modifiedEntry = null;
 
-  // The number of passwords contained in the modify operation.
+  /** The number of passwords contained in the modify operation. */
   private int numPasswords;
 
-  // The post-read request control, if present.
+  /** The post-read request control, if present.*/
   private LDAPPostReadRequestControl postReadRequest;
 
-  // The pre-read request control, if present.
+  /** The pre-read request control, if present.*/
   private LDAPPreReadRequestControl preReadRequest;
 
-  // The set of clear-text current passwords (if any were provided).
+  /** The set of clear-text current passwords (if any were provided).*/
   private List<AttributeValue> currentPasswords = null;
 
-  // The set of clear-text new passwords (if any were provided).
+  /** The set of clear-text new passwords (if any were provided).*/
   private List<AttributeValue> newPasswords = null;
 
   /**
@@ -293,355 +288,15 @@
   public void processLocalModify(final LocalBackendWorkflowElement wfe)
       throws CanceledOperationException
   {
-    boolean executePostOpPlugins = false;
     this.backend = wfe.getBackend();
 
     clientConnection = getClientConnection();
 
-    // Get the plugin config manager that will be used for invoking plugins.
-    PluginConfigManager pluginConfigManager =
-      DirectoryServer.getPluginConfigManager();
-
     // Check for a request to cancel this operation.
     checkIfCanceled(false);
 
-    // Create a labeled block of code that we can break out of if a problem is
-    // detected.
-modifyProcessing:
-    {
-      entryDN = getEntryDN();
-      if (entryDN == null){
-        break modifyProcessing;
-      }
-
-      // Process the modifications to convert them from their raw form to the
-      // form required for the rest of the modify processing.
-      modifications = getModifications();
-      if (modifications == null)
-      {
-        break modifyProcessing;
-      }
-
-      if (modifications.isEmpty())
-      {
-        setResultCode(ResultCode.CONSTRAINT_VIOLATION);
-        appendErrorMessage(ERR_MODIFY_NO_MODIFICATIONS.get(
-                                String.valueOf(entryDN)));
-        break modifyProcessing;
-      }
-
-      // Check for a request to cancel this operation.
-      checkIfCanceled(false);
-
-      // Acquire a write lock on the target entry.
-      final Lock entryLock = LockManager.lockWrite(entryDN);
-      if (entryLock == null)
-      {
-        setResultCode(ResultCode.BUSY);
-        appendErrorMessage(ERR_MODIFY_CANNOT_LOCK_ENTRY.get(
-                                String.valueOf(entryDN)));
-        break modifyProcessing;
-      }
-
-
-      try
-      {
-        // Check for a request to cancel this operation.
-        checkIfCanceled(false);
-
-
-        try
-        {
-          // Get the entry to modify.  If it does not exist, then fail.
-          currentEntry = backend.getEntry(entryDN);
-
-          if (currentEntry == null)
-          {
-            setResultCode(ResultCode.NO_SUCH_OBJECT);
-            appendErrorMessage(ERR_MODIFY_NO_SUCH_ENTRY.get(
-                String.valueOf(entryDN)));
-
-            // See if one of the entry's ancestors exists.
-            try
-            {
-              DN parentDN = entryDN.getParentDNInSuffix();
-              while (parentDN != null)
-              {
-                if (DirectoryServer.entryExists(parentDN))
-                {
-                  setMatchedDN(parentDN);
-                  break;
-                }
-
-                parentDN = parentDN.getParentDNInSuffix();
-              }
-            }
-            catch (Exception e)
-            {
-              if (debugEnabled())
-              {
-                TRACER.debugCaught(DebugLogLevel.ERROR, e);
-              }
-            }
-
-            break modifyProcessing;
-          }
-
-          // Check to see if there are any controls in the request.  If so, then
-          // see if there is any special processing required.
-          processRequestControls();
-
-          // Get the password policy state object for the entry that can be used
-          // to perform any appropriate password policy processing.  Also, see
-          // if the entry is being updated by the end user or an administrator.
-          DN authzDN = getAuthorizationDN();
-          selfChange = entryDN.equals(authzDN);
-
-          // Check that the authorizing account isn't required to change its
-          // password.
-          if (( !isInternalOperation()) && !selfChange
-              && getAuthorizationEntry() != null) {
-            AuthenticationPolicy authzPolicy = AuthenticationPolicy.forUser(
-                getAuthorizationEntry(), true);
-            if (authzPolicy.isPasswordPolicy())
-            {
-              PasswordPolicyState authzState = (PasswordPolicyState) authzPolicy
-                  .createAuthenticationPolicyState(getAuthorizationEntry());
-              if (authzState.mustChangePassword())
-              {
-                pwpErrorType = PasswordPolicyErrorType.CHANGE_AFTER_RESET;
-                setResultCode(ResultCode.CONSTRAINT_VIOLATION);
-                appendErrorMessage(ERR_MODIFY_MUST_CHANGE_PASSWORD
-                    .get(authzDN != null ? String.valueOf(authzDN)
-                        : "anonymous"));
-                break modifyProcessing;
-              }
-            }
-          }
-
-          // FIXME -- Need a way to enable debug mode.
-          AuthenticationPolicy policy = AuthenticationPolicy.forUser(
-              currentEntry, true);
-          if (policy.isPasswordPolicy())
-          {
-            pwPolicyState = (PasswordPolicyState) policy
-                .createAuthenticationPolicyState(currentEntry);
-          }
-        }
-        catch (DirectoryException de)
-        {
-          if (debugEnabled())
-          {
-            TRACER.debugCaught(DebugLogLevel.ERROR, de);
-          }
-
-          setResponseData(de);
-          break modifyProcessing;
-        }
-
-
-        // Create a duplicate of the entry and apply the changes to it.
-        modifiedEntry = currentEntry.duplicate(false);
-
-        if (! noOp)
-        {
-            if(!handleConflictResolution()) {
-                break modifyProcessing;
-            }
-        }
-
-
-        try
-        {
-          handleSchemaProcessing();
-        }
-        catch (DirectoryException de)
-        {
-          if (debugEnabled())
-          {
-            TRACER.debugCaught(DebugLogLevel.ERROR, de);
-          }
-
-          setResponseData(de);
-          break modifyProcessing;
-        }
-
-
-        // Check to see if the client has permission to perform the modify.
-        // The access control check is not made any earlier because the handler
-        // needs access to the modified entry.
-
-        // FIXME: for now assume that this will check all permissions
-        // pertinent to the operation. This includes proxy authorization
-        // and any other controls specified.
-
-        // FIXME: earlier checks to see if the entry already exists may have
-        // already exposed sensitive information to the client.
-        try
-        {
-          if (!AccessControlConfigManager.getInstance()
-              .getAccessControlHandler().isAllowed(this))
-          {
-            setResultCode(ResultCode.INSUFFICIENT_ACCESS_RIGHTS);
-            appendErrorMessage(ERR_MODIFY_AUTHZ_INSUFFICIENT_ACCESS_RIGHTS
-                .get(String.valueOf(entryDN)));
-            break modifyProcessing;
-          }
-        }
-        catch (DirectoryException e)
-        {
-          setResultCode(e.getResultCode());
-          appendErrorMessage(e.getMessageObject());
-          break modifyProcessing;
-        }
-
-
-        try
-        {
-          handleInitialPasswordPolicyProcessing();
-          performAdditionalPasswordChangedProcessing();
-        }
-        catch (DirectoryException de)
-        {
-          if (debugEnabled())
-          {
-            TRACER.debugCaught(DebugLogLevel.ERROR, de);
-          }
-
-          setResponseData(de);
-          break modifyProcessing;
-        }
-
-
-        if ((!passwordChanged) && (!isInternalOperation()) && selfChange
-            && pwPolicyState != null && pwPolicyState.mustChangePassword())
-        {
-          // The user did not attempt to change their password.
-          pwpErrorType = PasswordPolicyErrorType.CHANGE_AFTER_RESET;
-          setResultCode(ResultCode.CONSTRAINT_VIOLATION);
-          DN authzDN = getAuthorizationDN();
-          appendErrorMessage(ERR_MODIFY_MUST_CHANGE_PASSWORD
-              .get(authzDN != null ? String.valueOf(authzDN) : "anonymous"));
-          break modifyProcessing;
-        }
-
-
-        // If the server is configured to check the schema and the
-        // operation is not a synchronization operation,
-        // make sure that the new entry is valid per the server schema.
-        if ((DirectoryServer.checkSchema()) && (! isSynchronizationOperation()))
-        {
-          MessageBuilder invalidReason = new MessageBuilder();
-          if (! modifiedEntry.conformsToSchema(null, false, false, false,
-              invalidReason))
-          {
-            setResultCode(ResultCode.OBJECTCLASS_VIOLATION);
-            appendErrorMessage(ERR_MODIFY_VIOLATES_SCHEMA.get(
-                                    String.valueOf(entryDN), invalidReason));
-            break modifyProcessing;
-          }
-        }
-
-
-        // Check for a request to cancel this operation.
-        checkIfCanceled(false);
-
-        // If the operation is not a synchronization operation,
-        // Invoke the pre-operation modify plugins.
-        if (! isSynchronizationOperation())
-        {
-          executePostOpPlugins = true;
-          PluginResult.PreOperation preOpResult =
-            pluginConfigManager.invokePreOperationModifyPlugins(this);
-          if (!preOpResult.continueProcessing())
-          {
-            setResultCode(preOpResult.getResultCode());
-            appendErrorMessage(preOpResult.getErrorMessage());
-            setMatchedDN(preOpResult.getMatchedDN());
-            setReferralURLs(preOpResult.getReferralURLs());
-            break modifyProcessing;
-          }
-        }
-
-
-        // Actually perform the modify operation.  This should also include
-        // taking care of any synchronization that might be needed.
-        if (backend == null)
-        {
-          setResultCode(ResultCode.NO_SUCH_OBJECT);
-          appendErrorMessage(ERR_MODIFY_NO_BACKEND_FOR_ENTRY.get(
-                                  String.valueOf(entryDN)));
-          break modifyProcessing;
-        }
-
-        try
-        {
-          try
-          {
-            checkWritability();
-          }
-          catch (DirectoryException de)
-          {
-            if (debugEnabled())
-            {
-              TRACER.debugCaught(DebugLogLevel.ERROR, de);
-            }
-
-            setResponseData(de);
-            break modifyProcessing;
-          }
-
-
-          if (noOp)
-          {
-            appendErrorMessage(INFO_MODIFY_NOOP.get());
-            setResultCode(ResultCode.NO_OPERATION);
-          }
-          else
-          {
-            if (!processPreOperation())
-            {
-              break modifyProcessing;
-            }
-
-            backend.replaceEntry(currentEntry, modifiedEntry, this);
-
-            // See if we need to generate any account status notifications as a
-            // result of the changes.
-            handleAccountStatusNotifications();
-          }
-
-
-          // Handle any processing that may be needed for the pre-read and/or
-          // post-read controls.
-          LocalBackendWorkflowElement.addPreReadResponse(this,
-              preReadRequest, currentEntry);
-          LocalBackendWorkflowElement.addPostReadResponse(this,
-              postReadRequest, modifiedEntry);
-
-
-          if (! noOp)
-          {
-            setResultCode(ResultCode.SUCCESS);
-          }
-        }
-        catch (DirectoryException de)
-        {
-          if (debugEnabled())
-          {
-            TRACER.debugCaught(DebugLogLevel.ERROR, de);
-          }
-
-          setResponseData(de);
-          break modifyProcessing;
-        }
-      }
-      finally
-      {
-        LockManager.unlock(entryDN, entryLock);
-        processSynchPostOperationPlugins();
-      }
-    }
+    BooleanHolder executePostOpPlugins = new BooleanHolder();
+    processModify(executePostOpPlugins);
 
     // If the password policy request control was included, then make sure we
     // send the corresponding response control.
@@ -652,6 +307,8 @@
     }
 
     // Invoke the post-operation or post-synchronization modify plugins.
+    PluginConfigManager pluginConfigManager =
+        DirectoryServer.getPluginConfigManager();
     if (isSynchronizationOperation())
     {
       if (getResultCode() == ResultCode.SUCCESS)
@@ -659,7 +316,7 @@
         pluginConfigManager.invokePostSynchronizationModifyPlugins(this);
       }
     }
-    else if (executePostOpPlugins)
+    else if (executePostOpPlugins.value)
     {
       // FIXME -- Should this also be done while holding the locks?
       PluginResult.PostOperation postOpResult =
@@ -720,6 +377,278 @@
   }
 
 
+  private void processModify(BooleanHolder executePostOpPlugins)
+      throws CanceledOperationException
+  {
+    entryDN = getEntryDN();
+    if (entryDN == null)
+    {
+      return;
+    }
+
+    // Process the modifications to convert them from their raw form to the
+    // form required for the rest of the modify processing.
+    modifications = getModifications();
+    if (modifications == null)
+    {
+      return;
+    }
+
+    if (modifications.isEmpty())
+    {
+      setResultCode(ResultCode.CONSTRAINT_VIOLATION);
+      appendErrorMessage(ERR_MODIFY_NO_MODIFICATIONS.get(String
+          .valueOf(entryDN)));
+      return;
+    }
+
+    // Check for a request to cancel this operation.
+    checkIfCanceled(false);
+
+    // Acquire a write lock on the target entry.
+    final Lock entryLock = LockManager.lockWrite(entryDN);
+    if (entryLock == null)
+    {
+      setResultCode(ResultCode.BUSY);
+      appendErrorMessage(ERR_MODIFY_CANNOT_LOCK_ENTRY.get(String
+          .valueOf(entryDN)));
+      return;
+    }
+
+    try
+    {
+      // Check for a request to cancel this operation.
+      checkIfCanceled(false);
+
+      // Get the entry to modify. If it does not exist, then fail.
+      currentEntry = backend.getEntry(entryDN);
+
+      if (currentEntry == null)
+      {
+        setResultCode(ResultCode.NO_SUCH_OBJECT);
+        appendErrorMessage(ERR_MODIFY_NO_SUCH_ENTRY
+            .get(String.valueOf(entryDN)));
+
+        // See if one of the entry's ancestors exists.
+        try
+        {
+          DN parentDN = entryDN.getParentDNInSuffix();
+          while (parentDN != null)
+          {
+            if (DirectoryServer.entryExists(parentDN))
+            {
+              setMatchedDN(parentDN);
+              break;
+            }
+
+            parentDN = parentDN.getParentDNInSuffix();
+          }
+        }
+        catch (Exception e)
+        {
+          if (debugEnabled())
+          {
+            TRACER.debugCaught(DebugLogLevel.ERROR, e);
+          }
+        }
+
+        return;
+      }
+
+      // Check to see if there are any controls in the request. If so, then
+      // see if there is any special processing required.
+      processRequestControls();
+
+      // Get the password policy state object for the entry that can be used
+      // to perform any appropriate password policy processing. Also, see
+      // if the entry is being updated by the end user or an administrator.
+      final DN authzDN = getAuthorizationDN();
+      selfChange = entryDN.equals(authzDN);
+
+      // Check that the authorizing account isn't required to change its
+      // password.
+      if ((!isInternalOperation()) && !selfChange
+          && getAuthorizationEntry() != null)
+      {
+        AuthenticationPolicy authzPolicy =
+            AuthenticationPolicy.forUser(getAuthorizationEntry(), true);
+        if (authzPolicy.isPasswordPolicy())
+        {
+          PasswordPolicyState authzState =
+              (PasswordPolicyState) authzPolicy
+                  .createAuthenticationPolicyState(getAuthorizationEntry());
+          if (authzState.mustChangePassword())
+          {
+            pwpErrorType = PasswordPolicyErrorType.CHANGE_AFTER_RESET;
+            setResultCode(ResultCode.CONSTRAINT_VIOLATION);
+            appendErrorMessage(ERR_MODIFY_MUST_CHANGE_PASSWORD
+                .get(authzDN != null ? String.valueOf(authzDN) : "anonymous"));
+            return;
+          }
+        }
+      }
+
+      // FIXME -- Need a way to enable debug mode.
+      AuthenticationPolicy policy =
+          AuthenticationPolicy.forUser(currentEntry, true);
+      if (policy.isPasswordPolicy())
+      {
+        pwPolicyState =
+            (PasswordPolicyState) policy
+                .createAuthenticationPolicyState(currentEntry);
+      }
+
+      // Create a duplicate of the entry and apply the changes to it.
+      modifiedEntry = currentEntry.duplicate(false);
+
+      if (!noOp)
+      {
+        if (!handleConflictResolution())
+        {
+          return;
+        }
+      }
+
+      handleSchemaProcessing();
+
+      // Check to see if the client has permission to perform the modify.
+      // The access control check is not made any earlier because the handler
+      // needs access to the modified entry.
+
+      // FIXME: for now assume that this will check all permissions
+      // pertinent to the operation. This includes proxy authorization
+      // and any other controls specified.
+
+      // FIXME: earlier checks to see if the entry already exists may have
+      // already exposed sensitive information to the client.
+      try
+      {
+        if (!AccessControlConfigManager.getInstance().getAccessControlHandler()
+            .isAllowed(this))
+        {
+          setResultCode(ResultCode.INSUFFICIENT_ACCESS_RIGHTS);
+          appendErrorMessage(ERR_MODIFY_AUTHZ_INSUFFICIENT_ACCESS_RIGHTS
+              .get(String.valueOf(entryDN)));
+          return;
+        }
+      }
+      catch (DirectoryException e)
+      {
+        setResultCode(e.getResultCode());
+        appendErrorMessage(e.getMessageObject());
+        return;
+      }
+
+      handleInitialPasswordPolicyProcessing();
+      performAdditionalPasswordChangedProcessing();
+
+      if ((!passwordChanged) && (!isInternalOperation()) && selfChange
+          && pwPolicyState != null && pwPolicyState.mustChangePassword())
+      {
+        // The user did not attempt to change their password.
+        pwpErrorType = PasswordPolicyErrorType.CHANGE_AFTER_RESET;
+        setResultCode(ResultCode.CONSTRAINT_VIOLATION);
+        appendErrorMessage(ERR_MODIFY_MUST_CHANGE_PASSWORD
+            .get(authzDN != null ? String.valueOf(authzDN) : "anonymous"));
+        return;
+      }
+
+      // If the server is configured to check the schema and the
+      // operation is not a synchronization operation,
+      // make sure that the new entry is valid per the server schema.
+      if ((DirectoryServer.checkSchema()) && (!isSynchronizationOperation()))
+      {
+        MessageBuilder invalidReason = new MessageBuilder();
+        if (!modifiedEntry.conformsToSchema(null, false, false, false,
+            invalidReason))
+        {
+          setResultCode(ResultCode.OBJECTCLASS_VIOLATION);
+          appendErrorMessage(ERR_MODIFY_VIOLATES_SCHEMA.get(String
+              .valueOf(entryDN), invalidReason));
+          return;
+        }
+      }
+
+      // Check for a request to cancel this operation.
+      checkIfCanceled(false);
+
+      // If the operation is not a synchronization operation,
+      // Invoke the pre-operation modify plugins.
+      if (!isSynchronizationOperation())
+      {
+        executePostOpPlugins.value = true;
+        PluginResult.PreOperation preOpResult =
+            DirectoryServer.getPluginConfigManager()
+                .invokePreOperationModifyPlugins(this);
+        if (!preOpResult.continueProcessing())
+        {
+          setResultCode(preOpResult.getResultCode());
+          appendErrorMessage(preOpResult.getErrorMessage());
+          setMatchedDN(preOpResult.getMatchedDN());
+          setReferralURLs(preOpResult.getReferralURLs());
+          return;
+        }
+      }
+
+      // Actually perform the modify operation. This should also include
+      // taking care of any synchronization that might be needed.
+      if (backend == null)
+      {
+        setResultCode(ResultCode.NO_SUCH_OBJECT);
+        appendErrorMessage(ERR_MODIFY_NO_BACKEND_FOR_ENTRY.get(String
+            .valueOf(entryDN)));
+        return;
+      }
+
+      checkWritability();
+
+      if (noOp)
+      {
+        appendErrorMessage(INFO_MODIFY_NOOP.get());
+        setResultCode(ResultCode.NO_OPERATION);
+      }
+      else
+      {
+        if (!processPreOperation())
+        {
+          return;
+        }
+
+        backend.replaceEntry(currentEntry, modifiedEntry, this);
+
+        // See if we need to generate any account status notifications as a
+        // result of the changes.
+        handleAccountStatusNotifications();
+      }
+
+      // Handle any processing that may be needed for the pre-read and/or
+      // post-read controls.
+      LocalBackendWorkflowElement.addPreReadResponse(this, preReadRequest,
+          currentEntry);
+      LocalBackendWorkflowElement.addPostReadResponse(this, postReadRequest,
+          modifiedEntry);
+
+      if (!noOp)
+      {
+        setResultCode(ResultCode.SUCCESS);
+      }
+    }
+    catch (DirectoryException de)
+    {
+      if (debugEnabled())
+      {
+        TRACER.debugCaught(DebugLogLevel.ERROR, de);
+      }
+
+      setResponseData(de);
+      return;
+    }
+    finally
+    {
+      LockManager.unlock(entryDN, entryLock);
+      processSynchPostOperationPlugins();
+    }
+  }
 
   /**
    * Processes any controls contained in the modify request.
@@ -1490,6 +1419,8 @@
         {
           if (!syntax.valueIsAcceptable(v.getValue(), invalidReason))
           {
+            // FIXME remove next line of code. According to Matt, since this is
+            // just a warning, the code should not set the resultCode
             setResultCode(ResultCode.INVALID_ATTRIBUTE_SYNTAX);
             if (!syntax.isHumanReadable() || syntax.isBinary())
             {
@@ -1520,23 +1451,16 @@
     // Add the provided attribute or merge an existing attribute with
     // the values of the new attribute. If there are any duplicates,
     // then fail.
-    LinkedList<AttributeValue> duplicateValues =
+    List<AttributeValue> duplicateValues =
       new LinkedList<AttributeValue>();
     modifiedEntry.addAttribute(attr, duplicateValues);
     if (!duplicateValues.isEmpty() && !permissiveModify)
     {
-      StringBuilder buffer = new StringBuilder();
-      Iterator<AttributeValue> iterator = duplicateValues.iterator();
-      buffer.append(iterator.next().getValue().toString());
-      while (iterator.hasNext())
-      {
-        buffer.append(", ");
-        buffer.append(iterator.next().getValue().toString());
-      }
+      String duplicateValuesStr = collectionToString(duplicateValues, ", ");
 
       throw new DirectoryException(ResultCode.ATTRIBUTE_OR_VALUE_EXISTS,
           ERR_MODIFY_ADD_DUPLICATE_VALUE.get(String.valueOf(entryDN), attr
-              .getName(), buffer));
+              .getName(), duplicateValuesStr));
     }
   }
 
@@ -1609,7 +1533,7 @@
     // Remove the specified attribute values or the entire attribute from the
     // value.  If there are any specified values that were not present, then
     // fail.  If the RDN attribute value would be removed, then fail.
-    LinkedList<AttributeValue> missingValues = new LinkedList<AttributeValue>();
+    List<AttributeValue> missingValues = new LinkedList<AttributeValue>();
     boolean attrExists = modifiedEntry.removeAttribute(attr, missingValues);
 
     if (attrExists)
@@ -1633,18 +1557,12 @@
       {
         if (! permissiveModify)
         {
-          StringBuilder buffer = new StringBuilder();
-          Iterator<AttributeValue> iterator = missingValues.iterator();
-          buffer.append(iterator.next().getValue().toString());
-          while (iterator.hasNext())
-          {
-            buffer.append(", ");
-            buffer.append(iterator.next().getValue().toString());
-          }
+          String missingValuesStr = collectionToString(missingValues, ", ");
 
           throw new DirectoryException(ResultCode.NO_SUCH_ATTRIBUTE,
                        ERR_MODIFY_DELETE_MISSING_VALUES.get(
-                            String.valueOf(entryDN), attr.getName(), buffer));
+                            String.valueOf(entryDN), attr.getName(),
+                            missingValuesStr));
         }
       }
     }
@@ -2264,5 +2182,3 @@
       }
   }
 }
-
-

--
Gitblit v1.10.0