From 538d7b1abb28a9d9045c9301b5e10b18c0959eb2 Mon Sep 17 00:00:00 2001 From: jpraet Date: Fri, 1 Mar 2013 20:52:46 +0100 Subject: [PATCH 1/5] BATCH-1957: Add StaxEventItemWriter deleteIfEmpty property --- .../batch/item/xml/StaxEventItemWriter.java | 21 +++++ .../item/xml/StaxEventItemWriterTests.java | 87 +++++++++++++++++++ 2 files changed, 108 insertions(+) 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 4e8e351f3..fd3bb5c1c 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 @@ -142,6 +142,8 @@ ResourceAwareItemWriterItemStream, InitializingBean { private boolean forceSync; + private boolean shouldDeleteIfEmpty = false; + public StaxEventItemWriter() { setExecutionContextName(ClassUtils.getShortName(StaxEventItemWriter.class)); } @@ -203,6 +205,16 @@ ResourceAwareItemWriterItemStream, InitializingBean { this.forceSync = forceSync; } + /** + * Flag to indicate that the target file should be deleted if no items have + * been written (other than header and footer) on close. Defaults to false. + * + * @param shouldDeleteIfEmpty the flag value to set + */ + public void setShouldDeleteIfEmpty(boolean shouldDeleteIfEmpty) { + this.shouldDeleteIfEmpty = shouldDeleteIfEmpty; + } + /** * Get used encoding. * @@ -354,6 +366,7 @@ ResourceAwareItemWriterItemStream, InitializingBean { // otherwise start from beginning if (executionContext.containsKey(getExecutionContextKey(RESTART_DATA_NAME))) { startAtPosition = executionContext.getLong(getExecutionContextKey(RESTART_DATA_NAME)); + currentRecordCount = executionContext.getLong(getExecutionContextKey(WRITE_STATISTICS_NAME)); restarted = true; } @@ -656,6 +669,14 @@ ResourceAwareItemWriterItemStream, InitializingBean { } } } + if (currentRecordCount == 0 && shouldDeleteIfEmpty) { + try { + resource.getFile().delete(); + } + catch (IOException e) { + throw new ItemStreamException("Failed to delete empty file on close", e); + } + } } } diff --git a/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/xml/StaxEventItemWriterTests.java b/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/xml/StaxEventItemWriterTests.java index 474935745..7a373ee92 100644 --- a/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/xml/StaxEventItemWriterTests.java +++ b/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/xml/StaxEventItemWriterTests.java @@ -352,6 +352,93 @@ public class StaxEventItemWriterTests { assertEquals("Output resource must exist", e.getMessage()); } } + + /** + * Resource is not deleted when items have been written and shouldDeleteIfEmpty flag is set. + */ + @Test + public void testDeleteIfEmptyRecordsWritten() throws Exception { + writer.setShouldDeleteIfEmpty(true); + writer.open(executionContext); + writer.write(items); + writer.close(); + String content = getOutputFileContent(); + assertTrue("Wrong content: " + content, content.contains(TEST_STRING)); + } + + /** + * Resource is not deleted when items have been written and shouldDeleteIfEmpty flag is set. + */ + @Test + public void testDeleteIfEmptyNoRecordsWritten() throws Exception { + writer.setShouldDeleteIfEmpty(true); + writer.open(executionContext); + writer.close(); + assertFalse("file should be deleted" + resource, resource.getFile().exists()); + } + + /** + * Resource is not deleted when items have been written and shouldDeleteIfEmpty flag is set. + */ + @Test + public void testDeleteIfEmptyNoRecordsWrittenHeaderAndFooter() throws Exception { + writer.setShouldDeleteIfEmpty(true); + writer.setHeaderCallback(new StaxWriterCallback() { + + @Override + public void write(XMLEventWriter writer) throws IOException { + XMLEventFactory factory = XMLEventFactory.newInstance(); + try { + writer.add(factory.createStartElement("", "", "header")); + writer.add(factory.createEndElement("", "", "header")); + } + catch (XMLStreamException e) { + throw new RuntimeException(e); + } + + } + + }); + writer.setFooterCallback(new StaxWriterCallback() { + + @Override + public void write(XMLEventWriter writer) throws IOException { + XMLEventFactory factory = XMLEventFactory.newInstance(); + try { + writer.add(factory.createStartElement("", "", "footer")); + writer.add(factory.createEndElement("", "", "footer")); + } + catch (XMLStreamException e) { + throw new RuntimeException(e); + } + + } + + }); + writer.open(executionContext); + writer.close(); + assertFalse("file should be deleted" + resource, resource.getFile().exists()); + } + + /** + * Resource is not deleted when items have been written and shouldDeleteIfEmpty flag is set. + */ + @Test + public void testDeleteIfEmptyRecordsWrittenRestart() throws Exception { + writer.setShouldDeleteIfEmpty(true); + writer.open(executionContext); + writer.write(items); + writer.update(executionContext); + writer.close(); + + writer = createItemWriter(); + writer.setShouldDeleteIfEmpty(true); + writer.open(executionContext); + writer.close(); + String content = getOutputFileContent(); + assertTrue("Wrong content: " + content, content.contains(TEST_STRING)); + } + /** * Item is written to the output file with namespace. From 3f2aa18460ec64a875fa527992107142e6494cf6 Mon Sep 17 00:00:00 2001 From: jpraet Date: Tue, 5 Mar 2013 19:24:13 +0100 Subject: [PATCH 2/5] add unit test test showing bug in FlatFileItemWriter: output file is deleted on restart if shouldDeleteIfEmpty property is set and no new records have been written --- .../batch/item/file/FlatFileItemWriterTests.java | 13 +++++++++++++ 1 file changed, 13 insertions(+) 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 875587cba..e75d55bd5 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 @@ -602,6 +602,19 @@ public class FlatFileItemWriterTests { writer.write(Collections.singletonList("test2")); assertEquals("test2", readLine()); } + + @Test + public void testDeleteOnExitNoRecordsWrittenAfterRestart() throws Exception { + writer.setShouldDeleteIfEmpty(true); + writer.open(executionContext); + writer.write(Collections.singletonList("test2")); + writer.update(executionContext); + writer.close(); + assertTrue(outputFile.exists()); + writer.open(executionContext); + writer.close(); + assertTrue(outputFile.exists()); + } @Test public void testWriteHeaderAfterRestartOnFirstChunk() throws Exception { From 44d640990d4b6d7792776dcd5ed77caf36805799 Mon Sep 17 00:00:00 2001 From: jpraet Date: Thu, 7 Mar 2013 19:36:31 +0100 Subject: [PATCH 3/5] add additional test case that shows a bug in the current StaxEventItemWriter.shouldDeleteIfEmpty: when the writer restarts from a previous execution where the output file was deleted, this fails because FileUtils.setUpOutputFile expects the file to be there on restart --- .../item/xml/StaxEventItemWriterTests.java | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/xml/StaxEventItemWriterTests.java b/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/xml/StaxEventItemWriterTests.java index 7a373ee92..2901b5deb 100644 --- a/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/xml/StaxEventItemWriterTests.java +++ b/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/xml/StaxEventItemWriterTests.java @@ -431,6 +431,24 @@ public class StaxEventItemWriterTests { writer.update(executionContext); writer.close(); + writer = createItemWriter(); + writer.setShouldDeleteIfEmpty(true); + writer.open(executionContext); + writer.close(); + String content = getOutputFileContent(); + assertTrue("Wrong content: " + content, content.contains(TEST_STRING)); + } + + /** + * Test that the writer can restart if the previous execution deleted empty file. + */ + @Test + public void testDeleteIfEmptyRestart() throws Exception { + writer.setShouldDeleteIfEmpty(true); + writer.open(executionContext); + writer.update(executionContext); + writer.close(); + assertFalse(resource.getFile().exists()); writer = createItemWriter(); writer.setShouldDeleteIfEmpty(true); writer.open(executionContext); From c65ec9747423c8df338baec1edc964b9d1eb8b94 Mon Sep 17 00:00:00 2001 From: jpraet Date: Tue, 19 Mar 2013 22:23:33 +0100 Subject: [PATCH 4/5] - restore WRITE_STATISTICS_NAME from execution context on restart - don't assume output file is always present on restart (could be deleted by previous execution if shouldDeleteIfEmpty flag was set) --- .../batch/item/file/FlatFileItemWriter.java | 10 +++- .../batch/item/xml/StaxEventItemWriter.java | 7 +++ .../item/file/FlatFileItemWriterTests.java | 33 ++++++++++-- .../item/xml/StaxEventItemWriterTests.java | 54 ++++++++++++++++++- 4 files changed, 97 insertions(+), 7 deletions(-) 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 6748a03b4..62fd65f5e 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 @@ -469,7 +469,14 @@ InitializingBean { */ public void restoreFrom(ExecutionContext executionContext) { lastMarkedByteOffsetPosition = executionContext.getLong(getExecutionContextKey(RESTART_DATA_NAME)); - restarted = true; + linesWritten = executionContext.getLong(getExecutionContextKey(WRITTEN_STATISTICS_NAME)); + if (shouldDeleteIfEmpty && linesWritten == 0) { + // previous execution deleted the output file because no items were written + restarted = false; + lastMarkedByteOffsetPosition = 0; + } else { + restarted = true; + } } /** @@ -585,7 +592,6 @@ InitializingBean { } initialized = true; - linesWritten = 0; } public boolean isInitialized() { 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 fd3bb5c1c..c81c36250 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 @@ -368,6 +368,13 @@ ResourceAwareItemWriterItemStream, InitializingBean { startAtPosition = executionContext.getLong(getExecutionContextKey(RESTART_DATA_NAME)); currentRecordCount = executionContext.getLong(getExecutionContextKey(WRITE_STATISTICS_NAME)); restarted = true; + if (shouldDeleteIfEmpty && currentRecordCount == 0) { + // previous execution deleted the output file because no items were written + restarted = false; + startAtPosition = 0; + } else { + restarted = true; + } } open(startAtPosition, restarted); 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 e75d55bd5..22d125aba 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 @@ -323,8 +323,8 @@ public class FlatFileItemWriterTests { assertEquals("footer", readLine()); - // 3 lines were written to the file after restart - assertEquals(3, executionContext.getLong(ClassUtils.getShortName(FlatFileItemWriter.class) + ".written")); + // 8 lines were written to the file in total + assertEquals(8, executionContext.getLong(ClassUtils.getShortName(FlatFileItemWriter.class) + ".written")); } @@ -425,8 +425,8 @@ public class FlatFileItemWriterTests { assertEquals("footer", readLine()); - // 3 lines were written to the file after restart - assertEquals(3, executionContext.getLong(ClassUtils.getShortName(FlatFileItemWriter.class) + ".written")); + // 8 lines were written to the file in total + assertEquals(8, executionContext.getLong(ClassUtils.getShortName(FlatFileItemWriter.class) + ".written")); } @@ -595,6 +595,7 @@ public class FlatFileItemWriterTests { public void testDeleteOnExitReopen() throws Exception { writer.setShouldDeleteIfEmpty(true); writer.open(executionContext); + writer.update(executionContext); assertTrue(outputFile.exists()); writer.close(); assertFalse(outputFile.exists()); @@ -603,6 +604,30 @@ public class FlatFileItemWriterTests { assertEquals("test2", readLine()); } + @Test + public void testWriteHeaderAndDeleteOnExitReopen() throws Exception { + writer.setHeaderCallback(new FlatFileHeaderCallback() { + + @Override + public void writeHeader(Writer writer) throws IOException { + writer.write("a\nb"); + } + + }); + writer.setShouldDeleteIfEmpty(true); + writer.open(executionContext); + writer.update(executionContext); + assertTrue(outputFile.exists()); + writer.close(); + assertFalse(outputFile.exists()); + + writer.open(executionContext); + writer.write(Collections.singletonList("test2")); + assertEquals("a", readLine()); + assertEquals("b", readLine()); + assertEquals("test2", readLine()); + } + @Test public void testDeleteOnExitNoRecordsWrittenAfterRestart() throws Exception { writer.setShouldDeleteIfEmpty(true); diff --git a/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/xml/StaxEventItemWriterTests.java b/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/xml/StaxEventItemWriterTests.java index 2901b5deb..7515d2f2e 100644 --- a/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/xml/StaxEventItemWriterTests.java +++ b/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/xml/StaxEventItemWriterTests.java @@ -443,7 +443,7 @@ public class StaxEventItemWriterTests { * Test that the writer can restart if the previous execution deleted empty file. */ @Test - public void testDeleteIfEmptyRestart() throws Exception { + public void testDeleteIfEmptyRestartAfterDelete() throws Exception { writer.setShouldDeleteIfEmpty(true); writer.open(executionContext); writer.update(executionContext); @@ -452,11 +452,63 @@ public class StaxEventItemWriterTests { writer = createItemWriter(); writer.setShouldDeleteIfEmpty(true); writer.open(executionContext); + writer.write(items); + writer.update(executionContext); writer.close(); String content = getOutputFileContent(); assertTrue("Wrong content: " + content, content.contains(TEST_STRING)); } + /** + * Resource is not deleted when items have been written and shouldDeleteIfEmpty flag is set (restart after delete). + */ + @Test + public void testDeleteIfEmptyNoRecordsWrittenHeaderAndFooterRestartAfterDelete() throws Exception { + writer.setShouldDeleteIfEmpty(true); + writer.setHeaderCallback(new StaxWriterCallback() { + + @Override + public void write(XMLEventWriter writer) throws IOException { + XMLEventFactory factory = XMLEventFactory.newInstance(); + try { + writer.add(factory.createStartElement("", "", "header")); + writer.add(factory.createEndElement("", "", "header")); + } + catch (XMLStreamException e) { + throw new RuntimeException(e); + } + + } + + }); + writer.setFooterCallback(new StaxWriterCallback() { + + @Override + public void write(XMLEventWriter writer) throws IOException { + XMLEventFactory factory = XMLEventFactory.newInstance(); + try { + writer.add(factory.createStartElement("", "", "footer")); + writer.add(factory.createEndElement("", "", "footer")); + } + catch (XMLStreamException e) { + throw new RuntimeException(e); + } + + } + + }); + writer.open(executionContext); + writer.update(executionContext); + writer.close(); + assertFalse("file should be deleted" + resource, resource.getFile().exists()); + writer.open(executionContext); + writer.write(items); + writer.update(executionContext); + writer.close(); + String content = getOutputFileContent(); + assertTrue("Wrong content: " + content, content.contains(TEST_STRING)); + } + /** * Item is written to the output file with namespace. From 00f1d311276cb1dd31b56868d2085f2e407821b0 Mon Sep 17 00:00:00 2001 From: Michael Minella Date: Wed, 20 Mar 2013 10:36:06 -0500 Subject: [PATCH 5/5] BATCH-1957: Updated comments --- .../item/xml/StaxEventItemWriterTests.java | 44 +++++++++---------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/xml/StaxEventItemWriterTests.java b/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/xml/StaxEventItemWriterTests.java index 7515d2f2e..5b10370c1 100644 --- a/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/xml/StaxEventItemWriterTests.java +++ b/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/xml/StaxEventItemWriterTests.java @@ -352,7 +352,7 @@ public class StaxEventItemWriterTests { assertEquals("Output resource must exist", e.getMessage()); } } - + /** * Resource is not deleted when items have been written and shouldDeleteIfEmpty flag is set. */ @@ -364,10 +364,10 @@ public class StaxEventItemWriterTests { writer.close(); String content = getOutputFileContent(); assertTrue("Wrong content: " + content, content.contains(TEST_STRING)); - } + } /** - * Resource is not deleted when items have been written and shouldDeleteIfEmpty flag is set. + * Resource is deleted when no items have been written and shouldDeleteIfEmpty flag is set. */ @Test public void testDeleteIfEmptyNoRecordsWritten() throws Exception { @@ -376,16 +376,16 @@ public class StaxEventItemWriterTests { writer.close(); assertFalse("file should be deleted" + resource, resource.getFile().exists()); } - + /** - * Resource is not deleted when items have been written and shouldDeleteIfEmpty flag is set. + * Resource is deleted when items have not been written and shouldDeleteIfEmpty flag is set. */ @Test public void testDeleteIfEmptyNoRecordsWrittenHeaderAndFooter() throws Exception { writer.setShouldDeleteIfEmpty(true); writer.setHeaderCallback(new StaxWriterCallback() { - @Override + @Override public void write(XMLEventWriter writer) throws IOException { XMLEventFactory factory = XMLEventFactory.newInstance(); try { @@ -401,7 +401,7 @@ public class StaxEventItemWriterTests { }); writer.setFooterCallback(new StaxWriterCallback() { - @Override + @Override public void write(XMLEventWriter writer) throws IOException { XMLEventFactory factory = XMLEventFactory.newInstance(); try { @@ -414,12 +414,12 @@ public class StaxEventItemWriterTests { } - }); + }); writer.open(executionContext); writer.close(); assertFalse("file should be deleted" + resource, resource.getFile().exists()); - } - + } + /** * Resource is not deleted when items have been written and shouldDeleteIfEmpty flag is set. */ @@ -430,7 +430,7 @@ public class StaxEventItemWriterTests { writer.write(items); writer.update(executionContext); writer.close(); - + writer = createItemWriter(); writer.setShouldDeleteIfEmpty(true); writer.open(executionContext); @@ -438,7 +438,7 @@ public class StaxEventItemWriterTests { String content = getOutputFileContent(); assertTrue("Wrong content: " + content, content.contains(TEST_STRING)); } - + /** * Test that the writer can restart if the previous execution deleted empty file. */ @@ -453,12 +453,12 @@ public class StaxEventItemWriterTests { writer.setShouldDeleteIfEmpty(true); writer.open(executionContext); writer.write(items); - writer.update(executionContext); + writer.update(executionContext); writer.close(); String content = getOutputFileContent(); assertTrue("Wrong content: " + content, content.contains(TEST_STRING)); - } - + } + /** * Resource is not deleted when items have been written and shouldDeleteIfEmpty flag is set (restart after delete). */ @@ -467,7 +467,7 @@ public class StaxEventItemWriterTests { writer.setShouldDeleteIfEmpty(true); writer.setHeaderCallback(new StaxWriterCallback() { - @Override + @Override public void write(XMLEventWriter writer) throws IOException { XMLEventFactory factory = XMLEventFactory.newInstance(); try { @@ -483,7 +483,7 @@ public class StaxEventItemWriterTests { }); writer.setFooterCallback(new StaxWriterCallback() { - @Override + @Override public void write(XMLEventWriter writer) throws IOException { XMLEventFactory factory = XMLEventFactory.newInstance(); try { @@ -496,19 +496,19 @@ public class StaxEventItemWriterTests { } - }); + }); writer.open(executionContext); writer.update(executionContext); writer.close(); assertFalse("file should be deleted" + resource, resource.getFile().exists()); writer.open(executionContext); writer.write(items); - writer.update(executionContext); + writer.update(executionContext); writer.close(); String content = getOutputFileContent(); - assertTrue("Wrong content: " + content, content.contains(TEST_STRING)); - } - + assertTrue("Wrong content: " + content, content.contains(TEST_STRING)); + } + /** * Item is written to the output file with namespace.