Skip to content

Commit f46d06b

Browse files
authored
Merge pull request rails#52031 from matthewd/quieter-to_time
Don't emit to_time deprecations in known-safe contexts
2 parents acc8f3d + 2627c95 commit f46d06b

File tree

7 files changed

+66
-6
lines changed

7 files changed

+66
-6
lines changed

activerecord/lib/active_record/timestamp.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ def current_time_from_proper_timezone
162162

163163
def max_updated_column_timestamp
164164
timestamp_attributes_for_update_in_model
165-
.filter_map { |attr| self[attr]&.to_time }
165+
.filter_map { |attr| (v = self[attr]) && (v.is_a?(::Time) ? v : v.to_time) }
166166
.max
167167
end
168168

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -319,7 +319,12 @@ def compare_with_coercion(other)
319319
if other.class == Time
320320
compare_without_coercion(other)
321321
elsif other.is_a?(Time)
322-
compare_without_coercion(other.to_time)
322+
# also avoid ActiveSupport::TimeWithZone#to_time before Rails 8.0
323+
if other.respond_to?(:comparable_time)
324+
compare_without_coercion(other.comparable_time)
325+
else
326+
compare_without_coercion(other.to_time)
327+
end
323328
else
324329
to_datetime <=> other
325330
end

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,4 +13,20 @@ class Time
1313
def to_time
1414
preserve_timezone ? self : getlocal
1515
end
16+
17+
def preserve_timezone # :nodoc:
18+
active_support_local_zone == zone || super
19+
end
20+
21+
private
22+
@@active_support_local_tz = nil
23+
24+
def active_support_local_zone
25+
@@active_support_local_zone = nil if @@active_support_local_tz != ENV["TZ"]
26+
@@active_support_local_zone ||=
27+
begin
28+
@@active_support_local_tz = ENV["TZ"]
29+
Time.new.zone
30+
end
31+
end
1632
end

activesupport/lib/active_support/testing/time_helpers.rb

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -163,10 +163,10 @@ def travel_to(date_or_time, with_usec: false)
163163
now = date_or_time.midnight.to_time
164164
elsif date_or_time.is_a?(String)
165165
now = Time.zone.parse(date_or_time)
166-
elsif with_usec
167-
now = date_or_time.to_time
168166
else
169-
now = date_or_time.to_time.change(usec: 0)
167+
now = date_or_time
168+
now = now.to_time unless now.is_a?(Time)
169+
now = now.change(usec: 0) unless with_usec
170170
end
171171

172172
# +now+ must be in local system timezone, because +Time.at(now)+

activesupport/lib/active_support/time_with_zone.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,7 @@ def +(other)
332332
#
333333
def -(other)
334334
if other.acts_like?(:time)
335-
to_time - other.to_time
335+
getutc - other.getutc
336336
elsif duration_of_variable_length?(other)
337337
method_missing(:-, other)
338338
else

activesupport/test/core_ext/date_and_time_compatibility_test.rb

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,35 @@ def test_time_to_time_does_not_preserve_time_zone
4343
end
4444
end
4545

46+
def test_time_to_time_without_preserve_configured
47+
with_preserve_timezone(nil) do
48+
with_env_tz "US/Eastern" do
49+
source = Time.new(2016, 4, 23, 15, 11, 12)
50+
# No warning because it's already local
51+
base_time = source.to_time
52+
53+
utc_time = base_time.getutc
54+
converted_time = assert_deprecated(ActiveSupport.deprecator) { utc_time.to_time }
55+
56+
assert_equal source, base_time
57+
assert_equal source, converted_time
58+
assert_equal @system_offset, base_time.utc_offset
59+
assert_equal @system_offset, converted_time.utc_offset
60+
end
61+
end
62+
63+
with_preserve_timezone(nil) do
64+
with_env_tz "US/Eastern" do
65+
foreign_time = Time.new(2016, 4, 23, 15, 11, 12, in: "-0700")
66+
converted_time = assert_deprecated(ActiveSupport.deprecator) { foreign_time.to_time }
67+
68+
assert_equal foreign_time, converted_time
69+
assert_equal @system_offset, converted_time.utc_offset
70+
assert_not_equal foreign_time.utc_offset, converted_time.utc_offset
71+
end
72+
end
73+
end
74+
4675
def test_time_to_time_frozen_preserves_timezone
4776
with_preserve_timezone(true) do
4877
with_env_tz "US/Eastern" do

activesupport/test/core_ext/time_with_zone_test.rb

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -433,6 +433,16 @@ def test_minus_with_time_with_zone
433433
assert_equal 86_400.0, twz2 - twz1
434434
end
435435

436+
def test_minus_with_time_with_zone_without_preserve_configured
437+
with_preserve_timezone(nil) do
438+
twz1 = ActiveSupport::TimeWithZone.new(Time.utc(2000, 1, 1), ActiveSupport::TimeZone["UTC"])
439+
twz2 = ActiveSupport::TimeWithZone.new(Time.utc(2000, 1, 2), ActiveSupport::TimeZone["UTC"])
440+
441+
difference = assert_not_deprecated(ActiveSupport.deprecator) { twz2 - twz1 }
442+
assert_equal 86_400.0, difference
443+
end
444+
end
445+
436446
def test_minus_with_time_with_zone_precision
437447
twz1 = ActiveSupport::TimeWithZone.new(Time.utc(2000, 1, 1, 0, 0, 0, Rational(1, 1000)), ActiveSupport::TimeZone["UTC"])
438448
twz2 = ActiveSupport::TimeWithZone.new(Time.utc(2000, 1, 1, 23, 59, 59, Rational(999999999, 1000)), ActiveSupport::TimeZone["UTC"])

0 commit comments

Comments
 (0)