FasterXML / FasterXML/jackson-module-kotlin
Invalid targetType in KotlinInvalidNullException
- Dominant language
- Kotlin
- Stars
- 1.2k
- Forks
- 187
- Avg merge
- 9h 48m
- Merged PRs (30d)
- 16
Description
### Search before asking
- [x] I searched in the [issues](https://github.com/FasterXML/jackson-module-kotlin/issues) and found nothing similar.
- [x] I have confirmed that the same problem is not reproduced if I exclude the KotlinModule.
- [x] I searched in the [issues of databind](https://github.com/FasterXML/jackson-databind/issues) and other modules used and found nothing similar.
- [x] I have confirmed that the problem does not reproduce in Java and only occurs when using Kotlin and KotlinModule.
### Describe the bug
While deserializing objects that have non-nullable fields, the exception thrown behaves differently, depending on what is the underlying type.
If there's an underlying Java primitive, then a `TypeMismatchException` from `jackson-databind` allows to retrieve what was the type of the field.
```kotlin
// this is a modified snippet from KotlinInvalidNullExceptionTest.kt
private data class IntDto(
val foo: Int
)
class KotlinInvalidNullExceptionTest {
@Test
fun fooTest() {
val json = """{"bar":"bar"}"""
val ex2 = assertThrows { defaultMapper.readValue(json) }
assertEquals("foo", ex2.path.map { it.propertyName }.joinToString("."))
assertEquals(Int::class, ex2.targetType.kotlin)
// notice that here targetType returns Int
}
}
```
However, when I'm deserializing a type that is backed by a non-primitive, then a `KotlinInvalidNullException` is thrown and `getTargetType()` returns type of a DTO object, not type of a field.
This is even tested currently in `KotlinInvalidNullExceptionTest.kt`:
```kotlin
private data class Dto(
val foo: String,
@JsonProperty("bar")
val _bar: String
)
class KotlinInvalidNullExceptionTest {
@Test
fun fooTest() {
val json = """{"bar":"bar"}"""
val ex = assertThrows { defaultMapper.readValue(json) }
assertEquals("foo", ex.kotlinPropertyName)
assertEquals("foo", ex.propertyName.simpleName)
assertEquals(Dto::class, ex.targetType.kotlin)
// notice here that targetType returns Dto
}
}
```
### To Reproduce
This is a modified snippet from currently existing tests.
```kotlin
private data class Dto(
val foo: String,
@JsonProperty("bar")
val _bar: String
)
private data class IntDto(
val foo: Int
)
class KotlinInvalidNullExceptionTest {
@Test
fun fooTest() {
val json = """{"bar":"bar"}"""
val ex = assertThrows { defaultMapper.readValue(json) }
assertEquals("foo", ex.kotlinPropertyName)
assertEquals("foo", ex.propertyName.simpleName)
assertEquals("foo", ex.path.map { it.propertyName }.joinToString("."))
// I think this should be String::class, because the property is of type String
assertEquals(Dto::class, ex.targetType.kotlin)
val ex2 = assertThrows { defaultMapper.readValue(json) }
assertEquals("foo", ex2.path.map { it.propertyName }.joinToString("."))
assertEquals(Int::class, ex2.targetType.kotlin)
}
}
```
### Expected behavior
Because one exception inherits from another, I believe that it should also provide information in the same way - the information about field type, not the deserialized object.
### Versions
Kotlin:
Jackson-module-kotlin: noticed on 3.0.4, tested also on current 3.x branch
Jackson-databind: noticed on 3.0.4, tested also on current 3.x branch
### Additional context
I think the easy fix would be to change this in `KotlinValueInstantiator`, by replacing `this.valueClass` with `paramType.javaType as Class<*>`:
```kotlin
if (isMissingAndRequired || (!paramType.isMarkedNullable && !paramType.isGenericTypeVar())) {
throw KotlinInvalidNullException(
paramDef.name,
// this.valueClass, // this is currently
paramType.javaType as Class<*>, // this makes test return String::class instead of Dto::class
ctxt.parser,
"Instantiation of ${this.valueTypeDesc} value failed for JSON property $pname due to missing (therefore NULL) value for creator parameter ${paramDef.name} which is a non-nullable type",
jsonProp.fullName,
).wrapWithPath(this.valueClass, pname)
}
```
Sorry for not raising a PR for this, as I'm not exactly a Kotlin expert and I'm not sure about that `as Class<*>` casting if it wouldn't break something else 😅
Contributor guide
No contributing guide indexed for this repository
Research direction
Start in KotlinValueInstantiator and review KotlinInvalidNullExceptionTest.kt, especially the cases for nullable-backed and primitive-backed properties. Verify how KotlinInvalidNullException receives its target type and how the exception is wrapped with a path. Done means the exception reports the missing field's type, such as String or Int, while the existing property and path assertions still pass.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- kotlin
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100