From 0dc6082b01606c3c996b728541467ba5104b747f Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Thu, 12 Jun 2014 23:50:26 -0400 Subject: [PATCH] Support java.util.Optional for @MVC named value args After this change, java.util.Optional is supported with @RequestParam, @RequestHeader, and @MatrixVariable arguments in Java 8. When Optional is used the required flag is effectively ignored. Issue: SPR-11829 --- .../beans/TypeConverterDelegate.java | 16 ++++ .../support/DefaultConversionService.java | 7 ++ .../support/GenericConversionService.java | 19 ++++- .../support/ObjectToOptionalConverter.java | 81 +++++++++++++++++++ .../support/DefaultConversionTests.java | 23 ++++++ .../RequestHeaderMethodArgumentResolver.java | 9 ++- .../RequestParamMethodArgumentResolver.java | 4 +- ...questParamMethodArgumentResolverTests.java | 28 ++++++- .../MatrixVariableMethodArgumentResolver.java | 8 +- 9 files changed, 185 insertions(+), 10 deletions(-) create mode 100644 spring-core/src/main/java/org/springframework/core/convert/support/ObjectToOptionalConverter.java diff --git a/spring-beans/src/main/java/org/springframework/beans/TypeConverterDelegate.java b/spring-beans/src/main/java/org/springframework/beans/TypeConverterDelegate.java index d8b3eda2fd1..1bb2c088c15 100644 --- a/spring-beans/src/main/java/org/springframework/beans/TypeConverterDelegate.java +++ b/spring-beans/src/main/java/org/springframework/beans/TypeConverterDelegate.java @@ -53,6 +53,19 @@ class TypeConverterDelegate { private static final Log logger = LogFactory.getLog(TypeConverterDelegate.class); + /** Java 8's java.util.Optional.empty() instance */ + private static Object javaUtilOptionalEmpty = null; + + static { + try { + Class clazz = ClassUtils.forName("java.util.Optional", TypeConverterDelegate.class.getClassLoader()); + javaUtilOptionalEmpty = ClassUtils.getMethod(clazz, "empty").invoke(null); + } catch (Exception ex) { + // Java 8 not available - conversion to Optional not supported then. + } + } + + private final PropertyEditorRegistrySupport propertyEditorRegistry; private final Object targetObject; @@ -244,6 +257,9 @@ class TypeConverterDelegate { standardConversion = true; } } + else if (requiredType.equals(javaUtilOptionalEmpty.getClass())) { + convertedValue = javaUtilOptionalEmpty; + } if (!ClassUtils.isAssignableValue(requiredType, convertedValue)) { if (firstAttemptEx != null) { diff --git a/spring-core/src/main/java/org/springframework/core/convert/support/DefaultConversionService.java b/spring-core/src/main/java/org/springframework/core/convert/support/DefaultConversionService.java index b9538a7d23b..2b4007bf190 100644 --- a/spring-core/src/main/java/org/springframework/core/convert/support/DefaultConversionService.java +++ b/spring-core/src/main/java/org/springframework/core/convert/support/DefaultConversionService.java @@ -37,6 +37,10 @@ import org.springframework.util.ClassUtils; */ public class DefaultConversionService extends GenericConversionService { + /** Java 8's java.util.Optional class available? */ + private static final boolean javaUtilOptionalClassAvailable = + ClassUtils.isPresent("java.util.Optional", DefaultConversionService.class.getClassLoader()); + /** Java 8's java.time package available? */ private static final boolean jsr310Available = ClassUtils.isPresent("java.time.ZoneId", DefaultConversionService.class.getClassLoader()); @@ -71,6 +75,9 @@ public class DefaultConversionService extends GenericConversionService { converterRegistry.addConverter(new ObjectToObjectConverter()); converterRegistry.addConverter(new IdToEntityConverter((ConversionService) converterRegistry)); converterRegistry.addConverter(new FallbackObjectToStringConverter()); + if (javaUtilOptionalClassAvailable) { + converterRegistry.addConverter(new ObjectToOptionalConverter((ConversionService) converterRegistry)); + } } // internal helpers diff --git a/spring-core/src/main/java/org/springframework/core/convert/support/GenericConversionService.java b/spring-core/src/main/java/org/springframework/core/convert/support/GenericConversionService.java index d96cee5d817..a26e2805d16 100644 --- a/spring-core/src/main/java/org/springframework/core/convert/support/GenericConversionService.java +++ b/spring-core/src/main/java/org/springframework/core/convert/support/GenericConversionService.java @@ -59,6 +59,9 @@ import org.springframework.util.StringUtils; */ public class GenericConversionService implements ConfigurableConversionService { + /** Java 8's java.util.Optional.empty() */ + private static Object javaUtilOptionalEmpty = null; + /** * General NO-OP converter used when conversion is not required. */ @@ -70,6 +73,15 @@ public class GenericConversionService implements ConfigurableConversionService { */ private static final GenericConverter NO_MATCH = new NoOpConverter("NO_MATCH"); + static { + try { + Class clazz = ClassUtils.forName("java.util.Optional", GenericConversionService.class.getClassLoader()); + javaUtilOptionalEmpty = ClassUtils.getMethod(clazz, "empty").invoke(null); + } catch (Exception ex) { + // Java 8 not available - conversion to Optional not supported then. + } + } + private final Converters converters = new Converters(); @@ -204,13 +216,18 @@ public class GenericConversionService implements ConfigurableConversionService { /** * Template method to convert a null source. - *

Default implementation returns {@code null}. + *

Default implementation returns {@code null} or the Java 8 + * {@link java.util.Optional#empty()} instance if the target type is + * {@code java.uti.Optional}. * Subclasses may override to return custom null objects for specific target types. * @param sourceType the sourceType to convert from * @param targetType the targetType to convert to * @return the converted null object */ protected Object convertNullSource(TypeDescriptor sourceType, TypeDescriptor targetType) { + if (targetType.getObjectType().equals(javaUtilOptionalEmpty.getClass())) { + return javaUtilOptionalEmpty; + } return null; } diff --git a/spring-core/src/main/java/org/springframework/core/convert/support/ObjectToOptionalConverter.java b/spring-core/src/main/java/org/springframework/core/convert/support/ObjectToOptionalConverter.java new file mode 100644 index 00000000000..0f7c33d6afc --- /dev/null +++ b/spring-core/src/main/java/org/springframework/core/convert/support/ObjectToOptionalConverter.java @@ -0,0 +1,81 @@ +/* + * 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. + * 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.core.convert.support; + +import org.springframework.core.convert.ConversionService; +import org.springframework.core.convert.TypeDescriptor; +import org.springframework.core.convert.converter.ConditionalGenericConverter; + +import java.util.Collections; +import java.util.Optional; +import java.util.Set; + +/** + * Convert an Object to {@code java.util.Optional} if necessary using the + * {@code ConversionService} to convert the source Object to the generic type + * of Optional when known. + * + * @author Rossen Stoyanchev + * @since 4.1 + */ +final class ObjectToOptionalConverter implements ConditionalGenericConverter { + + private final ConversionService conversionService; + + + public ObjectToOptionalConverter(ConversionService conversionService) { + this.conversionService = conversionService; + } + + @Override + public Set getConvertibleTypes() { + return Collections.singleton(new ConvertiblePair(Object.class, Optional.class)); + } + + @Override + public boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType) { + if (targetType.getResolvableType() != null) { + return this.conversionService.canConvert(sourceType, new GenericTypeDescriptor(targetType)); + } + else { + return true; + } + } + + @Override + public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor targetType) { + if (source == null) { + return Optional.empty(); + } + if (targetType.getResolvableType() == null) { + return Optional.of(source); + } + else { + Object target = this.conversionService.convert(source, sourceType, new GenericTypeDescriptor(targetType)); + return Optional.of(target); + } + } + + + @SuppressWarnings("serial") + private static class GenericTypeDescriptor extends TypeDescriptor { + + public GenericTypeDescriptor(TypeDescriptor typeDescriptor) { + super(typeDescriptor.getResolvableType().getGeneric(0), null, typeDescriptor.getAnnotations()); + } + } +} diff --git a/spring-core/src/test/java/org/springframework/core/convert/support/DefaultConversionTests.java b/spring-core/src/test/java/org/springframework/core/convert/support/DefaultConversionTests.java index a832022a782..d932aa2eba0 100644 --- a/spring-core/src/test/java/org/springframework/core/convert/support/DefaultConversionTests.java +++ b/spring-core/src/test/java/org/springframework/core/convert/support/DefaultConversionTests.java @@ -17,6 +17,7 @@ package org.springframework.core.convert.support; import java.awt.Color; +import java.lang.reflect.Method; import java.math.BigDecimal; import java.math.BigInteger; import java.time.ZoneId; @@ -32,6 +33,7 @@ import java.util.LinkedList; import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.Optional; import java.util.Properties; import java.util.Set; @@ -43,6 +45,7 @@ import org.springframework.core.convert.ConverterNotFoundException; import org.springframework.core.convert.TypeDescriptor; import org.springframework.core.convert.converter.Converter; import org.springframework.core.convert.converter.ConverterRegistry; +import org.springframework.util.ClassUtils; import static org.hamcrest.Matchers.*; import static org.junit.Assert.*; @@ -787,6 +790,23 @@ public class DefaultConversionTests { assertArrayEquals(grid, convertedBack); } + @Test + @SuppressWarnings("unchecked") + public void convertObjectToOptional() { + Method method = ClassUtils.getMethod(TestEntity.class, "handleOptionalValue", Optional.class); + MethodParameter parameter = new MethodParameter(method, 0); + TypeDescriptor descriptor = new TypeDescriptor(parameter); + Object actual = conversionService.convert("1,2,3", TypeDescriptor.valueOf(String.class), descriptor); + assertEquals(Optional.class, actual.getClass()); + assertEquals(Arrays.asList(1,2,3), ((Optional>) actual).get()); + } + + @Test + public void convertObjectToOptionalNull() { + assertSame(Optional.empty(), conversionService.convert(null, TypeDescriptor.valueOf(Object.class), TypeDescriptor.valueOf(Optional.class))); + assertSame(Optional.empty(), conversionService.convert(null, Optional.class)); + } + public static class TestEntity { @@ -803,6 +823,9 @@ public class DefaultConversionTests { public static TestEntity findTestEntity(Long id) { return new TestEntity(id); } + + public void handleOptionalValue(Optional> value) { + } } diff --git a/spring-web/src/main/java/org/springframework/web/method/annotation/RequestHeaderMethodArgumentResolver.java b/spring-web/src/main/java/org/springframework/web/method/annotation/RequestHeaderMethodArgumentResolver.java index 70067715ac8..1aab9adcd62 100644 --- a/spring-web/src/main/java/org/springframework/web/method/annotation/RequestHeaderMethodArgumentResolver.java +++ b/spring-web/src/main/java/org/springframework/web/method/annotation/RequestHeaderMethodArgumentResolver.java @@ -77,9 +77,12 @@ public class RequestHeaderMethodArgumentResolver extends AbstractNamedValueMetho @Override protected void handleMissingValue(String headerName, MethodParameter param) throws ServletRequestBindingException { - String paramType = param.getParameterType().getName(); - throw new ServletRequestBindingException( - "Missing header '" + headerName + "' for method parameter type [" + paramType + "]"); + Class paramType = param.getParameterType(); + if (!paramType.getName().equals("java.util.Optional")) { + throw new ServletRequestBindingException( + "Missing header '" + headerName + "' for method parameter type [" + paramType.getName() + "]"); + + } } private static class RequestHeaderNamedValueInfo extends NamedValueInfo { diff --git a/spring-web/src/main/java/org/springframework/web/method/annotation/RequestParamMethodArgumentResolver.java b/spring-web/src/main/java/org/springframework/web/method/annotation/RequestParamMethodArgumentResolver.java index f763d1288ca..f881b095e48 100644 --- a/spring-web/src/main/java/org/springframework/web/method/annotation/RequestParamMethodArgumentResolver.java +++ b/spring-web/src/main/java/org/springframework/web/method/annotation/RequestParamMethodArgumentResolver.java @@ -252,7 +252,9 @@ public class RequestParamMethodArgumentResolver extends AbstractNamedValueMethod @Override protected void handleMissingValue(String paramName, MethodParameter parameter) throws ServletException { - throw new MissingServletRequestParameterException(paramName, parameter.getParameterType().getSimpleName()); + if (!parameter.getParameterType().getName().equals("java.util.Optional")) { + throw new MissingServletRequestParameterException(paramName, parameter.getParameterType().getSimpleName()); + } } @Override diff --git a/spring-web/src/test/java/org/springframework/web/method/annotation/RequestParamMethodArgumentResolverTests.java b/spring-web/src/test/java/org/springframework/web/method/annotation/RequestParamMethodArgumentResolverTests.java index 2619c51fd0e..236e8b36522 100644 --- a/spring-web/src/test/java/org/springframework/web/method/annotation/RequestParamMethodArgumentResolverTests.java +++ b/spring-web/src/test/java/org/springframework/web/method/annotation/RequestParamMethodArgumentResolverTests.java @@ -20,8 +20,10 @@ import java.lang.reflect.Method; import java.util.Arrays; import java.util.List; import java.util.Map; +import java.util.Optional; import javax.servlet.http.Part; +import javax.swing.text.html.Option; import org.junit.Before; import org.junit.Test; @@ -29,6 +31,7 @@ import org.springframework.beans.propertyeditors.StringTrimmerEditor; import org.springframework.core.LocalVariableTableParameterNameDiscoverer; import org.springframework.core.MethodParameter; import org.springframework.core.ParameterNameDiscoverer; +import org.springframework.core.convert.support.DefaultConversionService; import org.springframework.mock.web.test.MockHttpServletRequest; import org.springframework.mock.web.test.MockHttpServletResponse; import org.springframework.mock.web.test.MockMultipartFile; @@ -38,6 +41,8 @@ import org.springframework.web.bind.MissingServletRequestParameterException; import org.springframework.web.bind.WebDataBinder; import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.bind.annotation.RequestPart; +import org.springframework.web.bind.support.ConfigurableWebBindingInitializer; +import org.springframework.web.bind.support.DefaultDataBinderFactory; import org.springframework.web.bind.support.WebDataBinderFactory; import org.springframework.web.bind.support.WebRequestDataBinder; import org.springframework.web.context.request.NativeWebRequest; @@ -76,6 +81,7 @@ public class RequestParamMethodArgumentResolverTests { private MethodParameter paramRequestPartAnnot; private MethodParameter paramRequired; private MethodParameter paramNotRequired; + private MethodParameter paramOptional; private NativeWebRequest webRequest; @@ -91,7 +97,7 @@ public class RequestParamMethodArgumentResolverTests { Map.class, MultipartFile.class, List.class, MultipartFile[].class, Part.class, List.class, Part[].class, Map.class, String.class, MultipartFile.class, List.class, Part.class, - MultipartFile.class, String.class, String.class); + MultipartFile.class, String.class, String.class, Optional.class); paramNamedDefaultValueString = new MethodParameter(method, 0); paramNamedStringArray = new MethodParameter(method, 1); @@ -114,6 +120,7 @@ public class RequestParamMethodArgumentResolverTests { paramRequestPartAnnot = new MethodParameter(method, 14); paramRequired = new MethodParameter(method, 15); paramNotRequired = new MethodParameter(method, 16); + paramOptional = new MethodParameter(method, 17); request = new MockHttpServletRequest(); webRequest = new ServletWebRequest(request, new MockHttpServletResponse()); @@ -420,6 +427,22 @@ public class RequestParamMethodArgumentResolverTests { assertEquals("", result); } + @Test + public void resolveOptional() throws Exception { + ConfigurableWebBindingInitializer initializer = new ConfigurableWebBindingInitializer(); + initializer.setConversionService(new DefaultConversionService()); + WebDataBinderFactory binderFactory = new DefaultDataBinderFactory(initializer); + + Object result = resolver.resolveArgument(paramOptional, null, webRequest, binderFactory); + assertEquals(Optional.class, result.getClass()); + assertEquals(Optional.empty(), result); + + this.request.addParameter("name", "123"); + result = resolver.resolveArgument(paramOptional, null, webRequest, binderFactory); + assertEquals(Optional.class, result.getClass()); + assertEquals(123, ((Optional) result).get()); + } + public void params(@RequestParam(value = "name", defaultValue = "bar") String param1, @RequestParam("name") String[] param2, @@ -437,7 +460,8 @@ public class RequestParamMethodArgumentResolverTests { Part part, @RequestPart MultipartFile requestPartAnnot, @RequestParam(value = "name") String paramRequired, - @RequestParam(value = "name", required=false) String paramNotRequired) { + @RequestParam(value = "name", required=false) String paramNotRequired, + @RequestParam(value = "name") Optional paramOptional) { } } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/MatrixVariableMethodArgumentResolver.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/MatrixVariableMethodArgumentResolver.java index e4fd61ceac1..6fd415e6897 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/MatrixVariableMethodArgumentResolver.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/MatrixVariableMethodArgumentResolver.java @@ -115,9 +115,11 @@ public class MatrixVariableMethodArgumentResolver extends AbstractNamedValueMeth @Override protected void handleMissingValue(String name, MethodParameter param) throws ServletRequestBindingException { - String paramType = param.getParameterType().getName(); - throw new ServletRequestBindingException( - "Missing matrix variable '" + name + "' for method parameter type [" + paramType + "]"); + Class paramType = param.getParameterType(); + if (!paramType.getName().equals("java.util.Optional")) { + throw new ServletRequestBindingException( + "Missing matrix variable '" + name + "' for method parameter type [" + paramType.getName() + "]"); + } }