Handle #to_time and memoization taking into account memoization, frozen state, and preserve_timezone flag.

6 files changed, 206 insertions, 22 deletions

diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md
index 0f43b1256f..984b9400c8 100644
--- a/activesupport/CHANGELOG.md
+++ b/activesupport/CHANGELOG.md
@@ -1,5 +1,12 @@
 ## Rails 5.1.0.beta1 (February 23, 2017) ##
 
+*   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*
+
 *   Cache `ActiveSupport::TimeWithZone#to_datetime` before freezing.
 
     *Adam Rice*
diff --git a/activesupport/lib/active_support/core_ext/date_and_time/compatibility.rb b/activesupport/lib/active_support/core_ext/date_and_time/compatibility.rb
index db95ae0db5..3f4e236ab7 100644
--- a/activesupport/lib/active_support/core_ext/date_and_time/compatibility.rb
+++ b/activesupport/lib/active_support/core_ext/date_and_time/compatibility.rb
@@ -12,11 +12,7 @@ module DateAndTime
     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
+      preserve_timezone ? getlocal(utc_offset) : getlocal
     end
   end
 end
diff --git a/activesupport/lib/active_support/core_ext/time/compatibility.rb b/activesupport/lib/active_support/core_ext/time/compatibility.rb
index ca4b9574d5..32e5608b32 100644
--- a/activesupport/lib/active_support/core_ext/time/compatibility.rb
+++ b/activesupport/lib/active_support/core_ext/time/compatibility.rb
@@ -1,5 +1,12 @@
 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
+
+  def to_time
+    preserve_timezone ? self : getlocal
+  end
 end
diff --git a/activesupport/lib/active_support/time_with_zone.rb b/activesupport/lib/active_support/time_with_zone.rb
index 857cc1a664..9ad8453ccf 100644
--- a/activesupport/lib/active_support/time_with_zone.rb
+++ b/activesupport/lib/active_support/time_with_zone.rb
@@ -410,6 +410,15 @@ module ActiveSupport
       @to_datetime ||= utc.to_datetime.new_offset(Rational(utc_offset, 86_400))
     end
 
+    # Returns an instance of <tt>Time</tt>
+    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
@@ -428,7 +437,7 @@ module ActiveSupport
 
     def freeze
       # preload instance variables before freezing
-      period; utc; time; to_datetime
+      period; utc; time; to_datetime; to_time
       super
     end
 
diff --git a/activesupport/test/core_ext/date_and_time_compatibility_test.rb b/activesupport/test/core_ext/date_and_time_compatibility_test.rb
index 4c90460032..6c6205a4d2 100644
--- a/activesupport/test/core_ext/date_and_time_compatibility_test.rb
+++ b/activesupport/test/core_ext/date_and_time_compatibility_test.rb
@@ -16,11 +16,13 @@ class DateAndTimeCompatibilityTest < ActiveSupport::TestCase
   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
+        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
@@ -28,11 +30,43 @@ class DateAndTimeCompatibilityTest < ActiveSupport::TestCase
   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
+        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
@@ -40,7 +74,8 @@ class DateAndTimeCompatibilityTest < ActiveSupport::TestCase
   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
+        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
@@ -52,7 +87,8 @@ class DateAndTimeCompatibilityTest < ActiveSupport::TestCase
   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
+        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 @@ class DateAndTimeCompatibilityTest < ActiveSupport::TestCase
     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
+        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
@@ -84,19 +150,69 @@ class DateAndTimeCompatibilityTest < ActiveSupport::TestCase
   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
+        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
@@ -104,7 +220,8 @@ class DateAndTimeCompatibilityTest < ActiveSupport::TestCase
   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
+        source = "2016-04-23T15:11:12+01:00"
+        time = source.to_time
 
         assert_instance_of Time, time
         assert_equal @utc_time, time.getutc
@@ -116,11 +233,40 @@ class DateAndTimeCompatibilityTest < ActiveSupport::TestCase
   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
+        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
diff --git a/activesupport/test/core_ext/time_with_zone_test.rb b/activesupport/test/core_ext/time_with_zone_test.rb
index 1534daacb9..ba15ea6b32 100644
--- a/activesupport/test/core_ext/time_with_zone_test.rb
+++ b/activesupport/test/core_ext/time_with_zone_test.rb
@@ -421,11 +421,29 @@ class TimeWithZoneTest < ActiveSupport::TestCase
     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
 
@@ -508,6 +526,7 @@ class TimeWithZoneTest < ActiveSupport::TestCase
       @twz.period
       @twz.time
       @twz.to_datetime
+      @twz.to_time
     end
   end