Skip to content
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

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

Open
dkhalanskyjb opened this issue Feb 29, 2024 · 4 comments · May be fixed by #393
Assignees
Labels
formatters Related to parsing and formatting
Milestone

Comments

@dkhalanskyjb
Copy link
Contributor

dkhalanskyjb commented Feb 29, 2024

  • 23:59:00.000000000 is serialized as 23:59;
  • 23:59:01.000000000 is serialized as 23:59:01;
  • 23:59:01.100000000 is serialized as 23:59:01.100.

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 FasterXML/jackson-modules-java8#76.

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

#333 is a similar issue.

@dkhalanskyjb dkhalanskyjb added the formatters Related to parsing and formatting label Feb 29, 2024
@ArtRoman
Copy link

ArtRoman commented Mar 7, 2024

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
Copy link
Contributor Author

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
Copy link

ArtRoman commented Mar 7, 2024

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 dkhalanskyjb added this to the 0.6.0 milestone Apr 12, 2024
@dkhalanskyjb dkhalanskyjb self-assigned this Apr 12, 2024
@dkhalanskyjb dkhalanskyjb modified the milestones: 0.6.0, 0.7.0 Apr 12, 2024
dkhalanskyjb added a commit that referenced this issue Apr 17, 2024
Now, they will always include seconds and will not add trailing
zeros for prettiness to the fractional part.

Fixes #351
@dkhalanskyjb
Copy link
Contributor Author

After looking more carefully at both FasterXML/jackson-modules-java8#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 (#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.

dkhalanskyjb added a commit that referenced this issue May 13, 2024
Sometimes, `X.Formats.ISO` and `X.parse()`/`X.toString()` behave
subtly differently; currently, it's the case for `LocalDateTime`
and `LocalTime`.

With this change, every entity that supports custom formats obtains
a separate default serializer in addition to the
ISO 8601 serializer, which now properly delegates to the
corresponding `DateTimeFormat`.

Fixes #351
@dkhalanskyjb dkhalanskyjb linked a pull request May 13, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
formatters Related to parsing and formatting
Projects
None yet
2 participants