Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CSV EMPTY_STRING_AS_NULL with non-nullable default value = exception #605

Open
hosswald opened this issue Nov 16, 2022 · 5 comments
Open
Labels

Comments

@hosswald
Copy link

Describe the bug
I have a csv with empty cells. I generally want those to be parsed as null. However, I want to also have the ability to parse these cells into non-nullable properties by defining a default value in the constructor. Unfortunately, this leads to an exception:

Instantiation of [simple type, class mypackage.Foo] value failed for JSON property bar due to missing (therefore NULL) value for creator parameter bar which is a non-nullable type
 at [Source: (StringReader); line: 2, column: 2] (through reference chain: mypackage.Foo["bar"])
com.fasterxml.jackson.module.kotlin.MissingKotlinParameterException: Instantiation of [simple type, class mypackage.Foo] value failed for JSON property bar due to missing (therefore NULL) value for creator parameter bar which is a non-nullable type
 at [Source: (StringReader); line: 2, column: 2] (through reference chain: mypackage.Foo["bar"])
	at app//com.fasterxml.jackson.module.kotlin.KotlinValueInstantiator.createFromObjectWith(KotlinValueInstantiator.kt:84)
	at app//com.fasterxml.jackson.databind.deser.impl.PropertyBasedCreator.build(PropertyBasedCreator.java:202)
	at app//com.fasterxml.jackson.databind.deser.BeanDeserializer._deserializeUsingPropertyBased(BeanDeserializer.java:444)
	at app//com.fasterxml.jackson.databind.deser.BeanDeserializerBase.deserializeFromObjectUsingNonDefault(BeanDeserializerBase.java:1405)
	at app//com.fasterxml.jackson.databind.deser.BeanDeserializer.deserializeFromObject(BeanDeserializer.java:352)
	at app//com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:185)
	at app//com.fasterxml.jackson.databind.MappingIterator.nextValue(MappingIterator.java:283)
	at app//com.fasterxml.jackson.databind.MappingIterator.readAll(MappingIterator.java:323)
	at app//com.fasterxml.jackson.databind.MappingIterator.readAll(MappingIterator.java:309)

To Reproduce

data class Foo(val bar: String = "default", val baz: String)

class EmptyStringAsNullIntoNunNullableParameterWithDefaultValue {
    @Test
    internal fun test() {
        val parsed = CsvMapper()
            .registerModule(KotlinModule())
            .readerFor(Foo::class.java)
            .withFeatures(CsvParser.Feature.EMPTY_STRING_AS_NULL)
            .with(CsvSchema.emptySchema().withHeader())
            .readValues<Foo>(
                """
                bar,    baz
                ,       42                
            """.trimIndent()
            ).readAll()
        println(parsed)
    }
}

Expected behavior
It can be argued that a json like {"bar":null, "baz":"42"} should produce an error when parsed into Foo, because there is an explicit null.

However, in the case of CSV, there is no such thing as "explicit null". The value is only null because of CsvParser.Feature.EMPTY_STRING_AS_NULL. Therefore, I would expect the above test not to fail.
KotlinValueInstantiator should probably handle this case by setting isMissing = true, which would likely fix the problem (I did not test this, I only had a brief look at the code).

Versions
Kotlin: 1.7.20
Jackson-module-kotlin: 2.14.0
Jackson-databind: 2.14.0

@hosswald hosswald added the bug label Nov 16, 2022
@hosswald hosswald changed the title CsvParser.Feature.EMPTY_STRING_AS_NULL + non-nullable + default value = exception CSV EMPTY_STRING_AS_NULL with non-nullable default value = exception Nov 16, 2022
@hosswald
Copy link
Author

Ok, this fixes it for me:

KotlinModule.Builder()
    .configure(KotlinFeature.NullIsSameAsDefault, true)
.build()

However, I'm not really passing null here, I'm just deciding how empty cells in my CSV should be handled (absent any other value, such as a default value), so it feels a bit odd that I have to enable this KotlinFeature.

@hosswald
Copy link
Author

hosswald commented Nov 16, 2022

Put differently, the usage of default parameter should be the default when parsing a CSV and encountering an empty cell, regardless of whether or not EMPTY_STRING_AS_NULL is used, in my opinion.
default by default, so to say.

@cowtowncoder
Copy link
Member

@hosswald Unfortunately I am not sure it is possible to have such cross-dependencies; use of CSV backend is independent of use of Kotlin module. And from architecture perspective there is no good way to add such constraints, even if they might make sense.

@hosswald
Copy link
Author

hosswald commented Nov 17, 2022

@cowtowncoder I just looked a bit through the test cases for jackson-dataformats-text/csv...
MissingColumnsTest::testDefaultMissingHandling() handles missing columns the way I would like missing cells (in a present column) to be handled, but NullReadTest::testEmptyStringAsNull330() ensures empty cells are handled as null even if there is a default value.
Do you think it makes sense to start a discussion in the issuer tracker there?

@cowtowncoder
Copy link
Member

@hosswald Yes, I think that if handling can be defined in more general way, not relying on some specific Kotlin aspect, it'd make sense to file an RFE there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants