Skip to content

Commit

Permalink
Fix double occurrences in end of DST
Browse files Browse the repository at this point in the history
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 c93fa7e 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
  • Loading branch information
avit committed Jul 9, 2017
1 parent 0101cb4 commit 4371d53
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 45 deletions.
19 changes: 2 additions & 17 deletions lib/ice_cube/schedule.rb
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
48 changes: 20 additions & 28 deletions lib/ice_cube/time_util.rb
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
6 changes: 6 additions & 0 deletions spec/examples/dst_spec.rb
Expand Up @@ -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")
Expand Down

0 comments on commit 4371d53

Please sign in to comment.