Skip to content

Improved processing related to parameter parsing on Kotlin #760

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jan 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions release-notes/CREDITS-2.x
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions release-notes/VERSION-2.x
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -95,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
Expand Down Expand Up @@ -169,11 +168,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
}
}
Expand All @@ -182,37 +181,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))
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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<Any>)
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
}
}
Comment on lines -123 to -159
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the previous implementation of AnnotatedParameter.hasRequiredMarker in KotlinAnnotationIntrospector, parameter acquisition was called without a check.
In other words, these check processes are now considered meaningless and have been removed.
https://github.com/FasterXML/jackson-module-kotlin/pull/760/files#diff-938d5c566367fbca40b3ef96c99967dba4cd563f3854370272e0ae6b5d0f7f0d

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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand Down Expand Up @@ -68,10 +71,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<Any>
val constructor = _withArgsCreator.annotated

javaExecutableToValueCreator.get(constructor)
?: kotlinFromJava(constructor)?.let {
Expand Down Expand Up @@ -114,7 +116,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

Expand All @@ -139,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)
}