JadiraOrg / jadira

Jadira Framework
Apache License 2.0
74 stars 44 forks source link

TimestampColumnLocalDateTimeMapper#getDefault() timezone is parsed as offset #43

Closed mrubezhanskyy closed 8 years ago

mrubezhanskyy commented 9 years ago

Exactly here         if (zone == null) {             zone = ZoneOffset.of(java.util.TimeZone.getDefault().getID());         }     } catch (RuntimeException ex) {         zone = null; java.util.TimeZone.getDefault().getID() is a time zone like Europe/Berlin and never can be parsed as offset, therefore it always throws an exception

please refer also #42 here should be timezone instead of offset!

apolci commented 9 years ago

I have the same problem.

All the TiestampColumn*Mapper classes should use ZoneRegion instead of ZoneOffset, or better the abstract ZoneId that support both.

chrisphe commented 9 years ago

Please review

apolci commented 9 years ago

The change fix the problem with the Date mapper, but should be applied also to the DateTime mapper (this issue is on TimestampColumnLocalDateTimeMapper, so I think it should not be closed yet).

I'm not sure if the same fix can be applied also to the Time mapper since I'm not sure of the implications of replacing OffsetDateTime with ZonedDateTime in that class.

I was working on preparing a pull request for this, but I won't be able to finish it until a couple of days.

apolci commented 9 years ago

I've published my work on this issue on a new fork but I'm facing some problems: will need some more time to be sure that I don't introduce any new bug.

Currently a test I've added is failing, but I've yet to understand if it's due to my changes, if it's a pre-existing bug or if it's the expected behavior.

I've only changed the threeten classes.

@chrisphe please reopen this issue since imho it's not fixed yet

chrisphe commented 8 years ago

I believe a subsequent contribution addresses this

Adishwar8 commented 7 years ago

Hi, is it fixed in 6.0.1.GA?

It seems that for TimestampColumnLocalDateTimeMapper.java, the issue is still there.

Thanks and Regards