From fa4d13968451bfe96eab122634c0a1eab6083aaa Mon Sep 17 00:00:00 2001 From: Sebastien Deleuze Date: Thu, 20 Jul 2017 10:28:29 +0200 Subject: [PATCH] Support instantiating Kotlin classes with optional parameters This commit updates BeanUtils class in order to add Kotlin optional parameters with default values support to the immutable data classes support introduced by SPR-15199. Issue: SPR-15673 --- build.gradle | 5 + .../org/springframework/beans/BeanUtils.java | 117 +++++++++++++++++- .../springframework/beans/BeanUtilsTests.java | 31 +++++ .../beans/BeanUtilsKotlinTests.kt | 65 ++++++++++ .../springframework/core/MethodParameter.java | 13 +- .../ModelAttributeMethodProcessor.java | 18 +-- ...ationControllerHandlerMethodKotlinTests.kt | 81 ++++++++++++ 7 files changed, 316 insertions(+), 14 deletions(-) create mode 100644 spring-beans/src/test/kotlin/org/springframework/beans/BeanUtilsKotlinTests.kt create mode 100644 spring-webmvc/src/test/kotlin/org/springframework/web/servlet/mvc/method/annotation/ServletAnnotationControllerHandlerMethodKotlinTests.kt diff --git a/build.gradle b/build.gradle index d1a5a9b78e..336aeffbbe 100644 --- a/build.gradle +++ b/build.gradle @@ -517,6 +517,7 @@ project("spring-context") { optional("org.aspectj:aspectjweaver:${aspectjVersion}") optional("org.codehaus.groovy:groovy-all:${groovyVersion}") optional("org.beanshell:bsh:2.0b5") + optional("org.jetbrains.kotlin:kotlin-reflect:${kotlinVersion}") optional("org.jetbrains.kotlin:kotlin-stdlib:${kotlinVersion}") testCompile("org.apache.commons:commons-pool2:2.4.2") testCompile("org.slf4j:slf4j-api:${slf4jVersion}") @@ -755,6 +756,7 @@ project("spring-web") { optional("javax.xml.bind:jaxb-api:${jaxbVersion}") optional("javax.xml.ws:jaxws-api:${jaxwsVersion}") optional("javax.mail:javax.mail-api:${javamailVersion}") + optional("org.jetbrains.kotlin:kotlin-reflect:${kotlinVersion}") optional("org.jetbrains.kotlin:kotlin-stdlib:${kotlinVersion}") testCompile("io.projectreactor:reactor-test") testCompile("org.apache.taglibs:taglibs-standard-jstlel:1.2.1") { @@ -853,6 +855,8 @@ project("spring-webmvc") { } optional('org.webjars:webjars-locator:0.32-1') optional("org.reactivestreams:reactive-streams") + optional("org.jetbrains.kotlin:kotlin-reflect:${kotlinVersion}") + optional("org.jetbrains.kotlin:kotlin-stdlib:${kotlinVersion}") testCompile("org.xmlunit:xmlunit-matchers:${xmlunitVersion}") testCompile("dom4j:dom4j:1.6.1") { exclude group: "xml-apis", module: "xml-apis" @@ -1048,6 +1052,7 @@ project("spring-test") { optional("org.reactivestreams:reactive-streams") optional("io.projectreactor:reactor-core") optional("io.projectreactor:reactor-test") + optional("org.jetbrains.kotlin:kotlin-reflect:${kotlinVersion}") optional("org.jetbrains.kotlin:kotlin-stdlib:${kotlinVersion}") testCompile(project(":spring-context-support")) testCompile(project(":spring-oxm")) diff --git a/spring-beans/src/main/java/org/springframework/beans/BeanUtils.java b/spring-beans/src/main/java/org/springframework/beans/BeanUtils.java index a1d20808f6..b506593ad0 100644 --- a/spring-beans/src/main/java/org/springframework/beans/BeanUtils.java +++ b/spring-beans/src/main/java/org/springframework/beans/BeanUtils.java @@ -18,6 +18,7 @@ package org.springframework.beans; import java.beans.PropertyDescriptor; import java.beans.PropertyEditor; +import java.lang.annotation.Annotation; import java.lang.reflect.Constructor; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; @@ -27,10 +28,17 @@ import java.net.URL; import java.util.Arrays; import java.util.Collections; import java.util.Date; +import java.util.HashMap; import java.util.List; import java.util.Locale; +import java.util.Map; import java.util.Set; +import kotlin.jvm.JvmClassMappingKt; +import kotlin.reflect.KFunction; +import kotlin.reflect.KParameter; +import kotlin.reflect.full.KClasses; +import kotlin.reflect.jvm.ReflectJvmMapping; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -53,6 +61,7 @@ import org.springframework.util.StringUtils; * @author Juergen Hoeller * @author Rob Harrop * @author Sam Brannen + * @author Sebastien Deleuze */ public abstract class BeanUtils { @@ -61,6 +70,9 @@ public abstract class BeanUtils { private static final Set> unknownEditorTypes = Collections.newSetFromMap(new ConcurrentReferenceHashMap<>(64)); + private static final boolean kotlinPresent = + ClassUtils.isPresent("kotlin.Unit", BeanUtils.class.getClassLoader()); + /** * Convenience method to instantiate a class using its no-arg constructor. @@ -103,7 +115,12 @@ public abstract class BeanUtils { throw new BeanInstantiationException(clazz, "Specified class is an interface"); } try { - return instantiateClass(clazz.getDeclaredConstructor()); + Constructor ctor = (kotlinPresent && isKotlinClass(clazz) ? + KotlinDelegate.findPrimaryConstructor(clazz) : clazz.getDeclaredConstructor()); + if (ctor == null) { + throw new BeanInstantiationException(clazz, "No default constructor found"); + } + return instantiateClass(ctor); } catch (NoSuchMethodException ex) { throw new BeanInstantiationException(clazz, "No default constructor found", ex); @@ -132,9 +149,11 @@ public abstract class BeanUtils { /** * Convenience method to instantiate a class using the given constructor. *

Note that this method tries to set the constructor accessible if given a - * non-accessible (that is, non-public) constructor. + * non-accessible (that is, non-public) constructor, and supports Kotlin classes + * with optional parameters and default values. * @param ctor the constructor to instantiate - * @param args the constructor arguments to apply + * @param args the constructor arguments to apply (use null for unspecified parameter + * if needed for Kotlin classes with optional parameters and default values) * @return the new instance * @throws BeanInstantiationException if the bean cannot be instantiated * @see Constructor#newInstance @@ -143,7 +162,7 @@ public abstract class BeanUtils { Assert.notNull(ctor, "Constructor must not be null"); try { ReflectionUtils.makeAccessible(ctor); - return ctor.newInstance(args); + return (kotlinPresent && isKotlinClass(ctor.getDeclaringClass()) ? KotlinDelegate.instantiateClass(ctor, args) : ctor.newInstance(args)); } catch (InstantiationException ex) { throw new BeanInstantiationException(ctor, "Is it an abstract class?", ex); @@ -299,6 +318,37 @@ public abstract class BeanUtils { return targetMethod; } + /** + * Return the primary constructor of the provided class (single or default constructor + * for Java classes and primary constructor for Kotlin classes) if any. + * @param clazz the {@link Class} of the Kotlin class + * @see http://kotlinlang.org/docs/reference/classes.html#constructors + * @since 5.0 + */ + @SuppressWarnings("unchecked") + @Nullable + public static Constructor findPrimaryConstructor(Class clazz) { + Assert.notNull(clazz, "Class must not be null"); + Constructor ctor = null; + if (kotlinPresent && isKotlinClass(clazz)) { + ctor = KotlinDelegate.findPrimaryConstructor(clazz); + } + else { + Constructor[] ctors = (Constructor[])clazz.getConstructors(); + if (ctors.length == 1) { + ctor = ctors[0]; + } + else { + try { + ctor = clazz.getDeclaredConstructor(); + } + catch (NoSuchMethodException e) { + } + } + } + return ctor; + } + /** * Parse a method signature in the form {@code methodName[([arg_list])]}, * where {@code arg_list} is an optional, comma-separated list of fully-qualified @@ -646,4 +696,63 @@ public abstract class BeanUtils { } } + /** + * Return true if the specified class is a Kotlin one. + */ + private static boolean isKotlinClass(Class clazz) { + for (Annotation annotation : clazz.getDeclaredAnnotations()) { + if (annotation.annotationType().getName().equals("kotlin.Metadata")) { + return true; + } + } + return false; + } + + + /** + * Inner class to avoid a hard dependency on Kotlin at runtime. + */ + private static class KotlinDelegate { + + /** + * Return the Java constructor corresponding to the Kotlin primary constructor if any. + * @param clazz the {@link Class} of the Kotlin class + * @see http://kotlinlang.org/docs/reference/classes.html#constructors + */ + @Nullable + public static Constructor findPrimaryConstructor(Class clazz) { + KFunction primaryConstructor = KClasses.getPrimaryConstructor(JvmClassMappingKt.getKotlinClass(clazz)); + if (primaryConstructor == null) { + return null; + } + Constructor constructor = ReflectJvmMapping.getJavaConstructor(primaryConstructor); + Assert.notNull(constructor, "Can't get the Java constructor corresponding to the Kotlin primary constructor of " + clazz.getName()); + return constructor; + } + + /** + * Instantiate a Kotlin class using the provided constructor. + * @param ctor the constructor of the Kotlin class to instantiate + * @param args the constructor arguments to apply (use null for unspecified parameter if needed) + * @throws BeanInstantiationException if no primary constructor can be found + */ + public static T instantiateClass(Constructor ctor, Object... args) { + KFunction kotlinConstructor = ReflectJvmMapping.getKotlinFunction(ctor); + if (kotlinConstructor == null) { + throw new BeanInstantiationException(ctor.getDeclaringClass(), "No corresponding Kotlin constructor found"); + } + List parameters = kotlinConstructor.getParameters(); + Map argParameters = new HashMap<>(parameters.size()); + Assert.isTrue(args.length <= parameters.size(), + "The number of provided arguments should be less of equals than the number of constructor parameters"); + for (int i = 0 ; i < args.length ; i++) { + if (!(parameters.get(i).isOptional() && (args[i] == null))) { + argParameters.put(parameters.get(i), args[i]); + } + } + return kotlinConstructor.callBy(argParameters); + } + + } + } diff --git a/spring-beans/src/test/java/org/springframework/beans/BeanUtilsTests.java b/spring-beans/src/test/java/org/springframework/beans/BeanUtilsTests.java index f8f7e858ee..3afba43a53 100644 --- a/spring-beans/src/test/java/org/springframework/beans/BeanUtilsTests.java +++ b/spring-beans/src/test/java/org/springframework/beans/BeanUtilsTests.java @@ -18,6 +18,7 @@ package org.springframework.beans; import java.beans.Introspector; import java.beans.PropertyDescriptor; +import java.lang.reflect.Constructor; import java.lang.reflect.Method; import java.util.ArrayList; import java.util.List; @@ -274,6 +275,23 @@ public class BeanUtilsTests { } } } + + @Test + public void testFindDefaultConstructorAndInstantiate() { + Constructor ctor = BeanUtils.findPrimaryConstructor(Bean.class); + assertNotNull(ctor); + Bean bean = BeanUtils.instantiateClass(ctor); + assertNotNull(bean); + } + + @Test + public void testFindSingleNonDefaultConstructorAndInstantiate() { + Constructor ctor = BeanUtils.findPrimaryConstructor(BeanWithSingleNonDefaultConstructor.class); + assertNotNull(ctor); + BeanWithSingleNonDefaultConstructor bean = BeanUtils.instantiateClass(ctor, "foo"); + assertNotNull(bean); + assertEquals("foo", bean.getName()); + } private void assertSignatureEquals(Method desiredMethod, String signature) { assertEquals(desiredMethod, BeanUtils.resolveSignature(signature, MethodSignatureBean.class)); @@ -444,5 +462,18 @@ public class BeanUtilsTests { value = aValue; } } + + private static class BeanWithSingleNonDefaultConstructor { + + private final String name; + + public BeanWithSingleNonDefaultConstructor(String name) { + this.name = name; + } + + public String getName() { + return name; + } + } } diff --git a/spring-beans/src/test/kotlin/org/springframework/beans/BeanUtilsKotlinTests.kt b/spring-beans/src/test/kotlin/org/springframework/beans/BeanUtilsKotlinTests.kt new file mode 100644 index 0000000000..c7c4040e24 --- /dev/null +++ b/spring-beans/src/test/kotlin/org/springframework/beans/BeanUtilsKotlinTests.kt @@ -0,0 +1,65 @@ +/* + * 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. + * 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 org.junit.Assert.* +import org.junit.Test + +/** + * Kotlin tests for {@link BeanUtils} + * + * @author Sebastien Deleuze + */ +class BeanUtilsKotlinTests { + + @Test + fun `Instantiate immutable class`() { + val constructor = BeanUtils.findPrimaryConstructor(Foo::class.java) + val foo = BeanUtils.instantiateClass(constructor, "bar", 3) as Foo + assertEquals("bar", foo.param1) + assertEquals(3, foo.param2) + } + + @Test + fun `Instantiate immutable class with optional parameter and all parameters specified`() { + val constructor = BeanUtils.findPrimaryConstructor(Bar::class.java) + val bar = BeanUtils.instantiateClass(constructor, "baz", 8) as Bar + assertEquals("baz", bar.param1) + assertEquals(8, bar.param2) + } + + @Test + fun `Instantiate immutable class with optional parameter and only mandatory parameters specified by position`() { + val constructor = BeanUtils.findPrimaryConstructor(Bar::class.java) + val bar = BeanUtils.instantiateClass(constructor, "baz") as Bar + assertEquals("baz", bar.param1) + assertEquals(12, bar.param2) + } + + @Test + fun `Instantiate immutable class with optional parameter specified with null value`() { + val constructor = BeanUtils.findPrimaryConstructor(Bar::class.java) + val bar = BeanUtils.instantiateClass(constructor, "baz", null) as Bar + assertEquals("baz", bar.param1) + assertEquals(12, bar.param2) + } + + class Foo(val param1: String, val param2: Int) + + class Bar(val param1: String, val param2: Int = 12) + +} diff --git a/spring-core/src/main/java/org/springframework/core/MethodParameter.java b/spring-core/src/main/java/org/springframework/core/MethodParameter.java index 07ea4252c0..ad293a19af 100644 --- a/spring-core/src/main/java/org/springframework/core/MethodParameter.java +++ b/spring-core/src/main/java/org/springframework/core/MethodParameter.java @@ -31,7 +31,6 @@ import java.util.Map; import java.util.Optional; import java.util.stream.Collectors; -import kotlin.Metadata; import kotlin.reflect.KFunction; import kotlin.reflect.KParameter; import kotlin.reflect.jvm.ReflectJvmMapping; @@ -737,7 +736,7 @@ public class MethodParameter { * Check whether the specified {@link MethodParameter} represents a nullable Kotlin type or not. */ public static boolean isNullable(MethodParameter param) { - if (param.getContainingClass().isAnnotationPresent(Metadata.class)) { + if (isKotlinClass(param.getContainingClass())) { Method method = param.getMethod(); Constructor ctor = param.getConstructor(); int index = param.getParameterIndex(); @@ -767,6 +766,16 @@ public class MethodParameter { } return false; } + + private static boolean isKotlinClass(Class clazz) { + for (Annotation annotation : clazz.getDeclaredAnnotations()) { + if (annotation.annotationType().getName().equals("kotlin.Metadata")) { + return true; + } + } + return false; + } + } } diff --git a/spring-web/src/main/java/org/springframework/web/method/annotation/ModelAttributeMethodProcessor.java b/spring-web/src/main/java/org/springframework/web/method/annotation/ModelAttributeMethodProcessor.java index 5e8f409703..7c8245ef69 100644 --- a/spring-web/src/main/java/org/springframework/web/method/annotation/ModelAttributeMethodProcessor.java +++ b/spring-web/src/main/java/org/springframework/web/method/annotation/ModelAttributeMethodProcessor.java @@ -59,6 +59,7 @@ import org.springframework.web.method.support.ModelAndViewContainer; * * @author Rossen Stoyanchev * @author Juergen Hoeller + * @author Sebastien Deleuze * @since 3.1 */ public class ModelAttributeMethodProcessor implements HandlerMethodArgumentResolver, HandlerMethodReturnValueHandler { @@ -157,13 +158,13 @@ public class ModelAttributeMethodProcessor implements HandlerMethodArgumentResol protected Object createAttribute(String attributeName, MethodParameter parameter, WebDataBinderFactory binderFactory, NativeWebRequest webRequest) throws Exception { - Constructor[] ctors = parameter.getParameterType().getConstructors(); - if (ctors.length != 1) { - // No standard data class or standard JavaBeans arrangement -> - // defensively go with default constructor, expecting regular bean property bindings. - return BeanUtils.instantiateClass(parameter.getParameterType()); + Class type = parameter.getParameterType(); + + Constructor ctor = BeanUtils.findPrimaryConstructor(type); + if (ctor == null) { + throw new IllegalStateException("No primary constructor found for " + type.getName()); } - Constructor ctor = ctors[0]; + if (ctor.getParameterCount() == 0) { // A single default constructor -> clearly a standard JavaBeans arrangement. return BeanUtils.instantiateClass(ctor); @@ -179,8 +180,9 @@ public class ModelAttributeMethodProcessor implements HandlerMethodArgumentResol Object[] args = new Object[paramTypes.length]; WebDataBinder binder = binderFactory.createBinder(webRequest, null, attributeName); for (int i = 0; i < paramNames.length; i++) { - args[i] = binder.convertIfNecessary( - webRequest.getParameterValues(paramNames[i]), paramTypes[i], new MethodParameter(ctor, i)); + String[] parameterValues = webRequest.getParameterValues(paramNames[i]); + args[i] = (parameterValues != null ? binder.convertIfNecessary(parameterValues, paramTypes[i], + new MethodParameter(ctor, i)) : null); } return BeanUtils.instantiateClass(ctor, args); } diff --git a/spring-webmvc/src/test/kotlin/org/springframework/web/servlet/mvc/method/annotation/ServletAnnotationControllerHandlerMethodKotlinTests.kt b/spring-webmvc/src/test/kotlin/org/springframework/web/servlet/mvc/method/annotation/ServletAnnotationControllerHandlerMethodKotlinTests.kt new file mode 100644 index 0000000000..3867a6730a --- /dev/null +++ b/spring-webmvc/src/test/kotlin/org/springframework/web/servlet/mvc/method/annotation/ServletAnnotationControllerHandlerMethodKotlinTests.kt @@ -0,0 +1,81 @@ +/* + * 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. + * 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.web.servlet.mvc.method.annotation + +import org.junit.Assert.* +import org.junit.Test +import org.springframework.mock.web.test.MockHttpServletRequest +import org.springframework.mock.web.test.MockHttpServletResponse +import org.springframework.web.bind.annotation.RequestMapping +import org.springframework.web.bind.annotation.RestController + +/** + * @author Sebastien Deleuze + */ +class ServletAnnotationControllerHandlerMethodKotlinTests : AbstractServletHandlerMethodTests() { + + @Test + fun dataClassBinding() { + initServletWithControllers(DataClassController::class.java) + + val request = MockHttpServletRequest("GET", "/bind") + request.addParameter("param1", "value1") + request.addParameter("param2", "2") + val response = MockHttpServletResponse() + servlet.service(request, response) + assertEquals("value1-2", response.contentAsString) + } + + @Test + fun dataClassBindingWithOptionalParameterAndAllParameters() { + initServletWithControllers(DataClassController::class.java) + + val request = MockHttpServletRequest("GET", "/bind-optional-parameter") + request.addParameter("param1", "value1") + request.addParameter("param2", "2") + val response = MockHttpServletResponse() + servlet.service(request, response) + assertEquals("value1-2", response.contentAsString) + } + + @Test + fun dataClassBindingWithOptionalParameterAndOnlyMissingParameters() { + initServletWithControllers(DataClassController::class.java) + + val request = MockHttpServletRequest("GET", "/bind-optional-parameter") + request.addParameter("param1", "value1") + val response = MockHttpServletResponse() + servlet.service(request, response) + assertEquals("value1-12", response.contentAsString) + } + + + data class DataClass(val param1: String, val param2: Int) + + data class DataClassWithOptionalParameter(val param1: String, val param2: Int = 12) + + @RestController + class DataClassController { + + @RequestMapping("/bind") + fun handle(data: DataClass) = "${data.param1}-${data.param2}" + + @RequestMapping("/bind-optional-parameter") + fun handle(data: DataClassWithOptionalParameter) = "${data.param1}-${data.param2}" + } + +}