diff --git a/release-notes/CREDITS-2.x b/release-notes/CREDITS-2.x index d2b9c089..1daa0ec5 100644 --- a/release-notes/CREDITS-2.x +++ b/release-notes/CREDITS-2.x @@ -18,6 +18,7 @@ Contributors: # 2.18.0 (not yet released) WrongWrong (@k163377) +* #818: Optimize the search process for creators * #817: Fixed nullability of convertValue function argument * #782: Organize deprecated contents * #542: Remove meaningless checks and properties in KNAI diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x index dd7fa587..f86dd318 100644 --- a/release-notes/VERSION-2.x +++ b/release-notes/VERSION-2.x @@ -18,6 +18,11 @@ Co-maintainers: 2.18.0 (not yet released) +#818: The implementation of the search process for the `JsonCreator` (often the primary constructor) + used by default for deserialization has been changed to `AnnotationIntrospector#findDefaultCreator`. + This has improved first-time processing performance and memory usage. + It also solves the problem of `findCreatorAnnotation` results by `AnnotationIntrospector` registered by the user + being ignored depending on the order in which modules are registered. #817: The convertValue extension function now accepts null #803: Kotlin has been upgraded to 1.8.10. The reason 1.8.22 is not used is to avoid KT-65156. diff --git a/src/main/kotlin/tools/jackson/module/kotlin/KotlinNamesAnnotationIntrospector.kt b/src/main/kotlin/tools/jackson/module/kotlin/KotlinNamesAnnotationIntrospector.kt index 392638b6..59e537e5 100644 --- a/src/main/kotlin/tools/jackson/module/kotlin/KotlinNamesAnnotationIntrospector.kt +++ b/src/main/kotlin/tools/jackson/module/kotlin/KotlinNamesAnnotationIntrospector.kt @@ -1,20 +1,19 @@ package tools.jackson.module.kotlin -import com.fasterxml.jackson.annotation.JsonCreator import com.fasterxml.jackson.annotation.JsonProperty import tools.jackson.databind.JavaType import tools.jackson.databind.cfg.MapperConfig import tools.jackson.databind.introspect.Annotated -import tools.jackson.databind.introspect.AnnotatedConstructor +import tools.jackson.databind.introspect.AnnotatedClass import tools.jackson.databind.introspect.AnnotatedMember import tools.jackson.databind.introspect.AnnotatedMethod import tools.jackson.databind.introspect.AnnotatedParameter import tools.jackson.databind.introspect.NopAnnotationIntrospector +import tools.jackson.databind.introspect.PotentialCreator +import java.lang.reflect.Constructor import java.util.Locale import kotlin.reflect.KClass import kotlin.reflect.KFunction -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 @@ -84,62 +83,44 @@ internal class KotlinNamesAnnotationIntrospector( } } ?: baseType - private fun hasCreatorAnnotation(member: AnnotatedConstructor): Boolean { - // don't add a JsonCreator to any constructor if one is declared already - - val kClass = member.declaringClass.kotlin - val kConstructor = cache.kotlinFromJava(member.annotated) ?: return false - - // TODO: should we do this check or not? It could cause failures if we miss another way a property could be set - // val requiredProperties = kClass.declaredMemberProperties.filter {!it.returnType.isMarkedNullable }.map { it.name }.toSet() - // val areAllRequiredParametersInConstructor = kConstructor.parameters.all { requiredProperties.contains(it.name) } + override fun findDefaultCreator( + config: MapperConfig<*>, + valueClass: AnnotatedClass, + declaredConstructors: List, + declaredFactories: List + ): PotentialCreator? { + val kClass = valueClass.creatableKotlinClass() ?: return null val propertyNames = kClass.memberProperties.map { it.name }.toSet() - return when { - kConstructor.isPossibleSingleString(propertyNames) -> false - kConstructor.parameters.any { it.name == null } -> false - !kClass.isPrimaryConstructor(kConstructor) -> false - else -> { - val anyConstructorHasJsonCreator = kClass.constructors - .filterOutSingleStringCallables(propertyNames) - .any { it.hasAnnotation() } - - val anyCompanionMethodIsJsonCreator = member.type.rawClass.kotlin.companionObject?.declaredFunctions - ?.filterOutSingleStringCallables(propertyNames) - ?.any { it.hasAnnotation() && it.hasAnnotation() } - ?: false - - !(anyConstructorHasJsonCreator || anyCompanionMethodIsJsonCreator) - } + val defaultCreator = kClass.let { _ -> + // By default, the primary constructor or the only publicly available constructor may be used + val ctor = kClass.primaryConstructor ?: kClass.constructors.takeIf { it.size == 1 }?.single() + ctor?.takeIf { it.isPossibleCreator(propertyNames) } } - } - - // TODO: possible work around for JsonValue class that requires the class constructor to have the JsonCreator(DELEGATED) set? - // since we infer the creator at times for these methods, the wrong mode could be implied. - override fun findCreatorAnnotation(config: MapperConfig<*>, ann: Annotated): JsonCreator.Mode? { - if (ann !is AnnotatedConstructor || !ann.isKotlinConstructorWithParameters()) return null + ?: return null - return JsonCreator.Mode.DEFAULT.takeIf { - cache.checkConstructorIsCreatorAnnotated(ann) { hasCreatorAnnotation(it) } + return declaredConstructors.find { + // To avoid problems with constructors that include `value class` as an argument, + // convert to `KFunction` and compare + cache.kotlinFromJava(it.creator().annotated as Constructor<*>) == defaultCreator } } 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 -private fun AnnotatedConstructor.isKotlinConstructorWithParameters(): Boolean = - parameterCount > 0 && declaringClass.isKotlinClass() && !declaringClass.isEnum +// If it is not a Kotlin class or an Enum, Creator is not used +private fun AnnotatedClass.creatableKotlinClass(): KClass<*>? = annotated + .takeIf { it.isKotlinClass() && !it.isEnum } + ?.kotlin -private fun KFunction<*>.isPossibleSingleString(propertyNames: Set): Boolean = parameters.size == 1 && - parameters[0].name !in propertyNames && - parameters[0].type.javaType == String::class.java && - !parameters[0].hasAnnotation() +private fun KFunction<*>.isPossibleCreator(propertyNames: Set): Boolean = 0 < parameters.size + && !isPossibleSingleString(propertyNames) + && parameters.none { it.name == null } -private fun Collection>.filterOutSingleStringCallables(propertyNames: Set): Collection> = - this.filter { !it.isPossibleSingleString(propertyNames) } - -private fun KClass<*>.isPrimaryConstructor(kConstructor: KFunction<*>) = this.primaryConstructor.let { - it == kConstructor || (it == null && this.constructors.size == 1) -} +private fun KFunction<*>.isPossibleSingleString(propertyNames: Set): Boolean = parameters.singleOrNull()?.let { + it.name !in propertyNames + && it.type.javaType == String::class.java + && !it.hasAnnotation() +} == true diff --git a/src/main/kotlin/tools/jackson/module/kotlin/ReflectionCache.kt b/src/main/kotlin/tools/jackson/module/kotlin/ReflectionCache.kt index ac6f489a..bff8877f 100644 --- a/src/main/kotlin/tools/jackson/module/kotlin/ReflectionCache.kt +++ b/src/main/kotlin/tools/jackson/module/kotlin/ReflectionCache.kt @@ -22,7 +22,7 @@ import kotlin.reflect.jvm.kotlinFunction internal class ReflectionCache(reflectionCacheSize: Int) : Serializable { companion object { // Increment is required when properties that use LRUMap are changed. - private const val serialVersionUID = 2L + private const val serialVersionUID = 3L } sealed class BooleanTriState(val value: Boolean?) { @@ -47,7 +47,6 @@ internal class ReflectionCache(reflectionCacheSize: Int) : Serializable { private val javaExecutableToKotlin = SimpleLookupCache>(reflectionCacheSize, reflectionCacheSize) private val javaExecutableToValueCreator = SimpleLookupCache>(reflectionCacheSize, reflectionCacheSize) - private val javaConstructorIsCreatorAnnotated = SimpleLookupCache(reflectionCacheSize, reflectionCacheSize) private val javaMemberIsRequired = SimpleLookupCache(reflectionCacheSize, reflectionCacheSize) // Initial size is 0 because the value class is not always used @@ -103,9 +102,6 @@ internal class ReflectionCache(reflectionCacheSize: Int) : Serializable { ) } // we cannot reflect this method so do the default Java-ish behavior - fun checkConstructorIsCreatorAnnotated(key: AnnotatedConstructor, calc: (AnnotatedConstructor) -> Boolean): Boolean = javaConstructorIsCreatorAnnotated.get(key) - ?: calc(key).let { javaConstructorIsCreatorAnnotated.putIfAbsent(key, it) ?: it } - fun javaMemberIsRequired(key: AnnotatedMember, calc: (AnnotatedMember) -> Boolean?): Boolean? = javaMemberIsRequired.get(key)?.value ?: calc(key).let { javaMemberIsRequired.putIfAbsent(key, BooleanTriState.fromBoolean(it))?.value ?: it }