Add converter for OffsetDateTime and OffsetTime to JSR310 Converters [DATACMNS-698]

[spring-projects-issues]

Endre Czirbesz opened DATACMNS-698 and commented

The JSR310Converters should get converters added for OffsetDateTime and OffsetTime instances

---

Affects: 1.10 GA (Fowler)

Issue Links:

• DATAJPA-1242 AuditAware not converting to ZonedDateTime anymore

Referenced from: pull request #124

[spring-projects-issues]

Endre Czirbesz commented

Created a pull request: #124

[spring-projects-issues]

Endre Czirbesz commented

Added OffsetDateTime and OffsetTime Support to Jsr310Converters

[spring-projects-issues]

Oliver Drotbohm commented

I don't think this is gonna work. The test cases you provided use the system time zone so that of course they can be converted into a Date and back. However, if you construct an OffsetDateTime with a custom ZoneOffset that custom offset would get lost during the back and forth conversion, wouldn't it?

Just try that and see it fail:

ZoneOffset offset = ZoneOffset.ofHours(10);
OffsetTime now = OffsetTime.of(LocalTime.now(), offset);

Date intermediate = CONVERSION_SERVICE.convert(now, Date.class);
OffsetTime result = CONVERSION_SERVICE.convert(intermediate, OffsetTime.class);

assertThat(result.getOffset(), is(offset));

Generally speaking, anything dealing with time zones in JSR-310 cannot be generally converted into a legacy Date and back

[spring-projects-issues]

Endre Czirbesz commented

Yes, that is correct. But, as with most conversions in life, we should accept that this conversion is not lossless.
When I convert a time stamp, the only thing is important and has to be retained is the exact instant the time stamp represent.

In my opinion, the bigger problem when a class does not contain time zone information, for example the same LocalDateTime instance has different meaning for an American user and a central European user.

[spring-projects-issues]

Oliver Drotbohm commented

Exactly, and if the user uses that type she is accepting that difference. By no chance we can create the impression we actually keep all information of a type but then basically don't. Persisting those types would lose information by definition. You basically take away a property of OffsetDateTime which is responsible for the sole existence of the type in the first place. If you're fine with losing time-zone information, you can go ahead and just normalize the offsets and persist a LocalDateTime.

I'll resolve this as won't fix as this is basically asking for code that's not behaving as it should be

[spring-projects-issues]

Jostein Gogstad commented

Just throwing in my 5 cents here. I totally agree with @olivergierke that Spring data shouldn't support conversions between OffsetDateTime/ZonedDateTime and Date. It is however a fact that all modern APIs that handles dates always accept a "full" ISO8601 representation, i.e. "2017-05-03T09:10:10Z", and this is currently hard to handle with spring data.

I always find myself keeping these dates as OffsetDateTimes in code and as String in the database. Since "equals", "after" and "before" corresponds to "equals", "greater than" and "less than" as long as the string is in ISO8601, it doesn't hurt querying capabilities. I don't believe converting between String and OffsetDateTime is any weirder than converting between Long and OffsetDateTime. I would love to see the following in spring data:

1. Provide conversions to and from String in spring-data-commons when no other options are available
2. Implement vendor specific solutions when that's possible. The mongo driver is for instance fully capable of storing an ISODate from a OffsetDateTime or ZonedDateTime