jackson-module-kotlin icon indicating copy to clipboard operation
jackson-module-kotlin copied to clipboard

Unexpected behavior when using `NON_DEFAULT` with `data class`

Open Quantum64 opened this issue 4 years ago • 17 comments

Describe the bug

data class Data(val flag: Boolean = true)

The documentation for NON_DEFAULT suggests to me that the serializer will create a new instance of this class using the zero-argument constructor and retrieve default values. From the documentation:

When used for a POJO, definition is that only values that differ from the default values of POJO properties are included. This is done by creating an instance of POJO using zero-argument constructor, and accessing property values: value is used as the default value by using equals() method, except for the case where property has `null` value in which straight null check is used.

This class data class has default values specified for all fields, so it has a zero-argument constructor (at least from the perspective of the Kotlin language). However, this class is serialized as if the default value for flag is false rather than true. This causes the below roundtrip to fail.

To Reproduce

import com.fasterxml.jackson.annotation.JsonInclude
import com.fasterxml.jackson.databind.ObjectMapper
import com.fasterxml.jackson.module.kotlin.readValue
import com.fasterxml.jackson.module.kotlin.registerKotlinModule

fun main() {
    data class Data (val flag: Boolean = true)

    val mapper = ObjectMapper()
        .registerKotlinModule()
        .setSerializationInclusion(JsonInclude.Include.NON_DEFAULT)

    val initial = Data(flag = false)
    val serialized = mapper.writeValueAsString(initial)
    println(serialized)
    val deserialized = mapper.readValue<Data>(serialized)
    println(initial)
    println(deserialized)
    println(initial == deserialized)
}

Output

{}
Data(flag=false)
Data(flag=true)
false

Expected behavior I would expect NON_DEFAULT to utilize the Kotlin default values when serializing, preventing roundtrip inconsistency. At minimum, this behavior should be documented.

Versions Kotlin: 1.5.2 Jackson-module-kotlin: 2.12.4 Jackson-databind: 2.12.4

Quantum64 avatar Jul 19 '21 15:07 Quantum64

Weird, when I have a test like this:

  @Test
  fun default() {
      data class Data(val flag: Boolean = true)

      val mapper = jsonMapper {
          this.serializationInclusion(JsonInclude.Include.NON_DEFAULT)
      }

      val initial = Data(flag = false)
      val serialized = mapper.writeValueAsString(initial)
      assertEquals("{}", serialized)
      val deserialized = mapper.readValue<Data>(serialized)
      assertEquals(initial, deserialized)
  }

I get a runtime exception:

com.fasterxml.jackson.databind.exc.MismatchedInputException: Cannot construct instance of `com.fasterxml.jackson.module.kotlin.test.github.failing.Github478Test$default$Data` (although at least one Creator exists): cannot deserialize from Object value (no delegate- or property-based Creator)
 at [Source: (String)"{}"; line: 1, column: 2]

Adding @JsonProperty("flag") to the field causes everything to work properly, avoiding that exception and getting the non-default behavior you're looking for. I'm not sure what's causing either issue, though.

dinomite avatar Jul 20 '21 20:07 dinomite

@dinomite The issue is the jsonMapper {} DSL. I get the same error when using it, but not when using the ObjectMapper().registerKotlinModule() syntax.

Edit: It seems .registerKotlinModule() is still needed after using the jsonMapper {} DSL. Wasn't aware since I don't use that syntax.

Quantum64 avatar Jul 20 '21 20:07 Quantum64

Oh, duh, that's definitely the exception problem. Looks like the serialization inclusion flag is broken, though :-(

dinomite avatar Jul 20 '21 20:07 dinomite

…and looks to be newly-broken in 2.13

dinomite avatar Jul 20 '21 20:07 dinomite

Scratch that, not newly broken; it's also broken in 2.12

dinomite avatar Jul 20 '21 20:07 dinomite

I agree. I've been subtly working around this issue for at least a year and only had the initiative to report it the last time it took me on an hour-long debugging journey. 🙂

Quantum64 avatar Jul 20 '21 20:07 Quantum64

Thanks for reporting—I'm surprised this is the first report!

dinomite avatar Jul 20 '21 20:07 dinomite

Also interesting: even with a "real" zero-argument constructor in the bytecode generated with

data class Data @JvmOverloads constructor(val flag: Boolean = true) 

the test still fails.

Quantum64 avatar Jul 20 '21 20:07 Quantum64

I guess I should have tested this before reporting, but the exact same test fails in Java. Looks like this has nothing to do with the Kotlin module at all!

import com.fasterxml.jackson.annotation.JsonInclude.Include;
import com.fasterxml.jackson.databind.ObjectMapper;

import java.util.Objects;

public class TestNonDefaultJava {
    public static void main(String[] args) {
        try {
            var mapper = new ObjectMapper()
                    .setSerializationInclusion(Include.NON_DEFAULT);
            var data = new Data(false);
            var serialized = mapper.writeValueAsString(data);
            var deserialized = mapper.readValue(serialized, Data.class);
            System.out.println(serialized);
            System.out.println(data);
            System.out.println(deserialized);
            System.out.println(data.equals(deserialized));
        } catch (Exception e) {
            e.printStackTrace();
        }

    }

    public static class Data {
        private boolean flag = true;

        public Data() { }

        public Data(boolean flag) {
            this.flag = flag;
        }

        public boolean isFlag() {
            return flag;
        }

        public void setFlag(boolean flag) {
            this.flag = flag;
        }

        public boolean equals(Object o) {
            if (this == o) return true;
            if (o == null || getClass() != o.getClass()) return false;
            Data data = (Data) o;
            return flag == data.flag;
        }

        public int hashCode() {
            return Objects.hash(flag);
        }

        public String toString() {
            return "Data{" +
                    "flag=" + flag +
                    '}';
        }
    }
}
{}
Data{flag=false}
Data{flag=true}
false

Quantum64 avatar Jul 20 '21 21:07 Quantum64

Unfortunately definition of NON_DEFAULT is complicated, and specifically is different between global default and per-class annotation-based variant. I am thinking that this is the same issue as:

https://github.com/FasterXML/jackson-databind/issues/1757

Ultimately, if I remember correctly, there were a few challenges here:

  1. Definition of default for a type itself (like, 0 for int, say) and default for a property within POJO -- how to resolve
  2. Constructing "default" instance for POJOs (and POKOs etc :) ) in case of root value handling
  3. Users' expectations of round-tripping not necessarily being possible to implement reliably (NON_DEFAULT affects serialization, but potentially deserialization has as much effect)

In some ways I am almost tempted to remove NON_DEFAULT as an option from Jackson 3, given these issues, and trying to figure out some other filtering mechanism to handle use cases this is needed.

cowtowncoder avatar Jul 21 '21 00:07 cowtowncoder

We have come to rely on NON_DEFAULT to substantially reduce data storage requirements. One of our use cases involves a User data object with many fields. Realistically, most users only use our service very briefly, so most fields are never changed. The User object is then serialized and committed to a key-value store. For these users who sign in only one time, most fields are not saved. There is a similar concern with the network bandwidth of sending these objects between our services. If NON_DEFAULT were to be removed, a replacement would certainly be needed.

Quantum64 avatar Jul 21 '21 00:07 Quantum64

@Quantum64 yes, I understand usefulness: my only concern really is the correctness and reliability -- compared to simpler criteria like "no nulls" or "no nulls or empty Objects/Arrays", the current "no default value" is not well defined.

Still, esp. for 2.x, it would be good to figure out how to improve things.

cowtowncoder avatar Jul 21 '21 01:07 cowtowncoder

@cowtowncoder Is NON_DEFAULT even present as a serialization feature in databind 3.x?

dinomite avatar Jul 21 '21 14:07 dinomite

I think it is? Some options for setting defaults have been removed (methods in ObjectMapper), but I don't think I have removed that as setting (nor have concrete plans to do so).

cowtowncoder avatar Jul 21 '21 16:07 cowtowncoder

Indeed it is—I had trouble figuring out how to turn it on:

val mapper = jsonMapper {
    addModule(kotlinModule())
    changeDefaultPropertyInclusion { it.withValueInclusion(JsonInclude.Include.NON_DEFAULT) }
}

dinomite avatar Jul 21 '21 17:07 dinomite

This is also a problem for me, when dealing with entities that cannot be changed for retrocompatibility, it helps to avoid serializing trash into the database, NON_EMPTY happens to work for me as a workaround, but it is only because of my lucky case.

cezar-tech avatar Dec 09 '22 14:12 cezar-tech

I will share it here as well. In summary, it is not possible to support this feature in jackson-module-kotlin as they are not information that can be read by reflection. https://github.com/ProjectMapK/jackson-module-kogera/issues/84#issuecomment-1474147220

k163377 avatar Apr 23 '23 04:04 k163377