MangoAutomation / ma-core-public

Mango Automation Core public code
Other
79 stars 50 forks source link

User's 'timezone' string shared by different libraries #1283

Open Puckfist opened 6 years ago

Puckfist commented 6 years ago

It is likely the JODA, java.util.TimeZone and moment.js need the user's timezone to be from their set of accepted timezone IDs, which are not all the same. Perhaps part of this is solved by stripping out JODA from the user, maybe TimeZone too? java.time time?

jazdw commented 5 years ago

If a user sets their timezone to EST their charts will not render. This results in a REST query such as /rest/v2/point-values/time-period/DP_92a84525-1c9d-48e0-a9f6-d109b7016b3f?bookend=true&fields=TIMESTAMP&fields=VALUE&fields=RENDERED&fields=ANNOTATION&from=2019-03-22T14:55:14.921Z&limit=5001&timezone=EST&to=2019-03-22T15:25:14.921Z which the server responds to with a HTTP 400 status.

terrypacker commented 5 years ago

This is regarding timezones passed into the REST api:

For starters I improved the response message to actually say it is the timezone in the 400 response that is the cause of the bad request.

For a quick overview the Java Time implementation (which we use in to convert the 'timezone' parameter) supports 3 types of timezone formats:

  1. The simplest type of ID is that from ZoneOffset. This consists of 'Z' and IDs starting with '+' or '-'.
  2. The next type of ID are offset-style IDs with some form of prefix, such as 'GMT+2' or 'UTC+01:00'. The recognised prefixes are 'UTC', 'GMT' and 'UT'.
  3. The third type of ID are region-based IDs. A region-based ID must be of two or more characters, and not start with 'UTC', 'GMT', 'UT' '+' or '-'. (See quote below)

Next up the problem/issue is that EST would be classified as a region based ID but is not supported in the default set of ZoneIds as stated in the java doc:

The default set of data is supplied by the IANA Time Zone Database (TZDB). This has region IDs of the form '{area}/{city}', such as 'Europe/Paris' or 'America/New_York'.

However there is an arbitrary Map of short codes built into the ZoneId class to support short Zone Ids which includes HST,PST,MST,EST but not EDT or MDT etc. that can be used by the ZoneID conversion process.

There is a much more complicated way to expand the zone support but this requires building the rules to match the added zones and I am not advocating. ZoneRulesProvider is the Java class.

As I see it we have 2 options:

  1. Accept that Java has done a 'good enough job' with the region support and limit the acceptable timezones to what it can do by default.
  2. Create a map of Ids to IANA Time Zones or offsets that can be used to extend the timezone support and a Utility class to get ZoneIds etc ensuring that this map is always used in conjunction with the base java support.

FYI the arbitrary map in the ZoneId class is:

static {
        Map<String, String> map = new HashMap<>(64);
        map.put("ACT", "Australia/Darwin");
        map.put("AET", "Australia/Sydney");
        map.put("AGT", "America/Argentina/Buenos_Aires");
        map.put("ART", "Africa/Cairo");
        map.put("AST", "America/Anchorage");
        map.put("BET", "America/Sao_Paulo");
        map.put("BST", "Asia/Dhaka");
        map.put("CAT", "Africa/Harare");
        map.put("CNT", "America/St_Johns");
        map.put("CST", "America/Chicago");
        map.put("CTT", "Asia/Shanghai");
        map.put("EAT", "Africa/Addis_Ababa");
        map.put("ECT", "Europe/Paris");
        map.put("IET", "America/Indiana/Indianapolis");
        map.put("IST", "Asia/Kolkata");
        map.put("JST", "Asia/Tokyo");
        map.put("MIT", "Pacific/Apia");
        map.put("NET", "Asia/Yerevan");
        map.put("NST", "Pacific/Auckland");
        map.put("PLT", "Asia/Karachi");
        map.put("PNT", "America/Phoenix");
        map.put("PRT", "America/Puerto_Rico");
        map.put("PST", "America/Los_Angeles");
        map.put("SST", "Pacific/Guadalcanal");
        map.put("VST", "Asia/Ho_Chi_Minh");
        map.put("EST", "-05:00");
        map.put("MST", "-07:00");
        map.put("HST", "-10:00");
        SHORT_IDS = Collections.unmodifiableMap(map);
    }
terrypacker commented 5 years ago

See TimezoneUtility and the ServerRestV2Controller. This returns a very trimmed down list of zone ids, which isn't ideal as there are a lot of ids that are left out for some reason. We should probably improve that endpoint and at the very least get rid of the TimezoneUtility class.