Automatically serialize nulls when a nullable property has a non-null default value

[dlew]

The following code, which writes an object to JSON then parses it again, fails at an equality check:

data class Foo(val bar: String? = "oops")

val adapter = Moshi.Builder().build().adapter(Foo::class.java)

val foo = Foo(bar = null)
val json = adapter.toJson(foo)
val foo2 = adapter.fromJson(json)

check(foo == foo2) // Fails

The reason it fails is because toJson(Foo(null)) == "{}", yet fromJson("{}") == Foo("oops"). That happens only because we're mixing a nullable property has a non-null default value.

One solution is to serialize nulls. Theoretically, Moshi could serialize nulls on any property where:

1. It is nullable.
2. It has a non-null default value.
3. The current value is null.

Thus avoiding this lack of symmetry. (In the above example, it would mean that toJson(Foo(null)) == "{ \"bar\": null }")

[swankjesse]

Yup, good call. Need to get this working.

[ZacSweers]

I think our hands are tied because Kotlin metadata won't tell us what the default value is, only that it has one. That default could be null

[SnyersK]

Could there be a potential issue if Moshi would always serialize the current value for nullable properties that have a default value?
If that default value is null, that's a somewhat breaking change of course. But it does guarantee that a serialized class deserializes to its original values again.

Or perhaps it shouldn't be the default behaviour, but a wrapper, like serializeNulls?
That avoids the breaking change, and gives more control to the user.

I assume this loss of symmetry will mostly be an issue when you're using json locally?
with a wrapper, it could be something that you typically only use for local (de)serialization.
Remote communication will be unaffected that way.

I can't think of a short name for that though. alwaysSerializeNullablePropertiesWithDefaultValues() doesn't exactly roll off the tongue. 🙈

[dlew]

This came up locally for me but theoretically someone might write both a server and a client using the same base JSON model.

I'm fine with this being an optional setting that writes all keys with a default value, though at that point, maybe serializeNulls() is good enough? It can lead to writing more nulls than necessary, but there's something gained in the simplicity (vs. having to explain this corner case).

[SnyersK]

Just using serializeNulls() is way simpler indeed.

I'm kind of wondering why serializing nulls isn't the default behaviour actually?
I suppose not serializing nulls is an output size optimisation?

One that can potentially backfire when working with default values apparently. 😕

[Tom-Alphero]

I'd like to chime in and say that some APIs treat POST {} and {"phone": null} differently. In one API I work with, the latter would be a command to clear phone. So I would not expect this to be 'default-on'.

[dlew]

That's a good point. We probably shouldn't change defaults since that could hurt a lot of existing projects.

Maybe we should just close this out and say "use serializeNulls() if necessary" for this case?