From e61d6d0a55d022a0768e730521bfea36ad506f9d Mon Sep 17 00:00:00 2001
From: Jean-Noël Rouvignac <jean-noel.rouvignac@forgerock.com>
Date: Wed, 13 Jul 2016 14:16:36 +0000
Subject: [PATCH] dsreplication: Code cleanup

---
 opendj-server-legacy/src/main/java/org/opends/server/tools/dsreplication/ReplicationCliMain.java |   68 ++----
 opendj-server-legacy/src/main/java/org/opends/admin/ads/ReplicaDescriptor.java                   |   33 ++
 opendj-server-legacy/src/main/java/org/opends/admin/ads/SuffixDescriptor.java                    |   31 ++-
 opendj-server-legacy/src/main/java/org/opends/admin/ads/util/PreferredConnection.java            |    2 
 opendj-server-legacy/src/main/java/org/opends/admin/ads/ADSContext.java                          |   18 -
 opendj-server-legacy/src/main/java/org/opends/admin/ads/util/ServerLoader.java                   |   21 -
 opendj-server-legacy/src/main/java/org/opends/admin/ads/TopologyCache.java                       |  118 +++++++++--
 opendj-server-legacy/src/main/java/org/opends/server/api/MonitorData.java                        |    2 
 opendj-server-legacy/src/main/java/org/opends/admin/ads/ServerDescriptor.java                    |  299 +++++++++++++++++++----------
 9 files changed, 368 insertions(+), 224 deletions(-)

diff --git a/opendj-server-legacy/src/main/java/org/opends/admin/ads/ADSContext.java b/opendj-server-legacy/src/main/java/org/opends/admin/ads/ADSContext.java
index 0645cbe..c0d4835 100644
--- a/opendj-server-legacy/src/main/java/org/opends/admin/ads/ADSContext.java
+++ b/opendj-server-legacy/src/main/java/org/opends/admin/ads/ADSContext.java
@@ -91,7 +91,7 @@
   /** Enumeration containing the different server properties that are stored in the ADS. */
   public enum ServerProperty
   {
-    /** The ID used to identify the server. */
+    /** The ID used to identify the server (hostname + port). */
     ID("id",ADSPropertySyntax.STRING),
     /** The host name of the server. */
     HOST_NAME("hostname",ADSPropertySyntax.STRING),
@@ -379,7 +379,7 @@
       }
       groupList.add(ALL_SERVERGROUP_NAME);
       serverProperties.put(ServerProperty.GROUPS, groupList);
-      updateServer(serverProperties, null);
+      updateServer(serverProperties);
     }
     catch (ADSContextException ace)
     {
@@ -405,23 +405,13 @@
    * @throws ADSContextException
    *           if the server could not be registered.
    */
-  private void updateServer(Map<ServerProperty, Object> serverProperties, String newServerId)
+  private void updateServer(Map<ServerProperty, Object> serverProperties)
       throws ADSContextException
   {
     DN dn = makeDNFromServerProperties(serverProperties);
 
     try
     {
-      if (newServerId != null)
-      {
-        Map<ServerProperty, Object> newServerProps = new HashMap<>(serverProperties);
-        newServerProps.put(ServerProperty.ID, newServerId);
-        DN newDn = makeDNFromServerProperties(newServerProps);
-        throwIfNotSuccess(connectionWrapper.getConnection().modifyDN(dn.toString(), newDn.toString()));
-        dn = newDn;
-        serverProperties.put(ServerProperty.ID, newServerId);
-      }
-
       ModifyRequest request = newModifyRequest(dn);
       for (ServerProperty prop : serverProperties.keySet())
       {
@@ -596,7 +586,7 @@
     {
       if (x.getError() == ErrorType.ALREADY_REGISTERED)
       {
-        updateServer(serverProperties, null);
+        updateServer(serverProperties);
         return 1;
       }
 
diff --git a/opendj-server-legacy/src/main/java/org/opends/admin/ads/ReplicaDescriptor.java b/opendj-server-legacy/src/main/java/org/opends/admin/ads/ReplicaDescriptor.java
index e160f10..e9adff3 100644
--- a/opendj-server-legacy/src/main/java/org/opends/admin/ads/ReplicaDescriptor.java
+++ b/opendj-server-legacy/src/main/java/org/opends/admin/ads/ReplicaDescriptor.java
@@ -19,14 +19,19 @@
 import java.util.HashSet;
 import java.util.Set;
 
-
-/** The object of this class represent a Replica (i.e. a suffix in a given server). */
+/**
+ * The object of this class represent a Replica, i.e. a suffix in a given server instance.
+ * <p>
+ * Note: this does not represent a replication server.
+ */
 public class ReplicaDescriptor
 {
   private SuffixDescriptor suffix;
-  private int entries = -1;
+  /** Number of entries held by this replica. */
+  private int nbEntries = -1;
   private ServerDescriptor server;
   private final Set<String> replicationServers = new HashSet<>();
+  /** This corresponds to the server-id of this replica. */
   private int replicationId = -1;
   private int missingChanges = -1;
   private long ageOfOldestMissingChange = -1;
@@ -39,7 +44,7 @@
    */
   public int getEntries()
   {
-    return entries;
+    return nbEntries;
   }
 
   /**
@@ -64,11 +69,11 @@
 
   /**
    * Sets the number of entries contained in the replica.
-   * @param entries the number of entries contained in the replica.
+   * @param nbEntries the number of entries contained in the replica.
    */
-  public void setEntries(int entries)
+  public void setEntries(int nbEntries)
   {
-    this.entries = entries;
+    this.nbEntries = nbEntries;
   }
 
   /**
@@ -234,4 +239,18 @@
   {
     this.objectClasses = objectClasses;
   }
+
+  @Override
+  public String toString()
+  {
+    return getClass().getSimpleName()
+        + "(domain-name=" + suffix.getDN()
+        + ", server-id=" + replicationId
+        + ", host-name=" + server.getReplicationServerHostPort()
+        + ", nb-entries=" + nbEntries
+        + ", rs-port=" + server.getReplicationServerPort()
+        + ", missing-changes=" + missingChanges
+        + ", age-of-oldest-missing-change=" + ageOfOldestMissingChange
+        + ")";
+  }
 }
diff --git a/opendj-server-legacy/src/main/java/org/opends/admin/ads/ServerDescriptor.java b/opendj-server-legacy/src/main/java/org/opends/admin/ads/ServerDescriptor.java
index ab7225e..7afa413 100644
--- a/opendj-server-legacy/src/main/java/org/opends/admin/ads/ServerDescriptor.java
+++ b/opendj-server-legacy/src/main/java/org/opends/admin/ads/ServerDescriptor.java
@@ -27,6 +27,7 @@
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 
 import javax.naming.ldap.Rdn;
@@ -42,12 +43,17 @@
 import org.forgerock.opendj.ldap.requests.SearchRequest;
 import org.forgerock.opendj.ldap.responses.SearchResultEntry;
 import org.forgerock.opendj.ldif.ConnectionEntryReader;
+import org.forgerock.util.Pair;
 import org.opends.admin.ads.util.ConnectionWrapper;
 import org.opends.quicksetup.Constants;
 import org.opends.server.config.ConfigConstants;
 import org.opends.server.types.HostPort;
 
-/** The object of this class represent an OpenDS server. */
+/**
+ * The object of this class represent an OpenDS server instance.
+ * <p>
+ * It can represent either a DS-only, a RS-only or a combined DS-RS.
+ */
 public class ServerDescriptor
 {
   private static final LocalizedLogger logger = LocalizedLogger.getLoggerForThisClass();
@@ -65,48 +71,55 @@
   public enum ServerProperty
   {
     /** The associated value is a String. */
-    HOST_NAME,
-    /** The associated value is an ArrayList of Integer. */
-    LDAP_PORT,
-    /** The associated value is an ArrayList of Integer. */
-    LDAPS_PORT,
+    HOST_NAME(ADSContext.ServerProperty.HOST_NAME),
+    /** The associated value is an List of Integer. */
+    LDAP_PORT(ADSContext.ServerProperty.LDAP_PORT),
+    /** The associated value is an List of Integer. */
+    LDAPS_PORT(ADSContext.ServerProperty.LDAPS_PORT),
     /** The associated value is an Integer. */
-    ADMIN_PORT,
-    /** The associated value is an ArrayList of Boolean. */
-    LDAP_ENABLED,
-    /** The associated value is an ArrayList of Boolean. */
-    LDAPS_ENABLED,
-    /** The associated value is an ArrayList of Boolean. */
-    ADMIN_ENABLED,
-    /** The associated value is an ArrayList of Boolean. */
-    STARTTLS_ENABLED,
-    /** The associated value is an ArrayList of Integer. */
-    JMX_PORT,
-    /** The associated value is an ArrayList of Integer. */
-    JMXS_PORT,
-    /** The associated value is an ArrayList of Boolean. */
-    JMX_ENABLED,
-    /** The associated value is an ArrayList of Boolean. */
-    JMXS_ENABLED,
+    ADMIN_PORT(ADSContext.ServerProperty.ADMIN_PORT),
+    /** The associated value is an List of Boolean. */
+    LDAP_ENABLED(ADSContext.ServerProperty.LDAP_ENABLED),
+    /** The associated value is an List of Boolean. */
+    LDAPS_ENABLED(ADSContext.ServerProperty.LDAPS_ENABLED),
+    /** The associated value is an List of Boolean. */
+    ADMIN_ENABLED(ADSContext.ServerProperty.ADMIN_ENABLED),
+    /** The associated value is an List of Boolean. */
+    STARTTLS_ENABLED(ADSContext.ServerProperty.STARTTLS_ENABLED),
+    /** The associated value is an List of Integer. */
+    JMX_PORT(ADSContext.ServerProperty.JMX_PORT),
+    /** The associated value is an List of Integer. */
+    JMXS_PORT(ADSContext.ServerProperty.JMXS_PORT),
+    /** The associated value is an List of Boolean. */
+    JMX_ENABLED(ADSContext.ServerProperty.JMX_ENABLED),
+    /** The associated value is an List of Boolean. */
+    JMXS_ENABLED(ADSContext.ServerProperty.JMXS_ENABLED),
     /** The associated value is an Integer. */
-    REPLICATION_SERVER_PORT,
+    REPLICATION_SERVER_PORT(null),
     /** The associated value is a Boolean. */
-    IS_REPLICATION_SERVER,
+    IS_REPLICATION_SERVER(null),
     /** The associated value is a Boolean. */
-    IS_REPLICATION_ENABLED,
+    IS_REPLICATION_ENABLED(null),
     /** The associated value is a Boolean. */
-    IS_REPLICATION_SECURE,
+    IS_REPLICATION_SECURE(null),
     /** List of servers specified in the Replication Server configuration. This is a Set of String. */
-    EXTERNAL_REPLICATION_SERVERS,
+    EXTERNAL_REPLICATION_SERVERS(null),
     /** The associated value is an Integer. */
-    REPLICATION_SERVER_ID,
+    REPLICATION_SERVER_ID(null),
     /**
      * The instance key-pair public-key certificate. The associated value is a
      * byte[] (ds-cfg-public-key-certificate;binary).
      */
-    INSTANCE_PUBLIC_KEY_CERTIFICATE,
+    INSTANCE_PUBLIC_KEY_CERTIFICATE(ADSContext.ServerProperty.INSTANCE_PUBLIC_KEY_CERTIFICATE),
     /** The schema generation ID. */
-    SCHEMA_GENERATION_ID
+    SCHEMA_GENERATION_ID(null);
+
+    private org.opends.admin.ads.ADSContext.ServerProperty adsEquivalent;
+
+    private ServerProperty(ADSContext.ServerProperty adsEquivalent)
+    {
+      this.adsEquivalent = adsEquivalent;
+    }
   }
 
   /** Default constructor. */
@@ -152,9 +165,8 @@
   }
 
   /**
-   * Tells whether this server is registered in the ADS or not.
-   * @return <CODE>true</CODE> if the server is registered in the ADS and
-   * <CODE>false</CODE> otherwise.
+   * Tells whether this server is registered in the ADS.
+   * @return {@code true} if the server is registered in the ADS and {@code false} otherwise.
    */
   public boolean isRegistered()
   {
@@ -162,9 +174,8 @@
   }
 
   /**
-   * Tells whether this server is a replication server or not.
-   * @return <CODE>true</CODE> if the server is a replication server and
-   * <CODE>false</CODE> otherwise.
+   * Tells whether this server is a replication server.
+   * @return {@code true} if the server is a replication server and {@code false} otherwise.
    */
   public boolean isReplicationServer()
   {
@@ -173,9 +184,8 @@
   }
 
   /**
-   * Tells whether replication is enabled on this server or not.
-   * @return <CODE>true</CODE> if replication is enabled and
-   * <CODE>false</CODE> otherwise.
+   * Tells whether replication is enabled on this server.
+   * @return {@code true} if replication is enabled and {@code false} otherwise.
    */
   public boolean isReplicationEnabled()
   {
@@ -185,10 +195,10 @@
   /**
    * Returns the String representation of this replication server based
    * on the information we have ("hostname":"replication port") and
-   * <CODE>null</CODE> if this is not a replication server.
+   * {@code null} if this is not a replication server.
    * @return the String representation of this replication server based
    * on the information we have ("hostname":"replication port") and
-   * <CODE>null</CODE> if this is not a replication server.
+   * {@code null} if this is not a replication server.
    */
   public String getReplicationServerHostPort()
   {
@@ -218,10 +228,9 @@
   }
 
   /**
-   * Returns whether the communication with the replication port on the server
-   * is encrypted or not.
-   * @return <CODE>true</CODE> if the communication with the replication port on
-   * the server is encrypted and <CODE>false</CODE> otherwise.
+   * Returns whether the communication with the replication port on the server is encrypted.
+   * @return {@code true} if the communication with the replication port on
+   * the server is encrypted and {@code false} otherwise.
    */
   public boolean isReplicationSecure()
   {
@@ -255,10 +264,9 @@
   }
 
   /**
-   * Returns the URL to access this server using LDAP.  Returns
-   * <CODE>null</CODE> if the server is not configured to listen on an LDAP
-   * port.
-   * @return the URL to access this server using LDAP.
+   * Returns the URL to access this server using LDAP.
+   * @return the URL to access this server using LDAP,
+   *         {@code null} if the server is not configured to listen on an LDAP port.
    */
   public String getLDAPURL()
   {
@@ -266,10 +274,9 @@
   }
 
   /**
-   * Returns the URL to access this server using LDAPS.  Returns
-   * <CODE>null</CODE> if the server is not configured to listen on an LDAPS
-   * port.
-   * @return the URL to access this server using LDAP.
+   * Returns the URL to access this server using LDAPS.
+   * @return the URL to access this server using LDAP,
+   *         {@code null} if the server is not configured to listen on an LDAPS port.
    */
   public String getLDAPsURL()
   {
@@ -298,9 +305,8 @@
 
   /**
    * Returns the URL to access this server using the administration connector.
-   * Returns <CODE>null</CODE> if the server cannot get the administration
-   * connector.
-   * @return the URL to access this server using the administration connector.
+   * @return the URL to access this server using the administration connector,
+   *         {@code null} if the server cannot get the administration connector.
    */
   public String getAdminConnectorURL()
   {
@@ -314,8 +320,8 @@
   public List<Integer> getEnabledAdministrationPorts()
   {
     List<Integer> ports = new ArrayList<>(1);
-    ArrayList<?> s = (ArrayList<?>)serverProperties.get(ServerProperty.ADMIN_ENABLED);
-    ArrayList<?> p = (ArrayList<?>)serverProperties.get(ServerProperty.ADMIN_PORT);
+    List<?> s = (List<?>) serverProperties.get(ServerProperty.ADMIN_ENABLED);
+    List<?> p = (List<?>) serverProperties.get(ServerProperty.ADMIN_PORT);
     if (s != null)
     {
       for (int i=0; i<s.size(); i++)
@@ -334,7 +340,7 @@
    * the provided securePreferred is set to true the port that will be used
    * will be the administration connector port.
    * @param securePreferred whether to try to use the secure port as part
-   * of the returning String or not.
+   * of the returning String.
    * @return a String of type host-name:port-number for the server.
    */
   public HostPort getHostPort(boolean securePreferred)
@@ -343,15 +349,15 @@
 
     if (!serverProperties.isEmpty())
     {
-      port = getPort(ServerProperty.LDAP_ENABLED, ServerProperty.LDAP_PORT, port);
+      port = getLdapPort(port);
       if (securePreferred)
       {
-        port = getPort(ServerProperty.ADMIN_ENABLED, ServerProperty.ADMIN_PORT, port);
+        port = getAdminPort(port);
       }
     }
     else
     {
-      ArrayList<ADSContext.ServerProperty> enabledAttrs = new ArrayList<>();
+      List<ADSContext.ServerProperty> enabledAttrs = new ArrayList<>();
 
       if (securePreferred)
       {
@@ -397,6 +403,21 @@
     return new HostPort(getHostName(), port);
   }
 
+  private int getLdapPort(int port)
+  {
+    return getPort(ServerProperty.LDAP_ENABLED, ServerProperty.LDAP_PORT, port);
+  }
+
+  private int getLdapsPort(int port)
+  {
+    return getPort(ServerProperty.LDAPS_ENABLED, ServerProperty.LDAPS_PORT, port);
+  }
+
+  private int getAdminPort(int port)
+  {
+    return getPort(ServerProperty.ADMIN_ENABLED, ServerProperty.ADMIN_PORT, port);
+  }
+
   private ADSContext.ServerProperty getPortProperty(ADSContext.ServerProperty prop)
   {
     switch (prop)
@@ -439,15 +460,16 @@
     if (!serverProperties.isEmpty())
     {
       buf.append(serverProperties.get(ServerProperty.HOST_NAME));
-      ServerProperty [] props =
-      {
-          ServerProperty.LDAP_PORT, ServerProperty.LDAPS_PORT,
+      ServerProperty [] props = {
+          ServerProperty.LDAP_PORT,
+          ServerProperty.LDAPS_PORT,
           ServerProperty.ADMIN_PORT,
-          ServerProperty.LDAP_ENABLED, ServerProperty.LDAPS_ENABLED,
+          ServerProperty.LDAP_ENABLED,
+          ServerProperty.LDAPS_ENABLED,
           ServerProperty.ADMIN_ENABLED
       };
       for (ServerProperty prop : props) {
-        ArrayList<?> s = (ArrayList<?>) serverProperties.get(prop);
+        List<?> s = (List<?>) serverProperties.get(prop);
         for (Object o : s) {
           buf.append(":").append(o);
         }
@@ -455,8 +477,7 @@
     }
     else
     {
-      ADSContext.ServerProperty[] props =
-      {
+      ADSContext.ServerProperty[] props = {
           ADSContext.ServerProperty.HOST_NAME,
           ADSContext.ServerProperty.LDAP_PORT,
           ADSContext.ServerProperty.LDAPS_PORT,
@@ -553,8 +574,8 @@
 
     for (int i=0; i<sProps.length; i++)
     {
-      ArrayList<?> s = (ArrayList<?>)serverProperties.get(sProps[i][0]);
-      ArrayList<?> p = (ArrayList<?>)serverProperties.get(sProps[i][1]);
+      List<?> s = (List<?>) serverProperties.get(sProps[i][0]);
+      List<?> p = (List<?>) serverProperties.get(sProps[i][1]);
       if (s != null)
       {
         int port = getPort(s, p);
@@ -574,8 +595,7 @@
       }
     }
 
-    ArrayList<?> array = (ArrayList<?>)serverProperties.get(
-        ServerProperty.STARTTLS_ENABLED);
+    List<?> array = (List<?>) serverProperties.get(ServerProperty.STARTTLS_ENABLED);
     boolean startTLSEnabled = false;
     if (array != null && !array.isEmpty())
     {
@@ -653,11 +673,11 @@
         "objectclass");
     try (ConnectionEntryReader entryReader = conn.getConnection().search(request))
     {
-      ArrayList<Integer> ldapPorts = new ArrayList<>();
-      ArrayList<Integer> ldapsPorts = new ArrayList<>();
-      ArrayList<Boolean> ldapEnabled = new ArrayList<>();
-      ArrayList<Boolean> ldapsEnabled = new ArrayList<>();
-      ArrayList<Boolean> startTLSEnabled = new ArrayList<>();
+      List<Integer> ldapPorts = new ArrayList<>();
+      List<Integer> ldapsPorts = new ArrayList<>();
+      List<Boolean> ldapEnabled = new ArrayList<>();
+      List<Boolean> ldapsEnabled = new ArrayList<>();
+      List<Boolean> startTLSEnabled = new ArrayList<>();
 
       desc.serverProperties.put(ServerProperty.LDAP_PORT, ldapPorts);
       desc.serverProperties.put(ServerProperty.LDAPS_PORT, ldapsPorts);
@@ -697,8 +717,8 @@
 
     // Even if we have a single port, use an array to be consistent with
     // other protocols.
-    ArrayList<Integer> adminPorts = new ArrayList<>();
-    ArrayList<Boolean> adminEnabled = new ArrayList<>();
+    List<Integer> adminPorts = new ArrayList<>();
+    List<Boolean> adminEnabled = new ArrayList<>();
     if (adminConnectorPort != null)
     {
       adminPorts.add(adminConnectorPort);
@@ -710,10 +730,10 @@
 
   private static void updateJmxConfiguration(ServerDescriptor desc, ConnectionWrapper conn) throws IOException
   {
-    ArrayList<Integer> jmxPorts = new ArrayList<>();
-    ArrayList<Integer> jmxsPorts = new ArrayList<>();
-    ArrayList<Boolean> jmxEnabled = new ArrayList<>();
-    ArrayList<Boolean> jmxsEnabled = new ArrayList<>();
+    List<Integer> jmxPorts = new ArrayList<>();
+    List<Integer> jmxsPorts = new ArrayList<>();
+    List<Boolean> jmxEnabled = new ArrayList<>();
+    List<Boolean> jmxsEnabled = new ArrayList<>();
 
     desc.serverProperties.put(ServerProperty.JMX_PORT, jmxPorts);
     desc.serverProperties.put(ServerProperty.JMXS_PORT, jmxsPorts);
@@ -1102,32 +1122,30 @@
   }
 
   /**
-   * An convenience method to know if the provided ID corresponds to a
-   * configuration backend or not.
-   * @param id the backend ID to analyze
-   * @return <CODE>true</CODE> if the the id corresponds to a configuration
-   * backend and <CODE>false</CODE> otherwise.
+   * Returns whether the provided backendID corresponds to a configuration backend.
+   * @param backendId the backend ID to analyze
+   * @return {@code true} if the the id corresponds to a configuration
+   * backend and {@code false} otherwise.
    */
-  private static boolean isConfigBackend(String id)
+  private static boolean isConfigBackend(String backendId)
   {
-    return "tasks".equalsIgnoreCase(id) ||
-    "schema".equalsIgnoreCase(id) ||
-    "config".equalsIgnoreCase(id) ||
-    "monitor".equalsIgnoreCase(id) ||
-    "backup".equalsIgnoreCase(id) ||
-    "ads-truststore".equalsIgnoreCase(id);
+    return "tasks".equalsIgnoreCase(backendId)
+        || "schema".equalsIgnoreCase(backendId)
+        || "config".equalsIgnoreCase(backendId)
+        || "monitor".equalsIgnoreCase(backendId)
+        || "backup".equalsIgnoreCase(backendId)
+        || "ads-truststore".equalsIgnoreCase(backendId);
   }
 
   /**
-   * An convenience method to know if the provided ID corresponds to the schema
-   * backend or not.
-   * @param id the backend ID to analyze
-   * @return <CODE>true</CODE> if the the id corresponds to the schema backend
-   * and <CODE>false</CODE> otherwise.
+   * Returns whether the provided ID corresponds to the schema backend.
+   * @param backendId the backend ID to analyze
+   * @return {@code true} if the the id corresponds to the schema backend
+   * and {@code false} otherwise.
    */
-  private static boolean isSchemaBackend(String id)
+  private static boolean isSchemaBackend(String backendId)
   {
-    return "schema".equalsIgnoreCase(id);
+    return "schema".equalsIgnoreCase(backendId);
   }
 
   /**
@@ -1165,11 +1183,80 @@
    * Tells whether the provided server descriptor represents the same server
    * as this object.
    * @param server the server to make the comparison.
-   * @return whether the provided server descriptor represents the same server
-   * as this object or not.
+   * @return {@code true} if the provided server descriptor represents the same server
+   * as this object, {@code false} otherwise.
    */
   public boolean isSameServer(ServerDescriptor server)
   {
     return getId().equals(server.getId());
   }
+
+  @Override
+  public String toString()
+  {
+    final int defaultPort = -1;
+    final int adminPort = getAdminPort(defaultPort);
+    final int ldapPort = getLdapPort(defaultPort);
+    final int ldapsPort = getLdapsPort(defaultPort);
+    final boolean isRs = isReplicationServer();
+    StringBuilder sb = new StringBuilder(getClass().getSimpleName());
+    sb.append("(host-name=").append(getHostName());
+    if (adminPort != defaultPort)
+    {
+      sb.append(", adminPort=").append(adminPort);
+    }
+    if (ldapPort != defaultPort)
+    {
+      sb.append(", ldapPort=").append(ldapPort);
+    }
+    if (ldapsPort != defaultPort)
+    {
+      sb.append(", ldapsPort=").append(ldapsPort);
+    }
+    sb.append(", isReplicationServer=").append(isRs);
+    if (isRs)
+    {
+      sb.append(", replication-server-id=").append(getReplicationServerId());
+    }
+    appendInconsistencies(sb);
+    sb.append(")");
+    return sb.toString();
+  }
+
+  private void appendInconsistencies(StringBuilder sb)
+  {
+    Map<ServerProperty, Pair<?, ?>> inconsistencies = new HashMap<>();
+    for (ServerProperty prop : ServerProperty.values())
+    {
+      if (prop.adsEquivalent != null)
+      {
+        Object propVal = toScalar(serverProperties.get(prop));
+        Object propValue = propVal instanceof byte[] ? (byte[]) propVal : toStringValue(propVal);
+        Object adsPropValue = adsProperties.get(prop.adsEquivalent);
+        if (!Objects.equals(propValue, adsPropValue))
+        {
+          inconsistencies.put(prop, Pair.of(propValue, adsPropValue));
+        }
+      }
+    }
+    if (!inconsistencies.isEmpty())
+    {
+      sb.append(", inconsistencies=").append(inconsistencies);
+    }
+  }
+
+  private Object toScalar(Object propValue)
+  {
+    if (propValue instanceof List)
+    {
+      List<?> propValues = (List<?>) propValue;
+      return !propValues.isEmpty() ? propValues.get(0) : null;
+    }
+    return propValue;
+  }
+
+  private String toStringValue(Object propValue)
+  {
+    return propValue != null ? propValue.toString() : null;
+  }
 }
diff --git a/opendj-server-legacy/src/main/java/org/opends/admin/ads/SuffixDescriptor.java b/opendj-server-legacy/src/main/java/org/opends/admin/ads/SuffixDescriptor.java
index abda665..de4dc02 100644
--- a/opendj-server-legacy/src/main/java/org/opends/admin/ads/SuffixDescriptor.java
+++ b/opendj-server-legacy/src/main/java/org/opends/admin/ads/SuffixDescriptor.java
@@ -16,10 +16,9 @@
  */
 package org.opends.admin.ads;
 
-import static java.util.Collections.*;
-
 import java.util.HashSet;
 import java.util.Set;
+import java.util.TreeSet;
 
 import org.forgerock.opendj.ldap.DN;
 
@@ -44,7 +43,7 @@
   public SuffixDescriptor(DN suffixDn, ReplicaDescriptor replica)
   {
     this.suffixDN = suffixDn;
-    setReplicas(singleton(replica));
+    this.replicas.add(replica);
   }
 
   /**
@@ -80,16 +79,14 @@
   }
 
   /**
-   * Sets the replicas associated with this SuffixDescriptor.
+   * Associates the provided replica with this SuffixDescriptor.
    *
-   * @param replicas
-   *          a Set containing the replicas associated with this
-   *          SuffixDescriptor.
+   * @param replica
+   *          the replicate to associate with this SuffixDescriptor.
    */
-  public void setReplicas(Set<ReplicaDescriptor> replicas)
+  void addReplica(ReplicaDescriptor replica)
   {
-    this.replicas.clear();
-    this.replicas.addAll(replicas);
+    replicas.add(replica);
   }
 
   /**
@@ -129,4 +126,18 @@
     }
     return buf.toString();
   }
+
+  @Override
+  public String toString()
+  {
+    Set<String> replicaStrings = new TreeSet<>();
+    for (ReplicaDescriptor replica : replicas)
+    {
+      replicaStrings.add(replica.getServer().getHostPort(true).toString());
+    }
+    return getClass().getSimpleName()
+        + "(dn=" + suffixDN
+        + ", replicas=" + replicaStrings
+        + ")";
+  }
 }
diff --git a/opendj-server-legacy/src/main/java/org/opends/admin/ads/TopologyCache.java b/opendj-server-legacy/src/main/java/org/opends/admin/ads/TopologyCache.java
index 93b77fd..eac2329 100644
--- a/opendj-server-legacy/src/main/java/org/opends/admin/ads/TopologyCache.java
+++ b/opendj-server-legacy/src/main/java/org/opends/admin/ads/TopologyCache.java
@@ -17,12 +17,15 @@
 package org.opends.admin.ads;
 
 import java.io.IOException;
+import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
+import java.util.Comparator;
 import java.util.Date;
 import java.util.HashMap;
 import java.util.HashSet;
-import java.util.Iterator;
 import java.util.LinkedHashSet;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
@@ -42,13 +45,13 @@
 import org.opends.admin.ads.util.PreferredConnection;
 import org.opends.admin.ads.util.ServerLoader;
 import org.opends.quicksetup.util.Utils;
+import org.opends.server.types.HostPort;
 
 import static com.forgerock.opendj.cli.Utils.*;
 
 import static org.forgerock.opendj.ldap.SearchScope.*;
 import static org.opends.admin.ads.util.ConnectionUtils.*;
 import static org.opends.messages.QuickSetupMessages.*;
-
 /**
  * This class allows to read the configuration of the different servers that are
  * registered in a given ADS server. It provides a read only view of the
@@ -125,29 +128,14 @@
           DN dn = replica.getSuffix().getDN();
           logger.info(LocalizableMessage.raw("Handling replica with dn: " + dn));
 
-          boolean suffixFound = false;
           Set<SuffixDescriptor> sufs = hmSuffixes.get(dn);
-          if (sufs != null)
+          SuffixDescriptor suffix = findSuffix(replica, sufs);
+          if (suffix != null)
           {
-            Iterator<SuffixDescriptor> it = sufs.iterator();
-            while (it.hasNext() && !suffixFound)
-            {
-              SuffixDescriptor suffix = it.next();
-              Iterator<String> it2 = suffix.getReplicationServers().iterator();
-              while (it2.hasNext() && !suffixFound)
-              {
-                if (replica.getReplicationServers().contains(it2.next()))
-                {
-                  suffixFound = true;
-                  Set<ReplicaDescriptor> replicas = suffix.getReplicas();
-                  replicas.add(replica);
-                  suffix.setReplicas(replicas);
-                  replica.setSuffix(suffix);
-                }
-              }
-            }
+            suffix.addReplica(replica);
+            replica.setSuffix(suffix);
           }
-          if (!suffixFound)
+          else
           {
             if (sufs == null)
             {
@@ -177,6 +165,25 @@
     }
   }
 
+  private SuffixDescriptor findSuffix(ReplicaDescriptor replica, Set<SuffixDescriptor> sufs)
+  {
+    if (sufs != null)
+    {
+      for (SuffixDescriptor suffix : sufs)
+      {
+        Set<String> rssInSuffix = suffix.getReplicationServers();
+        for (String replicationServer : rssInSuffix)
+        {
+          if (replica.getReplicationServers().contains(replicationServer))
+          {
+            return suffix;
+          }
+        }
+      }
+    }
+    return null;
+  }
+
   /** Reads the replication monitoring. */
   private void readReplicationMonitoring()
   {
@@ -434,8 +441,11 @@
       throws NamingException, IOException
   {
     ServerLoader loader = getServerLoader(replicationServer.getAdsProperties());
-    SearchRequest request=Requests.newSearchRequest("cn=monitor", WHOLE_SUBTREE, "(missing-changes=*)",
-        "approx-older-change-not-synchronized-millis", "missing-changes", "domain-name", "server-id");
+    SearchRequest request = Requests.newSearchRequest("cn=monitor", WHOLE_SUBTREE, "(missing-changes=*)",
+        "domain-name",
+        "server-id",
+        "missing-changes",
+        "approx-older-change-not-synchronized-millis");
     try (ConnectionWrapper conn = loader.createConnectionWrapper();
         ConnectionEntryReader entryReader = conn.getConnection().search(request))
     {
@@ -443,7 +453,7 @@
       {
         SearchResultEntry sr = entryReader.readEntry();
 
-        String dnStr = firstValueAsString(sr, "domain-name");
+        final DN dn = DN.valueOf(firstValueAsString(sr, "domain-name"));
         int replicaId = -1;
         try
         {
@@ -460,7 +470,6 @@
           logger.warn(LocalizableMessage.raw("Unexpected error reading replica ID: " + t, t));
         }
 
-        final DN dn = DN.valueOf(dnStr);
         for (ReplicaDescriptor replica : candidateReplicas)
         {
           if (dn.equals(replica.getSuffix().getDN())
@@ -475,8 +484,9 @@
         }
       }
     }
-    catch (EntryNotFoundException e)
+    catch (EntryNotFoundException ignored)
     {
+      // no replicas updated this time. Try with another server higher up the stack.
     }
   }
 
@@ -513,4 +523,58 @@
       }
     }
   }
+
+  @Override
+  public String toString()
+  {
+    List<SuffixDescriptor> sortedSuffixes = new ArrayList<>(suffixes);
+    Collections.sort(sortedSuffixes, new Comparator<SuffixDescriptor>()
+    {
+      @Override
+      public int compare(SuffixDescriptor suffix1, SuffixDescriptor suffix2)
+      {
+        return suffix1.getDN().compareTo(suffix2.getDN());
+      }
+    });
+
+    final StringBuilder sb = new StringBuilder(getClass().getSimpleName()).append("\n");
+    sb.append("Suffix DN,Server,Entries,Replication enabled,DS ID,RS ID,RS Port,M.C.,A.O.M.C.,Security\n");
+    for (SuffixDescriptor suffix : sortedSuffixes)
+    {
+      List<ReplicaDescriptor> sortedReplicas = new ArrayList<>(suffix.getReplicas());
+      Collections.sort(sortedReplicas, new Comparator<ReplicaDescriptor>()
+      {
+        @Override
+        public int compare(ReplicaDescriptor replica1, ReplicaDescriptor replica2)
+        {
+          HostPort hp1 = replica1.getServer().getHostPort(true);
+          HostPort hp2 = replica2.getServer().getHostPort(true);
+          return hp1.toString().compareTo(hp2.toString());
+        }
+      });
+      for (ReplicaDescriptor replica : sortedReplicas)
+      {
+        ServerDescriptor server = replica.getServer();
+        boolean isReplEnabled = server.isReplicationEnabled();
+        boolean secureReplication = server.isReplicationSecure();
+        sb.append(suffix.getDN()).append(",")
+          .append(server.getHostPort(true)).append(",")
+          .append(replica.getEntries()).append(",")
+          .append(isReplEnabled).append(",")
+          .append(replica.getReplicationId()).append(",")
+          .append(orBlank(server.getReplicationServerId())).append(",")
+          .append(orBlank(server.getReplicationServerPort())).append(",")
+          .append(replica.getMissingChanges()).append(",")
+          .append(orBlank(replica.getAgeOfOldestMissingChange())).append(",")
+          .append(secureReplication ? secureReplication : "")
+          .append("\n");
+      }
+    }
+    return sb.toString();
+  }
+
+  private Object orBlank(long value)
+  {
+    return value!=-1 ? value : "";
+  }
 }
diff --git a/opendj-server-legacy/src/main/java/org/opends/admin/ads/util/PreferredConnection.java b/opendj-server-legacy/src/main/java/org/opends/admin/ads/util/PreferredConnection.java
index 8402809..5509e03 100644
--- a/opendj-server-legacy/src/main/java/org/opends/admin/ads/util/PreferredConnection.java
+++ b/opendj-server-legacy/src/main/java/org/opends/admin/ads/util/PreferredConnection.java
@@ -85,7 +85,7 @@
     if (o instanceof PreferredConnection)
     {
       PreferredConnection p = (PreferredConnection)o;
-      return type == p.getType()
+      return type == p.type
           && ldapUrl.equalsIgnoreCase(p.getLDAPURL());
     }
     return false;
diff --git a/opendj-server-legacy/src/main/java/org/opends/admin/ads/util/ServerLoader.java b/opendj-server-legacy/src/main/java/org/opends/admin/ads/util/ServerLoader.java
index 60262ae..6c98488 100644
--- a/opendj-server-legacy/src/main/java/org/opends/admin/ads/util/ServerLoader.java
+++ b/opendj-server-legacy/src/main/java/org/opends/admin/ads/util/ServerLoader.java
@@ -375,22 +375,11 @@
     for (PreferredConnection connection : preferredLDAPURLs)
     {
       String url = connection.getLDAPURL();
-      if (url.equalsIgnoreCase(adminConnectorUrl))
-      {
-        ldapUrls.add(connection);
-      }
-      else if (url.equalsIgnoreCase(ldapsUrl) &&
-          connection.getType() == LDAPS)
-      {
-        ldapUrls.add(connection);
-      }
-      else if (url.equalsIgnoreCase(startTLSUrl) &&
-          connection.getType() == START_TLS)
-      {
-        ldapUrls.add(connection);
-      }
-      else if (url.equalsIgnoreCase(ldapUrl) &&
-          connection.getType() == LDAP)
+      org.opends.admin.ads.util.PreferredConnection.Type type = connection.getType();
+      if (url.equalsIgnoreCase(adminConnectorUrl)
+          || (url.equalsIgnoreCase(ldapsUrl) && type == LDAPS)
+          || (url.equalsIgnoreCase(startTLSUrl) && type == START_TLS)
+          || (url.equalsIgnoreCase(ldapUrl) && type == LDAP))
       {
         ldapUrls.add(connection);
       }
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/api/MonitorData.java b/opendj-server-legacy/src/main/java/org/opends/server/api/MonitorData.java
index 37a9125..a317d01 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/api/MonitorData.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/api/MonitorData.java
@@ -224,6 +224,6 @@
   @Override
   public String toString()
   {
-    return getClass().getName() + attrs;
+    return getClass().getSimpleName() + attrs;
   }
 }
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/tools/dsreplication/ReplicationCliMain.java b/opendj-server-legacy/src/main/java/org/opends/server/tools/dsreplication/ReplicationCliMain.java
index b435df5..d27649d 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/tools/dsreplication/ReplicationCliMain.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/tools/dsreplication/ReplicationCliMain.java
@@ -5836,7 +5836,6 @@
   {
     ADSContext adsCtx = new ADSContext(conn);
 
-    boolean somethingDisplayed = false;
     TopologyCache cache;
     try
     {
@@ -5901,6 +5900,8 @@
       }
     }
 
+
+    boolean somethingDisplayed = false;
     if (!oneReplicated && displayAll)
     {
       // Maybe there are some replication server configured...
@@ -5932,35 +5933,22 @@
       if (oneReplicated && !uData.isScriptFriendly())
       {
         println();
-        print(INFO_REPLICATION_STATUS_REPLICATED_LEGEND.get());
+        println(INFO_REPLICATION_STATUS_REPLICATED_LEGEND.get());
 
         if (!replicasWithNoReplicationServer.isEmpty() ||
             !serversWithNoReplica.isEmpty())
         {
-          println();
-          print(
-              INFO_REPLICATION_STATUS_NOT_A_REPLICATION_SERVER_LEGEND.get());
-
-          println();
-          print(
-              INFO_REPLICATION_STATUS_NOT_A_REPLICATION_DOMAIN_LEGEND.get());
+          println(INFO_REPLICATION_STATUS_NOT_A_REPLICATION_SERVER_LEGEND.get());
+          println(INFO_REPLICATION_STATUS_NOT_A_REPLICATION_DOMAIN_LEGEND.get());
         }
-        println();
         somethingDisplayed = true;
       }
     }
     if (!somethingDisplayed)
     {
-      if (displayAll)
-      {
-        print(INFO_REPLICATION_STATUS_NO_REPLICATION_INFORMATION.get());
-        println();
-      }
-      else
-      {
-        print(INFO_REPLICATION_STATUS_NO_BASEDNS.get());
-        println();
-      }
+      println(displayAll
+          ? INFO_REPLICATION_STATUS_NO_REPLICATION_INFORMATION.get()
+          : INFO_REPLICATION_STATUS_NO_BASEDNS.get());
     }
   }
 
@@ -6029,31 +6017,10 @@
       Set<ServerDescriptor> serversWithNoReplica)
   {
     Set<ReplicaDescriptor> orderedReplicas = new LinkedHashSet<>();
-    Set<HostPort> hostPorts = new TreeSet<>(new Comparator<HostPort>()
-    {
-      @Override
-      public int compare(HostPort hp1, HostPort hp2)
-      {
-        return hp1.toString().compareTo(hp2.toString());
-      }
-    });
     Set<ServerDescriptor> notAddedReplicationServers = new TreeSet<>(new ReplicationServerComparator());
     for (Set<ReplicaDescriptor> replicas : orderedReplicaLists)
     {
-      for (ReplicaDescriptor replica : replicas)
-      {
-        hostPorts.add(getHostPort2(replica.getServer(), cnx));
-      }
-      for (HostPort hostPort : hostPorts)
-      {
-        for (ReplicaDescriptor replica : replicas)
-        {
-          if (getHostPort2(replica.getServer(), cnx).equals(hostPort))
-          {
-            orderedReplicas.add(replica);
-          }
-        }
-      }
+      addReplicasSortedByHostPort(orderedReplicas, replicas, cnx);
       for (ServerDescriptor server : servers)
       {
         if (server.isReplicationServer() && isRepServerNotInDomain(replicas, server))
@@ -6211,6 +6178,23 @@
     tableBuilder.print(printer);
   }
 
+  private void addReplicasSortedByHostPort(Set<ReplicaDescriptor> orderedReplicas, Set<ReplicaDescriptor> replicas,
+      final Set<PreferredConnection> cnx)
+  {
+    List<ReplicaDescriptor> sortedReplicas = new ArrayList<>(replicas);
+    Collections.sort(sortedReplicas, new Comparator<ReplicaDescriptor>()
+    {
+      @Override
+      public int compare(ReplicaDescriptor replica1, ReplicaDescriptor replica2)
+      {
+        HostPort hp1 = getHostPort2(replica1.getServer(), cnx);
+        HostPort hp2 = getHostPort2(replica2.getServer(), cnx);
+        return hp1.toString().compareTo(hp2.toString());
+      }
+    });
+    orderedReplicas.addAll(sortedReplicas);
+  }
+
   private LocalizableMessage fromObject(Object value)
   {
     return LocalizableMessage.raw("%s", value);

--
Gitblit v1.10.0