zio / zio-json

Fast, secure JSON library with tight ZIO integration.
https://zio.dev/zio-json
Apache License 2.0
410 stars 146 forks source link

JavaDateTime parsers offset issue #694

Open sirocchj opened 2 years ago

sirocchj commented 2 years ago

Hi,

according to ISO 8601 Time offset from UTC offsets should be expressible, for example, as "+03", "+03:00" and "+0300". However, the last case is not handled correctly, at least on the zio2 branch from which I believe 0.3.0[-RCX] releases are being cut.

I verified it for OffsetDateTime but it is applicable to other *Offset* cases too.

This way all tests pass (notice how the suite was missing an && as well):

diff --git a/zio-json/shared/src/test/scala/zio/json/JavaTimeSpec.scala b/zio-json/shared/src/test/scala/zio/json/JavaTimeSpec.scala
index c0d7ff7..060f2af 100644
--- a/zio-json/shared/src/test/scala/zio/json/JavaTimeSpec.scala
+++ b/zio-json/shared/src/test/scala/zio/json/JavaTimeSpec.scala
@@ -210,8 +210,10 @@ object JavaTimeSpec extends ZIOSpecDefault {
           val n = OffsetDateTime.now()
           val p = OffsetDateTime.of(2020, 1, 1, 12, 36, 12, 0, ZoneOffset.UTC)
           assert(stringify(n).fromJson[OffsetDateTime])(isRight(equalTo(n))) &&
-          assert(stringify("2020-01-01T12:36:12Z").fromJson[OffsetDateTime])(isRight(equalTo(p)))
-          assert(stringify("2020-01-01T12:36:12.Z").fromJson[OffsetDateTime])(isRight(equalTo(p)))
+          assert(stringify("2020-01-01T12:36:12Z").fromJson[OffsetDateTime])(isRight(equalTo(p))) &&
+          assert(stringify("2020-01-01T12:36:12.Z").fromJson[OffsetDateTime])(isRight(equalTo(p))) &&
+          assert(stringify("2020-01-01T12:36:12+00").fromJson[OffsetDateTime])(isRight(equalTo(p))) &&
+          assert(stringify("2020-01-01T12:36:12+00:00").fromJson[OffsetDateTime])(isRight(equalTo(p)))
         },
         test("OffsetTime") {
           val n = OffsetTime.now()

However, adding a positive test for the +0000 case, i.e.:

diff --git a/zio-json/shared/src/test/scala/zio/json/JavaTimeSpec.scala b/zio-json/shared/src/test/scala/zio/json/JavaTimeSpec.scala
index c0d7ff7..5610484 100644
--- a/zio-json/shared/src/test/scala/zio/json/JavaTimeSpec.scala
+++ b/zio-json/shared/src/test/scala/zio/json/JavaTimeSpec.scala
@@ -210,8 +210,11 @@ object JavaTimeSpec extends ZIOSpecDefault {
           val n = OffsetDateTime.now()
           val p = OffsetDateTime.of(2020, 1, 1, 12, 36, 12, 0, ZoneOffset.UTC)
           assert(stringify(n).fromJson[OffsetDateTime])(isRight(equalTo(n))) &&
-          assert(stringify("2020-01-01T12:36:12Z").fromJson[OffsetDateTime])(isRight(equalTo(p)))
-          assert(stringify("2020-01-01T12:36:12.Z").fromJson[OffsetDateTime])(isRight(equalTo(p)))
+          assert(stringify("2020-01-01T12:36:12Z").fromJson[OffsetDateTime])(isRight(equalTo(p))) &&
+          assert(stringify("2020-01-01T12:36:12.Z").fromJson[OffsetDateTime])(isRight(equalTo(p))) &&
+          assert(stringify("2020-01-01T12:36:12+00").fromJson[OffsetDateTime])(isRight(equalTo(p))) &&
+          assert(stringify("2020-01-01T12:36:12+00:00").fromJson[OffsetDateTime])(isRight(equalTo(p))) &&
+          assert(stringify("2020-01-01T12:36:12+0000").fromJson[OffsetDateTime])(isRight(equalTo(p)))
         },
         test("OffsetTime") {
           val n = OffsetTime.now()

fails with:

[info]     - java.time / Decoder / OffsetDateTime
[info]       ✗ Either was Left
[info]       stringify("2020-01-01T12:36:12+0000").fromJson[OffsetDateTime] did not satisfy isRight(equalTo(p))
[info]       stringify("2020-01-01T12:36:12+0000").fromJson[OffsetDateTime] = Left(value = "(2020-01-01T12:36:12+0000 is not a valid ISO-8601 format, illegal offset date time at index 23)")
[info]       at /Users/jsirocchi/code/zio-json/zio-json/shared/src/test/scala/zio/json/JavaTimeSpec.scala:217

The culprit looks to be this check, which seems to be commonly used elsewhere, that expect a : to be present to divide between hours and minutes.

By the way, ISO 8601 does not seem to allow second offsets but the parser can handle that too.. is this by design?

plokhotnyuk commented 2 years ago

@sirocchj Hi, Julien! Thanks for the feedback!

I've proposed a PR with a fix for missing &&: https://github.com/zio/zio-json/pull/695/files

Currently, zio-json supports only those formats that are supported by OffsetDateTime.parse.

Main tests for them are in RoundTripSpec.

As a workaround you can use a custom codec with a different format:

$ scala
Welcome to Scala 3.1.0-RC1 (17.0.2, Java OpenJDK 64-Bit Server VM).
Type in expressions for evaluation. Or try :help.

scala> import java.time._

scala> import java.time.format._

scala> val str = "2020-01-01T22:36:12+0300"
val str: String = 2020-01-01T22:36:12+0300

scala> OffsetDateTime.parse(str, DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ssZ"))
val res0: java.time.OffsetDateTime = 2020-01-01T22:36:12+03:00
sirocchj commented 2 years ago

Thanks for the quick turnaround! Yes I have been doing that workaround, I'm hoping we could do better than Java on this one :) (unless ofc the spec is not correct)