BATCH-1603: change item readers so they can still read after open fails

Author: dsyer

spring-batch-infrastructure/src/main/java/org/springframework/batch/item/file/FlatFileItemReader.java

@@ -179,17 +179,17 @@ protected T doRead() throws Exception {
 		}
 
 		String line = readLine();
-		
+
 		if (line == null) {
 			return null;
 		}
 		else {
-			try{
+			try {
 				return lineMapper.mapLine(line, lineCount);
 			}
-			catch(Exception ex){
-				logger.error("Parsing error at line: " + lineCount + " in resource=" + 
-						resource.getDescription() + ", input=[" + line + "]", ex);
+			catch (Exception ex) {
+				logger.error("Parsing error at line: " + lineCount + " in resource=" + resource.getDescription()
+						+ ", input=[" + line + "]", ex);
 				throw ex;
 			}
 		}
@@ -219,7 +219,7 @@ private String readLine() {
 				}
 				lineCount++;
 			}
-			
+
 			line = applyRecordSeparatorPolicy(line);
 		}
 		catch (IOException e) {
@@ -250,21 +250,20 @@ protected void doOpen() throws Exception {
 		Assert.notNull(resource, "Input resource must be set");
 		Assert.notNull(recordSeparatorPolicy, "RecordSeparatorPolicy must be set");
 
-		noInput = false;
+		noInput = true;
 		if (!resource.exists()) {
 			if (strict) {
 				throw new IllegalStateException("Input resource must exist (reader is in 'strict' mode): " + resource);
 			}
-			noInput = true;
 			logger.warn("Input resource does not exist " + resource.getDescription());
 			return;
 		}
 
 		if (!resource.isReadable()) {
 			if (strict) {
-				throw new IllegalStateException("Input resource must be readable (reader is in 'strict' mode): " + resource);
+				throw new IllegalStateException("Input resource must be readable (reader is in 'strict' mode): "
+						+ resource);
 			}
-			noInput = true;
 			logger.warn("Input resource is not readable " + resource.getDescription());
 			return;
 		}
@@ -276,21 +275,22 @@ protected void doOpen() throws Exception {
 				skippedLinesCallback.handleLine(line);
 			}
 		}
+		noInput = false;
 	}
 
 	public void afterPropertiesSet() throws Exception {
 		Assert.notNull(lineMapper, "LineMapper is required");
 	}
-	
+
 	@Override
 	protected void jumpToItem(int itemIndex) throws Exception {
 		for (int i = 0; i < itemIndex; i++) {
 			readLine();
 		}
 	}
-	
-	private String applyRecordSeparatorPolicy(String line) throws IOException{
-		
+
+	private String applyRecordSeparatorPolicy(String line) throws IOException {
+
 		String record = line;
 		while (line != null && !recordSeparatorPolicy.isEndOfRecord(record)) {
 			line = this.reader.readLine();
@@ -302,18 +302,19 @@ private String applyRecordSeparatorPolicy(String line) throws IOException{
 				}
 				else {
 					// Record has no text but it might still be post processed
-					// to something (skipping preProcess since that was already done)
+					// to something (skipping preProcess since that was already
+					// done)
 					break;
 				}
-			} else {
+			}
+			else {
 				lineCount++;
-            }
+			}
 			record = recordSeparatorPolicy.preProcess(record) + line;
 		}
-		
+
 		return recordSeparatorPolicy.postProcess(record);
-		
-	}
 
+	}
 
 }

spring-batch-infrastructure/src/main/java/org/springframework/batch/item/file/FlatFileItemWriter.java

@@ -33,7 +33,6 @@
 import org.springframework.batch.item.ItemStreamException;
 import org.springframework.batch.item.WriteFailedException;
 import org.springframework.batch.item.WriterNotOpenException;
-import org.springframework.batch.item.database.JdbcBatchItemWriter;
 import org.springframework.batch.item.file.transform.LineAggregator;
 import org.springframework.batch.item.util.ExecutionContextUserSupport;
 import org.springframework.batch.item.util.FileUtils;

spring-batch-infrastructure/src/main/java/org/springframework/batch/item/xml/StaxEventItemReader.java

@@ -148,7 +148,8 @@ protected boolean moveCursorToNextFragment(XMLEventReader reader) {
 				}
 				QName startElementName = ((StartElement) reader.peek()).getName();
 				if (startElementName.getLocalPart().equals(fragmentRootElementName)) {
-					if (fragmentRootElementNameSpace==null || startElementName.getNamespaceURI().equals(fragmentRootElementNameSpace)) {
+					if (fragmentRootElementNameSpace == null
+							|| startElementName.getNamespaceURI().equals(fragmentRootElementNameSpace)) {
 						return true;
 					}
 				}
@@ -180,27 +181,26 @@ protected void doClose() throws Exception {
 	protected void doOpen() throws Exception {
 		Assert.notNull(resource, "The Resource must not be null.");
 
-		noInput = false;
+		noInput = true;
 		if (!resource.exists()) {
 			if (strict) {
 				throw new IllegalStateException("Input resource must exist (reader is in 'strict' mode)");
 			}
-			noInput = true;
 			logger.warn("Input resource does not exist " + resource.getDescription());
 			return;
 		}
 		if (!resource.isReadable()) {
 			if (strict) {
 				throw new IllegalStateException("Input resource must be readable (reader is in 'strict' mode)");
 			}
-			noInput = true;
 			logger.warn("Input resource is not readable " + resource.getDescription());
 			return;
 		}
 
 		inputStream = resource.getInputStream();
 		eventReader = XMLInputFactory.newInstance().createXMLEventReader(inputStream);
 		fragmentReader = new DefaultFragmentEventReader(eventReader);
+		noInput = false;
 
 	}
 

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

@@ -302,6 +302,37 @@ public void testNonExistentResource() throws Exception {
 		reader.close();
 	}
 
+    @Test
+    public void testOpenBadIOInput() throws Exception {
+
+        reader.setResource(new AbstractResource() {
+            public String getDescription() {
+                return null;
+            }
+
+            public InputStream getInputStream() throws IOException {
+                throw new IOException();
+            }
+
+            public boolean exists() {
+                return true;
+            }
+        });
+
+        try {
+            reader.open(executionContext);
+            fail();
+        }
+        catch (ItemStreamException ex) {
+            // expected
+        }
+        
+        // read() should then return a null
+        assertNull(reader.read());
+        reader.close();
+
+    }
+	
 	@Test
 	public void testDirectoryResource() throws Exception {
 

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

@@ -345,6 +345,41 @@ public InputStream getInputStream() throws IOException {
 		}
 	}
 
+	@Test
+    public void testBadIOInput() throws Exception {
+        
+        Resource badResource = new AbstractResource() {
+            
+            public boolean exists() {
+                // Looks good ...
+                return true;
+            }
+            
+            public InputStream getInputStream() throws IOException {
+                // ... but fails during read 
+                throw new RuntimeException();
+            }
+            
+            public String getDescription() {return null;}
+        };
+
+        tested.setResources(new Resource[] { badResource, r2, r3, r4, r5 });
+        
+        tested.open(ctx);
+
+        try{
+            assertEquals("1", tested.read());
+            fail();
+        }
+        catch(ItemStreamException ex){
+            // expected
+        }
+        
+        // Now check the next read gets the next resource
+        assertEquals("4", tested.read());
+        
+    }
+	
 	@Test
 	public void testGetCurrentResourceBeforeRead() throws Exception {
 		tested.open(ctx);

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

@@ -234,7 +234,7 @@ public void testClose() throws Exception {
 	}
 
 	@Test
-	public void testOpenBadIOInput() {
+	public void testOpenBadIOInput() throws Exception {
 
 		source.setResource(new AbstractResource() {
 			public String getDescription() {
@@ -252,10 +252,15 @@ public boolean exists() {
 
 		try {
 			source.open(executionContext);
+			fail();
 		}
 		catch (ItemStreamException ex) {
 			// expected
 		}
+		
+        // read() should then return a null
+        assertNull(source.read());
+        source.close();
 
 	}