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

Add timezone to message #32

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jperedadnr
Copy link
Collaborator

No description provided.

@tiainen
Copy link

tiainen commented May 11, 2020

It looks like this is an issue that should be fixed in the serialization part of DataClient instead. It has support to serialize fields of type LocalDate, LocalTime and LocalDateTime, but it doesn't store timezone information. This will cause the time to change when it is read back in on a device that has a different timezone than the device where the message was originally created.

@tiainen
Copy link

tiainen commented May 11, 2020

It appears I made the wrong assumption. LocalDateTime doesn't store or represent a timezone. I think the easier approach here would be to store the creation date of the message as a timezone independent timestamp. Only when we want to display the time again in a human friendly format, we show the times in the timezone of the device.

@jperedadnr
Copy link
Collaborator Author

By "timezone independent timestamp" you mean saving every message in UTC format, so we have the same reference for all the messages from all the users?

Then serializing the LocalDateTime field doesn't work out of the box. Having the extra zoneId as proposed in this PR field helps precisely on that.

@tiainen
Copy link

tiainen commented May 11, 2020

Timezone should only be stored if it is relevant and from what I can see, it seems it is not relevant. You only want to know the point in time when a message was created. When you display that time to the user, you do it in their (aka the device's) timezone.

As an example, to make it more clear: a message is created on May 11th at 5h15 in a device that has the timezone Europe/Brussels (equals UTC+2 because of summer time). This is 3h15 in the UTC timezone, so we store the epoch seconds (or milliseconds if you need it to be more finegrained): 1589166900. You can then use that value to display it in human friendly format again, in any timezone that you want. For instance, in timezone Asia/Tokyo, it would be May 11th at 12h15.

@jperedadnr
Copy link
Collaborator Author

You say that we should remove LocalDateTime and use long timestamp and System.currentMillis()? That will help, but shouldn't we stick to Java Time API?

@tiainen
Copy link

tiainen commented May 11, 2020

Yes, using System.currentTimeMillis() would be the simplest solution for this. You will still use the Java Time API when you format the time into a readable string.

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

Successfully merging this pull request may close these issues.

None yet

2 participants