Code Review Fixes

* AbstractReplyProducingMessageHandler - throw MessageHandlingException instead of MessagingException
* In Parser classes reduce validation logic, where the the Xml Schema already enforces it
* Better null handling in class ProcedureParameter + more unit tests

spring-integration-jdbc/src/main/java/org/springframework/integration/jdbc/StoredProcOutboundGateway.java

@@ -21,8 +21,8 @@ import java.util.Map;
 
 import javax.sql.DataSource;
 
-import org.springframework.beans.factory.InitializingBean;
 import org.springframework.integration.Message;
+import org.springframework.integration.MessageHandlingException;
 import org.springframework.integration.MessagingException;
 import org.springframework.integration.handler.AbstractReplyProducingMessageHandler;
 import org.springframework.integration.jdbc.storedproc.ProcedureParameter;
@@ -38,7 +38,7 @@ import org.springframework.util.Assert;
  *
  * @since 2.1
  */
-public class StoredProcOutboundGateway extends AbstractReplyProducingMessageHandler implements InitializingBean {
+public class StoredProcOutboundGateway extends AbstractReplyProducingMessageHandler {
 
 	private final StoredProcExecutor executor;
 
@@ -87,7 +87,7 @@ public class StoredProcOutboundGateway extends AbstractReplyProducingMessageHand
                 payload = resultMap.values().iterator().next();
         	} else if (this.expectSingleResult && resultMap.size() > 1) {
         		
-        		throw new MessagingException(
+        		throw new MessageHandlingException(requestMessage,
         				"Stored Procedure/Function call returned more than "
         		      + "1 result object and expectSingleResult was 'true'. ");
 

spring-integration-jdbc/src/main/java/org/springframework/integration/jdbc/StoredProcPollingChannelAdapter.java

@@ -43,7 +43,7 @@ import org.springframework.util.Assert;
  */
 public class StoredProcPollingChannelAdapter extends IntegrationObjectSupport implements MessageSource<Object> {
 
-    final StoredProcExecutor executor;
+    private final StoredProcExecutor executor;
 
 	private volatile boolean expectSingleResult = false;
 	
@@ -117,8 +117,7 @@ public class StoredProcPollingChannelAdapter extends IntegrationObjectSupport im
     }
 
     protected Map<String, ?> doPoll() {
-        Map<String, Object> payload = this.executor.executeStoredProcedure();
-        return payload;
+        return this.executor.executeStoredProcedure();
     }
 
     public String getComponentType(){

spring-integration-jdbc/src/main/java/org/springframework/integration/jdbc/config/StoredProcParserUtils.java

@@ -43,7 +43,7 @@ import org.w3c.dom.Element;
  */
 public final class StoredProcParserUtils {
 
-	private static final Log logger = LogFactory
+	private static final Log LOGGER = LogFactory
 			.getLog(StoredProcParserUtils.class);
 
 	/** Prevent instantiation. */
@@ -134,12 +134,7 @@ public final class StoredProcParserUtils {
 
 			if (StringUtils.hasText(name)) {
 				parameterBuilder.addPropertyValue("name", name);
-			} else {
-				parserContext
-						.getReaderContext()
-						.error("The 'name' attribute must be set for the Stored Procedure parameter element.",
-								storedProcComponent);
-			}
+			} 
 
 			if (StringUtils.hasText(expression)) {
 				parameterBuilder.addPropertyValue("expression", expression);
@@ -148,9 +143,14 @@ public final class StoredProcParserUtils {
 			if (StringUtils.hasText(value)) {
 
 				if (!StringUtils.hasText(type)) {
-					logger.info(String
-							.format("Type attribute not set for Store Procedure parameter '%s'. Defaulting to 'java.lang.String'.",
-									value));
+					
+					if (LOGGER.isInfoEnabled()) {
+						LOGGER.info(String
+								.format("Type attribute not set for Store " 
+						              + "Procedure parameter '%s'. Defaulting to " 
+									  + "'java.lang.String'.", value));
+					}
+					
 					parameterBuilder.addPropertyValue("value",
 							new TypedStringValue(value, String.class));
 

spring-integration-jdbc/src/main/java/org/springframework/integration/jdbc/storedproc/ProcedureParameter.java

@@ -64,7 +64,7 @@ public class ProcedureParameter {
     public ProcedureParameter(String name, Object value, String expression) {
         super();
         
-        Assert.hasText(name, "Please provide a name.");
+        Assert.hasText(name, "'name' must not be empty.");
 
         this.name = name;
         this.value = value;
@@ -99,6 +99,10 @@ public class ProcedureParameter {
     	
     	Assert.notNull(procedureParameters, "The Collection of procedureParameters must not be null.");
     	
+    	for (ProcedureParameter parameter : procedureParameters) {
+    		Assert.notNull(parameter, "'procedureParameters' must not contain null values.");
+    	}
+    	
     	Map<String, String> staticParameters = new HashMap<String, String>();
     	
     	for (ProcedureParameter parameter : procedureParameters) {
@@ -119,6 +123,12 @@ public class ProcedureParameter {
 	 */
     public static Map<String, Object> convertStaticParameters(Collection<ProcedureParameter> procedureParameters) {
     	
+    	Assert.notNull(procedureParameters, "The Collection of procedureParameters must not be null.");
+    	
+    	for (ProcedureParameter parameter : procedureParameters) {
+    		Assert.notNull(parameter, "'procedureParameters' must not contain null values.");
+    	}
+    	
     	Map<String, Object> staticParameters = new HashMap<String, Object>();
     	
     	for (ProcedureParameter parameter : procedureParameters) {

spring-integration-jdbc/src/test/java/org/springframework/integration/jdbc/storedproc/ProcedureParameterTest.java

@@ -0,0 +1,93 @@
+package org.springframework.integration.jdbc.storedproc;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+
+import org.junit.Test;
+
+public class ProcedureParameterTest {
+
+	@Test
+	public void testProcedureParameterStringObjectString() {
+		
+		try {
+			new ProcedureParameter(null, "value", "expression");
+		} catch(IllegalArgumentException e) {
+			assertEquals("'name' must not be empty.", e.getMessage());
+			return;
+		}
+
+		fail("Expected Exception");
+	}
+
+	@Test
+	public void testConvertExpressions() {
+		List<ProcedureParameter> procedureParameters = getProcedureParameterList();
+		Map<String, String> expressionParameters = 
+				ProcedureParameter.convertExpressions(procedureParameters);
+		
+        assertTrue("Expected 2 expression parameters.", expressionParameters.size() == 2);
+	}
+
+	@Test
+	public void testConvertStaticParameters() {
+		
+		List<ProcedureParameter> procedureParameters = getProcedureParameterList();
+		Map<String, Object> staticParameters = 
+				ProcedureParameter.convertStaticParameters(procedureParameters);
+		
+        assertTrue("Expected 3 static parameters.", staticParameters.size() == 3);
+	}
+	
+	@Test
+	public void testConvertStaticParametersWithNullValueInList() {
+		
+		List<ProcedureParameter> procedureParameters = getProcedureParameterList();
+		procedureParameters.add(1, null);
+		
+		try {
+			ProcedureParameter.convertStaticParameters(procedureParameters);
+		} catch(IllegalArgumentException e) {
+			assertEquals("'procedureParameters' must not contain null values.", e.getMessage());
+			return;
+		}
+		
+		fail("Expected Exception");
+		
+	}
+	
+	@Test
+	public void testConvertExpressionParametersWithNullValueInList() {
+		
+		List<ProcedureParameter> procedureParameters = getProcedureParameterList();
+		procedureParameters.add(1, null);
+		
+		try {
+			ProcedureParameter.convertExpressions(procedureParameters);
+		} catch(IllegalArgumentException e) {
+			assertEquals("'procedureParameters' must not contain null values.", e.getMessage());
+			return;
+		}
+		
+		fail("Expected Exception");
+		
+	}
+	
+	private List<ProcedureParameter> getProcedureParameterList() {
+		
+		List<ProcedureParameter> procedureParameterList = new ArrayList<ProcedureParameter>();
+		procedureParameterList.add(new ProcedureParameter("param1", "value1", null));
+		procedureParameterList.add(new ProcedureParameter("param2", "value1", null));
+		procedureParameterList.add(new ProcedureParameter("param3", "value1", null));
+		procedureParameterList.add(new ProcedureParameter("param4", null, "expression1"));
+		procedureParameterList.add(new ProcedureParameter("param5", null, "expression2"));
+
+		return procedureParameterList;
+	}
+
+}