DataBinder unwraps Optional objects and allows for proper handling of Optional.empty()

Issue: SPR-12241

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

@@ -568,11 +568,10 @@ public class BeanWrapperImpl extends AbstractPropertyAccessor implements BeanWra
 		// Get value of bean property.
 		PropertyTokenHolder tokens = getPropertyNameTokens(nestedProperty);
 		String canonicalName = tokens.canonicalName;
-		Object propertyValue = getPropertyValue(tokens);
+		Object value = getPropertyValue(tokens);
-		if (propertyValue == null ||
+		if (value == null || (value.getClass().equals(javaUtilOptionalClass) && OptionalUnwrapper.isEmpty(value))) {
-				(propertyValue.getClass().equals(javaUtilOptionalClass) && OptionalUnwrapper.isEmpty(propertyValue))) {
 			if (isAutoGrowNestedPaths()) {
-				propertyValue = setDefaultValue(tokens);
+				value = setDefaultValue(tokens);
 			}
 			else {
 				throw new NullValueInNestedPathException(getRootClass(), this.nestedPath + canonicalName);
@@ -581,11 +580,12 @@ public class BeanWrapperImpl extends AbstractPropertyAccessor implements BeanWra
 
 		// Lookup cached sub-BeanWrapper, create new one if not found.
 		BeanWrapperImpl nestedBw = this.nestedBeanWrappers.get(canonicalName);
-		if (nestedBw == null || nestedBw.getWrappedInstance() != propertyValue) {
+		if (nestedBw == null || nestedBw.getWrappedInstance() !=
+				(value.getClass().equals(javaUtilOptionalClass) ? OptionalUnwrapper.unwrap(value) : value)) {
 			if (logger.isTraceEnabled()) {
 				logger.trace("Creating new nested BeanWrapper for property '" + canonicalName + "'");
 			}
-			nestedBw = newNestedBeanWrapper(propertyValue, this.nestedPath + canonicalName + NESTED_PROPERTY_SEPARATOR);
+			nestedBw = newNestedBeanWrapper(value, this.nestedPath + canonicalName + NESTED_PROPERTY_SEPARATOR);
 			// Inherit all type-specific PropertyEditors.
 			copyDefaultEditorsTo(nestedBw);
 			copyCustomEditorsTo(nestedBw, canonicalName);
@@ -1212,7 +1212,9 @@ public class BeanWrapperImpl extends AbstractPropertyAccessor implements BeanWra
 		public static Object unwrap(Object optionalObject) {
 			Optional<?> optional = (Optional<?>) optionalObject;
 			Assert.isTrue(optional.isPresent(), "Optional value must be present");
-			return optional.get();
+			Object result = optional.get();
+			Assert.isTrue(!(result instanceof Optional), "Multi-level Optional usage not supported");
+			return result;
 		}
 
 		public static boolean isEmpty(Object optionalObject) {

spring-context/src/main/java/org/springframework/validation/DataBinder.java

@@ -24,6 +24,7 @@ import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -41,7 +42,9 @@ import org.springframework.beans.TypeConverter;
 import org.springframework.beans.TypeMismatchException;
 import org.springframework.core.MethodParameter;
 import org.springframework.core.convert.ConversionService;
+import org.springframework.lang.UsesJava8;
 import org.springframework.util.Assert;
+import org.springframework.util.ClassUtils;
 import org.springframework.util.ObjectUtils;
 import org.springframework.util.PatternMatchUtils;
 import org.springframework.util.StringUtils;
@@ -119,6 +122,19 @@ public class DataBinder implements PropertyEditorRegistry, TypeConverter {
 	 */
 	protected static final Log logger = LogFactory.getLog(DataBinder.class);
 
+	private static Class<?> javaUtilOptionalClass = null;
+
+	static {
+		try {
+			javaUtilOptionalClass =
+					ClassUtils.forName("java.util.Optional", DataBinder.class.getClassLoader());
+		}
+		catch (ClassNotFoundException ex) {
+			// Java 8 not available - Optional references simply not supported then.
+		}
+	}
+
+
 	private final Object target;
 
 	private final String objectName;
@@ -165,7 +181,12 @@ public class DataBinder implements PropertyEditorRegistry, TypeConverter {
 	 * @param objectName the name of the target object
 	 */
 	public DataBinder(Object target, String objectName) {
-		this.target = target;
+		if (target != null && target.getClass().equals(javaUtilOptionalClass)) {
+			this.target = OptionalUnwrapper.unwrap(target);
+		}
+		else {
+			this.target = target;
+		}
 		this.objectName = objectName;
 	}
 
@@ -779,4 +800,22 @@ public class DataBinder implements PropertyEditorRegistry, TypeConverter {
 		return getBindingResult().getModel();
 	}
 
+
+	/**
+	 * Inner class to avoid a hard dependency on Java 8.
+	 */
+	@UsesJava8
+	private static class OptionalUnwrapper {
+
+		public static Object unwrap(Object optionalObject) {
+			Optional<?> optional = (Optional<?>) optionalObject;
+			if (!optional.isPresent()) {
+				return null;
+			}
+			Object result = optional.get();
+			Assert.isTrue(!(result instanceof Optional), "Multi-level Optional usage not supported");
+			return result;
+		}
+	}
+
 }

spring-web/src/main/java/org/springframework/web/method/annotation/ModelAttributeMethodProcessor.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2014 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.
@@ -21,6 +21,7 @@ import java.util.Map;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
+
 import org.springframework.beans.BeanUtils;
 import org.springframework.core.MethodParameter;
 import org.springframework.core.annotation.AnnotationUtils;
@@ -58,6 +59,7 @@ public class ModelAttributeMethodProcessor implements HandlerMethodArgumentResol
 
 	private final boolean annotationNotRequired;
 
+
 	/**
 	 * @param annotationNotRequired if "true", non-simple method arguments and
 	 * return values are considered model attributes with or without a
@@ -67,6 +69,7 @@ public class ModelAttributeMethodProcessor implements HandlerMethodArgumentResol
 		this.annotationNotRequired = annotationNotRequired;
 	}
 
+
 	/**
 	 * @return true if the parameter is annotated with {@link ModelAttribute}
 	 * or in default resolution mode also if it is not a simple type.
@@ -94,18 +97,16 @@ public class ModelAttributeMethodProcessor implements HandlerMethodArgumentResol
 	 * @throws Exception if WebDataBinder initialization fails.
 	 */
 	@Override
-	public final Object resolveArgument(
+	public final Object resolveArgument(MethodParameter parameter, ModelAndViewContainer mavContainer,
-			MethodParameter parameter, ModelAndViewContainer mavContainer,
+			NativeWebRequest webRequest, WebDataBinderFactory binderFactory) throws Exception {
-			NativeWebRequest request, WebDataBinderFactory binderFactory)
-			throws Exception {
 
 		String name = ModelFactory.getNameForParameter(parameter);
-		Object attribute = (mavContainer.containsAttribute(name)) ?
+		Object attribute = (mavContainer.containsAttribute(name) ?
-				mavContainer.getModel().get(name) : createAttribute(name, parameter, binderFactory, request);
+				mavContainer.getModel().get(name) : createAttribute(name, parameter, binderFactory, webRequest));
 
-		WebDataBinder binder = binderFactory.createBinder(request, attribute, name);
+		WebDataBinder binder = binderFactory.createBinder(webRequest, attribute, name);
 		if (binder.getTarget() != null) {
-			bindRequestParameters(binder, request);
+			bindRequestParameters(binder, webRequest);
 			validateIfApplicable(binder, parameter);
 			if (binder.getBindingResult().hasErrors()) {
 				if (isBindExceptionRequired(binder, parameter)) {
@@ -120,17 +121,17 @@ public class ModelAttributeMethodProcessor implements HandlerMethodArgumentResol
 		mavContainer.removeAttributes(bindingResultModel);
 		mavContainer.addAllAttributes(bindingResultModel);
 
-		return binder.getTarget();
+		return binder.convertIfNecessary(binder.getTarget(), parameter.getParameterType(), parameter);
 	}
 
 	/**
 	 * Extension point to create the model attribute if not found in the model.
 	 * The default implementation uses the default constructor.
-	 * @param attributeName the name of the attribute, never {@code null}
+	 * @param attributeName the name of the attribute (never {@code null})
 	 * @param parameter the method parameter
 	 * @param binderFactory for creating WebDataBinder instance
 	 * @param request the current request
-	 * @return the created model attribute, never {@code null}
+	 * @return the created model attribute (never {@code null})
 	 */
 	protected Object createAttribute(String attributeName, MethodParameter parameter,
 			WebDataBinderFactory binderFactory,  NativeWebRequest request) throws Exception {
@@ -155,9 +156,9 @@ public class ModelAttributeMethodProcessor implements HandlerMethodArgumentResol
 	 */
 	protected void validateIfApplicable(WebDataBinder binder, MethodParameter parameter) {
 		Annotation[] annotations = parameter.getParameterAnnotations();
-		for (Annotation annot : annotations) {
+		for (Annotation ann : annotations) {
-			if (annot.annotationType().getSimpleName().startsWith("Valid")) {
+			if (ann.annotationType().getSimpleName().startsWith("Valid")) {
-				Object hints = AnnotationUtils.getValue(annot);
+				Object hints = AnnotationUtils.getValue(ann);
 				binder.validate(hints instanceof Object[] ? (Object[]) hints : new Object[] {hints});
 				break;
 			}
@@ -199,14 +200,13 @@ public class ModelAttributeMethodProcessor implements HandlerMethodArgumentResol
 	 * Add non-null return values to the {@link ModelAndViewContainer}.
 	 */
 	@Override
-	public void handleReturnValue(
+	public void handleReturnValue(Object returnValue, MethodParameter returnType,
-			Object returnValue, MethodParameter returnType,
+			ModelAndViewContainer mavContainer, NativeWebRequest webRequest) throws Exception {
-			ModelAndViewContainer mavContainer, NativeWebRequest webRequest)
-			throws Exception {
 
 		if (returnValue != null) {
 			String name = ModelFactory.getNameForReturnValue(returnValue, returnType);
 			mavContainer.addAttribute(name, returnValue);
 		}
 	}
+
 }

spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletModelAttributeMethodProcessorTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2013 the original author or authors.
+ * Copyright 2002-2014 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.
@@ -16,19 +16,18 @@
 
 package org.springframework.web.servlet.mvc.method.annotation;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
-
 import java.lang.reflect.Method;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.Optional;
 
 import org.junit.Before;
 import org.junit.Test;
-import org.springframework.tests.sample.beans.TestBean;
+
 import org.springframework.core.MethodParameter;
 import org.springframework.core.convert.support.DefaultConversionService;
 import org.springframework.mock.web.test.MockHttpServletRequest;
+import org.springframework.tests.sample.beans.TestBean;
 import org.springframework.web.bind.annotation.ModelAttribute;
 import org.springframework.web.bind.support.ConfigurableWebBindingInitializer;
 import org.springframework.web.bind.support.WebDataBinderFactory;
@@ -37,13 +36,15 @@ import org.springframework.web.context.request.ServletWebRequest;
 import org.springframework.web.method.support.ModelAndViewContainer;
 import org.springframework.web.servlet.HandlerMapping;
 
+import static org.junit.Assert.*;
+
 /**
  * Test fixture for {@link ServletModelAttributeMethodProcessor} specific tests.
  * Also see org.springframework.web.method.annotation.support.ModelAttributeMethodProcessorTests
  *
  * @author Rossen Stoyanchev
  */
-public class SerlvetModelAttributeMethodProcessorTests {
+public class ServletModelAttributeMethodProcessorTests {
 
 	private ServletModelAttributeMethodProcessor processor;
 
@@ -51,6 +52,8 @@ public class SerlvetModelAttributeMethodProcessorTests {
 
 	private MethodParameter testBeanWithoutStringConstructorModelAttr;
 
+	private MethodParameter testBeanWithOptionalModelAttr;
+
 	private ModelAndViewContainer mavContainer;
 
 	private NativeWebRequest webRequest;
@@ -59,26 +62,29 @@ public class SerlvetModelAttributeMethodProcessorTests {
 
 	private WebDataBinderFactory binderFactory;
 
+
 	@Before
 	public void setUp() throws Exception {
 		this.processor = new ServletModelAttributeMethodProcessor(false);
 
 		Method method = getClass().getDeclaredMethod("modelAttribute",
-				TestBean.class, TestBeanWithoutStringConstructor.class);
+				TestBean.class, TestBeanWithoutStringConstructor.class, Optional.class);
 
 		this.testBeanModelAttr = new MethodParameter(method, 0);
 		this.testBeanWithoutStringConstructorModelAttr = new MethodParameter(method, 1);
+		this.testBeanWithOptionalModelAttr = new MethodParameter(method, 2);
 
 		ConfigurableWebBindingInitializer initializer = new ConfigurableWebBindingInitializer();
 		initializer.setConversionService(new DefaultConversionService());
 
-		this.binderFactory = new ServletRequestDataBinderFactory(null, initializer );
+		this.binderFactory = new ServletRequestDataBinderFactory(null, initializer);
 		this.mavContainer = new ModelAndViewContainer();
 
 		this.request = new MockHttpServletRequest();
 		this.webRequest = new ServletWebRequest(request);
 	}
 
+
 	@Test
 	public void createAttributeUriTemplateVar() throws Exception {
 		Map<String, String> uriTemplateVars = new HashMap<String, String>();
@@ -107,6 +113,21 @@ public class SerlvetModelAttributeMethodProcessorTests {
 		assertNotNull(testBean);
 	}
 
+	@Test
+	public void createAttributeUriTemplateVarWithOptional() throws Exception {
+		Map<String, String> uriTemplateVars = new HashMap<String, String>();
+		uriTemplateVars.put("testBean3", "Patty");
+		this.request.setAttribute(HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE, uriTemplateVars);
+
+		// Type conversion from "Patty" to TestBean via TestBean(String) constructor
+
+		Optional<TestBean> testBean =
+				(Optional<TestBean>) this.processor.resolveArgument(
+						this.testBeanWithOptionalModelAttr, this.mavContainer, this.webRequest, this.binderFactory);
+
+		assertEquals("Patty", testBean.get().getName());
+	}
+
 	@Test
 	public void createAttributeRequestParameter() throws Exception {
 		this.request.addParameter("testBean1", "Patty");
@@ -122,7 +143,7 @@ public class SerlvetModelAttributeMethodProcessorTests {
 
 	@Test
 	public void createAttributeRequestParameterCannotConvert() throws Exception {
-		this.request.addParameter("testBean1", "Patty");
+		this.request.addParameter("testBean2", "Patty");
 
 		TestBeanWithoutStringConstructor testBean =
 			(TestBeanWithoutStringConstructor) this.processor.resolveArgument(
@@ -131,10 +152,62 @@ public class SerlvetModelAttributeMethodProcessorTests {
 		assertNotNull(testBean);
 	}
 
+	@Test
+	public void createAttributeRequestParameterWithOptional() throws Exception {
+		this.request.addParameter("testBean3", "Patty");
+
+		Optional<TestBean> testBean =
+				(Optional<TestBean>) this.processor.resolveArgument(
+						this.testBeanWithOptionalModelAttr, this.mavContainer, this.webRequest, this.binderFactory);
+
+		assertEquals("Patty", testBean.get().getName());
+	}
+
+	@Test
+	public void attributesAsNullValues() throws Exception {
+		this.request.addParameter("name", "Patty");
+
+		this.mavContainer.getModel().put("testBean1", null);
+		this.mavContainer.getModel().put("testBean2", null);
+		this.mavContainer.getModel().put("testBean3", null);
+
+		assertNull(this.processor.resolveArgument(
+				this.testBeanModelAttr, this.mavContainer, this.webRequest, this.binderFactory));
+
+		assertNull(this.processor.resolveArgument(
+				this.testBeanWithoutStringConstructorModelAttr, this.mavContainer, this.webRequest, this.binderFactory));
+
+		Optional<TestBean> testBean =
+				(Optional<TestBean>) this.processor.resolveArgument(
+						this.testBeanWithOptionalModelAttr, this.mavContainer, this.webRequest, this.binderFactory);
+		assertFalse(testBean.isPresent());
+	}
+
+	@Test
+	public void attributesAsOptionalEmpty() throws Exception {
+		this.request.addParameter("name", "Patty");
+
+		this.mavContainer.getModel().put("testBean1", Optional.empty());
+		this.mavContainer.getModel().put("testBean2", Optional.empty());
+		this.mavContainer.getModel().put("testBean3", Optional.empty());
+
+		assertNull(this.processor.resolveArgument(
+				this.testBeanModelAttr, this.mavContainer, this.webRequest, this.binderFactory));
+
+		assertNull(this.processor.resolveArgument(
+				this.testBeanWithoutStringConstructorModelAttr, this.mavContainer, this.webRequest, this.binderFactory));
+
+		Optional<TestBean> testBean =
+				(Optional<TestBean>) this.processor.resolveArgument(
+						this.testBeanWithOptionalModelAttr, this.mavContainer, this.webRequest, this.binderFactory);
+		assertFalse(testBean.isPresent());
+	}
+
 
 	@SuppressWarnings("unused")
 	private void modelAttribute(@ModelAttribute("testBean1") TestBean testBean1,
-								@ModelAttribute("testBean2") TestBeanWithoutStringConstructor testBean2) {
+								@ModelAttribute("testBean2") TestBeanWithoutStringConstructor testBean2,
+								@ModelAttribute("testBean3") Optional<TestBean> testBean3) {
 	}