OPEN - BATCH-587: Validation : add error object in ValidationException

richer exception messages in SpringValidator + cleaned up the implementation a bit

spring-batch-infrastructure/src/main/java/org/springframework/batch/item/validator/SpringValidator.java

@@ -16,43 +16,64 @@
 
 package org.springframework.batch.item.validator;
 
-import java.util.List;
+import java.util.Collection;
+import java.util.Iterator;
 
-import org.apache.commons.logging.Log;
-import org.apache.commons.logging.LogFactory;
+import org.springframework.beans.factory.InitializingBean;
+import org.springframework.util.Assert;
 import org.springframework.validation.BeanPropertyBindingResult;
-import org.springframework.validation.FieldError;
+import org.springframework.validation.Errors;
 
 /**
- * Adapter for the spring validator interface.
+ * Adapts the {@link org.springframework.validation.Validator} interface to
+ * {@link org.springframework.batch.item.validator.Validator}.
  * 
- * @see Validator
+ * @author Tomas Slanina
+ * @author Robert Kasanicky
  */
-public class SpringValidator implements Validator {
-	private static final Log log = LogFactory.getLog(SpringValidator.class);
+public class SpringValidator implements Validator, InitializingBean {
 
 	private org.springframework.validation.Validator validator;
 
 	/**
 	 * @see Validator#validate(Object)
 	 */
-	public void validate(Object value) throws ValidationException {
-		if (validator == null) {
-			throw new ValidationException("Validator not specified.");
+	public void validate(Object item) throws ValidationException {
+
+		if (!validator.supports(item.getClass())) {
+			throw new ValidationException("Validation failed for " + item + ": " + item.getClass().getName()
+					+ " class is not supported by validator.");
 		}
 
-		BeanPropertyBindingResult errors = new BeanPropertyBindingResult(value, "object");
+		BeanPropertyBindingResult errors = new BeanPropertyBindingResult(item, "item");
 
-		if (validator.supports(value.getClass())) {
-			validator.validate(value, errors);
-		}
-		else {
-			throw new ValidationException(value.getClass() + " is not supported by validator.");
-		}
+		validator.validate(item, errors);
 
 		if (errors.hasErrors()) {
-			log.debug(errors);
-			throw new ValidationException("SpringValidator >> validation failed on: " + getInvalidColumnNames(errors));
+			throw new ValidationException("Validation failed for " + item + ": " + errorsToString(errors));
+		}
+	}
+
+	/**
+	 * @return string of field errors followed by global errors.
+	 */
+	private String errorsToString(Errors errors) {
+		StringBuilder builder = new StringBuilder();
+
+		appendCollection(errors.getFieldErrors(), builder);
+		appendCollection(errors.getGlobalErrors(), builder);
+
+		return builder.toString();
+	}
+
+	/**
+	 * Append the string representation of elements of the collection (separated
+	 * by new lines) to the given StringBuilder.
+	 */
+	private void appendCollection(Collection collection, StringBuilder builder) {
+		for (Iterator iterator = collection.iterator(); iterator.hasNext();) {
+			builder.append("\n");
+			builder.append(iterator.next().toString());
 		}
 	}
 
@@ -60,18 +81,8 @@ public class SpringValidator implements Validator {
 		this.validator = validator;
 	}
 
-	private String getInvalidColumnNames(BeanPropertyBindingResult errors) {
-		StringBuffer stringBuffer = new StringBuffer();
-		List list = errors.getFieldErrors();
+	public void afterPropertiesSet() throws Exception {
+		Assert.notNull(validator, "validator must be set");
 
-		for (int i = 0; i < list.size(); i++) {
-			if (i > 0) {
-				stringBuffer.append(", ");
-			}
-
-			stringBuffer.append(((FieldError) list.get(i)).getField());
-		}
-
-		return stringBuffer.toString();
 	}
 }

spring-batch-infrastructure/src/test/java/org/springframework/batch/item/validator/SpringValidatorTests.java

@@ -21,10 +21,14 @@ import junit.framework.TestCase;
 import org.springframework.validation.Errors;
 import org.springframework.validation.Validator;
 
+/**
+ * Tests for {@link SpringValidator}.
+ */
 public class SpringValidatorTests extends TestCase {
-	private SpringValidator validator = new SpringValidator();
 
-	private Validator mockValidator;
+	SpringValidator validator = new SpringValidator();
+
+	Validator mockValidator;
 
 	protected void setUp() throws Exception {
 		mockValidator = new MockSpringValidator();
@@ -34,15 +38,16 @@ public class SpringValidatorTests extends TestCase {
 	/**
 	 * Validator property is not set
 	 */
-	public void testValidateNullValidator() {
+	public void testNullValidator() throws Exception {
+
 		validator.setValidator(null);
 
 		try {
-			validator.validate(MockSpringValidator.ACCEPT_VALUE);
-			fail("must not validate with null validator");
+			validator.afterPropertiesSet();
+			fail("null validator must cause exception");
 		}
-		catch (ValidationException expected) {
-			assertTrue(true);
+		catch (IllegalArgumentException e) {
+			// expected
 		}
 	}
 
@@ -60,33 +65,29 @@ public class SpringValidatorTests extends TestCase {
 	}
 
 	/**
-	 * Typical successful validation
+	 * Typical successful validation - no exception is thrown.
 	 */
 	public void testValidateSuccessfully() {
-		try {
-			validator.validate(MockSpringValidator.ACCEPT_VALUE);
-			assertTrue(true);
-		}
-		catch (ValidationException unexpected) {
-			throw unexpected;
-		}
+		validator.validate(MockSpringValidator.ACCEPT_VALUE);
+		assertTrue(true);
 	}
 
 	/**
-	 * Typical failed validation
+	 * Typical failed validation - {@link ValidationException} is thrown
 	 */
 	public void testValidateFailure() {
 		try {
 			validator.validate(MockSpringValidator.REJECT_VALUE);
 			fail("exception should have been thrown on invalid value");
 		}
-		catch (ValidationException expected) {
-			assertTrue(true);
+		catch (ValidationException e) {
+			// expected
 		}
 	}
-	
+
 	/**
-	 * Typical failed validation
+	 * Typical failed validation - message contains the item and names of
+	 * invalid fields.
 	 */
 	public void testValidateFailureWithFields() {
 		try {
@@ -94,10 +95,14 @@ public class SpringValidatorTests extends TestCase {
 			fail("exception should have been thrown on invalid value");
 		}
 		catch (ValidationException expected) {
-			assertTrue("Wonrg message: "+expected.getMessage(), expected.getMessage().indexOf("foo, bar")>=0);
+//			System.out.println(expected.getMessage());
+			assertTrue("message should contain the item#toString() value", expected.getMessage().contains(
+					"TestBeanToString"));
+			assertTrue("message should contain names of the invalid fields", expected.getMessage().contains("foo"));
+			assertTrue("message should contain names of the invalid fields", expected.getMessage().contains("bar"));
 		}
 	}
-	
+
 	static class MockSpringValidator implements Validator {
 		public static final TestBean ACCEPT_VALUE = new TestBean();
 
@@ -125,24 +130,33 @@ public class SpringValidatorTests extends TestCase {
 			}
 		}
 	}
-	
+
 	static class TestBean {
 		private String foo;
+
 		private String bar;
+
 		public String getFoo() {
 			return foo;
 		}
+
 		public String getBar() {
 			return bar;
 		}
+
 		public TestBean() {
 			super();
 		}
+
 		public TestBean(String foo, String bar) {
 			this();
 			this.foo = foo;
 			this.bar = bar;
 		}
-		
+
+		public String toString() {
+			return "TestBeanToString";
+		}
+
 	}
 }