Merge pull request #27610 from Envek/fix_and_speed_up_duration_parsing
Fix inconsistent parsing of Durations with both months and years
This commit is contained in:
commit
033f654027
@ -130,7 +130,7 @@ def assert_hsts(expected, url: "https://example.org", hsts: { subdomains: true }
|
|||||||
end
|
end
|
||||||
|
|
||||||
test ":expires supports AS::Duration arguments" do
|
test ":expires supports AS::Duration arguments" do
|
||||||
assert_hsts "max-age=31557600; includeSubDomains", hsts: { expires: 1.year }
|
assert_hsts "max-age=31556952; includeSubDomains", hsts: { expires: 1.year }
|
||||||
end
|
end
|
||||||
|
|
||||||
test "include subdomains" do
|
test "include subdomains" do
|
||||||
|
@ -1,3 +1,28 @@
|
|||||||
|
* Fix inconsistent results when parsing large durations and constructing durations from code
|
||||||
|
|
||||||
|
ActiveSupport::Duration.parse('P3Y') == 3.years # It should be true
|
||||||
|
|
||||||
|
Duration parsing made independent from any moment of time:
|
||||||
|
Fixed length in seconds is assigned to each duration part during parsing.
|
||||||
|
|
||||||
|
Changed duration of months and years in seconds to more accurate and logical:
|
||||||
|
|
||||||
|
1. The value of 365.2425 days in Gregorian year is more accurate
|
||||||
|
as it accounts for every 400th non-leap year.
|
||||||
|
|
||||||
|
2. Month's length is bound to year's duration, which makes
|
||||||
|
sensible comparisons like `12.months == 1.year` to be `true`
|
||||||
|
and nonsensical ones like `30.days == 1.month` to be `false`.
|
||||||
|
|
||||||
|
Calculations on times and dates with durations shouldn't be affected as
|
||||||
|
duration's numeric value isn't used in calculations, only parts are used.
|
||||||
|
|
||||||
|
Methods on `Numeric` like `2.days` now use these predefined durations
|
||||||
|
to avoid duplicating of duration constants through the codebase and
|
||||||
|
eliminate creation of intermediate durations.
|
||||||
|
|
||||||
|
*Andrey Novikov, Andrew White*
|
||||||
|
|
||||||
* Change return value of `Rational#duplicable?`, `ComplexClass#duplicable?`
|
* Change return value of `Rational#duplicable?`, `ComplexClass#duplicable?`
|
||||||
to false.
|
to false.
|
||||||
|
|
||||||
|
@ -18,12 +18,12 @@ class Integer
|
|||||||
# # equivalent to Time.now.advance(months: 4, years: 5)
|
# # equivalent to Time.now.advance(months: 4, years: 5)
|
||||||
# (4.months + 5.years).from_now
|
# (4.months + 5.years).from_now
|
||||||
def months
|
def months
|
||||||
ActiveSupport::Duration.new(self * 30.days, [[:months, self]])
|
ActiveSupport::Duration.new(self * ActiveSupport::Duration::PARTS_IN_SECONDS[:months].to_i, [[:months, self]])
|
||||||
end
|
end
|
||||||
alias :month :months
|
alias :month :months
|
||||||
|
|
||||||
def years
|
def years
|
||||||
ActiveSupport::Duration.new(self * 365.25.days.to_i, [[:years, self]])
|
ActiveSupport::Duration.new(self * ActiveSupport::Duration::PARTS_IN_SECONDS[:years].to_i, [[:years, self]])
|
||||||
end
|
end
|
||||||
alias :year :years
|
alias :year :years
|
||||||
end
|
end
|
||||||
|
@ -27,7 +27,7 @@ def seconds
|
|||||||
#
|
#
|
||||||
# 2.minutes # => 2 minutes
|
# 2.minutes # => 2 minutes
|
||||||
def minutes
|
def minutes
|
||||||
ActiveSupport::Duration.new(self * 60, [[:minutes, self]])
|
ActiveSupport::Duration.new(self * ActiveSupport::Duration::PARTS_IN_SECONDS[:minutes], [[:minutes, self]])
|
||||||
end
|
end
|
||||||
alias :minute :minutes
|
alias :minute :minutes
|
||||||
|
|
||||||
@ -35,7 +35,7 @@ def minutes
|
|||||||
#
|
#
|
||||||
# 2.hours # => 2 hours
|
# 2.hours # => 2 hours
|
||||||
def hours
|
def hours
|
||||||
ActiveSupport::Duration.new(self * 3600, [[:hours, self]])
|
ActiveSupport::Duration.new(self * ActiveSupport::Duration::PARTS_IN_SECONDS[:hours], [[:hours, self]])
|
||||||
end
|
end
|
||||||
alias :hour :hours
|
alias :hour :hours
|
||||||
|
|
||||||
@ -43,7 +43,7 @@ def hours
|
|||||||
#
|
#
|
||||||
# 2.days # => 2 days
|
# 2.days # => 2 days
|
||||||
def days
|
def days
|
||||||
ActiveSupport::Duration.new(self * 24.hours, [[:days, self]])
|
ActiveSupport::Duration.new(self * ActiveSupport::Duration::PARTS_IN_SECONDS[:days], [[:days, self]])
|
||||||
end
|
end
|
||||||
alias :day :days
|
alias :day :days
|
||||||
|
|
||||||
@ -51,7 +51,7 @@ def days
|
|||||||
#
|
#
|
||||||
# 2.weeks # => 2 weeks
|
# 2.weeks # => 2 weeks
|
||||||
def weeks
|
def weeks
|
||||||
ActiveSupport::Duration.new(self * 7.days, [[:weeks, self]])
|
ActiveSupport::Duration.new(self * ActiveSupport::Duration::PARTS_IN_SECONDS[:weeks], [[:weeks, self]])
|
||||||
end
|
end
|
||||||
alias :week :weeks
|
alias :week :weeks
|
||||||
|
|
||||||
@ -59,7 +59,7 @@ def weeks
|
|||||||
#
|
#
|
||||||
# 2.fortnights # => 4 weeks
|
# 2.fortnights # => 4 weeks
|
||||||
def fortnights
|
def fortnights
|
||||||
ActiveSupport::Duration.new(self * 2.weeks, [[:weeks, self * 2]])
|
ActiveSupport::Duration.new(self * 2 * ActiveSupport::Duration::PARTS_IN_SECONDS[:weeks], [[:weeks, self * 2]])
|
||||||
end
|
end
|
||||||
alias :fortnight :fortnights
|
alias :fortnight :fortnights
|
||||||
|
|
||||||
|
@ -7,7 +7,15 @@ module ActiveSupport
|
|||||||
#
|
#
|
||||||
# 1.month.ago # equivalent to Time.now.advance(months: -1)
|
# 1.month.ago # equivalent to Time.now.advance(months: -1)
|
||||||
class Duration
|
class Duration
|
||||||
EPOCH = ::Time.utc(2000)
|
PARTS_IN_SECONDS = {
|
||||||
|
seconds: 1, # Used in parse method for ease of handling part hashes with seconds
|
||||||
|
minutes: 60,
|
||||||
|
hours: 60 * 60,
|
||||||
|
days: 24 * 60 * 60,
|
||||||
|
weeks: 7 * 24 * 60 * 60,
|
||||||
|
months: ((365.2425 / 12) * 24 * 60 * 60).ceil, # 1 year is always 12 months long, no sense to bind it to days
|
||||||
|
years: (365.2425 * 24 * 60 * 60).ceil, # Gregorian year length to account for the every 400 non-leap year
|
||||||
|
}.freeze
|
||||||
|
|
||||||
attr_accessor :value, :parts
|
attr_accessor :value, :parts
|
||||||
|
|
||||||
@ -78,14 +86,14 @@ def to_s
|
|||||||
# 1.day.to_i # => 86400
|
# 1.day.to_i # => 86400
|
||||||
#
|
#
|
||||||
# Note that this conversion makes some assumptions about the
|
# Note that this conversion makes some assumptions about the
|
||||||
# duration of some periods, e.g. months are always 30 days
|
# duration of some periods, e.g. months are always 1/12 of year
|
||||||
# and years are 365.25 days:
|
# and years are 365.2425 days:
|
||||||
#
|
#
|
||||||
# # equivalent to 30.days.to_i
|
# # equivalent to (1.year / 12).to_i
|
||||||
# 1.month.to_i # => 2592000
|
# 1.month.to_i # => 2629746
|
||||||
#
|
#
|
||||||
# # equivalent to 365.25.days.to_i
|
# # equivalent to 365.2425.days.to_i
|
||||||
# 1.year.to_i # => 31557600
|
# 1.year.to_i # => 31556952
|
||||||
#
|
#
|
||||||
# In such cases, Ruby's core
|
# In such cases, Ruby's core
|
||||||
# Date[http://ruby-doc.org/stdlib/libdoc/date/rdoc/Date.html] and
|
# Date[http://ruby-doc.org/stdlib/libdoc/date/rdoc/Date.html] and
|
||||||
@ -148,7 +156,10 @@ def respond_to_missing?(method, include_private = false) #:nodoc:
|
|||||||
# If invalid string is provided, it will raise +ActiveSupport::Duration::ISO8601Parser::ParsingError+.
|
# If invalid string is provided, it will raise +ActiveSupport::Duration::ISO8601Parser::ParsingError+.
|
||||||
def self.parse(iso8601duration)
|
def self.parse(iso8601duration)
|
||||||
parts = ISO8601Parser.new(iso8601duration).parse!
|
parts = ISO8601Parser.new(iso8601duration).parse!
|
||||||
new(EPOCH.advance(parts) - EPOCH, parts)
|
total_seconds = parts.inject(0) do |total, (part, value)|
|
||||||
|
total + value * PARTS_IN_SECONDS[part]
|
||||||
|
end
|
||||||
|
new(total_seconds, parts)
|
||||||
end
|
end
|
||||||
|
|
||||||
# Build ISO 8601 Duration string for this duration.
|
# Build ISO 8601 Duration string for this duration.
|
||||||
|
@ -340,6 +340,7 @@ def test_iso8601_parsing_across_autumn_dst_boundary
|
|||||||
travel_to Time.utc(2016, 11, 4) do
|
travel_to Time.utc(2016, 11, 4) do
|
||||||
assert_equal 604800, ActiveSupport::Duration.parse("P7D").to_i
|
assert_equal 604800, ActiveSupport::Duration.parse("P7D").to_i
|
||||||
assert_equal 604800, ActiveSupport::Duration.parse("P1W").to_i
|
assert_equal 604800, ActiveSupport::Duration.parse("P1W").to_i
|
||||||
|
assert_equal ActiveSupport::Duration.parse(3.years.iso8601).to_i, 3.years.to_i
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
@ -12,7 +12,7 @@ def setup
|
|||||||
10.minutes => 600,
|
10.minutes => 600,
|
||||||
1.hour + 15.minutes => 4500,
|
1.hour + 15.minutes => 4500,
|
||||||
2.days + 4.hours + 30.minutes => 189000,
|
2.days + 4.hours + 30.minutes => 189000,
|
||||||
5.years + 1.month + 1.fortnight => 161589600
|
5.years + 1.month + 1.fortnight => 161624106
|
||||||
}
|
}
|
||||||
end
|
end
|
||||||
|
|
||||||
@ -61,10 +61,10 @@ def test_chaining_duration_operations
|
|||||||
end
|
end
|
||||||
|
|
||||||
def test_duration_after_conversion_is_no_longer_accurate
|
def test_duration_after_conversion_is_no_longer_accurate
|
||||||
assert_equal 30.days.to_i.seconds.since(@now), 1.month.to_i.seconds.since(@now)
|
assert_equal (1.year / 12).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 365.2425.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 (1.year / 12).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)
|
assert_equal 365.2425.days.to_f.seconds.since(@dtnow), 1.year.to_f.seconds.since(@dtnow)
|
||||||
end
|
end
|
||||||
|
|
||||||
def test_add_one_year_to_leap_day
|
def test_add_one_year_to_leap_day
|
||||||
|
@ -683,7 +683,7 @@ Ruby instruction to be executed -- in this case, Active Support's `week` method.
|
|||||||
51: #
|
51: #
|
||||||
52: # 2.weeks # => 14 days
|
52: # 2.weeks # => 14 days
|
||||||
53: def weeks
|
53: def weeks
|
||||||
=> 54: ActiveSupport::Duration.new(self * 7.days, [[:days, self * 7]])
|
=> 54: ActiveSupport::Duration.new(self * ActiveSupport::Duration::PARTS_IN_SECONDS[:weeks], [[:weeks, self]])
|
||||||
55: end
|
55: end
|
||||||
56: alias :week :weeks
|
56: alias :week :weeks
|
||||||
57:
|
57:
|
||||||
|
Loading…
Reference in New Issue
Block a user