From cc90ce92b0612c7935e422b13b46c85ef1303758 Mon Sep 17 00:00:00 2001 From: Gary Russell Date: Fri, 9 Nov 2012 11:35:29 -0500 Subject: [PATCH] INT-2438 Fix Temporary Remote Directory The final remote directory expression processor was being used instead of the temporary directory expression processor. Add a test to confirm the correct processors are used. --- .../FileTransferringMessageHandler.java | 38 ++++++++++--------- .../FileTransferringMessageHandlerTests.java | 30 +++++++++++++++ 2 files changed, 50 insertions(+), 18 deletions(-) diff --git a/spring-integration-file/src/main/java/org/springframework/integration/file/remote/handler/FileTransferringMessageHandler.java b/spring-integration-file/src/main/java/org/springframework/integration/file/remote/handler/FileTransferringMessageHandler.java index 14d3ef7f2a..4445d495ff 100644 --- a/spring-integration-file/src/main/java/org/springframework/integration/file/remote/handler/FileTransferringMessageHandler.java +++ b/spring-integration-file/src/main/java/org/springframework/integration/file/remote/handler/FileTransferringMessageHandler.java @@ -45,6 +45,7 @@ import org.springframework.util.StringUtils; * @author Josh Long * @author Oleg Zhurakousky * @author David Turanski + * @author Gary Russell * @since 2.0 */ public class FileTransferringMessageHandler extends AbstractMessageHandler { @@ -81,7 +82,7 @@ public class FileTransferringMessageHandler extends AbstractMessageHandler { public void setAutoCreateDirectory(boolean autoCreateDirectory) { this.autoCreateDirectory = autoCreateDirectory; } - + public void setRemoteFileSeparator(String remoteFileSeparator) { Assert.notNull(remoteFileSeparator, "'remoteFileSeparator' must not be null"); this.remoteFileSeparator = remoteFileSeparator; @@ -91,12 +92,12 @@ public class FileTransferringMessageHandler extends AbstractMessageHandler { Assert.notNull(remoteDirectoryExpression, "remoteDirectoryExpression must not be null"); this.directoryExpressionProcessor = new ExpressionEvaluatingMessageProcessor(remoteDirectoryExpression, String.class); } - + public void setTemporaryRemoteDirectoryExpression(Expression temporaryRemoteDirectoryExpression) { Assert.notNull(temporaryRemoteDirectoryExpression, "temporaryRemoteDirectoryExpression must not be null"); this.temporaryDirectoryExpressionProcessor = new ExpressionEvaluatingMessageProcessor(temporaryRemoteDirectoryExpression, String.class); } - + protected String getTemporaryFileSuffix() { return this.temporaryFileSuffix; } @@ -123,13 +124,14 @@ public class FileTransferringMessageHandler extends AbstractMessageHandler { public void setCharset(String charset) { this.charset = charset; } - + public void setTemporaryFileSuffix(String temporaryFileSuffix) { Assert.notNull(temporaryFileSuffix, "'temporaryFileSuffix' must not be null"); this.hasExplicitlySetSuffix = true; this.temporaryFileSuffix = temporaryFileSuffix; } + @Override protected void onInit() throws Exception { Assert.notNull(this.directoryExpressionProcessor, "remoteDirectoryExpression is required"); if (this.autoCreateDirectory){ @@ -149,7 +151,7 @@ public class FileTransferringMessageHandler extends AbstractMessageHandler { String remoteDirectory = this.directoryExpressionProcessor.processMessage(message); String temporaryRemoteDirectory = remoteDirectory; if (this.temporaryDirectoryExpressionProcessor != null){ - temporaryRemoteDirectory = this.directoryExpressionProcessor.processMessage(message); + temporaryRemoteDirectory = this.temporaryDirectoryExpressionProcessor.processMessage(message); } String fileName = this.fileNameGenerator.generateFileName(message); this.sendFileToRemoteDirectory(file, temporaryRemoteDirectory, remoteDirectory, fileName, session); @@ -192,7 +194,7 @@ public class FileTransferringMessageHandler extends AbstractMessageHandler { if (payload instanceof File) { sendableFile = (File) payload; } - else if (payload instanceof byte[] || payload instanceof String) { + else if (payload instanceof byte[] || payload instanceof String) { String tempFileName = this.fileNameGenerator.generateFileName(message) + ".tmp"; sendableFile = new File(this.temporaryDirectory, tempFileName); // will only create temp file for String/byte[] byte[] bytes = null; @@ -215,7 +217,7 @@ public class FileTransferringMessageHandler extends AbstractMessageHandler { } } - private void sendFileToRemoteDirectory(File file, String temporaryRemoteDirectory, String remoteDirectory, String fileName, Session session) + private void sendFileToRemoteDirectory(File file, String temporaryRemoteDirectory, String remoteDirectory, String fileName, Session session) throws FileNotFoundException, IOException { remoteDirectory = this.normalizeDirectoryPath(remoteDirectory); @@ -226,17 +228,17 @@ public class FileTransferringMessageHandler extends AbstractMessageHandler { // write remote file first with temporary file extension if enabled String tempFilePath = tempRemoteFilePath + (useTemporaryFileName ? this.temporaryFileSuffix : ""); - + if (this.autoCreateDirectory) { try { this.makeDirectories(remoteDirectory, session); - } + } catch (IllegalStateException e) { // Revert to old FTP behavior if recursive mkdir fails, for backwards compatibility session.mkdir(remoteDirectory); } } - + FileInputStream fileInputStream = new FileInputStream(file); try { session.write(fileInputStream, tempFilePath); @@ -244,7 +246,7 @@ public class FileTransferringMessageHandler extends AbstractMessageHandler { if (useTemporaryFileName){ session.rename(tempFilePath, remoteFilePath); } - } + } catch (Exception e) { throw new MessagingException("Failed to write to '" + tempFilePath + "' while uploading the file", e); } @@ -252,22 +254,22 @@ public class FileTransferringMessageHandler extends AbstractMessageHandler { fileInputStream.close(); } } - + private String normalizeDirectoryPath(String directoryPath){ if (!StringUtils.hasText(directoryPath)) { directoryPath = ""; } else if (!directoryPath.endsWith(this.remoteFileSeparator)) { - directoryPath += this.remoteFileSeparator; + directoryPath += this.remoteFileSeparator; } return directoryPath; } - + private void makeDirectories(String path, Session session) throws IOException { - if (!session.exists(path)){ + if (!session.exists(path)){ int nextSeparatorIndex = path.lastIndexOf(remoteFileSeparator); - + if (nextSeparatorIndex > -1){ List pathsToCreate = new LinkedList(); while (nextSeparatorIndex > -1){ @@ -276,12 +278,12 @@ public class FileTransferringMessageHandler extends AbstractMessageHandler { // no more paths to create break; } - else { + else { pathsToCreate.add(0, pathSegment); nextSeparatorIndex = pathSegment.lastIndexOf(remoteFileSeparator); } } - + for (String pathToCreate : pathsToCreate) { if (logger.isDebugEnabled()){ logger.debug("Creating '" + pathToCreate + "'"); diff --git a/spring-integration-file/src/test/java/org/springframework/integration/file/remote/handler/FileTransferringMessageHandlerTests.java b/spring-integration-file/src/test/java/org/springframework/integration/file/remote/handler/FileTransferringMessageHandlerTests.java index 02ce576872..466fddd5c9 100644 --- a/spring-integration-file/src/test/java/org/springframework/integration/file/remote/handler/FileTransferringMessageHandlerTests.java +++ b/spring-integration-file/src/test/java/org/springframework/integration/file/remote/handler/FileTransferringMessageHandlerTests.java @@ -17,6 +17,7 @@ package org.springframework.integration.file.remote.handler; import static junit.framework.Assert.assertFalse; +import static org.junit.Assert.assertEquals; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; @@ -24,12 +25,14 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import java.io.InputStream; +import java.util.concurrent.atomic.AtomicReference; import org.junit.Test; import org.mockito.Mockito; import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; import org.springframework.expression.ExpressionParser; +import org.springframework.expression.common.LiteralExpression; import org.springframework.expression.spel.standard.SpelExpressionParser; import org.springframework.integration.Message; import org.springframework.integration.file.remote.session.Session; @@ -39,6 +42,7 @@ import org.springframework.integration.support.MessageBuilder; /** * @author Oleg Zhurakousky + * @author Gary Russell */ public class FileTransferringMessageHandlerTests { @@ -64,6 +68,32 @@ public class FileTransferringMessageHandlerTests { verify(session, times(1)).write(Mockito.any(InputStream.class), Mockito.anyString()); } + @SuppressWarnings("unchecked") + @Test + public void testTemporaryRemoteDir() throws Exception{ + SessionFactory sf = mock(SessionFactory.class); + Session session = mock(Session.class); + + final AtomicReference temporaryPath = new AtomicReference(); + final AtomicReference finalPath = new AtomicReference(); + when(sf.getSession()).thenReturn(session); + doAnswer(new Answer() { + public Object answer(InvocationOnMock invocation) throws Throwable { + temporaryPath.set((String) invocation.getArguments()[0]); + finalPath.set((String) invocation.getArguments()[1]); + return null; + } + }).when(session).rename(Mockito.anyString(), Mockito.anyString()); + FileTransferringMessageHandler handler = new FileTransferringMessageHandler(sf); + handler.setRemoteDirectoryExpression(new LiteralExpression("foo")); + handler.setTemporaryRemoteDirectoryExpression(new LiteralExpression("bar")); + handler.afterPropertiesSet(); + handler.handleMessage(new GenericMessage("hello")); + verify(session, times(1)).write(Mockito.any(InputStream.class), Mockito.anyString()); + assertEquals("bar", temporaryPath.get().substring(0, 3)); + assertEquals("foo", finalPath.get().substring(0, 3)); + } + @SuppressWarnings("unchecked") @Test public void testRemoteDirWithNull() throws Exception{