SPR-5732 - When no type conversion strategy is found on a @Controller handler...

SPR-5732 - When no type conversion strategy is found on a @Controller handler method bind target, a 500 error code should be returned not a 400.

org.springframework.beans/src/main/java/org/springframework/beans/BeanWrapperImpl.java

@@ -355,6 +355,8 @@ public class BeanWrapperImpl extends AbstractPropertyAccessor implements BeanWra
 		}
 		catch (IllegalArgumentException ex) {
 			throw new TypeMismatchException(value, requiredType, ex);
+		} catch (IllegalStateException ex) {
+			throw new ConversionNotSupportedException(value, requiredType, ex);
 		}
 	}
 
@@ -382,6 +384,11 @@ public class BeanWrapperImpl extends AbstractPropertyAccessor implements BeanWra
 					new PropertyChangeEvent(this.rootObject, this.nestedPath + propertyName, null, value);
 			throw new TypeMismatchException(pce, pd.getPropertyType(), ex);
 		}
+		catch (IllegalStateException ex) {
+			PropertyChangeEvent pce =
+					new PropertyChangeEvent(this.rootObject, this.nestedPath + propertyName, null, value);
+			throw new ConversionNotSupportedException(pce, pd.getPropertyType(), ex);
+		}
 	}
 
 
@@ -840,6 +847,11 @@ public class BeanWrapperImpl extends AbstractPropertyAccessor implements BeanWra
 						new PropertyChangeEvent(this.rootObject, this.nestedPath + propertyName, oldValue, pv.getValue());
 				throw new TypeMismatchException(pce, pd.getPropertyType(), ex);
 			}
+			catch (IllegalStateException ex) {
+				PropertyChangeEvent pce =
+						new PropertyChangeEvent(this.rootObject, this.nestedPath + propertyName, oldValue, pv.getValue());
+				throw new ConversionNotSupportedException(pce, pd.getPropertyType(), ex);
+			}
 			catch (IllegalAccessException ex) {
 				PropertyChangeEvent pce =
 						new PropertyChangeEvent(this.rootObject, this.nestedPath + propertyName, oldValue, pv.getValue());

org.springframework.beans/src/main/java/org/springframework/beans/ConversionNotSupportedException.java

+/*
+ * Copyright 2002-2009 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.beans;
+
+import java.beans.PropertyChangeEvent;
+
+/**
+ * Exception thrown when no suitable editor can be found to set a bean property.
+ *
+ * @author Arjen Poutsma
+ * @since 3.0
+ */
+public class ConversionNotSupportedException extends TypeMismatchException {
+
+	public ConversionNotSupportedException(PropertyChangeEvent propertyChangeEvent, Class requiredType) {
+		super(propertyChangeEvent, requiredType);
+	}
+
+	public ConversionNotSupportedException(PropertyChangeEvent propertyChangeEvent, Class requiredType, Throwable cause) {
+		super(propertyChangeEvent, requiredType, cause);
+	}
+
+	public ConversionNotSupportedException(Object value, Class requiredType) {
+		super(value, requiredType);
+	}
+
+	public ConversionNotSupportedException(Object value, Class requiredType, Throwable cause) {
+		super(value, requiredType, cause);
+	}
+}

org.springframework.beans/src/main/java/org/springframework/beans/DirectFieldAccessor.java

 /*
- * Copyright 2002-2008 the original author or authors.
+ * Copyright 2002-2009 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -124,6 +124,10 @@ public class DirectFieldAccessor extends AbstractPropertyAccessor {
 			PropertyChangeEvent pce = new PropertyChangeEvent(this.target, propertyName, oldValue, newValue);
 			throw new TypeMismatchException(pce, field.getType(), ex);
 		}
+		catch (IllegalStateException ex) {
+			PropertyChangeEvent pce = new PropertyChangeEvent(this.target, propertyName, oldValue, newValue);
+			throw new ConversionNotSupportedException(pce, field.getType(), ex);
+		}
 	}
 
 	public Object convertIfNecessary(
@@ -134,6 +138,9 @@ public class DirectFieldAccessor extends AbstractPropertyAccessor {
 		catch (IllegalArgumentException ex) {
 			throw new TypeMismatchException(value, requiredType, ex);
 		}
+		catch (IllegalStateException ex) {
+			throw new ConversionNotSupportedException(value, requiredType, ex);
+		}
 	}
 
 }

org.springframework.beans/src/main/java/org/springframework/beans/SimpleTypeConverter.java

 /*
- * Copyright 2002-2006 the original author or authors.
+ * Copyright 2002-2009 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -49,6 +49,9 @@ public class SimpleTypeConverter extends PropertyEditorRegistrySupport implement
 		catch (IllegalArgumentException ex) {
 			throw new TypeMismatchException(value, requiredType, ex);
 		}
+		catch (IllegalStateException ex) {
+			throw new ConversionNotSupportedException(value, requiredType, ex);
+		}
 	}
 
 }

org.springframework.beans/src/main/java/org/springframework/beans/TypeConverterDelegate.java

 /*
- * Copyright 2002-2008 the original author or authors.
+ * Copyright 2002-2009 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -209,7 +209,7 @@ class TypeConverterDelegate {
 			}
 
 			if (!ClassUtils.isAssignableValue(requiredType, convertedValue)) {
-				// Definitely doesn't match: throw IllegalArgumentException.
+				// Definitely doesn't match: throw IllegalArgumentException/IllegalStateException
 				StringBuilder msg = new StringBuilder();
 				msg.append("Cannot convert value of type [").append(ClassUtils.getDescriptiveType(newValue));
 				msg.append("] to required type [").append(ClassUtils.getQualifiedName(requiredType)).append("]");
@@ -219,11 +219,12 @@ class TypeConverterDelegate {
 				if (editor != null) {
 					msg.append(": PropertyEditor [").append(editor.getClass().getName()).append(
 							"] returned inappropriate value");
+					throw new IllegalArgumentException(msg.toString());
 				}
 				else {
 					msg.append(": no matching editors or conversion strategy found");
+					throw new IllegalStateException(msg.toString());
 				}
-				throw new IllegalArgumentException(msg.toString());
 			}
 		}
 

org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/support/DefaultHandlerExceptionResolver.java

@@ -23,6 +23,7 @@ import javax.servlet.http.HttpServletResponse;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
+import org.springframework.beans.ConversionNotSupportedException;
 import org.springframework.beans.TypeMismatchException;
 import org.springframework.core.Ordered;
 import org.springframework.http.MediaType;
@@ -95,6 +96,9 @@ public class DefaultHandlerExceptionResolver extends AbstractHandlerExceptionRes
 				return handleMissingServletRequestParameter((MissingServletRequestParameterException) ex, request,
 						response, handler);
 			}
+			else if (ex instanceof ConversionNotSupportedException) {
+				return handleConversionNotSupported((ConversionNotSupportedException) ex, request, response, handler);
+			}
 			else if (ex instanceof TypeMismatchException) {
 				return handleTypeMismatch((TypeMismatchException) ex, request, response, handler);
 			}
@@ -211,6 +215,27 @@ public class DefaultHandlerExceptionResolver extends AbstractHandlerExceptionRes
 		return new ModelAndView();
 	}
 
+	/**
+	 * Handle the case when a {@link org.springframework.web.bind.WebDataBinder} conversion cannot occur. <p>The default
+	 * implementation sends an HTTP 500 error, and returns an empty {@code ModelAndView}. Alternatively, a fallback view
+	 * could be chosen, or the TypeMismatchException could be rethrown as-is.
+	 *
+	 * @param ex the ConversionNotSupportedException to be handled
+	 * @param request current HTTP request
+	 * @param response current HTTP response
+	 * @param handler the executed handler, or <code>null</code> if none chosen at the time of the exception (for example,
+	 * if multipart resolution failed)
+	 * @return a ModelAndView to render, or <code>null</code> if handled directly
+	 * @throws Exception an Exception that should be thrown as result of the servlet request
+	 */
+	protected ModelAndView handleConversionNotSupported(ConversionNotSupportedException ex,
+			HttpServletRequest request,
+			HttpServletResponse response,
+			Object handler) throws Exception {
+		response.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
+		return new ModelAndView();
+	}
+
 	/**
 	 * Handle the case when a {@link org.springframework.web.bind.WebDataBinder} conversion error occurs. <p>The default
 	 * implementation sends an HTTP 400 error, and returns an empty {@code ModelAndView}. Alternatively, a fallback view
@@ -228,7 +253,6 @@ public class DefaultHandlerExceptionResolver extends AbstractHandlerExceptionRes
 			HttpServletRequest request,
 			HttpServletResponse response,
 			Object handler) throws Exception {
-
 		response.sendError(HttpServletResponse.SC_BAD_REQUEST);
 		return new ModelAndView();
 	}

org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/annotation/ServletAnnotationControllerTests.java

@@ -17,8 +17,8 @@
 package org.springframework.web.servlet.mvc.annotation;
 
 import java.io.IOException;
-import java.io.Writer;
 import java.io.Serializable;
+import java.io.Writer;
 import java.lang.annotation.ElementType;
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
@@ -106,6 +106,7 @@ import org.springframework.web.util.NestedServletException;
 /**
  * @author Juergen Hoeller
  * @author Sam Brannen
+ * @author Arjen Poutsma
  * @since 2.5
  */
 public class ServletAnnotationControllerTests {

org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/annotation/UriTemplateServletAnnotationControllerTests.java

@@ -74,7 +74,18 @@ public class UriTemplateServletAnnotationControllerTests {
 		MockHttpServletRequest request = new MockHttpServletRequest("GET", "/hotels/42/dates/2008-11-18");
 		MockHttpServletResponse response = new MockHttpServletResponse();
 		servlet.service(request, response);
-		assertEquals("test-42", response.getContentAsString());
+		assertEquals(200, response.getStatus());
+
+		request = new MockHttpServletRequest("GET", "/hotels/42/dates/2008-foo-bar");
+		response = new MockHttpServletResponse();
+		servlet.service(request, response);
+		assertEquals(400, response.getStatus());
+
+		initServlet(NonBindingUriTemplateController.class);
+		request = new MockHttpServletRequest("GET", "/hotels/42/dates/2008-foo-bar");
+		response = new MockHttpServletResponse();
+		servlet.service(request, response);
+		assertEquals(500, response.getStatus());
 	}
 
 	@Test
@@ -276,6 +287,16 @@ public class UriTemplateServletAnnotationControllerTests {
 
 	}
 
+	@Controller
+	public static class NonBindingUriTemplateController {
+
+		@RequestMapping("/hotels/{hotel}/dates/{date}")
+		public void handle(@PathVariable("hotel") String hotel, @PathVariable Date date, Writer writer)
+				throws IOException {
+		}
+
+	}
+
 	@Controller
 	@RequestMapping("/hotels/{hotel}")
 	public static class RelativePathUriTemplateController {