[BUGFIX] Correctly deserialize ISO 8601 string with timezone offset

[sgrossberndt]

When unserializing a ISO 8601 string with timezone offset like 2019-01-31T10:37:20.631+01:00 using the DateTimeConverter it currently gets converted to UTC 2019-01-31T09:37:20.631Z. In order to re-instatiate the DateTime object as it was passed to Gson the offset has to be parsed.

[sgrossberndt]

We are using this in production in a project and I'd like to fix it there. Could you please release 1.7.1 after merging this pull request? Thanks in advance!

[gkopff]

@sgrossberndt I should have it released in the next 18 hours.

[gkopff]

@sgrossberndt And thanks for the fix.

[gkopff]

@sgrossberndt I had hoped to release this for you, but I get unit test failures running the build after the change.

Failed tests:   testDeserializeWithMilliseconds(com.fatboyindustrial.gsonjodatime.DateTimeConverterTest): (..)
  testDeserializeWithoutMilliseconds(com.fatboyindustrial.gsonjodatime.DateTimeConverterTest): (..)
  testRoundtrip(com.fatboyindustrial.gsonjodatime.DateTimeConverterTest): (..)
  testRegisterAll(com.fatboyindustrial.gsonjodatime.ConvertersTest): (..)

Tests run: 35, Failures: 4, Errors: 0, Skipped: 0

The non-obvious thing about the failure, is that (by the DateTime.toString() output) the expected and actual results are the same:

Expected :<2019-02-04T09:17:03.074+10:00>
Actual   :<2019-02-04T09:17:03.074+10:00>

...so I take it there is some other aspect of the DateTime equality check that is failing.

I don't have time to dig into this at the moment unfortunately. Do you get the test failures? (The tests pass if I back out your change).

[gkopff]

@sgrossberndt It looks like they end up with different Chronologys.

    final Gson gson = Converters.registerDateTime(new GsonBuilder()).create();
    final DateTime dt = new DateTime();

    System.out.println(dt.getChronology());
    System.out.println(gson.fromJson(gson.toJson(dt), DateTime.class).getChronology());

yields:

ISOChronology[Australia/Brisbane]
ISOChronology[+10:00]

and if the Chronologies are different, the DateTime instances are not equal.

Would you be able to see if you can find out what the right thing to do here is. These days I am more familiar with the java.time way of handling time zones and time zone offsets, which I think is subtly different to how Jodatime does it.

[gkopff]

@sgrossberndt any luck?

[sgrossberndt]

You're welcome. Thanks for the merge.

ISOChronology[Australia/Brisbane] has the same value for the timezone as ISOChronology[+10:00], its the name which is lost: http://joda-time.sourceforge.net/timezones.html

In the serialized string only the timezone value is saved (not the name) - so the behaviour of losing the timezone name is to be expected. This was the case before too - its just visible now as before the reconstructed DateTime always was UTC.

Mapping back to the named timezone is not trivial as in most cases there are several named timezones with the same value.

One option would be to serialize the timezone name too, but that is potentially breaking.

Other (de)serializers have the same problem:
https://stackoverflow.com/questions/20222376/default-timezone-for-datetime-deserialization-with-jackson-joda-time-module
FasterXML/jackson-datatype-joda#43

As Java 8 DateTime do not seem to suffer from that I wonder if it is worth the hassle to fix this.

Of course with this the roundtrip is "broken", one option would be to use a UTC DateTime in that test.

[gkopff]

@sgrossberndt: 1.7.1 has been release to Maven Central.

[sgrossberndt]

Thank you very much!