RESOLVED - issue BATCH-1452: Stream closed exception when combining MultiResourceItemWriter and FlatFileItemWriter

spring-batch-infrastructure/src/test/java/org/springframework/batch/item/file/AbstractMultiResourceItemWriterTests.java

@@ -4,7 +4,6 @@ import java.io.BufferedReader;
 import java.io.File;
 import java.io.FileReader;
 
-import org.junit.Before;
 import org.springframework.batch.item.ExecutionContext;
 import org.springframework.core.io.FileSystemResource;
 
@@ -22,12 +21,9 @@ public class AbstractMultiResourceItemWriterTests {
 
 	protected ResourceSuffixCreator suffixCreator = new SimpleResourceSuffixCreator();
 
-	protected ResourceAwareItemWriterItemStream<String> delegate;
-
 	protected ExecutionContext executionContext = new ExecutionContext();
 
-	@Before
-	public void setUp() throws Exception {
+	protected void setUp(ResourceAwareItemWriterItemStream<String> delegate) throws Exception {
 		file = File.createTempFile(MultiResourceItemWriterFlatFileTests.class.getSimpleName(), null);
 		tested.setResource(new FileSystemResource(file));
 		tested.setDelegate(delegate);

spring-batch-infrastructure/src/test/java/org/springframework/batch/item/file/MultiResourceItemWriterFlatFileTests.java

@@ -4,11 +4,18 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 
 import java.io.File;
+import java.io.IOException;
+import java.io.Writer;
 import java.util.Arrays;
+import java.util.List;
 
 import org.junit.Before;
 import org.junit.Test;
 import org.springframework.batch.item.file.transform.PassThroughLineAggregator;
+import org.springframework.batch.support.transaction.ResourcelessTransactionManager;
+import org.springframework.transaction.TransactionStatus;
+import org.springframework.transaction.support.TransactionCallback;
+import org.springframework.transaction.support.TransactionTemplate;
 
 /**
  * Tests for {@link MultiResourceItemWriter} delegating to
@@ -16,20 +23,42 @@ import org.springframework.batch.item.file.transform.PassThroughLineAggregator;
  */
 public class MultiResourceItemWriterFlatFileTests extends AbstractMultiResourceItemWriterTests {
 
-	@Override
+	/**
+	 * @author dsyer
+	 *
+	 */
+	private final class WriterCallback implements TransactionCallback {
+		private List<? extends String> list;
+
+		public WriterCallback(List<? extends String> list) {
+			super();
+			this.list = list;
+		}
+
+		public Object doInTransaction(TransactionStatus status) {
+			try {
+				tested.write(list);
+			}
+			catch (Exception e) {
+				throw new IllegalStateException("Unexpected");
+			}
+			return null;
+		}
+	}
+
+	private FlatFileItemWriter<String> delegate;
+
 	@Before
 	public void setUp() throws Exception {
-		delegate = new FlatFileItemWriter<String>() {
-			{
-				setLineAggregator(new PassThroughLineAggregator<String>());
-			}
-		};
-		super.setUp();
+		delegate = new FlatFileItemWriter<String>();
+		delegate.setLineAggregator(new PassThroughLineAggregator<String>());
 	}
 
 	@Test
 	public void testBasicMultiResourceWriteScenario() throws Exception {
 
+		super.setUp(delegate);
+
 		tested.write(Arrays.asList("1", "2", "3"));
 
 		File part1 = new File(file.getAbsolutePath() + suffixCreator.getSuffix(1));
@@ -50,8 +79,80 @@ public class MultiResourceItemWriterFlatFileTests extends AbstractMultiResourceI
 		assertEquals("6789", readFile(part3));
 	}
 
+	@Test
+	public void testUpdateAfterDelegateClose() throws Exception {
+
+		super.setUp(delegate);
+
+		tested.update(executionContext);
+		assertEquals(0, executionContext.getInt(tested.getKey("resource.item.count")));
+		assertEquals(1, executionContext.getInt(tested.getKey("resource.index")));
+		tested.write(Arrays.asList("1", "2", "3"));
+		tested.update(executionContext);
+		assertEquals(0, executionContext.getInt(tested.getKey("resource.item.count")));
+		assertEquals(2, executionContext.getInt(tested.getKey("resource.index")));
+
+	}
+
+	@Test
+	public void testMultiResourceWriteScenarioWithFooter() throws Exception {
+
+		delegate.setFooterCallback(new FlatFileFooterCallback() {
+			public void writeFooter(Writer writer) throws IOException {
+				writer.write("f");
+			}
+		});
+		super.setUp(delegate);
+
+		tested.write(Arrays.asList("1", "2", "3"));
+
+		File part1 = new File(file.getAbsolutePath() + suffixCreator.getSuffix(1));
+		assertTrue(part1.exists());
+
+		tested.write(Arrays.asList("4"));
+		File part2 = new File(file.getAbsolutePath() + suffixCreator.getSuffix(2));
+		assertTrue(part2.exists());
+		
+		tested.close();
+
+		assertEquals("123f", readFile(part1));
+		assertEquals("4f", readFile(part2));
+
+	}
+
+	@Test
+	public void testTransactionalMultiResourceWriteScenarioWithFooter() throws Exception {
+
+		delegate.setFooterCallback(new FlatFileFooterCallback() {
+			public void writeFooter(Writer writer) throws IOException {
+				writer.write("f");
+			}
+		});
+		super.setUp(delegate);
+		
+		ResourcelessTransactionManager transactionManager = new ResourcelessTransactionManager();
+		
+		new TransactionTemplate(transactionManager).execute(new WriterCallback(Arrays.asList("1", "2", "3")));
+
+		File part1 = new File(file.getAbsolutePath() + suffixCreator.getSuffix(1));
+		assertTrue(part1.exists());
+
+		new TransactionTemplate(transactionManager).execute(new WriterCallback(Arrays.asList("4")));
+		File part2 = new File(file.getAbsolutePath() + suffixCreator.getSuffix(2));
+		assertTrue(part2.exists());
+		
+		tested.close();
+
+		assertEquals("123f", readFile(part1));
+		assertEquals("4f", readFile(part2));
+
+	}
+
 	@Test
 	public void testRestart() throws Exception {
+
+		super.setUp(delegate);
+
 		tested.write(Arrays.asList("1", "2", "3"));
 
 		File part1 = new File(file.getAbsolutePath() + suffixCreator.getSuffix(1));
@@ -75,4 +176,70 @@ public class MultiResourceItemWriterFlatFileTests extends AbstractMultiResourceI
 		assertTrue(part3.exists());
 		assertEquals("6789", readFile(part3));
 	}
+
+	@Test
+	public void testRestartWithFooter() throws Exception {
+
+		delegate.setFooterCallback(new FlatFileFooterCallback() {
+			public void writeFooter(Writer writer) throws IOException {
+				writer.write("f");
+			}
+		});
+		super.setUp(delegate);
+
+		tested.write(Arrays.asList("1", "2", "3"));
+
+		File part1 = new File(file.getAbsolutePath() + suffixCreator.getSuffix(1));
+		assertTrue(part1.exists());
+		assertEquals("123f", readFile(part1));
+
+		tested.write(Arrays.asList("4"));
+		File part2 = new File(file.getAbsolutePath() + suffixCreator.getSuffix(2));
+		assertTrue(part2.exists());
+		assertEquals("4", readFile(part2));
+
+		tested.update(executionContext);
+		tested.close();
+		tested.open(executionContext);
+
+		tested.write(Arrays.asList("5"));
+		assertEquals("45f", readFile(part2));
+
+		tested.write(Arrays.asList("6", "7", "8", "9"));
+		File part3 = new File(file.getAbsolutePath() + suffixCreator.getSuffix(3));
+		assertTrue(part3.exists());
+		assertEquals("6789f", readFile(part3));
+	}
+
+	@Test
+	public void testTransactionalRestartWithFooter() throws Exception {
+
+		delegate.setFooterCallback(new FlatFileFooterCallback() {
+			public void writeFooter(Writer writer) throws IOException {
+				writer.write("f");
+			}
+		});
+		super.setUp(delegate);
+
+		ResourcelessTransactionManager transactionManager = new ResourcelessTransactionManager();
+		
+		new TransactionTemplate(transactionManager).execute(new WriterCallback(Arrays.asList("1", "2", "3")));
+
+		File part1 = new File(file.getAbsolutePath() + suffixCreator.getSuffix(1));
+		assertTrue(part1.exists());
+		assertEquals("123f", readFile(part1));
+
+		new TransactionTemplate(transactionManager).execute(new WriterCallback(Arrays.asList("4")));
+		File part2 = new File(file.getAbsolutePath() + suffixCreator.getSuffix(2));
+		assertTrue(part2.exists());
+		assertEquals("4", readFile(part2));
+
+		tested.update(executionContext);
+		tested.close();
+		tested.open(executionContext);
+
+		new TransactionTemplate(transactionManager).execute(new WriterCallback(Arrays.asList("5")));
+		assertEquals("45f", readFile(part2));
+	}
+
 }

spring-batch-infrastructure/src/test/java/org/springframework/batch/item/file/MultiResourceItemWriterXmlTests.java

@@ -1,4 +1,7 @@
 package org.springframework.batch.item.file;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
 import java.io.File;
 import java.io.IOException;
 import java.util.Arrays;
@@ -8,10 +11,8 @@ import javax.xml.stream.XMLEventWriter;
 import javax.xml.stream.XMLStreamException;
 import javax.xml.transform.Result;
 
-import static org.junit.Assert.*;
 import org.junit.Before;
 import org.junit.Test;
-import org.springframework.batch.item.file.MultiResourceItemWriter;
 import org.springframework.batch.item.xml.StaxEventItemWriter;
 import org.springframework.oxm.Marshaller;
 import org.springframework.oxm.XmlMappingException;
@@ -28,21 +29,19 @@ public class MultiResourceItemWriterXmlTests extends AbstractMultiResourceItemWr
 
 	final static private String xmlDocEnd = "</root>";
 
-	@Override
+	private StaxEventItemWriter<String> delegate;
+
 	@Before
 	public void setUp() throws Exception {
-		delegate = new StaxEventItemWriter<String>() {
-			{
-				setMarshaller(new SimpleMarshaller());
-			}
-		};
-		super.setUp();
+		delegate = new StaxEventItemWriter<String>();
+		delegate.setMarshaller(new SimpleMarshaller());
 	}
 
 	/**
 	 * Writes object's toString representation as tag.
 	 */
 	private static class SimpleMarshaller implements Marshaller {
+		
 		public void marshal(Object graph, Result result) throws XmlMappingException, IOException {
 			Assert.isInstanceOf(StaxResult.class, result);
 
@@ -75,6 +74,8 @@ public class MultiResourceItemWriterXmlTests extends AbstractMultiResourceItemWr
 
 	@Test
 	public void multiResourceWritingWithRestart() throws Exception {
+		
+		setUp(delegate);
 
 		tested.write(Arrays.asList("1", "2", "3"));
 

spring-batch-infrastructure/src/test/java/org/springframework/batch/item/xml/StaxEventItemWriterTests.java

@@ -59,8 +59,8 @@ public class StaxEventItemWriterTests {
 
 	private List<? extends Object> items = Collections.singletonList(item);
 
-	private static final String TEST_STRING = "<!--" + ClassUtils.getShortName(StaxEventItemWriter.class)
-			+ "-testString-->";
+	private static final String TEST_STRING = "<" + ClassUtils.getShortName(StaxEventItemWriter.class)
+			+ "-testString/>";
 
 	@Before
 	public void setUp() throws Exception {
@@ -77,7 +77,7 @@ public class StaxEventItemWriterTests {
 		writer.open(executionContext);
 		writer.write(items);
 		writer.close();
-		String content = outputFileContent();
+		String content = getOutputFileContent();
 		assertTrue("Wrong content: " + content, content.contains(TEST_STRING));
 	}
 
@@ -100,9 +100,9 @@ public class StaxEventItemWriterTests {
 
 		// check the output is concatenation of 'before restart' and 'after
 		// restart' writes.
-		String outputFile = outputFileContent();
+		String outputFile = getOutputFileContent();
 		assertEquals(2, StringUtils.countOccurrencesOf(outputFile, TEST_STRING));
-		assertTrue(outputFile.contains("<root>" + TEST_STRING + TEST_STRING + "</root>"));
+		assertEquals("<root>" + TEST_STRING + TEST_STRING + "</root>", outputFile.replace(" ", ""));
 	}
 
 	@Test
@@ -147,7 +147,7 @@ public class StaxEventItemWriterTests {
 
 		// check the output is concatenation of 'before restart' and 'after
 		// restart' writes.
-		String outputFile = outputFileContent();
+		String outputFile = getOutputFileContent();
 		assertEquals(2, StringUtils.countOccurrencesOf(outputFile, TEST_STRING));
 		assertTrue(outputFile.contains("<root>" + TEST_STRING + TEST_STRING + "</root>"));
 	}
@@ -175,7 +175,7 @@ public class StaxEventItemWriterTests {
 		});
 		writer.open(executionContext);
 		writer.write(items);
-		String content = outputFileContent();
+		String content = getOutputFileContent();
 		assertTrue("Wrong content: " + content, content.contains(("<header/>")));
 		assertTrue("Wrong content: " + content, content.contains(TEST_STRING));
 	}
@@ -237,7 +237,7 @@ public class StaxEventItemWriterTests {
 		writer.setRootElementAttributes(Collections.<String, String> singletonMap("attribute", "value"));
 		writer.open(executionContext);
 		writer.close();
-		String content = outputFileContent();
+		String content = getOutputFileContent();
 
 		assertTrue(content.contains("<testroot attribute=\"value\">"));
 		assertTrue(content.contains("<header/>"));
@@ -272,7 +272,8 @@ public class StaxEventItemWriterTests {
 
 			StaxResult staxResult = (StaxResult) result;
 			try {
-				staxResult.getXMLEventWriter().add(XMLEventFactory.newInstance().createComment(graph.toString()));
+				staxResult.getXMLEventWriter().add(XMLEventFactory.newInstance().createStartElement("", "", graph.toString()));
+				staxResult.getXMLEventWriter().add(XMLEventFactory.newInstance().createEndElement("", "", graph.toString()));
 			}
 			catch (XMLStreamException e) {
 				throw new RuntimeException("Exception while writing to output file", e);
@@ -288,8 +289,10 @@ public class StaxEventItemWriterTests {
 	/**
 	 * @return output file content as String
 	 */
-	private String outputFileContent() throws IOException {
-		return FileUtils.readFileToString(resource.getFile(), null);
+	private String getOutputFileContent() throws IOException {
+		String value = FileUtils.readFileToString(resource.getFile(), null);
+		value = value.replace("<?xml version='1.0' encoding='UTF-8'?>", "");
+		return value;
 	}
 
 	/**

spring-batch-infrastructure/src/test/java/org/springframework/batch/support/transaction/TransactionAwareBufferedWriterTests.java

@@ -37,7 +37,16 @@ public class TransactionAwareBufferedWriterTests {
 
 	private Writer stringWriter = new StringWriter();
 
-	private TransactionAwareBufferedWriter writer = new TransactionAwareBufferedWriter(stringWriter, "someName");
+	private TransactionAwareBufferedWriter writer = new TransactionAwareBufferedWriter(stringWriter, new Runnable() {
+		public void run() {
+			try {
+				stringWriter.append("c");
+			}
+			catch (IOException e) {
+				throw new IllegalStateException(e);
+			}
+		}
+	});
 
 	private PlatformTransactionManager transactionManager = new ResourcelessTransactionManager();
 
@@ -53,6 +62,7 @@ public class TransactionAwareBufferedWriterTests {
 	public void testWriteOutsideTransaction() throws Exception {
 		writer.write("foo");
 		writer.flush();
+		// Not closed yet
 		assertEquals("foo", stringWriter.toString());
 	}
 
@@ -66,7 +76,7 @@ public class TransactionAwareBufferedWriterTests {
 	public void testCloseOutsideTransaction() throws Exception {
 		writer.write("foo");
 		writer.close();
-		assertEquals("foo", stringWriter.toString());
+		assertEquals("fooc", stringWriter.toString());
 	}
 
 	@Test
@@ -86,7 +96,10 @@ public class TransactionAwareBufferedWriterTests {
 			public void write(char[] cbuf, int off, int len) throws IOException {
 			}
 		};
-		writer = new TransactionAwareBufferedWriter(mock, "someName");
+		writer = new TransactionAwareBufferedWriter(mock, new Runnable() {
+			public void run() {				
+			}
+		});
 		new TransactionTemplate(transactionManager).execute(new TransactionCallback() {
 			public Object doInTransaction(TransactionStatus status) {
 				try {
@@ -117,6 +130,7 @@ public class TransactionAwareBufferedWriterTests {
 				return null;
 			}
 		});
+		// Not closed in transaction
 		assertEquals("foo", stringWriter.toString());
 	}