kotlinx.serialization icon indicating copy to clipboard operation
kotlinx.serialization copied to clipboard

[Protobuf] Respect default values from official documentation

Open glureau opened this issue 1 year ago • 0 comments

(I did some research but not sure if I missed some documentation or discussions somewhere, sorry if it's a duplicate.)

What is your use-case and why do you need this feature?

My use-case is to generate correct protobuf3 messages so that other services (that may use other tools than kotlinx) are able to read properly.

A successful test as introduction:

    enum class Foo { A, B }

    @Serializable
    data class Bar(val foo: Foo = Foo.B)

    @Test
    fun defaultTest() { // ✅ 
        val protobuf = ProtoBuf {
            encodeDefaults = false
        }
        val bar = Bar(Foo.B)
        val serialized = protobuf.encodeToByteArray(bar)
        assertEquals(0, serialized.size)
        val deserialized = protobuf.decodeFromByteArray<Bar>(serialized)
        assertEquals(bar, deserialized)
    }

This is not matching the official documentation as far as I understand: https://protobuf.dev/programming-guides/proto3/#default , especially those 2 quotes

For enums, the default value is the first defined enum value, which must be 0.

Also note that if a scalar message field is set to its default, the value will not be serialized on the wire.

It looks like we are mixing the Kotlin default values and what Protobuf considers the default values. If this example is able to produce an empty ByteArray, decoding with the same protobuf from Java will lead to a Bar(Foo.A) instead of Bar(Foo.B).

This error is not limited to enums but all scalar types as mentioned in protobuf documentation (strings, bytes, bools, numeric types and enums).

Describe the solution you'd like

I'd like to have a serialization that respects the proto 3 format:

  • Default value in Kotlin code shouldn't be used when serializing/deserializing. Ktx should use the protobuf default values instead (ex: if Boolean is not set, then a false will be used, no matter the data class constructor signature).
  • For message fiels I presume null for nullable fields is the best things to do.

I consider the current encodeDefaults = false to be the best solution, given official doc "the value will not be serialized on the wire", and the fact that encoding default make it unusable with nullable fields. I presume it's ok to keep this flag for retro-compatibility if needed, but having a proper proto3 support is higher in my priority list.

glureau avatar Oct 10 '24 09:10 glureau