From 629dde297ce5e4b7614fbb0218f2593effaf7e28 Mon Sep 17 00:00:00 2001 From: Paul Sadauskas Date: Mon, 11 Jul 2016 13:45:04 -0600 Subject: [PATCH] AS::Duration should serialize empty values correctly. (#25656) The current implementation serializes zero-length durations incorrectly (it serializes as `"-P"`), and cannot un-serialize itself: ``` [1] pry(main)> ActiveSupport::Duration.parse(0.minutes.iso8601) ActiveSupport::Duration::ISO8601Parser::ParsingError: Invalid ISO 8601 duration: "-P" is empty duration from /Users/rando/.gem/ruby/2.3.1/gems/activesupport-5.0.0/lib/active_support/duration/iso8601_parser.rb:96:in `raise_parsing_error' ``` Postgres empty intervals are serialized as `"PT0S"`, which is also parseable by the Duration deserializer, so I've modified the `ISO8601Serializer` to do the same. Additionally, the `#normalize` function returned a negative sign if `parts` was blank (all zero). Even though this fix does not rely on the sign, I've gone ahead and corrected that, too, in case a future refactoring of `#serialize` uses it. --- .../lib/active_support/duration/iso8601_serializer.rb | 4 +++- activesupport/test/core_ext/duration_test.rb | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/activesupport/lib/active_support/duration/iso8601_serializer.rb b/activesupport/lib/active_support/duration/iso8601_serializer.rb index 05c6a083a9..eb8136e208 100644 --- a/activesupport/lib/active_support/duration/iso8601_serializer.rb +++ b/activesupport/lib/active_support/duration/iso8601_serializer.rb @@ -12,8 +12,10 @@ module ActiveSupport # Builds and returns output string. def serialize - output = 'P' parts, sign = normalize + return "PT0S".freeze if parts.empty? + + output = 'P' output << "#{parts[:years]}Y" if parts.key?(:years) output << "#{parts[:months]}M" if parts.key?(:months) output << "#{parts[:weeks]}W" if parts.key?(:weeks) diff --git a/activesupport/test/core_ext/duration_test.rb b/activesupport/test/core_ext/duration_test.rb index 502e2811fa..a24915dfef 100644 --- a/activesupport/test/core_ext/duration_test.rb +++ b/activesupport/test/core_ext/duration_test.rb @@ -279,6 +279,7 @@ class DurationTest < ActiveSupport::TestCase ['PT1S', 1.second ], ['PT1.4S', (1.4).seconds ], ['P1Y1M1DT1H', 1.year + 1.month + 1.day + 1.hour], + ['PT0S', 0.minutes ], ] expectations.each do |expected_output, duration| assert_equal expected_output, duration.iso8601, expected_output.inspect