Permalink
Browse files

Add compatibility for Ruby 2.4 `to_time` changes

In Ruby 2.4 the `to_time` method for both `DateTime` and `Time` will
preserve the timezone of the receiver when converting to an instance
of `Time`. Since Rails 5.0 will support Ruby 2.2, 2.3 and later we
need to introduce a compatibility layer so that apps that upgrade do
not break. New apps will have a config initializer file that defaults
to match the new Ruby 2.4 behavior going forward.

For information about the changes to Ruby see:
https://bugs.ruby-lang.org/issues/12189
https://bugs.ruby-lang.org/issues/12271

Fixes #24617.
  • Loading branch information...
pixeltrix committed Apr 23, 2016
1 parent 1ffa1a8 commit c9c5788a527b70d7f983e2b4b47e3afd863d9f48
View
@@ -1,3 +1,23 @@
* Add `ActiveSupport.to_time_preserves_timezone` config option to control
how `to_time` handles timezones. In Ruby 2.4+ the behavior will change
from converting to the local system timezone to preserving the timezone
of the receiver. This config option defaults to false so that apps made
with earlier versions of Rails are not affected when upgrading, e.g:
>> ENV['TZ'] = 'US/Eastern'
>> "2016-04-23T10:23:12.000Z".to_time
=> "2016-04-23T06:23:12.000-04:00"
>> ActiveSupport.to_time_preserves_timezone = true
>> "2016-04-23T10:23:12.000Z".to_time
=> "2016-04-23T10:23:12.000Z"
Fixes #24617.
*Andrew White*
* `ActiveSupport::TimeZone.country_zones(country_code)` looks up the
country's time zones by its two-letter ISO3166 country code, e.g.
@@ -26,6 +26,7 @@
require "active_support/version"
require "active_support/logger"
require "active_support/lazy_load_hooks"
require "active_support/core_ext/date_and_time/compatibility"
module ActiveSupport
extend ActiveSupport::Autoload
@@ -85,6 +86,14 @@ def self.halt_callback_chains_on_return_false
def self.halt_callback_chains_on_return_false=(value)
Callbacks.halt_and_display_warning_on_return_false = value
end
def self.to_time_preserves_timezone
DateAndTime::Compatibility.preserve_timezone
end
def self.to_time_preserves_timezone=(value)
DateAndTime::Compatibility.preserve_timezone = value
end
end
autoload :I18n, "active_support/i18n"
@@ -0,0 +1,16 @@
module DateAndTime
module Compatibility
# If true, +to_time+ preserves the the timezone offset.
#
# NOTE: With Ruby 2.4+ the default for +to_time+ changed from
# converting to the local system time to preserving the offset
# of the receiver. For backwards compatibility we're overriding
# this behavior but new apps will have an initializer that sets
# this to true because the new behavior is preferred.
mattr_accessor(:preserve_timezone, instance_writer: false) { false }

This comment has been minimized.

Show comment
Hide comment
@kbrock

kbrock Apr 25, 2016

Contributor

using/depending upon mattr_accessor in a few places in active support is causing some havoc for us.
We've had at least 2 gems break from this.

@kbrock

kbrock Apr 25, 2016

Contributor

using/depending upon mattr_accessor in a few places in active support is causing some havoc for us.
We've had at least 2 gems break from this.

This comment has been minimized.

Show comment
Hide comment
@pixeltrix

pixeltrix Apr 25, 2016

Member

@kbrock can you provide more details please?

@pixeltrix

pixeltrix Apr 25, 2016

Member

@kbrock can you provide more details please?

This comment has been minimized.

Show comment
Hide comment
@kbrock

kbrock Apr 25, 2016

Contributor

@pixeltrix very sorry. that was vague on my part.

At this point, there is no guarantee that mattr_accessor is defined.
This means that an innocently targeted require blows up.

==> #24729

@kbrock

kbrock Apr 25, 2016

Contributor

@pixeltrix very sorry. that was vague on my part.

At this point, there is no guarantee that mattr_accessor is defined.
This means that an innocently targeted require blows up.

==> #24729

This comment has been minimized.

Show comment
Hide comment
@pixeltrix

pixeltrix Apr 25, 2016

Member

@kbrock thanks for catching this - PR merged.

@pixeltrix

pixeltrix Apr 25, 2016

Member

@kbrock thanks for catching this - PR merged.

def to_time
preserve_timezone ? getlocal(utc_offset) : getlocal
end
end
end
@@ -1,4 +1,5 @@
require 'active_support/core_ext/date_time/acts_like'
require 'active_support/core_ext/date_time/blank'
require 'active_support/core_ext/date_time/calculations'
require 'active_support/core_ext/date_time/compatibility'
require 'active_support/core_ext/date_time/conversions'
@@ -0,0 +1,16 @@
require 'active_support/core_ext/date_and_time/compatibility'
class DateTime
prepend DateAndTime::Compatibility
# Returns a <tt>Time.local()</tt> instance of the simultaneous time in your
# system's <tt>ENV['TZ']</tt> zone.
def getlocal(utc_offset = nil)

This comment has been minimized.

Show comment
Hide comment
@yui-knk

yui-knk Apr 23, 2016

Contributor

Is it a (new) Public API ?

@yui-knk

yui-knk Apr 23, 2016

Contributor

Is it a (new) Public API ?

This comment has been minimized.

Show comment
Hide comment
@jeremy

jeremy Apr 26, 2016

Member

@pixeltrix Do we need/intend to expose new Time-like API here? May be a recipe for getting burned if/when Ruby adds DateTime#getlocal. We could use a private internal method to do "coerce to local Time."

@jeremy

jeremy Apr 26, 2016

Member

@pixeltrix Do we need/intend to expose new Time-like API here? May be a recipe for getting burned if/when Ruby adds DateTime#getlocal. We could use a private internal method to do "coerce to local Time."

This comment has been minimized.

Show comment
Hide comment
@pixeltrix

pixeltrix Apr 26, 2016

Member

Could do - but we added getutc in 47ff3aa so my feeling was we'd already crossed that line. Do you want me to change it?

@pixeltrix

pixeltrix Apr 26, 2016

Member

Could do - but we added getutc in 47ff3aa so my feeling was we'd already crossed that line. Do you want me to change it?

This comment has been minimized.

Show comment
Hide comment
@jeremy

jeremy Apr 26, 2016

Member

We've def crossed that line, but worth judging whether to step further. If we only need the method internally, let's not expose it publicly. If it bears public use, let's expose.

@jeremy

jeremy Apr 26, 2016

Member

We've def crossed that line, but worth judging whether to step further. If we only need the method internally, let's not expose it publicly. If it bears public use, let's expose.

utc = getutc
Time.utc(
utc.year, utc.month, utc.day,
utc.hour, utc.min, utc.sec + utc.sec_fraction
).getlocal(utc_offset)
end
end
@@ -32,7 +32,7 @@ def to_time(form = :local)
parts.fetch(:offset, form == :utc ? 0 : nil)
)
form == :utc ? time.utc : time.getlocal
form == :utc ? time.utc : time.to_time
end
# Converts a string to a Date value.
@@ -1,4 +1,5 @@
require 'active_support/core_ext/time/acts_like'
require 'active_support/core_ext/time/calculations'
require 'active_support/core_ext/time/compatibility'
require 'active_support/core_ext/time/conversions'
require 'active_support/core_ext/time/zones'
@@ -0,0 +1,5 @@
require 'active_support/core_ext/date_and_time/compatibility'
class Time
prepend DateAndTime::Compatibility
end
@@ -1,6 +1,7 @@
require 'active_support/duration'
require 'active_support/values/time_zone'
require 'active_support/core_ext/object/acts_like'
require 'active_support/core_ext/date_and_time/compatibility'
module ActiveSupport
# A Time-like class that can represent a time in any time zone. Necessary
@@ -44,7 +45,7 @@ def self.name
PRECISIONS = Hash.new { |h, n| h[n] = "%FT%T.%#{n}N".freeze }
PRECISIONS[0] = '%FT%T'.freeze
include Comparable
include Comparable, DateAndTime::Compatibility
attr_reader :time_zone
def initialize(utc_time, time_zone, local_time = nil, period = nil)
@@ -401,11 +402,6 @@ def to_r
utc.to_r
end
# Returns an instance of Time in the system timezone.
def to_time
utc.to_time
end
# Returns an instance of DateTime with the timezone's UTC offset
#
# Time.zone.now.to_datetime # => Tue, 18 Aug 2015 02:32:20 +0000
@@ -0,0 +1,110 @@
require 'abstract_unit'
require 'active_support/time'
require 'time_zone_test_helpers'
class DateAndTimeCompatibilityTest < ActiveSupport::TestCase
include TimeZoneTestHelpers
def setup
@utc_time = Time.utc(2016, 4, 23, 14, 11, 12)
@utc_offset = 3600
@system_offset = -14400
@zone = ActiveSupport::TimeZone['London']
end
def test_time_to_time_preserves_timezone
with_preserve_timezone(true) do
with_env_tz 'US/Eastern' do
time = Time.new(2016, 4, 23, 15, 11, 12, 3600).to_time
assert_instance_of Time, time
assert_equal @utc_time, time.getutc
assert_equal @utc_offset, time.utc_offset
end
end
end
def test_time_to_time_does_not_preserve_time_zone
with_preserve_timezone(false) do
with_env_tz 'US/Eastern' do
time = Time.new(2016, 4, 23, 15, 11, 12, 3600).to_time
assert_instance_of Time, time
assert_equal @utc_time, time.getutc
assert_equal @system_offset, time.utc_offset
end
end
end
def test_datetime_to_time_preserves_timezone
with_preserve_timezone(true) do
with_env_tz 'US/Eastern' do
time = DateTime.new(2016, 4, 23, 15, 11, 12, Rational(1,24)).to_time
assert_instance_of Time, time
assert_equal @utc_time, time.getutc
assert_equal @utc_offset, time.utc_offset
end
end
end
def test_datetime_to_time_does_not_preserve_time_zone
with_preserve_timezone(false) do
with_env_tz 'US/Eastern' do
time = DateTime.new(2016, 4, 23, 15, 11, 12, Rational(1,24)).to_time
assert_instance_of Time, time
assert_equal @utc_time, time.getutc
assert_equal @system_offset, time.utc_offset
end
end
end
def test_twz_to_time_preserves_timezone
with_preserve_timezone(true) do
with_env_tz 'US/Eastern' do
time = ActiveSupport::TimeWithZone.new(@utc_time, @zone).to_time
assert_instance_of Time, time
assert_equal @utc_time, time.getutc
assert_equal @utc_offset, time.utc_offset
end
end
end
def test_twz_to_time_does_not_preserve_time_zone
with_preserve_timezone(false) do
with_env_tz 'US/Eastern' do
time = ActiveSupport::TimeWithZone.new(@utc_time, @zone).to_time
assert_instance_of Time, time
assert_equal @utc_time, time.getutc
assert_equal @system_offset, time.utc_offset
end
end
end
def test_string_to_time_preserves_timezone
with_preserve_timezone(true) do
with_env_tz 'US/Eastern' do
time = "2016-04-23T15:11:12+01:00".to_time
assert_instance_of Time, time
assert_equal @utc_time, time.getutc
assert_equal @utc_offset, time.utc_offset
end
end
end
def test_string_to_time_does_not_preserve_time_zone
with_preserve_timezone(false) do
with_env_tz 'US/Eastern' do
time = "2016-04-23T15:11:12+01:00".to_time
assert_instance_of Time, time
assert_equal @utc_time, time.getutc
assert_equal @system_offset, time.utc_offset
end
end
end
end
@@ -40,6 +40,14 @@ def test_custom_date_format
Time::DATE_FORMATS.delete(:custom)
end
def test_getlocal
with_env_tz 'US/Eastern' do
assert_equal Time.local(2016, 3, 11, 10, 11, 12), DateTime.new(2016, 3, 11, 15, 11, 12, 0).getlocal
assert_equal Time.local(2016, 3, 21, 11, 11, 12), DateTime.new(2016, 3, 21, 15, 11, 12, 0).getlocal
assert_equal Time.local(2016, 4, 1, 11, 11, 12), DateTime.new(2016, 4, 1, 16, 11, 12, Rational(1,24)).getlocal
end
end
def test_to_date
assert_equal Date.new(2005, 2, 21), DateTime.new(2005, 2, 21, 14, 30, 0).to_date
end
@@ -13,4 +13,12 @@ def with_env_tz(new_tz = 'US/Eastern')
ensure
old_tz ? ENV['TZ'] = old_tz : ENV.delete('TZ')
end
def with_preserve_timezone(value)
old_preserve_tz = ActiveSupport.to_time_preserves_timezone
ActiveSupport.to_time_preserves_timezone = value
yield
ensure
ActiveSupport.to_time_preserves_timezone = old_preserve_tz
end
end
View
@@ -1,3 +1,11 @@
* Add `config/initializers/to_time_preserves_timezone.rb`, which tells
Active Support to preserve the receiver's timezone when calling `to_time`.
This matches the new behavior that will be part of Ruby 2.4.
Fixes #24617.
*Andrew White*
* Make `rails restart` command work with Puma by passing the restart command
which Puma can use to restart rails server.
@@ -92,6 +92,7 @@ def config_when_updating
cookie_serializer_config_exist = File.exist?('config/initializers/cookies_serializer.rb')
callback_terminator_config_exist = File.exist?('config/initializers/callback_terminator.rb')
active_record_belongs_to_required_by_default_config_exist = File.exist?('config/initializers/active_record_belongs_to_required_by_default.rb')
to_time_preserves_timezone_config_exist = File.exist?('config/initializers/to_time_preserves_timezone.rb')
action_cable_config_exist = File.exist?('config/cable.yml')
ssl_options_exist = File.exist?('config/initializers/ssl_options.rb')
rack_cors_config_exist = File.exist?('config/initializers/cors.rb')
@@ -112,6 +113,10 @@ def config_when_updating
remove_file 'config/initializers/active_record_belongs_to_required_by_default.rb'
end
unless to_time_preserves_timezone_config_exist
remove_file 'config/initializers/to_time_preserves_timezone.rb'
end
unless action_cable_config_exist
template 'config/cable.yml'
end
@@ -0,0 +1,10 @@
# Be sure to restart your server when you modify this file.
# Preserve the timezone of the receiver when calling to `to_time`.
# Ruby 2.4 will change the behavior of `to_time` to preserve the timezone
# when converting to an instance of `Time` instead of the previous behavior
# of converting to the local system timezone.
#
# Rails 5.0 introduced this config option so that apps made with earlier
# versions of Rails are not affected when upgrading.
ActiveSupport.to_time_preserves_timezone = true
@@ -257,6 +257,34 @@ def test_rails_update_does_not_remove_active_record_belongs_to_required_by_defau
end
end
def test_rails_update_does_not_create_to_time_preserves_timezone
app_root = File.join(destination_root, 'myapp')
run_generator [app_root]
FileUtils.rm("#{app_root}/config/initializers/to_time_preserves_timezone.rb")
stub_rails_application(app_root) do
generator = Rails::Generators::AppGenerator.new ["rails"], [], destination_root: app_root, shell: @shell
generator.send(:app_const)
quietly { generator.send(:update_config_files) }
assert_no_file "#{app_root}/config/initializers/to_time_preserves_timezone.rb"
end
end
def test_rails_update_does_not_remove_to_time_preserves_timezone_if_already_present
app_root = File.join(destination_root, 'myapp')
run_generator [app_root]
FileUtils.touch("#{app_root}/config/initializers/to_time_preserves_timezone.rb")
stub_rails_application(app_root) do
generator = Rails::Generators::AppGenerator.new ["rails"], [], destination_root: app_root, shell: @shell
generator.send(:app_const)
quietly { generator.send(:update_config_files) }
assert_file "#{app_root}/config/initializers/to_time_preserves_timezone.rb"
end
end
def test_rails_update_does_not_create_ssl_options_by_default
app_root = File.join(destination_root, 'myapp')
run_generator [app_root]

0 comments on commit c9c5788

Please sign in to comment.