FasterXML / FasterXML/jackson-module-kotlin

Invalid targetType in KotlinInvalidNullException

Open
#1,140 1 comment 0 reactions 0 assignees View on GitHub
enhancement
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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.