Skip to content

Commit

Permalink
Merge pull request #52031 from matthewd/quieter-to_time
Browse files Browse the repository at this point in the history
Don't emit to_time deprecations in known-safe contexts
  • Loading branch information
rafaelfranca committed Jun 26, 2024
1 parent 4e5e630 commit 2d4061f
Show file tree
Hide file tree
Showing 7 changed files with 66 additions and 6 deletions.
2 changes: 1 addition & 1 deletion activerecord/lib/active_record/timestamp.rb
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ def current_time_from_proper_timezone

def max_updated_column_timestamp
timestamp_attributes_for_update_in_model
.filter_map { |attr| self[attr]&.to_time }
.filter_map { |attr| (v = self[attr]) && (v.is_a?(::Time) ? v : v.to_time) }
.max
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,12 @@ def compare_with_coercion(other)
if other.class == Time
compare_without_coercion(other)
elsif other.is_a?(Time)
compare_without_coercion(other.to_time)
# also avoid ActiveSupport::TimeWithZone#to_time before Rails 8.0
if other.respond_to?(:comparable_time)
compare_without_coercion(other.comparable_time)
else
compare_without_coercion(other.to_time)
end
else
to_datetime <=> other
end
Expand Down
16 changes: 16 additions & 0 deletions activesupport/lib/active_support/core_ext/time/compatibility.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,20 @@ class Time
def to_time
preserve_timezone ? self : getlocal
end

def preserve_timezone # :nodoc:
active_support_local_zone == zone || super
end

private
@@active_support_local_tz = nil

def active_support_local_zone
@@active_support_local_zone = nil if @@active_support_local_tz != ENV["TZ"]
@@active_support_local_zone ||=
begin
@@active_support_local_tz = ENV["TZ"]
Time.new.zone
end
end
end
6 changes: 3 additions & 3 deletions activesupport/lib/active_support/testing/time_helpers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -163,10 +163,10 @@ def travel_to(date_or_time, with_usec: false)
now = date_or_time.midnight.to_time
elsif date_or_time.is_a?(String)
now = Time.zone.parse(date_or_time)
elsif with_usec
now = date_or_time.to_time
else
now = date_or_time.to_time.change(usec: 0)
now = date_or_time
now = now.to_time unless now.is_a?(Time)
now = now.change(usec: 0) unless with_usec
end

# +now+ must be in local system timezone, because +Time.at(now)+
Expand Down
2 changes: 1 addition & 1 deletion activesupport/lib/active_support/time_with_zone.rb
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ def +(other)
#
def -(other)
if other.acts_like?(:time)
to_time - other.to_time
getutc - other.getutc
elsif duration_of_variable_length?(other)
method_missing(:-, other)
else
Expand Down
29 changes: 29 additions & 0 deletions activesupport/test/core_ext/date_and_time_compatibility_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,35 @@ def test_time_to_time_does_not_preserve_time_zone
end
end

def test_time_to_time_without_preserve_configured
with_preserve_timezone(nil) do
with_env_tz "US/Eastern" do
source = Time.new(2016, 4, 23, 15, 11, 12)
# No warning because it's already local
base_time = source.to_time

utc_time = base_time.getutc
converted_time = assert_deprecated(ActiveSupport.deprecator) { utc_time.to_time }

assert_equal source, base_time
assert_equal source, converted_time
assert_equal @system_offset, base_time.utc_offset
assert_equal @system_offset, converted_time.utc_offset
end
end

with_preserve_timezone(nil) do
with_env_tz "US/Eastern" do
foreign_time = Time.new(2016, 4, 23, 15, 11, 12, in: "-0700")
converted_time = assert_deprecated(ActiveSupport.deprecator) { foreign_time.to_time }

assert_equal foreign_time, converted_time
assert_equal @system_offset, converted_time.utc_offset
assert_not_equal foreign_time.utc_offset, converted_time.utc_offset
end
end
end

def test_time_to_time_frozen_preserves_timezone
with_preserve_timezone(true) do
with_env_tz "US/Eastern" do
Expand Down
10 changes: 10 additions & 0 deletions activesupport/test/core_ext/time_with_zone_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,16 @@ def test_minus_with_time_with_zone
assert_equal 86_400.0, twz2 - twz1
end

def test_minus_with_time_with_zone_without_preserve_configured
with_preserve_timezone(nil) do
twz1 = ActiveSupport::TimeWithZone.new(Time.utc(2000, 1, 1), ActiveSupport::TimeZone["UTC"])
twz2 = ActiveSupport::TimeWithZone.new(Time.utc(2000, 1, 2), ActiveSupport::TimeZone["UTC"])

difference = assert_not_deprecated(ActiveSupport.deprecator) { twz2 - twz1 }
assert_equal 86_400.0, difference
end
end

def test_minus_with_time_with_zone_precision
twz1 = ActiveSupport::TimeWithZone.new(Time.utc(2000, 1, 1, 0, 0, 0, Rational(1, 1000)), ActiveSupport::TimeZone["UTC"])
twz2 = ActiveSupport::TimeWithZone.new(Time.utc(2000, 1, 1, 23, 59, 59, Rational(999999999, 1000)), ActiveSupport::TimeZone["UTC"])
Expand Down

0 comments on commit 2d4061f

Please sign in to comment.