Add CBOR kotlinx-io support #3149
Conversation
pdvrieze
left a comment
pdvrieze
left a comment
There was a problem hiding this comment.
As more formats support kotlinx-io they all need some sort of text or binary input/output abstraction. It makes sense that this is more or less "internal", but such abstractions are not format specific at all. As such it makes more sense to put them into the core module (with a serialization internal api flag).
At some level it might even make sense to have these interfaces as public, but that may be better for a different discussion.
| @CborFriendModuleApi | ||
| public interface Input { | ||
| public val availableBytes: Int | ||
| /** Returns a -1 if no bytes are available. Otherwise returns a value between 0 and 255 (inclusive). */ | ||
| public fun read(): Int | ||
| public fun read(b: ByteArray, offset: Int, length: Int): Int | ||
| public fun skip(length: Int) | ||
| } | ||
|
|
||
| @CborFriendModuleApi | ||
| public interface Output { | ||
| public fun write(buffer: ByteArray, offset: Int = 0, count: Int = buffer.size) | ||
| public fun write(byteValue: Byte) | ||
| } |
There was a problem hiding this comment.
These types appear to be format independent and relevant to all binary formats. It would be worthwhile to consider whether these interfaces should be put in a common context.
There was a problem hiding this comment.
Should I handle that as part of this PR? My original intent with this API was to be as non-invasive as possible. If so, I'm happy to make the change but I'm not sure where it should live. formats/io?
There was a problem hiding this comment.
I would suggest leaving these base Input/Output interface in the core module, and change the parameter of BinaryFormat and StringFormat to use them.
| public val availableBytes: Int | ||
| /** Returns a -1 if no bytes are available. Otherwise returns a value between 0 and 255 (inclusive). */ |
There was a problem hiding this comment.
It is not clear what this means. What is the meaning of availableBytes==0 and how do you deal with (pending) network reads where the bytes are not available yet (and stream size is not known).
There was a problem hiding this comment.
Sorry, I didn't do the best job on the docs for Input/Output, I'll go back and update them 😅.
This one comment was me attempting to describe the pre-existing behavior of read in ByteArrayInput, since it took me a minute to figure it out. I had originally thought that this should return a Byte (the 0-255 value returned on the happy path), but there's a failure path if there's no more data available where it returns -1 (which forces the Int typing).
Re. network streams, I don't think this is even handled by kotlinx-io yet? It seems like the APIs presented are all synchronous (until Kotlin/kotlinx-io#163 has an implementation, at least).
There was a problem hiding this comment.
In most cases (including Java IO streams) returning 0 means that nothing was returned, but another call may return something again (although most API's that support blocking will block instead of returning 0). A negative (or -1) return value tends to mean end of file/stream.
3 participants
Looking to follow in the footsteps of #2707 with CBOR support for kotlinx-io.
(Sorry for the noise re #3148, accidentally pointed it at the wrong branch and couldn't find the UI to change it.)