INT-2586 FTP Session Factory: Expose Timeout Props

JIRA: https://jira.spring.io/browse/INT-2586

Add timeout properties to the `DefaultFtpSessionFactory`
avoiding the need to subclass it just to set these
common properties.

spring-integration-ftp/src/main/java/org/springframework/integration/ftp/session/AbstractFtpSessionFactory.java

@@ -63,6 +63,12 @@ public abstract class AbstractFtpSessionFactory<T extends FTPClient> implements
 
 	protected String controlEncoding = FTP.DEFAULT_CONTROL_ENCODING;
 
+	private Integer connectTimeout;
+
+	private Integer defaultTimeout;
+
+	private Integer dataTimeout;
+
 
 	/**
 	 * File types defined by {@link org.apache.commons.net.ftp.FTP} constants:
@@ -133,6 +139,30 @@ public abstract class AbstractFtpSessionFactory<T extends FTPClient> implements
 		this.clientMode = clientMode;
 	}
 
+	/**
+	 * Set the connect timeout for the socket.
+	 * @param connectTimeout the timeout
+	 */
+	public void setConnectTimeout(int connectTimeout) {
+		 this.connectTimeout = connectTimeout;
+	}
+
+	/**
+	 * Set the (socket option) timeout on the command socket.
+	 * @param defaultTimeout the timeout.
+	 */
+	public void setDefaultTimeout(int defaultTimeout) {
+		this.defaultTimeout = defaultTimeout;
+	}
+
+	/**
+	 * Set the (socket option) timeout on the data connection.
+	 * @param dataTimeout the timeout.
+	 */
+	public void setDataTimeout(int dataTimeout) {
+		this.dataTimeout = dataTimeout;
+	}
+
 	@Override
 	public Session<FTPFile> getSession() {
 		try {
@@ -148,6 +178,15 @@ public abstract class AbstractFtpSessionFactory<T extends FTPClient> implements
 		Assert.notNull(client, "client must not be null");
 		client.configure(this.config);
 		Assert.hasText(this.username, "username is required");
+		if (this.connectTimeout != null) {
+			client.setConnectTimeout(this.connectTimeout);
+		}
+		if (this.defaultTimeout != null) {
+			client.setDefaultTimeout(this.defaultTimeout);
+		}
+		if (this.dataTimeout != null) {
+			client.setDataTimeout(this.dataTimeout);
+		}
 
 		this.postProcessClientBeforeConnect(client);
 

spring-integration-ftp/src/test/java/org/springframework/integration/ftp/session/SessionFactoryTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2013 the original author or authors.
+ * Copyright 2002-2014 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -15,6 +15,12 @@
  */
 package org.springframework.integration.ftp.session;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+
 import java.lang.reflect.Field;
 import java.util.Random;
 import java.util.concurrent.ExecutorService;
@@ -22,30 +28,50 @@ import java.util.concurrent.Executors;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
 
-import org.junit.Assert;
 import org.apache.commons.net.ftp.FTPClient;
+import org.junit.Assert;
 import org.junit.Ignore;
 import org.junit.Test;
 import org.mockito.Mockito;
-import org.springframework.messaging.MessagingException;
+
 import org.springframework.integration.file.remote.session.CachingSessionFactory;
 import org.springframework.integration.file.remote.session.Session;
 import org.springframework.integration.file.remote.session.SessionFactory;
 import org.springframework.integration.test.util.TestUtils;
-
-import static org.junit.Assert.fail;
-
-import static org.junit.Assert.assertEquals;
+import org.springframework.messaging.MessagingException;
 
 /**
  * @author Oleg Zhurakousky
  * @author Gunnar Hillert
+ * @author Gary Russell
  *
  */
 @SuppressWarnings({"rawtypes","unchecked"})
 public class SessionFactoryTests {
 
 
+	@Test
+	public void testTimeouts() throws Exception {
+		final FTPClient client = mock(FTPClient.class);
+		DefaultFtpSessionFactory sessionFactory = new DefaultFtpSessionFactory() {
+
+			@Override
+			protected FTPClient createClientInstance() {
+				return client;
+			}
+		};
+		sessionFactory.setUsername("foo");
+		sessionFactory.setConnectTimeout(123);
+		sessionFactory.setDefaultTimeout(456);
+		sessionFactory.setDataTimeout(789);
+		doReturn(200).when(client).getReplyCode();
+		doReturn(true).when(client).login("foo", null);
+		sessionFactory.getSession();
+		verify(client).setConnectTimeout(123);
+		verify(client).setDefaultTimeout(456);
+		verify(client).setDataTimeout(789);
+	}
+
 	@Test
 	public void testWithControlEncoding() {
 		DefaultFtpSessionFactory sessionFactory = new DefaultFtpSessionFactory();
@@ -164,6 +190,7 @@ public class SessionFactoryTests {
 		final AtomicInteger failures = new AtomicInteger();
 		for (int i = 0; i < 30; i++) {
 			executor.execute(new Runnable() {
+				@Override
 				public void run() {
 					try {
 						Session session = factory.getSession();

src/reference/docbook/ftp.xml

@@ -104,15 +104,19 @@ xsi:schemaLocation="http://www.springframework.org/schema/integration/ftp
   </para>
 
   <para>
-  	<emphasis>Advanced Configuration</emphasis>
+	<emphasis role="bold">Advanced Configuration</emphasis>
   </para>
 
   <para>
   <classname>DefaultFtpSessionFactory</classname> provides an abstraction over the underlying client API which,
   since <emphasis>Spring Integration 2.0</emphasis>, is <ulink url="http://commons.apache.org/net/">Apache Commons Net</ulink>.
   This spares you from the low level configuration details
-  of the <classname>org.apache.commons.net.ftp.FTPClient</classname>. However there are times when access to lower level <classname>FTPClient</classname> details is
-  necessary to achieve more advanced configuration (e.g., setting data timeout, default timeout etc.). For that purpose, <classname>AbstractFtpSessionFactory</classname>
+  of the <classname>org.apache.commons.net.ftp.FTPClient</classname>. Several common properties are exposed on the
+  session factory (since <emphasis>version 4.0</emphasis>, this now includes <code>connectTimeout</code>,
+  <code>defaultTimeout</code> and <code>dataTimeout</code>).
+  However there are times when access to lower level <classname>FTPClient</classname> configuration is
+  necessary to achieve more advanced configuration (e.g., setting the port range for active mode etc.).
+  For that purpose, <classname>AbstractFtpSessionFactory</classname>
   (the base class for all FTP Session Factories) exposes hooks, in the form of the two post-processing methods below.
   <programlisting language="java"><![CDATA[/**
  * Will handle additional initialization after client.connect() method was invoked,
@@ -132,8 +136,7 @@ protected void postProcessClientBeforeConnect(T client) throws IOException {
 	<programlisting language="java"><![CDATA[public class AdvancedFtpSessionFactory extends DefaultFtpSessionFactory {
 
     protected void postProcessClientBeforeConnect(FTPClient ftpClient) throws IOException {
-       ftpClient.setDataTimeout(5000);
-       ftpClient.setDefaultTimeout(5000);
+       ftpClient.setActivePortRange(4000, 5000);
     }
 }]]></programlisting>
   </para>

src/reference/docbook/whats-new.xml

@@ -312,5 +312,15 @@
 				<xref linkend="amqp-outbound-gateway"/>.
 			</para>
 		</section>
+		<section id="4.0-ftp">
+			<title>FTP Timeouts</title>
+			<para>
+				The <classname>DefaultFtpSessionFactory</classname> now exposes the
+				<code>connectTimeout</code>, <code>defaultTimeout</code> and <code>dataTimeout</code>
+				properties, avoiding the need to subclass the factory just to set these common
+				properties. The <code>postProcess*</code> methods are still available for more
+				advanced configuration. See <xref linkend="ftp-session-factory"/> for more information.
+			</para>
+		</section>
 	</section>
 </chapter>