From a7568e3951cc454e6b6cfa4ad09afe539980e679 Mon Sep 17 00:00:00 2001 From: wrongwrong Date: Fri, 19 Jan 2024 13:05:56 +0900 Subject: [PATCH 1/5] Remove a cast that was no longer needed --- .../com/fasterxml/jackson/module/kotlin/ReflectionCache.kt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/ReflectionCache.kt b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/ReflectionCache.kt index eef3e61b2..632795e66 100644 --- a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/ReflectionCache.kt +++ b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/ReflectionCache.kt @@ -68,10 +68,9 @@ internal class ReflectionCache(reflectionCacheSize: Int) : Serializable { * - contains extensionReceiverParameter * - instance parameter is not companion object or can't get */ - @Suppress("UNCHECKED_CAST") fun valueCreatorFromJava(_withArgsCreator: AnnotatedWithParams): ValueCreator<*>? = when (_withArgsCreator) { is AnnotatedConstructor -> { - val constructor = _withArgsCreator.annotated as Constructor + val constructor = _withArgsCreator.annotated javaExecutableToValueCreator.get(constructor) ?: kotlinFromJava(constructor)?.let { From 292ee738bcf6362e4985572e21b63b0a9fe41bf1 Mon Sep 17 00:00:00 2001 From: wrongwrong Date: Sat, 20 Jan 2024 11:38:16 +0900 Subject: [PATCH 2/5] Fix to use cached function --- .../com/fasterxml/jackson/module/kotlin/ReflectionCache.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/ReflectionCache.kt b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/ReflectionCache.kt index 632795e66..fa804a2a1 100644 --- a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/ReflectionCache.kt +++ b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/ReflectionCache.kt @@ -113,7 +113,7 @@ internal class ReflectionCache(reflectionCacheSize: Int) : Serializable { // KotlinReflectionInternalError is raised in GitHub167 test, // but it looks like an edge case, so it is ignored. val prop = runCatching { kClass.memberProperties }.getOrNull()?.find { it.javaGetter == getter } - (prop?.returnType ?: runCatching { getter.kotlinFunction }.getOrNull()?.returnType) + (prop?.returnType ?: runCatching { kotlinFromJava(getter) }.getOrNull()?.returnType) ?.classifier as? KClass<*> } ?: return null From dd19492c9462a7a44c256eb14937501cf54a8282 Mon Sep 17 00:00:00 2001 From: wrongwrong Date: Sat, 20 Jan 2024 12:10:52 +0900 Subject: [PATCH 3/5] Introduce a common process to retrieve Kotlin parameters and replace existing In addition, kotlinFunction calls were replaced with calls from the cache. The hasRequiredMarker process had no problem without the check, so the findKotlinParameterName check process was removed. --- .../kotlin/KotlinAnnotationIntrospector.kt | 37 ++++++---------- .../KotlinNamesAnnotationIntrospector.kt | 43 +------------------ .../jackson/module/kotlin/ReflectionCache.kt | 9 ++++ 3 files changed, 23 insertions(+), 66 deletions(-) diff --git a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinAnnotationIntrospector.kt b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinAnnotationIntrospector.kt index 073ee2a9f..270e510f4 100644 --- a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinAnnotationIntrospector.kt +++ b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinAnnotationIntrospector.kt @@ -14,16 +14,20 @@ import java.lang.reflect.Method import kotlin.reflect.KClass import kotlin.reflect.KFunction import kotlin.reflect.KMutableProperty1 +import kotlin.reflect.KParameter import kotlin.reflect.KProperty1 import kotlin.reflect.KType import kotlin.reflect.full.createType import kotlin.reflect.full.declaredMemberProperties import kotlin.reflect.full.memberProperties import kotlin.reflect.full.valueParameters -import kotlin.reflect.jvm.* +import kotlin.reflect.jvm.javaField +import kotlin.reflect.jvm.javaGetter +import kotlin.reflect.jvm.javaSetter +import kotlin.reflect.jvm.javaType +import kotlin.reflect.jvm.kotlinProperty import kotlin.time.Duration - internal class KotlinAnnotationIntrospector( private val context: Module.SetupContext, private val cache: ReflectionCache, @@ -169,11 +173,11 @@ internal class KotlinAnnotationIntrospector( } // Is the member method a regular method of the data class or - private fun Method.getRequiredMarkerFromAccessorLikeMethod(): Boolean? = this.kotlinFunction?.let { method -> + private fun Method.getRequiredMarkerFromAccessorLikeMethod(): Boolean? = cache.kotlinFromJava(this)?.let { func -> val byAnnotation = this.isRequiredByAnnotation() return when { - method.isGetterLike() -> requiredAnnotationOrNullability(byAnnotation, method.returnType.isRequired()) - method.isSetterLike() -> requiredAnnotationOrNullability(byAnnotation, method.isMethodParameterRequired(0)) + func.isGetterLike() -> requiredAnnotationOrNullability(byAnnotation, func.returnType.isRequired()) + func.isSetterLike() -> requiredAnnotationOrNullability(byAnnotation, func.valueParameters[0].isRequired()) else -> null } } @@ -182,37 +186,22 @@ internal class KotlinAnnotationIntrospector( private fun KFunction<*>.isSetterLike(): Boolean = parameters.size == 2 && returnType == UNIT_TYPE private fun AnnotatedParameter.hasRequiredMarker(): Boolean? { - val member = this.member val byAnnotation = this.getAnnotation(JsonProperty::class.java)?.required - - val byNullability = when (member) { - is Constructor<*> -> member.kotlinFunction?.isConstructorParameterRequired(index) - is Method -> member.kotlinFunction?.isMethodParameterRequired(index) - else -> null - } + val byNullability = cache.findKotlinParameter(this)?.isRequired() return requiredAnnotationOrNullability(byAnnotation, byNullability) } private fun AnnotatedMethod.findValueClassReturnType() = cache.findValueClassReturnType(this) - private fun KFunction<*>.isConstructorParameterRequired(index: Int): Boolean { - return isParameterRequired(index) - } - - private fun KFunction<*>.isMethodParameterRequired(index: Int): Boolean { - return isParameterRequired(index + 1) - } - - private fun KFunction<*>.isParameterRequired(index: Int): Boolean { - val param = parameters[index] - val paramType = param.type + private fun KParameter.isRequired(): Boolean { + val paramType = type val isPrimitive = when (val javaType = paramType.javaType) { is Class<*> -> javaType.isPrimitive else -> false } - return !paramType.isMarkedNullable && !param.isOptional && !param.isVararg && + return !paramType.isMarkedNullable && !isOptional && !isVararg && !(isPrimitive && !context.isEnabled(DeserializationFeature.FAIL_ON_NULL_FOR_PRIMITIVES)) } diff --git a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinNamesAnnotationIntrospector.kt b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinNamesAnnotationIntrospector.kt index 710e56a1a..7e0bdbf1f 100644 --- a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinNamesAnnotationIntrospector.kt +++ b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinNamesAnnotationIntrospector.kt @@ -9,21 +9,16 @@ import com.fasterxml.jackson.databind.introspect.AnnotatedMember import com.fasterxml.jackson.databind.introspect.AnnotatedMethod import com.fasterxml.jackson.databind.introspect.AnnotatedParameter import com.fasterxml.jackson.databind.introspect.NopAnnotationIntrospector -import java.lang.reflect.Constructor -import java.lang.reflect.Method import java.util.Locale import kotlin.reflect.KClass import kotlin.reflect.KFunction -import kotlin.reflect.KParameter import kotlin.reflect.full.companionObject import kotlin.reflect.full.declaredFunctions import kotlin.reflect.full.hasAnnotation import kotlin.reflect.full.memberProperties import kotlin.reflect.full.primaryConstructor -import kotlin.reflect.jvm.internal.KotlinReflectionInternalError import kotlin.reflect.jvm.javaGetter import kotlin.reflect.jvm.javaType -import kotlin.reflect.jvm.kotlinFunction internal class KotlinNamesAnnotationIntrospector( private val cache: ReflectionCache, @@ -120,43 +115,7 @@ internal class KotlinNamesAnnotationIntrospector( } } - @Suppress("UNCHECKED_CAST") - private fun findKotlinParameterName(param: AnnotatedParameter): String? { - return if (param.declaringClass.isKotlinClass()) { - val member = param.owner.member - if (member is Constructor<*>) { - val ctor = (member as Constructor) - val ctorParmCount = ctor.parameterTypes.size - val ktorParmCount = try { ctor.kotlinFunction?.parameters?.size ?: 0 } - catch (ex: KotlinReflectionInternalError) { 0 } - catch (ex: UnsupportedOperationException) { 0 } - if (ktorParmCount > 0 && ktorParmCount == ctorParmCount) { - ctor.kotlinFunction?.parameters?.get(param.index)?.name - } else { - null - } - } else if (member is Method) { - try { - val temp = member.kotlinFunction - - val firstParamKind = temp?.parameters?.firstOrNull()?.kind - val idx = if (firstParamKind != KParameter.Kind.VALUE) param.index + 1 else param.index - val parmCount = temp?.parameters?.size ?: 0 - if (parmCount > idx) { - temp?.parameters?.get(idx)?.name - } else { - null - } - } catch (ex: KotlinReflectionInternalError) { - null - } - } else { - null - } - } else { - null - } - } + private fun findKotlinParameterName(param: AnnotatedParameter): String? = cache.findKotlinParameter(param)?.name } // if has parameters, is a Kotlin class, and the parameters all have parameter annotations, then pretend we have a JsonCreator diff --git a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/ReflectionCache.kt b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/ReflectionCache.kt index fa804a2a1..9cb9b0b0a 100644 --- a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/ReflectionCache.kt +++ b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/ReflectionCache.kt @@ -3,6 +3,7 @@ package com.fasterxml.jackson.module.kotlin import com.fasterxml.jackson.databind.introspect.AnnotatedConstructor import com.fasterxml.jackson.databind.introspect.AnnotatedMember import com.fasterxml.jackson.databind.introspect.AnnotatedMethod +import com.fasterxml.jackson.databind.introspect.AnnotatedParameter import com.fasterxml.jackson.databind.introspect.AnnotatedWithParams import com.fasterxml.jackson.databind.util.LRUMap import java.io.Serializable @@ -12,7 +13,9 @@ import java.lang.reflect.Method import java.util.* import kotlin.reflect.KClass import kotlin.reflect.KFunction +import kotlin.reflect.KParameter import kotlin.reflect.full.memberProperties +import kotlin.reflect.full.valueParameters import kotlin.reflect.jvm.javaGetter import kotlin.reflect.jvm.kotlinFunction @@ -138,4 +141,10 @@ internal class ReflectionCache(reflectionCacheSize: Int) : Serializable { val value = ValueClassBoxConverter(unboxedClass, valueClass) (valueClassBoxConverterCache.putIfAbsent(valueClass, value) ?: value) } + + fun findKotlinParameter(param: AnnotatedParameter): KParameter? = when (val owner = param.owner.member) { + is Constructor<*> -> kotlinFromJava(owner) + is Method -> kotlinFromJava(owner) + else -> null + }?.valueParameters?.get(param.index) } From 6c94512133ecea885ed2d833983e568806184c3e Mon Sep 17 00:00:00 2001 From: wrongwrong Date: Sat, 20 Jan 2024 12:20:55 +0900 Subject: [PATCH 4/5] Apply common parameter acquisition process --- .../jackson/module/kotlin/KotlinAnnotationIntrospector.kt | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinAnnotationIntrospector.kt b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinAnnotationIntrospector.kt index 270e510f4..0fa688d9a 100644 --- a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinAnnotationIntrospector.kt +++ b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinAnnotationIntrospector.kt @@ -99,12 +99,7 @@ internal class KotlinAnnotationIntrospector( if (!useJavaDurationConversion) return null return (a as? AnnotatedParameter)?.let { param -> - val function: KFunction<*> = when (val owner = param.owner.member) { - is Constructor<*> -> cache.kotlinFromJava(owner) - is Method -> cache.kotlinFromJava(owner) - else -> null - } ?: return@let null - val valueParameter = function.valueParameters[a.index] + val valueParameter = cache.findKotlinParameter(param) ?: return@let null if (valueParameter.type.classifier == Duration::class) { JavaToKotlinDurationConverter From 38fca7b7dd205d01670e3208f565af631b37ef9e Mon Sep 17 00:00:00 2001 From: wrongwrong Date: Sat, 20 Jan 2024 12:29:18 +0900 Subject: [PATCH 5/5] Update release notes wrt #760 --- release-notes/CREDITS-2.x | 1 + release-notes/VERSION-2.x | 1 + 2 files changed, 2 insertions(+) diff --git a/release-notes/CREDITS-2.x b/release-notes/CREDITS-2.x index e2516b91e..d6f45d6d4 100644 --- a/release-notes/CREDITS-2.x +++ b/release-notes/CREDITS-2.x @@ -18,6 +18,7 @@ Contributors: # 2.17.0 (not yet released) WrongWrong (@k163377) +* #760: Improved processing related to parameter parsing on Kotlin. * #759: Organize internal commons. * #758: Deprecated SingletonSupport. * #755: Changes in constructor invocation and argument management. diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x index 8f077ea83..8fa4a95b5 100644 --- a/release-notes/VERSION-2.x +++ b/release-notes/VERSION-2.x @@ -18,6 +18,7 @@ Co-maintainers: 2.17.0 (not yet released) +#760: Caching is now applied to the entire parameter parsing process on Kotlin. #758: Deprecated SingletonSupport and related properties to be consistent with KotlinFeature.SingletonSupport. #755: Changes in constructor invocation and argument management. This change degrades performance in cases where the constructor is called without default arguments, but improves performance in other cases.