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

Java 8 LocalDate/Time conversion #1218

Closed
hatsuyuki15 opened this issue Dec 29, 2017 · 5 comments
Closed

Java 8 LocalDate/Time conversion #1218

hatsuyuki15 opened this issue Dec 29, 2017 · 5 comments
Milestone

Comments

@hatsuyuki15
Copy link

Hi, currently the LocalDateConverter is using systemDefault() as the zone id for converting from Date to LocalDate and vice verse.

This behaviour, however, will create a dependency on the OS timezone and thus, contradicts to the concept of LocalDate (date-time object without timezone). It also leads to inconsistency if the code are executed on multiple systems with different timezones, for example, persisting a LocalDate 2018-12-02 will result as 2018-12-01 17:00:00.000Z on UTC+7, and 2018-12-02 00:00:00.000Z on UTC.

I suggest using UTC timezone instead of systemDefault() one for java 8 LocalDate/Time conversion.

@tbroyer
Copy link

tbroyer commented Jul 2, 2018

May I suggest storing them as strings? (or possibly numbers to take less storage space, e.g. Number(20181202) for a 2018-12-02 local date)

@evanchooly
Copy link
Member

Changing the on disk representation would break anyone with existing data. The "nicest" way to change that would be to use a flag on MapperOptions, say, or a different Converter to be a configured. Neither of those options are terribly appealing at the moment.

@evanchooly
Copy link
Member

OK. I've come up with a plan, finally, I'm ok with. Here's what I'm thinking:

  1. Add an option to determine the timezone to MapperOptions: SYSTEM_DEFAULT and UTC
  2. The default value will be SYSTEM_DEFAULT.
  3. SYSTEM_DEFAULT will be deprecated in 1.5.0 and Morphia will log a warning when it is used suggesting a data migration to use UTC instead
  4. In 2.0, the option will be removed altogether and Morphia will only use UTC when storing dates/times.

How does that sound to you? I like that it course corrects an admittedly bad design without immediately breaking any existing databases.

@evanchooly evanchooly added this to the 1.5.0 milestone Mar 11, 2019
@hatsuyuki15
Copy link
Author

That sounds pretty good to me!

@evanchooly
Copy link
Member

If you could look over the commit above and check my sanity, but that seems to be what you need. Dates are easy to mess up so I'd love another set of eyes to double check me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants