Merge pull request #818 from k163377/feat/creator

Optimize the search process for creators

Author: k163377

pom.xml

@@ -273,6 +273,9 @@
                             <exclude>
                                 com.fasterxml.jackson.module.kotlin.KotlinNamesAnnotationIntrospector#KotlinNamesAnnotationIntrospector(com.fasterxml.jackson.module.kotlin.ReflectionCache,java.util.Set,boolean)
                             </exclude>
+                            <exclude>
+                                com.fasterxml.jackson.module.kotlin.ReflectionCache#checkConstructorIsCreatorAnnotated(com.fasterxml.jackson.databind.introspect.AnnotatedConstructor,kotlin.jvm.functions.Function1)
+                            </exclude>
 
                         </excludes>
                     </parameter>

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

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.

src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinNamesAnnotationIntrospector.kt

@@ -5,11 +5,14 @@ import com.fasterxml.jackson.annotation.JsonProperty
 import com.fasterxml.jackson.databind.JavaType
 import com.fasterxml.jackson.databind.cfg.MapperConfig
 import com.fasterxml.jackson.databind.introspect.Annotated
+import com.fasterxml.jackson.databind.introspect.AnnotatedClass
 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.NopAnnotationIntrospector
+import com.fasterxml.jackson.databind.introspect.PotentialCreator
+import java.lang.reflect.Constructor
 import java.util.Locale
 import kotlin.reflect.KClass
 import kotlin.reflect.KFunction
@@ -18,6 +21,7 @@ import kotlin.reflect.full.declaredFunctions
 import kotlin.reflect.full.hasAnnotation
 import kotlin.reflect.full.memberProperties
 import kotlin.reflect.full.primaryConstructor
+import kotlin.reflect.jvm.javaConstructor
 import kotlin.reflect.jvm.javaGetter
 import kotlin.reflect.jvm.javaType
 
@@ -84,62 +88,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<PotentialCreator>,
+        declaredFactories: List<PotentialCreator>
+    ): 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<JsonCreator>() }
-
-                val anyCompanionMethodIsJsonCreator = member.type.rawClass.kotlin.companionObject?.declaredFunctions
-                    ?.filterOutSingleStringCallables(propertyNames)
-                    ?.any { it.hasAnnotation<JsonCreator>() && it.hasAnnotation<JvmStatic>() }
-                    ?: 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<String>): Boolean = parameters.size == 1 &&
-        parameters[0].name !in propertyNames &&
-        parameters[0].type.javaType == String::class.java &&
-        !parameters[0].hasAnnotation<JsonProperty>()
+private fun KFunction<*>.isPossibleCreator(propertyNames: Set<String>): Boolean = 0 < parameters.size
+    && !isPossibleSingleString(propertyNames)
+    && parameters.none { it.name == null }
 
-private fun Collection<KFunction<*>>.filterOutSingleStringCallables(propertyNames: Set<String>): Collection<KFunction<*>> =
-    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<String>): Boolean = parameters.singleOrNull()?.let {
+    it.name !in propertyNames
+        && it.type.javaType == String::class.java
+        && !it.hasAnnotation<JsonProperty>()
+} == true

src/main/kotlin/com/fasterxml/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 = LRUMap<Executable, KFunction<*>>(reflectionCacheSize, reflectionCacheSize)
     private val javaExecutableToValueCreator = LRUMap<Executable, ValueCreator<*>>(reflectionCacheSize, reflectionCacheSize)
-    private val javaConstructorIsCreatorAnnotated = LRUMap<AnnotatedConstructor, Boolean>(reflectionCacheSize, reflectionCacheSize)
     private val javaMemberIsRequired = LRUMap<AnnotatedMember, BooleanTriState?>(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 }