Skip to content

Commit df0c9b4

Browse files
k163377k163377
authored
Merge pull request #318 from ProjectMapK/required-3
Fixed `hasRequiredMarker` for fields and setters to ignore `nullToEmpty` option.
2 parents 84cb9e3 + 7747184 commit df0c9b4

File tree

4 files changed

+110
-30
lines changed

4 files changed

+110
-30
lines changed

‎src/main/kotlin/io/github/projectmapk/jackson/module/kogera/annotationIntrospector/KotlinPrimaryAnnotationIntrospector.kt‎

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,6 @@ internal class KotlinPrimaryAnnotationIntrospector(
5959
private fun JavaType.hasDefaultEmptyValue() = (nullToEmptyCollection && isCollectionLikeType) ||
6060
(nullToEmptyMap && isMapLikeType)
6161

62-
// The nullToEmpty option also affects serialization,
63-
// but deserialization is preferred because there is currently no way to distinguish between contexts.
6462
private fun AnnotatedField.hasRequiredMarker(jmClass: JmClass): Boolean? {
6563
// Direct access to `AnnotatedField` is only performed if there is no accessor (defined as JvmField),
6664
// so if an accessor is defined, it is ignored.
@@ -69,7 +67,7 @@ internal class KotlinPrimaryAnnotationIntrospector(
6967
// only a check for the existence of a getter is performed.
7068
// https://youtrack.jetbrains.com/issue/KT-6519
7169
?.let {
72-
if (it.getterName == null) !(it.returnType.isNullable || type.hasDefaultEmptyValue()) else null
70+
if (it.getterName == null) !it.returnType.isNullable else null
7371
}
7472
}
7573

@@ -80,12 +78,8 @@ internal class KotlinPrimaryAnnotationIntrospector(
8078
): Boolean? = when (parameterCount) {
8179
0 -> jmClass.findPropertyByGetter(member)?.isRequiredByNullability()
8280
1 -> {
83-
if (this.getParameter(0).type.hasDefaultEmptyValue()) {
84-
false
85-
} else {
86-
val memberSignature = member.toSignature()
87-
jmClass.properties.find { it.setterSignature == memberSignature }?.isRequiredByNullability()
88-
}
81+
val memberSignature = member.toSignature()
82+
jmClass.properties.find { it.setterSignature == memberSignature }?.isRequiredByNullability()
8983
}
9084
else -> null
9185
}

‎src/test/kotlin/io/github/projectmapk/jackson/module/kogera/zIntegration/deser/HasRequiredMarkerTest.kt‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -108,10 +108,10 @@ class HasRequiredMarkerTest {
108108

109109
assertFalse(desc.isRequired("nullableProp"))
110110
assertFalse(desc.isRequired("nullableField"))
111-
assertFalse(desc.isRequired("collectionProp"))
112-
assertFalse(desc.isRequired("collectionField"))
113-
assertFalse(desc.isRequired("mapProp"))
114-
assertFalse(desc.isRequired("mapField"))
111+
assertTrue(desc.isRequired("collectionProp"))
112+
assertTrue(desc.isRequired("collectionField"))
113+
assertTrue(desc.isRequired("mapProp"))
114+
assertTrue(desc.isRequired("mapField"))
115115
assertTrue(desc.isRequired("nonNullProp"))
116116
assertTrue(desc.isRequired("nonNullField"))
117117
}

‎src/test/kotlin/io/github/projectmapk/jackson/module/kogera/zIntegration/ser/HasRequiredMarkerTest.kt‎

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -67,9 +67,8 @@ class HasRequiredMarkerTest {
6767
val map: Map<*, *> = emptyMap<Any, Any>()
6868
}
6969

70-
// @see KotlinPrimaryAnnotationIntrospector::AnnotatedField.hasRequiredMarker
7170
@Test
72-
fun failing() {
71+
fun `nullToEmpty does not affect for field`() {
7372
val nullToDefaultMapper = ObjectMapper().registerModule(
7473
KotlinModule.Builder()
7574
.enable(KotlinFeature.NullToEmptyCollection)
@@ -78,13 +77,7 @@ class HasRequiredMarkerTest {
7877
)
7978
val desc = nullToDefaultMapper.introspectSer<NullToDefaultTarget>()
8079

81-
assertFalse(
82-
desc.isRequired("collection"),
83-
"KotlinPrimaryAnnotationIntrospector::AnnotatedField.hasRequiredMarker fixed"
84-
)
85-
assertFalse(
86-
desc.isRequired("map"),
87-
"KotlinPrimaryAnnotationIntrospector::AnnotatedField.hasRequiredMarker fixed"
88-
)
80+
assertTrue(desc.isRequired("collection"))
81+
assertTrue(desc.isRequired("map"))
8982
}
9083
}

‎src/test/kotlin/io/github/projectmapk/jackson/module/kogera/zPorted/test/github/GitHub922.kt‎

Lines changed: 100 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,13 @@ import org.junit.jupiter.api.Test
1313
import kotlin.reflect.full.memberProperties
1414

1515
class GitHub922 {
16+
companion object {
17+
val nullToEmptyMapper = jacksonObjectMapper {
18+
enable(KotlinFeature.NullToEmptyCollection)
19+
enable(KotlinFeature.NullToEmptyMap)
20+
}
21+
}
22+
1623
private inline fun <reified T : Any> ObjectMapper.introspectSerialization(): BeanDescription =
1724
serializationConfig.introspect(serializationConfig.constructType(T::class.java))
1825

@@ -24,15 +31,101 @@ class GitHub922 {
2431

2532
@Test
2633
fun `nullToEmpty does not override specification by Java annotation`() {
27-
val mapper = jacksonObjectMapper {
28-
enable(KotlinFeature.NullToEmptyCollection)
29-
enable(KotlinFeature.NullToEmptyMap)
30-
}
34+
val defaultDesc = defaultMapper.introspectDeserialization<GitHub922RequiredCollectionsDtoJava>()
35+
36+
assertTrue(defaultDesc.isRequired("list"))
37+
assertTrue(defaultDesc.isRequired("map"))
38+
39+
val nullToEmptyDesc = nullToEmptyMapper.introspectDeserialization<GitHub922RequiredCollectionsDtoJava>()
40+
41+
assertTrue(nullToEmptyDesc.isRequired("list"))
42+
assertTrue(nullToEmptyDesc.isRequired("map"))
43+
}
44+
45+
data class RequiredCollectionsDto1(
46+
@JsonProperty(required = true) val list: List<String>,
47+
@JsonProperty(required = true) val map: Map<String, String>
48+
)
49+
50+
data class RequiredCollectionsDto2(
51+
@JsonProperty(isRequired = OptBoolean.TRUE) val list: List<String>,
52+
@JsonProperty(isRequired = OptBoolean.TRUE) val map: Map<String, String>
53+
)
54+
55+
@Test
56+
fun `nullToEmpty does not override specification by annotation`() {
57+
val defaultDesc1 = defaultMapper.introspectDeserialization<RequiredCollectionsDto1>()
58+
59+
assertTrue(defaultDesc1.isRequired("list"))
60+
assertTrue(defaultDesc1.isRequired("map"))
61+
62+
val nullToEmptyDesc1 = nullToEmptyMapper.introspectDeserialization<RequiredCollectionsDto1>()
63+
64+
assertTrue(nullToEmptyDesc1.isRequired("list"))
65+
assertTrue(nullToEmptyDesc1.isRequired("map"))
66+
67+
val defaultDesc2 = defaultMapper.introspectDeserialization<RequiredCollectionsDto2>()
68+
69+
assertTrue(defaultDesc2.isRequired("list"))
70+
assertTrue(defaultDesc2.isRequired("map"))
71+
72+
val nullToEmptyDesc2 = nullToEmptyMapper.introspectDeserialization<RequiredCollectionsDto2>()
73+
74+
assertTrue(nullToEmptyDesc2.isRequired("list"))
75+
assertTrue(nullToEmptyDesc2.isRequired("map"))
76+
}
77+
78+
data class CollectionsDto(val list: List<String>, val map: Map<String, String>)
79+
80+
@Test
81+
fun `nullToEmpty does not affect for serialization`() {
82+
val defaultDesc = defaultMapper.introspectSerialization<CollectionsDto>()
83+
84+
assertTrue(defaultDesc.isRequired("list"))
85+
assertTrue(defaultDesc.isRequired("map"))
86+
87+
val nullToEmptyDesc = nullToEmptyMapper.introspectSerialization<CollectionsDto>()
88+
89+
assertTrue(nullToEmptyDesc.isRequired("list"))
90+
assertTrue(nullToEmptyDesc.isRequired("map"))
91+
}
92+
93+
class SetterCollectionsDto {
94+
lateinit var list: List<String>
95+
lateinit var map: Map<String, String>
96+
}
97+
98+
@Test
99+
fun `nullToEmpty does not affect for setter`() {
100+
val defaultDesc = defaultMapper.introspectDeserialization<SetterCollectionsDto>()
101+
102+
assertTrue(defaultDesc.isRequired("list"))
103+
assertTrue(defaultDesc.isRequired("map"))
104+
105+
val nullToEmptyDesc = nullToEmptyMapper.introspectDeserialization<SetterCollectionsDto>()
106+
107+
assertTrue(nullToEmptyDesc.isRequired("list"))
108+
assertTrue(nullToEmptyDesc.isRequired("map"))
109+
}
110+
111+
class FieldCollectionsDto {
112+
@JvmField
113+
var list: List<String> = emptyList()
114+
@JvmField
115+
var map: Map<String, String> = emptyMap()
116+
}
117+
118+
@Test
119+
fun `nullToEmpty does not affect for field`() {
120+
val defaultDesc = defaultMapper.introspectDeserialization<FieldCollectionsDto>()
121+
122+
assertTrue(defaultDesc.isRequired("list"))
123+
assertTrue(defaultDesc.isRequired("map"))
31124

32-
val desc = mapper.introspectDeserialization<GitHub922RequiredCollectionsDtoJava>()
125+
val nullToEmptyDesc = nullToEmptyMapper.introspectDeserialization<FieldCollectionsDto>()
33126

34-
assertTrue(desc.isRequired("list"))
35-
assertTrue(desc.isRequired("map"))
127+
assertTrue(nullToEmptyDesc.isRequired("list"))
128+
assertTrue(nullToEmptyDesc.isRequired("map"))
36129
}
37130

38131
// isRequired_required_nullability_expected

0 commit comments

Comments
 (0)