From 8131d3a858b60cb97fe3f022d8fd399cba52f368 Mon Sep 17 00:00:00 2001
From: Jean-Noel Rouvignac <jean-noel.rouvignac@forgerock.com>
Date: Wed, 19 Nov 2014 13:45:12 +0000
Subject: [PATCH] More code cleanup

---
 opendj3-server-dev/src/server/org/opends/server/core/WorkflowTopologyNode.java       |  125 +++++++++----------------------
 opendj3-server-dev/src/server/org/opends/server/core/networkgroups/NetworkGroup.java |   72 ++++++++----------
 opendj3-server-dev/src/server/org/opends/server/core/Workflow.java                   |   30 ++-----
 3 files changed, 78 insertions(+), 149 deletions(-)

diff --git a/opendj3-server-dev/src/server/org/opends/server/core/Workflow.java b/opendj3-server-dev/src/server/org/opends/server/core/Workflow.java
index 7e1b6af..ecab32b 100644
--- a/opendj3-server-dev/src/server/org/opends/server/core/Workflow.java
+++ b/opendj3-server-dev/src/server/org/opends/server/core/Workflow.java
@@ -22,35 +22,25 @@
  *
  *
  *      Copyright 2008 Sun Microsystems, Inc.
+ *      Portions Copyright 2014 ForgeRock AS
  */
 package org.opends.server.core;
 
-
 import org.opends.server.types.DN;
 import org.opends.server.types.Operation;
 import org.opends.server.types.CanceledOperationException;
 
-
 /**
- * This class defines the workflow interface. There can be two
- * implementations for the workflows.
+ * This class defines the workflow interface.
  *
- * In the first workflow implementation a workflow is a list of
- * structured tasks (aka workflow element). Each task is working
- * on a set of data being identified by a base DN. The order of the
- * tasks and their synchronization are defined statically by a task
- * tree.
- *
- * In the second workflow implementation each workflow is a node
- * in a workflow tree (aka worflow topology). Each node in the tree
- * is linked to a workflow object of the first implementation and the
- * base DN of the node is the base DN of the attached workflow object.
- * The relationship of the nodes in the tree is based on the base DNs
- * of the nodes. A workflow node is a subordinate of another workflow
- * node when the base DN of the former is a superior of the base DN of
- * the latter. Workflow topology are useful, for example, in subtree
- * searches: search is performed on a node as well as on all the
- * subordinate nodes.
+ * Each workflow is a node in a workflow tree (aka workflow topology).
+ * Each node in the tree is linked to a workflow object of the first implementation
+ * and the base DN of the node is the base DN of the attached workflow object.
+ * The relationship of the nodes in the tree is based on the base DNs of the nodes.
+ * A workflow node is a subordinate of another workflow node when the base DN
+ * of the former is a superior of the base DN of the latter.
+ * Workflow topology are useful, for example, in subtree searches:
+ * search is performed on a node as well as on all the subordinate nodes.
  */
 public interface Workflow
 {
diff --git a/opendj3-server-dev/src/server/org/opends/server/core/WorkflowTopologyNode.java b/opendj3-server-dev/src/server/org/opends/server/core/WorkflowTopologyNode.java
index 886167b..056d730 100644
--- a/opendj3-server-dev/src/server/org/opends/server/core/WorkflowTopologyNode.java
+++ b/opendj3-server-dev/src/server/org/opends/server/core/WorkflowTopologyNode.java
@@ -146,7 +146,7 @@
       }
 
       // If the request base DN is not a subordinate of the subordinate
-      // workflow base DN then don't search in the subordinate workflow.
+      // workflow base DN then do not search in the subordinate workflow.
       if (! originalBaseDN.isAncestorOf(subordinateDN))
       {
         continue;
@@ -228,38 +228,28 @@
       return null;
     }
 
-    // parent base DN to return
-    DN parentBaseDN = null;
-
     // Is the dn a subordinate of the current base DN?
     DN curBaseDN = getBaseDN();
     if (curBaseDN != null && dn.isDescendantOf(curBaseDN))
     {
       // The dn may be handled by the current workflow.
-      // Now we have to check whether the dn is handled by
-      // a subordinate.
+      // Now we have to check whether the dn is handled by a subordinate.
       for (WorkflowTopologyNode subordinate: getSubordinates())
       {
-        parentBaseDN = subordinate.getParentBaseDN(dn);
+        final DN parentBaseDN = subordinate.getParentBaseDN(dn);
         if (parentBaseDN != null)
         {
           // the dn is handled by a subordinate
-          break;
+          return parentBaseDN;
         }
       }
 
-      // If the dn is not handled by any subordinate, then it is
-      // handled by the current workflow.
-      if (parentBaseDN == null)
-      {
-        parentBaseDN = curBaseDN;
-      }
+      // no subordinate handle the DN, then it is handled by the current workflow
+      return curBaseDN;
     }
-
-    return parentBaseDN;
+    return null;
   }
 
-
   /**
    * Adds a workflow to the list of workflow subordinates without
    * additional control.
@@ -267,10 +257,7 @@
    * @param newWorkflow     the workflow to add to the subordinate list
    * @param parentWorkflow  the parent workflow of the new workflow
    */
-  private void addSubordinateNoCheck(
-      WorkflowTopologyNode newWorkflow,
-      WorkflowTopologyNode parentWorkflow
-      )
+  private void addSubordinateNoCheck(WorkflowTopologyNode newWorkflow, WorkflowTopologyNode parentWorkflow)
   {
     subordinates.add(newWorkflow);
     newWorkflow.setParent(parentWorkflow);
@@ -286,13 +273,11 @@
    *
    * @param newWorkflow  the workflow to add in the subordinate list
    */
-  private void addSubordinate(
-      WorkflowTopologyNode newWorkflow
-      )
+  private void addSubordinate(WorkflowTopologyNode newWorkflow)
   {
-    // Don't try to add the workflow to itself.
     if (newWorkflow == this)
     {
+      // Do not try to add the workflow to itself.
       return;
     }
 
@@ -306,9 +291,8 @@
       DN newDN = newWorkflow.getBaseDN();
       DN subordinateDN = curSubordinate.getBaseDN();
 
-      // Don't try to add workflow when baseDNs are
-      // the same on both workflows.
       if (newDN.equals(subordinateDN)) {
+        // Do not try to add workflow when baseDNs are the same on both workflows.
         return;
       }
 
@@ -344,54 +328,30 @@
    * @return <code>true</code> if the new workflow has been inserted
    *         in any subordinate list
    */
-  public boolean insertSubordinate(
-      WorkflowTopologyNode newWorkflow
-      )
+  public boolean insertSubordinate(WorkflowTopologyNode newWorkflow)
   {
-    // don't try to insert the workflow in itself!
-    if (newWorkflow == this)
-    {
-      return false;
-    }
-
-    // the returned status
-    boolean insertDone = false;
-
     DN parentBaseDN = getBaseDN();
-    DN newBaseDN    = newWorkflow.getBaseDN();
-
-    // don't try to insert workflows when baseDNs are the same on both
-    // workflows
-    if (parentBaseDN.equals(newBaseDN))
+    DN newBaseDN = newWorkflow.getBaseDN();
+    if (newWorkflow == this
+        || parentBaseDN.equals(newBaseDN)
+        || !newBaseDN.isDescendantOf(parentBaseDN))
     {
       return false;
     }
 
-    // try to insert the new workflow
-    if (newBaseDN.isDescendantOf(parentBaseDN))
+    // the new workflow is a subordinate for this parent DN
+    for (WorkflowTopologyNode subordinate : getSubordinates())
     {
-      // the new workflow is a subordinate for this parent DN, let's
-      // insert the new workflow in the list of subordinates
-      for (WorkflowTopologyNode subordinate: getSubordinates())
+      if (subordinate.insertSubordinate(newWorkflow))
       {
-        insertDone = subordinate.insertSubordinate(newWorkflow);
-        if (insertDone)
-        {
-          // the newBaseDN is handled by a subordinate
-          break;
-        }
-      }
-
-      // if the newBaseDN is not handled by a subordinate then the workflow
-      // is inserted it in the current workflow subordinate list
-      if (! insertDone)
-      {
-        addSubordinate(newWorkflow);
-        insertDone = true;
+        // the newBaseDN is handled by a subordinate
+        return true;
       }
     }
 
-    return insertDone;
+    // no subordinate handle the newBaseDN, then it is handled by the current workflow
+    addSubordinate(newWorkflow);
+    return true;
   }
 
 
@@ -452,41 +412,28 @@
    * @return the highest workflow that can handle the requestDN
    *         <code>null</code> if none was found
    */
-  public WorkflowTopologyNode getWorkflowCandidate(
-      DN requestDN
-      )
+  public WorkflowTopologyNode getWorkflowCandidate(DN requestDN)
   {
-    // the returned workflow
-    WorkflowTopologyNode workflowCandidate = null;
-
-    // does the current workflow handle the request baseDN?
     DN baseDN = getParentBaseDN(requestDN);
     if (baseDN == null)
     {
-      // the current workflow does not handle the requestDN,
-      // let's return null
+      // the current workflow does not handle the requestDN
+      return null;
     }
-    else
+
+    // is there any subordinate that can handle the requestDN?
+    for (WorkflowTopologyNode subordinate : getSubordinates())
     {
-      // is there any subordinate that can handle the requestDN?
-      for (WorkflowTopologyNode subordinate: getSubordinates())
+      WorkflowTopologyNode candidate = subordinate.getWorkflowCandidate(requestDN);
+      if (candidate != null)
       {
-        workflowCandidate = subordinate.getWorkflowCandidate(requestDN);
-        if (workflowCandidate != null)
-        {
-          break;
-        }
-      }
-
-      // none of the subordinates can handle the requestDN, so the current
-      // workflow is the best root workflow candidate
-      if (workflowCandidate == null)
-      {
-        workflowCandidate = this;
+        return candidate;
       }
     }
 
-    return workflowCandidate;
+    // none of the subordinates can handle the requestDN, so the current
+    // workflow is the best root workflow candidate
+    return this;
   }
 
 
diff --git a/opendj3-server-dev/src/server/org/opends/server/core/networkgroups/NetworkGroup.java b/opendj3-server-dev/src/server/org/opends/server/core/networkgroups/NetworkGroup.java
index 76f0aa4..6318d85 100644
--- a/opendj3-server-dev/src/server/org/opends/server/core/networkgroups/NetworkGroup.java
+++ b/opendj3-server-dev/src/server/org/opends/server/core/networkgroups/NetworkGroup.java
@@ -60,8 +60,7 @@
    * they can be attached to a specific network group.
    */
   private static final String DEFAULT_NETWORK_GROUP_NAME = "default";
-  private static NetworkGroup defaultNetworkGroup =
-      new NetworkGroup(DEFAULT_NETWORK_GROUP_NAME);
+  private static NetworkGroup defaultNetworkGroup = new NetworkGroup(DEFAULT_NETWORK_GROUP_NAME);
 
   /**
    * Deregisters all network groups that have been registered. This
@@ -88,8 +87,7 @@
   }
 
   /** List of naming contexts handled by the network group. */
-  private NetworkGroupNamingContexts namingContexts =
-      new NetworkGroupNamingContexts();
+  private NetworkGroupNamingContexts namingContexts = new NetworkGroupNamingContexts();
 
   /** The network group internal identifier. */
   private final String networkGroupID;
@@ -137,30 +135,32 @@
 
     if (baseDN.isRootDN())
     {
-      // deregister the rootDSE
       deregisterWorkflow(rootDSEWorkflowNode);
     }
     else
     {
-      // deregister a workflow node
-      synchronized (registeredWorkflowNodesLock)
+      WorkflowTopologyNode node = findWorkflowNode(baseDN);
+      if (node != null)
       {
-        for (WorkflowTopologyNode node : registeredWorkflowNodes.values())
-        {
-          DN curDN = node.getBaseDN();
-          if (curDN.equals(baseDN))
-          {
-            // Call deregisterWorkflow() instead of
-            // deregisterWorkflowNode() because we want the naming
-            // context list to be updated as well.
-            deregisterWorkflow(node);
+        // Call deregisterWorkflow() instead of deregisterWorkflowNode()
+        // because we want the naming context list to be updated as well.
+        deregisterWorkflow(node);
+      }
+    }
+  }
 
-            // Only one workflow can match the baseDN, so we can break
-            // the loop here.
-            break;
-          }
+  private WorkflowTopologyNode findWorkflowNode(DN baseDN)
+  {
+    synchronized (registeredWorkflowNodesLock)
+    {
+      for (WorkflowTopologyNode node : registeredWorkflowNodes.values())
+      {
+        if (node.getBaseDN().equals(baseDN))
+        {
+          return node;
         }
       }
+      return null;
     }
   }
 
@@ -237,15 +237,12 @@
 
     // The workflow base DN should not be already present in the
     // network group. Bypass the check for the private workflows...
-    for (WorkflowTopologyNode node : registeredWorkflowNodes.values())
+    WorkflowTopologyNode node = findWorkflowNode(workflowNode.getBaseDN());
+    if (node != null)
     {
-      DN nodeBaseDN = node.getBaseDN();
-      if (nodeBaseDN.equals(workflowNode.getBaseDN()))
-      {
-        LocalizableMessage message = ERR_REGISTER_WORKFLOW_BASE_DN_ALREADY_EXISTS.get(
-            workflowID, networkGroupID, workflowID, workflowNode.getBaseDN());
-        throw new DirectoryException(ResultCode.UNWILLING_TO_PERFORM, message);
-      }
+      LocalizableMessage message = ERR_REGISTER_WORKFLOW_BASE_DN_ALREADY_EXISTS.get(
+          workflowID, networkGroupID, node.getWorkflowId(), workflowNode.getBaseDN());
+      throw new DirectoryException(ResultCode.UNWILLING_TO_PERFORM, message);
     }
   }
 
@@ -257,13 +254,13 @@
    */
   private void deregisterWorkflow(Workflow workflow)
   {
-    if (workflow == rootDSEWorkflowNode)
+    if (rootDSEWorkflowNode == workflow)
     {
       rootDSEWorkflowNode = null;
       return;
     }
 
-    WorkflowTopologyNode workflowNode = (WorkflowTopologyNode) workflow;
+    final WorkflowTopologyNode workflowNode = (WorkflowTopologyNode) workflow;
     deregisterWorkflowNode(workflowNode);
     // Remove it from the workflow topology.
     workflowNode.remove();
@@ -312,7 +309,6 @@
    */
   private void rebuildNamingContextList()
   {
-    // reset lists of naming contexts
     namingContexts.resetLists();
 
     for (WorkflowTopologyNode workflowNode : registeredWorkflowNodes.values())
@@ -360,14 +356,11 @@
     // Now add the workflow in the workflow topology...
     for (WorkflowTopologyNode curNode : registeredWorkflowNodes.values())
     {
-      if (
-          // Try to insert the new workflow under an existing workflow...
-          curNode.insertSubordinate(workflowNode)
-          // ... or try to insert the existing workflow below the new workflow
-          || workflowNode.insertSubordinate(curNode))
+      // Try to insert the new workflow under an existing workflow...
+      if (!curNode.insertSubordinate(workflowNode))
       {
-        // new workflow has been inserted in the topology
-        continue;
+        // ... or try to insert the existing workflow below the new workflow
+        workflowNode.insertSubordinate(curNode);
       }
     }
 
@@ -402,8 +395,7 @@
 
       // All is fine, let's register the workflow
       TreeMap<String, WorkflowTopologyNode> newRegisteredWorkflowNodes =
-          new TreeMap<String, WorkflowTopologyNode>(
-              registeredWorkflowNodes);
+          new TreeMap<String, WorkflowTopologyNode>(registeredWorkflowNodes);
       newRegisteredWorkflowNodes.put(workflowID, workflowNode);
       registeredWorkflowNodes = newRegisteredWorkflowNodes;
     }

--
Gitblit v1.10.0