govitia / navitia

A client library in Go for the Navitia API (navitia.io) (v2 WIP)
The Unlicense
6 stars 2 forks source link

Datetimes issues (UTC vs local time) #20

Open adelcasse opened 3 years ago

adelcasse commented 3 years ago

As far as I know (we didn't test much and went to the easier solution on our side), Navitia returns datetimes as "YYYYMMDDTHHMMSS".

As implemented here with the time.Parse function, datetimes are parsed as if they were UTC https://github.com/govitia/navitia/blob/master/types/json.go#L23 instead of the correct local time.

In our fork of the library, we've used time.ParseInLocation to get the correct timezone (but it's hardcoded to our use cases only in France with "Europe/Paris" ;))

The correct way might be to use the "context" given by navitia on every endpoint response (https://doc.navitia.io/#other-objects) with the timezone in it, and then use ParseInLocation with this timezone. But this would probably have quite a big impact on everything related to unmarshalling in the library, as it's returned aside of the types themselves.

What do you think ?

thomas-tacquet commented 3 years ago

As navitia returns timezones it's a good idea to use it directly while Unmarshalling messages.

To avoid breaking changes I suggest to duplicate every structure member having a time.Time type.

For exemple :

type Example struct {
    BeginTime time.Time
}

Will be changed to :

type Example struct {
        // BeginTime value will not change
    BeginTime time.Time
        // but VALUE_NAME_Localized will be parse using Location given by navtitia
        BeginTimeLocalized time.Time
}

What do you thnik about this design ?

adelcasse commented 3 years ago

We can do this yes, it's a good idea (even if I think it's a bug to have an incorrect date, but it's probably better like this if other users of the library had workarounds on this).

Concerning design, I think the most important thing will be to define how we'll pass the timezone context to the unmarshaller, as timezone is not directly in the unmarshalled data but in another JSON node of the response (not inside journeys but inside a separate "context" node directly in the root of the response).

(I'm speaking about journeys here because we only use that on our side, but it's probably the same in other requests with datetimes)