From 6ced6853f5bc254d4e6c537afb732320aaffb54c Mon Sep 17 00:00:00 2001 From: wrongwrong Date: Mon, 13 Jan 2025 17:28:57 +0900 Subject: [PATCH 1/6] Commonize parameter acquisition --- .../kotlin/KotlinNamesAnnotationIntrospector.kt | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) 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 204ae7d4..272ef52d 100644 --- a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinNamesAnnotationIntrospector.kt +++ b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinNamesAnnotationIntrospector.kt @@ -73,14 +73,12 @@ internal class KotlinNamesAnnotationIntrospector( } override fun refineDeserializationType(config: MapperConfig<*>, a: Annotated, baseType: JavaType): JavaType = - (a as? AnnotatedParameter)?.let { _ -> - cache.findKotlinParameter(a)?.let { param -> - val rawType = a.rawType - (param.type.classifier as? KClass<*>) - ?.java - ?.takeIf { it.isUnboxableValueClass() && it != rawType } - ?.let { config.constructType(it) } - } + findKotlinParameter(a)?.let { param -> + val rawType = a.rawType + (param.type.classifier as? KClass<*>) + ?.java + ?.takeIf { it.isUnboxableValueClass() && it != rawType } + ?.let { config.constructType(it) } } ?: baseType override fun findDefaultCreator( @@ -106,6 +104,9 @@ internal class KotlinNamesAnnotationIntrospector( } private fun findKotlinParameterName(param: AnnotatedParameter): String? = cache.findKotlinParameter(param)?.name + + private fun findKotlinParameter(param: Annotated) = (param as? AnnotatedParameter) + ?.let { cache.findKotlinParameter(it) } } // If it is not a Kotlin class or an Enum, Creator is not used From c346d40081e28727165a997552a6587665ee10e6 Mon Sep 17 00:00:00 2001 From: wrongwrong Date: Mon, 13 Jan 2025 17:44:26 +0900 Subject: [PATCH 2/6] Fixed StrictNullChecks backend to use JsonSetter from https://github.com/ProjectMapK/jackson-module-kogera/blob/ec6e5e644cc9574f461962e1246f87a42b8fab66/src/main/kotlin/io/github/projectmapk/jackson/module/kogera/annotationIntrospector/KotlinFallbackAnnotationIntrospector.kt#L116-L126 --- .../jackson/module/kotlin/KotlinModule.kt | 4 +++- .../KotlinNamesAnnotationIntrospector.kt | 24 +++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinModule.kt b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinModule.kt index d396324d..a7776780 100644 --- a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinModule.kt +++ b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinModule.kt @@ -109,7 +109,9 @@ class KotlinModule private constructor( nullIsSameAsDefault, useJavaDurationConversion )) - context.appendAnnotationIntrospector(KotlinNamesAnnotationIntrospector(cache, kotlinPropertyNameAsImplicitName)) + context.appendAnnotationIntrospector( + KotlinNamesAnnotationIntrospector(cache, strictNullChecks, kotlinPropertyNameAsImplicitName) + ) context.addDeserializers(KotlinDeserializers(cache, useJavaDurationConversion)) context.addKeyDeserializers(KotlinKeyDeserializers) 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 272ef52d..877e24d4 100644 --- a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinNamesAnnotationIntrospector.kt +++ b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinNamesAnnotationIntrospector.kt @@ -1,6 +1,8 @@ package com.fasterxml.jackson.module.kotlin import com.fasterxml.jackson.annotation.JsonProperty +import com.fasterxml.jackson.annotation.JsonSetter +import com.fasterxml.jackson.annotation.Nulls import com.fasterxml.jackson.databind.JavaType import com.fasterxml.jackson.databind.cfg.MapperConfig import com.fasterxml.jackson.databind.introspect.Annotated @@ -12,8 +14,10 @@ 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.collections.getOrNull import kotlin.reflect.KClass import kotlin.reflect.KFunction +import kotlin.reflect.KParameter import kotlin.reflect.full.hasAnnotation import kotlin.reflect.full.memberProperties import kotlin.reflect.full.primaryConstructor @@ -22,6 +26,7 @@ import kotlin.reflect.jvm.javaType internal class KotlinNamesAnnotationIntrospector( private val cache: ReflectionCache, + private val strictNullChecks: Boolean, private val kotlinPropertyNameAsImplicitName: Boolean ) : NopAnnotationIntrospector() { private fun getterNameFromJava(member: AnnotatedMethod): String? { @@ -81,6 +86,18 @@ internal class KotlinNamesAnnotationIntrospector( ?.let { config.constructType(it) } } ?: baseType + override fun findSetterInfo(ann: Annotated): JsonSetter.Value = ann.takeIf { strictNullChecks } + ?.let { _ -> + findKotlinParameter(ann)?.let { param -> + if (param.requireStrictNullCheck(ann.type)) { + JsonSetter.Value.forContentNulls(Nulls.FAIL) + } else { + null + } + } + } + ?: super.findSetterInfo(ann) + override fun findDefaultCreator( config: MapperConfig<*>, valueClass: AnnotatedClass, @@ -109,6 +126,13 @@ internal class KotlinNamesAnnotationIntrospector( ?.let { cache.findKotlinParameter(it) } } +private fun KParameter.markedNonNullAt(index: Int) = type.arguments.getOrNull(index)?.type?.isMarkedNullable == false + +private fun KParameter.requireStrictNullCheck(type: JavaType): Boolean = + ((type.isArrayType || type.isCollectionLikeType) && this.markedNonNullAt(0)) || + (type.isMapLikeType && this.markedNonNullAt(1)) + + // 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 } From d1f63af8fce7ae5f21efdbcfe0bcd7cada36dd1e Mon Sep 17 00:00:00 2001 From: wrongwrong Date: Mon, 13 Jan 2025 17:48:36 +0900 Subject: [PATCH 3/6] Remove old StrictNullChecks backend --- .../jackson/module/kotlin/KotlinModule.kt | 2 +- .../module/kotlin/KotlinValueInstantiator.kt | 35 ------------------- 2 files changed, 1 insertion(+), 36 deletions(-) diff --git a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinModule.kt b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinModule.kt index a7776780..c2473ca7 100644 --- a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinModule.kt +++ b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinModule.kt @@ -95,7 +95,7 @@ class KotlinModule private constructor( val cache = ReflectionCache(reflectionCacheSize) - context.addValueInstantiators(KotlinInstantiators(cache, nullToEmptyCollection, nullToEmptyMap, nullIsSameAsDefault, strictNullChecks)) + context.addValueInstantiators(KotlinInstantiators(cache, nullToEmptyCollection, nullToEmptyMap, nullIsSameAsDefault)) if (singletonSupport) { context.addBeanDeserializerModifier(KotlinBeanDeserializerModifier) diff --git a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinValueInstantiator.kt b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinValueInstantiator.kt index 8c3f2529..176d494b 100644 --- a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinValueInstantiator.kt +++ b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinValueInstantiator.kt @@ -12,9 +12,7 @@ import com.fasterxml.jackson.databind.deser.ValueInstantiators import com.fasterxml.jackson.databind.deser.impl.PropertyValueBuffer import com.fasterxml.jackson.databind.deser.std.StdValueInstantiator import java.lang.reflect.TypeVariable -import kotlin.reflect.KParameter import kotlin.reflect.KType -import kotlin.reflect.KTypeProjection import kotlin.reflect.jvm.javaType internal class KotlinValueInstantiator( @@ -23,15 +21,12 @@ internal class KotlinValueInstantiator( private val nullToEmptyCollection: Boolean, private val nullToEmptyMap: Boolean, private val nullIsSameAsDefault: Boolean, - private val strictNullChecks: Boolean ) : StdValueInstantiator(src) { private fun JavaType.requireEmptyValue() = (nullToEmptyCollection && this.isCollectionLikeType) || (nullToEmptyMap && this.isMapLikeType) private fun KType.isGenericTypeVar() = javaType is TypeVariable<*> - private fun List.markedNonNullAt(index: Int) = getOrNull(index)?.type?.isMarkedNullable == false - // If the argument is a value class that wraps nullable and non-null, // and the input is explicit null, the value class is instantiated with null as input. private fun requireValueClassSpecialNullValue( @@ -101,34 +96,6 @@ internal class KotlinValueInstantiator( ).wrapWithPath(this.valueClass, jsonProp.name) } } - } else if (strictNullChecks) { - val arguments = paramType.arguments - - var paramTypeStr: String? = null - var itemType: KType? = null - - if (propType.isCollectionLikeType && arguments.markedNonNullAt(0) && (paramVal as Collection<*>).any { it == null }) { - paramTypeStr = "collection" - itemType = arguments[0].type - } - - if (propType.isMapLikeType && arguments.markedNonNullAt(1) && (paramVal as Map<*, *>).any { it.value == null }) { - paramTypeStr = "map" - itemType = arguments[1].type - } - - if (propType.isArrayType && arguments.markedNonNullAt(0) && (paramVal as Array<*>).any { it == null }) { - paramTypeStr = "array" - itemType = arguments[0].type - } - - if (paramTypeStr != null && itemType != null) { - throw MissingKotlinParameterException( - parameter = paramDef, - processor = ctxt.parser, - msg = "Instantiation of $itemType $paramType failed for JSON property ${jsonProp.name} due to null value in a $paramType that does not allow null values" - ).wrapWithPath(this.valueClass, jsonProp.name) - } } bucket[paramDef] = paramVal @@ -147,7 +114,6 @@ internal class KotlinInstantiators( private val nullToEmptyCollection: Boolean, private val nullToEmptyMap: Boolean, private val nullIsSameAsDefault: Boolean, - private val strictNullChecks: Boolean ) : ValueInstantiators { override fun findValueInstantiator( deserConfig: DeserializationConfig, @@ -162,7 +128,6 @@ internal class KotlinInstantiators( nullToEmptyCollection, nullToEmptyMap, nullIsSameAsDefault, - strictNullChecks ) } else { // TODO: return defaultInstantiator and let default method parameters and nullability go unused? or die with exception: From 9d66d6d8ecab2fef3bfb97e8f116a7e2cf1de3d1 Mon Sep 17 00:00:00 2001 From: wrongwrong Date: Mon, 13 Jan 2025 17:49:55 +0900 Subject: [PATCH 4/6] Fix tests --- .../module/kotlin/KotlinInstantiatorsTest.kt | 1 - .../module/kotlin/test/StrictNullChecksTest.kt | 14 +++++++------- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/test/kotlin/com/fasterxml/jackson/module/kotlin/KotlinInstantiatorsTest.kt b/src/test/kotlin/com/fasterxml/jackson/module/kotlin/KotlinInstantiatorsTest.kt index 628f5afc..afb252bc 100644 --- a/src/test/kotlin/com/fasterxml/jackson/module/kotlin/KotlinInstantiatorsTest.kt +++ b/src/test/kotlin/com/fasterxml/jackson/module/kotlin/KotlinInstantiatorsTest.kt @@ -13,7 +13,6 @@ class KotlinInstantiatorsTest { nullToEmptyCollection = false, nullToEmptyMap = false, nullIsSameAsDefault = false, - strictNullChecks = false ) @Test diff --git a/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/StrictNullChecksTest.kt b/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/StrictNullChecksTest.kt index ebf1ad61..1522830d 100644 --- a/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/StrictNullChecksTest.kt +++ b/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/StrictNullChecksTest.kt @@ -1,8 +1,8 @@ package com.fasterxml.jackson.module.kotlin.test import com.fasterxml.jackson.databind.ObjectMapper +import com.fasterxml.jackson.databind.exc.InvalidNullException import com.fasterxml.jackson.module.kotlin.KotlinFeature.StrictNullChecks -import com.fasterxml.jackson.module.kotlin.MissingKotlinParameterException import com.fasterxml.jackson.module.kotlin.kotlinModule import com.fasterxml.jackson.module.kotlin.readValue import org.junit.jupiter.api.Assertions.assertArrayEquals @@ -30,7 +30,7 @@ class StrictNullChecksTest { @Test fun testListOfInt() { - assertThrows { + assertThrows { val json = """{"samples":[1, null]}""" mapper.readValue(json) } @@ -60,7 +60,7 @@ class StrictNullChecksTest { @Test fun testArrayOfInt() { - assertThrows { + assertThrows { val json = """{"samples":[1, null]}""" mapper.readValue(json) } @@ -90,7 +90,7 @@ class StrictNullChecksTest { @Test fun testMapOfStringToIntWithNullValue() { - assertThrows { + assertThrows { val json = """{ "samples": { "key": null } }""" mapper.readValue(json) } @@ -119,7 +119,7 @@ class StrictNullChecksTest { @Disabled // this is a hard problem to solve and is currently not addressed @Test fun testListOfGenericWithNullValue() { - assertThrows { + assertThrows { val json = """{"samples":[1, null]}""" mapper.readValue>>(json) } @@ -135,7 +135,7 @@ class StrictNullChecksTest { @Disabled // this is a hard problem to solve and is currently not addressed @Test fun testMapOfGenericWithNullValue() { - assertThrows { + assertThrows { val json = """{ "samples": { "key": null } }""" mapper.readValue>>(json) } @@ -151,7 +151,7 @@ class StrictNullChecksTest { @Disabled // this is a hard problem to solve and is currently not addressed @Test fun testArrayOfGenericWithNullValue() { - assertThrows { + assertThrows { val json = """{"samples":[1, null]}""" mapper.readValue>>(json) } From 765e1a01a1e85f1ff8fde23212a9c754d7a639b5 Mon Sep 17 00:00:00 2001 From: wrongwrong Date: Mon, 13 Jan 2025 17:58:30 +0900 Subject: [PATCH 5/6] Porting test from https://github.com/ProjectMapK/jackson-module-kogera/blob/ecc766d97dc4c2bec574fba7cc6bdeacc2b07c29/src/test/kotlin/io/github/projectmapk/jackson/module/kogera/zIntegration/deser/StrictNullChecksTest.kt --- .../deser/StrictNullChecksTest.kt | 139 ++++++++++++++++++ 1 file changed, 139 insertions(+) create mode 100644 src/test/kotlin/com/fasterxml/jackson/module/kotlin/kogeraIntegration/deser/StrictNullChecksTest.kt diff --git a/src/test/kotlin/com/fasterxml/jackson/module/kotlin/kogeraIntegration/deser/StrictNullChecksTest.kt b/src/test/kotlin/com/fasterxml/jackson/module/kotlin/kogeraIntegration/deser/StrictNullChecksTest.kt new file mode 100644 index 00000000..3cad0edc --- /dev/null +++ b/src/test/kotlin/com/fasterxml/jackson/module/kotlin/kogeraIntegration/deser/StrictNullChecksTest.kt @@ -0,0 +1,139 @@ +package com.fasterxml.jackson.module.kotlin.kogeraIntegration.deser + +import com.fasterxml.jackson.annotation.JsonSetter +import com.fasterxml.jackson.annotation.Nulls +import com.fasterxml.jackson.databind.ObjectMapper +import com.fasterxml.jackson.databind.exc.InvalidNullException +import com.fasterxml.jackson.module.kotlin.KotlinFeature +import com.fasterxml.jackson.module.kotlin.KotlinModule +import com.fasterxml.jackson.module.kotlin.readValue +import org.junit.jupiter.api.Assertions +import org.junit.jupiter.api.Nested +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.assertThrows + +class StrictNullChecksTest { + val mapper: ObjectMapper = ObjectMapper() + .registerModule( + KotlinModule.Builder() + .enable(KotlinFeature.StrictNullChecks) + .build() + ) + + class ArrayWrapper(val value: Array) + data class ListWrapper(val value: List) + data class MapWrapper(val value: Map) + + @Nested + inner class NonNullInput { + @Test + fun array() { + val expected = ArrayWrapper(arrayOf(1)) + val src = mapper.writeValueAsString(expected) + val result = mapper.readValue(src) + + Assertions.assertArrayEquals(expected.value, result.value) + } + + @Test + fun list() { + val expected = ListWrapper(listOf(1)) + val src = mapper.writeValueAsString(expected) + val result = mapper.readValue(src) + + Assertions.assertEquals(expected, result) + } + + @Test + fun map() { + val expected = MapWrapper(mapOf("foo" to 1)) + val src = mapper.writeValueAsString(expected) + val result = mapper.readValue(src) + + Assertions.assertEquals(expected, result) + } + } + + data class AnyWrapper(val value: Any) + + @Nested + inner class NullInput { + @Test + fun array() { + val src = mapper.writeValueAsString(AnyWrapper(arrayOf(null))) + assertThrows { mapper.readValue(src) } + } + + @Test + fun list() { + val src = mapper.writeValueAsString(AnyWrapper(arrayOf(null))) + assertThrows { mapper.readValue(src) } + } + + @Test + fun map() { + val src = mapper.writeValueAsString(AnyWrapper(mapOf("foo" to null))) + assertThrows { mapper.readValue(src) } + } + } + + class ContentNullsSkipArrayWrapper(@JsonSetter(contentNulls = Nulls.SKIP) val value: Array) + data class ContentNullsSkipListWrapper(@JsonSetter(contentNulls = Nulls.SKIP) val value: List) + data class ContentNullsSkipMapWrapper(@JsonSetter(contentNulls = Nulls.SKIP) val value: Map) + + @Nested + inner class CustomByAnnotationTest { + @Test + fun array() { + val expected = ContentNullsSkipArrayWrapper(emptyArray()) + val src = mapper.writeValueAsString(AnyWrapper(arrayOf(null))) + val result = mapper.readValue(src) + + Assertions.assertArrayEquals(expected.value, result.value) + } + + @Test + fun list() { + val expected = ContentNullsSkipListWrapper(emptyList()) + val src = mapper.writeValueAsString(AnyWrapper(listOf(null))) + val result = mapper.readValue(src) + + Assertions.assertEquals(expected, result) + } + + @Test + fun map() { + val expected = ContentNullsSkipMapWrapper(emptyMap()) + val src = mapper.writeValueAsString(AnyWrapper(mapOf("foo" to null))) + val result = mapper.readValue(src) + + Assertions.assertEquals(expected, result) + } + } + + class AnnotatedArrayWrapper(@JsonSetter(nulls = Nulls.SKIP) val value: Array = emptyArray()) + data class AnnotatedListWrapper(@JsonSetter(nulls = Nulls.SKIP) val value: List = emptyList()) + data class AnnotatedMapWrapper(@JsonSetter(nulls = Nulls.SKIP) val value: Map = emptyMap()) + + // If Default is specified by annotation, it is not overridden. + @Nested + inner class AnnotatedNullInput { + @Test + fun array() { + val src = mapper.writeValueAsString(AnyWrapper(arrayOf(null))) + assertThrows { mapper.readValue(src) } + } + + @Test + fun list() { + val src = mapper.writeValueAsString(AnyWrapper(arrayOf(null))) + assertThrows { mapper.readValue(src) } + } + + @Test + fun map() { + val src = mapper.writeValueAsString(AnyWrapper(mapOf("foo" to null))) + assertThrows { mapper.readValue(src) } + } + } +} \ No newline at end of file From fb73525ad457d7f9cc6d3650b7084e046b08234e Mon Sep 17 00:00:00 2001 From: wrongwrong Date: Mon, 13 Jan 2025 18:20:57 +0900 Subject: [PATCH 6/6] Fix cmp --- pom.xml | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 62e5b277..7d249947 100644 --- a/pom.xml +++ b/pom.xml @@ -254,7 +254,15 @@ com.fasterxml.jackson.module.kotlin.KotlinModule#getSingletonSupport() com.fasterxml.jackson.module.kotlin.SingletonSupport - + + com.fasterxml.jackson.module.kotlin.KotlinNamesAnnotationIntrospector#KotlinNamesAnnotationIntrospector(com.fasterxml.jackson.module.kotlin.ReflectionCache,boolean) + + + com.fasterxml.jackson.module.kotlin.KotlinValueInstantiator#KotlinValueInstantiator(com.fasterxml.jackson.databind.deser.std.StdValueInstantiator,com.fasterxml.jackson.module.kotlin.ReflectionCache,boolean,boolean,boolean,boolean) + + + com.fasterxml.jackson.module.kotlin.KotlinInstantiators#KotlinInstantiators(com.fasterxml.jackson.module.kotlin.ReflectionCache,boolean,boolean,boolean,boolean) +