diff --git a/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/xml/StaxEventItemReader.java b/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/xml/StaxEventItemReader.java index d143fe3ee..94e9e8eb9 100644 --- a/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/xml/StaxEventItemReader.java +++ b/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/xml/StaxEventItemReader.java @@ -42,16 +42,16 @@ import org.springframework.util.ClassUtils; /** * Item reader for reading XML input based on StAX. * - * It extracts fragments from the input XML document which correspond to records - * for processing. The fragments are wrapped with StartDocument and EndDocument - * events so that the fragments can be further processed like standalone XML + * It extracts fragments from the input XML document which correspond to records for processing. The fragments are + * wrapped with StartDocument and EndDocument events so that the fragments can be further processed like standalone XML * documents. * * The implementation is *not* thread-safe. * * @author Robert Kasanicky */ -public class StaxEventItemReader extends AbstractItemCountingItemStreamItemReader implements ResourceAwareItemReaderItemStream, InitializingBean { +public class StaxEventItemReader extends AbstractItemCountingItemStreamItemReader implements + ResourceAwareItemReaderItemStream, InitializingBean { private static final Log logger = LogFactory.getLog(StaxEventItemReader.class); @@ -79,8 +79,7 @@ public class StaxEventItemReader extends AbstractItemCountingItemStreamItemRe /** * In strict mode the reader will throw an exception on - * {@link #open(org.springframework.batch.item.ExecutionContext)} if the - * input resource does not exist. + * {@link #open(org.springframework.batch.item.ExecutionContext)} if the input resource does not exist. * @param strict false by default */ public void setStrict(boolean strict) { @@ -92,8 +91,7 @@ public class StaxEventItemReader extends AbstractItemCountingItemStreamItemRe } /** - * @param unmarshaller maps xml fragments corresponding to records to - * objects + * @param unmarshaller maps xml fragments corresponding to records to objects */ public void setUnmarshaller(Unmarshaller unmarshaller) { this.unmarshaller = unmarshaller; @@ -107,12 +105,11 @@ public class StaxEventItemReader extends AbstractItemCountingItemStreamItemRe } /** - * Ensure that all required dependencies for the ItemReader to run are - * provided after all properties have been set. + * Ensure that all required dependencies for the ItemReader to run are provided after all properties have been set. * * @see org.springframework.beans.factory.InitializingBean#afterPropertiesSet() - * @throws IllegalArgumentException if the Resource, FragmentDeserializer or - * FragmentRootElementName is null, or if the root element is empty. + * @throws IllegalArgumentException if the Resource, FragmentDeserializer or FragmentRootElementName is null, or if + * the root element is empty. * @throws IllegalStateException if the Resource does not exist. */ public void afterPropertiesSet() throws Exception { @@ -125,19 +122,15 @@ public class StaxEventItemReader extends AbstractItemCountingItemStreamItemRe } /** - * Responsible for moving the cursor before the StartElement of the fragment - * root. + * Responsible for moving the cursor before the StartElement of the fragment root. * - * This implementation simply looks for the next corresponding element, it - * does not care about element nesting. You will need to override this - * method to correctly handle composite fragments. + * This implementation simply looks for the next corresponding element, it does not care about element nesting. You + * will need to override this method to correctly handle composite fragments. * - * @return true if next fragment was found, false - * otherwise. + * @return true if next fragment was found, false otherwise. * - * @throws NonTransientResourceException if the cursor could not be - * moved. This will be treated as fatal and subsequent calls to read will - * return null. + * @throws NonTransientResourceException if the cursor could not be moved. This will be treated as fatal and + * subsequent calls to read will return null. */ protected boolean moveCursorToNextFragment(XMLEventReader reader) throws NonTransientResourceException { try { @@ -229,20 +222,22 @@ public class StaxEventItemReader extends AbstractItemCountingItemStreamItemRe if (success) { fragmentReader.markStartFragment(); - @SuppressWarnings("unchecked") - T mappedFragment = (T) unmarshaller.unmarshal(StaxUtils.getSource(fragmentReader)); - - item = mappedFragment; - fragmentReader.markFragmentProcessed(); + try { + @SuppressWarnings("unchecked") + T mappedFragment = (T) unmarshaller.unmarshal(StaxUtils.getSource(fragmentReader)); + item = mappedFragment; + } + finally { + fragmentReader.markFragmentProcessed(); + } } return item; } /* - * jumpToItem is overridden because reading in and attempting to bind an - * entire fragment is unacceptable in a restart scenario, and may cause - * exceptions to be thrown that were already skipped in previous runs. + * jumpToItem is overridden because reading in and attempting to bind an entire fragment is unacceptable in a + * restart scenario, and may cause exceptions to be thrown that were already skipped in previous runs. */ @Override protected void jumpToItem(int itemIndex) throws Exception { @@ -253,10 +248,9 @@ public class StaxEventItemReader extends AbstractItemCountingItemStreamItemRe } /* - * Read until the first StartElement tag that matches the provided - * fragmentRootElementName. Because there may be any number of tags in - * between where the reader is now and the fragment start, this is done in a - * loop until the element type and name match. + * Read until the first StartElement tag that matches the provided fragmentRootElementName. Because there may be any + * number of tags in between where the reader is now and the fragment start, this is done in a loop until the + * element type and name match. */ private void readToStartFragment() throws XMLStreamException { while (true) { @@ -269,10 +263,9 @@ public class StaxEventItemReader extends AbstractItemCountingItemStreamItemRe } /* - * Read until the first EndElement tag that matches the provided - * fragmentRootElementName. Because there may be any number of tags in - * between where the reader is now and the fragment end tag, this is done in - * a loop until the element type and name match + * Read until the first EndElement tag that matches the provided fragmentRootElementName. Because there may be any + * number of tags in between where the reader is now and the fragment end tag, this is done in a loop until the + * element type and name match */ private void readToEndFragment() throws XMLStreamException { while (true) { diff --git a/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/xml/stax/DefaultFragmentEventReader.java b/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/xml/stax/DefaultFragmentEventReader.java index 313f287ae..75fe512d9 100644 --- a/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/xml/stax/DefaultFragmentEventReader.java +++ b/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/xml/stax/DefaultFragmentEventReader.java @@ -191,6 +191,8 @@ public class DefaultFragmentEventReader extends AbstractEventReaderWrapper imple startFragmentFollows = false; endFragmentFollows = false; fakeDocumentEnd = false; + fragmentRootName = null; + matchCounter = 0; } } diff --git a/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/xml/StaxEventItemReaderTests.java b/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/xml/StaxEventItemReaderTests.java index 91b032b14..c55daa79a 100644 --- a/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/xml/StaxEventItemReaderTests.java +++ b/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/xml/StaxEventItemReaderTests.java @@ -24,7 +24,9 @@ import org.springframework.core.io.AbstractResource; import org.springframework.core.io.ByteArrayResource; import org.springframework.core.io.FileSystemResource; import org.springframework.core.io.Resource; +import org.springframework.oxm.UncategorizedXmlMappingException; import org.springframework.oxm.Unmarshaller; +import org.springframework.oxm.UnmarshallingFailureException; import org.springframework.oxm.XmlMappingException; import org.springframework.util.ClassUtils; @@ -89,8 +91,8 @@ public class StaxEventItemReaderTests { } /** - * Regular usage scenario. ItemReader should pass XML fragments to - * unmarshaller wrapped with StartDocument and EndDocument events. + * Regular usage scenario. ItemReader should pass XML fragments to unmarshaller wrapped with StartDocument and + * EndDocument events. */ @Test public void testFragmentWrapping() throws Exception { @@ -197,8 +199,7 @@ public class StaxEventItemReaderTests { } /** - * Statistics return the current record count. Calling read after end of - * input does not increase the counter. + * Statistics return the current record count. Calling read after end of input does not increase the counter. */ @Test public void testExecutionContext() throws Exception { @@ -335,6 +336,55 @@ public class StaxEventItemReaderTests { } + /** + * Make sure the reader doesn't end up in inconsistent state if there's an error during unmarshalling (BATCH-1738). + * After an error during read the next read call should continue with reading the next + * fragment. + */ + @Test + public void exceptionDuringUnmarshalling() throws Exception { + source.setUnmarshaller(new TroublemakerUnmarshaller()); + source.afterPropertiesSet(); + + source.open(executionContext); + try { + source.read(); + fail(); + } + catch (UnmarshallingFailureException expected) { + assert expected.getMessage() == TroublemakerUnmarshaller.MESSAGE; + } + + try { + source.read(); + fail(); + } + catch (UnmarshallingFailureException expected) { + assert expected.getMessage() == TroublemakerUnmarshaller.MESSAGE; + } + assertNull(source.read()); + } + + /** + * Stub emulating problems during unmarshalling. + */ + private static class TroublemakerUnmarshaller implements Unmarshaller { + + public static final String MESSAGE = "Unmarshallers on strike."; + + @Override + public Object unmarshal(Source source) throws XmlMappingException, IOException { + throw new UnmarshallingFailureException(MESSAGE); + } + + @SuppressWarnings("rawtypes") + @Override + public boolean supports(Class clazz) { + return true; + } + + } + private StaxEventItemReader> createNewInputSouce() { Resource resource = new ByteArrayResource(xml.getBytes()); @@ -349,9 +399,8 @@ public class StaxEventItemReaderTests { } /** - * A simple XMLEvent unmarshaller mock - check for the start and end - * document events for the fragment root & end tags + skips the fragment - * contents. + * A simple XMLEvent unmarshaller mock - check for the start and end document events for the fragment root & end + * tags + skips the fragment contents. */ private static class MockFragmentUnmarshaller implements Unmarshaller { @@ -379,8 +428,7 @@ public class StaxEventItemReaderTests { } /** - * A simple mapFragment implementation checking the - * StaxEventReaderItemReader basic read functionality. + * A simple mapFragment implementation checking the StaxEventReaderItemReader basic read functionality. * * @param source * @return list of the events from fragment body @@ -389,7 +437,7 @@ public class StaxEventItemReaderTests { List fragmentContent; try { - XMLEventReader eventReader = StaxUtils.getXmlEventReader( source); + XMLEventReader eventReader = StaxUtils.getXmlEventReader(source); // first event should be StartDocument XMLEvent event1 = eventReader.nextEvent();