From 8038218a1ba94e01bf994eac71a1994444372b72 Mon Sep 17 00:00:00 2001 From: wrongwrong Date: Thu, 20 Mar 2025 11:54:51 +0900 Subject: [PATCH 1/4] Added test to make sure no processing is done for Java DTO --- .../module/kotlin/test/github/GitHub922.kt | 32 +++++++++++++++++++ .../GitHub922RequiredCollectionsDtoJava.java | 29 +++++++++++++++++ 2 files changed, 61 insertions(+) create mode 100644 src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/github/GitHub922.kt create mode 100644 src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/github/GitHub922RequiredCollectionsDtoJava.java diff --git a/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/github/GitHub922.kt b/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/github/GitHub922.kt new file mode 100644 index 000000000..6c67e3c28 --- /dev/null +++ b/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/github/GitHub922.kt @@ -0,0 +1,32 @@ +package com.fasterxml.jackson.module.kotlin.test.github + +import com.fasterxml.jackson.databind.BeanDescription +import com.fasterxml.jackson.databind.ObjectMapper +import com.fasterxml.jackson.module.kotlin.KotlinFeature +import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper +import org.junit.jupiter.api.Assertions.assertTrue +import kotlin.test.Test + +class GitHub922 { + private inline fun ObjectMapper.introspectSerialization(): BeanDescription = + serializationConfig.introspect(serializationConfig.constructType(T::class.java)) + + private inline fun ObjectMapper.introspectDeserialization(): BeanDescription = + deserializationConfig.introspect(deserializationConfig.constructType(T::class.java)) + + private fun BeanDescription.isRequired(propertyName: String): Boolean = + this.findProperties().first { it.name == propertyName }.isRequired + + @Test + fun `nullToEmpty does not override specification by Java annotation`() { + val mapper = jacksonObjectMapper { + enable(KotlinFeature.NullToEmptyCollection) + enable(KotlinFeature.NullToEmptyMap) + } + + val desc = mapper.introspectDeserialization() + + assertTrue(desc.isRequired("list")) + assertTrue(desc.isRequired("map")) + } +} diff --git a/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/github/GitHub922RequiredCollectionsDtoJava.java b/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/github/GitHub922RequiredCollectionsDtoJava.java new file mode 100644 index 000000000..03df40ce1 --- /dev/null +++ b/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/github/GitHub922RequiredCollectionsDtoJava.java @@ -0,0 +1,29 @@ +package com.fasterxml.jackson.module.kotlin.test.github; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonProperty; + +import java.util.List; +import java.util.Map; + +public class GitHub922RequiredCollectionsDtoJava { + private final List list; + private final Map map; + + @JsonCreator + public GitHub922RequiredCollectionsDtoJava( + @JsonProperty(value = "list", required = true) List list, + @JsonProperty(value = "map", required = true) Map map + ) { + this.list = list; + this.map = map; + } + + public List getList() { + return list; + } + + public Map getMap() { + return map; + } +} From 380ff09afb3f9b04d736df23c0ea704cbd4b4f03 Mon Sep 17 00:00:00 2001 From: wrongwrong Date: Thu, 20 Mar 2025 11:58:51 +0900 Subject: [PATCH 2/4] Fixed hasRequiredMarker to only process content defined in Kotlin Processing efficiency has also been improved because the results of processing for Java are no longer cached. --- .../jackson/module/kotlin/KotlinAnnotationIntrospector.kt | 7 ++++--- 1 file changed, 4 insertions(+), 3 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 f8d51cb15..cf5db5698 100644 --- a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinAnnotationIntrospector.kt +++ b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinAnnotationIntrospector.kt @@ -36,8 +36,10 @@ internal class KotlinAnnotationIntrospector( // TODO: implement nullIsSameAsDefault flag, which represents when TRUE that if something has a default value, it can be passed a null to default it // this likely impacts this class to be accurate about what COULD be considered required - override fun hasRequiredMarker(m: AnnotatedMember): Boolean? { - val hasRequired = cache.javaMemberIsRequired(m) { + override fun hasRequiredMarker( + m: AnnotatedMember + ): Boolean? = m.takeIf { it.member.declaringClass.isKotlinClass() }?.let { _ -> + cache.javaMemberIsRequired(m) { try { when { nullToEmptyCollection && m.type.isCollectionLikeType -> false @@ -54,7 +56,6 @@ internal class KotlinAnnotationIntrospector( null } } - return hasRequired } override fun findSerializationConverter(a: Annotated): Converter<*, *>? = when (a) { From 4e500ac1fd746ea4f570ddec89a0f35dde10adbc Mon Sep 17 00:00:00 2001 From: wrongwrong Date: Thu, 20 Mar 2025 12:00:57 +0900 Subject: [PATCH 3/4] Fix to not use JUnit5 assertions --- .../fasterxml/jackson/module/kotlin/test/github/GitHub922.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/github/GitHub922.kt b/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/github/GitHub922.kt index 6c67e3c28..9150504bf 100644 --- a/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/github/GitHub922.kt +++ b/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/github/GitHub922.kt @@ -4,8 +4,8 @@ import com.fasterxml.jackson.databind.BeanDescription import com.fasterxml.jackson.databind.ObjectMapper import com.fasterxml.jackson.module.kotlin.KotlinFeature import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper -import org.junit.jupiter.api.Assertions.assertTrue import kotlin.test.Test +import kotlin.test.assertTrue class GitHub922 { private inline fun ObjectMapper.introspectSerialization(): BeanDescription = From dba7a143250b77afbfceec7efda29d9329fb454b Mon Sep 17 00:00:00 2001 From: wrongwrong Date: Thu, 20 Mar 2025 12:08:08 +0900 Subject: [PATCH 4/4] Update release notes wrt #923 --- release-notes/CREDITS-2.x | 1 + release-notes/VERSION-2.x | 2 ++ 2 files changed, 3 insertions(+) diff --git a/release-notes/CREDITS-2.x b/release-notes/CREDITS-2.x index c42d41d1b..920e4df73 100644 --- a/release-notes/CREDITS-2.x +++ b/release-notes/CREDITS-2.x @@ -18,6 +18,7 @@ Contributors: # 2.18.4 (not yet released) WrongWrong (@k163377) +* #923: Fixed hasRequiredMarker to only process content defined in Kotlin * #920: Minor refactors that do not affect behavior # 2.18.3 (28-Feb-2025) diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x index fb6023430..8200dc9aa 100644 --- a/release-notes/VERSION-2.x +++ b/release-notes/VERSION-2.x @@ -18,6 +18,8 @@ Co-maintainers: 2.18.4 (not yet released) +#923: Fixed a problem where the result of processing `hasRequiredMarker ` by a `KotlinModule` would also apply to + classes defined in `Java` when `NullToEmptyCollection` or `NullToEmptyMap` was enabled. #920: Minor refactorings were made that did not affect behavior. 2.18.3 (28-Feb-2025)