From fada91e538590e2eab0b05230a8a449ca41af695 Mon Sep 17 00:00:00 2001 From: Raman Gupta Date: Thu, 8 Sep 2016 14:06:01 -0400 Subject: [PATCH] Treat Kotlin nullable as non-required Where `isOptional` is used, also check for `isNullable` i.e. values are not considered required if they are Kotlin nullables: - spring-messaging: named value method arguments - spring-web: named value method arguments - spring-webmvc: request parts This means that Kotlin client code no longer has to explicity specify "required=false" for Kotlin nullables -- this information is inferred automatically by the framework. Issue: SPR-14165 --- build.gradle | 6 ++ .../springframework/core/MethodParameter.java | 11 +++ .../org/springframework/util/KotlinUtils.java | 74 +++++++++++++++++++ .../springframework/util/KotlinUtilsTests.kt | 60 +++++++++++++++ ...tractNamedValueMethodArgumentResolver.java | 2 +- ...tractNamedValueMethodArgumentResolver.java | 2 +- .../RequestPartMethodArgumentResolver.java | 3 +- 7 files changed, 155 insertions(+), 3 deletions(-) create mode 100644 spring-core/src/main/java/org/springframework/util/KotlinUtils.java create mode 100644 spring-core/src/test/kotlin/org/springframework/util/KotlinUtilsTests.kt diff --git a/build.gradle b/build.gradle index f58d8b6a8f..b8580f9a32 100644 --- a/build.gradle +++ b/build.gradle @@ -5,6 +5,7 @@ buildscript { dependencies { classpath("org.springframework.build.gradle:propdeps-plugin:0.0.7") classpath("org.asciidoctor:asciidoctor-gradle-plugin:1.5.3") + classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:1.0.4" classpath("io.spring.gradle:docbook-reference-plugin:0.3.1") } } @@ -70,6 +71,7 @@ configure(allprojects) { project -> ext.junitPlatformVersion = '1.0.0-M2' ext.log4jVersion = '2.7' ext.nettyVersion = "4.1.6.Final" + ext.kotlinVersion = "1.0.4" ext.okhttpVersion = "2.7.5" ext.okhttp3Version = "3.4.2" ext.poiVersion = "3.15" @@ -324,6 +326,8 @@ project("spring-build-src") { project("spring-core") { description = "Spring Core" + apply plugin: "kotlin" + // As of Spring 4.0.3, spring-core includes asm 5.x and repackages cglib 3.2, inlining // both into the spring-core jar. cglib 3.2 itself depends on asm 5.x and is therefore // further transformed by the JarJar task to depend on org.springframework.asm; this @@ -389,6 +393,8 @@ project("spring-core") { compile("commons-logging:commons-logging:1.2") optional("org.aspectj:aspectjweaver:${aspectjVersion}") optional("net.sf.jopt-simple:jopt-simple:5.0.2") + optional("org.jetbrains.kotlin:kotlin-reflect:${kotlinVersion}") + optional("org.jetbrains.kotlin:kotlin-stdlib:${kotlinVersion}") optional("org.reactivestreams:reactive-streams:${reactivestreamsVersion}") optional("io.projectreactor:reactor-core:${reactorCoreVersion}") optional "io.reactivex:rxjava:${rxjavaVersion}" 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 ea1cc4c631..d55381ad49 100644 --- a/spring-core/src/main/java/org/springframework/core/MethodParameter.java +++ b/spring-core/src/main/java/org/springframework/core/MethodParameter.java @@ -30,6 +30,7 @@ import java.util.Map; import java.util.Optional; import org.springframework.util.Assert; +import org.springframework.util.KotlinUtils; /** * Helper class that encapsulates the specification of a method parameter, i.e. a {@link Method} @@ -329,6 +330,16 @@ public class MethodParameter { return (isOptional() ? nested() : this); } + /** + * Return whether this method parameter is declared as a "nullable" value, if supported by + * the underlying language. Currently the only supported language is Kotlin. + * @since 5.0 + */ + public boolean isNullable() { + return KotlinUtils.isKotlinPresent() && + KotlinUtils.isKotlinClass(getContainingClass()) && + KotlinUtils.isNullable(this.parameterIndex, this.method, this.constructor); + } /** * Set a containing class to resolve the parameter type against. diff --git a/spring-core/src/main/java/org/springframework/util/KotlinUtils.java b/spring-core/src/main/java/org/springframework/util/KotlinUtils.java new file mode 100644 index 0000000000..1a08d7079c --- /dev/null +++ b/spring-core/src/main/java/org/springframework/util/KotlinUtils.java @@ -0,0 +1,74 @@ +/* + * Copyright 2002-2016 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.util; + +import kotlin.Metadata; +import kotlin.reflect.KFunction; +import kotlin.reflect.KParameter; +import kotlin.reflect.jvm.ReflectJvmMapping; +import org.springframework.core.MethodParameter; + +import java.lang.reflect.Constructor; +import java.lang.reflect.Method; +import java.util.List; +import java.util.stream.Collectors; + +/** + * Miscellaneous Kotlin utility methods. + * + * @author Raman Gupta + * @since 5.0 + */ +public class KotlinUtils { + + private static final boolean kotlinPresent; + + static { + kotlinPresent = ClassUtils.isPresent("kotlin.Unit", MethodParameter.class.getClassLoader()); + } + + public static boolean isKotlinPresent() { + return kotlinPresent; + } + + public static boolean isKotlinClass(Class type) { + return type != null && type.getDeclaredAnnotation(Metadata.class) != null; + } + + public static boolean isNullable(int parameterIndex, Method method, Constructor constructor) { + if(parameterIndex < 0) { + KFunction function = ReflectJvmMapping.getKotlinFunction(method); + return function != null && function.getReturnType().isMarkedNullable(); + } else { + KFunction function = method != null ? + ReflectJvmMapping.getKotlinFunction(method) : + ReflectJvmMapping.getKotlinFunction(constructor); + if(function != null) { + @SuppressWarnings("unchecked") + List parameters = function.getParameters(); + return parameters + .stream() + .filter(p -> KParameter.Kind.VALUE.equals(p.getKind())) + .collect(Collectors.toList()) + .get(parameterIndex) + .getType() + .isMarkedNullable(); + } + return false; + } + } +} diff --git a/spring-core/src/test/kotlin/org/springframework/util/KotlinUtilsTests.kt b/spring-core/src/test/kotlin/org/springframework/util/KotlinUtilsTests.kt new file mode 100644 index 0000000000..e09700b99e --- /dev/null +++ b/spring-core/src/test/kotlin/org/springframework/util/KotlinUtilsTests.kt @@ -0,0 +1,60 @@ +package org.springframework.util + +import org.junit.Before +import org.junit.Test + +import java.lang.reflect.Method + +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.springframework.core.MethodParameter +import org.springframework.util.KotlinUtils.* + +/** + * Unit tests for [KotlinUtils]. + + * @author Raman Gupta + */ +class KotlinUtilsTests { + + private lateinit var methodNullable: Method + private lateinit var methodNonNullable: Method + + @Before + @Throws(NoSuchMethodException::class) + fun setUp() { + methodNullable = javaClass.getMethod("methodNullable", String::class.java, java.lang.Long.TYPE) + methodNonNullable = javaClass.getMethod("methodNonNullable", String::class.java, java.lang.Long.TYPE) + } + + @Test + fun `Is kotlin present`() { + // we'd have to change the build to test the opposite + assertTrue(isKotlinPresent()) + } + + @Test + fun `Are kotlin classes detected`() { + assertFalse(isKotlinClass(null)) + assertFalse(isKotlinClass(MethodParameter::class.java)) + assertTrue(isKotlinClass(javaClass)) + } + + @Test + fun `Obtains method return type nullability`() { + assertTrue(isNullable(-1, methodNullable, null)) + assertFalse(isNullable(-1, methodNonNullable, null)) + } + + @Test + fun `Obtains method parameter nullability`() { + assertTrue(isNullable(0, methodNullable, null)) + assertFalse(isNullable(1, methodNullable, null)) + } + + @Suppress("unused", "unused_parameter") + fun methodNullable(p1: String?, p2: Long): Int? = 42 + + @Suppress("unused", "unused_parameter") + fun methodNonNullable(p1: String?, p2: Long): Int = 42 +} diff --git a/spring-messaging/src/main/java/org/springframework/messaging/handler/annotation/support/AbstractNamedValueMethodArgumentResolver.java b/spring-messaging/src/main/java/org/springframework/messaging/handler/annotation/support/AbstractNamedValueMethodArgumentResolver.java index 8f26c0c9e8..d7e730d644 100644 --- a/spring-messaging/src/main/java/org/springframework/messaging/handler/annotation/support/AbstractNamedValueMethodArgumentResolver.java +++ b/spring-messaging/src/main/java/org/springframework/messaging/handler/annotation/support/AbstractNamedValueMethodArgumentResolver.java @@ -98,7 +98,7 @@ public abstract class AbstractNamedValueMethodArgumentResolver implements Handle if (namedValueInfo.defaultValue != null) { arg = resolveStringValue(namedValueInfo.defaultValue); } - else if (namedValueInfo.required && !nestedParameter.isOptional()) { + else if (namedValueInfo.required && !nestedParameter.isOptional() && !nestedParameter.isNullable()) { handleMissingValue(namedValueInfo.name, nestedParameter, message); } arg = handleNullValue(namedValueInfo.name, arg, nestedParameter.getNestedParameterType()); diff --git a/spring-web/src/main/java/org/springframework/web/method/annotation/AbstractNamedValueMethodArgumentResolver.java b/spring-web/src/main/java/org/springframework/web/method/annotation/AbstractNamedValueMethodArgumentResolver.java index 678fc300d0..104ad867eb 100644 --- a/spring-web/src/main/java/org/springframework/web/method/annotation/AbstractNamedValueMethodArgumentResolver.java +++ b/spring-web/src/main/java/org/springframework/web/method/annotation/AbstractNamedValueMethodArgumentResolver.java @@ -100,7 +100,7 @@ public abstract class AbstractNamedValueMethodArgumentResolver implements Handle if (namedValueInfo.defaultValue != null) { arg = resolveStringValue(namedValueInfo.defaultValue); } - else if (namedValueInfo.required && !nestedParameter.isOptional()) { + else if (namedValueInfo.required && !nestedParameter.isOptional() && !nestedParameter.isNullable()) { handleMissingValue(namedValueInfo.name, nestedParameter, webRequest); } arg = handleNullValue(namedValueInfo.name, arg, nestedParameter.getNestedParameterType()); diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestPartMethodArgumentResolver.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestPartMethodArgumentResolver.java index d2b58c60e7..b198d00247 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestPartMethodArgumentResolver.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestPartMethodArgumentResolver.java @@ -113,7 +113,8 @@ public class RequestPartMethodArgumentResolver extends AbstractMessageConverterM HttpServletRequest servletRequest = request.getNativeRequest(HttpServletRequest.class); RequestPart requestPart = parameter.getParameterAnnotation(RequestPart.class); - boolean isRequired = ((requestPart == null || requestPart.required()) && !parameter.isOptional()); + boolean isRequired = ((requestPart == null || requestPart.required()) && !parameter.isOptional() && + !parameter.isNullable()); String name = getPartName(parameter, requestPart); parameter = parameter.nestedIfOptional();