diff --git a/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/file/FlatFileItemReader.java b/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/file/FlatFileItemReader.java index b772a2aae..4ca4c567c 100644 --- a/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/file/FlatFileItemReader.java +++ b/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/file/FlatFileItemReader.java @@ -34,10 +34,10 @@ import org.springframework.util.ClassUtils; import org.springframework.util.StringUtils; /** - * Restartable {@link ItemReader} that reads lines from input - * {@link #setResource(Resource)}. Line is defined by the - * {@link #setRecordSeparatorPolicy(RecordSeparatorPolicy)} and mapped to item - * using {@link #setLineMapper(LineMapper)}. + * Restartable {@link ItemReader} that reads lines from input {@link #setResource(Resource)}. Line is defined by the + * {@link #setRecordSeparatorPolicy(RecordSeparatorPolicy)} and mapped to item using {@link #setLineMapper(LineMapper)}. + * If an exception is thrown during line mapping it is rethrown as {@link FlatFileParseException} adding information + * about the problematic line and its line number. * * @author Robert Kasanicky */ @@ -79,8 +79,7 @@ public class FlatFileItemReader extends AbstractItemCountingItemStreamItemRea /** * 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) { @@ -88,18 +87,15 @@ public class FlatFileItemReader extends AbstractItemCountingItemStreamItemRea } /** - * @param skippedLinesCallback will be called for each one of the initial - * skipped lines before any items are read. + * @param skippedLinesCallback will be called for each one of the initial skipped lines before any items are read. */ public void setSkippedLinesCallback(LineCallbackHandler skippedLinesCallback) { this.skippedLinesCallback = skippedLinesCallback; } /** - * Public setter for the number of lines to skip at the start of a file. Can - * be used if the file contains a header without useful (column name) - * information, and without a comment delimiter at the beginning of the - * lines. + * Public setter for the number of lines to skip at the start of a file. Can be used if the file contains a header + * without useful (column name) information, and without a comment delimiter at the beginning of the lines. * * @param linesToSkip the number of lines to skip */ @@ -116,21 +112,18 @@ public class FlatFileItemReader extends AbstractItemCountingItemStreamItemRea } /** - * Setter for the encoding for this input source. Default value is - * {@link #DEFAULT_CHARSET}. + * Setter for the encoding for this input source. Default value is {@link #DEFAULT_CHARSET}. * - * @param encoding a properties object which possibly contains the encoding - * for this input file; + * @param encoding a properties object which possibly contains the encoding for this input file; */ public void setEncoding(String encoding) { this.encoding = encoding; } /** - * Factory for the {@link BufferedReader} that will be used to extract lines - * from the file. The default is fine for plain text files, but this is a - * useful strategy for binary files where the standard BufferedReaader from - * java.io is limiting. + * Factory for the {@link BufferedReader} that will be used to extract lines from the file. The default is fine for + * plain text files, but this is a useful strategy for binary files where the standard BufferedReaader from java.io + * is limiting. * * @param bufferedReaderFactory the bufferedReaderFactory to set */ @@ -139,8 +132,8 @@ public class FlatFileItemReader extends AbstractItemCountingItemStreamItemRea } /** - * Setter for comment prefixes. Can be used to ignore header lines as well - * by using e.g. the first couple of column names as a prefix. + * Setter for comment prefixes. Can be used to ignore header lines as well by using e.g. the first couple of column + * names as a prefix. * * @param comments an array of comment line prefixes. */ @@ -157,9 +150,8 @@ public class FlatFileItemReader extends AbstractItemCountingItemStreamItemRea } /** - * Public setter for the recordSeparatorPolicy. Used to determine where the - * line endings are and do things like continue over a line ending if inside - * a quoted string. + * Public setter for the recordSeparatorPolicy. Used to determine where the line endings are and do things like + * continue over a line ending if inside a quoted string. * * @param recordSeparatorPolicy the recordSeparatorPolicy to set */ @@ -169,8 +161,7 @@ public class FlatFileItemReader extends AbstractItemCountingItemStreamItemRea /** * @return string corresponding to logical record according to - * {@link #setRecordSeparatorPolicy(RecordSeparatorPolicy)} (might span - * multiple lines in file). + * {@link #setRecordSeparatorPolicy(RecordSeparatorPolicy)} (might span multiple lines in file). */ @Override protected T doRead() throws Exception { @@ -188,15 +179,14 @@ public class FlatFileItemReader extends AbstractItemCountingItemStreamItemRea return lineMapper.mapLine(line, lineCount); } catch (Exception ex) { - logger.error("Parsing error at line: " + lineCount + " in resource=" + resource.getDescription() - + ", input=[" + line + "]", ex); - throw ex; + throw new FlatFileParseException("Parsing error at line: " + lineCount + " in resource=[" + + resource.getDescription() + "], input=[" + line + "]", ex, line, lineCount); } } } /** - * @return next line (skip comments). + * @return next line (skip comments).getCurrentResource */ private String readLine() { @@ -226,7 +216,8 @@ public class FlatFileItemReader extends AbstractItemCountingItemStreamItemRea // Prevent IOException from recurring indefinitely // if client keeps catching and re-calling noInput = true; - throw new NonTransientFlatFileException("Unable to read from resource: [" + resource + "]", e, line, lineCount); + throw new NonTransientFlatFileException("Unable to read from resource: [" + resource + "]", e, line, + lineCount); } return line; } diff --git a/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/file/mapping/DefaultLineMapper.java b/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/file/mapping/DefaultLineMapper.java index 1b199959f..4a21f1d86 100644 --- a/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/file/mapping/DefaultLineMapper.java +++ b/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/file/mapping/DefaultLineMapper.java @@ -16,7 +16,6 @@ package org.springframework.batch.item.file.mapping; -import org.springframework.batch.item.file.FlatFileParseException; import org.springframework.batch.item.file.LineMapper; import org.springframework.batch.item.file.transform.FieldSet; import org.springframework.batch.item.file.transform.LineTokenizer; @@ -24,11 +23,9 @@ import org.springframework.beans.factory.InitializingBean; import org.springframework.util.Assert; /** - * Two-phase {@link LineMapper} implementation consisting of tokenization of the - * line into {@link FieldSet} followed by mapping to item. By default, any exceptions - * thrown by either delegates will be wrapped in a {@link FlatFileParseException} before - * being rethrown. If finer grained control of exceptions is needed, the {@link LineMapper} - * interface should be implemented directly. + * Two-phase {@link LineMapper} implementation consisting of tokenization of the line into {@link FieldSet} followed by + * mapping to item. If finer grained control of exceptions is needed, the {@link LineMapper} interface should be + * implemented directly. * * @author Robert Kasanicky * @author Lucas Ward @@ -42,13 +39,7 @@ public class DefaultLineMapper implements LineMapper, InitializingBean { private FieldSetMapper fieldSetMapper; public T mapLine(String line, int lineNumber) throws Exception { - try{ - return fieldSetMapper.mapFieldSet(tokenizer.tokenize(line)); - } - catch(Exception ex){ - throw new FlatFileParseException("Parsing error at line: " + lineNumber + - ", input=[" + line + "]", ex, line, lineNumber); - } + return fieldSetMapper.mapFieldSet(tokenizer.tokenize(line)); } public void setLineTokenizer(LineTokenizer tokenizer) { diff --git a/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/file/mapping/JsonLineMapper.java b/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/file/mapping/JsonLineMapper.java index 602bb3ec5..41b0f4d8f 100644 --- a/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/file/mapping/JsonLineMapper.java +++ b/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/file/mapping/JsonLineMapper.java @@ -4,13 +4,11 @@ import java.util.Map; import org.codehaus.jackson.JsonParser; import org.codehaus.jackson.map.MappingJsonFactory; -import org.springframework.batch.item.file.FlatFileParseException; import org.springframework.batch.item.file.LineMapper; /** - * Interpret a line as a Json object and parse it up to a Map. The line should - * be a standard Json object, starting with "{" and ending with "}" and composed - * of name:value pairs separated by commas. Whitespace is ignored, + * Interpret a line as a Json object and parse it up to a Map. The line should be a standard Json object, starting with + * "{" and ending with "}" and composed of name:value pairs separated by commas. Whitespace is ignored, * e.g. * *
@@ -31,21 +29,16 @@ public class JsonLineMapper implements LineMapper> {
 	private MappingJsonFactory factory = new MappingJsonFactory();
 
 	/**
-	 * Interpret the line as a Json object and create a Map from it.  
+	 * Interpret the line as a Json object and create a Map from it.
 	 * 
 	 * @see LineMapper#mapLine(String, int)
 	 */
 	public Map mapLine(String line, int lineNumber) throws Exception {
 		Map result;
-		try {
-			JsonParser parser = factory.createJsonParser(line);
-			@SuppressWarnings("unchecked")
-			Map token = parser.readValueAs(Map.class);
-			result = token;
-		}
-		catch (Exception e) {
-			throw new FlatFileParseException("Cannot parse line to JSON", e, line, lineNumber);
-		}
+		JsonParser parser = factory.createJsonParser(line);
+		@SuppressWarnings("unchecked")
+		Map token = parser.readValueAs(Map.class);
+		result = token;
 		return result;
 	}
 
diff --git a/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/file/FlatFileItemReaderTests.java b/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/file/FlatFileItemReaderTests.java
index 9c5b93bba..50f42c5d3 100644
--- a/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/file/FlatFileItemReaderTests.java
+++ b/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/file/FlatFileItemReaderTests.java
@@ -193,8 +193,7 @@ public class FlatFileItemReaderTests {
 		// close input
 		reader.close();
 
-		reader
-				.setResource(getInputResource("header\nignoreme\ntestLine1\ntestLine2\ntestLine3\ntestLine4\ntestLine5\ntestLine6"));
+		reader.setResource(getInputResource("header\nignoreme\ntestLine1\ntestLine2\ntestLine3\ntestLine4\ntestLine5\ntestLine6"));
 
 		// init for restart
 		reader.open(executionContext);
@@ -302,37 +301,37 @@ public class FlatFileItemReaderTests {
 		reader.close();
 	}
 
-    @Test
-    public void testOpenBadIOInput() throws Exception {
+	@Test
+	public void testOpenBadIOInput() throws Exception {
 
-        reader.setResource(new AbstractResource() {
-            public String getDescription() {
-                return null;
-            }
+		reader.setResource(new AbstractResource() {
+			public String getDescription() {
+				return null;
+			}
 
-            public InputStream getInputStream() throws IOException {
-                throw new IOException();
-            }
+			public InputStream getInputStream() throws IOException {
+				throw new IOException();
+			}
 
-            public boolean exists() {
-                return true;
-            }
-        });
+			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();
+		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 {
 
@@ -381,6 +380,40 @@ public class FlatFileItemReaderTests {
 		reader.open(executionContext);
 	}
 
+	/**
+	 * Exceptions from {@link LineMapper} are wrapped as {@link FlatFileParseException} containing contextual info about
+	 * the problematic line and its line number.
+	 */
+	@Test
+	public void testMappingExceptionWrapping() throws Exception {
+		LineMapper exceptionLineMapper = new LineMapper() {
+
+			@Override
+			public String mapLine(String line, int lineNumber) throws Exception {
+				if (lineNumber == 2) {
+					throw new Exception("Couldn't map line 2");
+				}
+				return line;
+			}
+		};
+		reader.setLineMapper(exceptionLineMapper);
+		reader.afterPropertiesSet();
+
+		reader.open(executionContext);
+		assertNotNull(reader.read());
+
+		try {
+			reader.read();
+			fail();
+		}
+		catch (FlatFileParseException expected) {
+			assertEquals(2, expected.getLineNumber());
+			assertEquals("testLine2", expected.getInput());
+			assertEquals("Couldn't map line 2", expected.getCause().getMessage());
+			assertEquals("Parsing error at line: 2 in resource=[resource loaded from byte array], input=[testLine2]", expected.getMessage());
+		}
+	}
+
 	private Resource getInputResource(String input) {
 		return new ByteArrayResource(input.getBytes());
 	}
diff --git a/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/file/mapping/DefaultLineMapperTests.java b/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/file/mapping/DefaultLineMapperTests.java
index 1ce6f2044..7e633e6b3 100644
--- a/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/file/mapping/DefaultLineMapperTests.java
+++ b/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/file/mapping/DefaultLineMapperTests.java
@@ -1,10 +1,12 @@
 package org.springframework.batch.item.file.mapping;
 
-import static org.easymock.EasyMock.*;
-import static org.junit.Assert.*;
+import static org.easymock.EasyMock.createStrictMock;
+import static org.easymock.EasyMock.expect;
+import static org.easymock.EasyMock.replay;
+import static org.easymock.EasyMock.verify;
+import static org.junit.Assert.assertSame;
 
 import org.junit.Test;
-import org.springframework.batch.item.file.FlatFileParseException;
 import org.springframework.batch.item.file.transform.DefaultFieldSet;
 import org.springframework.batch.item.file.transform.DelimitedLineTokenizer;
 import org.springframework.batch.item.file.transform.FieldSet;
@@ -54,53 +56,4 @@ public class DefaultLineMapperTests {
 		
 	}
 	
-	@Test
-	public void testTokenizerException() throws Exception {
-		final String line = "TEST";
-		
-		LineTokenizer tokenizer = createStrictMock(LineTokenizer.class);
-		expect(tokenizer.tokenize(line)).andThrow(new RuntimeException());
-		replay(tokenizer);
-		
-		@SuppressWarnings("unchecked")
-		FieldSetMapper fsMapper = createStrictMock(FieldSetMapper.class);
-		
-		tested.setLineTokenizer(tokenizer);
-		tested.setFieldSetMapper(fsMapper);
-		
-		try{
-			tested.mapLine(line, 1);
-		}
-		catch(FlatFileParseException ex){
-			assertEquals(ex.getLineNumber(), 1);
-			assertEquals(ex.getInput(), line);
-		}
-	}
-	
-	@Test
-	public void testMapperException() throws Exception {
-		final String line = "TEST";
-		final FieldSet fs = new DefaultFieldSet(new String[]{"token1", "token2"});
-		
-		LineTokenizer tokenizer = createStrictMock(LineTokenizer.class);
-		expect(tokenizer.tokenize(line)).andReturn(fs);
-		replay(tokenizer);
-		
-		@SuppressWarnings("unchecked")
-		FieldSetMapper fsMapper = createStrictMock(FieldSetMapper.class);
-		expect(fsMapper.mapFieldSet(fs)).andThrow(new RuntimeException());
-		replay(fsMapper);
-		
-		tested.setLineTokenizer(tokenizer);
-		tested.setFieldSetMapper(fsMapper);
-		
-		try{
-			tested.mapLine(line, 1);
-		}
-		catch(FlatFileParseException ex){
-			assertEquals(ex.getLineNumber(), 1);
-			assertEquals(ex.getInput(), line);
-		}
-		
-	}
 }
diff --git a/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/file/mapping/JsonLineMapperTests.java b/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/file/mapping/JsonLineMapperTests.java
index 4acafd5fe..2c4ac6e69 100644
--- a/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/file/mapping/JsonLineMapperTests.java
+++ b/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/file/mapping/JsonLineMapperTests.java
@@ -1,12 +1,11 @@
 package org.springframework.batch.item.file.mapping;
 
-import static org.junit.Assert.*;
+import static org.junit.Assert.assertEquals;
 
 import java.util.Map;
 
+import org.codehaus.jackson.JsonParseException;
 import org.junit.Test;
-import org.springframework.batch.item.file.FlatFileParseException;
-import org.springframework.batch.item.file.mapping.JsonLineMapper;
 
 public class JsonLineMapperTests {
 	
@@ -26,7 +25,7 @@ public class JsonLineMapperTests {
 		assertEquals(2, ((Map) map.get("bar")).get("foo"));
 	}
 
-	@Test(expected=FlatFileParseException.class)
+	@Test(expected=JsonParseException.class)
 	public void testMappingError() throws Exception {
 		Map map = mapper.mapLine("{\"foo\": 1", 1);
 		assertEquals(1, map.get("foo"));