From b8f7d01f69a63c9d0cb6a37d4ac277ca288fdce7 Mon Sep 17 00:00:00 2001 From: Gary Russell Date: Thu, 5 Jul 2012 16:55:33 -0400 Subject: [PATCH] INT-2652 Fix mget for FTP FTP servers return full path from listNames() whereas SFTP returns just the filename. mget logic assumed SFTP behavior. Add a test to see if the filename already starts with the remote directory and remove it before calling get() (which assumes just the filename). Includes an @Ignored test for FTP and SFTP that runs an mget against a server. --- .../AbstractRemoteFileOutboundGateway.java | 21 ++++-- .../integration/file/test/BigMGetTests.java | 64 +++++++++++++++++++ .../RemoteFileOutboundGatewayTests.java | 29 +++++++-- .../ftp/outbound/BigMGetTests-context.xml | 31 +++++++++ .../ftp/outbound/BigMGetTests.java | 39 +++++++++++ .../sftp/outbound/BigMGetTests-context.xml | 31 +++++++++ .../sftp/outbound/BigMGetTests.java | 39 +++++++++++ 7 files changed, 243 insertions(+), 11 deletions(-) create mode 100644 spring-integration-file/src/main/java/org/springframework/integration/file/test/BigMGetTests.java create mode 100644 spring-integration-ftp/src/test/java/org/springframework/integration/ftp/outbound/BigMGetTests-context.xml create mode 100644 spring-integration-ftp/src/test/java/org/springframework/integration/ftp/outbound/BigMGetTests.java create mode 100644 spring-integration-sftp/src/test/java/org/springframework/integration/sftp/outbound/BigMGetTests-context.xml create mode 100644 spring-integration-sftp/src/test/java/org/springframework/integration/sftp/outbound/BigMGetTests.java diff --git a/spring-integration-file/src/main/java/org/springframework/integration/file/remote/gateway/AbstractRemoteFileOutboundGateway.java b/spring-integration-file/src/main/java/org/springframework/integration/file/remote/gateway/AbstractRemoteFileOutboundGateway.java index 9c5b54dfd1..556500e80e 100644 --- a/spring-integration-file/src/main/java/org/springframework/integration/file/remote/gateway/AbstractRemoteFileOutboundGateway.java +++ b/spring-integration-file/src/main/java/org/springframework/integration/file/remote/gateway/AbstractRemoteFileOutboundGateway.java @@ -46,7 +46,7 @@ import org.springframework.util.StringUtils; /** * Base class for Outbound Gateways that perform remote file operations. - * + * * @author Gary Russell * @since 2.1 */ @@ -196,7 +196,7 @@ public abstract class AbstractRemoteFileOutboundGateway extends AbstractReply @Override protected Object handleRequestMessage(Message requestMessage) { - Session session = this.sessionFactory.getSession(); + Session session = this.sessionFactory.getSession(); try { if (COMMAND_LS.equals(this.command)) { String dir = this.processor.processMessage(requestMessage); @@ -376,7 +376,7 @@ public abstract class AbstractRemoteFileOutboundGateway extends AbstractReply protected List mGet(Session session, String remoteDirectory, String remoteFilename) throws IOException { - String path = fixPath(remoteDirectory, remoteFilename); + String path = generateFullPath(remoteDirectory, remoteFilename); String[] fileNames = session.listNames(path); if (fileNames == null) { fileNames = new String[0]; @@ -387,12 +387,21 @@ public abstract class AbstractRemoteFileOutboundGateway extends AbstractReply } List files = new ArrayList(); for (String fileName : fileNames) { - files.add(this.get(session, fixPath(remoteDirectory, fileName), fileName, false)); + File file; + if (fileName.contains(this.remoteFileSeparator) && + fileName.startsWith(remoteDirectory)) { // the server returned the full path + file = this.get(session, fileName, + fileName.substring(fileName.lastIndexOf(this.remoteFileSeparator)), false); + } + else { + file = this.get(session, generateFullPath(remoteDirectory, fileName), fileName, false); + } + files.add(file); } return files; } - private String fixPath(String remoteDirectory, String remoteFilename) { + private String generateFullPath(String remoteDirectory, String remoteFilename) { String path; if (this.remoteFileSeparator.equals(remoteDirectory)) { path = remoteFilename; @@ -414,7 +423,7 @@ public abstract class AbstractRemoteFileOutboundGateway extends AbstractReply int index = remoteFilePath.lastIndexOf(this.remoteFileSeparator); if (index < 0) { remoteFileName = remoteFilePath; - } + } else { remoteFileName = remoteFilePath.substring(index + 1); } diff --git a/spring-integration-file/src/main/java/org/springframework/integration/file/test/BigMGetTests.java b/spring-integration-file/src/main/java/org/springframework/integration/file/test/BigMGetTests.java new file mode 100644 index 0000000000..974e877ed4 --- /dev/null +++ b/spring-integration-file/src/main/java/org/springframework/integration/file/test/BigMGetTests.java @@ -0,0 +1,64 @@ +/* + * Copyright 2002-2012 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.integration.file.test; + +import java.io.File; +import java.io.FileOutputStream; +import java.util.List; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.integration.Message; +import org.springframework.integration.MessageChannel; +import org.springframework.integration.core.PollableChannel; +import org.springframework.integration.message.GenericMessage; + +/** + * Created this test because a customer reported hanging with large mget. + * + * Could not reproduce; code is checked in, but test is @Ignored. + * @author Gary Russell + * @since 2.2 + * + */ +public abstract class BigMGetTests { + + protected static final int FILES = 600; + + @Autowired + private MessageChannel inbound; + + @Autowired + private PollableChannel resultChannel; + + protected Message> mgetManyFiles() throws Exception { + byte[] buff = new byte[150]; + for (int i = 0; i < FILES; i++) { + File f = new File("/tmp/bigmget/file" + i); + f.createNewFile(); + FileOutputStream fos = new FileOutputStream(f); + fos.write(buff); + fos.close(); + } + inbound.send(new GenericMessage("/tmp/bigmget/f*")); + for (int i = 0; i < FILES; i++) { + new File("/tmp/bigmget/file" + i).delete(); + new File("/tmp/out/file" + i).delete(); + } + @SuppressWarnings("unchecked") + Message> results = (Message>) resultChannel.receive(600000); + return results; + } +} diff --git a/spring-integration-file/src/test/java/org/springframework/integration/file/remote/gateway/RemoteFileOutboundGatewayTests.java b/spring-integration-file/src/test/java/org/springframework/integration/file/remote/gateway/RemoteFileOutboundGatewayTests.java index 89349421ee..c50d7b4688 100644 --- a/spring-integration-file/src/test/java/org/springframework/integration/file/remote/gateway/RemoteFileOutboundGatewayTests.java +++ b/spring-integration-file/src/test/java/org/springframework/integration/file/remote/gateway/RemoteFileOutboundGatewayTests.java @@ -38,7 +38,6 @@ import org.springframework.integration.Message; import org.springframework.integration.MessagingException; import org.springframework.integration.file.FileHeaders; import org.springframework.integration.file.filters.AbstractSimplePatternFileListFilter; -import org.springframework.integration.file.filters.FileListFilter; import org.springframework.integration.file.remote.AbstractFileInfo; import org.springframework.integration.file.remote.session.Session; import org.springframework.integration.file.remote.session.SessionFactory; @@ -55,7 +54,7 @@ public class RemoteFileOutboundGatewayTests { private String tmpDir = System.getProperty("java.io.tmpdir"); - + @Test(expected=IllegalArgumentException.class) public void testBad() throws Exception { SessionFactory sessionFactory = mock(SessionFactory.class); @@ -130,6 +129,19 @@ public class RemoteFileOutboundGatewayTests { @Test public void testMGetWild() throws Exception { + testMGetWildGuts("f1", "f2"); + } + + /** + * Test a wildcard mget where the full path is returned for each file + * @throws Exception + */ + @Test + public void testMGetWildFullPath() throws Exception { + testMGetWildGuts("testremote/f1", "testremote/f2"); + } + + private void testMGetWildGuts(final String path1, final String path2) { SessionFactory sessionFactory = mock(SessionFactory.class); TestRemoteFileOutboundGateway gw = new TestRemoteFileOutboundGateway (sessionFactory, "mget", "payload"); @@ -138,6 +150,7 @@ public class RemoteFileOutboundGatewayTests { new File(this.tmpDir + "/f1").delete(); new File(this.tmpDir + "/f2").delete(); when(sessionFactory.getSession()).thenReturn(new Session() { + int n; public boolean remove(String path) throws IOException { return false; } @@ -146,6 +159,12 @@ public class RemoteFileOutboundGatewayTests { } public void read(String source, OutputStream outputStream) throws IOException { + if (n++ == 0) { + assertEquals("testremote/f1", source); + } + else { + assertEquals("testremote/f2", source); + } outputStream.write("testData".getBytes()); } public void write(InputStream inputStream, String destination) @@ -166,7 +185,7 @@ public class RemoteFileOutboundGatewayTests { return false; } public String[] listNames(String path) throws IOException { - return new String[] {"f1", "f2"}; + return new String[] {path1, path2}; } }); @SuppressWarnings("unchecked") @@ -533,7 +552,7 @@ public class RemoteFileOutboundGatewayTests { } public boolean isOpen() { return open; - } + } public boolean exists(String path) throws IOException { return true; } @@ -591,7 +610,7 @@ public class RemoteFileOutboundGatewayTests { } public boolean isOpen() { return open; - } + } public boolean exists(String path) throws IOException { return true; } diff --git a/spring-integration-ftp/src/test/java/org/springframework/integration/ftp/outbound/BigMGetTests-context.xml b/spring-integration-ftp/src/test/java/org/springframework/integration/ftp/outbound/BigMGetTests-context.xml new file mode 100644 index 0000000000..988870a23d --- /dev/null +++ b/spring-integration-ftp/src/test/java/org/springframework/integration/ftp/outbound/BigMGetTests-context.xml @@ -0,0 +1,31 @@ + + + + + + + + + + + + + + + + + diff --git a/spring-integration-ftp/src/test/java/org/springframework/integration/ftp/outbound/BigMGetTests.java b/spring-integration-ftp/src/test/java/org/springframework/integration/ftp/outbound/BigMGetTests.java new file mode 100644 index 0000000000..57e0d09fc0 --- /dev/null +++ b/spring-integration-ftp/src/test/java/org/springframework/integration/ftp/outbound/BigMGetTests.java @@ -0,0 +1,39 @@ +/* + * Copyright 2002-2012 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.integration.ftp.outbound; + +import static org.junit.Assert.assertEquals; + +import org.junit.Ignore; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; + +/** + * @author Gary Russell + * @since 2.2 + * + */ +@ContextConfiguration +@RunWith(SpringJUnit4ClassRunner.class) +public class BigMGetTests extends org.springframework.integration.file.test.BigMGetTests { + + @Test @Ignore // needs directories and server (FTP and SFTP) + public void doTest() throws Exception { + assertEquals(FILES, this.mgetManyFiles().getPayload().size()); + } +} diff --git a/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/outbound/BigMGetTests-context.xml b/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/outbound/BigMGetTests-context.xml new file mode 100644 index 0000000000..7249bd1379 --- /dev/null +++ b/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/outbound/BigMGetTests-context.xml @@ -0,0 +1,31 @@ + + + + + + + + + + + + + + + + + diff --git a/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/outbound/BigMGetTests.java b/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/outbound/BigMGetTests.java new file mode 100644 index 0000000000..5f26cf5c75 --- /dev/null +++ b/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/outbound/BigMGetTests.java @@ -0,0 +1,39 @@ +/* + * Copyright 2002-2012 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.integration.sftp.outbound; + +import static org.junit.Assert.assertEquals; + +import org.junit.Ignore; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; + +/** + * @author Gary Russell + * @since 2.2 + * + */ +@ContextConfiguration +@RunWith(SpringJUnit4ClassRunner.class) +public class BigMGetTests extends org.springframework.integration.file.test.BigMGetTests { + + @Test @Ignore // needs directories and server (FTP and SFTP) + public void doTest() throws Exception { + assertEquals(FILES, this.mgetManyFiles().getPayload().size()); + } +}