From 742d4defd681c9e78e0445ca4a0ffb2316441b08 Mon Sep 17 00:00:00 2001
From: Jean-Noel Rouvignac <jean-noel.rouvignac@forgerock.com>
Date: Mon, 20 Jan 2014 16:32:41 +0000
Subject: [PATCH] Code review: Matthew Swift
---
opendj-sdk/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/LDAPListener.java | 75 ++++++++++++++
opendj-sdk/opendj-ldap-sdk/src/test/java/org/forgerock/opendj/ldap/TestCaseUtils.java | 2
opendj-sdk/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/Connections.java | 97 ++++++++++++++++++-
opendj-sdk/opendj-ldap-sdk/src/test/java/org/forgerock/opendj/ldap/LDAPServer.java | 4
opendj-sdk/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/ErrorResultException.java | 47 +++++++++
opendj-sdk/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/LDAPConnectionFactory.java | 59 +++++++++++
6 files changed, 267 insertions(+), 17 deletions(-)
diff --git a/opendj-sdk/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/Connections.java b/opendj-sdk/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/Connections.java
index 1f389bb..1129af5 100644
--- a/opendj-sdk/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/Connections.java
+++ b/opendj-sdk/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/Connections.java
@@ -34,6 +34,7 @@
import org.forgerock.opendj.ldap.requests.BindRequest;
import org.forgerock.opendj.ldap.requests.SearchRequest;
+import org.forgerock.opendj.ldap.requests.UnbindRequest;
import com.forgerock.opendj.ldap.InternalConnection;
import com.forgerock.opendj.util.Validator;
@@ -43,6 +44,9 @@
* factories and connections.
*/
public final class Connections {
+
+ private static final int DEFAULT_TIMEOUT_IN_SECONDS = 3;
+
/**
* Creates a new authenticated connection factory which will obtain
* connections using the provided connection factory and immediately perform
@@ -241,6 +245,8 @@
return new CachedConnectionPool(factory, poolSize, poolSize, 0L, null, null);
}
+ // FIXME We should remove the newHeartBeatConnectionFactory() methods and use a builder instead.
+
/**
* Creates a new heart-beat connection factory which will create connections
* using the provided connection factory and periodically ping any created
@@ -255,7 +261,30 @@
* If {@code factory} was {@code null}.
*/
public static ConnectionFactory newHeartBeatConnectionFactory(final ConnectionFactory factory) {
- return new HeartBeatConnectionFactory(factory, 10, 3, TimeUnit.SECONDS, null, null);
+ return new HeartBeatConnectionFactory(factory, 10, DEFAULT_TIMEOUT_IN_SECONDS, TimeUnit.SECONDS, null, null);
+ }
+
+ /**
+ * Creates a new heart-beat connection factory which will create connections
+ * using the provided connection factory and periodically ping any created
+ * connections in order to detect that they are still alive using the
+ * specified frequency and the default scheduler.
+ *
+ * @param factory
+ * The connection factory to use for creating connections.
+ * @param interval
+ * The interval between keepalive pings.
+ * @param unit
+ * The time unit for the interval between keepalive pings.
+ * @return The new heart-beat connection factory.
+ * @throws IllegalArgumentException
+ * If {@code interval} was negative.
+ * @throws NullPointerException
+ * If {@code factory} or {@code unit} was {@code null}.
+ */
+ public static ConnectionFactory newHeartBeatConnectionFactory(final ConnectionFactory factory,
+ final long interval, final TimeUnit unit) {
+ return new HeartBeatConnectionFactory(factory, interval, DEFAULT_TIMEOUT_IN_SECONDS, unit, null, null);
}
/**
@@ -294,6 +323,32 @@
* The connection factory to use for creating connections.
* @param interval
* The interval between keepalive pings.
+ * @param unit
+ * The time unit for the interval between keepalive pings.
+ * @param heartBeat
+ * The search request to use for keepalive pings.
+ * @return The new heart-beat connection factory.
+ * @throws IllegalArgumentException
+ * If {@code interval} was negative.
+ * @throws NullPointerException
+ * If {@code factory}, {@code unit}, or {@code heartBeat} was
+ * {@code null}.
+ */
+ public static ConnectionFactory newHeartBeatConnectionFactory(final ConnectionFactory factory,
+ final long interval, final TimeUnit unit, final SearchRequest heartBeat) {
+ return new HeartBeatConnectionFactory(factory, interval, DEFAULT_TIMEOUT_IN_SECONDS, unit, heartBeat, null);
+ }
+
+ /**
+ * Creates a new heart-beat connection factory which will create connections
+ * using the provided connection factory and periodically ping any created
+ * connections using the specified search request in order to detect that
+ * they are still alive.
+ *
+ * @param factory
+ * The connection factory to use for creating connections.
+ * @param interval
+ * The interval between keepalive pings.
* @param timeout
* The heart-beat timeout after which a connection will be marked
* as failed.
@@ -324,6 +379,37 @@
* The connection factory to use for creating connections.
* @param interval
* The interval between keepalive pings.
+ * @param unit
+ * The time unit for the interval between keepalive pings.
+ * @param heartBeat
+ * The search request to use for keepalive pings.
+ * @param scheduler
+ * The scheduler which should for periodically sending keepalive
+ * pings.
+ * @return The new heart-beat connection factory.
+ * @throws IllegalArgumentException
+ * If {@code interval} was negative.
+ * @throws NullPointerException
+ * If {@code factory}, {@code unit}, or {@code heartBeat} was
+ * {@code null}.
+ */
+ public static ConnectionFactory newHeartBeatConnectionFactory(final ConnectionFactory factory,
+ final long interval, final TimeUnit unit,
+ final SearchRequest heartBeat, final ScheduledExecutorService scheduler) {
+ return new HeartBeatConnectionFactory(factory, interval, DEFAULT_TIMEOUT_IN_SECONDS, unit, heartBeat,
+ scheduler);
+ }
+
+ /**
+ * Creates a new heart-beat connection factory which will create connections
+ * using the provided connection factory and periodically ping any created
+ * connections using the specified search request in order to detect that
+ * they are still alive.
+ *
+ * @param factory
+ * The connection factory to use for creating connections.
+ * @param interval
+ * The interval between keepalive pings.
* @param timeout
* The heart-beat timeout after which a connection will be marked
* as failed.
@@ -628,8 +714,7 @@
/**
* Returns an uncloseable view of the provided connection. Attempts to call
* {@link Connection#close()} or
- * {@link Connection#close(org.forgerock.opendj.ldap.requests.UnbindRequest, String)}
- * will be ignored.
+ * {@link Connection#close(UnbindRequest, String)} will be ignored.
*
* @param connection
* The connection whose {@code close} methods are to be disabled.
@@ -642,10 +727,10 @@
// Do nothing.
}
- public void close(org.forgerock.opendj.ldap.requests.UnbindRequest request,
- String reason) {
+ @Override
+ public void close(UnbindRequest request, String reason) {
// Do nothing.
- };
+ }
};
}
diff --git a/opendj-sdk/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/ErrorResultException.java b/opendj-sdk/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/ErrorResultException.java
index 45949d3..df73d31 100644
--- a/opendj-sdk/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/ErrorResultException.java
+++ b/opendj-sdk/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/ErrorResultException.java
@@ -70,6 +70,28 @@
* If the provided result code does not represent a failure.
* @throws NullPointerException
* If {@code resultCode} was {@code null}.
+ * @deprecated use {@link #newErrorResult(ResultCode, CharSequence)} instead
+ */
+ @Deprecated
+ public static ErrorResultException newErrorResult(ResultCode resultCode,
+ String diagnosticMessage) {
+ return newErrorResult(resultCode, (CharSequence) diagnosticMessage);
+ }
+
+ /**
+ * Creates a new error result exception with the provided result code and
+ * diagnostic message.
+ *
+ * @param resultCode
+ * The result code.
+ * @param diagnosticMessage
+ * The diagnostic message, which may be empty or {@code null}
+ * indicating that none was provided.
+ * @return The new error result exception.
+ * @throws IllegalArgumentException
+ * If the provided result code does not represent a failure.
+ * @throws NullPointerException
+ * If {@code resultCode} was {@code null}.
*/
public static ErrorResultException newErrorResult(ResultCode resultCode,
CharSequence diagnosticMessage) {
@@ -112,6 +134,31 @@
* If the provided result code does not represent a failure.
* @throws NullPointerException
* If {@code resultCode} was {@code null}.
+ * @deprecated use {@link #newErrorResult(ResultCode, CharSequence, Throwable)} instead
+ */
+ @Deprecated
+ public static ErrorResultException newErrorResult(ResultCode resultCode,
+ String diagnosticMessage, Throwable cause) {
+ return newErrorResult(resultCode, (CharSequence) diagnosticMessage, cause);
+ }
+
+ /**
+ * Creates a new error result exception with the provided result code,
+ * diagnostic message, and cause.
+ *
+ * @param resultCode
+ * The result code.
+ * @param diagnosticMessage
+ * The diagnostic message, which may be empty or {@code null}
+ * indicating that none was provided.
+ * @param cause
+ * The throwable cause, which may be {@code null} indicating that
+ * none was provided.
+ * @return The new error result exception.
+ * @throws IllegalArgumentException
+ * If the provided result code does not represent a failure.
+ * @throws NullPointerException
+ * If {@code resultCode} was {@code null}.
*/
public static ErrorResultException newErrorResult(ResultCode resultCode,
CharSequence diagnosticMessage, Throwable cause) {
diff --git a/opendj-sdk/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/LDAPConnectionFactory.java b/opendj-sdk/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/LDAPConnectionFactory.java
index b6da837..08eb75e 100644
--- a/opendj-sdk/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/LDAPConnectionFactory.java
+++ b/opendj-sdk/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/LDAPConnectionFactory.java
@@ -29,6 +29,7 @@
import java.net.InetAddress;
import java.net.InetSocketAddress;
+import java.net.SocketAddress;
import com.forgerock.opendj.ldap.LDAPConnectionFactoryImpl;
import com.forgerock.opendj.util.StaticUtils;
@@ -53,6 +54,38 @@
* The address of the Directory Server.
* @throws NullPointerException
* If {@code address} was {@code null}.
+ * @deprecated use {@link #LDAPConnectionFactory(InetSocketAddress)} instead
+ */
+ @Deprecated
+ public LDAPConnectionFactory(final SocketAddress address) {
+ this((InetSocketAddress) address, new LDAPOptions());
+ }
+
+ /**
+ * Creates a new LDAP connection factory which can be used to create LDAP
+ * connections to the Directory Server at the provided address.
+ *
+ * @param address
+ * The address of the Directory Server.
+ * @param options
+ * The LDAP options to use when creating connections.
+ * @throws NullPointerException
+ * If {@code address} or {@code options} was {@code null}.
+ * @deprecated use {@link #LDAPConnectionFactory(InetSocketAddress, LDAPOptions)} instead
+ */
+ @Deprecated
+ public LDAPConnectionFactory(final SocketAddress address, final LDAPOptions options) {
+ this((InetSocketAddress) address, options);
+ }
+
+ /**
+ * Creates a new LDAP connection factory which can be used to create LDAP
+ * connections to the Directory Server at the provided address.
+ *
+ * @param address
+ * The address of the Directory Server.
+ * @throws NullPointerException
+ * If {@code address} was {@code null}.
*/
public LDAPConnectionFactory(final InetSocketAddress address) {
this(address, new LDAPOptions());
@@ -117,7 +150,7 @@
* or {@code null} if it is unknown.
*/
public InetAddress getAddress() {
- return getSocketAddress().getAddress();
+ return getInetSocketAddress().getAddress();
}
@Override
@@ -143,9 +176,23 @@
* will not perform a reverse DNS lookup.
*
* @return The host name that this LDAP listener is listening on.
+ * @deprecated use {@link #getHostName()} instead
+ */
+ @Deprecated
+ public String getHostname() {
+ return getHostName();
+ }
+
+ /**
+ * Returns the host name that this LDAP listener is listening on. The
+ * returned host name is the same host name that was provided during
+ * construction and may be an IP address. More specifically, this method
+ * will not perform a reverse DNS lookup.
+ *
+ * @return The host name that this LDAP listener is listening on.
*/
public String getHostName() {
- return StaticUtils.getHostName(getSocketAddress());
+ return StaticUtils.getHostName(getInetSocketAddress());
}
/**
@@ -154,7 +201,7 @@
* @return The port that this LDAP listener is listening on.
*/
public int getPort() {
- return getSocketAddress().getPort();
+ return getInetSocketAddress().getPort();
}
/**
@@ -162,7 +209,11 @@
*
* @return The address that this LDAP listener is listening on.
*/
- public InetSocketAddress getSocketAddress() {
+ public SocketAddress getSocketAddress() {
+ return getInetSocketAddress();
+ }
+
+ private InetSocketAddress getInetSocketAddress() {
return impl.getSocketAddress();
}
diff --git a/opendj-sdk/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/LDAPListener.java b/opendj-sdk/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/LDAPListener.java
index 2113d16..539072d 100644
--- a/opendj-sdk/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/LDAPListener.java
+++ b/opendj-sdk/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/LDAPListener.java
@@ -31,6 +31,7 @@
import java.io.IOException;
import java.net.InetAddress;
import java.net.InetSocketAddress;
+import java.net.SocketAddress;
import com.forgerock.opendj.ldap.LDAPListenerImpl;
import com.forgerock.opendj.util.StaticUtils;
@@ -155,6 +156,28 @@
* address.
* @throws NullPointerException
* If {@code address} or {code factory} was {@code null}.
+ * @deprecated use {@link #LDAPListener(InetSocketAddress, ServerConnectionFactory)} instead
+ */
+ @Deprecated
+ public LDAPListener(final SocketAddress address,
+ final ServerConnectionFactory<LDAPClientContext, Integer> factory) throws IOException {
+ this((InetSocketAddress) address, factory, new LDAPListenerOptions());
+ }
+
+ /**
+ * Creates a new LDAP listener implementation which will listen for LDAP
+ * client connections at the provided address.
+ *
+ * @param address
+ * The address to listen on.
+ * @param factory
+ * The server connection factory which will be used to create
+ * server connections.
+ * @throws IOException
+ * If an error occurred while trying to listen on the provided
+ * address.
+ * @throws NullPointerException
+ * If {@code address} or {code factory} was {@code null}.
*/
public LDAPListener(final InetSocketAddress address,
final ServerConnectionFactory<LDAPClientContext, Integer> factory) throws IOException {
@@ -178,6 +201,32 @@
* @throws NullPointerException
* If {@code address}, {code factory}, or {@code options} was
* {@code null}.
+ * @deprecated use {@link #LDAPListener(InetSocketAddress, ServerConnectionFactory, LDAPListenerOptions)} instead
+ */
+ @Deprecated
+ public LDAPListener(final SocketAddress address,
+ final ServerConnectionFactory<LDAPClientContext, Integer> factory,
+ final LDAPListenerOptions options) throws IOException {
+ this((InetSocketAddress) address, factory, options);
+ }
+
+ /**
+ * Creates a new LDAP listener implementation which will listen for LDAP
+ * client connections at the provided address.
+ *
+ * @param address
+ * The address to listen on.
+ * @param factory
+ * The server connection factory which will be used to create
+ * server connections.
+ * @param options
+ * The LDAP listener options.
+ * @throws IOException
+ * If an error occurred while trying to listen on the provided
+ * address.
+ * @throws NullPointerException
+ * If {@code address}, {code factory}, or {@code options} was
+ * {@code null}.
*/
public LDAPListener(final InetSocketAddress address,
final ServerConnectionFactory<LDAPClientContext, Integer> factory,
@@ -250,7 +299,21 @@
* @return The {@code InetAddress} that this LDAP listener is listening on.
*/
public InetAddress getAddress() {
- return getSocketAddress().getAddress();
+ return getInetSocketAddress().getAddress();
+ }
+
+ /**
+ * Returns the host name that this LDAP listener is listening on. The
+ * returned host name is the same host name that was provided during
+ * construction and may be an IP address. More specifically, this method
+ * will not perform a reverse DNS lookup.
+ *
+ * @return The host name that this LDAP listener is listening on.
+ * @deprecated use {@link #getHostName()} instead
+ */
+ @Deprecated
+ public String getHostname() {
+ return getHostName();
}
/**
@@ -262,7 +325,7 @@
* @return The host name that this LDAP listener is listening on.
*/
public String getHostName() {
- return StaticUtils.getHostName(getSocketAddress());
+ return StaticUtils.getHostName(getInetSocketAddress());
}
/**
@@ -271,7 +334,7 @@
* @return The port that this LDAP listener is listening on.
*/
public int getPort() {
- return getSocketAddress().getPort();
+ return getInetSocketAddress().getPort();
}
/**
@@ -279,7 +342,11 @@
*
* @return The address that this LDAP listener is listening on.
*/
- public InetSocketAddress getSocketAddress() {
+ public SocketAddress getSocketAddress() {
+ return getInetSocketAddress();
+ }
+
+ private InetSocketAddress getInetSocketAddress() {
return impl.getSocketAddress();
}
diff --git a/opendj-sdk/opendj-ldap-sdk/src/test/java/org/forgerock/opendj/ldap/LDAPServer.java b/opendj-sdk/opendj-ldap-sdk/src/test/java/org/forgerock/opendj/ldap/LDAPServer.java
index ad66304..4f6df1d 100644
--- a/opendj-sdk/opendj-ldap-sdk/src/test/java/org/forgerock/opendj/ldap/LDAPServer.java
+++ b/opendj-sdk/opendj-ldap-sdk/src/test/java/org/forgerock/opendj/ldap/LDAPServer.java
@@ -30,7 +30,7 @@
import static com.forgerock.opendj.ldap.LDAPConstants.TYPE_AUTHENTICATION_SASL;
import java.io.IOException;
-import java.net.InetSocketAddress;
+import java.net.SocketAddress;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
@@ -563,7 +563,7 @@
*
* @return The socket address of the server.
*/
- public synchronized InetSocketAddress getSocketAddress() {
+ public synchronized SocketAddress getSocketAddress() {
if (!isRunning) {
throw new IllegalStateException("Server is not running");
}
diff --git a/opendj-sdk/opendj-ldap-sdk/src/test/java/org/forgerock/opendj/ldap/TestCaseUtils.java b/opendj-sdk/opendj-ldap-sdk/src/test/java/org/forgerock/opendj/ldap/TestCaseUtils.java
index e265f5d..99035c2 100644
--- a/opendj-sdk/opendj-ldap-sdk/src/test/java/org/forgerock/opendj/ldap/TestCaseUtils.java
+++ b/opendj-sdk/opendj-ldap-sdk/src/test/java/org/forgerock/opendj/ldap/TestCaseUtils.java
@@ -129,7 +129,7 @@
*
* @return The socket address of the server.
*/
- public static InetSocketAddress getServerSocketAddress() {
+ public static SocketAddress getServerSocketAddress() {
return LDAPServer.getInstance().getSocketAddress();
}
--
Gitblit v1.10.0