Add missing support for modulo operations on durations
Rails 5.1 introduce an `ActiveSupport::Duration::Scalar` class as a wrapper around a numeric value as a way of ensuring a duration was the outcome of an expression. However the implementation was missing support for modulo operations. This commit adds support for those operations and should result in a duration being returned from expressions involving them. Fixes #29603 and #29743.
This commit is contained in:
parent
c6a28b290a
commit
a54e13bd2e
@ -1,3 +1,25 @@
|
|||||||
|
* Fix modulo operations involving durations
|
||||||
|
|
||||||
|
Rails 5.1 introduce an `ActiveSupport::Duration::Scalar` class as a wrapper
|
||||||
|
around a numeric value as a way of ensuring a duration was the outcome of
|
||||||
|
an expression. However the implementation was missing support for modulo
|
||||||
|
operations. This support has now been added and should result in a duration
|
||||||
|
being returned from expressions involving modulo operations.
|
||||||
|
|
||||||
|
Prior to Rails 5.1:
|
||||||
|
|
||||||
|
5.minutes % 2.minutes
|
||||||
|
=> 60
|
||||||
|
|
||||||
|
Now:
|
||||||
|
|
||||||
|
5.minutes % 2.minutes
|
||||||
|
=> 1 minute
|
||||||
|
|
||||||
|
Fixes #29603 and #29743.
|
||||||
|
|
||||||
|
*Sayan Chakraborty*, *Andrew White*
|
||||||
|
|
||||||
* Fix division where a duration is the denominator
|
* Fix division where a duration is the denominator
|
||||||
|
|
||||||
PR #29163 introduced a change in behavior when a duration was the denominator
|
PR #29163 introduced a change in behavior when a duration was the denominator
|
||||||
|
@ -82,6 +82,14 @@ def /(other)
|
|||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def %(other)
|
||||||
|
if Duration === other
|
||||||
|
Duration.build(value % other.value)
|
||||||
|
else
|
||||||
|
calculate(:%, other)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
private
|
private
|
||||||
def calculate(op, other)
|
def calculate(op, other)
|
||||||
if Scalar === other
|
if Scalar === other
|
||||||
@ -115,6 +123,8 @@ def raise_type_error(other)
|
|||||||
years: SECONDS_PER_YEAR
|
years: SECONDS_PER_YEAR
|
||||||
}.freeze
|
}.freeze
|
||||||
|
|
||||||
|
PARTS = [:years, :months, :weeks, :days, :hours, :minutes, :seconds].freeze
|
||||||
|
|
||||||
attr_accessor :value, :parts
|
attr_accessor :value, :parts
|
||||||
|
|
||||||
autoload :ISO8601Parser, "active_support/duration/iso8601_parser"
|
autoload :ISO8601Parser, "active_support/duration/iso8601_parser"
|
||||||
@ -165,6 +175,30 @@ def years(value) #:nodoc:
|
|||||||
new(value * SECONDS_PER_YEAR, [[:years, value]])
|
new(value * SECONDS_PER_YEAR, [[:years, value]])
|
||||||
end
|
end
|
||||||
|
|
||||||
|
# Creates a new Duration from a seconds value that is converted
|
||||||
|
# to the individual parts:
|
||||||
|
#
|
||||||
|
# ActiveSupport::Duration.build(31556952).parts # => {:years=>1}
|
||||||
|
# ActiveSupport::Duration.build(2716146).parts # => {:months=>1, :days=>1}
|
||||||
|
#
|
||||||
|
def build(value)
|
||||||
|
parts = {}
|
||||||
|
remainder = value.to_f
|
||||||
|
|
||||||
|
PARTS.each do |part|
|
||||||
|
unless part == :seconds
|
||||||
|
part_in_seconds = PARTS_IN_SECONDS[part]
|
||||||
|
parts[part] = remainder.div(part_in_seconds)
|
||||||
|
remainder = (remainder % part_in_seconds).round(9)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
parts[:seconds] = remainder
|
||||||
|
parts.reject! { |k, v| v.zero? }
|
||||||
|
|
||||||
|
new(value, parts)
|
||||||
|
end
|
||||||
|
|
||||||
private
|
private
|
||||||
|
|
||||||
def calculate_total_seconds(parts)
|
def calculate_total_seconds(parts)
|
||||||
@ -242,6 +276,18 @@ def /(other)
|
|||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
# Returns the modulo of this Duration by another Duration or Numeric.
|
||||||
|
# Numeric values are treated as seconds.
|
||||||
|
def %(other)
|
||||||
|
if Duration === other || Scalar === other
|
||||||
|
Duration.build(value % other.value)
|
||||||
|
elsif Numeric === other
|
||||||
|
Duration.build(value % other)
|
||||||
|
else
|
||||||
|
raise_type_error(other)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
def -@ #:nodoc:
|
def -@ #:nodoc:
|
||||||
Duration.new(-value, parts.map { |type, number| [type, -number] })
|
Duration.new(-value, parts.map { |type, number| [type, -number] })
|
||||||
end
|
end
|
||||||
@ -326,7 +372,7 @@ def ago(time = ::Time.current)
|
|||||||
def inspect #:nodoc:
|
def inspect #:nodoc:
|
||||||
parts.
|
parts.
|
||||||
reduce(::Hash.new(0)) { |h, (l, r)| h[l] += r; h }.
|
reduce(::Hash.new(0)) { |h, (l, r)| h[l] += r; h }.
|
||||||
sort_by { |unit, _ | [:years, :months, :weeks, :days, :hours, :minutes, :seconds].index(unit) }.
|
sort_by { |unit, _ | PARTS.index(unit) }.
|
||||||
map { |unit, val| "#{val} #{val == 1 ? unit.to_s.chop : unit.to_s}" }.
|
map { |unit, val| "#{val} #{val == 1 ? unit.to_s.chop : unit.to_s}" }.
|
||||||
to_sentence(locale: ::I18n.default_locale)
|
to_sentence(locale: ::I18n.default_locale)
|
||||||
end
|
end
|
||||||
|
@ -111,7 +111,44 @@ def test_multiply
|
|||||||
def test_divide
|
def test_divide
|
||||||
assert_equal 1.day, 7.days / 7
|
assert_equal 1.day, 7.days / 7
|
||||||
assert_instance_of ActiveSupport::Duration, 7.days / 7
|
assert_instance_of ActiveSupport::Duration, 7.days / 7
|
||||||
|
|
||||||
|
assert_equal 1.hour, 1.day / 24
|
||||||
|
assert_instance_of ActiveSupport::Duration, 1.day / 24
|
||||||
|
|
||||||
|
assert_equal 24, 86400 / 1.hour
|
||||||
|
assert_kind_of Integer, 86400 / 1.hour
|
||||||
|
|
||||||
|
assert_equal 24, 1.day / 1.hour
|
||||||
|
assert_kind_of Integer, 1.day / 1.hour
|
||||||
|
|
||||||
assert_equal 1, 1.day / 1.day
|
assert_equal 1, 1.day / 1.day
|
||||||
|
assert_kind_of Integer, 1.day / 1.hour
|
||||||
|
end
|
||||||
|
|
||||||
|
def test_modulo
|
||||||
|
assert_equal 1.minute, 5.minutes % 120
|
||||||
|
assert_instance_of ActiveSupport::Duration, 5.minutes % 120
|
||||||
|
|
||||||
|
assert_equal 1.minute, 5.minutes % 2.minutes
|
||||||
|
assert_instance_of ActiveSupport::Duration, 5.minutes % 2.minutes
|
||||||
|
|
||||||
|
assert_equal 1.minute, 5.minutes % 120.seconds
|
||||||
|
assert_instance_of ActiveSupport::Duration, 5.minutes % 120.seconds
|
||||||
|
|
||||||
|
assert_equal 5.minutes, 5.minutes % 1.hour
|
||||||
|
assert_instance_of ActiveSupport::Duration, 5.minutes % 1.hour
|
||||||
|
|
||||||
|
assert_equal 1.day, 36.days % 604800
|
||||||
|
assert_instance_of ActiveSupport::Duration, 36.days % 604800
|
||||||
|
|
||||||
|
assert_equal 1.day, 36.days % 7.days
|
||||||
|
assert_instance_of ActiveSupport::Duration, 36.days % 7.days
|
||||||
|
|
||||||
|
assert_equal 800.seconds, 8000 % 1.hour
|
||||||
|
assert_instance_of ActiveSupport::Duration, 8000 % 1.hour
|
||||||
|
|
||||||
|
assert_equal 1.month, 13.months % 1.year
|
||||||
|
assert_instance_of ActiveSupport::Duration, 13.months % 1.year
|
||||||
end
|
end
|
||||||
|
|
||||||
def test_date_added_with_multiplied_duration
|
def test_date_added_with_multiplied_duration
|
||||||
@ -411,8 +448,6 @@ def test_scalar_divide
|
|||||||
assert_instance_of ActiveSupport::Duration::Scalar, scalar / 2
|
assert_instance_of ActiveSupport::Duration::Scalar, scalar / 2
|
||||||
assert_equal 10, 100.seconds / scalar
|
assert_equal 10, 100.seconds / scalar
|
||||||
assert_instance_of ActiveSupport::Duration, 100.seconds / scalar
|
assert_instance_of ActiveSupport::Duration, 100.seconds / scalar
|
||||||
assert_equal 20, 2.seconds * scalar
|
|
||||||
assert_instance_of ActiveSupport::Duration, 2.seconds * scalar
|
|
||||||
assert_equal 5, scalar / 2.seconds
|
assert_equal 5, scalar / 2.seconds
|
||||||
assert_kind_of Integer, scalar / 2.seconds
|
assert_kind_of Integer, scalar / 2.seconds
|
||||||
|
|
||||||
@ -423,6 +458,31 @@ def test_scalar_divide
|
|||||||
assert_equal "no implicit conversion of String into ActiveSupport::Duration::Scalar", exception.message
|
assert_equal "no implicit conversion of String into ActiveSupport::Duration::Scalar", exception.message
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def test_scalar_modulo
|
||||||
|
scalar = ActiveSupport::Duration::Scalar.new(10)
|
||||||
|
|
||||||
|
assert_equal 1, 31 % scalar
|
||||||
|
assert_instance_of ActiveSupport::Duration::Scalar, 31 % scalar
|
||||||
|
assert_equal 1, scalar % 3
|
||||||
|
assert_instance_of ActiveSupport::Duration::Scalar, scalar % 3
|
||||||
|
assert_equal 1, 31.seconds % scalar
|
||||||
|
assert_instance_of ActiveSupport::Duration, 31.seconds % scalar
|
||||||
|
assert_equal 1, scalar % 3.seconds
|
||||||
|
assert_instance_of ActiveSupport::Duration, scalar % 3.seconds
|
||||||
|
|
||||||
|
exception = assert_raises(TypeError) do
|
||||||
|
scalar % "foo"
|
||||||
|
end
|
||||||
|
|
||||||
|
assert_equal "no implicit conversion of String into ActiveSupport::Duration::Scalar", exception.message
|
||||||
|
end
|
||||||
|
|
||||||
|
def test_scalar_modulo_parts
|
||||||
|
scalar = ActiveSupport::Duration::Scalar.new(82800)
|
||||||
|
assert_equal({ hours: 1 }, (scalar % 2.hours).parts)
|
||||||
|
assert_equal(3600, (scalar % 2.hours).value)
|
||||||
|
end
|
||||||
|
|
||||||
def test_twelve_months_equals_one_year
|
def test_twelve_months_equals_one_year
|
||||||
assert_equal 12.months, 1.year
|
assert_equal 12.months, 1.year
|
||||||
end
|
end
|
||||||
@ -431,17 +491,6 @@ def test_thirty_days_does_not_equal_one_month
|
|||||||
assert_not_equal 30.days, 1.month
|
assert_not_equal 30.days, 1.month
|
||||||
end
|
end
|
||||||
|
|
||||||
def test_division
|
|
||||||
assert_equal 1.hour, 1.day / 24
|
|
||||||
assert_instance_of ActiveSupport::Duration, 1.day / 24
|
|
||||||
|
|
||||||
assert_equal 24, 86400 / 1.hour
|
|
||||||
assert_kind_of Integer, 86400 / 1.hour
|
|
||||||
|
|
||||||
assert_equal 24, 1.day / 1.hour
|
|
||||||
assert_kind_of Integer, 1.day / 1.hour
|
|
||||||
end
|
|
||||||
|
|
||||||
def test_adding_one_month_maintains_day_of_month
|
def test_adding_one_month_maintains_day_of_month
|
||||||
(1..11).each do |month|
|
(1..11).each do |month|
|
||||||
[1, 14, 28].each do |day|
|
[1, 14, 28].each do |day|
|
||||||
|
Loading…
Reference in New Issue
Block a user