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

ParseInstallation.CurrentInstallation.SaveAsync() not working when device is not in English. #138

Closed
meneses-pt opened this issue Dec 21, 2015 · 6 comments · Fixed by #145

Comments

@meneses-pt
Copy link

When I try to call the method:

ParseInstallation.CurrentInstallation.SaveAsync();

on a phone that is configured with a language other than English

I get an InvalidOperationException: "Cannot change the timeZone property of a _Installation object."

This occurs because Windows Phone is returning a localized string for the local timezone information. Because that string is localized it won't find a match on the TimeZoneNameMap dictionary, thus returning null.

Returning null will make the SDK try to remove the timeZone property from the Installation object.

UPDATE: I was trying to run this in a previous version, so I went and checked if there was a newer version and installed the 1.6.2 version out of NuGet.

This version allows the SDK to set the timeZone to null and does not try to remove the property, mostly because you guys changed the SetIfDifferent method.
But this raises an exception anyway. When sending the data to the Parse servers I now get "BadRequest", and I think the problem is still the same.

Can you please fix this?

@richardjrossiii
Copy link
Contributor

Thanks for the report. I'll look into both parts of this bug (the localized timezone information, and the SetIfDifferent bug), and get back to you.

richardjrossiii added a commit that referenced this issue Jan 6, 2016
… Unity.

Previously, TimeZone string generation that we sent along with
ParseInstallation was dependent upon the `CurrentCulture` of the device
being set to the english locale (as `StandardName` is localized, WHAT?!)

Our new, fancy TimeZone string is now generated agnostically of the
current device locale. There is potential for information about the
specific region of the time zone to be lost here, but most applications
should not require this.

This should fix #138.
richardjrossiii added a commit that referenced this issue Jan 6, 2016
… Unity.

Previously, TimeZone string generation that we sent along with
ParseInstallation was dependent upon the `CurrentCulture` of the device
being set to the english locale (as `StandardName` is localized, WHAT?!)

Our new, fancy TimeZone string is now generated agnostically of the
current device locale. There is potential for information about the
specific region of the time zone to be lost here, but most applications
should not require this.

This should fix #138.
richardjrossiii added a commit that referenced this issue Jan 6, 2016
… Unity.

Previously, TimeZone string generation that we sent along with
ParseInstallation was dependent upon the CurrentCulture of the device
being set to the english locale (as StandardName is localized, WHAT?!)

Our new, fancy TimeZone string is now generated agnostically of the
current device locale. There is potential for information about the
specific region of the time zone to be lost here, but most applications
should not require this.

This should fix #138.
@bnham
Copy link

bnham commented Jan 6, 2016

So I think the general heuristic should be something like:

  1. Look up IANA identifier from StandardName => IANA dict. If there's a match, use it. This works for English locales and doesn't lose daylight savings time info.
  2. If (1) fails, use this map to translate between UTC offsets and named time zones for time zones without an appropriate Etc/* time zone: UTC-09:30 => Pacific/Marquesas, UTC-04:30 => America/Caracas, UTC-03:30 => Canada/Newfoundland, UTC+03:30 => Asia/Tehran, UTC+04:30 => Asia/Kabul, UTC+05:30 => Asia/Kolkata, UTC+05:45 => Asia/Kathmandu, UTC+06:30 => Asia/Rangoon, UTC+08:30 => Asia/Pyongyang, UTC+08:45 => Australia/Eucla, UTC+09:30 => Australia/Adelaide, UTC+10:30 => Australia/Lord_Howe, UTC+12:45 => Pacific/Chatham.
  3. If (1) and (2) fail, then use the hour offset only to construct the appropriate Etc/GMT-x or Etc/GMT+x time zone name. This loses daylight savings time info but is the best we can do. Note that the IANA time zone sign for the Etc area is the opposite of what you'd expect. For instance, Eastern Standard Time is UTC-5, but the corresponding Etc time zone name is actually Etc/GMT+5.
  4. Give up.

For the record here are the list of Etc time zones in our db (this comes straight out of the IANA list):

GMT    GMT+1   GMT-12  GMT+2  GMT-5  GMT+7  Greenwich
GMT0   GMT-10  GMT+12  GMT-3  GMT+5  GMT-8  UCT
GMT-0  GMT+10  GMT-13  GMT+3  GMT-6  GMT+8  Universal
GMT+0  GMT-11  GMT-14  GMT-4  GMT+6  GMT-9  UTC
GMT-1  GMT+11  GMT-2   GMT+4  GMT-7  GMT+9  Zulu

I do not think the server should be expected to handle non-IANA time zones correctly. In other words, changing the server to know what to do with Etc/GMT+05:30 seems like a bad idea because that's not an actual time zone.

FWIW, even third party libraries that try to help you with this need to guess. It looks like the correct thing to do would be to use the Id property on TimeZoneInfo as the key into the map but that doesn't work on PCL devices like WP8, so you just have to use StandardName and guess, as Noda Time does: https://github.com/nodatime/nodatime/blob/master/src/NodaTime/TimeZones/TzdbDateTimeZoneSource.cs#L198-L222.

@inlined
Copy link

inlined commented Jan 7, 2016

Oh man, it's been years since I ran into this. Adding some historic context since I'm no longer at Parse. If I recall correctly, the methods for asking for the locale-neutral (aka en-US) time zone is not available in WinRT so you can't do it reliably in a portable binary. The temporary fix was to change the current thread's locale temporarily to locale-insensitive, but this threw a security error on Windows Mobile. The "correct" solution was to create a (non-winRT) DLL for each windows platform which would expose the locale-insensitive name that mapped to a TZ-database name which captures daylight savings info, but this API call alone didn't seem to meet the minimum bar for complicating the Parse SDK deployment story.

@meneses-pt
Copy link
Author

Well, I have a solution that I have used in a project of mine, using TimeZones because of the exact same problem.

If you feel like trying it, go ahead. Also if you have questions, please feel free to ask.

public static string GetDeviceTimeZone()
{
    var timezone = "Etc/GMT";

    try
    {
        var currentTime = DateTime.Now;
        var currentUtcTime = TimeZoneInfo.ConvertTime(currentTime, TimeZoneInfo.Utc);
        var currentOffset = Offset.FromHours((currentTime - currentUtcTime).Hours);
        var currentInstant = new Instant(currentTime.Ticks);

        var timeZonesList = TzdbDateTimeZoneSource.Default.ZoneLocations;

        for (int i = 0; i < 52; i++)
        {
            var laterTime = currentTime.AddDays(7*i);
            var laterUtcTime = TimeZoneInfo.ConvertTime(laterTime, TimeZoneInfo.Utc);
            var laterOffset = Offset.FromHours((laterTime - laterUtcTime).Hours);
            var laterInstant = new Instant(laterTime.Ticks);
            var tzdb = DateTimeZoneProviders.Tzdb;

            var list =
                from location in timeZonesList
                let zoneId = location.ZoneId  
                let tz = tzdb[zoneId]
                let currentOffsetTemp = tz.GetZoneInterval(currentInstant).WallOffset
                let laterOffsetTemp = tz.GetZoneInterval(laterInstant).WallOffset
                where currentOffsetTemp.Ticks == currentOffset.Ticks
                where laterOffsetTemp.Ticks == laterOffset.Ticks
                select location;

            var tzdbZoneLocations = list as TzdbZoneLocation[] ?? list.ToArray();
            if (!tzdbZoneLocations.Any())
            {
                continue;
            }

            timeZonesList = new List<TzdbZoneLocation>(tzdbZoneLocations);
        }

        var finalList =
            from location in timeZonesList
            let zoneId = location.ZoneId
            select new
            {
                Id = zoneId,
                DisplayValue = zoneId
            };

        var localTimeZones = finalList.ToDictionary(x => x.Id, x => x.DisplayValue);

        return localTimeZones.FirstOrDefault().Value ?? timezone;
    }
    catch (Exception exception)
    {
        Debug.WriteLine(exception);
    }

    return timezone;
}

@inlined
Copy link

inlined commented Jan 8, 2016

Well, I'm unlikely to contribute to the Parse SDK anymore since I now work at Firebase. If you end up with any timezone in the TZ-database you'll get support for local push delivery times. Try to get ones like America/Los_Angeles, which capture DST rules so you don't end up with customers getting notifications at the wrong between a DST transition and the next time they open the app.

@richardjrossiii
Copy link
Contributor

@inlined @meneses-pt I have a solution locally that should cover all of these use-cases, it will be public within a few hours after some additional testing (testing the north Korean timezone is surprisingly difficult).

Thanks for all of your input.

richardjrossiii added a commit that referenced this issue Jan 8, 2016
This was causing installation saves to fail when not on english locale.

For real, this time.

Fixes #138.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants