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

Using Kotlin Default Parameter Values when JSON value is null and Kotlin parameter type is Non-Nullable #130

Closed
grundleborg opened this issue Feb 21, 2018 · 40 comments
Milestone

Comments

@grundleborg
Copy link

I've got the following simplified JSON example, which I'm trying to decode into the simplified Kotlin Data Class below.

{ 
   "boolField": null,
    "stringField": null
}
data class TestObject(
        val boolField: Boolean = true,
        val stringField: String = "default"
)

The key thing here is that the Kotlin properties are not nullable, but there is a known default value for them. However, the JSON sometimes contains null for those fields.

I am trying to get the example JSON to decode using the default values in place of the nulls since the type is non-nullable. However, this doesn't work out of the box, instead throwing a MissingKotlinParameterException.

I had a look at modifying the code with a feature flag to behave the way I wanted. This was easy enough to do with some minor alterations to createFromObjectWith() in KotlinValueInstantiator for the String case. However, for the Boolean case it does not work, as in Java, that non-optional Boolean becomes a boolean primitive type, which cannot take null and thus Jackson Data Binding sets it with the default value of false.

So, assuming I haven't missed the point completely with this, I'm wondering if there's a way in the KotlinValueInstantiator to know that the primitive types were set with their default values by Jackson Data Binding in order to make this work for primitive types too?

@gerob311
Copy link

gerob311 commented Feb 27, 2018

I've also noticed that, an Int field, regardless of whether or not it has a default value, will be set to 0 if the parsed JSON field is null.

I think, if the Int field is non-null, then an exception is more appropriate, as null and zero are not the same thing. Perhaps the MissingKotlinParameterException should be the default behaviour, and some kind of annotation could set an option to allow either converting nulls to 0, false, "" etc, or to allow replacing the input null with the default value if a default value is defined.

I think that in some cases, null is a perfectly valid value to use instead of the default value, and the absence of a field in the JSON we are deserialising is the only time where the default value always makes sense.

@crypticmind
Copy link

I'm facing this issue in its more basic form. I have a non nullable no default value Kotlin Boolean value. Since Kotlin's Boolean is an alias of the primitive, like Int, the module doesn't require the value in the JSON input.

@neoXfire
Copy link

neoXfire commented Mar 27, 2018

I totally aggree with both @crypticmind and @gerob311.
If I write a Kotlin data class this way :
data class TestObject( val myInteger: Int )
i need myInteger to be present, not default to 0. It should throw MissingKotlinParameterException if absent.

If i wanted myInteger to default to 0 in case of missing, i would have written :
data class TestObject( val myInteger: Int? )
and then later testObject.myInteger ?: 0 in my code. This way, I can default to 1 or -1 if I prefer

Please remove || paramDef.isPrimitive() here :

val paramVal = if (!isMissing || paramDef.isPrimitive() || jsonProp.hasInjectableValueId()) {

@gerob311
Copy link

gerob311 commented Mar 27, 2018

In my travels, I have discovered that enabling DeserializationFeature.FAIL_ON_NULL_FOR_PRIMITIVES will give a MissingKotlinParameterException if the input value is null. I think this isn't a bad workaround, or maybe this is what was always intended in this module?

I've also discovered a slightly related case, which I'm not sure if it should be a separate ticket or not.

If I declare something like class TestObject(val timesOfDay: List<LocalTime>), Jackson allows null values to be deserialised into the list. For example, there are no exceptions if I deserialise {"timesOfDay": ["08:00", null, "09:00"]}. I would expect this should only be allowed if I declared class TestObject(val timesOfDay: List<LocalTime?>). Though, I'm not sure if type erasure means that this is impossible to fix.

@crypticmind
Copy link

I tried with DeserializationFeature.FAIL_ON_NULL_FOR_PRIMITIVES and it works as expected.
I'm using Jackson 2.8.0 and jackson-module-kotlin 2.9.4.1 (I don't know if these should match).

@cowtowncoder
Copy link
Member

@crypticmind Minor version of components should be the same, as compatibility across minor versions is not guaranteed (patch versions need not match). But often adjacent minor versions do work in some combinations.
So I would recommend upgrading databind to 2.9(.4) as well if possible.

@crypticmind
Copy link

@cowtowncoder will do. Thank you for the heads up!

@neoXfire
Copy link

neoXfire commented Mar 29, 2018

It should do the trick. Too bad it's wrapped into a generic JsonProcessingException instead of the more specific MissingKotlinParameterException.
Thank you, everyone !

@Dougrinch
Copy link

@gerob311 There is no type erasure.

val prop = TestObject::class.memberProperties.single { it.name == "timesOfDay" }
println(prop.returnType)
//kotlin.collections.List<kotlin.String>

@dri94
Copy link

dri94 commented Jun 22, 2018

Has anyone figured this out? or found a kotlin json marshaller that is a simple as GSON but respects default values?

@or-dvir
Copy link

or-dvir commented Jun 27, 2018

also curious about this. i am using a third party REST api which i have no control over and theoretically everything could be null...
i have 2 possible solutions/workarounds in mind but neither of them is good enough...

  1. use custom setter such as:
class Person(var name:String)
{
    @jsonSetter("name")
    fun setName(value: String?)
    {
        field = value ?: "default value"
    }
}

the problem with this solution is a lot of boilerplate!. i gave a very simple example with only 1 variable, but in reality there are many more...

  1. make all fields nullable:
class Person(var name: String?, var lastName: String?, var middleName:String?, var isMale:Boolean?, var mom:Person?, var dad:Person?, var friends: List<Person?>? /*and so on and so on...*/)

obviously the problem with this is that its kind of stupid to make every single variable nullable, expecially
given kotlins null-safety "feature". also this would lead to a lot of null-handling everytime i want to access a variable

val user:Person = getUser()
tvName.text = user.name ?: "default value"
tvLastName.text = user.lastName ?: "default value"
tvMom.text = user.mom?.name ?: "default value"

//and so on and so on...

ideally i would have this:

class Person(name: String = "default value" /*other variables with default values*/)

and if in the json i get from the server name is null, then "default value" would be assigned name

how can i go about achieving this?

@apatrida
Copy link
Member

There are conflicts with various settings and picking one model.

Kotlin itself does not treat null and as the same. Neither does Jackson by default which would throw an exception for null being set into a primitive depending on the setting of the FAIL_ON_NULL_FOR_PRIMITIVES feature switch.

For a non nullable type should we treat null as when there is a default value available in the method signature? what about a nullable type? how do we know when null is intentional versus meaning ?

@apatrida
Copy link
Member

Your workaround for now is to remove the null values from your JSON completely to mean "absent" instead of using null

@or-dvir
Copy link

or-dvir commented Jul 16, 2018

There are conflicts with various settings and picking one model.

Kotlin itself does not treat null and as the same. Neither does Jackson by default which would throw an exception for null being set into a primitive depending on the setting of the FAIL_ON_NULL_FOR_PRIMITIVES feature switch.

For a non nullable type should we treat null as when there is a default value available in the method signature? what about a nullable type? how do we know when null is intentional versus meaning ?

expected behavior (at least for me):

  • trying to set null to a non-nullable variable (e.g. String)
    if variable has default value, use it. otherwise throw exception
  • trying to set null to a nullable variable (e,g, 'String?')
    set to null regardless if there is a default value or not (as the developer has explicitly declared that null is acceptable).

Your workaround for now is to remove the null values from your JSON completely to mean "absent" instead of using null

not a good solution as many developers are using 3rd party APIs which they have no control over

@halfhp
Copy link

halfhp commented Oct 24, 2018

This is coming up when trying to use Arrow's Option as well. Kotlin's nullable is a step in the right direction but isnt a replacement for real optionals. I'd love to be able to write val foo: Option<T> instead of val foo: T?, which means defaulting to None<T> for fields of type Option<T>.

@jmiecz
Copy link

jmiecz commented Dec 27, 2018

Hello,

Is there a way to just ignore nulls during deserialization? Similar to how we have a @include(NON_NULLS) for serialization.

@cowtowncoder
Copy link
Member

@jmiecz Yes, Jackson 2.9 has @JsonSetter which is mainly used to configure action taken on encountering explicit null value. See

https://medium.com/@cowtowncoder/jackson-2-9-features-b2a19029e9ff

for an example of how it works.

@jmiecz
Copy link

jmiecz commented Dec 30, 2018

@cowtowncoder That does not work with Kotlin's data classes. I see an error being thrown saying a non-null value is being assigned to null.

@cowtowncoder
Copy link
Member

@jmiecz That may be (and if so, sounds like a possible bug in Kotlin module). But that is the mechanism that should work from general Jackson API perspective. It might make sense to file a separate issue showing intended working, based on how similar POJO works wrt @JsonSetter.

@jmiecz
Copy link

jmiecz commented Dec 31, 2018

@cowtowncoder Not really, with Kotlin's data class, you have to set the values or provide defaults (IE: data class someClass(@JsonProperty("test") someJson: String = "") )

Jackson isn't ignoring the setter (even though you config it to ignore it) and setting a value it shouldn't be setting (IE of json: { "test": null } )

@araqnid
Copy link

araqnid commented Dec 31, 2018

Kotlin data class vals are creator parameters, not setters, so I suspect that's why marking setters as ignored doesn't have any effect. The data class will only have two constructors - one to specify every val, and an additional one to also indicate which vals should take their default value -- calling that from Java is quite painful, as I remember.

Some issues above can be solved by having Jackson go through a data class (with lots of nullable values) as a builder, but if you need to do it a lot, that's going to feel like lots of boilerplate too.

@cowtowncoder
Copy link
Member

@jmiecz I think you may be misunderstanding what I am saying (or reading too much into naming): @JsonSetter is annotation that affects handling of values for deserialization, regardless of whether value itself is injected using setter, field, or passed via constructor. It is not specific to use of setter methods.

It may be that the way Kotlin runtime handles default values has effect on how jackson-databind/kotlin module need to use that information, but configuration of what should happen should come from @JsonSetter (and equivalent), as provided by AnnotationIntrospector or config overrides (set on ObjectMapper).

@nikolaikopernik
Copy link

Hey, is there any plans to fix it?

For now my workaround - creating a separate constructor special for Jackson:

data class TestObject(
        val boolField: Boolean = true,
        val stringField: String = "default") {
   @JsonCreator
    constructor(@JsonProperty("boolField") boolField: Boolean?,
                @JsonProperty("stringField") stringField: String?) :
            this(boolField ?: true, stringField ?: "default")
}

@Darych
Copy link

Darych commented Mar 14, 2019

Another way to workaround when you don't have primitive types in constructors.

data class TestObject(
    val name: String,
    val type: String,
    @JsonProperty("info")
    private val nullInfo: String?
) {
    val info: String
        get() = nullInfo ?: ""

@smaurya52
Copy link

I am also facing the same issue with deserialization of csv data containing null/empty values.
Is there any update on this issue ?

@NumezmaT
Copy link
Contributor

For me I fixed it, add ?: return@forEachIndexed in
KotlinValueInstantiator

var paramVal = if (!isMissing || paramDef.isPrimitive() || jsonProp.hasInjectableValueId()) {
	buffer.getParameter(jsonProp) ?: return@forEachIndexed
} else {
	jsonProp.valueDeserializer?.getNullValue(ctxt)
}

I think it will be better if this behavior will be depend on @JsonSetter or another configurable. But in kotlin this behavior it should be by default.

@cowtowncoder
Copy link
Member

It would be possible for Kotlin-module-provided AnnotationIntrospector to indicate that "null setter" logic was "use empty". But someone has to provide the PR to do that, including tests.
Asking or demanding that someone fix this will not help if no one is working on it. I do not work on Kotlin module since I don't have enough Kotlin knowledge and since there is plenty of other necessary work (including support for modules lie Kotlin). I can help get things merged, answer questions on databind core logic. But not do fixes here.

@bastman
Copy link

bastman commented Oct 14, 2019

I just did a couple of tests ...

source "{}" -> to be parsed as

a) data class Foo(val s:String?="default", val i:Int?=100)
b) data class Foo(val s:String="default", val i:Int=100)

Both cases get parsed properly ...

a)

source: {}
parsed: Foo(s=default, i=100)
parsed.toJson(): {"s":"default","i":100}

b)

source: {}
parsed: Foo(s=default, i=100)
parsed.toJson(): {"s":"default","i":100}

tested with jackson 2.9.9 using the following configuration ...

        fun defaultMapper(): ObjectMapper = jacksonObjectMapper()
                .findAndRegisterModules()
                // toJson()
                .disable(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS)
                .disable(SerializationFeature.WRITE_DURATIONS_AS_TIMESTAMPS)
                // fromJson()
                .disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES)
                .disable(DeserializationFeature.ACCEPT_EMPTY_ARRAY_AS_NULL_OBJECT)
                .disable(DeserializationFeature.ACCEPT_EMPTY_STRING_AS_NULL_OBJECT)
                .disable(DeserializationFeature.ACCEPT_SINGLE_VALUE_AS_ARRAY)
                .enable(DeserializationFeature.FAIL_ON_NULL_FOR_PRIMITIVES)
                .enable(DeserializationFeature.FAIL_ON_NUMBERS_FOR_ENUMS)
    }

Hope this helps.

@NumezmaT
Copy link
Contributor

@bastman try parsed same like this
source
{ "s": null }

Spoiler: you do not receive the expected result

@bastman
Copy link

bastman commented Oct 15, 2019

@NumezmaT , just tried your use case.
You are right. Thanks a lot for the hint.

I summed it up here: https://gist.github.com/bastman/94f6c8afb86edfaf6a302b990dcfe210

@sdelicata
Copy link

Hey, is there any plans to fix it?

For now my workaround - creating a separate constructor special for Jackson:

data class TestObject(
        val boolField: Boolean = true,
        val stringField: String = "default") {
   @JsonCreator
    constructor(@JsonProperty("boolField") boolField: Boolean?,
                @JsonProperty("stringField") stringField: String?) :
            this(boolField ?: true, stringField ?: "default")
}

Cannot work, you have both constructors with the same JVM signature :(

@apatrida
Copy link
Member

This issue is confusing because most of the cases are not represented as full test cases. The best way to state a case for something that is a bit hard to describe verbally is to provide a unit test you expect to pass that is not passing, in its entirety here. I think you would then find opposing unit tests that counter wanting this change as-is.

This seems like something that must be configured to treat null as "take default" instead of as null. Kotlin for one is very clear about what null means and it never means "default".

So to change the behavior, because obviously it is desired to have the option, it should be configurable, and not default behavior.

@apatrida
Copy link
Member

Branch 2.10 now allows null to be treated as "use the default" value, but requires that you set a flag when creating the Kotlin module, otherwise the behavior stays as it was as the default behavior.

ObjectMapper().registerModule(KotlinModule(nullisSameAsDefault = true)

@cowtowncoder
Copy link
Member

@apatrida feel free to revert if this was a conflicting change taking things in wrong direction.

@apatrida
Copy link
Member

@cowtowncoder I worked it out with an option, so it would not break expectations of people who want it enforced as-is, but allowing the flexibility for those who need it and cannot change their JSON to avoid the problem. We are good.

@mario-paniccia
Copy link

mario-paniccia commented Nov 9, 2022

Branch 2.10 now allows null to be treated as "use the default" value, but requires that you set a flag when creating the Kotlin module, otherwise the behavior stays as it was as the default behavior.

ObjectMapper().registerModule(KotlinModule(nullisSameAsDefault = true)

Brilliant this worked thanks.
But KotlinModule is now deprecated. Current solution:

val kotlinModule = KotlinModule.Builder()
            .configure(KotlinFeature.NullIsSameAsDefault, true)
            .build()

@MartinX3
Copy link

MartinX3 commented Nov 9, 2022

ObjectMapper()
        .registerKotlinModule()
        .registerModules(
            Builder()
                .enable(NullIsSameAsDefault)
                .build()
        )

@alturkovic
Copy link

This does not work for me with 2.16.0.

Here is a simple example:

fun main() {
    val mapper = jacksonObjectMapper()
    val component = Component()
    val json = mapper.writeValueAsString(component)
    println(json) // {} works correctly
    println(mapper.readValue<Component>(json)) // fails
}

class Component(
    random: Random = Random
)

The failure:

Exception in thread "main" com.fasterxml.jackson.databind.exc.InvalidDefinitionException: Cannot construct instance of `kotlin.random.Random` (no Creators, like default constructor, exist): abstract types either need to be mapped to concrete types, have custom deserializer, or contain additional type information
 at [Source: REDACTED (`StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION` disabled); line: 1, column: 2]
	at com.fasterxml.jackson.databind.exc.InvalidDefinitionException.from(InvalidDefinitionException.java:67)
	at com.fasterxml.jackson.databind.DeserializationContext.reportBadDefinition(DeserializationContext.java:1887)
	at com.fasterxml.jackson.databind.DatabindContext.reportBadDefinition(DatabindContext.java:414)
	at com.fasterxml.jackson.databind.DeserializationContext.handleMissingInstantiator(DeserializationContext.java:1375)
	at com.fasterxml.jackson.databind.deser.AbstractDeserializer.deserialize(AbstractDeserializer.java:274)
	at com.fasterxml.jackson.databind.deser.BeanDeserializerBase.deserializeFromObjectUsingNonDefault(BeanDeserializerBase.java:1412)
	at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserializeFromObject(BeanDeserializer.java:348)
	at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:185)
	at com.fasterxml.jackson.databind.deser.DefaultDeserializationContext.readRootValue(DefaultDeserializationContext.java:342)
	at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4899)
	at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3846)
	at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3829)
	at example.ComponentKt.main(Component.kt:23)
	at example.ComponentKt.main(Component.kt)

It does work however with the following example:

class Component(
    id: String = UUID.randomUUID().toString()
)

@spartan-vutran
Copy link

On 2.13.0, I met the same issue with @alturkovic. It seems the feature doesn't still work when using

val kotlinModule = KotlinModule.Builder()
            .configure(KotlinFeature.NullIsSameAsDefault, true)
            .build()

@ivelasquez-rl
Copy link

On 2.13.0, I met the same issue with @alturkovic. It seems the feature doesn't still work when using

val kotlinModule = KotlinModule.Builder()
            .configure(KotlinFeature.NullIsSameAsDefault, true)
            .build()

This works fine in 2.15. Thanks for the answer!!!

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

No branches or pull requests