Skip to content
Browse files

Merge branch 'deprecate_numeric_since_ago' of https://github.com/chan…

…cancode/rails

Conflicts:
	activesupport/CHANGELOG.md
  • Loading branch information...
2 parents f15bd23 + 1f16136 commit d4016f24fb5266b361f44da66de445abfbb086e1 @jeremy jeremy committed Nov 27, 2013
View
17 activesupport/CHANGELOG.md
@@ -1,3 +1,20 @@
+* Deprecated Numeric#{ago,until,since,from_now}, the user is expected to explicitly
+ convert the value into an AS::Duration, i.e. 5.ago => 5.seconds.ago
+
+ This will help to catch subtle bugs like:
+
+ def recent?(days = 3)
+ self.created_at >= days.ago
+ end
+
+ The above code would check if the model is created within the last 3 **seconds**.
+
+ In the future, Numeric#{ago,until,since,from_now} should be removed completely,
+ or throw some sort of errors to indicate there are no implicit conversion from
+ Numeric to AS::Duration.
+
+ *Godfrey Chan*
+
* Requires JSON gem version 1.7.7 or above due to a security issue in older versions.
*Godfrey Chan*
View
2 activesupport/lib/active_support/core_ext/numeric/time.rb
@@ -63,6 +63,7 @@ def fortnights
# Reads best without arguments: 10.minutes.ago
def ago(time = ::Time.current)
+ ActiveSupport::Deprecation.warn "Calling #ago or #until on a number (e.g. 5.ago) is deprecated and will be removed in the future, use 5.seconds.ago instead"
time - self
end
@@ -71,6 +72,7 @@ def ago(time = ::Time.current)
# Reads best with argument: 10.minutes.since(time)
def since(time = ::Time.current)
+ ActiveSupport::Deprecation.warn "Calling #since or #from_now on a number (e.g. 5.since) is deprecated and will be removed in the future, use 5.seconds.since instead"
time + self
end
View
21 activesupport/test/core_ext/duration_test.rb
@@ -71,6 +71,19 @@ def test_fractional_days
assert_equal 86400 * 1.7, 1.7.days
end
+ def test_since_and_ago
+ t = Time.local(2000)
+ assert t + 1, 1.second.since(t)
+ assert t - 1, 1.second.ago(t)
+ end
+
+ def test_since_and_ago_without_argument
+ now = Time.now
+ assert 1.second.since >= now + 1
+ now = Time.now
+ assert 1.second.ago >= now - 1
+ end
+
def test_since_and_ago_with_fractional_days
t = Time.local(2000)
# since
@@ -96,10 +109,10 @@ def test_since_and_ago_anchored_to_time_now_when_time_zone_is_not_set
with_env_tz 'US/Eastern' do
Time.stubs(:now).returns Time.local(2000)
# since
- assert_equal false, 5.seconds.since.is_a?(ActiveSupport::TimeWithZone)
+ assert_not_instance_of ActiveSupport::TimeWithZone, 5.seconds.since
assert_equal Time.local(2000,1,1,0,0,5), 5.seconds.since
# ago
- assert_equal false, 5.seconds.ago.is_a?(ActiveSupport::TimeWithZone)
+ assert_not_instance_of ActiveSupport::TimeWithZone, 5.seconds.ago
assert_equal Time.local(1999,12,31,23,59,55), 5.seconds.ago
end
end
@@ -109,11 +122,11 @@ def test_since_and_ago_anchored_to_time_zone_now_when_time_zone_is_set
with_env_tz 'US/Eastern' do
Time.stubs(:now).returns Time.local(2000)
# since
- assert_equal true, 5.seconds.since.is_a?(ActiveSupport::TimeWithZone)
+ assert_instance_of ActiveSupport::TimeWithZone, 5.seconds.since
assert_equal Time.utc(2000,1,1,0,0,5), 5.seconds.since.time
assert_equal 'Eastern Time (US & Canada)', 5.seconds.since.time_zone.name
# ago
- assert_equal true, 5.seconds.ago.is_a?(ActiveSupport::TimeWithZone)
+ assert_instance_of ActiveSupport::TimeWithZone, 5.seconds.ago
assert_equal Time.utc(1999,12,31,23,59,55), 5.seconds.ago.time
assert_equal 'Eastern Time (US & Canada)', 5.seconds.ago.time_zone.name
end
View
49 activesupport/test/core_ext/numeric_ext_test.rb
@@ -22,21 +22,16 @@ def test_units
end
end
- def test_intervals
- @seconds.values.each do |seconds|
- assert_equal seconds.since(@now), @now + seconds
- assert_equal seconds.until(@now), @now - seconds
- end
+ def test_deprecated_since_and_ago
+ assert_equal @now + 1, assert_deprecated { 1.since(@now) }
+ assert_equal @now - 1, assert_deprecated { 1.ago(@now) }
end
- # Test intervals based from Time.now
- def test_now
- @seconds.values.each do |seconds|
- now = Time.now
- assert seconds.ago >= now - seconds
- now = Time.now
- assert seconds.from_now >= now + seconds
- end
+ def test_deprecated_since_and_ago_without_argument
+ now = Time.now
+ assert assert_deprecated { 1.since } >= now + 1
+ now = Time.now
+ assert assert_deprecated { 1.ago } >= now - 1
end
def test_irregular_durations
@@ -78,10 +73,10 @@ def test_chaining_duration_operations
end
def test_duration_after_conversion_is_no_longer_accurate
- assert_equal 30.days.to_i.since(@now), 1.month.to_i.since(@now)
- assert_equal 365.25.days.to_f.since(@now), 1.year.to_f.since(@now)
- assert_equal 30.days.to_i.since(@dtnow), 1.month.to_i.since(@dtnow)
- assert_equal 365.25.days.to_f.since(@dtnow), 1.year.to_f.since(@dtnow)
+ assert_equal 30.days.to_i.seconds.since(@now), 1.month.to_i.seconds.since(@now)
+ assert_equal 365.25.days.to_f.seconds.since(@now), 1.year.to_f.seconds.since(@now)
+ assert_equal 30.days.to_i.seconds.since(@dtnow), 1.month.to_i.seconds.since(@dtnow)
+ assert_equal 365.25.days.to_f.seconds.since(@dtnow), 1.year.to_f.seconds.since(@dtnow)
end
def test_add_one_year_to_leap_day
@@ -94,11 +89,11 @@ def test_since_and_ago_anchored_to_time_now_when_time_zone_is_not_set
with_env_tz 'US/Eastern' do
Time.stubs(:now).returns Time.local(2000)
# since
- assert_equal false, 5.since.is_a?(ActiveSupport::TimeWithZone)
- assert_equal Time.local(2000,1,1,0,0,5), 5.since
+ assert_not_instance_of ActiveSupport::TimeWithZone, assert_deprecated { 5.since }
+ assert_equal Time.local(2000,1,1,0,0,5), assert_deprecated { 5.since }
# ago
- assert_equal false, 5.ago.is_a?(ActiveSupport::TimeWithZone)
- assert_equal Time.local(1999,12,31,23,59,55), 5.ago
+ assert_not_instance_of ActiveSupport::TimeWithZone, assert_deprecated { 5.ago }
+ assert_equal Time.local(1999,12,31,23,59,55), assert_deprecated { 5.ago }
end
end
@@ -107,13 +102,13 @@ def test_since_and_ago_anchored_to_time_zone_now_when_time_zone_is_set
with_env_tz 'US/Eastern' do
Time.stubs(:now).returns Time.local(2000)
# since
- assert_equal true, 5.since.is_a?(ActiveSupport::TimeWithZone)
- assert_equal Time.utc(2000,1,1,0,0,5), 5.since.time
- assert_equal 'Eastern Time (US & Canada)', 5.since.time_zone.name
+ assert_instance_of ActiveSupport::TimeWithZone, assert_deprecated { 5.since }
+ assert_equal Time.utc(2000,1,1,0,0,5), assert_deprecated { 5.since.time }
+ assert_equal 'Eastern Time (US & Canada)', assert_deprecated { 5.since.time_zone.name }
# ago
- assert_equal true, 5.ago.is_a?(ActiveSupport::TimeWithZone)
- assert_equal Time.utc(1999,12,31,23,59,55), 5.ago.time
- assert_equal 'Eastern Time (US & Canada)', 5.ago.time_zone.name
+ assert_instance_of ActiveSupport::TimeWithZone, assert_deprecated { 5.ago }
+ assert_equal Time.utc(1999,12,31,23,59,55), assert_deprecated { 5.ago.time }
+ assert_equal 'Eastern Time (US & Canada)', assert_deprecated { 5.ago.time_zone.name }
end
ensure
Time.zone = nil

0 comments on commit d4016f2

Please sign in to comment.
Something went wrong with that request. Please try again.