This post is intended to be the more technical complement to the post on our main website blog.
Background
We recently discovered the following within our automated unit tests against our date and time system:
- Although we test gmt offset conversion to a timezone string via
EEH_DTT_Helper_Test::test_get_timezone_string_from_gmt_offset
, the primary purpose of that test is just to verify that there are no fatals. Primarily that list of offsets includes what were determined to be invalid offsets within php. However, this list is a hardcoded list of invalid offsets that are not necessarily invalid for every system environment. - There is no where in our unit tests where we test setting the WordPress
gmt_offset
option to an offset, set thetimezone_string
option to an empty string, and then test the EE generated dates and times in the model system against what WP returns for it’s date methods.
As I began going about correcting the above issues, I started discovering other flaws within our code.
Before getting into the issues uncovered below, keep in mind that one of the reasons EE works with timezone strings as opposed to offsets is because that is primarily how PHP is oriented for its DateTime
system. If you want to work with DateTime objects accurately in PHP, then you need to work with timezone strings not offsets.
The fact that WordPress allows users to set a GMT offset for times in their system may be fine for a general blog, but its a huge pain for an application around events because offsets have no location awareness and do not inherently track any DST that might exist for that location. This problem is magnified with software like Event Espresso.
Issue One: DST
EEH_DTT_Helper::get_timezone_string_from_gmt_offset
was not considering that a set timezone_string
in the database could be in DST.
In WordPress, when a call is made to get_option(‘gmt_offset’) there is actually a default hook added by WordPress core on the pre_get_option_gmt_offset which checks get_option('timezone_string')
first and if that’s present, returns the offset for that timezone_string. So even if both gmt_offset and timezone_string are set on a WP install (which is possible, just not via the ui), then the offset on timezone_string
gets returned.
Where this is problematic is that offsets are timezone agnostic, however, the offset for a timezone_string could vary depending on whether that timezone is currently in DST or not. So if this method was called with NO offset supplied, and the current set timezone_string on the site was in DST, then the resulting offset used for the initial search in timezone_name_from_abbr
could result in an INCORRECT match.
I fixed this so that in this scenario if there is a timezone_string set in the db, we just return that instead of deriving it from what gets returned by WP as the offset.
Issue Two: Offset of +0
EEH_DTT_Helper::get_timezone_string_from_gmt_offset
was not properly handling scenarios where an offset of 0 was supplied. For all purposes 0 === UTC
so there is no need to go through all the logic that could return something that is 0
but currently only because the site is in DST. If client code is supplying an offset to get a timezone_string, then we assume not DST information.
So this was fixed so that now if this method explicitly receives an offset, the assumption is explicit that the given value has no DST information.
Issue Three: Historical Timezones
EEH_DTT_Helper:;get_timezone_string_from_gmt_offset
was returning matches against historical timezones.
The PHP methods timezone_name_from_abbr
and timezone_abbreviations_list
contain not only current timezone data, but also historical timezone data. I discovered this when running some new unit tests we have setup in the working branch. For the offset -12
, that would get flipped by the EE usage of these php methods to +12
!!! The reason for this is because although -12
matched a timezone_string using the php methods, the current actual offset in real life for the matched timezone_string
(when using that matched timezone_string
) to instantiate a DateTimeZone
object is +12
. So the timezone_string
matched historically had an offset of -12
but in current day no longer has that offset.
To fix this, I added some further checks on matched timezone strings to make sure that the current offset for that matched timezone_string equals the incoming offset. If they don’t match then that timezone_string
match is rejected.
Doing this in turn revealed a number of offsets that are settable via the WordPress UI that have no equivalent currenttimezone_string
matches in PHP! To complicate things, that list of invalid offsets is dynamic and depends on whether the server the site is on has up to date timezone offset maps which in turn is influenced by the server OS and/or PHP version installed. The fixes implemented account for this.
Issue Four: Inability to do certain tests.
I added some comments to the Model_Data_Translator_Test::test_prepare_conditions_query_params_for_models__gmt_datetimes
that explains why certain offsets were removed from the list that gets tested. This was done intentionally, because the offsets that get adjusted by EE in the EEH_DTT_Helper::adjust_invalid_gmt_offset
with the implemented fixes, are changed to the closest offset with a corresponding current (historically) timezone_string
. This means that sometimes, the values for “now” saved to the db will NOT match the value for “now” that is generated by the WordPress current_time
function because that function works with offsets directly and does not rely on php’s timezones at all (when the only time information on the WP site is gmt_offset
which is what this test working against). So this means its pretty much impossible to reliably test comparisons for offsets we convert against the offset WP uses because this could vary between server environments.
Practically speaking, the tests that matter are still covering critical functionality.
What this means
If you are using any code that interacts directly (or indirectly) with our EEH_DTT_Helper::get_timezone_string_from_offset
method (or any of the public methods it calls), you need to be aware of how this could change when things are released (as described above).
This also means that for sites using a GMT offset (as opposed to a timezone_string), the resulting values for saved dates and times in the database (when displayed) may not be as expected because the database values were converted using an incorrect offset to begin with.
To fix the above scenario on affected sites, there are a couple options:
- You can use the bundled tool that provides a UI for fixing the offset on all saved EE date and time values in the database.
- You can manually fix things for affected sites via using a variation of the query found here (note this query only affects datetime offsets, there are other values in the database that use EE_Datetime values which are affected that you’ll want to run the query against as well): https://github.com/eventespresso/ee-code-snippet-library/blob/master/mysql-queries/update-offset-on-all-datetimes.sql
The good news is that if you have sites that are not using UTC offsets but are using timezone_strings
then they will not be affected by any of this.