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

Support specifying the proto integer types for integer collections

Open ShreckYe opened this issue 1 year ago • 2 comments

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

The Serialization Guide teaches a way to specify proto integer types for integer types, but I don't see a way to specify one for a type of collection of integers.

Describe the solution you'd like

Solution 1: add AnnotationTarget.TYPE to the @Target of @ProtoType

@Serializable
class IntsWrapper(val value : Array<@ProtoType(ProtoIntegerType.SIGNED) Int>)
ProtoBuf.encodeToHexString(arrayOf<@ProtoType(ProtoIntegerType.SIGNED) Int>(1, -1))

In this way we can reuse the @ProtoType annotation class, but we won't be able to specify for primitive arrays (ShortArray, IntArray, and LongArray).

Solution 2: reuse the @ProtoType annotation on collections

@Serializable
class IntsWrapper(@ProtoType(ProtoIntegerType.SIGNED) val value: IntArray)

In this way we can also reuse the @ProtoType annotation class. I don't think there are ambiguities, however for the user the semantics of @ProtoType might not look consistent.

Solution 3: add a new annotation analogous to @ProtoType like @ElementProtoType

@Serializable
class IntsWrapper(@ElementProtoType(ProtoIntegerType.SIGNED) val value: IntArray)

IMO this is the best one out of the 3.

ShreckYe avatar Mar 06 '24 02:03 ShreckYe

AFAIR SerialInfo annotations on types Array<@ProtoType(ProtoIntegerType.SIGNED) Int> are not stored anywhere, as there isn't an API in SerialDescriptor to retrieve them. So 2 or 3 is the only viable options. I don't mind reusing @ProtoType, it semantics seem to be still reasonable. However, it is not clear what to do with nested arrays (e.g. List<List<Int>> — if protobuf even supports them) and behavior with value classes should be thoroughly tested.

sandwwraith avatar Mar 11 '24 18:03 sandwwraith

I don't mind reusing @ProtoType, it semantics seem to be still reasonable.

OK.

However, it is not clear what to do with nested arrays (e.g. List<List<Int>> — if protobuf even supports them)

Protobuf doesn't support this natively I think. See this SO question. However, this library seems to support by implicitly providing such wrappers (in the not packed way). See the code and the output:

@Serializable
class Wrapper<T>(val value: T)

@Serializable
class PackedWrapper<T>(@ProtoPacked val value: T)

// without wrappers
println(ProtoBuf.encodeToHexString(Wrapper(1)))
println(ProtoBuf.encodeToHexString(Wrapper(listOf(1))))
println(ProtoBuf.encodeToHexString(Wrapper(listOf(listOf(1)))))
println(ProtoBuf.encodeToHexString(Wrapper(listOf(listOf(listOf(1))))))
println()

// with wrappers, same output as above
println(ProtoBuf.encodeToHexString(Wrapper(listOf(Wrapper(listOf(Wrapper(listOf(1))))))))
println()

// with packed wrappers
println(ProtoBuf.encodeToHexString(PackedWrapper(listOf(1))))
println(ProtoBuf.encodeToHexString(PackedWrapper(listOf(PackedWrapper(listOf(1))))))
println(ProtoBuf.encodeToHexString(PackedWrapper(listOf(PackedWrapper(listOf(PackedWrapper(listOf(1))))))))
println()

// strange inconsistent result, maybe a bug?
println(ProtoBuf.encodeToHexString(PackedWrapper(listOf(listOf(1, 2), listOf(3, 4)))))
println()

// this one seems fine with 3 layers of `List`
println(ProtoBuf.encodeToHexString(PackedWrapper(listOf(listOf(listOf(1))))))
0801
0801
0a020801
0a040a020801

0a040a020801

0a0101
0a030a0101
0a050a030a0101

0a0201020a020304

0a040a020801

and behavior with value classes should be thoroughly tested.

I think there are several scenarios here. For example for such a value class:

@Serializable
@JvmInline
value class ValueClassWrapper<T>(val value: T)

There 2 kinds of data definitions:

@Serializable
class Data1(@ProtoType(ProtoIntegerType.SIGNED) val list: ValueClassWrapper<List<Int>>)

@Serializable
class Data2(@ProtoType(ProtoIntegerType.SIGNED) val list: List<ValueClassWrapper<Int>>)

Do you mean that both kinds should be supported?

It's also possible that both are combined:

@Serializable
class Data12(@ProtoType(ProtoIntegerType.SIGNED) val list: ValueClassWrapper<List<ValueClassWrapper<Int>>>)

Also there is a third kind which is not supported even when given a single Int yet:

@Serializable
@JvmInline
value class ValueClassIntWrapper(@ProtoType(ProtoIntegerType.SIGNED) val value: Int) // this does not work

This may be a separate issue then.

ShreckYe avatar Mar 12 '24 13:03 ShreckYe