From 53a9697ff1437896ae323b0108c8b82a2ba1c551 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Wed, 20 Sep 2017 18:28:49 +0200 Subject: [PATCH] Consistent conversion of Optional array/list arrangements Issue: SPR-15918 Issue: SPR-15919 Issue: SPR-15676 (cherry picked from commit 15c82af) --- .../support/ObjectToOptionalConverter.java | 20 +++-- .../org/springframework/util/ObjectUtils.java | 8 +- ...questParamMethodArgumentResolverTests.java | 85 ++++++++++++++++++- 3 files changed, 103 insertions(+), 10 deletions(-) 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 index 6137012b935..3a88521602c 100644 --- 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2017 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,7 +16,9 @@ package org.springframework.core.convert.support; -import java.util.Collections; +import java.lang.reflect.Array; +import java.util.Collection; +import java.util.LinkedHashSet; import java.util.Optional; import java.util.Set; @@ -47,7 +49,11 @@ final class ObjectToOptionalConverter implements ConditionalGenericConverter { @Override public Set getConvertibleTypes() { - return Collections.singleton(new ConvertiblePair(Object.class, Optional.class)); + Set convertibleTypes = new LinkedHashSet(4); + convertibleTypes.add(new ConvertiblePair(Collection.class, Optional.class)); + convertibleTypes.add(new ConvertiblePair(Object[].class, Optional.class)); + convertibleTypes.add(new ConvertiblePair(Object.class, Optional.class)); + return convertibleTypes; } @Override @@ -70,7 +76,11 @@ final class ObjectToOptionalConverter implements ConditionalGenericConverter { } else if (targetType.getResolvableType() != null) { Object target = this.conversionService.convert(source, sourceType, new GenericTypeDescriptor(targetType)); - return Optional.ofNullable(target); + if (target == null || (target.getClass().isArray() && Array.getLength(target) == 0) || + (target instanceof Collection && ((Collection) target).isEmpty())) { + return Optional.empty(); + } + return Optional.of(target); } else { return Optional.of(source); @@ -82,7 +92,7 @@ final class ObjectToOptionalConverter implements ConditionalGenericConverter { private static class GenericTypeDescriptor extends TypeDescriptor { public GenericTypeDescriptor(TypeDescriptor typeDescriptor) { - super(typeDescriptor.getResolvableType().getGeneric(0), null, typeDescriptor.getAnnotations()); + super(typeDescriptor.getResolvableType().getGeneric(), null, typeDescriptor.getAnnotations()); } } diff --git a/spring-core/src/main/java/org/springframework/util/ObjectUtils.java b/spring-core/src/main/java/org/springframework/util/ObjectUtils.java index 624d6d35678..10db9f54f50 100644 --- a/spring-core/src/main/java/org/springframework/util/ObjectUtils.java +++ b/spring-core/src/main/java/org/springframework/util/ObjectUtils.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2017 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. @@ -131,12 +131,12 @@ public abstract class ObjectUtils { return true; } - if (obj.getClass().isArray()) { - return Array.getLength(obj) == 0; - } if (obj instanceof CharSequence) { return ((CharSequence) obj).length() == 0; } + if (obj.getClass().isArray()) { + return Array.getLength(obj) == 0; + } if (obj instanceof Collection) { return ((Collection) obj).isEmpty(); } 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 f77ce2bc65f..881e1d28c19 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 @@ -84,6 +84,8 @@ public class RequestParamMethodArgumentResolverTests { private MethodParameter paramRequired; private MethodParameter paramNotRequired; private MethodParameter paramOptional; + private MethodParameter paramOptionalArray; + private MethodParameter paramOptionalList; private MethodParameter multipartFileOptional; private NativeWebRequest webRequest; @@ -119,7 +121,9 @@ public class RequestParamMethodArgumentResolverTests { paramRequired = new SynthesizingMethodParameter(method, 15); paramNotRequired = new SynthesizingMethodParameter(method, 16); paramOptional = new SynthesizingMethodParameter(method, 17); - multipartFileOptional = new SynthesizingMethodParameter(method, 18); + paramOptionalArray = new SynthesizingMethodParameter(method, 18); + paramOptionalList = new SynthesizingMethodParameter(method, 19); + multipartFileOptional = new SynthesizingMethodParameter(method, 20); request = new MockHttpServletRequest(); webRequest = new ServletWebRequest(request, new MockHttpServletResponse()); @@ -437,6 +441,83 @@ public class RequestParamMethodArgumentResolverTests { assertEquals(123, ((Optional) result).get()); } + @Test + @SuppressWarnings("rawtypes") + public void missingOptionalParamValue() 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.empty(), result); + + result = resolver.resolveArgument(paramOptional, null, webRequest, binderFactory); + assertEquals(Optional.class, result.getClass()); + assertFalse(((Optional) result).isPresent()); + } + + @Test + @SuppressWarnings("rawtypes") + public void resolveOptionalParamArray() throws Exception { + ConfigurableWebBindingInitializer initializer = new ConfigurableWebBindingInitializer(); + initializer.setConversionService(new DefaultConversionService()); + WebDataBinderFactory binderFactory = new DefaultDataBinderFactory(initializer); + + Object result = resolver.resolveArgument(paramOptionalArray, null, webRequest, binderFactory); + assertEquals(Optional.empty(), result); + + this.request.addParameter("name", "123", "456"); + result = resolver.resolveArgument(paramOptionalArray, null, webRequest, binderFactory); + assertEquals(Optional.class, result.getClass()); + assertArrayEquals(new Integer[] {123, 456}, (Integer[]) ((Optional) result).get()); + } + + @Test + @SuppressWarnings("rawtypes") + public void missingOptionalParamArray() throws Exception { + ConfigurableWebBindingInitializer initializer = new ConfigurableWebBindingInitializer(); + initializer.setConversionService(new DefaultConversionService()); + WebDataBinderFactory binderFactory = new DefaultDataBinderFactory(initializer); + + Object result = resolver.resolveArgument(paramOptionalArray, null, webRequest, binderFactory); + assertEquals(Optional.empty(), result); + + result = resolver.resolveArgument(paramOptionalArray, null, webRequest, binderFactory); + assertEquals(Optional.class, result.getClass()); + assertFalse(((Optional) result).isPresent()); + } + + @Test + @SuppressWarnings("rawtypes") + public void resolveOptionalParamList() throws Exception { + ConfigurableWebBindingInitializer initializer = new ConfigurableWebBindingInitializer(); + initializer.setConversionService(new DefaultConversionService()); + WebDataBinderFactory binderFactory = new DefaultDataBinderFactory(initializer); + + Object result = resolver.resolveArgument(paramOptionalList, null, webRequest, binderFactory); + assertEquals(Optional.empty(), result); + + this.request.addParameter("name", "123", "456"); + result = resolver.resolveArgument(paramOptionalList, null, webRequest, binderFactory); + assertEquals(Optional.class, result.getClass()); + assertEquals(Arrays.asList("123", "456"), ((Optional) result).get()); + } + + @Test + @SuppressWarnings("rawtypes") + public void missingOptionalParamList() throws Exception { + ConfigurableWebBindingInitializer initializer = new ConfigurableWebBindingInitializer(); + initializer.setConversionService(new DefaultConversionService()); + WebDataBinderFactory binderFactory = new DefaultDataBinderFactory(initializer); + + Object result = resolver.resolveArgument(paramOptionalList, null, webRequest, binderFactory); + assertEquals(Optional.empty(), result); + + result = resolver.resolveArgument(paramOptionalList, null, webRequest, binderFactory); + assertEquals(Optional.class, result.getClass()); + assertFalse(((Optional) result).isPresent()); + } + @Test public void resolveOptionalMultipartFile() throws Exception { ConfigurableWebBindingInitializer initializer = new ConfigurableWebBindingInitializer(); @@ -493,6 +574,8 @@ public class RequestParamMethodArgumentResolverTests { @RequestParam("name") String paramRequired, @RequestParam(name = "name", required = false) String paramNotRequired, @RequestParam("name") Optional paramOptional, + @RequestParam("name") Optional paramOptionalArray, + @RequestParam("name") Optional paramOptionalList, @RequestParam("mfile") Optional multipartFileOptional) { }