Skip to content

Commit 579d92c

Browse files
k163377k163377
authored
Merge pull request #18 from ProjectMapK/refactors
Refactors
2 parents 88ac39b + d509099 commit 579d92c

File tree

4 files changed

+60
-69
lines changed

4 files changed

+60
-69
lines changed

‎src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinAnnotationIntrospector.kt

Lines changed: 10 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,8 @@
11
package com.fasterxml.jackson.module.kotlin
22

3-
import com.fasterxml.jackson.annotation.JsonCreator
43
import com.fasterxml.jackson.annotation.JsonProperty
54
import com.fasterxml.jackson.databind.DeserializationFeature
65
import com.fasterxml.jackson.databind.Module
7-
import com.fasterxml.jackson.databind.cfg.MapperConfig
86
import com.fasterxml.jackson.databind.introspect.Annotated
97
import com.fasterxml.jackson.databind.introspect.AnnotatedField
108
import com.fasterxml.jackson.databind.introspect.AnnotatedMember
@@ -23,7 +21,6 @@ import kotlinx.metadata.jvm.getterSignature
2321
import kotlinx.metadata.jvm.setterSignature
2422
import java.lang.reflect.AccessibleObject
2523
import java.lang.reflect.Constructor
26-
import java.lang.reflect.Field
2724
import java.lang.reflect.Method
2825
import kotlin.reflect.KFunction
2926
import kotlin.reflect.jvm.javaType
@@ -37,7 +34,8 @@ internal class KotlinAnnotationIntrospector(
3734
private val nullIsSameAsDefault: Boolean
3835
) : NopAnnotationIntrospector() {
3936

40-
// TODO: implement nullIsSameAsDefault flag, which represents when TRUE that if something has a default value, it can be passed a null to default it
37+
// TODO: implement nullIsSameAsDefault flag, which represents when TRUE that if something has a default value,
38+
// it can be passed a null to default it
4139
// this likely impacts this class to be accurate about what COULD be considered required
4240

4341
override fun hasRequiredMarker(m: AnnotatedMember): Boolean? {
@@ -48,7 +46,7 @@ internal class KotlinAnnotationIntrospector(
4846
nullToEmptyMap && m.type.isMapLikeType -> false
4947
m.member.declaringClass.isKotlinClass() -> when (m) {
5048
is AnnotatedField -> m.hasRequiredMarker()
51-
is AnnotatedMethod -> m.hasRequiredMarker()
49+
is AnnotatedMethod -> m.getRequiredMarkerFromCorrespondingAccessor()
5250
is AnnotatedParameter -> m.hasRequiredMarker()
5351
else -> null
5452
}
@@ -61,14 +59,6 @@ internal class KotlinAnnotationIntrospector(
6159
return hasRequired
6260
}
6361

64-
override fun findCreatorAnnotation(config: MapperConfig<*>, a: Annotated): JsonCreator.Mode? {
65-
// TODO: possible work around for JsonValue class that requires the class constructor to have the JsonCreator(Mode.DELEGATED) set?
66-
// since we infer the creator at times for these methods, the wrong mode could be implied.
67-
68-
// findCreatorBinding used to be a clearer way to set this, but we need to set the mode here to disambugiate the intent of the constructor
69-
return super.findCreatorAnnotation(config, a)
70-
}
71-
7262
// Find a serializer to handle the case where the getter returns an unboxed value from the value class.
7363
override fun findSerializer(am: Annotated): StdSerializer<*>? = when (am) {
7464
is AnnotatedMethod -> {
@@ -122,7 +112,7 @@ internal class KotlinAnnotationIntrospector(
122112
}
123113

124114
private fun AnnotatedField.hasRequiredMarker(): Boolean? {
125-
val member = member as Field
115+
val member = annotated
126116

127117
val byAnnotation = member.isRequiredByAnnotation()
128118
val fieldSignature = member.toSignature()
@@ -135,29 +125,18 @@ internal class KotlinAnnotationIntrospector(
135125
}
136126

137127
private fun AccessibleObject.isRequiredByAnnotation(): Boolean? = annotations
138-
?.firstOrNull { it.annotationClass == JsonProperty::class }
139-
?.let { it as JsonProperty }
128+
.filterIsInstance<JsonProperty>()
129+
.firstOrNull()
140130
?.required
141131

142-
private fun requiredAnnotationOrNullability(byAnnotation: Boolean?, byNullability: Boolean?): Boolean? {
143-
if (byAnnotation != null && byNullability != null) {
144-
return byAnnotation || byNullability
145-
} else if (byNullability != null) {
146-
return byNullability
147-
}
148-
return byAnnotation
149-
}
150-
151-
private fun Method.isRequiredByAnnotation(): Boolean? {
152-
return (this.annotations.firstOrNull { it.annotationClass.java == JsonProperty::class.java } as? JsonProperty)?.required
132+
private fun requiredAnnotationOrNullability(byAnnotation: Boolean?, byNullability: Boolean?): Boolean? = when {
133+
byAnnotation != null && byNullability != null -> byAnnotation || byNullability
134+
byNullability != null -> byNullability
135+
else -> byAnnotation
153136
}
154137

155138
private fun KmProperty.isRequiredByNullability(): Boolean = !Flag.Type.IS_NULLABLE(this.returnType.flags)
156139

157-
// This could be a setter or a getter of a class property or
158-
// a setter-like/getter-like method.
159-
private fun AnnotatedMethod.hasRequiredMarker(): Boolean? = this.getRequiredMarkerFromCorrespondingAccessor()
160-
161140
private fun AnnotatedMethod.getRequiredMarkerFromCorrespondingAccessor(): Boolean? {
162141
val memberSignature = member.toSignature()
163142
member.declaringClass.toKmClass()?.properties?.forEach { kmProperty ->

‎src/main/kotlin/com/fasterxml/jackson/module/kotlin/deser/value_instantiator/KotlinValueInstantiator.kt

Lines changed: 47 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package com.fasterxml.jackson.module.kotlin.deser.value_instantiator
33
import com.fasterxml.jackson.databind.BeanDescription
44
import com.fasterxml.jackson.databind.DeserializationConfig
55
import com.fasterxml.jackson.databind.DeserializationContext
6+
import com.fasterxml.jackson.databind.JavaType
67
import com.fasterxml.jackson.databind.deser.SettableBeanProperty
78
import com.fasterxml.jackson.databind.deser.ValueInstantiator
89
import com.fasterxml.jackson.databind.deser.ValueInstantiators
@@ -28,6 +29,34 @@ internal class KotlinValueInstantiator(
2829
// @see com.fasterxml.jackson.module.kotlin._ported.test.StrictNullChecksTest#testListOfGenericWithNullValue
2930
private fun ValueParameter.isNullishTypeAt(index: Int) = arguments.getOrNull(index)?.isNullable ?: true
3031

32+
private fun JavaType.requireEmptyValue() =
33+
(nullToEmptyCollection && this.isCollectionLikeType) || (nullToEmptyMap && this.isMapLikeType)
34+
35+
private fun strictNullCheck(
36+
ctxt: DeserializationContext,
37+
jsonProp: SettableBeanProperty,
38+
paramDef: ValueParameter,
39+
paramVal: Any
40+
) {
41+
// If an error occurs, Argument.name is always non-null
42+
// @see com.fasterxml.jackson.module.kotlin.deser.value_instantiator.creator.Argument
43+
when {
44+
paramVal is Collection<*> && !paramDef.isNullishTypeAt(0) && paramVal.any { it == null } ->
45+
"collection" to paramDef.arguments[0].name!!
46+
paramVal is Array<*> && !paramDef.isNullishTypeAt(0) && paramVal.any { it == null } ->
47+
"array" to paramDef.arguments[0].name!!
48+
paramVal is Map<*, *> && !paramDef.isNullishTypeAt(1) && paramVal.values.any { it == null } ->
49+
"map" to paramDef.arguments[1].name!!
50+
else -> null
51+
}?.let { (paramType, itemType) ->
52+
throw MissingKotlinParameterException(
53+
parameter = paramDef,
54+
processor = ctxt.parser,
55+
msg = "Instantiation of $itemType $paramType failed for JSON property ${jsonProp.name} due to null value in a $paramType that does not allow null values"
56+
).wrapWithPath(this.valueClass, jsonProp.name)
57+
}
58+
}
59+
3160
override fun createFromObjectWith(
3261
ctxt: DeserializationContext,
3362
props: Array<out SettableBeanProperty>,
@@ -47,11 +76,9 @@ internal class KotlinValueInstantiator(
4776
}
4877

4978
var paramVal = if (!isMissing || paramDef.isPrimitive || jsonProp.hasInjectableValueId()) {
50-
val tempParamVal = buffer.getParameter(jsonProp)
51-
if (nullIsSameAsDefault && tempParamVal == null && paramDef.isOptional) {
52-
return@forEachIndexed
79+
buffer.getParameter(jsonProp).apply {
80+
if (nullIsSameAsDefault && this == null && paramDef.isOptional) return@forEachIndexed
5381
}
54-
tempParamVal
5582
} else {
5683
if (paramDef.isNullable) {
5784
// do not try to create any object if it is nullable and the value is missing
@@ -62,37 +89,23 @@ internal class KotlinValueInstantiator(
6289
}
6390
}
6491

65-
if (paramVal == null && ((nullToEmptyCollection && jsonProp.type.isCollectionLikeType) || (nullToEmptyMap && jsonProp.type.isMapLikeType))) {
66-
paramVal = NullsAsEmptyProvider(jsonProp.valueDeserializer).getNullValue(ctxt)
67-
}
68-
69-
val isMissingAndRequired = paramVal == null && isMissing && jsonProp.isRequired
70-
if (isMissingAndRequired || (!paramDef.isGenericType && paramVal == null && !paramDef.isNullable)) {
71-
throw MissingKotlinParameterException(
72-
parameter = paramDef,
73-
processor = ctxt.parser,
74-
msg = "Instantiation of ${this.valueTypeDesc} value failed for JSON property ${jsonProp.name} due to missing (therefore NULL) value for creator parameter ${paramDef.name} which is a non-nullable type"
75-
).wrapWithPath(this.valueClass, jsonProp.name)
76-
}
77-
78-
if (strictNullChecks && paramVal != null) {
79-
// If an error occurs, Argument.name is always non-null
80-
// @see com.fasterxml.jackson.module.kotlin.deser.value_instantiator.creator.Argument
81-
when {
82-
paramVal is Collection<*> && !paramDef.isNullishTypeAt(0) && paramVal.any { it == null } ->
83-
"collection" to paramDef.arguments[0].name!!
84-
paramVal is Array<*> && !paramDef.isNullishTypeAt(0) && paramVal.any { it == null } ->
85-
"array" to paramDef.arguments[0].name!!
86-
paramVal is Map<*, *> && !paramDef.isNullishTypeAt(1) && paramVal.values.any { it == null } ->
87-
"map" to paramDef.arguments[1].name!!
88-
else -> null
89-
}?.let { (paramType, itemType) ->
90-
throw MissingKotlinParameterException(
91-
parameter = paramDef,
92-
processor = ctxt.parser,
93-
msg = "Instantiation of $itemType $paramType failed for JSON property ${jsonProp.name} due to null value in a $paramType that does not allow null values"
94-
).wrapWithPath(this.valueClass, jsonProp.name)
92+
if (paramVal == null) {
93+
if (jsonProp.type.requireEmptyValue()) {
94+
paramVal = NullsAsEmptyProvider(jsonProp.valueDeserializer).getNullValue(ctxt)
95+
} else {
96+
val isMissingAndRequired = isMissing && jsonProp.isRequired
97+
if (isMissingAndRequired || (!paramDef.isGenericType && !paramDef.isNullable)) {
98+
throw MissingKotlinParameterException(
99+
parameter = paramDef,
100+
processor = ctxt.parser,
101+
msg = "Instantiation of $valueTypeDesc value failed for JSON property ${jsonProp.name} " +
102+
"due to missing (therefore NULL) value for creator parameter ${paramDef.name} " +
103+
"which is a non-nullable type"
104+
).wrapWithPath(this.valueClass, jsonProp.name)
105+
}
95106
}
107+
} else {
108+
if (strictNullChecks) strictNullCheck(ctxt, jsonProp, paramDef, paramVal)
96109
}
97110

98111
bucket[idx] = paramVal

‎src/test/kotlin/com/fasterxml/jackson/module/kotlin/_ported/test/PropertyRequirednessTests.kt

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,9 @@ class TestPropertyRequiredness {
8585
val f: TestParamClass?,
8686
val g: TestParamClass = TestParamClass(),
8787
val h: TestParamClass? = TestParamClass(),
88-
@JsonProperty("x", required = true) val x: Int?, // TODO: either error in test case with this not being on the property getter, or error in introspection not seeing this on the constructor parameter
88+
// TODO: either error in test case with this not being on the property getter,
89+
// or error in introspection not seeing this on the constructor parameter
90+
@JsonProperty("x", required = true) val x: Int?,
8991
@get:JsonProperty("z", required = true) val z: Int
9092
)
9193

‎src/test/kotlin/com/fasterxml/jackson/module/kotlin/_ported/test/github/failing/GitHub337.kt

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import com.fasterxml.jackson.databind.MapperFeature.SORT_PROPERTIES_ALPHABETICAL
66
import com.fasterxml.jackson.module.kotlin.jacksonMapperBuilder
77
import com.fasterxml.jackson.module.kotlin.testPrettyWriter
88
import org.junit.jupiter.api.Assertions.assertEquals
9-
import org.junit.jupiter.api.Disabled
109
import org.junit.jupiter.api.Test
1110

1211
/**
@@ -20,7 +19,6 @@ class TestGitHub337 {
2019
private val writer = mapper.testPrettyWriter()
2120

2221
@Test
23-
@Disabled
2422
fun test_ClassWithIsFields() {
2523
data class ClassWithIsFields(
2624
val isBooleanField: Boolean,
@@ -38,7 +36,6 @@ class TestGitHub337 {
3836
}
3937

4038
@Test
41-
@Disabled
4239
fun test_AnnotatedClassWithIsFields() {
4340
data class ClassWithIsFields(
4441
@JsonProperty("isBooleanField") val isBooleanField: Boolean,

0 commit comments

Comments
 (0)