INT-3919: FTP: Allow null for Remote Directory

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

Since `FtpClient` supports `null` for the `LS` command, treating it as a current `working directory`,
there is no reason to forbid `null` from the FTP adapters end-user perspective.

* Allow `null` for the `FtpSession` `list()` and `listNames()` methods
* Allow `null` in the `remote-directory` for the `<int-ftp:inbound-channel-adapter>`
* Allow `null` in the `expression` for the `FtpOutboundGateway`

Polishing - send error if error on async output

Cover `onFailure()` from `onSuccess()` with the `errorChannel`

Address PR Comments

* Get rid of `null` population for the `remoteDirectoryExpression` in the `AbstractPollingInboundChannelAdapterParser`
* Populate `new LiteralExpression(null)` from the `FtpInboundFileSynchronizer` ctor
* Introduce `buildRemotePath(parent, child)` function in the `AbstractRemoteFileOutboundGateway` with the `null` logic for `parent`
* Rework `mGetWithoutRecursion()` to use `LS` command and allow `null` for the dir.
* Fix tests according the new `mGetWithoutRecursion()` logic

Polishing
This commit is contained in:
Artem Bilan
2016-02-05 16:06:10 -05:00
committed by Gary Russell
parent 2fad35d9b8
commit 7595e4f142
13 changed files with 297 additions and 129 deletions

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2002-2015 the original author or authors.
* Copyright 2002-2016 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.
@@ -82,6 +82,32 @@ public class FtpOutboundGateway extends AbstractRemoteFileOutboundGateway<FTPFil
super(remoteFileTemplate, command, expression);
}
/**
* Construct an instance with the supplied session factory, a command ('ls', 'get'
* etc).
* <p> The {@code remoteDirectory} expression is {@code null} assuming to use
* the {@code workingDirectory} from the FTP Client.
* @param sessionFactory the session factory.
* @param command the command.
* @since 4.3
*/
public FtpOutboundGateway(SessionFactory<FTPFile> sessionFactory, String command) {
this(sessionFactory, command, null);
}
/**
* Construct an instance with the supplied remote file template, a command ('ls',
* 'get' etc).
* <p> The {@code remoteDirectory} expression is {@code null} assuming to use
* the {@code workingDirectory} from the FTP Client.
* @param remoteFileTemplate the remote file template.
* @param command the command.
* @since 4.3
*/
public FtpOutboundGateway(RemoteFileTemplate<FTPFile> remoteFileTemplate, String command) {
this(remoteFileTemplate, command, null);
}
@Override
public String getComponentType() {
return "ftp:outbound-gateway";

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2002-2014 the original author or authors.
* Copyright 2002-2016 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.
@@ -18,6 +18,7 @@ package org.springframework.integration.ftp.inbound;
import org.apache.commons.net.ftp.FTPFile;
import org.springframework.expression.common.LiteralExpression;
import org.springframework.integration.file.remote.session.Session;
import org.springframework.integration.file.remote.session.SessionFactory;
import org.springframework.integration.file.remote.synchronizer.AbstractInboundFileSynchronizer;
@@ -40,9 +41,9 @@ public class FtpInboundFileSynchronizer extends AbstractInboundFileSynchronizer<
*/
public FtpInboundFileSynchronizer(SessionFactory<FTPFile> sessionFactory) {
super(sessionFactory);
setRemoteDirectoryExpression(new LiteralExpression(null));
}
@Override
protected boolean isFile(FTPFile file) {
return file != null && file.isFile();

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2002-2014 the original author or authors.
* Copyright 2002-2016 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.
@@ -16,7 +16,6 @@
package org.springframework.integration.ftp.session;
import java.io.IOException;
import java.net.SocketException;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
@@ -40,8 +39,6 @@ import org.springframework.util.Assert;
*/
public abstract class AbstractFtpSessionFactory<T extends FTPClient> implements SessionFactory<FTPFile> {
public static final String DEFAULT_REMOTE_WORKING_DIRECTORY = "/";
private final Log logger = LogFactory.getLog(this.getClass());
protected FTPClientConfig config;
@@ -172,7 +169,7 @@ public abstract class AbstractFtpSessionFactory<T extends FTPClient> implements
}
}
private T createClient() throws SocketException, IOException {
private T createClient() throws IOException {
final T client = this.createClientInstance();
Assert.notNull(client, "client must not be null");
client.configure(this.config);
@@ -200,7 +197,7 @@ public abstract class AbstractFtpSessionFactory<T extends FTPClient> implements
// Login
if (!client.login(username, password)) {
throw new IllegalStateException("Login failed. The respponse from the server is: " +
throw new IllegalStateException("Login failed. The response from the server is: " +
client.getReplyString());
}

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2002-2013 the original author or authors.
* Copyright 2002-2016 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.
@@ -36,6 +36,7 @@ import org.springframework.util.Assert;
* @author Mark Fisher
* @author Oleg Zhurakousky
* @author Gary Russell
* @author Artem Bilan
* @since 2.0
*/
public class FtpSession implements Session<FTPFile> {
@@ -55,22 +56,21 @@ public class FtpSession implements Session<FTPFile> {
@Override
public boolean remove(String path) throws IOException {
Assert.hasText(path, "path must not be null");
boolean completed = this.client.deleteFile(path);
if (!completed) {
if (!this.client.deleteFile(path)) {
throw new IOException("Failed to delete '" + path + "'. Server replied with: " + client.getReplyString());
}
return completed;
else {
return true;
}
}
@Override
public FTPFile[] list(String path) throws IOException {
Assert.hasText(path, "path must not be null");
return this.client.listFiles(path);
}
@Override
public String[] listNames(String path) throws IOException {
Assert.hasText(path, "path must not be null");
return this.client.listNames(path);
}
@@ -83,7 +83,7 @@ public class FtpSession implements Session<FTPFile> {
throw new IOException("Failed to copy '" + path +
"'. Server replied with: " + this.client.getReplyString());
}
logger.info("File has been successfully transfered from: " + path);
logger.info("File has been successfully transferred from: " + path);
}
@Override
@@ -93,7 +93,8 @@ public class FtpSession implements Session<FTPFile> {
}
InputStream inputStream = this.client.retrieveFileStream(source);
if (inputStream == null) {
throw new IOException("Failed to obtain InputStream for remote file " + source + ": " + this.client.getReplyCode());
throw new IOException("Failed to obtain InputStream for remote file " + source + ": "
+ this.client.getReplyCode());
}
return inputStream;
}
@@ -123,7 +124,7 @@ public class FtpSession implements Session<FTPFile> {
+ "'. Server replied with: " + this.client.getReplyString());
}
if (logger.isInfoEnabled()) {
logger.info("File has been successfully transfered to: " + path);
logger.info("File has been successfully transferred to: " + path);
}
}
@@ -196,7 +197,8 @@ public class FtpSession implements Session<FTPFile> {
Assert.hasText(path, "'path' must not be empty");
String currentWorkingPath = this.client.printWorkingDirectory();
Assert.state(currentWorkingPath != null, "working directory cannot be determined, therefore exists check can not be completed");
Assert.state(currentWorkingPath != null,
"working directory cannot be determined, therefore exists check can not be completed");
boolean exists = false;
try {

View File

@@ -41,8 +41,9 @@
<int-ftp:outbound-gateway session-factory="ftpSessionFactory"
request-channel="inboundMGet"
command="mget"
command-options="-f"
expression="payload"
local-directory-expression="@ftpServer.targetLocalDirectoryName + #remoteDirectory"
local-directory-expression="@ftpServer.targetLocalDirectoryName + (#remoteDirectory ?: '')"
local-filename-generator-expression="#remoteFileName.replaceFirst('ftpSource', 'localTarget')"
reply-channel="output"/>
@@ -169,5 +170,22 @@
<bean id="messageSessionCallback"
class="org.springframework.integration.ftp.outbound.FtpServerOutboundTests$TestMessageSessionCallback"/>
<int-ftp:outbound-gateway session-factory="ftpSessionFactory"
request-channel="inboundLs"
command="ls"
command-options="-1"
reply-channel="output"/>
<int-ftp:inbound-channel-adapter id="ftpInbound"
channel="output"
auto-startup="false"
session-factory="ftpSessionFactory"
auto-create-local-directory="true"
delete-remote-files="false"
filename-pattern="*.txt"
temporary-file-suffix=".foo"
local-directory="#{T (System).getProperty('java.io.tmpdir') + T (java.util.UUID).randomUUID().toString()}">
<int:poller fixed-delay="100"/>
</int-ftp:inbound-channel-adapter>
</beans>

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2013-2015 the original author or authors.
* Copyright 2013-2016 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.
@@ -17,12 +17,17 @@
package org.springframework.integration.ftp.outbound;
import static org.hamcrest.Matchers.anyOf;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.isOneOf;
import static org.hamcrest.Matchers.not;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
@@ -44,6 +49,7 @@ import java.util.Set;
import java.util.concurrent.BlockingQueue;
import java.util.concurrent.atomic.AtomicBoolean;
import org.apache.commons.net.ftp.FTPClient;
import org.apache.commons.net.ftp.FTPFile;
import org.hamcrest.Matchers;
import org.junit.Before;
@@ -57,6 +63,7 @@ import org.springframework.beans.factory.BeanFactory;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.expression.spel.standard.SpelExpressionParser;
import org.springframework.integration.channel.DirectChannel;
import org.springframework.integration.endpoint.SourcePollingChannelAdapter;
import org.springframework.integration.file.FileHeaders;
import org.springframework.integration.file.filters.FileListFilter;
import org.springframework.integration.file.remote.InputStreamCallback;
@@ -139,6 +146,12 @@ public class FtpServerOutboundTests {
@Autowired
private DirectChannel inboundCallback;
@Autowired
private DirectChannel inboundLs;
@Autowired
private SourcePollingChannelAdapter ftpInbound;
@Before
public void setup() {
this.ftpServer.recursiveDelete(ftpServer.getTargetLocalDirectory());
@@ -183,7 +196,7 @@ public class FtpServerOutboundTests {
@SuppressWarnings("unchecked")
public void testInt2866LocalDirectoryExpressionMGET() {
String dir = "ftpSource/";
this.inboundMGet.send(new GenericMessage<Object>(dir + "*.txt"));
this.inboundMGet.send(new GenericMessage<Object>("*.txt"));
Message<?> result = this.output.receive(1000);
assertNotNull(result);
List<File> localFiles = (List<File>) result.getPayload();
@@ -205,11 +218,29 @@ public class FtpServerOutboundTests {
}
}
@Test
@SuppressWarnings("unchecked")
public void testMGETOnNullDir() throws IOException {
Session<FTPFile> session = ftpSessionFactory.getSession();
((FTPClient) session.getClientInstance()).changeWorkingDirectory("ftpSource");
session.close();
this.inboundMGet.send(new GenericMessage<Object>(""));
Message<?> result = this.output.receive(1000);
assertNotNull(result);
List<File> localFiles = (List<File>) result.getPayload();
for (File file : localFiles) {
assertThat(file.getName(), isOneOf("localTarget1.txt", "localTarget2.txt"));
assertThat(file.getName(), not(containsString("null")));
}
}
@Test
@SuppressWarnings("unchecked")
public void testInt3172LocalDirectoryExpressionMGETRecursive() {
String dir = "ftpSource/";
this.inboundMGetRecursive.send(new GenericMessage<Object>(dir + "*"));
this.inboundMGetRecursive.send(new GenericMessage<Object>("*"));
Message<?> result = this.output.receive(1000);
assertNotNull(result);
List<File> localFiles = (List<File>) result.getPayload();
@@ -399,17 +430,19 @@ public class FtpServerOutboundTests {
@Test
public void testMgetPartial() throws Exception {
Session<FTPFile> session = spyOnSession();
doAnswer(new Answer<String[]>() {
doAnswer(new Answer<FTPFile[]>() {
@Override
public String[] answer(InvocationOnMock invocation) throws Throwable {
String[] files = (String[]) invocation.callRealMethod();
public FTPFile[] answer(InvocationOnMock invocation) throws Throwable {
FTPFile[] files = (FTPFile[]) invocation.callRealMethod();
// add an extra file where the get will fail
files = Arrays.copyOf(files, files.length + 1);
files[files.length - 1] = "bogus.txt";
FTPFile bogusFile = new FTPFile();
bogusFile.setName("bogus.txt");
files[files.length - 1] = bogusFile;
return files;
}
}).when(session).listNames("ftpSource/subFtpSource/*");
}).when(session).list("ftpSource/subFtpSource/*");
String dir = "ftpSource/subFtpSource/";
try {
this.inboundMGet.send(new GenericMessage<Object>(dir + "*"));
@@ -547,6 +580,51 @@ public class FtpServerOutboundTests {
assertEquals("FOO", receive.getPayload());
}
@Test
@SuppressWarnings("unchecked")
public void testLsForNullDir() throws IOException {
Session<FTPFile> session = ftpSessionFactory.getSession();
((FTPClient) session.getClientInstance()).changeWorkingDirectory("ftpSource");
session.close();
this.inboundLs.send(new GenericMessage<String>("foo"));
Message<?> receive = this.output.receive(10000);
assertNotNull(receive);
assertThat(receive.getPayload(), instanceOf(List.class));
List<String> files = (List<String>) receive.getPayload();
assertEquals(2, files.size());
assertThat(files, containsInAnyOrder("ftpSource1.txt", "ftpSource2.txt"));
FTPFile[] ftpFiles = ftpSessionFactory.getSession().list(null);
for (FTPFile ftpFile : ftpFiles) {
if (!ftpFile.isDirectory()) {
assertTrue(files.contains(ftpFile.getName()));
}
}
}
@Test
public void testInboundChannelAdapterWithNullDir() throws IOException {
Session<FTPFile> session = ftpSessionFactory.getSession();
((FTPClient) session.getClientInstance()).changeWorkingDirectory("ftpSource");
session.close();
this.ftpInbound.start();
Message<?> message = this.output.receive(10000);
assertNotNull(message);
assertThat(message.getPayload(), instanceOf(File.class));
assertEquals("ftpSource1.txt", ((File) message.getPayload()).getName());
message = this.output.receive(10000);
assertNotNull(message);
assertThat(message.getPayload(), instanceOf(File.class));
assertEquals("ftpSource2.txt", ((File) message.getPayload()).getName());
assertNull(this.output.receive(10));
this.ftpInbound.stop();
}
public static class SortingFileListFilter implements FileListFilter<File> {
@Override