Add kotlin.time.Instant serializers #2945

dkhalanskyjb · 2025-03-04T16:03:05Z

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

sandwwraith · 2025-03-04T16:56:05Z

For InstantSerializer to be picked by the compiler automatically, it should be added to this list: https://github.com/JetBrains/kotlin/blob/ab7d84b1ff9fd99c4035fc1c65f9ae7d50a10aa4/plugins/kotlinx-serialization/kotlinx-serialization.common/src/org/jetbrains/kotlinx/serialization/compiler/resolve/NamingConventions.kt#L215

sandwwraith · 2025-03-04T16:58:37Z

For now, you can use serializer(typeOf<Instant>()) to force runtime lookup instead of the intrinsic one. Do not forget to add smth like // serializer<Instant>() TODO: uncomment in 2.1.20

dkhalanskyjb · 2025-03-05T07:56:59Z

Something like this? JetBrains/kotlin#5412

sandwwraith · 2025-03-06T11:21:44Z

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

dkhalanskyjb · 2025-03-06T12:10:34Z

Sure thing! Are there any other changes I should make in addition to that? For example, I copied the test from kotlinx-datetime directly, without trying to adjust its structure to that of surrounding tests, and I'm not sure that I've even put it in the right place.

…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 <dmitry.khalanskiy@jetbrains.com>

core/commonMain/src/kotlinx/serialization/builtins/InstantComponentSerializer.kt

sandwwraith · 2025-03-06T14:05:51Z

core/commonMain/src/kotlinx/serialization/builtins/BuiltinSerializers.kt

@@ -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).


Suggested change

* 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

sandwwraith · 2025-03-06T14:07:01Z

core/commonMain/src/kotlinx/serialization/builtins/InstantComponentSerializer.kt

+                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


This ticket is said to be fixed in Kotlin 1.4.30

sandwwraith · 2025-03-06T14:17:17Z

formats/json-tests/commonTest/src/kotlinx/serialization/InstantSerializationTest.kt

+            Pair(Instant.fromEpochSeconds(987654321, 0),
+                "\"2001-04-19T04:25:21Z\""),
+        )) {
+            assertEquals(json, Json.encodeToString(serializer, instant))


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\"")

It would check that all Json flavors (string, inputStream, JsonElement) behave identically

sandwwraith · 2025-03-06T15:37:23Z

core/commonTest/src/kotlinx/serialization/BasicTypesSerializationTest.kt

+        val sb = StringBuilder()
+        val out = KeyValueOutput(sb)
+
+        val instant = Instant.parse("2020-12-09T09:16:56.000124Z")


Generally, having Json string tests is enough, but if you think these are necessary as well, you can keep them.

sandwwraith

Don't forget to squash commits on merging

sandwwraith · 2025-04-10T10:36:31Z

core/commonMain/src/kotlinx/serialization/builtins/BuiltinSerializers.kt

+/**
+ * Returns serializer for [Instant].
+ * It is serialized as a string that represents an instant in the format used by [Instant.toString]
+ * and described in ISO-8601-1:2019, 5.4.2.1b).


Suggested change

* and described in ISO-8601-1:2019, 5.4.2.1b).

* and described in ISO-8601-1:2019, 5.4.2.1b.

The parenthesis is not a typo, it's there because the format is called b) in that part of the ISO document.

pdvrieze

I've added some suggestions on augmentation/correctness.

pdvrieze · 2025-04-10T14:08:28Z

core/commonMain/src/kotlinx/serialization/builtins/InstantComponentSerializer.kt

+                    else -> throw SerializationException("Unexpected index: $index")
+                }
+            }
+            if (epochSeconds == null) throw MissingFieldException(


To avoid boxing it may be better to either have a separate primitive value that records whether epochSeconds was seen, or have a "sentinel" value that is not a valid Instant (e.g. Long.MAX or Long.MIN)

pdvrieze · 2025-04-10T14:14:30Z

core/commonMain/src/kotlinx/serialization/builtins/InstantComponentSerializer.kt

+            if (value.nanosecondsOfSecond != 0) {
+                encodeIntElement(descriptor, 1, value.nanosecondsOfSecond)
+            }


This should have a call to shouldEncodeElementDefault as it is up to the format to decide whether or not to encode things (while this doesn't matter for Json it would for a fixed length format).

It depends on whether we really want to include nanoseconds:0 in the output even with encodeDefaults = true.

Current behavior would be equal to having @EncodeDefault(NEVER) annotation on Instant.nanosecondsOfSecond

@sandwwraith The issue is for a fixed length format that requires all attributes to be serialized (for example Android's Parcelable). In such case there must be bits written to represent the default value. It is not possible to write a placeholder as any placeholder value could be a valid value, so either the actual default is written or a marker is written (changing the wire format to add a marker byte).

@sandwwraith I've played around a bit and added binary tests to the jsonTests in #2979 . This is based upon the EfficientBinaryFormat from the documentation (but cross platform), and for now only in the test code (it was not designed to be used as a production format). To make the tests actually pass it is absolutely necessary to return false to shouldEncodeElementDefault (and EncodeDefault(Mode.NEVER) is not valid for this format).

I'm not sure what the problem is. Descriptor says that nanosecondsOfSecond is optional, so there is no contradiction here. I would say that supporting @EncodeDefault(Mode.NEVER) is a reasonable expectation from any format, including binary. Deserializer would be also fine if format would never return 1 from the decodeElementIndex.

To show it breaks I have created another branch applying the binary test to the InstantSerializer:
https://github.com/pdvrieze/kotlinx.serialization/tree/pdvrieze/instant-serializers-with-binary-test

This test fails to decode because it tries to decode the nanosecondsOfSecond component.

Btw. I've also updated the branch underlying #2979. It now triggers more often (and the binary format has been rewritten to support out-of-order serialization). There were two sets of changes needed. The error message test needed to be adjusted to check the correct message. More interesting is that for JsonCustomSerializersTest all custom serializers needed to be fixed to call shouldEncodeElementDefault.

I see what you mean. Do you have any real-world examples of such formats? IIRC, even binary formats with schema (such as Protobuf or Avro) have some way of denoting the id of the next element in the stream (e.g., protoId).

Still, it would mean that such a format is unable to support @EncodeDefault(NEVER), which IMO limits its usability with kotlinx.serialization.

Besides, InstantComponentSerializer is not the default. I imagine that for such a format one can easily write InstantComponentWithMandatoryNanosecondsSerializer and use it instead. I highly doubt that we should sacrifice convenience for the sake of some hypothetical ultra-fast binary format.

There are quite some cases (not supported by kotlinx.serialization). I have looked at some examples:

Java serialization (ObjectOutputStream) - when writing primitives no class is written, only the value

Android Parcelable - again just writes the values, not types where not required

FlatBuffers - It is aimed at access without parsing. The "Structs" type just groups the members.

It is also common in many binary file formats (e.g. PNG, ZIP) or binary network protocols (IP/TCP/TLS/HTTP2.0/dbus) - but even textual line separated formats (e.g. Advent of code data) would may not be self-describing. The EfficientBinaryFormat was actually taken from the documentation (and it has this property). The key factor is whether the format is self-describing (in other words, can be parsed without access to a schema).

The fix for this case would be where the format can influence behaviour (by default it returns false and constant folding should be able to easily optimize the call away):

if (value.nanosecondsOfSecond != 0 || shouldEncodeElementDefault(descriptor, 1)) {

As to @EncodeDefault(NEVER), I would say this has been implemented incorrectly (and possibly misnamed - AVOID would be better). The implementation would be better if shouldEncodeElementDefault were invoked in all cases, but this being an overload that passes the "never" (or avoid) value. The format could then return true if the storage is invalid when defaults are omitted. This approach is probably preferable over requiring a SerializationStrategy to somehow be able to produce default values on request (and certainly more binary compatible).

We're unlikely to support Java serialization. Parcelable is a good example, but usually people just use @Parcelize annotation.
As I said, since this is a non-default serializer, IMO it is acceptable to sacrifice supporting non-standard formats in favor of better representation in Json (which is a primary aim for this serializer). If there would be problems with such an approach, we can always either fix them (as long as it is experimental) or introduce other serializer (e.g. InstantBinaryComponentSerializer.

Just calling the shouldEncodeElementDefault will return false in the normal json case so the json format will be the same. Most likely the optimizer will make the check disappear as well. Also note that this is what the generated serializers do. On the never annotation, that is is still user controlled.

pdvrieze · 2025-04-10T14:16:46Z

formats/json-tests/commonTest/src/kotlinx/serialization/InstantSerializationTest.kt

+import kotlin.reflect.typeOf
+
+@OptIn(ExperimentalTime::class)
+class InstantSerializationTest: JsonTestBase() {


It may be worthwhile to also test on another format that is not Json (or JsonLike)

pdvrieze · 2025-04-10T14:20:23Z

formats/json-tests/commonTest/src/kotlinx/serialization/InstantSerializationTest.kt

+        for ((instant, json) in listOf(
+            Pair(Instant.fromEpochSeconds(1607505416, 124000),
+                "\"2020-12-09T09:16:56.000124Z\""),
+            Pair(Instant.fromEpochSeconds(-1607505416, -124000),
+                "\"1919-01-23T14:43:03.999876Z\""),
+            Pair(Instant.fromEpochSeconds(987654321, 123456789),
+                "\"2001-04-19T04:25:21.123456789Z\""),
+            Pair(Instant.fromEpochSeconds(987654321, 0),
+                "\"2001-04-19T04:25:21Z\""),


You should include some values with non-utc timezones (especially ones with strange offsets like India). And include the fraction handling with millis, or less than 3 digits in the fraction of seconds.

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

Original version: https://github.com/Kotlin/kotlinx-datetime/blob/72681c2acaf9addf5effdef8ecd0975f6f7d10a7/serialization/common/test/InstantSerializationTest.kt

pdvrieze · 2025-04-30T19:18:03Z

formats/json-tests/commonTest/src/kotlinx/serialization/InstantSerializationTest.kt

+        // by default, `nanosecondsOfSecond` is optional
+        assertJsonFormAndRestored(serializer, Instant.fromEpochSeconds(987654321, 0),
+            "{\"epochSeconds\":987654321}", Json { })


The comment might clarify that the default test json instance has encoding defaults enabled. the nanosecondsOfSecond element is always optional, but the format decides whether to serialize the default value. You might replace Json {} with Json(default) { encodeDefaults = false } to both take in the remainder of the default test configuration and communicate that it set to false here.

dkhalanskyjb mentioned this pull request Mar 5, 2025

Add a serializer for kotlin.time.Instant JetBrains/kotlin#5412

Closed

dkhalanskyjb requested a review from sandwwraith March 6, 2025 12:09

sandwwraith requested changes Mar 6, 2025

View reviewed changes

dkhalanskyjb requested a review from sandwwraith March 7, 2025 11:12

dkhalanskyjb force-pushed the dkhalanskyjb/instant-serializers branch from c28031f to 89a6f95 Compare March 7, 2025 11:14

dkhalanskyjb force-pushed the dkhalanskyjb/instant-serializers branch from 89a6f95 to 7d5981e Compare April 2, 2025 14:59

dkhalanskyjb marked this pull request as ready for review April 2, 2025 14:59

sandwwraith approved these changes Apr 10, 2025

View reviewed changes

Update to Dokka 2.0.0 (#2927)

288f73c

pdvrieze reviewed Apr 10, 2025

View reviewed changes

dkhalanskyjb and others added 7 commits April 15, 2025 11:30

Add kotlin.time.Instant serializers

4a0530d

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

Move the tests for the Instant serializers

dd9909f

Original version: https://github.com/Kotlin/kotlinx-datetime/blob/72681c2acaf9addf5effdef8ecd0975f6f7d10a7/serialization/common/test/InstantSerializationTest.kt

Improve the documentation

eea8b48

Remove an old workaround for the JS compiler bug

9030611

Remove an excessive test

22a5509

Revork the Instant serializers tests

973a6e8

Address the review

7587980

dkhalanskyjb force-pushed the dkhalanskyjb/instant-serializers branch from 7d5981e to 7587980 Compare April 15, 2025 12:12

Check shouldEncodeElementDefault in InstantComponentSerializer

5efb20f

dkhalanskyjb requested a review from sandwwraith April 30, 2025 10:27

pdvrieze reviewed Apr 30, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add kotlin.time.Instant serializers #2945

Add kotlin.time.Instant serializers #2945

dkhalanskyjb commented Mar 4, 2025

sandwwraith commented Mar 4, 2025

sandwwraith commented Mar 4, 2025

dkhalanskyjb commented Mar 5, 2025

sandwwraith commented Mar 6, 2025

dkhalanskyjb commented Mar 6, 2025

sandwwraith Mar 6, 2025

sandwwraith Mar 6, 2025

sandwwraith Mar 6, 2025

sandwwraith Mar 6, 2025

sandwwraith Mar 6, 2025

sandwwraith left a comment

sandwwraith Apr 10, 2025

dkhalanskyjb Apr 10, 2025

pdvrieze left a comment

pdvrieze Apr 10, 2025

pdvrieze Apr 10, 2025

sandwwraith Apr 10, 2025 •

edited

Loading

pdvrieze Apr 10, 2025

pdvrieze Apr 10, 2025

sandwwraith Apr 15, 2025

pdvrieze Apr 15, 2025

sandwwraith Apr 16, 2025

pdvrieze Apr 16, 2025

sandwwraith Apr 28, 2025

pdvrieze Apr 28, 2025 •

edited

Loading

pdvrieze Apr 10, 2025

pdvrieze Apr 10, 2025

pdvrieze Apr 30, 2025

	* 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.

	* and described in ISO-8601-1:2019, 5.4.2.1b).
	* and described in ISO-8601-1:2019, 5.4.2.1b.

Add kotlin.time.Instant serializers #2945

Are you sure you want to change the base?

Add kotlin.time.Instant serializers #2945

Conversation

dkhalanskyjb commented Mar 4, 2025

sandwwraith commented Mar 4, 2025

sandwwraith commented Mar 4, 2025

dkhalanskyjb commented Mar 5, 2025

sandwwraith commented Mar 6, 2025

dkhalanskyjb commented Mar 6, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sandwwraith left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pdvrieze left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sandwwraith Apr 10, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pdvrieze Apr 28, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sandwwraith Apr 10, 2025 •

edited

Loading

pdvrieze Apr 28, 2025 •

edited

Loading