From 4371d53312c1c1b2947da1c4ac1c1c4483a6a07c Mon Sep 17 00:00:00 2001 From: Andrew Vit Date: Sun, 9 Jul 2017 15:11:18 -0700 Subject: [PATCH] Fix double occurrences in end of DST Simplify the method we use for calculating time changes across DST boundaries by performing arithmetic under UTC to get "absolute" values. This is a revert of the "unrolling in pairs" method introduced in c93fa7e7 for fixing #189. The method of converting to UTC and back for doing time arithmetic only applies when DST needs to be taken into consideration. For rules with less than daily intervals, both hours still need to occur. This distinction was added in the original fix. However, the method was not correctly tested for non-ActiveSupport times (local system TZ) and the advancing of time to the next occurrence was validating both the last DT hour (before fall back) and first ST hour (after fall back) causing double occurrences for the "same" hour on the clock face. Fixes #398 --- lib/ice_cube/schedule.rb | 19 ++-------------- lib/ice_cube/time_util.rb | 48 ++++++++++++++++----------------------- spec/examples/dst_spec.rb | 6 +++++ 3 files changed, 28 insertions(+), 45 deletions(-) diff --git a/lib/ice_cube/schedule.rb b/lib/ice_cube/schedule.rb index 32c2a595..4f07f843 100644 --- a/lib/ice_cube/schedule.rb +++ b/lib/ice_cube/schedule.rb @@ -437,16 +437,7 @@ def enumerate_occurrences(opening_time, closing_time = nil, options = {}) if (spans ? (t0.end_time > opening_time) : (t0 >= opening_time)) yielder << (block_given? ? yield(t0) : t0) end - break unless (t1 = next_time(t0 + 1, closing_time)) - break if closing_time && t1 > closing_time - if TimeUtil.same_clock?(t0, t1) && recurrence_rules.any?(&:dst_adjust?) - wind_back_dst - next (t1 += 1) - end - if (spans ? (t1.end_time > opening_time) : (t1 >= opening_time)) - yielder << (block_given? ? block.call(t1) : t1) - end - next (t1 += 1) + t1 = t0 + 1 end end end @@ -462,7 +453,7 @@ def next_time(time, closing_time) min_time end end - break nil unless min_time + break unless min_time next (time = min_time + 1) if exception_time?(min_time) break Occurrence.new(min_time, min_time + duration) end @@ -513,12 +504,6 @@ def recurrence_rules_with_implicit_start_occurrence end end - def wind_back_dst - recurrence_rules.each do |rule| - rule.skipped_for_dst - end - end - end end diff --git a/lib/ice_cube/time_util.rb b/lib/ice_cube/time_util.rb index 4b500790..1b95b550 100644 --- a/lib/ice_cube/time_util.rb +++ b/lib/ice_cube/time_util.rb @@ -47,7 +47,7 @@ def self.match_zone(input_time, reference) time.in_time_zone(reference.time_zone) else if reference.utc? - time.utc + time.getgm elsif reference.zone time.getlocal else @@ -260,27 +260,32 @@ class TimeWrapper def initialize(time, dst_adjust = true) @dst_adjust = dst_adjust - @time = time + @base = time + if dst_adjust + @time = Time.utc(time.year, time.month, time.day, time.hour, time.min, time.sec + time.subsec) + else + @time = time + end end - # Get the wrapper time back + # Get the wrapped time back in its original zone & format def to_time - @time + return @time unless @dst_adjust + parts = @time.year, @time.month, @time.day, @time.hour, @time.min, @time.sec + @time.subsec + TimeUtil.build_in_zone(parts, @base) end # DST-safely add an interval of time to the wrapped time def add(type, val) type = :day if type == :wday - adjust do - @time += case type - when :year then TimeUtil.days_in_n_years(@time, val) * ONE_DAY - when :month then TimeUtil.days_in_n_months(@time, val) * ONE_DAY - when :day then val * ONE_DAY - when :hour then val * ONE_HOUR - when :min then val * ONE_MINUTE - when :sec then val - end - end + @time += case type + when :year then TimeUtil.days_in_n_years(@time, val) * ONE_DAY + when :month then TimeUtil.days_in_n_months(@time, val) * ONE_DAY + when :day then val * ONE_DAY + when :hour then val * ONE_HOUR + when :min then val * ONE_MINUTE + when :sec then val + end end # Clear everything below a certain type @@ -289,25 +294,12 @@ def clear_below(type) type = :day if type == :wday CLEAR_ORDER.each do |ptype| break if ptype == type - adjust do - send(:"clear_#{ptype}") - end + send :"clear_#{ptype}" end end private - def adjust(&block) - if @dst_adjust - off = @time.utc_offset - yield - diff = off - @time.utc_offset - @time += diff if diff != 0 - else - yield - end - end - def clear_sec @time.sec > 0 ? @time -= @time.sec : @time end diff --git a/spec/examples/dst_spec.rb b/spec/examples/dst_spec.rb index f3d705fc..96944c67 100644 --- a/spec/examples/dst_spec.rb +++ b/spec/examples/dst_spec.rb @@ -270,6 +270,12 @@ expect(schedule.all_occurrences).to eq([t0, t0 + 25*ONE_HOUR, t0 + 49*ONE_HOUR]) end + it "skips double monthly occurrences from end of DST", :system_time_zone => "America/Nome" do + t0 = Time.local(2017, 10, 5, 1, 0, 0) + schedule = IceCube::Schedule.new(t0) { |s| s.rrule IceCube::Rule.monthly.day_of_month(5).count(3) } + expect(schedule.all_occurrences).to eq([t0, t0 + 31*IceCube::ONE_DAY + IceCube::ONE_HOUR, t0 + 61*IceCube::ONE_DAY + IceCube::ONE_HOUR]) + end + it "does not skip hourly rules over DST" do Time.zone = "America/Denver" t0 = Time.zone.parse("Sun, 03 Nov 2013 01:30:00 MDT -06:00")