From e2147e24955c1c7933eb133f5797b26313bbf991 Mon Sep 17 00:00:00 2001 From: Anne-Lore Montagne Date: Thu, 12 May 2016 15:05:16 +0200 Subject: [PATCH] BATCH-2500: MultiResourceItemWriter with StaxEventItemWriter restarts at position 0 MultiResourceItemWriter delegating to a StaxEventItemWriter doesn't restart properly. * Changed the open() method in MultiResourceItemWriter by setting the flag 'opened' to true. Updated PR with code review comments. --- .../item/file/MultiResourceItemWriter.java | 3 +- .../AbstractMultiResourceItemWriterTests.java | 11 +- .../MultiResourceItemWriterFlatFileTests.java | 542 +++++++++--------- .../file/MultiResourceItemWriterXmlTests.java | 9 +- 4 files changed, 292 insertions(+), 273 deletions(-) diff --git a/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/file/MultiResourceItemWriter.java b/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/file/MultiResourceItemWriter.java index abf2a9137..3255ba84b 100644 --- a/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/file/MultiResourceItemWriter.java +++ b/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/file/MultiResourceItemWriter.java @@ -1,5 +1,5 @@ /* - * Copyright 2006-2007 the original author or authors. + * Copyright 2006-2017 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. @@ -151,6 +151,7 @@ public class MultiResourceItemWriter extends AbstractItemStreamItemWriter if (executionContext.containsKey(getExecutionContextKey(CURRENT_RESOURCE_ITEM_COUNT))) { // It's a restart delegate.open(executionContext); + opened = true; } else { opened = false; diff --git a/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/file/AbstractMultiResourceItemWriterTests.java b/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/file/AbstractMultiResourceItemWriterTests.java index 0d9254337..5fe6bdf06 100644 --- a/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/file/AbstractMultiResourceItemWriterTests.java +++ b/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/file/AbstractMultiResourceItemWriterTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2008-2009 the original author or authors. + * Copyright 2008-2016 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. @@ -30,7 +30,7 @@ import org.springframework.core.io.FileSystemResource; */ public class AbstractMultiResourceItemWriterTests { - protected MultiResourceItemWriter tested = new MultiResourceItemWriter(); + protected MultiResourceItemWriter tested; protected File file; @@ -39,13 +39,16 @@ public class AbstractMultiResourceItemWriterTests { protected ExecutionContext executionContext = new ExecutionContext(); protected void setUp(ResourceAwareItemWriterItemStream delegate) throws Exception { - file = File.createTempFile(MultiResourceItemWriterFlatFileTests.class.getSimpleName(), null); + tested = new MultiResourceItemWriter(); tested.setResource(new FileSystemResource(file)); tested.setDelegate(delegate); tested.setResourceSuffixCreator(suffixCreator); tested.setItemCountLimitPerResource(2); tested.setSaveState(true); - tested.open(executionContext); + } + + public void createFile() throws Exception { + file = File.createTempFile(MultiResourceItemWriterFlatFileTests.class.getSimpleName(), null); } protected String readFile(File f) throws Exception { diff --git a/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/file/MultiResourceItemWriterFlatFileTests.java b/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/file/MultiResourceItemWriterFlatFileTests.java index 81b563192..a2e0bf61a 100644 --- a/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/file/MultiResourceItemWriterFlatFileTests.java +++ b/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/file/MultiResourceItemWriterFlatFileTests.java @@ -1,265 +1,277 @@ -/* - * Copyright 2008-2014 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. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -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.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 - * {@link FlatFileItemWriter}. - */ -public class MultiResourceItemWriterFlatFileTests extends AbstractMultiResourceItemWriterTests { - - /** - * @author dsyer - * - */ - private final class WriterCallback implements TransactionCallback { - private List list; - - public WriterCallback(List list) { - super(); - this.list = list; - } - - @Override - public Void doInTransaction(TransactionStatus status) { - try { - tested.write(list); - } - catch (Exception e) { - throw new IllegalStateException("Unexpected"); - } - return null; - } - } - - private FlatFileItemWriter delegate; - - @Before - public void setUp() throws Exception { - delegate = new FlatFileItemWriter(); - delegate.setLineAggregator(new PassThroughLineAggregator()); - } - - @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)); - assertTrue(part1.exists()); - assertEquals("123", readFile(part1)); - - tested.write(Arrays.asList("4")); - File part2 = new File(file.getAbsolutePath() + suffixCreator.getSuffix(2)); - assertTrue(part2.exists()); - assertEquals("4", readFile(part2)); - - tested.write(Arrays.asList("5")); - assertEquals("45", readFile(part2)); - - tested.write(Arrays.asList("6", "7", "8", "9")); - File part3 = new File(file.getAbsolutePath() + suffixCreator.getSuffix(3)); - assertTrue(part3.exists()); - assertEquals("6789", readFile(part3)); - } - - @Test - public void testUpdateAfterDelegateClose() throws Exception { - - super.setUp(delegate); - - tested.update(executionContext); - assertEquals(0, executionContext.getInt(tested.getExecutionContextKey("resource.item.count"))); - assertEquals(1, executionContext.getInt(tested.getExecutionContextKey("resource.index"))); - tested.write(Arrays.asList("1", "2", "3")); - tested.update(executionContext); - assertEquals(0, executionContext.getInt(tested.getExecutionContextKey("resource.item.count"))); - assertEquals(2, executionContext.getInt(tested.getExecutionContextKey("resource.index"))); - - } - - @Test - public void testMultiResourceWriteScenarioWithFooter() throws Exception { - - delegate.setFooterCallback(new FlatFileFooterCallback() { - @Override - 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() { - @Override - 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)); - assertTrue(part1.exists()); - assertEquals("123", 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("45", readFile(part2)); - - tested.write(Arrays.asList("6", "7", "8", "9")); - File part3 = new File(file.getAbsolutePath() + suffixCreator.getSuffix(3)); - assertTrue(part3.exists()); - assertEquals("6789", readFile(part3)); - } - - @Test - public void testRestartWithFooter() throws Exception { - - delegate.setFooterCallback(new FlatFileFooterCallback() { - @Override - 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() { - @Override - 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)); - } - -} +/* + * Copyright 2008-2017 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +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.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 + * {@link FlatFileItemWriter}. + */ +public class MultiResourceItemWriterFlatFileTests extends AbstractMultiResourceItemWriterTests { + + /** + * @author dsyer + * + */ + private final class WriterCallback implements TransactionCallback { + private List list; + + public WriterCallback(List list) { + super(); + this.list = list; + } + + @Override + public Void doInTransaction(TransactionStatus status) { + try { + tested.write(list); + } + catch (Exception e) { + throw new IllegalStateException("Unexpected"); + } + return null; + } + } + + private FlatFileItemWriter delegate; + + @Before + public void setUp() throws Exception { + super.createFile(); + delegate = new FlatFileItemWriter(); + delegate.setLineAggregator(new PassThroughLineAggregator()); + } + + @Test + public void testBasicMultiResourceWriteScenario() throws Exception { + + super.setUp(delegate); + tested.open(executionContext); + + tested.write(Arrays.asList("1", "2", "3")); + + File part1 = new File(file.getAbsolutePath() + suffixCreator.getSuffix(1)); + assertTrue(part1.exists()); + assertEquals("123", readFile(part1)); + + tested.write(Arrays.asList("4")); + File part2 = new File(file.getAbsolutePath() + suffixCreator.getSuffix(2)); + assertTrue(part2.exists()); + assertEquals("4", readFile(part2)); + + tested.write(Arrays.asList("5")); + assertEquals("45", readFile(part2)); + + tested.write(Arrays.asList("6", "7", "8", "9")); + File part3 = new File(file.getAbsolutePath() + suffixCreator.getSuffix(3)); + assertTrue(part3.exists()); + assertEquals("6789", readFile(part3)); + } + + @Test + public void testUpdateAfterDelegateClose() throws Exception { + + super.setUp(delegate); + tested.open(executionContext); + + tested.update(executionContext); + assertEquals(0, executionContext.getInt(tested.getExecutionContextKey("resource.item.count"))); + assertEquals(1, executionContext.getInt(tested.getExecutionContextKey("resource.index"))); + tested.write(Arrays.asList("1", "2", "3")); + tested.update(executionContext); + assertEquals(0, executionContext.getInt(tested.getExecutionContextKey("resource.item.count"))); + assertEquals(2, executionContext.getInt(tested.getExecutionContextKey("resource.index"))); + + } + + @Test + public void testMultiResourceWriteScenarioWithFooter() throws Exception { + + delegate.setFooterCallback(new FlatFileFooterCallback() { + @Override + public void writeFooter(Writer writer) throws IOException { + writer.write("f"); + } + }); + super.setUp(delegate); + tested.open(executionContext); + + 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() { + @Override + public void writeFooter(Writer writer) throws IOException { + writer.write("f"); + } + }); + super.setUp(delegate); + tested.open(executionContext); + + 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.open(executionContext); + + tested.write(Arrays.asList("1", "2", "3")); + + File part1 = new File(file.getAbsolutePath() + suffixCreator.getSuffix(1)); + assertTrue(part1.exists()); + assertEquals("123", 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("45", readFile(part2)); + + tested.write(Arrays.asList("6", "7", "8", "9")); + File part3 = new File(file.getAbsolutePath() + suffixCreator.getSuffix(3)); + assertTrue(part3.exists()); + assertEquals("6789", readFile(part3)); + } + + @Test + public void testRestartWithFooter() throws Exception { + + delegate.setFooterCallback(new FlatFileFooterCallback() { + @Override + public void writeFooter(Writer writer) throws IOException { + writer.write("f"); + } + }); + + super.setUp(delegate); + tested.open(executionContext); + + 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() { + @Override + public void writeFooter(Writer writer) throws IOException { + writer.write("f"); + } + }); + super.setUp(delegate); + tested.open(executionContext); + + 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)); + } + +} diff --git a/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/file/MultiResourceItemWriterXmlTests.java b/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/file/MultiResourceItemWriterXmlTests.java index e4615a55a..8b7fe910c 100644 --- a/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/file/MultiResourceItemWriterXmlTests.java +++ b/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/file/MultiResourceItemWriterXmlTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2009-2014 the original author or authors. + * Copyright 2009-2017 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. @@ -47,6 +47,7 @@ public class MultiResourceItemWriterXmlTests extends AbstractMultiResourceItemWr @Before public void setUp() throws Exception { + super.createFile(); delegate = new StaxEventItemWriter(); delegate.setMarshaller(new SimpleMarshaller()); } @@ -88,8 +89,9 @@ public class MultiResourceItemWriterXmlTests extends AbstractMultiResourceItemWr @Test public void multiResourceWritingWithRestart() throws Exception { - - setUp(delegate); + + super.setUp(delegate); + tested.open(executionContext); tested.write(Arrays.asList("1", "2", "3")); @@ -103,6 +105,7 @@ public class MultiResourceItemWriterXmlTests extends AbstractMultiResourceItemWr tested.update(executionContext); tested.close(); + assertEquals(xmlDocStart + "" + xmlDocEnd, readFile(part2)); assertEquals(xmlDocStart + "" + xmlDocEnd, readFile(part1));