-
Notifications
You must be signed in to change notification settings - Fork 635
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
Add kotlin.time.Instant serializers #2945
base: dev
Are you sure you want to change the base?
Conversation
Can be merged after moving to Kotlin 2.1.20, which introduces kotlin.time.Instant. kotlinx.datetime.Instant entered the stdlib as kotlin.time.Instant, and so kotlinx.serialization takes over its serializers. See Kotlin/KEEP#387
For |
For now, you can use |
Something like this? JetBrains/kotlin#5412 |
Note that even after merging Kotlin PR, changes in this one are required, because compiler is not updated instantly |
After moving `Instant` from `kotlinx.datetime` to `kotlin.time`, we also need to preserve it having a default `kotlinx.serialization` serializer. See Kotlin/KEEP#387 The corresponding request in `kotlinx.serialization`: Kotlin/kotlinx.serialization#2945
Sure thing! Are there any other changes I should make in addition to that? For example, I copied the test from |
…rializers After moving `Instant` from `kotlinx.datetime` to `kotlin.time`, we also need to preserve it having a default `kotlinx.serialization` serializer. See Kotlin/KEEP#387 The corresponding request in `kotlinx.serialization`: Kotlin/kotlinx.serialization#2945 Additionally, fix a test that was not being run correctly. Merge-request: KT-MR-20493 Merged-by: Dmitry Khalanskiy <[email protected]>
import kotlin.time.Instant | ||
|
||
@ExperimentalTime | ||
public object InstantComponentSerializer : KSerializer<Instant> { |
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.
Documentation is missing
@@ -251,6 +253,19 @@ public fun UShort.Companion.serializer(): KSerializer<UShort> = UShortSerializer | |||
*/ | |||
public fun Duration.Companion.serializer(): KSerializer<Duration> = DurationSerializer | |||
|
|||
/** | |||
* Returns serializer for [Instant]. | |||
* It is serialized as a string that represents an instant in the format described in ISO-8601-1:2019, 5.4.2.1b). |
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 serialized as a string that represents an instant in the format described in ISO-8601-1:2019, 5.4.2.1b). | |
* It is serialized as a string that represents an instant in the format same as the result of [Instant.toString] operation, described in ISO-8601-1:2019, 5.4.2.1b. |
I think hardly anyone remembers what exactly is written in section 5.4.2.1b of a paid document
when (val index = decodeElementIndex(descriptor)) { | ||
0 -> epochSeconds = decodeLongElement(descriptor, 0) | ||
1 -> nanosecondsOfSecond = decodeIntElement(descriptor, 1) | ||
CompositeDecoder.DECODE_DONE -> break@loop // https://youtrack.jetbrains.com/issue/KT-42262 |
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 ticket is said to be fixed in Kotlin 1.4.30
Pair(Instant.fromEpochSeconds(987654321, 0), | ||
"\"2001-04-19T04:25:21Z\""), | ||
)) { | ||
assertEquals(json, Json.encodeToString(serializer, instant)) |
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.
You can use assertJsonFormAndRestored
test helper function as in e.g. here:
kotlinx.serialization/formats/json-tests/commonTest/src/kotlinx/serialization/features/UuidTest.kt
Line 18 in fa797bc
assertJsonFormAndRestored(Uuid.serializer(), uuid, "\"$uuid\"") |
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 check that all Json flavors (string, inputStream, JsonElement) behave identically
val sb = StringBuilder() | ||
val out = KeyValueOutput(sb) | ||
|
||
val instant = Instant.parse("2020-12-09T09:16:56.000124Z") |
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.
Generally, having Json string tests is enough, but if you think these are necessary as well, you can keep them.
c28031f
to
89a6f95
Compare
Can be merged after moving to Kotlin 2.1.20, which introduces
kotlin.time.Instant.
kotlinx.datetime.Instant entered the stdlib as kotlin.time.Instant,
and so kotlinx.serialization takes over its serializers.
See Kotlin/KEEP#387