[Backport] Freeze and convert to_time

activesupport/CHANGELOG.md

@@ -1,3 +1,10 @@
+*   Fixed bug in `DateAndTime::Compatibility#to_time` that caused it to
+    raise `RuntimeError: can't modify frozen Time` when called on any frozen `Time`.
+    Properly pass through the frozen `Time` or `ActiveSupport::TimeWithZone` object
+    when calling `#to_time`.
+
+    *Kevin McPhillips* & *Andrew White*
+
 *   Fix inconsistent results when parsing large durations and constructing durations from code
 
         ActiveSupport::Duration.parse('P3Y') == 3.years # It should be true

activesupport/lib/active_support/core_ext/date_and_time/compatibility.rb

@@ -10,13 +10,5 @@ module Compatibility
     # 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 }
-
-    def to_time
-      if preserve_timezone
-        @_to_time_with_instance_offset ||= getlocal(utc_offset)
-      else
-        @_to_time_with_system_offset ||= getlocal
-      end
-    end
   end
 end

activesupport/lib/active_support/core_ext/date_time/compatibility.rb

@@ -1,5 +1,15 @@
 require 'active_support/core_ext/date_and_time/compatibility'
 
 class DateTime
-  prepend DateAndTime::Compatibility
+  include DateAndTime::Compatibility
+
+  remove_possible_method :to_time
+
+  # Either return an instance of `Time` with the same UTC offset
+  # as +self+ or an instance of `Time` representing the same time
+  # in the the local system timezone depending on the setting of
+  # on the setting of +ActiveSupport.to_time_preserves_timezone+.
+  def to_time
+    preserve_timezone ? getlocal(utc_offset) : getlocal
+  end
 end

activesupport/lib/active_support/core_ext/time/compatibility.rb

@@ -1,5 +1,14 @@
-require 'active_support/core_ext/date_and_time/compatibility'
+require "active_support/core_ext/date_and_time/compatibility"
+require "active_support/core_ext/module/remove_method"
 
 class Time
-  prepend DateAndTime::Compatibility
+  include DateAndTime::Compatibility
+
+  remove_possible_method :to_time
+
+  # Either return +self+ or the time in the local system timezone depending
+  # on the setting of +ActiveSupport.to_time_preserves_timezone+.
+  def to_time
+    preserve_timezone ? self : getlocal
+  end
 end

activesupport/lib/active_support/time_with_zone.rb

@@ -411,6 +411,17 @@ def to_datetime
       utc.to_datetime.new_offset(Rational(utc_offset, 86_400))
     end
 
+    # Returns an instance of +Time+, either with the same UTC offset
+    # as +self+ or in the local system timezone depending on the setting
+    # of +ActiveSupport.to_time_preserves_timezone+.
+    def to_time
+      if preserve_timezone
+        @to_time_with_instance_offset ||= getlocal(utc_offset)
+      else
+        @to_time_with_system_offset ||= getlocal
+      end
+    end
+
     # So that +self+ <tt>acts_like?(:time)</tt>.
     def acts_like_time?
       true
@@ -429,7 +440,7 @@ def blank?
 
     def freeze
       # preload instance variables before freezing
-      period; utc; time; to_datetime
+      period; utc; time; to_datetime; to_time
       super
     end
 

activesupport/test/core_ext/date_and_time_compatibility_test.rb

@@ -15,32 +15,67 @@ def setup
 
   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
+      with_env_tz "US/Eastern" do
+        source = Time.new(2016, 4, 23, 15, 11, 12, 3600)
+        time = source.to_time
 
         assert_instance_of Time, time
         assert_equal @utc_time, time.getutc
         assert_equal @utc_offset, time.utc_offset
+        assert_equal source.object_id, time.object_id
       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
+      with_env_tz "US/Eastern" do
+        source = Time.new(2016, 4, 23, 15, 11, 12, 3600)
+        time = source.to_time
 
         assert_instance_of Time, time
         assert_equal @utc_time, time.getutc
         assert_equal @system_offset, time.utc_offset
+        assert_not_equal source.object_id, time.object_id
+      end
+    end
+  end
+
+  def test_time_to_time_frozen_preserves_timezone
+    with_preserve_timezone(true) do
+      with_env_tz "US/Eastern" do
+        source = Time.new(2016, 4, 23, 15, 11, 12, 3600).freeze
+        time = source.to_time
+
+        assert_instance_of Time, time
+        assert_equal @utc_time, time.getutc
+        assert_equal @utc_offset, time.utc_offset
+        assert_equal source.object_id, time.object_id
+        assert_predicate time, :frozen?
+      end
+    end
+  end
+
+  def test_time_to_time_frozen_does_not_preserve_time_zone
+    with_preserve_timezone(false) do
+      with_env_tz "US/Eastern" do
+        source = Time.new(2016, 4, 23, 15, 11, 12, 3600).freeze
+        time = source.to_time
+
+        assert_instance_of Time, time
+        assert_equal @utc_time, time.getutc
+        assert_equal @system_offset, time.utc_offset
+        assert_not_equal source.object_id, time.object_id
+        assert_not_predicate time, :frozen?
       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
+      with_env_tz "US/Eastern" do
+        source = DateTime.new(2016, 4, 23, 15, 11, 12, Rational(1, 24))
+        time = source.to_time
 
         assert_instance_of Time, time
         assert_equal @utc_time, time.getutc
@@ -51,8 +86,9 @@ def test_datetime_to_time_preserves_timezone
 
   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
+      with_env_tz "US/Eastern" do
+        source = DateTime.new(2016, 4, 23, 15, 11, 12, Rational(1, 24))
+        time = source.to_time
 
         assert_instance_of Time, time
         assert_equal @utc_time, time.getutc
@@ -61,17 +97,47 @@ def test_datetime_to_time_does_not_preserve_time_zone
     end
   end
 
+  def test_datetime_to_time_frozen_preserves_timezone
+    with_preserve_timezone(true) do
+      with_env_tz "US/Eastern" do
+        source = DateTime.new(2016, 4, 23, 15, 11, 12, Rational(1, 24)).freeze
+        time = source.to_time
+
+        assert_instance_of Time, time
+        assert_equal @utc_time, time.getutc
+        assert_equal @utc_offset, time.utc_offset
+        assert_not_predicate time, :frozen?
+      end
+    end
+  end
+
+  def test_datetime_to_time_frozen_does_not_preserve_time_zone
+    with_preserve_timezone(false) do
+      with_env_tz "US/Eastern" do
+        source = DateTime.new(2016, 4, 23, 15, 11, 12, Rational(1, 24)).freeze
+        time = source.to_time
+
+        assert_instance_of Time, time
+        assert_equal @utc_time, time.getutc
+        assert_equal @system_offset, time.utc_offset
+        assert_not_predicate time, :frozen?
+      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
+      with_env_tz "US/Eastern" do
+        source = ActiveSupport::TimeWithZone.new(@utc_time, @zone)
+        time = source.to_time
 
         assert_instance_of Time, time
         assert_equal @utc_time, time.getutc
         assert_instance_of Time, time.getutc
         assert_equal @utc_offset, time.utc_offset
 
-        time = ActiveSupport::TimeWithZone.new(@date_time, @zone).to_time
+        source = ActiveSupport::TimeWithZone.new(@date_time, @zone)
+        time = source.to_time
 
         assert_instance_of Time, time
         assert_equal @date_time, time.getutc
@@ -83,28 +149,79 @@ def test_twz_to_time_preserves_timezone
 
   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
+      with_env_tz "US/Eastern" do
+        source = ActiveSupport::TimeWithZone.new(@utc_time, @zone)
+        time = source.to_time
+
+        assert_instance_of Time, time
+        assert_equal @utc_time, time.getutc
+        assert_instance_of Time, time.getutc
+        assert_equal @system_offset, time.utc_offset
+
+        source = ActiveSupport::TimeWithZone.new(@date_time, @zone)
+        time = source.to_time
+
+        assert_instance_of Time, time
+        assert_equal @date_time, time.getutc
+        assert_instance_of Time, time.getutc
+        assert_equal @system_offset, time.utc_offset
+      end
+    end
+  end
+
+  def test_twz_to_time_frozen_preserves_timezone
+    with_preserve_timezone(true) do
+      with_env_tz "US/Eastern" do
+        source = ActiveSupport::TimeWithZone.new(@utc_time, @zone).freeze
+        time = source.to_time
+
+        assert_instance_of Time, time
+        assert_equal @utc_time, time.getutc
+        assert_instance_of Time, time.getutc
+        assert_equal @utc_offset, time.utc_offset
+        assert_not_predicate time, :frozen?
+
+        source = ActiveSupport::TimeWithZone.new(@date_time, @zone).freeze
+        time = source.to_time
+
+        assert_instance_of Time, time
+        assert_equal @date_time, time.getutc
+        assert_instance_of Time, time.getutc
+        assert_equal @utc_offset, time.utc_offset
+        assert_not_predicate time, :frozen?
+      end
+    end
+  end
+
+  def test_twz_to_time_frozen_does_not_preserve_time_zone
+    with_preserve_timezone(false) do
+      with_env_tz "US/Eastern" do
+        source = ActiveSupport::TimeWithZone.new(@utc_time, @zone).freeze
+        time = source.to_time
 
         assert_instance_of Time, time
         assert_equal @utc_time, time.getutc
         assert_instance_of Time, time.getutc
         assert_equal @system_offset, time.utc_offset
+        assert_not_predicate time, :frozen?
 
-        time = ActiveSupport::TimeWithZone.new(@date_time, @zone).to_time
+        source = ActiveSupport::TimeWithZone.new(@date_time, @zone).freeze
+        time = source.to_time
 
         assert_instance_of Time, time
         assert_equal @date_time, time.getutc
         assert_instance_of Time, time.getutc
         assert_equal @system_offset, time.utc_offset
+        assert_not_predicate time, :frozen?
       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
+      with_env_tz "US/Eastern" do
+        source = "2016-04-23T15:11:12+01:00"
+        time = source.to_time
 
         assert_instance_of Time, time
         assert_equal @utc_time, time.getutc
@@ -115,12 +232,41 @@ def test_string_to_time_preserves_timezone
 
   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
+      with_env_tz "US/Eastern" do
+        source = "2016-04-23T15:11:12+01:00"
+        time = source.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_frozen_preserves_timezone
+    with_preserve_timezone(true) do
+      with_env_tz "US/Eastern" do
+        source = "2016-04-23T15:11:12+01:00".freeze
+        time = source.to_time
+
+        assert_instance_of Time, time
+        assert_equal @utc_time, time.getutc
+        assert_equal @utc_offset, time.utc_offset
+        assert_not_predicate time, :frozen?
+      end
+    end
+  end
+
+  def test_string_to_time_frozen_does_not_preserve_time_zone
+    with_preserve_timezone(false) do
+      with_env_tz "US/Eastern" do
+        source = "2016-04-23T15:11:12+01:00".freeze
+        time = source.to_time
 
         assert_instance_of Time, time
         assert_equal @utc_time, time.getutc
         assert_equal @system_offset, time.utc_offset
+        assert_not_predicate time, :frozen?
       end
     end
   end

activesupport/test/core_ext/time_with_zone_test.rb

@@ -421,11 +421,29 @@ def test_time_at
     assert_equal time, Time.at(time)
   end
 
-  def test_to_time
-    with_env_tz 'US/Eastern' do
-      assert_equal Time, @twz.to_time.class
-      assert_equal Time.local(1999, 12, 31, 19), @twz.to_time
-      assert_equal Time.local(1999, 12, 31, 19).utc_offset, @twz.to_time.utc_offset
+  def test_to_time_with_preserve_timezone
+    with_preserve_timezone(true) do
+      with_env_tz "US/Eastern" do
+        time = @twz.to_time
+
+        assert_equal Time, time.class
+        assert_equal time.object_id, @twz.to_time.object_id
+        assert_equal Time.local(1999, 12, 31, 19), time
+        assert_equal Time.local(1999, 12, 31, 19).utc_offset, time.utc_offset
+      end
+    end
+  end
+
+  def test_to_time_without_preserve_timezone
+    with_preserve_timezone(false) do
+      with_env_tz "US/Eastern" do
+        time = @twz.to_time
+
+        assert_equal Time, time.class
+        assert_equal time.object_id, @twz.to_time.object_id
+        assert_equal Time.local(1999, 12, 31, 19), time
+        assert_equal Time.local(1999, 12, 31, 19).utc_offset, time.utc_offset
+      end
     end
   end
 
@@ -504,6 +522,7 @@ def test_freeze_preloads_instance_variables
       @twz.period
       @twz.time
       @twz.to_datetime
+      @twz.to_time
     end
   end