diff --git a/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/file/FlatFileItemWriter.java b/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/file/FlatFileItemWriter.java index 492c67118..f16fff382 100644 --- a/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/file/FlatFileItemWriter.java +++ b/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/file/FlatFileItemWriter.java @@ -524,7 +524,7 @@ public class FlatFileItemWriter extends ExecutionContextUserSupport implement private void initializeBufferedWriter() throws IOException { File file = resource.getFile(); - FileUtils.setUpOutputFile(file, restarted || append, shouldDeleteIfExists); + FileUtils.setUpOutputFile(file, restarted, append, shouldDeleteIfExists); os = new FileOutputStream(file.getAbsolutePath(), true); fileChannel = os.getChannel(); diff --git a/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/util/FileUtils.java b/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/util/FileUtils.java index 8b5df8970..4a16dd1dd 100644 --- a/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/util/FileUtils.java +++ b/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/util/FileUtils.java @@ -34,43 +34,51 @@ public final class FileUtils { } /** - * Set up output file for batch processing. This method implements common - * logic for handling output files when starting or restarting file I/O. - * When starting output file processing, creates/overwrites new file. When + * Set up output file for batch processing. This method implements common logic for handling output files when + * starting or restarting file I/O. When starting output file processing, creates/overwrites new file. When * restarting output file processing, checks whether file is writable. * * @param file file to be set up - * @param restarted true signals that we are restarting output file - * processing - * @param overwriteOutputFile If set to true, output file will be - * overwritten (this flag is ignored when processing is restart) + * @param restarted true signals that we are restarting output file processing + * @param append true signals input file may already exist (but doesn't have to) + * @param overwriteOutputFile If set to true, output file will be overwritten (this flag is ignored when processing + * is restart) * * @throws IllegalArgumentException when file is null - * @throws ItemStreamException when starting output file processing, file - * exists and flag "overwriteOutputFile" is set to false - * @throws ItemStreamException when unable to create file or file is not - * writable + * @throws ItemStreamException when starting output file processing, file exists and flag "overwriteOutputFile" is + * set to false + * @throws ItemStreamException when unable to create file or file is not writable */ - public static void setUpOutputFile(File file, boolean restarted, boolean overwriteOutputFile) { + public static void setUpOutputFile(File file, boolean restarted, boolean append, boolean overwriteOutputFile) { Assert.notNull(file); try { if (!restarted) { - if (file.exists()) { - if (!overwriteOutputFile) { - throw new ItemStreamException("File already exists: [" + file.getAbsolutePath() + "]"); + if (!append) { + if (file.exists()) { + if (!overwriteOutputFile) { + throw new ItemStreamException("File already exists: [" + file.getAbsolutePath() + "]"); + } + if (!file.delete()) { + throw new IOException("Could not delete file: " + file); + } } - if (!file.delete()) { - throw new IOException("Could not delete file: " + file); - } - } - if (file.getParent() != null) { - new File(file.getParent()).mkdirs(); + if (file.getParent() != null) { + new File(file.getParent()).mkdirs(); + } + if (!createNewFile(file)) { + throw new ItemStreamException("Output file was not created: [" + file.getAbsolutePath() + "]"); + } } - if (!createNewFile(file)) { - throw new ItemStreamException("Output file was not created: [" + file.getAbsolutePath() + "]"); + else { + if (!file.exists()) { + if (!createNewFile(file)) { + throw new ItemStreamException("Output file was not created: [" + file.getAbsolutePath() + + "]"); + } + } } } } @@ -83,6 +91,13 @@ public final class FileUtils { } } + /** + * @deprecated use the version with explicit append parameter instead. Here append=false is assumed. + */ + public static void setUpOutputFile(File file, boolean restarted, boolean overwriteOutputFile) { + setUpOutputFile(file, restarted, false, overwriteOutputFile); + } + /** * Create a new file if it doesn't already exist. * diff --git a/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/xml/StaxEventItemWriter.java b/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/xml/StaxEventItemWriter.java index 1390222da..1285dccb3 100644 --- a/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/xml/StaxEventItemWriter.java +++ b/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/xml/StaxEventItemWriter.java @@ -361,7 +361,7 @@ public class StaxEventItemWriter extends ExecutionContextUserSupport implemen try { file = resource.getFile(); - FileUtils.setUpOutputFile(file, restarted, overwriteOutput); + FileUtils.setUpOutputFile(file, restarted, false, overwriteOutput); Assert.state(resource.exists(), "Output resource must exist"); os = new FileOutputStream(file, true); channel = os.getChannel(); diff --git a/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/file/FlatFileItemWriterTests.java b/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/file/FlatFileItemWriterTests.java index f6450e078..dbc3b238d 100644 --- a/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/file/FlatFileItemWriterTests.java +++ b/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/file/FlatFileItemWriterTests.java @@ -44,6 +44,7 @@ import org.springframework.batch.item.file.transform.LineAggregator; import org.springframework.batch.item.file.transform.PassThroughLineAggregator; import org.springframework.batch.support.transaction.ResourcelessTransactionManager; import org.springframework.core.io.FileSystemResource; +import org.springframework.core.io.Resource; import org.springframework.transaction.PlatformTransactionManager; import org.springframework.transaction.TransactionStatus; import org.springframework.transaction.support.TransactionCallback; @@ -51,9 +52,8 @@ import org.springframework.transaction.support.TransactionTemplate; import org.springframework.util.ClassUtils; /** - * Tests of regular usage for {@link FlatFileItemWriter} Exception cases will be - * in separate TestCase classes with different setUp and - * tearDown methods + * Tests of regular usage for {@link FlatFileItemWriter} Exception cases will be in separate TestCase classes with + * different setUp and tearDown methods * * @author Robert Kasanicky * @author Dave Syer @@ -76,8 +76,7 @@ public class FlatFileItemWriterTests { private ExecutionContext executionContext; /** - * Create temporary output file, define mock behaviour, set dependencies and - * initialize the object under test + * Create temporary output file, define mock behaviour, set dependencies and initialize the object under test */ @Before public void setUp() throws Exception { @@ -105,9 +104,8 @@ public class FlatFileItemWriterTests { } /* - * Read a line from the output file, if the reader has not been created, - * recreate. This method is only necessary because running the tests in a - * UNIX environment locks the file if it's open for writing. + * Read a line from the output file, if the reader has not been created, recreate. This method is only necessary + * because running the tests in a UNIX environment locks the file if it's open for writing. */ private String readLine() throws IOException { @@ -293,7 +291,7 @@ public class FlatFileItemWriterTests { private void writeStringTransactionCheck(final String expectedInTransaction) { PlatformTransactionManager transactionManager = new ResourcelessTransactionManager(); - + writer.open(executionContext); new TransactionTemplate(transactionManager).execute(new TransactionCallback() { public Object doInTransaction(TransactionStatus status) { @@ -304,7 +302,7 @@ public class FlatFileItemWriterTests { catch (Exception e) { throw new UnexpectedInputException("Could not write data", e); } - + return null; } }); @@ -325,7 +323,7 @@ public class FlatFileItemWriterTests { writer.open(executionContext); PlatformTransactionManager transactionManager = new ResourcelessTransactionManager(); - + new TransactionTemplate(transactionManager).execute(new TransactionCallback() { public Object doInTransaction(TransactionStatus status) { try { @@ -628,4 +626,26 @@ public class FlatFileItemWriterTests { // nothing was written to output assertNull(readLine()); } + + @Test + /** + * If append=true a new output file should still be created on the first run (not restart). + */ + public void testAppendToNotYetExistingFile() throws Exception { + Resource toBeCreated = new FileSystemResource("target/FlatFileItemWriterTests.out"); + + outputFile = toBeCreated.getFile(); //enable easy content reading and auto-delete the file + + assertFalse("output file does not exist yet", toBeCreated.exists()); + writer.setResource(toBeCreated); + writer.setAppendAllowed(true); + writer.afterPropertiesSet(); + + writer.open(executionContext); + assertTrue("output file was created", toBeCreated.exists()); + + writer.write(Collections.singletonList("test1")); + writer.close(); + assertEquals("test1", readLine()); + } } diff --git a/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/util/FileUtilsTests.java b/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/util/FileUtilsTests.java index 0eaed6f11..8d10599e2 100644 --- a/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/util/FileUtilsTests.java +++ b/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/util/FileUtilsTests.java @@ -28,11 +28,11 @@ public class FileUtilsTests { */ @Test public void testNoRestart() throws Exception { - FileUtils.setUpOutputFile(file, false, false); + FileUtils.setUpOutputFile(file, false, false, false); assertTrue(file.exists()); try { - FileUtils.setUpOutputFile(file, false, false); + FileUtils.setUpOutputFile(file, false, false, false); fail(); } catch (Exception e) { @@ -42,7 +42,7 @@ public class FileUtilsTests { file.delete(); Assert.state(!file.exists()); - FileUtils.setUpOutputFile(file, false, true); + FileUtils.setUpOutputFile(file, false, false, true); assertTrue(file.exists()); BufferedWriter writer = new BufferedWriter(new FileWriter(file)); @@ -51,7 +51,7 @@ public class FileUtilsTests { long size = file.length(); Assert.state(size > 0); - FileUtils.setUpOutputFile(file, false, true); + FileUtils.setUpOutputFile(file, false, false, true); long newSize = file.length(); assertTrue(size != newSize); @@ -65,7 +65,7 @@ public class FileUtilsTests { @Test public void testRestart() throws Exception { try { - FileUtils.setUpOutputFile(file, true, false); + FileUtils.setUpOutputFile(file, true, false, false); fail(); } catch (ItemStreamException e) { @@ -73,7 +73,7 @@ public class FileUtilsTests { } try { - FileUtils.setUpOutputFile(file, true, true); + FileUtils.setUpOutputFile(file, true, false, true); fail(); } catch (ItemStreamException e) { @@ -84,8 +84,8 @@ public class FileUtilsTests { assertTrue(file.exists()); // with existing file there should be no trouble - FileUtils.setUpOutputFile(file, true, false); - FileUtils.setUpOutputFile(file, true, true); + FileUtils.setUpOutputFile(file, true, false, false); + FileUtils.setUpOutputFile(file, true, false, true); } /** @@ -98,7 +98,7 @@ public class FileUtilsTests { File dir2 = new File("testDirectory/testDirectory2"); try { - FileUtils.setUpOutputFile(file, false, false); + FileUtils.setUpOutputFile(file, false, false, false); assertTrue(file.exists()); assertTrue(dir1.exists()); assertTrue(dir2.exists()); @@ -119,7 +119,7 @@ public class FileUtilsTests { } }; try{ - FileUtils.setUpOutputFile(file, false, false); + FileUtils.setUpOutputFile(file, false, false, false); fail(); }catch(ItemStreamException ex){ assertTrue(ex.getCause() instanceof IOException); @@ -140,7 +140,7 @@ public class FileUtilsTests { }; try{ - FileUtils.setUpOutputFile(file, false, false); + FileUtils.setUpOutputFile(file, false, false, false); fail("Expected IOException because file doesn't exist"); }catch(ItemStreamException ex){ String message = ex.getMessage();