From f031867e1677c1bdb72a961f9138e98a05ba3923 Mon Sep 17 00:00:00 2001 From: Gary Russell Date: Tue, 16 Jul 2013 09:06:53 -0400 Subject: [PATCH] INT-3089 Fix FileExistsMode.IGNORE When writing files with this mode setting, the FWMH detected that the final file already exists, but did not detect that the temporary (default ....writing) file exists. This could happen if two adapters were simulataneously processing the same file. Also, while a comment in the setter said an empty temp file suffix is allowed, it did not work because the file rename is performed unconditionally, fails, and throws an exception. Add a check for the temporary file (if the suffix is not ""). Don't rename if the file was written in-place (no temp file suffix). Add test cases for the FileExistsMode.IGNORE for both the final and temporary files. Add a test where the temporary file suffix is "", a ...writing file exists, the final file does not exist and so is created by the handler. The ...writing file is untouched. --- .../file/FileWritingMessageHandler.java | 9 +- .../file/FileWritingMessageHandlerTests.java | 96 ++++++++++++++++--- 2 files changed, 90 insertions(+), 15 deletions(-) diff --git a/spring-integration-file/src/main/java/org/springframework/integration/file/FileWritingMessageHandler.java b/spring-integration-file/src/main/java/org/springframework/integration/file/FileWritingMessageHandler.java index c163d228d4..eef25fd62d 100644 --- a/spring-integration-file/src/main/java/org/springframework/integration/file/FileWritingMessageHandler.java +++ b/spring-integration-file/src/main/java/org/springframework/integration/file/FileWritingMessageHandler.java @@ -25,6 +25,7 @@ import java.nio.charset.Charset; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; + import org.springframework.beans.factory.BeanFactory; import org.springframework.context.expression.BeanFactoryResolver; import org.springframework.context.expression.MapAccessor; @@ -43,6 +44,7 @@ import org.springframework.integration.util.PassThruLockRegistry; import org.springframework.integration.util.WhileLockedProcessor; import org.springframework.util.Assert; import org.springframework.util.FileCopyUtils; +import org.springframework.util.StringUtils; /** * A {@link MessageHandler} implementation that writes the Message payload to a @@ -68,6 +70,7 @@ import org.springframework.util.FileCopyUtils; * @author Oleg Zhurakousky * @author Artem Bilan * @author Gunnar Hillert + * @author Gary Russell */ public class FileWritingMessageHandler extends AbstractReplyProducingMessageHandler { @@ -268,7 +271,9 @@ public class FileWritingMessageHandler extends AbstractReplyProducingMessageHand "The destination file already exists at '" + resultFile.getAbsolutePath() + "'."); } - final boolean ignore = FileExistsMode.IGNORE.equals(this.fileExistsMode) && resultFile.exists(); + final boolean ignore = FileExistsMode.IGNORE.equals(this.fileExistsMode) && + (resultFile.exists() || + (StringUtils.hasText(this.temporaryFileSuffix) && tempFile.exists())); if (!ignore) { @@ -413,7 +418,7 @@ public class FileWritingMessageHandler extends AbstractReplyProducingMessageHand } private void cleanUpAfterCopy(File fileToWriteTo, File resultFile, File originalFile) throws IOException{ - if (!FileExistsMode.APPEND.equals(this.fileExistsMode)) { + if (!FileExistsMode.APPEND.equals(this.fileExistsMode) && StringUtils.hasText(this.temporaryFileSuffix)) { this.renameTo(fileToWriteTo, resultFile); } diff --git a/spring-integration-file/src/test/java/org/springframework/integration/file/FileWritingMessageHandlerTests.java b/spring-integration-file/src/test/java/org/springframework/integration/file/FileWritingMessageHandlerTests.java index e52d9f71b2..f132515da4 100644 --- a/spring-integration-file/src/test/java/org/springframework/integration/file/FileWritingMessageHandlerTests.java +++ b/spring-integration-file/src/test/java/org/springframework/integration/file/FileWritingMessageHandlerTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2009 the original author or authors. + * Copyright 2002-2013 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,28 +16,39 @@ package org.springframework.integration.file; -import org.junit.*; -import org.junit.rules.TemporaryFolder; -import org.springframework.integration.Message; -import org.springframework.integration.MessageHandlingException; -import org.springframework.integration.channel.NullChannel; -import org.springframework.integration.channel.QueueChannel; -import org.springframework.integration.message.GenericMessage; -import org.springframework.integration.support.MessageBuilder; -import org.springframework.util.FileCopyUtils; +import static org.hamcrest.CoreMatchers.instanceOf; +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.CoreMatchers.notNullValue; +import static org.hamcrest.CoreMatchers.nullValue; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotSame; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; import java.io.File; import java.io.FileOutputStream; import java.io.IOException; import java.io.UnsupportedEncodingException; -import static org.hamcrest.CoreMatchers.*; -import static org.junit.Assert.*; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; + +import org.springframework.integration.Message; +import org.springframework.integration.MessageHandlingException; +import org.springframework.integration.channel.NullChannel; +import org.springframework.integration.channel.QueueChannel; +import org.springframework.integration.file.support.FileExistsMode; +import org.springframework.integration.message.GenericMessage; +import org.springframework.integration.support.MessageBuilder; +import org.springframework.util.FileCopyUtils; /** * @author Mark Fisher * @author Iwein Fuld * @author Alex Peters + * @author Gary Russell */ public class FileWritingMessageHandlerTests { @@ -50,6 +61,7 @@ public class FileWritingMessageHandlerTests { @Rule public TemporaryFolder temp = new TemporaryFolder() { + @Override public void create() throws IOException { super.create(); outputDirectory = temp.newFolder("outputDirectory"); @@ -219,14 +231,72 @@ public class FileWritingMessageHandlerTests { assertThat(result.getName(), is(anyFilename)); } + @Test + public void existingFileIgnored() throws Exception { + Message message = MessageBuilder.withPayload(SAMPLE_CONTENT).build(); + QueueChannel output = new QueueChannel(); + File outFile = temp.newFile("/outputDirectory/" + message.getHeaders().getId().toString() + ".msg"); + FileCopyUtils.copy("foo".getBytes(), new FileOutputStream(outFile)); + handler.setCharset(DEFAULT_ENCODING); + handler.setOutputChannel(output); + handler.setFileExistsMode(FileExistsMode.IGNORE); + handler.handleMessage(message); + Message result = output.receive(0); + assertFileContentIs(result, "foo"); + } + + @Test + public void existingWritingFileIgnored() throws Exception { + Message message = MessageBuilder.withPayload(SAMPLE_CONTENT).build(); + QueueChannel output = new QueueChannel(); + File outFile = temp.newFile("/outputDirectory/" + message.getHeaders().getId().toString() + ".msg.writing"); + FileCopyUtils.copy("foo".getBytes(), new FileOutputStream(outFile)); + handler.setCharset(DEFAULT_ENCODING); + handler.setOutputChannel(output); + handler.setFileExistsMode(FileExistsMode.IGNORE); + handler.handleMessage(message); + Message result = output.receive(0); + File destFile = (File) result.getPayload(); + assertNotSame(destFile, sourceFile); + assertThat(destFile.exists(), is(false)); + assertThat(outFile.exists(), is(true)); + } + + @Test + public void existingWritingFileNotIgnoredIfEmptySuffix() throws Exception { + Message message = MessageBuilder.withPayload(SAMPLE_CONTENT).build(); + QueueChannel output = new QueueChannel(); + File outFile = temp.newFile("/outputDirectory/" + message.getHeaders().getId().toString() + ".msg.writing"); + FileCopyUtils.copy("foo".getBytes(), new FileOutputStream(outFile)); + handler.setCharset(DEFAULT_ENCODING); + handler.setOutputChannel(output); + handler.setFileExistsMode(FileExistsMode.IGNORE); + handler.setTemporaryFileSuffix(""); + handler.handleMessage(message); + Message result = output.receive(0); + File destFile = (File) result.getPayload(); + assertNotSame(destFile, sourceFile); + assertFileContentIsMatching(result); + assertThat(outFile.exists(), is(true)); + assertFileContentIs(outFile, "foo"); + } + void assertFileContentIsMatching(Message result) throws IOException, UnsupportedEncodingException { + assertFileContentIs(result, SAMPLE_CONTENT); + } + + void assertFileContentIs(Message result, String expected) throws IOException, UnsupportedEncodingException { assertThat(result, is(notNullValue())); assertThat(result.getPayload(), is(instanceOf(File.class))); File destFile = (File) result.getPayload(); + assertFileContentIs(destFile, expected); + } + + void assertFileContentIs(File destFile, String expected) throws IOException, UnsupportedEncodingException { assertNotSame(destFile, sourceFile); assertThat(destFile.exists(), is(true)); byte[] destFileContent = FileCopyUtils.copyToByteArray(destFile); - assertThat(new String(destFileContent, DEFAULT_ENCODING), is(SAMPLE_CONTENT)); + assertThat(new String(destFileContent, DEFAULT_ENCODING), is(expected)); } }