From ffc1e5f889a9a3dce91314fed32164de660d1684 Mon Sep 17 00:00:00 2001 From: Caleb Buxton Date: Fri, 10 Dec 2021 11:03:04 -0800 Subject: [PATCH] fix: equivalent negative durations add to the same time (#43795) * bug: illustrate negative durations don't add to the same time * fix: equivalent negative durations add to the same time Co-authored-by: Caleb Co-authored-by: Braden Staudacher * Updates CHANGELOG with fix to `ActiveSupport::Duration.build` --- activesupport/CHANGELOG.md | 7 +++++++ activesupport/lib/active_support/duration.rb | 7 ++++--- activesupport/test/core_ext/duration_test.rb | 11 +++++++++++ 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index 04e45a01e2a..86e54924393 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,10 @@ +* Fix `ActiveSupport::Duration.build` to support negative values. + The algorithm to collect the `parts` of the `ActiveSupport::Duration` + ignored the sign of the `value` and accumulated incorrect part values. This + impacted `ActiveSupport::Duration#sum` (which is dependent on `parts`) but + not `ActiveSupport::Duration#eql?` (which is dependent on `value`). + + *Caleb Buxton*, *Braden Staudacher* Please check [7-0-stable](https://github.com/rails/rails/blob/7-0-stable/activesupport/CHANGELOG.md) for previous changes. diff --git a/activesupport/lib/active_support/duration.rb b/activesupport/lib/active_support/duration.rb index a393c9a07ab..19986d1d0c9 100644 --- a/activesupport/lib/active_support/duration.rb +++ b/activesupport/lib/active_support/duration.rb @@ -191,13 +191,14 @@ module ActiveSupport end parts = {} - remainder = value.round(9) + remainder_sign = value <=> 0 + remainder = value.round(9).abs variable = false PARTS.each do |part| unless part == :seconds part_in_seconds = PARTS_IN_SECONDS[part] - parts[part] = remainder.div(part_in_seconds) + parts[part] = remainder.div(part_in_seconds) * remainder_sign remainder %= part_in_seconds unless parts[part].zero? @@ -206,7 +207,7 @@ module ActiveSupport end end unless value == 0 - parts[:seconds] = remainder + parts[:seconds] = remainder * remainder_sign new(value, parts, variable) end diff --git a/activesupport/test/core_ext/duration_test.rb b/activesupport/test/core_ext/duration_test.rb index 5f38ccf2905..f957daa8948 100644 --- a/activesupport/test/core_ext/duration_test.rb +++ b/activesupport/test/core_ext/duration_test.rb @@ -752,6 +752,17 @@ class DurationTest < ActiveSupport::TestCase assert (1.day + 12.hours).variable? end + def test_duration_symmetry + time = Time.parse("Dec 7, 2021") + expected_time = Time.parse("2021-12-06 23:59:59") + + assert_equal expected_time, time + -1.second + assert_equal expected_time, time + ActiveSupport::Duration.build(1) * -1 + assert_equal expected_time, time + -ActiveSupport::Duration.build(1) + assert_equal expected_time, time + ActiveSupport::Duration::Scalar.new(-1) + assert_equal expected_time, time + ActiveSupport::Duration.build(-1) + end + private def eastern_time_zone if Gem.win_platform?