GH-3199: TCP FailoverClientCF fail back default

Resolves #3199

- change the default behavior to not fail back until the current connection fails
- reduce method complexity (Sonar)
- 5.3 only

Author: garyrussell

spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/FailoverClientConnectionFactory.java

@@ -42,15 +42,17 @@
  */
 public class FailoverClientConnectionFactory extends AbstractClientConnectionFactory {
 
-	private static final long DEFAULT_REFRESH_SHARED_INTERVAL = 0L;
+	private static final long DEFAULT_REFRESH_SHARED_INTERVAL = Long.MAX_VALUE;
 
 	private final List<AbstractClientConnectionFactory> factories;
 
 	private final boolean cachingDelegates;
 
 	private long refreshSharedInterval = DEFAULT_REFRESH_SHARED_INTERVAL;
 
-	private boolean closeOnRefresh;
+	private boolean closeOnRefresh = true;
+
+	private boolean failBack;
 
 	private volatile long creationTime;
 
@@ -70,10 +72,9 @@ public FailoverClientConnectionFactory(List<AbstractClientConnectionFactory> fac
 	/**
 	 * When using a shared connection {@link #setSingleUse(boolean) singleUse} is false,
 	 * specify how long to wait before trying to fail back to start from the beginning of
-	 * the factory list. Default is 0 for backwards compatibility to always try to get a
-	 * connection to the primary server. If you don't want to fail back until the current
-	 * connection is closed, set this to {@link Long#MAX_VALUE}.
-	 * Cannot be changed when using {@link CachingClientConnectionFactory} delegates.
+	 * the factory list. Default is {@link Long#MAX_VALUE} - meaning only fail back when
+	 * the current connection fails. Cannot be changed when using
+	 * {@link CachingClientConnectionFactory} delegates.
 	 * @param refreshSharedInterval the interval in milliseconds.
 	 * @since 4.3.22
 	 * @see #setSingleUse(boolean)
@@ -83,6 +84,7 @@ public void setRefreshSharedInterval(long refreshSharedInterval) {
 		Assert.isTrue(!this.cachingDelegates,
 				"'refreshSharedInterval' cannot be changed when using 'CachingClientConnectionFactory` delegates");
 		this.refreshSharedInterval = refreshSharedInterval;
+		this.failBack = refreshSharedInterval != Long.MAX_VALUE;
 	}
 
 	/**
@@ -149,7 +151,7 @@ public void registerSender(TcpSender sender) {
 	protected TcpConnectionSupport obtainConnection() throws InterruptedException {
 		FailoverTcpConnection sharedConnection = (FailoverTcpConnection) getTheConnection();
 		boolean shared = !isSingleUse() && !this.cachingDelegates;
-		boolean refreshShared = shared
+		boolean refreshShared = this.failBack && shared
 				&& sharedConnection != null
 				&& System.currentTimeMillis() > this.creationTime + this.refreshSharedInterval;
 		if (sharedConnection != null && sharedConnection.isOpen() && !refreshShared) {
@@ -162,20 +164,26 @@ protected TcpConnectionSupport obtainConnection() throws InterruptedException {
 		}
 		failoverTcpConnection.incrementEpoch();
 		if (shared) {
-			this.creationTime = System.currentTimeMillis();
-			/*
-			 * We may have simply wrapped the same connection in a new wrapper; don't close.
-			 */
-			if (refreshShared && this.closeOnRefresh
-					&& !sharedConnection.delegate.equals(failoverTcpConnection.delegate)
-					&& sharedConnection.isOpen()) {
-				sharedConnection.close();
-			}
+			closeRefreshedIfNecessary(sharedConnection, refreshShared, failoverTcpConnection);
 			setTheConnection(failoverTcpConnection);
 		}
 		return failoverTcpConnection;
 	}
 
+	private void closeRefreshedIfNecessary(FailoverTcpConnection sharedConnection, boolean refreshShared,
+			FailoverTcpConnection failoverTcpConnection) {
+
+		this.creationTime = System.currentTimeMillis();
+		/*
+		 * We may have simply wrapped the same connection in a new wrapper; don't close.
+		 */
+		if (refreshShared && this.closeOnRefresh
+				&& !sharedConnection.delegate.equals(failoverTcpConnection.delegate)
+				&& sharedConnection.isOpen()) {
+			sharedConnection.close();
+		}
+	}
+
 	@Override
 	public void start() {
 		for (AbstractClientConnectionFactory factory : this.factories) {

src/reference/asciidoc/ip.adoc

@@ -536,23 +536,25 @@ The following example shows how to configure a failover client connection factor
 
 NOTE: When using the failover connection factory, the `singleUse` property must be consistent between the factory itself and the list of factories it is configured to use.
 
-The connection factory has two properties when used with a shared connection (`singleUse=false`):
+The connection factory has two properties related to failing back, when used with a shared connection (`singleUse=false`):
 
 * `refreshSharedInterval`
 * `closeOnRefresh`
 
-These are `0` and `false` to retain the same behavior that existed before the properties were added.
-
 Consider the following scenario based on the above configuration:
 Let's say `clientFactory1` cannot establish a connection but `clientFactory2` can.
-Each time the `failCF` `getConnection()` method is called, we will again attempt to connect using `clientFactory1`; if successful, the "old" connection will remain open and may be reused in future if the first factory fails once more.
+When the `failCF` `getConnection()` method is called after the `refreshSharedInterval` has passed, we will again attempt to connect using `clientFactory1`; if successful, the connection to `clientFactory2` will be closed.
+If `closeOnRefresh` is `false`, the "old" connection will remain open and may be reused in future if the first factory fails once more.
 
-Set `refreshSharedInterval` to only attempt to reconnect with the first factory after that time has expired; set it to `Long.MAX_VALUE` if you only want to fail back to the first factory when the current connection fails.
+Set `refreshSharedInterval` to only attempt to reconnect with the first factory after that time has expired; setting it to `Long.MAX_VALUE` (default) if you only want to fail back to the first factory when the current connection fails.
 
 Set `closeOnRefresh` to close the "old" connection after a refresh actually creates a new connection.
 
 IMPORTANT: These properties do not apply if any of the delegate factories is a `CachingClientConnectionFactory` because the connection caching is handled there; in that case the list of connection factories will always be consulted to get a connection.
 
+Starting with version 5.3, these default to `Long.MAX_VALUE` and `true` so the factory only attempts to fail back when the current connection fails.
+To revert to the default behavior of previous versions, set them to `0` and `false`.
+
 [[tcp-affinity-cf]]
 ==== TCP Thread Affinity Connection Factory