From 30cd53eb593f7d71e74fcece6007c5fd3f416985 Mon Sep 17 00:00:00 2001 From: ddomanine Date: Thu, 26 Aug 2021 21:21:04 +0200 Subject: [PATCH 1/5] JsonNode? missing value should be deserialized as null and not as NullNode --- .../module/kotlin/KotlinValueInstantiator.kt | 9 +++-- .../module/kotlin/test/github/Github490.kt | 34 +++++++++++++++++++ 2 files changed, 41 insertions(+), 2 deletions(-) create mode 100644 src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/github/Github490.kt 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 aed1ee847..4d79f1d30 100644 --- a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinValueInstantiator.kt +++ b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinValueInstantiator.kt @@ -101,8 +101,13 @@ internal class KotlinValueInstantiator( } tempParamVal } else { - // trying to get suitable "missing" value provided by deserializer - jsonProp.valueDeserializer?.getNullValue(ctxt) + if(paramDef.type.isMarkedNullable) { + // do not try to create any object if it is nullable + null + } else { + // trying to get suitable "missing" value provided by deserializer + jsonProp.valueDeserializer?.getNullValue(ctxt) + } } if (paramVal == null && ((nullToEmptyCollection && jsonProp.type.isCollectionLikeType) || (nullToEmptyMap && jsonProp.type.isMapLikeType))) { diff --git a/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/github/Github490.kt b/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/github/Github490.kt new file mode 100644 index 000000000..2447affe6 --- /dev/null +++ b/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/github/Github490.kt @@ -0,0 +1,34 @@ +package com.fasterxml.jackson.module.kotlin.test.github + +import com.fasterxml.jackson.databind.JsonNode +import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper +import com.fasterxml.jackson.module.kotlin.readValue +import org.hamcrest.CoreMatchers +import org.hamcrest.MatcherAssert.assertThat +import org.junit.Test + +class Github490 { + + @Test + fun testKotlinDeserialization() { + val mapper = jacksonObjectMapper() + val value: DataClassWithAllNullableParams = mapper.readValue("{}") + assertThat( + "Nullable missing Int value should be deserialized as null", + value.intValue, + CoreMatchers.nullValue() + ) + assertThat( + "Nullable missing String value should be deserialized as null", + value.stringValue, + CoreMatchers.nullValue() + ) + assertThat( + "Nullable missing JsonNode value should be deserialized as null and not as NullNode", + value.jsonNodeValue, + CoreMatchers.nullValue() + ) + } +} + +data class DataClassWithAllNullableParams(val intValue: Int?, val stringValue: String?, val jsonNodeValue: JsonNode?) \ No newline at end of file From 47bf1cf980afae821e63027a2f853b97b91d497f Mon Sep 17 00:00:00 2001 From: ddomanine Date: Fri, 27 Aug 2021 15:56:05 +0200 Subject: [PATCH 2/5] Added more cases that fail: fields with default values and payload with explicit null values --- .../module/kotlin/test/github/Github490.kt | 28 +++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/github/Github490.kt b/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/github/Github490.kt index 2447affe6..c8bf67229 100644 --- a/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/github/Github490.kt +++ b/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/github/Github490.kt @@ -12,7 +12,9 @@ class Github490 { @Test fun testKotlinDeserialization() { val mapper = jacksonObjectMapper() - val value: DataClassWithAllNullableParams = mapper.readValue("{}") + val value: DataClassWithAllNullableParams = mapper.readValue("{" + + "\"jsonNodeValueWithNullAsDefaultProvidedNull\":null, " + + "\"jsonNodeValueProvidedNull\":null}") assertThat( "Nullable missing Int value should be deserialized as null", value.intValue, @@ -28,7 +30,29 @@ class Github490 { value.jsonNodeValue, CoreMatchers.nullValue() ) + assertThat( + "Nullable missing JsonNode value should be deserialized as null and not as NullNode", + value.jsonNodeValueProvidedNull, + CoreMatchers.nullValue() + ) + assertThat( + "Nullable by default missing JsonNode value should be deserialized as null and not as NullNode", + value.jsonNodeValueWithNullAsDefault, + CoreMatchers.nullValue() + ) + assertThat( + "Nullable by default JsonNode with provided null value in payload should be deserialized as null and not as NullNode", + value.jsonNodeValueWithNullAsDefaultProvidedNull, + CoreMatchers.nullValue() + ) } } -data class DataClassWithAllNullableParams(val intValue: Int?, val stringValue: String?, val jsonNodeValue: JsonNode?) \ No newline at end of file +data class DataClassWithAllNullableParams( + val intValue: Int?, + val stringValue: String?, + val jsonNodeValue: JsonNode?, + val jsonNodeValueProvidedNull: JsonNode?, + val jsonNodeValueWithNullAsDefault: JsonNode? = null, + val jsonNodeValueWithNullAsDefaultProvidedNull: JsonNode? = null +) \ No newline at end of file From d88a3d6bd356b8c5192ecd967674ed4724cd4dc7 Mon Sep 17 00:00:00 2001 From: ddomanine Date: Tue, 7 Sep 2021 23:17:58 +0200 Subject: [PATCH 3/5] rebased to 2.13 and applied patch --- .../module/kotlin/test/github/Github490.kt | 38 +++++++++++++++---- 1 file changed, 30 insertions(+), 8 deletions(-) diff --git a/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/github/Github490.kt b/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/github/Github490.kt index c8bf67229..55792b6de 100644 --- a/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/github/Github490.kt +++ b/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/github/Github490.kt @@ -7,39 +7,61 @@ import org.hamcrest.CoreMatchers import org.hamcrest.MatcherAssert.assertThat import org.junit.Test -class Github490 { +class TestGithub490 { + val mapper = jacksonObjectMapper() + val value: DataClassWithAllNullableParams = mapper.readValue( + "{" + + "\"jsonNodeValueWithNullAsDefaultProvidedNull\":null, " + + "\"jsonNodeValueProvidedNull\":null}" + ) @Test - fun testKotlinDeserialization() { - val mapper = jacksonObjectMapper() - val value: DataClassWithAllNullableParams = mapper.readValue("{" + - "\"jsonNodeValueWithNullAsDefaultProvidedNull\":null, " + - "\"jsonNodeValueProvidedNull\":null}") + fun testKotlinDeserialization_intValue() { assertThat( "Nullable missing Int value should be deserialized as null", value.intValue, CoreMatchers.nullValue() ) + } + + @Test + fun testKotlinDeserialization_stringValue() { assertThat( "Nullable missing String value should be deserialized as null", value.stringValue, CoreMatchers.nullValue() ) + } + + @Test + fun testKotlinDeserialization_jsonNodeValue() { assertThat( "Nullable missing JsonNode value should be deserialized as null and not as NullNode", value.jsonNodeValue, CoreMatchers.nullValue() ) + } + + @Test + fun testKotlinDeserialization_jsonNodeValueProvidedNull() { assertThat( - "Nullable missing JsonNode value should be deserialized as null and not as NullNode", + "Nullable JsonNode value provided as null should be deserialized as null and not as NullNode", value.jsonNodeValueProvidedNull, CoreMatchers.nullValue() ) + } + + @Test + fun testKotlinDeserialization_jsonNodeValueWithNullAsDefault() { assertThat( "Nullable by default missing JsonNode value should be deserialized as null and not as NullNode", value.jsonNodeValueWithNullAsDefault, CoreMatchers.nullValue() ) + } + + @Test + fun testKotlinDeserialization_jsonNodeValueWithNullAsDefaultProvidedNull() { assertThat( "Nullable by default JsonNode with provided null value in payload should be deserialized as null and not as NullNode", value.jsonNodeValueWithNullAsDefaultProvidedNull, @@ -55,4 +77,4 @@ data class DataClassWithAllNullableParams( val jsonNodeValueProvidedNull: JsonNode?, val jsonNodeValueWithNullAsDefault: JsonNode? = null, val jsonNodeValueWithNullAsDefaultProvidedNull: JsonNode? = null -) \ No newline at end of file +) From f1a3c9b6d31627642cdd0b82354cc61cd2a342c3 Mon Sep 17 00:00:00 2001 From: ddomanine Date: Wed, 8 Sep 2021 09:31:17 +0200 Subject: [PATCH 4/5] if value is missing, rely on deserilizer.getAbscentValue instead of deserilizer.getNullValue. Fixing tests --- .../jackson/module/kotlin/KotlinValueInstantiator.kt | 6 +++--- .../jackson/module/kotlin/test/github/Github490.kt | 9 +++++---- 2 files changed, 8 insertions(+), 7 deletions(-) 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 4d79f1d30..2196d9687 100644 --- a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinValueInstantiator.kt +++ b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinValueInstantiator.kt @@ -102,11 +102,11 @@ internal class KotlinValueInstantiator( tempParamVal } else { if(paramDef.type.isMarkedNullable) { - // do not try to create any object if it is nullable + // do not try to create any object if it is nullable and the value is missing null } else { - // trying to get suitable "missing" value provided by deserializer - jsonProp.valueDeserializer?.getNullValue(ctxt) + // to get suitable "missing" value provided by deserializer + jsonProp.valueDeserializer?.getAbsentValue(ctxt) } } diff --git a/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/github/Github490.kt b/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/github/Github490.kt index 55792b6de..38fe26109 100644 --- a/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/github/Github490.kt +++ b/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/github/Github490.kt @@ -1,6 +1,7 @@ package com.fasterxml.jackson.module.kotlin.test.github import com.fasterxml.jackson.databind.JsonNode +import com.fasterxml.jackson.databind.node.NullNode import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper import com.fasterxml.jackson.module.kotlin.readValue import org.hamcrest.CoreMatchers @@ -45,9 +46,9 @@ class TestGithub490 { @Test fun testKotlinDeserialization_jsonNodeValueProvidedNull() { assertThat( - "Nullable JsonNode value provided as null should be deserialized as null and not as NullNode", + "Nullable JsonNode value provided as null should be deserialized as NullNode", value.jsonNodeValueProvidedNull, - CoreMatchers.nullValue() + CoreMatchers.equalTo(NullNode.instance) ) } @@ -63,9 +64,9 @@ class TestGithub490 { @Test fun testKotlinDeserialization_jsonNodeValueWithNullAsDefaultProvidedNull() { assertThat( - "Nullable by default JsonNode with provided null value in payload should be deserialized as null and not as NullNode", + "Nullable by default JsonNode with provided null value in payload should be deserialized as NullNode", value.jsonNodeValueWithNullAsDefaultProvidedNull, - CoreMatchers.nullValue() + CoreMatchers.equalTo(NullNode.instance) ) } } From aaee7fb448c7855d8cdc95d20d3808cf44e30ce1 Mon Sep 17 00:00:00 2001 From: ddomanine Date: Wed, 8 Sep 2021 09:48:36 +0200 Subject: [PATCH 5/5] credits --- pom.xml | 2 +- release-notes/CREDITS-2.x | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 3844ea014..aa02b5b35 100644 --- a/pom.xml +++ b/pom.xml @@ -8,7 +8,7 @@ com.fasterxml.jackson jackson-base - 2.13.0-rc2-SNAPSHOT + 2.13.0-SNAPSHOT com.fasterxml.jackson.module jackson-module-kotlin diff --git a/release-notes/CREDITS-2.x b/release-notes/CREDITS-2.x index ff253804e..629e1f913 100644 --- a/release-notes/CREDITS-2.x +++ b/release-notes/CREDITS-2.x @@ -32,6 +32,10 @@ wrongwrong (k163377@github) * Contributed #438: Fixed mapping failure when `private` `companion object` is named (2.13) +Dmitri Domanine (novtor@github) +* Contributed fix for #490: Missing value of type JsonNode? is deserialized as NullNode instead of null + (2.13) + Marshall Pierce (marshallpierce@github) * #474: Reported disrespect for @JsonProperty on parent class (2.12.NEXT)