From f37ec90f2f932cb1327f127a5d130dd78cfecb1d Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Tue, 3 Sep 2019 13:55:56 +0200 Subject: [PATCH] Consider Void.class a primitive wrapper in ClassUtils Prior to this commit, ClassUtils.isPrimitiveOrWrapper() and ClassUtils.isPrimitiveWrapper() did not return true for Void.class. However, ClassUtils.isPrimitiveOrWrapper() did return true for void.class. This lacking symmetry is inconsistent and can lead to bugs in reflective code. See: https://github.com/spring-projects/spring-data-r2dbc/issues/159 This commit addresses this by adding an entry for Void.class -> void.class in the internal primitiveWrapperTypeMap in ClassUtils. Closes gh-23572 --- .../org/springframework/util/ClassUtils.java | 12 ++++-- .../springframework/util/ClassUtilsTests.java | 43 ++++++++++++++++++- .../view/ViewResolutionResultHandler.java | 2 +- .../ViewResolutionResultHandlerTests.java | 7 ++- 4 files changed, 56 insertions(+), 8 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/util/ClassUtils.java b/spring-core/src/main/java/org/springframework/util/ClassUtils.java index 5c04eb5861..3303f169f6 100644 --- a/spring-core/src/main/java/org/springframework/util/ClassUtils.java +++ b/spring-core/src/main/java/org/springframework/util/ClassUtils.java @@ -120,6 +120,7 @@ public abstract class ClassUtils { primitiveWrapperTypeMap.put(Integer.class, int.class); primitiveWrapperTypeMap.put(Long.class, long.class); primitiveWrapperTypeMap.put(Short.class, short.class); + primitiveWrapperTypeMap.put(Void.class, void.class); // Map entry iteration is less expensive to initialize than forEach with lambdas for (Map.Entry, Class> entry : primitiveWrapperTypeMap.entrySet()) { @@ -462,7 +463,8 @@ public abstract class ClassUtils { /** * Check if the given class represents a primitive wrapper, - * i.e. Boolean, Byte, Character, Short, Integer, Long, Float, or Double. + * i.e. Boolean, Byte, Character, Short, Integer, Long, Float, Double, or + * Void. * @param clazz the class to check * @return whether the given class is a primitive wrapper class */ @@ -473,10 +475,12 @@ public abstract class ClassUtils { /** * Check if the given class represents a primitive (i.e. boolean, byte, - * char, short, int, long, float, or double) or a primitive wrapper - * (i.e. Boolean, Byte, Character, Short, Integer, Long, Float, or Double). + * char, short, int, long, float, or double), {@code void}, or a wrapper for + * those types (i.e. Boolean, Byte, Character, Short, Integer, Long, Float, + * Double, or Void). * @param clazz the class to check - * @return whether the given class is a primitive or primitive wrapper class + * @return {@code true} if the given class represents a primitive, void, or + * a wrapper class */ public static boolean isPrimitiveOrWrapper(Class clazz) { Assert.notNull(clazz, "Class must not be null"); diff --git a/spring-core/src/test/java/org/springframework/util/ClassUtilsTests.java b/spring-core/src/test/java/org/springframework/util/ClassUtilsTests.java index 4bd817f596..4426b25182 100644 --- a/spring-core/src/test/java/org/springframework/util/ClassUtilsTests.java +++ b/spring-core/src/test/java/org/springframework/util/ClassUtilsTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 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. @@ -40,14 +40,17 @@ import org.springframework.tests.sample.objects.TestObject; import static org.junit.Assert.*; /** + * Unit tests for {@link ClassUtils}. + * * @author Colin Sampaleanu * @author Juergen Hoeller * @author Rob Harrop * @author Rick Evans + * @author Sam Brannen */ public class ClassUtilsTests { - private ClassLoader classLoader = getClass().getClassLoader(); + private final ClassLoader classLoader = getClass().getClassLoader(); @Before @@ -384,6 +387,42 @@ public class ClassUtilsTests { assertNull(ClassUtils.determineCommonAncestor(String.class, List.class)); } + @Test + public void isPrimitiveWrapper() { + assertTrue(ClassUtils.isPrimitiveWrapper(Boolean.class)); + assertTrue(ClassUtils.isPrimitiveWrapper(Character.class)); + assertTrue(ClassUtils.isPrimitiveWrapper(Byte.class)); + assertTrue(ClassUtils.isPrimitiveWrapper(Short.class)); + assertTrue(ClassUtils.isPrimitiveWrapper(Integer.class)); + assertTrue(ClassUtils.isPrimitiveWrapper(Long.class)); + assertTrue(ClassUtils.isPrimitiveWrapper(Float.class)); + assertTrue(ClassUtils.isPrimitiveWrapper(Double.class)); + assertTrue(ClassUtils.isPrimitiveWrapper(Void.class)); + } + + @Test + public void isPrimitiveOrWrapper() { + assertTrue(ClassUtils.isPrimitiveOrWrapper(boolean.class)); + assertTrue(ClassUtils.isPrimitiveOrWrapper(char.class)); + assertTrue(ClassUtils.isPrimitiveOrWrapper(byte.class)); + assertTrue(ClassUtils.isPrimitiveOrWrapper(short.class)); + assertTrue(ClassUtils.isPrimitiveOrWrapper(int.class)); + assertTrue(ClassUtils.isPrimitiveOrWrapper(long.class)); + assertTrue(ClassUtils.isPrimitiveOrWrapper(float.class)); + assertTrue(ClassUtils.isPrimitiveOrWrapper(double.class)); + assertTrue(ClassUtils.isPrimitiveOrWrapper(void.class)); + + assertTrue(ClassUtils.isPrimitiveOrWrapper(Boolean.class)); + assertTrue(ClassUtils.isPrimitiveOrWrapper(Character.class)); + assertTrue(ClassUtils.isPrimitiveOrWrapper(Byte.class)); + assertTrue(ClassUtils.isPrimitiveOrWrapper(Short.class)); + assertTrue(ClassUtils.isPrimitiveOrWrapper(Integer.class)); + assertTrue(ClassUtils.isPrimitiveOrWrapper(Long.class)); + assertTrue(ClassUtils.isPrimitiveOrWrapper(Float.class)); + assertTrue(ClassUtils.isPrimitiveOrWrapper(Double.class)); + assertTrue(ClassUtils.isPrimitiveOrWrapper(Void.class)); + } + public static class InnerClass { diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandler.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandler.java index 691fac712b..66637a2ac8 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandler.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandler.java @@ -162,7 +162,7 @@ public class ViewResolutionResultHandler extends HandlerResultHandlerSupport return (CharSequence.class.isAssignableFrom(type) || Rendering.class.isAssignableFrom(type) || Model.class.isAssignableFrom(type) || Map.class.isAssignableFrom(type) || - void.class.equals(type) || View.class.isAssignableFrom(type) || + Void.class.equals(type) || void.class.equals(type) || View.class.isAssignableFrom(type) || !BeanUtils.isSimpleProperty(type)); } diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandlerTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandlerTests.java index 08ff19cc77..54b62e57ae 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandlerTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandlerTests.java @@ -114,7 +114,12 @@ public class ViewResolutionResultHandlerTests { private void testSupports(MethodParameter returnType, boolean supports) { ViewResolutionResultHandler resultHandler = resultHandler(mock(ViewResolver.class)); HandlerResult handlerResult = new HandlerResult(new Object(), null, returnType, this.bindingContext); - assertEquals(supports, resultHandler.supports(handlerResult)); + if (supports) { + assertTrue("return type [" + returnType + "] should be supported", resultHandler.supports(handlerResult)); + } + else { + assertFalse("return type [" + returnType + "] should not be supported", resultHandler.supports(handlerResult)); + } } @Test