Skip to content

Commit 39d30f2

Browse files
committed
Merge remote-tracking branch 'FasterXML/2.19'
2 parents 432505d + 6b4b19f commit 39d30f2

File tree

10 files changed

+405
-23
lines changed

10 files changed

+405
-23
lines changed

release-notes/CREDITS-2.x

+1
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ Contributors:
1818
# 2.19.0 (not yet released)
1919

2020
WrongWrong (@k163377)
21+
* #885: Performance improvement of strictNullChecks
2122
* #884: Changed the base class of MissingKotlinParameterException to InvalidNullException
2223
* #878: Fix for #876
2324
* #868: Added test case for FAIL_ON_NULL_FOR_PRIMITIVES

release-notes/VERSION-2.x

+4
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@ Co-maintainers:
1717
------------------------------------------------------------------------
1818

1919
2.19.0 (not yet released)
20+
#885: A new `StrictNullChecks` option(KotlinFeature.NewStrictNullChecks) has been added which greatly improves throughput.
21+
Benchmarks show a consistent throughput drop of less than 2% when enabled (prior to the improvement, the worst throughput drop was more than 30%).
22+
Note that the new backend changes the exception thrown to `InvalidNullException` and with it the error message.
23+
Also note that the base class for `MissingKotlinParameterException` was changed to `InvalidNullException` in #884.
2024
#884: The base class for `MissingKotlinParameterException` has been changed to `InvalidNullException`.
2125
If you do not catch this exception or catch `MismatchedInputException`, the behavior is unchanged.
2226
If you are catching both `MismatchedKotlinParameterException` and `InvalidNullException`, you must catch `MismatchedKotlinParameterException` first.

src/main/kotlin/tools/jackson/module/kotlin/KotlinFeature.kt

+24-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package tools.jackson.module.kotlin
22

3+
import com.fasterxml.jackson.annotation.JsonSetter
4+
import tools.jackson.databind.exc.InvalidNullException
35
import java.util.BitSet
46

57
/**
@@ -40,6 +42,11 @@ enum class KotlinFeature(internal val enabledByDefault: Boolean) {
4042
* may contain null values after deserialization.
4143
* Enabling it protects against this but has significant performance impact.
4244
*/
45+
@Deprecated(
46+
level = DeprecationLevel.WARNING,
47+
message = "This option will be migrated to the new backend in 2.21.",
48+
replaceWith = ReplaceWith("NewStrictNullChecks")
49+
)
4350
StrictNullChecks(enabledByDefault = false),
4451

4552
/**
@@ -66,7 +73,23 @@ enum class KotlinFeature(internal val enabledByDefault: Boolean) {
6673
* `@JsonFormat` annotations need to be declared either on getter using `@get:JsonFormat` or field using `@field:JsonFormat`.
6774
* See [jackson-module-kotlin#651] for details.
6875
*/
69-
UseJavaDurationConversion(enabledByDefault = false);
76+
UseJavaDurationConversion(enabledByDefault = false),
77+
78+
/**
79+
* New [StrictNullChecks] feature with improved throughput.
80+
* Internally, it will be the same as if [JsonSetter] (contentNulls = FAIL) had been granted.
81+
* Benchmarks show that it can check for illegal nulls with throughput nearly identical to the default (see [jackson-module-kotlin#719]).
82+
*
83+
* Note that in the new backend, the exception thrown has changed from [MissingKotlinParameterException] to [InvalidNullException].
84+
* The message will be changed accordingly.
85+
* Since 2.19, the base class of [MissingKotlinParameterException] has also been changed to [InvalidNullException],
86+
* so be careful when catching it.
87+
*
88+
* This is a temporary option for a phased backend migration,
89+
* which will eventually be merged into [StrictNullChecks].
90+
* Also, specifying both this and [StrictNullChecks] is not permitted.
91+
*/
92+
NewStrictNullChecks(enabledByDefault = false);
7093

7194
internal val bitSet: BitSet = (1 shl ordinal).toBitSet()
7295

src/main/kotlin/tools/jackson/module/kotlin/KotlinModule.kt

+20-3
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,25 @@ class KotlinModule private constructor(
3636
val nullToEmptyMap: Boolean = NullToEmptyMap.enabledByDefault,
3737
val nullIsSameAsDefault: Boolean = NullIsSameAsDefault.enabledByDefault,
3838
val singletonSupport: Boolean = SingletonSupport.enabledByDefault,
39-
val strictNullChecks: Boolean = StrictNullChecks.enabledByDefault,
39+
strictNullChecks: Boolean = StrictNullChecks.enabledByDefault,
4040
val kotlinPropertyNameAsImplicitName: Boolean = KotlinPropertyNameAsImplicitName.enabledByDefault,
4141
val useJavaDurationConversion: Boolean = UseJavaDurationConversion.enabledByDefault,
42+
private val newStrictNullChecks: Boolean = NewStrictNullChecks.enabledByDefault,
4243
) : SimpleModule(KotlinModule::class.java.name, PackageVersion.VERSION) {
4344

45+
private val oldStrictNullChecks: Boolean = strictNullChecks
46+
47+
// To reduce the amount of destructive changes, no properties will be added to the public.
48+
val strictNullChecks: Boolean = if (strictNullChecks) {
49+
if (newStrictNullChecks) {
50+
throw IllegalArgumentException("Enabling both StrictNullChecks and NewStrictNullChecks is not permitted.")
51+
}
52+
53+
true
54+
} else {
55+
newStrictNullChecks
56+
}
57+
4458
companion object {
4559
// Increment when option is added
4660
private const val serialVersionUID = 3L
@@ -61,6 +75,7 @@ class KotlinModule private constructor(
6175
builder.isEnabled(StrictNullChecks),
6276
builder.isEnabled(KotlinPropertyNameAsImplicitName),
6377
builder.isEnabled(UseJavaDurationConversion),
78+
builder.isEnabled(NewStrictNullChecks),
6479
)
6580

6681
override fun setupModule(context: SetupContext) {
@@ -72,7 +87,7 @@ class KotlinModule private constructor(
7287

7388
val cache = ReflectionCache(reflectionCacheSize)
7489

75-
context.addValueInstantiators(KotlinInstantiators(cache, nullToEmptyCollection, nullToEmptyMap, nullIsSameAsDefault, strictNullChecks))
90+
context.addValueInstantiators(KotlinInstantiators(cache, nullToEmptyCollection, nullToEmptyMap, nullIsSameAsDefault, oldStrictNullChecks))
7691

7792
if (singletonSupport) {
7893
// [module-kotlin#225]: keep Kotlin singletons as singletons
@@ -87,7 +102,9 @@ class KotlinModule private constructor(
87102
nullIsSameAsDefault,
88103
useJavaDurationConversion
89104
))
90-
context.appendAnnotationIntrospector(KotlinNamesAnnotationIntrospector(cache, kotlinPropertyNameAsImplicitName))
105+
context.appendAnnotationIntrospector(
106+
KotlinNamesAnnotationIntrospector(cache, newStrictNullChecks, kotlinPropertyNameAsImplicitName)
107+
)
91108

92109
context.addDeserializers(KotlinDeserializers(cache, useJavaDurationConversion))
93110
context.addKeyDeserializers(KotlinKeyDeserializers)

src/main/kotlin/tools/jackson/module/kotlin/KotlinNamesAnnotationIntrospector.kt

+33-8
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package tools.jackson.module.kotlin
22

33
import com.fasterxml.jackson.annotation.JsonProperty
4+
import com.fasterxml.jackson.annotation.JsonSetter
5+
import com.fasterxml.jackson.annotation.Nulls
46
import tools.jackson.databind.JavaType
57
import tools.jackson.databind.cfg.MapperConfig
68
import tools.jackson.databind.introspect.Annotated
@@ -12,8 +14,10 @@ import tools.jackson.databind.introspect.NopAnnotationIntrospector
1214
import tools.jackson.databind.introspect.PotentialCreator
1315
import java.lang.reflect.Constructor
1416
import java.util.Locale
17+
import kotlin.collections.getOrNull
1518
import kotlin.reflect.KClass
1619
import kotlin.reflect.KFunction
20+
import kotlin.reflect.KParameter
1721
import kotlin.reflect.full.hasAnnotation
1822
import kotlin.reflect.full.memberProperties
1923
import kotlin.reflect.full.primaryConstructor
@@ -22,6 +26,7 @@ import kotlin.reflect.jvm.javaType
2226

2327
internal class KotlinNamesAnnotationIntrospector(
2428
private val cache: ReflectionCache,
29+
private val strictNullChecks: Boolean,
2530
private val kotlinPropertyNameAsImplicitName: Boolean
2631
) : NopAnnotationIntrospector() {
2732
private fun getterNameFromJava(member: AnnotatedMethod): String? {
@@ -73,16 +78,26 @@ internal class KotlinNamesAnnotationIntrospector(
7378
}
7479

7580
override fun refineDeserializationType(config: MapperConfig<*>, a: Annotated, baseType: JavaType): JavaType =
76-
(a as? AnnotatedParameter)?.let { _ ->
77-
cache.findKotlinParameter(a)?.let { param ->
78-
val rawType = a.rawType
79-
(param.type.classifier as? KClass<*>)
80-
?.java
81-
?.takeIf { it.isUnboxableValueClass() && it != rawType }
82-
?.let { config.constructType(it) }
83-
}
81+
findKotlinParameter(a)?.let { param ->
82+
val rawType = a.rawType
83+
(param.type.classifier as? KClass<*>)
84+
?.java
85+
?.takeIf { it.isUnboxableValueClass() && it != rawType }
86+
?.let { config.constructType(it) }
8487
} ?: baseType
8588

89+
override fun findSetterInfo(config: MapperConfig<*>, ann: Annotated): JsonSetter.Value = ann.takeIf { strictNullChecks }
90+
?.let { _ ->
91+
findKotlinParameter(ann)?.let { param ->
92+
if (param.requireStrictNullCheck(ann.type)) {
93+
JsonSetter.Value.forContentNulls(Nulls.FAIL)
94+
} else {
95+
null
96+
}
97+
}
98+
}
99+
?: super.findSetterInfo(config, ann)
100+
86101
override fun findPreferredCreator(
87102
config: MapperConfig<*>,
88103
valueClass: AnnotatedClass,
@@ -106,8 +121,18 @@ internal class KotlinNamesAnnotationIntrospector(
106121
}
107122

108123
private fun findKotlinParameterName(param: AnnotatedParameter): String? = cache.findKotlinParameter(param)?.name
124+
125+
private fun findKotlinParameter(param: Annotated) = (param as? AnnotatedParameter)
126+
?.let { cache.findKotlinParameter(it) }
109127
}
110128

129+
private fun KParameter.markedNonNullAt(index: Int) = type.arguments.getOrNull(index)?.type?.isMarkedNullable == false
130+
131+
private fun KParameter.requireStrictNullCheck(type: JavaType): Boolean =
132+
((type.isArrayType || type.isCollectionLikeType) && this.markedNonNullAt(0)) ||
133+
(type.isMapLikeType && this.markedNonNullAt(1))
134+
135+
111136
// If it is not a Kotlin class or an Enum, Creator is not used
112137
private fun AnnotatedClass.creatableKotlinClass(): KClass<*>? = annotated
113138
.takeIf { it.isKotlinClass() && !it.isEnum }

src/test/kotlin/tools/jackson/module/kotlin/DslTest.kt

+2-2
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import tools.jackson.module.kotlin.KotlinFeature.NullIsSameAsDefault
66
import tools.jackson.module.kotlin.KotlinFeature.NullToEmptyCollection
77
import tools.jackson.module.kotlin.KotlinFeature.NullToEmptyMap
88
import tools.jackson.module.kotlin.KotlinFeature.SingletonSupport
9-
import tools.jackson.module.kotlin.KotlinFeature.StrictNullChecks
9+
import tools.jackson.module.kotlin.KotlinFeature.NewStrictNullChecks
1010
import org.junit.jupiter.api.Assertions.assertNotNull
1111
import org.junit.jupiter.api.Test
1212
import kotlin.test.assertEquals
@@ -35,7 +35,7 @@ class DslTest {
3535
enable(NullToEmptyMap)
3636
enable(NullIsSameAsDefault)
3737
enable(SingletonSupport)
38-
enable(StrictNullChecks)
38+
enable(NewStrictNullChecks)
3939
}
4040

4141
assertNotNull(module)

src/test/kotlin/tools/jackson/module/kotlin/KotlinModuleTest.kt

+15
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,24 @@ import org.junit.jupiter.api.Assertions.assertEquals
66
import org.junit.jupiter.api.Assertions.assertFalse
77
import org.junit.jupiter.api.Assertions.assertTrue
88
import org.junit.jupiter.api.Test
9+
import org.junit.jupiter.api.assertThrows
910
import kotlin.test.assertNotNull
1011

1112
class KotlinModuleTest {
13+
// After the final migration is complete, this test will be removed.
14+
@Test
15+
fun strictNullChecksTests() {
16+
assertTrue(kotlinModule { enable(StrictNullChecks) }.strictNullChecks)
17+
assertTrue(kotlinModule { enable(NewStrictNullChecks) }.strictNullChecks)
18+
19+
assertThrows<IllegalArgumentException> {
20+
kotlinModule {
21+
enable(StrictNullChecks)
22+
enable(NewStrictNullChecks)
23+
}
24+
}
25+
}
26+
1227
@Test
1328
fun builder_Defaults() {
1429
val module = KotlinModule.Builder().build()
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
package com.fasterxml.jackson.module.kotlin.kogeraIntegration.deser
2+
3+
import com.fasterxml.jackson.annotation.JsonSetter
4+
import com.fasterxml.jackson.annotation.Nulls
5+
import org.junit.jupiter.api.Assertions
6+
import org.junit.jupiter.api.Nested
7+
import org.junit.jupiter.api.Test
8+
import org.junit.jupiter.api.assertThrows
9+
import tools.jackson.databind.exc.InvalidNullException
10+
import tools.jackson.databind.json.JsonMapper
11+
import tools.jackson.module.kotlin.KotlinFeature.NewStrictNullChecks
12+
import tools.jackson.module.kotlin.kotlinModule
13+
import tools.jackson.module.kotlin.readValue
14+
15+
class StrictNullChecksTest {
16+
private val mapper = JsonMapper.builder()
17+
.addModule(kotlinModule { enable(NewStrictNullChecks) })
18+
.build()
19+
20+
class ArrayWrapper(val value: Array<Int>)
21+
data class ListWrapper(val value: List<Int>)
22+
data class MapWrapper(val value: Map<String, Int>)
23+
24+
@Nested
25+
inner class NonNullInput {
26+
@Test
27+
fun array() {
28+
val expected = ArrayWrapper(arrayOf(1))
29+
val src = mapper.writeValueAsString(expected)
30+
val result = mapper.readValue<ArrayWrapper>(src)
31+
32+
Assertions.assertArrayEquals(expected.value, result.value)
33+
}
34+
35+
@Test
36+
fun list() {
37+
val expected = ListWrapper(listOf(1))
38+
val src = mapper.writeValueAsString(expected)
39+
val result = mapper.readValue<ListWrapper>(src)
40+
41+
Assertions.assertEquals(expected, result)
42+
}
43+
44+
@Test
45+
fun map() {
46+
val expected = MapWrapper(mapOf("foo" to 1))
47+
val src = mapper.writeValueAsString(expected)
48+
val result = mapper.readValue<MapWrapper>(src)
49+
50+
Assertions.assertEquals(expected, result)
51+
}
52+
}
53+
54+
data class AnyWrapper(val value: Any)
55+
56+
@Nested
57+
inner class NullInput {
58+
@Test
59+
fun array() {
60+
val src = mapper.writeValueAsString(AnyWrapper(arrayOf<Int?>(null)))
61+
assertThrows<InvalidNullException> { mapper.readValue<ArrayWrapper>(src) }
62+
}
63+
64+
@Test
65+
fun list() {
66+
val src = mapper.writeValueAsString(AnyWrapper(arrayOf<Int?>(null)))
67+
assertThrows<InvalidNullException> { mapper.readValue<ListWrapper>(src) }
68+
}
69+
70+
@Test
71+
fun map() {
72+
val src = mapper.writeValueAsString(AnyWrapper(mapOf("foo" to null)))
73+
assertThrows<InvalidNullException> { mapper.readValue<MapWrapper>(src) }
74+
}
75+
}
76+
77+
class ContentNullsSkipArrayWrapper(@JsonSetter(contentNulls = Nulls.SKIP) val value: Array<Int>)
78+
data class ContentNullsSkipListWrapper(@JsonSetter(contentNulls = Nulls.SKIP) val value: List<Int>)
79+
data class ContentNullsSkipMapWrapper(@JsonSetter(contentNulls = Nulls.SKIP) val value: Map<String, Int>)
80+
81+
@Nested
82+
inner class CustomByAnnotationTest {
83+
@Test
84+
fun array() {
85+
val expected = ContentNullsSkipArrayWrapper(emptyArray())
86+
val src = mapper.writeValueAsString(AnyWrapper(arrayOf<Int?>(null)))
87+
val result = mapper.readValue<ContentNullsSkipArrayWrapper>(src)
88+
89+
Assertions.assertArrayEquals(expected.value, result.value)
90+
}
91+
92+
@Test
93+
fun list() {
94+
val expected = ContentNullsSkipListWrapper(emptyList())
95+
val src = mapper.writeValueAsString(AnyWrapper(listOf<Int?>(null)))
96+
val result = mapper.readValue<ContentNullsSkipListWrapper>(src)
97+
98+
Assertions.assertEquals(expected, result)
99+
}
100+
101+
@Test
102+
fun map() {
103+
val expected = ContentNullsSkipMapWrapper(emptyMap())
104+
val src = mapper.writeValueAsString(AnyWrapper(mapOf("foo" to null)))
105+
val result = mapper.readValue<ContentNullsSkipMapWrapper>(src)
106+
107+
Assertions.assertEquals(expected, result)
108+
}
109+
}
110+
111+
class AnnotatedArrayWrapper(@JsonSetter(nulls = Nulls.SKIP) val value: Array<Int> = emptyArray())
112+
data class AnnotatedListWrapper(@JsonSetter(nulls = Nulls.SKIP) val value: List<Int> = emptyList())
113+
data class AnnotatedMapWrapper(@JsonSetter(nulls = Nulls.SKIP) val value: Map<String, Int> = emptyMap())
114+
115+
// If Default is specified by annotation, it is not overridden.
116+
@Nested
117+
inner class AnnotatedNullInput {
118+
@Test
119+
fun array() {
120+
val src = mapper.writeValueAsString(AnyWrapper(arrayOf<Int?>(null)))
121+
assertThrows<InvalidNullException> { mapper.readValue<AnnotatedArrayWrapper>(src) }
122+
}
123+
124+
@Test
125+
fun list() {
126+
val src = mapper.writeValueAsString(AnyWrapper(arrayOf<Int?>(null)))
127+
assertThrows<InvalidNullException> { mapper.readValue<AnnotatedListWrapper>(src) }
128+
}
129+
130+
@Test
131+
fun map() {
132+
val src = mapper.writeValueAsString(AnyWrapper(mapOf("foo" to null)))
133+
assertThrows<InvalidNullException> { mapper.readValue<AnnotatedMapWrapper>(src) }
134+
}
135+
}
136+
}

0 commit comments

Comments
 (0)