-
Notifications
You must be signed in to change notification settings - Fork 670
Structured CBOR. Fixes #2975, fixes #2848, fixes #3143 #3036
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Conversation
|
Full disclosure: This PR incorporates code from a draft generated by Junie (albeit an impressive draft that saved a day of work). This is not a dumb copypasta of AI-generated code. Even if it were already feature-complete It would still not yet be marked ready for review because we have yet to review everything internally. I also want to stress that "we" is not a euphemism. There will be at least two of us reviewing and discussing internally, almost certainly with additional input from other humans in the process of readying this PR. |
pdvrieze
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've reviewed the code at a general level. There is a lot of repetition, so I've only commented on the first case, not every one. I guess some of the AI generation is visible in the details.
formats/cbor/commonMain/src/kotlinx/serialization/cbor/CborElement.kt
Outdated
Show resolved
Hide resolved
formats/cbor/commonMain/src/kotlinx/serialization/cbor/CborElement.kt
Outdated
Show resolved
Hide resolved
formats/cbor/commonMain/src/kotlinx/serialization/cbor/CborElement.kt
Outdated
Show resolved
Hide resolved
formats/cbor/commonMain/src/kotlinx/serialization/cbor/internal/CborElementSerializers.kt
Show resolved
Hide resolved
formats/cbor/commonMain/src/kotlinx/serialization/cbor/internal/CborElementSerializers.kt
Outdated
Show resolved
Hide resolved
formats/cbor/commonMain/src/kotlinx/serialization/cbor/CborElement.kt
Outdated
Show resolved
Hide resolved
formats/cbor/commonMain/src/kotlinx/serialization/cbor/internal/CborElementSerializers.kt
Show resolved
Hide resolved
cd963c7 to
c89fd46
Compare
|
Performance seems to be OK (
My hot takes:
|
|
I just noticed something that looks weird to me. See this test case here that is failing and closely compare expected vs actual. the byte string is wrapped twice for the reference. |
e3c207b to
8d49f2c
Compare
whyoleg
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First round of review from my side :)
Mostly reviewed API surface, will take a look on the implementation details later
formats/cbor/commonMain/src/kotlinx/serialization/cbor/CborElement.kt
Outdated
Show resolved
Hide resolved
| * traditional methods like [List.get] or [List.size] to obtain CBOR elements. | ||
| */ | ||
| @Serializable(with = CborListSerializer::class) | ||
| public class CborList( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might make sense to call it CborArray, as spec says, it's array, and Json format also calls it JsonArray because it's called array in spec :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sadly, there is an annotation by the very same name ins the same package. Now moving the cbor elements is easily done, but it will probably not be very intuitive (thinking of autocompletion here).
In any case: Good point but not my call!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, yes, how did I miss that :(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But this is on me anyhow because git blame says "Prünster" for the whole file containing the @CborArray annotation. So my short-sighted naming is to blame.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remains an open discussion point
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice to rename @CborArray into something else, and reuse the name here. And it would be also nice to rename @ByteString annotation too.
While CBOR format is experimental, we are free to break things, but it would be nice to make a transition a bit smoother, and, probably, deprecate @CborAray right now and provide an alternative annotation, and as a next step provide the CborElement API with CborArray reused for the aggregate type.
@sandwwraith WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some migration path would be nice, as I did mess up with naming. Better sooner than later, given the impact of the kotlinx-datetime / kotlin.time migration
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm in favor of renaming the annotation, but I'm currently unable to think of a suitable name right now. Maybe @CborAsArray or @CborObjectAsArray, but these look clumsy
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@EncodeAsCborArray?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
went with @CborObjectAsArray and renamed CborList -> CborArray
| * See [RFC 8949 3.4. Tagging of Items](https://datatracker.ietf.org/doc/html/rfc8949#name-tagging-of-items). | ||
| */ | ||
| @OptIn(ExperimentalUnsignedTypes::class) | ||
| public var tags: ULongArray = tags |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe we should make the implementation in a way, that it will be not mutable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can make it so that there is a val with a custom getter that delegates to the actual field, but the tags should still be simply written to a CborElement as they are collected upon deserialisation, to avoid serialization overhead. Actually, the elements should probably be mutable too internally so the whole container structure that now collects tags and elements for Cbor structures can go away.
In the end this would mean that we have:
- an
internal varmaking it possible to collect tags during deserialisation, but apublic val tagswith a custom getter and no backing field - an
internal val elementsfor array and map so we can also collect elements as we go and get rid of another instantiation, but apublic val elements: List<CborElement>
This should boost performance for deserialisation significantly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Of course, this is related to #3036 (comment), but it would also be nice to compare it to how JSON works with lists. Maybe there is some pattern there that could be used inside the CBOR implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not touching this yet, as it does relate to #3036 (comment). I still stand by my proposal from the comment above, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are two issues with unsigned arrays being a part of public API:
- arrays are mutable (which is not very nice in general, but given that
tagsare used to calculate equals & hashCode, it makes things even worse); - unsigned arrays are experimental and experimental API should be a part of the public API.
And it's definitely should be val, not var.
As of tags mutability (I mean, tags being an array itself), we can introduce a dedicated CborTags collection-type or something, but it's up to discussion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, we have already gone down the road with experimentality with
@KeyTags/ValueTags.
True. Yet, the mutability concern holds still (and there's a similar one for CborByteString, BTW), so we need some other way to represent tags than by using an array.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll have to dig into the internals and see how this is currently done. If there seems to be too much array copying going on, a list is worth reconsidering. Otherwise, make the public-facing property a val with a custom getter on an internally mutable property, I guess?!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
less memory pressure tho
Speaking of memory pressure, the current implementation allocates a fresh ULongArray instance for every CborElement. Assuming that majority of element won't have tags associated with them, it feels pretty excessive.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A bit dirty, but what if we treat null and empty arrays both as "no tags"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so i stayed with arrays for now and added a constant that holds an uLongArrayOf(), even though i seem to have stumbled upon internals that showed that calling it without arguments would just return a constant of an empty array and never actually allocate anything.
formats/cbor/commonMain/src/kotlinx/serialization/cbor/internal/CborElementSerializers.kt
Outdated
Show resolved
Hide resolved
| @Serializable(with = CborPositiveIntSerializer::class) | ||
| public class CborPositiveInt( | ||
| value: ULong, | ||
| vararg tags: ULong |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This vararg feels very unfortunate. For example, currently, it's possible to call CborPositiveInt(1u, 2u, 3u), and it's really hard to tell what's going on here.
UlongArray will probably be better
Overall, it's applied to all other declarations; e.g., CborString("1", 1, 2, 3) doesn't feel better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with the first part, but the second part is simply in line with how the tagging annotations behave and it is convenient for the user (much more so that ulongArrrayOf().
Yet, I am aware that it should be consistent, so making ints work differently from everything else is also not really nice… In the end, don't really have an opinion. Sort it out internally, it's a straight-forward refactor anyways.
What would help in such cases is a language feature that forces parameter name specification at the call site, but that is not going to happen anytime soon, if ever.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just in case, for me additional tags field looks very similar to how annotations are applied currently.
Additionally, with Collection Literals it will look like CborString("hello", [1u, 2u, 3u]) or with Named-only parameters it could even be forced to call it like CborString("hello", tags = [1u, 2u, 3u])
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those two features would help, but in the meantime, we need something else for a solution
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this remains an open discussion point and is ultimately not my call
| * The whole hierarchy is [serializable][Serializable] only by [Cbor] format. | ||
| */ | ||
| @Serializable(with = CborElementSerializer::class) | ||
| public sealed class CborElement( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about the alternative design:
sealed class CborElement {
abstract val tags: ULongArray
abstract fun withTags(tags: ULongArray): CborElement
}
// and all other
class CborString(value: String): CborElement() {
override fun withTags(tags: ULongArray): CborString
}Or, even having CborElement have no tags at all, but have a wrapper element CborTagged?
sealed class CborElement
// and all other
class CborString(value: String): CborElement()
class CborTagged(element: CborElement, tags: ULongArray): CborElementBoth variants may apply some performance penalty, though.
It's just that, reading the spec and current implementation of CborElementSerializers, the current placement of tags seems unfortunate and does not result in straightforward logic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The first one solves the issues nicely and it would work well with the internally mutable tags array s.t. it would avoid instantiation overhead just for modifying tags. BUT, that would be a footgun for the consumer.
I have thought about CborTagged, also because of the asymmetric serialization/deserialization behaviour, which really is not all that nice, but also very niche (why would anyone in their right mind do this??)
When I started with the implementation, I was imagining how the most straight-forward and usable way would look like: Tags are attached to something and have a hierarchy. An array is enough to represent this and makes for a nice API that mirrors the pattern of @ValueTags, @ObjectTags, and @KeyTags. Now you could get rid of that pesky asymmetry, if you disallow those annotations on CborElement. Then I'd assume we'd be back to straightforward logic again?!
Bottom line: there are issues that need sorting out. The inconsistency issue is documented in the readme, so I was not trying to cover anything up, but I wanted unbiased opinions and fresh ideas because I could not get to a satisfactory solution by myself. It worked once already. I just hope Leonid checks the code first, and the comments later ;-)
004dd1d to
1d97928
Compare
|
Any updates on the open discussion points? |
fzhinkin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@JesusMcCloud, sorry for the delay with review. 😿
I didn't thoroughly review the serialization machinery, but focused on the API part for now.
formats/cbor/commonMain/src/kotlinx/serialization/cbor/internal/CborElementSerializers.kt
Outdated
Show resolved
Hide resolved
formats/cbor/commonMain/src/kotlinx/serialization/cbor/internal/CborElementSerializers.kt
Outdated
Show resolved
Hide resolved
|
|
||
| /*need to expose writer to access encodeTag()*/ | ||
| internal fun Encoder.asCborEncoder() = this as? CborEncoder | ||
| ?: throw IllegalStateException( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't forget to cover this behavior with tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added tests
| */ | ||
| @Serializable(with = CborIntSerializer::class) | ||
| @ExperimentalSerializationApi | ||
| public class CborInt( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about CborInteger instead? It's closer to major type descriptions from the spec (an unsigned integer, a negative integer) and it is farther from Kotlin's Int.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine with both. Completely up to you guys
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's make it CborInteger then.
| } | ||
|
|
||
| /** | ||
| * **WARNING! Possible truncation/overflow!** E.g., `-2^64` -> `1` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Silent truncation/overflow is something we should avoid. I'd vote for an exception if a value could not be represented as Long (Int, Short, or Byte - more on that later), and an additional toLongOrNull function, that'll return null on overflow.
It would be nice to align the API with what we already have for JsonElement and instead of providing a member function, provide extension properties.
And it seems reasonable to provide properties returning other integer types too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
toLongOrNull sound good (also for other integer types)!
What would be core functionality (implemented as member functions) and what should be extensions? Or is it really just "look at JsonElement and replicate that" without leaving anything ambiguous?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now there's throwing toLong and to…OrNull for all signed number types.
I think we also want helpers for unsigned number types. Strictly speaking, only positive CBOR integers should succeed with the unsigned type helpers, so this is easy to implement. Still, I feel this will go two ways:
- No unsigned helpers: People will misencode and misdecode unsigned numbers as signed numbers and convert them in Kotlin, instead of just using a positive CBOR integer.
- Yes unsigned helpers: I see the potential of people being put off by the fact that -127 will fail to deserialise into a
UByte. This is very CBOR-specific and you need to be aware of how CBOR numbers work to make sense of such failures.
The latter case is correct and I do want to have it to get rid of a footgun. WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, it's more than a foogun, I just reported #3143 after rebasing from dev. The way how unsigned types are handled does not conform to the CBOR spec.
This just re-enforces my conviction that we need dedicated unsigned helper, but also for the whole CBOR format, nor just structured CBOR representation
| @Serializable(with = CborNullSerializer::class) | ||
| @ExperimentalSerializationApi | ||
| public class CborNull(vararg tags: ULong) : CborPrimitive<Unit>(Unit, tags) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Topic for later discussion: do we need a CborUndefined? It's one of the values that could not be parsed with the existing API, yet it is something a valid CBOR document might contain.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great catch! We need a CborUndefined. I totally forgot about that!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
| * traditional methods like [List.get] or [List.size] to obtain CBOR elements. | ||
| */ | ||
| @Serializable(with = CborListSerializer::class) | ||
| public class CborList( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice to rename @CborArray into something else, and reuse the name here. And it would be also nice to rename @ByteString annotation too.
While CBOR format is experimental, we are free to break things, but it would be nice to make a transition a bit smoother, and, probably, deprecate @CborAray right now and provide an alternative annotation, and as a next step provide the CborElement API with CborArray reused for the aggregate type.
@sandwwraith WDYT?
| ")" | ||
| } | ||
|
|
||
| } No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are few things that are currently missing, but that would otherwise improve DX when it comes to using CborElement API:
- casting functions/properties such as
val CborElement.cborFloat: CborFloat,val CborElement.cborList: CborListfun CborElement.asCborFloat(): CborFloat, ...
- builders for aggregates, similar to
JsonObjectBuilderandJsonArrayBuilder
Those could be added later, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that these should be there and all of those are easy wins, given a foundation is there and I can add all of these. I'd just like a complete list (even if it is "only" a link to some JSON API Docs or whatever). BUT: I'd really like this PR to make it into the closest possible release, as we have a very concrete need for this.
That being said, I'd propose the following strategy going forward:
- I'll address all the open issues that are sorted out already (i. e. stuff that just needs to be done).
- All the open discussion points that affect the API should be sorted out ASAP
- We will integrate a snapshot build into our codebase, so we have some real-world feedback to check if the API is missing anything or if some behaviour is too limiting
- Then we'll finalise this PR
- Depending on how large everything is and how pressing the need for builders is, they should be part of this PR or part of a separate one. I just don't want something small being the reason for having to wait for another release cycle…
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Depending on how large everything is and how pressing the need for builders is, they should be part of this PR or part of a separate one
We can definitely add them as a follow up:
Those could be added later, though.
|
|
||
| override fun deserialize(decoder: Decoder): CborFloat { | ||
| decoder.asCborDecoder() | ||
| return CborFloat(decoder.decodeDouble()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tags will be lost here during deserialization (the same is true for CborMap and CborList deserialization routines).
| override fun isElementOptional(index: Int): Boolean = original.isElementOptional(index) | ||
| } | ||
|
|
||
| private fun CborWriter.encodeTags(value: CborElement) { // Encode tags if present |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is unused
| val cborEncoder = encoder.asCborEncoder() | ||
| cborEncoder.encodeTags(value.tags) | ||
| //this we really don't want to expose so we cast here | ||
| (cborEncoder as CborWriter).encodeByteString(value.value) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But the CborEncoder is public and it could be anything else.
| } | ||
|
|
||
| decoder.decodeNull() | ||
| return CborNull() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tags will be lost here too
| fun finalize() = when (this) { | ||
| is List -> CborList(content = elements, tags = tags) | ||
| is Map -> CborMap( | ||
| content = if (elements.isNotEmpty()) IntRange(0, elements.size / 2 - 1).associate { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It worth adding a check that elements.size is even.
| is Map -> CborMap( | ||
| content = if (elements.isNotEmpty()) IntRange(0, elements.size / 2 - 1).associate { | ||
| elements[it * 2] to elements[it * 2 + 1] | ||
| } else mapOf(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would also vote for
buildMap {
for (i in 0 until elements.size / 2 - 1) {
put(elements[i * 2], elements[i * 2 + 1])
}
}as an alternative to if (..) associate {} or mapOf()
| protected val elements = elements | ||
|
|
||
| var tags = tags | ||
| internal set |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CborContainer is internal anyway
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
made it private
This reverts commit 847be40.
- Align byte parser/skip logic with dev (length checks, truncation, indefinite chunks, error messages)\n- Keep structured CBOR helpers warning-free under -Werror\n- Restore tree reader EOF semantics for CborElement decoding
d866512 to
82c9e7d
Compare
Handle kotlin.UByte/UShort/UInt/ULong inline serializers via format-specific encodeInline/decodeInline wrappers so values use CBOR major type 0 and encode/decode correctly.
|
This is really getting huge. To be completely frank: I am losing track with me discovering upstream bugs (some introduced by myself, some having always been there) being fixed here, behavioural changes, API-breaking changes, Discussions touching the internal state machine of the decoder… I'm being completely frank here: without Codex helping me, I'd already be lost (and yes, my commits from January were created with massive help from Codex. It's a game-changer! and I still need to re-check them, because I don't want anyone sorting through slop) What we need is a comprehensive test suite way beyond the manually created (and thus error prone) tests. Since I don't expect any bugs hiding in platform specific code being triggered by any test vectors, this can be JVM-only doing round-trips from/to bytes representations both with and without the detour through structured CBOR classes, so we at leas know that nothing is being messed up wrt. the underlying representation of data. We could start collecting this and lifting it from test vectors inside our codebases, but we need help to get to a significant amount of test data |
|
Just added an example for map-based delegation and it seems to tick all the boxes. I have re-requiested a review from all of you, not because I am convinced it is 100% ready, but because I would really like to settle all the still open discussion points before i pour any more work into this. (I am confident, that I addressed all the open issues that just needed fixing, though). Once the we have settled on the API surface, (esp. wrt. tags and behavioural changes wrt. unsigned numbers). I'll have another go at it and i will also also have others on the team look through before requesting a final review. |
fixes #2975, #2848
This PR introduces structured CBOR encoding and decoding
Encoding from/to
CborElementBytes can be decoded into an instance of
CborElementwith the [Cbor.decodeFromByteArray] function by either manuallyspecifying [CborElement.serializer()] or specifying [CborElement] as generic type parameter.
It is also possible to encode arbitrary serializable structures to a
CborElementthrough [Cbor.encodeToCborElement].Since these operations use the same code paths as regular serialization (but with specialized serializers), the config flags
behave as expected
Newly introduced CBOR-specific structures
[CborPrimitive] represents primitive CBOR elements, such as string, integer, float boolean, and null.
CBOR byte strings are also treated as primitives
Each primitive has a [value][CborPrimitive.value]. Depending on the concrete type of the primitive, it maps
to corresponding Kotlin Types such as
String,Int,Double, etc.Note that Cbor discriminates between positive ("unsigned") and negative ("signed") integers!
CborPrimitiveis itself an umbrella type (a sealed class) for the following concrete primitives:nullBoolean(it is still possible to instantiate it as the
invokeoperator on its companion is overridden accordingly):Longnumbers≥0Longnumbers<0StringDoubleByteArrayand is used to encode them as CBOR byte string (in contrast to a listof individual bytes)
[CborList] represents a CBOR array. It is a Kotlin [List] of
CborElementitems.[CborMap] represents a CBOR map/object. It is a Kotlin [Map] from
CborElementkeys toCborElementvalues.This is typically the result of serializing an arbitrary
Example
Decoding it results in the following CborElement (shown in manually formatted diagnostic notation):
Implementation Details
I tried to stick to the existing CBOR codepaths as closely as possible, and the approach to add tags directly to CborElements is the most pragmatic way of getting expressiveness and convenient use. It does come with a caveat (also taken from the Readme:
Tags are properties of
CborElements, and it is possible to mixing arbitrary serializable values withCborElements thatcontain tags inside a serializable structure. It is also possible to annotate any [CborElement] property
of a generic serializable class with
@ValueTags.This can lead to asymmetric behavior when serializing and deserializing such structures!
The test cases (and comments in the test cases reflect this
Closing Remarks
I also fixed a faulty hex input test vector that I introduced myself, last year, if I pieced it together correctly (see here) and I amended the benchmarks. (see here).
Since the commits from here will be squashed anyways, I did not care for a clean history.