Kotlin / kotlinx-datetime

KotlinX multiplatform date/time library
Apache License 2.0
2.37k stars 96 forks source link

Fix the ISO serializers omitting seconds when zero and emitting fractional parts in groups of three signs #351

Open dkhalanskyjb opened 6 months ago

dkhalanskyjb commented 6 months ago

It looks like people value consistency over prettiness when it comes to values produced by serializers, and it's more straightforward to parse a value when all parts of the format are always there. See https://github.com/FasterXML/jackson-modules-java8/issues/76.

Other links where people are confused/irritated even by the behavior of LocalTime.toString in Java:

https://github.com/Kotlin/kotlinx-datetime/issues/333 is a similar issue.

ArtRoman commented 6 months ago

I've found that fractions of second are parsed in different manner than java-time and korlibs.time. For example, 1 millisecond should be equal (in parsing and printing) to 01, 001, 0001, etc. But not equal to 10, 100, 1000, etc.

Here is test for this:

    @Test
    fun testMillisKotlinxDateTime() {
        // Can't set variable size for fraction of second
        //val dateTimeIsoFormat = DateTimeComponents.Format { byUnicodePattern("yyyy-MM-dd'T'HH:mm[:ss[.S]]X") }
        val dateTimeIsoFormat = DateTimeComponents.Formats.ISO_DATE_TIME_OFFSET
        val date = LocalDateTime(2020, 1, 1, 13, 12, 30, 100_000_000).toInstant(TimeZone.UTC)

        assertEquals(date, Instant.parse("2020-01-01T13:12:30.1Z", dateTimeIsoFormat))
        // Fails here and next lines: expected:<2020-01-01T13:12:30.100Z> but was:<2020-01-01T13:12:30.010Z>
        assertEquals(date, Instant.parse("2020-01-01T13:12:30.01Z", dateTimeIsoFormat)) // ❌
        assertEquals(date, Instant.parse("2020-01-01T13:12:30.001Z", dateTimeIsoFormat)) // ❌
        assertEquals(date, Instant.parse("2020-01-01T13:12:30.0001Z", dateTimeIsoFormat)) // ❌
        assertEquals(date, Instant.parse("2020-01-01T13:12:30.00001Z", dateTimeIsoFormat)) // ❌
        assertEquals(date, Instant.parse("2020-01-01T13:12:30.000001Z", dateTimeIsoFormat)) // ❌
        assertEquals(date, Instant.parse("2020-01-01T13:12:30.0000001Z", dateTimeIsoFormat)) // ❌
        assertEquals(date, Instant.parse("2020-01-01T13:12:30.00000001Z", dateTimeIsoFormat)) // ❌
        assertEquals(date, Instant.parse("2020-01-01T13:12:30.000000001Z", dateTimeIsoFormat)) // ❌
        assertEquals(date, Instant.parse("2020-01-01T13:12:30.0000000001Z", dateTimeIsoFormat)) // ❌

        assertFailsWith<IllegalArgumentException> {
            // Out of precision: Text '2020-01-01T13:12:30.00000000001Z' could not be parsed at index 29
            assertNotEquals(date, Instant.parse("2020-01-01T13:12:30.00000000001Z", dateTimeIsoFormat))
        }

        // Fails here and next lines: Values should be different. Actual: 2020-01-01T13:12:30.100Z
        assertNotEquals(date, Instant.parse("2020-01-01T13:12:30.10Z", dateTimeIsoFormat)) // ❌
        assertNotEquals(date, Instant.parse("2020-01-01T13:12:30.100Z", dateTimeIsoFormat)) // ❌
        assertNotEquals(date, Instant.parse("2020-01-01T13:12:30.1000Z", dateTimeIsoFormat)) // ❌
        assertNotEquals(date, Instant.parse("2020-01-01T13:12:30.10000Z", dateTimeIsoFormat)) // ❌
        assertNotEquals(date, Instant.parse("2020-01-01T13:12:30.100000Z", dateTimeIsoFormat)) // ❌
        assertNotEquals(date, Instant.parse("2020-01-01T13:12:30.1000000Z", dateTimeIsoFormat)) // ❌
        assertNotEquals(date, Instant.parse("2020-01-01T13:12:30.10000000Z", dateTimeIsoFormat)) // ❌
        assertNotEquals(date, Instant.parse("2020-01-01T13:12:30.100000000Z", dateTimeIsoFormat)) // ❌
        assertNotEquals(date, Instant.parse("2020-01-01T13:12:30.1000000000Z", dateTimeIsoFormat)) // ❌

        assertFailsWith<IllegalArgumentException> {
            // Out of precision: Text '2020-01-01T13:12:30.10000000000Z' could not be parsed at index 29
            assertNotEquals(date, Instant.parse("2020-01-01T13:12:30.10000000000Z", dateTimeIsoFormat))
        }
    }

For compare, here is the same test for Java Time, which doesn't fail:

    @Test
    fun testMillisJavaTime() {
        val format = DateTimeFormatter.ISO_OFFSET_DATE_TIME
        val date = ZonedDateTime.of(2020, 1, 1, 13, 12, 30, 100_000_000, ZoneOffset.UTC).toInstant()

        // Instant:
        assertEquals(date, format.parse("2020-01-01T13:12:30.1Z", java.time.Instant::from))
        assertEquals(date, format.parse("2020-01-01T13:12:30.10Z", java.time.Instant::from))
        assertEquals(date, format.parse("2020-01-01T13:12:30.100Z", java.time.Instant::from))
        assertEquals(date, format.parse("2020-01-01T13:12:30.1000Z", java.time.Instant::from))
        assertEquals(date, format.parse("2020-01-01T13:12:30.10000Z", java.time.Instant::from))
        assertEquals(date, format.parse("2020-01-01T13:12:30.100000Z", java.time.Instant::from))
        assertEquals(date, format.parse("2020-01-01T13:12:30.1000000Z", java.time.Instant::from))
        assertEquals(date, format.parse("2020-01-01T13:12:30.10000000Z", java.time.Instant::from))
        assertEquals(date, format.parse("2020-01-01T13:12:30.100000000Z", java.time.Instant::from))

        assertFailsWith<DateTimeParseException> {
            // Out of precision: Text '2020-01-01T13:12:30.10000000000Z' could not be parsed at index 29
            format.parse("2020-01-01T13:12:30.1000000000Z", java.time.Instant::from)
        }

        assertNotEquals(date, format.parse("2020-01-01T13:12:30.01Z", java.time.Instant::from))
        assertNotEquals(date, format.parse("2020-01-01T13:12:30.001Z", java.time.Instant::from))
        assertNotEquals(date, format.parse("2020-01-01T13:12:30.0001Z", java.time.Instant::from))
        assertNotEquals(date, format.parse("2020-01-01T13:12:30.00001Z", java.time.Instant::from))
        assertNotEquals(date, format.parse("2020-01-01T13:12:30.000001Z", java.time.Instant::from))
        assertNotEquals(date, format.parse("2020-01-01T13:12:30.0000001Z", java.time.Instant::from))
        assertNotEquals(date, format.parse("2020-01-01T13:12:30.00000001Z", java.time.Instant::from))
        assertNotEquals(date, format.parse("2020-01-01T13:12:30.000000001Z", java.time.Instant::from))

        assertFailsWith<DateTimeParseException> {
            // Out of precision: Text '2020-01-01T13:12:30.0000000001Z' could not be parsed at index 29
            format.parse("2020-01-01T13:12:30.0000000001Z", java.time.Instant::from)
        }
    }

And the same one for korlibs.time, which also doesn't fail:

    @Test
    fun testMillisKorlibs() {
        val format = korlibs.time.DateFormat("yyyy-MM-ddTHH:mm[:ss[.S]]Z").withOptional()
        val date = korlibs.time.DateTime(2020, 1, 1, 13, 12, 30, 1)

        assertEquals(date, format.parseUtc("2020-01-01T13:12:30.1Z"))
        assertEquals(date, format.parseUtc("2020-01-01T13:12:30.01Z"))
        assertEquals(date, format.parseUtc("2020-01-01T13:12:30.001Z"))
        assertEquals(date, format.parseUtc("2020-01-01T13:12:30.0001Z"))
        assertEquals(date, format.parseUtc("2020-01-01T13:12:30.00001Z"))
        assertEquals(date, format.parseUtc("2020-01-01T13:12:30.000001Z"))
        assertEquals(date, format.parseUtc("2020-01-01T13:12:30.0000001Z"))
        assertEquals(date, format.parseUtc("2020-01-01T13:12:30.00000001Z"))
        assertEquals(date, format.parseUtc("2020-01-01T13:12:30.000000001Z"))
        // Out of precision
        assertNotEquals(date, format.parseUtc("2020-01-01T13:12:30.0000000001Z"))

        assertNotEquals(date, format.parseUtc("2020-01-01T13:12:30.10Z"))
        assertNotEquals(date, format.parseUtc("2020-01-01T13:12:30.100Z"))
        assertNotEquals(date, format.parseUtc("2020-01-01T13:12:30.1000Z"))
        assertNotEquals(date, format.parseUtc("2020-01-01T13:12:30.10000Z"))
        assertNotEquals(date, format.parseUtc("2020-01-01T13:12:30.100000Z"))
        assertNotEquals(date, format.parseUtc("2020-01-01T13:12:30.1000000Z"))
        assertNotEquals(date, format.parseUtc("2020-01-01T13:12:30.10000000Z"))
        assertNotEquals(date, format.parseUtc("2020-01-01T13:12:30.100000000Z"))
        // Out of precision
        assertNotEquals(date, format.parseUtc("2020-01-01T13:12:30.1000000000Z"))
    }
dkhalanskyjb commented 6 months ago

For example, 1 millisecond should be equal (in parsing and printing) to 01, 001, 0001, etc. But not equal to 10, 100, 1000, etc.

No, that's incorrect. I don't know good resources that explain this, but here's one: https://www.splashlearn.com/math-vocabulary/decimals/decimal-fraction In short, one millisecond is 1/1000 of a second (written as 0.001), which is equal to 10/10000 (written as 0.0010), 100/100000 (written as 0.00100), and so on.

For compare, here is the same test for Java Time, which doesn't fail:

The reason this test doesn't fail is that you switched the order in which you check .1, .10, .100 and .1, .01, .001. If we adapt your Java.Time test exactly to kotlinx-datetime, word-for-word, without confusing what we check, the test passes for kotlinx-datetime:

@Test
fun testMillisKotlinTime() {
    val format = DateTimeComponents.Formats.ISO_DATE_TIME_OFFSET
    val date = LocalDateTime(2020, 1, 1, 13, 12, 30, 100_000_000).toInstant(UtcOffset.ZERO)

    // Instant:
    assertEquals(date, format.parse("2020-01-01T13:12:30.1Z").toInstantUsingOffset())
    assertEquals(date, format.parse("2020-01-01T13:12:30.10Z").toInstantUsingOffset())
    assertEquals(date, format.parse("2020-01-01T13:12:30.100Z").toInstantUsingOffset())
    assertEquals(date, format.parse("2020-01-01T13:12:30.1000Z").toInstantUsingOffset())
    assertEquals(date, format.parse("2020-01-01T13:12:30.10000Z").toInstantUsingOffset())
    assertEquals(date, format.parse("2020-01-01T13:12:30.100000Z").toInstantUsingOffset())
    assertEquals(date, format.parse("2020-01-01T13:12:30.1000000Z").toInstantUsingOffset())
    assertEquals(date, format.parse("2020-01-01T13:12:30.10000000Z").toInstantUsingOffset())
    assertEquals(date, format.parse("2020-01-01T13:12:30.100000000Z").toInstantUsingOffset())

    assertFailsWith<IllegalArgumentException> {
        // Out of precision: Text '2020-01-01T13:12:30.10000000000Z' could not be parsed at index 29
        format.parse("2020-01-01T13:12:30.1000000000Z").toInstantUsingOffset()
    }

    assertNotEquals(date, format.parse("2020-01-01T13:12:30.01Z").toInstantUsingOffset())
    assertNotEquals(date, format.parse("2020-01-01T13:12:30.001Z").toInstantUsingOffset())
    assertNotEquals(date, format.parse("2020-01-01T13:12:30.0001Z").toInstantUsingOffset())
    assertNotEquals(date, format.parse("2020-01-01T13:12:30.00001Z").toInstantUsingOffset())
    assertNotEquals(date, format.parse("2020-01-01T13:12:30.000001Z").toInstantUsingOffset())
    assertNotEquals(date, format.parse("2020-01-01T13:12:30.0000001Z").toInstantUsingOffset())
    assertNotEquals(date, format.parse("2020-01-01T13:12:30.00000001Z").toInstantUsingOffset())
    assertNotEquals(date, format.parse("2020-01-01T13:12:30.000000001Z").toInstantUsingOffset())

    assertFailsWith<IllegalArgumentException> {
        // Out of precision: Text '2020-01-01T13:12:30.0000000001Z' could not be parsed at index 29
        format.parse("2020-01-01T13:12:30.0000000001Z").toInstantUsingOffset()
    }
}
ArtRoman commented 6 months ago

If we adapt your Java.Time test

Yes, my bad. I'm now in process of switching from korlibs.time to kotlinx.datetime and testMillisKorlibs test was initial. In Java.Time I've mixed up the zeroes' positions, it was unintentional.

As a result, if Java.Time and kotlinx.datetime behaves the same, the real culprit is the korlibs.time 🤦 Thank you!

dkhalanskyjb commented 3 months ago

After looking more carefully at both https://github.com/FasterXML/jackson-modules-java8/issues/76, we concluded that fixing trailing zeros and optional seconds is insufficient for proper interoperability: for example, in Python, just having optional fractional part already means that several separate formats have to be defined: https://stackoverflow.com/questions/30584364/python-strptime-format-with-optional-bits

Looks like the proper solution to the overall problem is providing a straightforward way of defining custom formats (https://github.com/Kotlin/kotlinx-datetime/issues/350). However, the issue of ISO serializers behaving differently from X.Formats.ISO is still important for reasons of consistency.

After an internal discussion, we decided to introduce separate "default" serializers (available as X.serializer(), not by any name initially), delegating to toString/parse and aiming for readability, and, separately, ISO 8601 serializers, aiming for consistency with X.Formats.ISO and interoperability.