aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAndrew White <pixeltrix@users.noreply.github.com>2017-03-03 10:48:41 +0000
committerGitHub <noreply@github.com>2017-03-03 10:48:41 +0000
commitc9bc4de19850ce3e66d0bf1f9ef70468c0366670 (patch)
treed6610b4eb96c4c69bb69a486c43d270ce1b46b82
parente1a72fef7de9a5c0871b0ef1a4acb9483d2bf1f3 (diff)
parent75924c4517c8f87712d3f59c11f10152ed57b9d8 (diff)
downloadrails-c9bc4de19850ce3e66d0bf1f9ef70468c0366670.tar.gz
rails-c9bc4de19850ce3e66d0bf1f9ef70468c0366670.tar.bz2
rails-c9bc4de19850ce3e66d0bf1f9ef70468c0366670.zip
Merge pull request #28204 from rails/deprecate-implicit-coercion-of-durations
Deprecate implicit coercion of `ActiveSupport::Duration`
-rw-r--r--actionmailer/test/message_delivery_test.rb2
-rw-r--r--actionview/test/template/date_helper_test.rb2
-rw-r--r--activesupport/CHANGELOG.md52
-rw-r--r--activesupport/lib/active_support/cache/mem_cache_store.rb2
-rw-r--r--activesupport/lib/active_support/duration.rb41
-rw-r--r--activesupport/test/core_ext/duration_test.rb58
6 files changed, 149 insertions, 8 deletions
diff --git a/actionmailer/test/message_delivery_test.rb b/actionmailer/test/message_delivery_test.rb
index a79d77e1e5..f4c4f43bdc 100644
--- a/actionmailer/test/message_delivery_test.rb
+++ b/actionmailer/test/message_delivery_test.rb
@@ -76,7 +76,7 @@ class MessageDeliveryTest < ActiveSupport::TestCase
test "should enqueue a delivery with a delay" do
travel_to Time.new(2004, 11, 24, 01, 04, 44) do
- assert_performed_with(job: ActionMailer::DeliveryJob, at: Time.current.to_f + 600.seconds, args: ["DelayedMailer", "test_message", "deliver_now", 1, 2, 3]) do
+ assert_performed_with(job: ActionMailer::DeliveryJob, at: Time.current.to_f + 600, args: ["DelayedMailer", "test_message", "deliver_now", 1, 2, 3]) do
@mail.deliver_later wait: 600.seconds
end
end
diff --git a/actionview/test/template/date_helper_test.rb b/actionview/test/template/date_helper_test.rb
index bfd3ecd6fd..d257147e1f 100644
--- a/actionview/test/template/date_helper_test.rb
+++ b/actionview/test/template/date_helper_test.rb
@@ -832,7 +832,7 @@ class DateHelperTest < ActionView::TestCase
def test_select_date_with_too_big_range_between_start_year_and_end_year
assert_raise(ArgumentError) { select_date(Time.mktime(2003, 8, 16), start_year: 2000, end_year: 20000, prefix: "date[first]", order: [:month, :day, :year]) }
- assert_raise(ArgumentError) { select_date(Time.mktime(2003, 8, 16), start_year: Date.today.year - 100.years, end_year: 2000, prefix: "date[first]", order: [:month, :day, :year]) }
+ assert_raise(ArgumentError) { select_date(Time.mktime(2003, 8, 16), start_year: 100, end_year: 2000, prefix: "date[first]", order: [:month, :day, :year]) }
end
def test_select_date_can_have_more_then_1000_years_interval_if_forced_via_parameter
diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md
index 2999820c42..6ca227f40a 100644
--- a/activesupport/CHANGELOG.md
+++ b/activesupport/CHANGELOG.md
@@ -1,3 +1,55 @@
+* Deprecate implicit coercion of `ActiveSupport::Duration`
+
+ Currently `ActiveSupport::Duration` implicitly converts to a seconds
+ value when used in a calculation except for the explicit examples of
+ addition and subtraction where the duration is the receiver, e.g:
+
+ >> 2 * 1.day
+ => 172800
+
+ This results in lots of confusion especially when using durations
+ with dates because adding/subtracting a value from a date treats
+ integers as a day and not a second, e.g:
+
+ >> Date.today
+ => Wed, 01 Mar 2017
+ >> Date.today + 2 * 1.day
+ => Mon, 10 Apr 2490
+
+ To fix this we're implementing `coerce` so that we can provide a
+ deprecation warning with the intent of removing the implicit coercion
+ in Rails 5.2, e.g:
+
+ >> 2 * 1.day
+ DEPRECATION WARNING: Implicit coercion of ActiveSupport::Duration
+ to a Numeric is deprecated and will raise a TypeError in Rails 5.2.
+ => 172800
+
+ In Rails 5.2 it will raise `TypeError`, e.g:
+
+ >> 2 * 1.day
+ TypeError: ActiveSupport::Duration can't be coerced into Integer
+
+ This is the same behavior as with other types in Ruby, e.g:
+
+ >> 2 * "foo"
+ TypeError: String can't be coerced into Integer
+ >> "foo" * 2
+ => "foofoo"
+
+ As part of this deprecation add `*` and `/` methods to `AS::Duration`
+ so that calculations that keep the duration as the receiver work
+ correctly whether the final receiver is a `Date` or `Time`, e.g:
+
+ >> Date.today
+ => Wed, 01 Mar 2017
+ >> Date.today + 1.day * 2
+ => Fri, 03 Mar 2017
+
+ Fixes #27457.
+
+ *Andrew White*
+
* Update `DateTime#change` to support `:usec` and `:nsec` options.
Adding support for these options now allows us to update the `DateTime#end_of`
diff --git a/activesupport/lib/active_support/cache/mem_cache_store.rb b/activesupport/lib/active_support/cache/mem_cache_store.rb
index e09cee3335..5eee04a34e 100644
--- a/activesupport/lib/active_support/cache/mem_cache_store.rb
+++ b/activesupport/lib/active_support/cache/mem_cache_store.rb
@@ -156,7 +156,7 @@ module ActiveSupport
expires_in = options[:expires_in].to_i
if expires_in > 0 && !options[:raw]
# Set the memcache expire a few minutes in the future to support race condition ttls on read
- expires_in += 5.minutes
+ expires_in += 300
end
rescue_error_with false do
@data.send(method, key, value, expires_in, options)
diff --git a/activesupport/lib/active_support/duration.rb b/activesupport/lib/active_support/duration.rb
index 70cf78519d..b92891c511 100644
--- a/activesupport/lib/active_support/duration.rb
+++ b/activesupport/lib/active_support/duration.rb
@@ -1,5 +1,7 @@
require "active_support/core_ext/array/conversions"
require "active_support/core_ext/object/acts_like"
+require "active_support/core_ext/string/filters"
+require "active_support/deprecation"
module ActiveSupport
# Provides accurate date and time measurements using Date#advance and
@@ -88,6 +90,25 @@ module ActiveSupport
@parts.default = 0
end
+ def coerce(other) #:nodoc:
+ ActiveSupport::Deprecation.warn(<<-MSG.squish)
+ Implicit coercion of ActiveSupport::Duration to a Numeric
+ is deprecated and will raise a TypeError in Rails 5.2.
+ MSG
+
+ [other, value]
+ end
+
+ # Compares one Duration with another or a Numeric to this Duration.
+ # Numeric values are treated as seconds.
+ def <=>(other)
+ if Duration === other
+ value <=> other.value
+ elsif Numeric === other
+ value <=> other
+ end
+ end
+
# Adds another Duration or a Numeric to this Duration. Numeric values
# are treated as seconds.
def +(other)
@@ -109,6 +130,24 @@ module ActiveSupport
self + (-other)
end
+ # Multiplies this Duration by a Numeric and returns a new Duration.
+ def *(other)
+ if Numeric === other
+ Duration.new(value * other, parts.map { |type, number| [type, number * other] })
+ else
+ value * other
+ end
+ end
+
+ # Devides this Duration by a Numeric and returns a new Duration.
+ def /(other)
+ if Numeric === other
+ Duration.new(value / other, parts.map { |type, number| [type, number / other] })
+ else
+ value / other
+ end
+ end
+
def -@ #:nodoc:
Duration.new(-value, parts.map { |type, number| [type, -number] })
end
@@ -212,8 +251,6 @@ module ActiveSupport
ISO8601Serializer.new(self, precision: precision).serialize
end
- delegate :<=>, to: :value
-
private
def sum(sign, time = ::Time.current)
diff --git a/activesupport/test/core_ext/duration_test.rb b/activesupport/test/core_ext/duration_test.rb
index 6facb04f1f..2b1a715b7a 100644
--- a/activesupport/test/core_ext/duration_test.rb
+++ b/activesupport/test/core_ext/duration_test.rb
@@ -84,8 +84,46 @@ class DurationTest < ActiveSupport::TestCase
assert_nothing_raised { Date.today - Date.today }
end
+ def test_plus
+ assert_equal 2.seconds, 1.second + 1.second
+ assert_instance_of ActiveSupport::Duration, 1.second + 1.second
+ assert_equal 2.seconds, 1.second + 1
+ assert_instance_of ActiveSupport::Duration, 1.second + 1
+ end
+
+ def test_minus
+ assert_equal 1.second, 2.seconds - 1.second
+ assert_instance_of ActiveSupport::Duration, 2.seconds - 1.second
+ assert_equal 1.second, 2.seconds - 1
+ assert_instance_of ActiveSupport::Duration, 2.seconds - 1
+ end
+
+ def test_multiply
+ assert_equal 7.days, 1.day * 7
+ assert_instance_of ActiveSupport::Duration, 1.day * 7
+
+ assert_deprecated do
+ assert_equal 86400, 1.day * 1.second
+ end
+ end
+
+ def test_divide
+ assert_equal 1.day, 7.days / 7
+ assert_instance_of ActiveSupport::Duration, 7.days / 7
+
+ assert_deprecated do
+ assert_equal 1, 1.day / 1.day
+ end
+ end
+
+ def test_date_added_with_multiplied_duration
+ assert_equal Date.civil(2017, 1, 3), Date.civil(2017, 1, 1) + 1.day * 2
+ end
+
def test_plus_with_time
- assert_equal 1 + 1.second, 1.second + 1, "Duration + Numeric should == Numeric + Duration"
+ assert_deprecated do
+ assert_equal 1 + 1.second, 1.second + 1, "Duration + Numeric should == Numeric + Duration"
+ end
end
def test_time_plus_duration_returns_same_time_datatype
@@ -104,6 +142,13 @@ class DurationTest < ActiveSupport::TestCase
assert_equal 'expected a time or date, got ""', e.message, "ensure ArgumentError is not being raised by dependencies.rb"
end
+ def test_implicit_coercion_is_deprecated
+ assert_deprecated { 1 + 1.second }
+ assert_deprecated { 1 - 1.second }
+ assert_deprecated { 1 * 1.second }
+ assert_deprecated { 1 / 1.second }
+ end
+
def test_fractional_weeks
assert_equal((86400 * 7) * 1.5, 1.5.weeks)
assert_equal((86400 * 7) * 1.7, 1.7.weeks)
@@ -241,13 +286,20 @@ class DurationTest < ActiveSupport::TestCase
def test_comparable
assert_equal(-1, (0.seconds <=> 1.second))
assert_equal(-1, (1.second <=> 1.minute))
- assert_equal(-1, (1 <=> 1.minute))
+
+ assert_deprecated do
+ assert_equal(-1, (1 <=> 1.minute))
+ end
+
assert_equal(0, (0.seconds <=> 0.seconds))
assert_equal(0, (0.seconds <=> 0.minutes))
assert_equal(0, (1.second <=> 1.second))
assert_equal(1, (1.second <=> 0.second))
assert_equal(1, (1.minute <=> 1.second))
- assert_equal(1, (61 <=> 1.minute))
+
+ assert_deprecated do
+ assert_equal(1, (61 <=> 1.minute))
+ end
end
def test_twelve_months_equals_one_year