Skip to content

Commit

Permalink
Do not cache ActiveSupport::TimeZone#utc_offset
Browse files Browse the repository at this point in the history
This can be an issue when TZInfo::TimeZone#current_period is refreshed
due to timezone period transition, but it's not reflected in
ActiveSupport::TimeZone object.

For example, on Sun, 26 Oct 2014 22:00 UTC, Moscow changed its TZ from
MSK +04:00 to MSK +03:00 (-1 hour). If ActiveSupport::TimeZone['Moscow']
happens to be initialized just before the timezone transition, it will
cache its stale utc_offset even after the timezone transition.

This commit removes cache and fixes this issue.

Signed-off-by: Jeremy Daer <jeremydaer@gmail.com>
  • Loading branch information
conf authored and jeremy committed Apr 26, 2016
1 parent ea628f7 commit 420730b
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 3 deletions.
6 changes: 6 additions & 0 deletions activesupport/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
* Time zones: Ensure that the UTC offset reflects DST changes that occurred
since the app started. Removes UTC offset caching, reducing performance,
but this is still relatively quick and isn't in any hot paths.

*Alexey Shein*

* Make `getlocal` and `getutc` always return instances of `Time` for
`ActiveSupport::TimeWithZone` and `DateTime`. This eliminates a possible
stack level too deep error in `to_time` where `ActiveSupport::TimeWithZone`
Expand Down
4 changes: 1 addition & 3 deletions activesupport/lib/active_support/values/time_zone.rb
Original file line number Diff line number Diff line change
Expand Up @@ -278,16 +278,14 @@ def initialize(name, utc_offset = nil, tzinfo = nil)
@name = name
@utc_offset = utc_offset
@tzinfo = tzinfo || TimeZone.find_tzinfo(name)
@current_period = nil
end

# Returns the offset of this time zone from UTC in seconds.
def utc_offset
if @utc_offset
@utc_offset
else
@current_period ||= tzinfo.current_period if tzinfo
@current_period.utc_offset if @current_period
tzinfo.current_period.utc_offset if tzinfo && tzinfo.current_period
end
end

Expand Down
11 changes: 11 additions & 0 deletions activesupport/test/time_zone_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,17 @@ def test_utc_offset_lazy_loaded_from_tzinfo_when_not_passed_in_to_initialize
assert_equal(-18_000, zone.utc_offset)
end

def test_utc_offset_is_not_cached_when_current_period_gets_stale
tz = ActiveSupport::TimeZone.create('Moscow')
travel_to(Time.utc(2014, 10, 25, 21)) do # 1 hour before TZ change
assert_equal 14400, tz.utc_offset, 'utc_offset should be initialized according to current_period'
end

travel_to(Time.utc(2014, 10, 25, 22)) do # after TZ change
assert_equal 10800, tz.utc_offset, 'utc_offset should not be cached when current_period gets stale'
end
end

def test_seconds_to_utc_offset_with_colon
assert_equal "-06:00", ActiveSupport::TimeZone.seconds_to_utc_offset(-21_600)
assert_equal "+00:00", ActiveSupport::TimeZone.seconds_to_utc_offset(0)
Expand Down

0 comments on commit 420730b

Please sign in to comment.