cbor: check inline value classes if marked as @ByteString (fixes #2187) #2466
Conversation
the-eater
force-pushed
the
cbor/fix-inline-bytestring
branch
from
6838fd8 to
dfee01c
Compare
|
|
||
| private fun SerialDescriptor.isInlineByteString(): Boolean { | ||
| // inline item classes should only have 1 item | ||
| return isInline && isByteString(0) |
There was a problem hiding this comment.
Yes, this is a correct way to do it
| @Suppress("UNCHECKED_CAST") | ||
| decoder.nextByteString() as T | ||
| } else { | ||
| decodeByteArrayAsByteString = decodeByteArrayAsByteString || deserializer.descriptor.isInlineByteString() |
There was a problem hiding this comment.
This looks a bit suspicious. If the current descriptor is not a ByteArraySerializer().descriptor and not a value class over @ByteString ByteArray, why we should still try to decode it as ByteArray?
If this is to support cases when value class property is not annotated itself, like this:
@Serializable data class Outer(@ByteString val i: InnerValue)
@Serializable
@JvmInline value class InnerValue(val b: ByteArray)Then I don't see tests for it.
There was a problem hiding this comment.
oh good point, yes that was the intended outcome! I'm not sure if that is wanted, but I can see how it might be unwanted? I guess I could probably expose it as an option in the serializer?
I will add the tests anyhow :)
There was a problem hiding this comment.
I'm not sure if that is wanted
I'm also unsure because I didn't see any requests for that. At the first look, it sounds reasonable, but I do not use byte strings that often
the-eater
force-pushed
the
cbor/fix-inline-bytestring
branch
from
35e57ef to
6292f2c
Compare
2 participants
This fixed inline value classes not being processed as a ByteString, by checking inside
encodeSerializableValueanddecodeSerializableValueto see if the current descriptor is marked as inline and with@ByteStringthis is as far as I understand the most viable way to do it