From 7f879fe399f7ce99a39177b6a52c95b1143b190c Mon Sep 17 00:00:00 2001 From: Andrew Vit Date: Sat, 16 Sep 2017 18:29:23 -0700 Subject: [PATCH 1/6] Move spec out of module --- spec/examples/minutely_rule_spec.rb | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/spec/examples/minutely_rule_spec.rb b/spec/examples/minutely_rule_spec.rb index 574b1932..82b90615 100644 --- a/spec/examples/minutely_rule_spec.rb +++ b/spec/examples/minutely_rule_spec.rb @@ -1,7 +1,9 @@ require File.dirname(__FILE__) + '/../spec_helper' -module IceCube - describe MinutelyRule, 'interval validation' do +describe IceCube::MinutelyRule do + + describe 'interval validation' do + it 'converts a string integer to an actual int when using the interval method' do rule = Rule.minutely.interval("2") expect(rule.validations_for(:interval).first.interval).to eq(2) @@ -23,11 +25,8 @@ module IceCube Rule.minutely.interval("invalid") }.to raise_error(ArgumentError, "'invalid' is not a valid input for interval. Please pass a postive integer.") end - end - describe MinutelyRule do - it 'should update previous interval' do t0 = Time.now rule = Rule.minutely(7) @@ -74,5 +73,4 @@ module IceCube expect(schedule.next_occurrence(Time.new(2013, 11, 1, 1, 4, 0))).to eq(Time.new(2013, 11, 1, 1, 8, 0)) end - end end From c6ef94c64401dd98668db78fffe47c39476e9b5a Mon Sep 17 00:00:00 2001 From: Andrew Vit Date: Sun, 17 Sep 2017 20:18:23 -0700 Subject: [PATCH 2/6] Prevent specs from running indefinitely Limit each example to 1 second in case of infinite loops. Can be redefined with timeout: 0 on example metadata. --- spec/spec_helper.rb | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 2056d36c..90f8a48a 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,5 +1,6 @@ require "bundler/setup" require 'ice_cube' +require 'timeout' begin require 'simplecov' @@ -69,4 +70,10 @@ def Object.const_missing(sym) example.run end end + + config.around :each do |example| + Timeout.timeout(example.metadata.fetch(:timeout, 1)) do + example.run + end + end end From 70d2e59a27f29948d13cc8fadb0f83af5d94b841 Mon Sep 17 00:00:00 2001 From: Andrew Vit Date: Mon, 18 Sep 2017 00:34:24 -0700 Subject: [PATCH 3/6] Raise errors for invalid combinations of interval Reject illogical combinations of rule types with intervals that would never align with any occurrences. --- lib/ice_cube/rules/daily_rule.rb | 20 +++++++++++ lib/ice_cube/rules/monthly_rule.rb | 13 ++++++++ lib/ice_cube/validated_rule.rb | 31 +++++++++++++++++ lib/ice_cube/validations/daily_interval.rb | 6 +++- lib/ice_cube/validations/day.rb | 6 ++++ lib/ice_cube/validations/day_of_month.rb | 5 +++ lib/ice_cube/validations/hour_of_day.rb | 7 ++++ lib/ice_cube/validations/hourly_interval.rb | 2 ++ lib/ice_cube/validations/minute_of_hour.rb | 7 ++++ lib/ice_cube/validations/minutely_interval.rb | 2 ++ lib/ice_cube/validations/month_of_year.rb | 5 +++ lib/ice_cube/validations/monthly_interval.rb | 5 ++- lib/ice_cube/validations/schedule_lock.rb | 4 +++ lib/ice_cube/validations/second_of_minute.rb | 13 ++++++-- lib/ice_cube/validations/secondly_interval.rb | 2 ++ spec/examples/daily_rule_spec.rb | 33 +++++++++++++++++++ spec/examples/hourly_rule_spec.rb | 12 +++++++ spec/examples/minutely_rule_spec.rb | 13 +++++++- spec/examples/monthly_rule_spec.rb | 18 ++++++++++ spec/examples/secondly_rule_spec.rb | 13 ++++++++ 20 files changed, 211 insertions(+), 6 deletions(-) diff --git a/lib/ice_cube/rules/daily_rule.rb b/lib/ice_cube/rules/daily_rule.rb index 2eea9e1e..68c79fc4 100644 --- a/lib/ice_cube/rules/daily_rule.rb +++ b/lib/ice_cube/rules/daily_rule.rb @@ -11,6 +11,26 @@ def initialize(interval = 1) reset end + def verify_alignment(value, freq, rule_part) + return unless freq == :wday || freq == :day + return unless @validations[:interval] + + interval_validation = @validations[:interval].first + interval_value = (rule_part == :interval) ? value : interval_validation.interval + return if interval_value == 1 + + if freq == :wday + return if (interval_value % 7).zero? + return if Array(@validations[:day]).empty? + message = "day can only be used with multiples of interval(7)" + else + (fixed_validation = other_fixed_value_validations.first) or return + message = "#{fixed_validation.key} can only be used with interval(1)" + end + + yield ArgumentError.new(message) + end + end end diff --git a/lib/ice_cube/rules/monthly_rule.rb b/lib/ice_cube/rules/monthly_rule.rb index 00b8a4e0..20803267 100644 --- a/lib/ice_cube/rules/monthly_rule.rb +++ b/lib/ice_cube/rules/monthly_rule.rb @@ -11,6 +11,19 @@ def initialize(interval = 1) reset end + def verify_alignment(value, freq, rule_part) + return unless freq == :month + return unless @validations[:interval] + + interval_validation = @validations[:interval].first + interval_value = (rule_part == :interval) ? value : interval_validation.interval + return if interval_value == 1 || (interval_value % 12).zero? + return if other_fixed_value_validations.empty? + + message = "month_of_year can only be used with interval(1) or multiples of interval(12)" + yield ArgumentError.new(message) + end + end end diff --git a/lib/ice_cube/validated_rule.rb b/lib/ice_cube/validated_rule.rb index 72f9e9df..e0e340dc 100644 --- a/lib/ice_cube/validated_rule.rb +++ b/lib/ice_cube/validated_rule.rb @@ -51,6 +51,14 @@ def other_interval_validations Array(@validations[base_interval_validation.type]) end + def other_fixed_value_validations + @validations.values.flatten.select { |v| + interval_type = (v.type == :wday ? :day : v.type) + v.class < Validations::FixedValue && + interval_type == base_interval_validation.type + } + end + # Compute the next time after (or including) the specified time in respect # to the given start time def next_time(time, start_time, closing_time) @@ -185,6 +193,29 @@ def validation_names VALIDATION_ORDER & @validations.keys end + def verify_alignment(value, freq, rule_part, options={}) + @validations[:interval] or return + interval_validation = @validations[:interval].first + interval_validation.type == freq or return + fixed_validations = other_fixed_value_validations + (last_validation = fixed_validations.min_by(&:value)) or return + + alignment = (value - last_validation.value) % interval_validation.interval + return if alignment.zero? + + validation_values = fixed_validations.map(&:value).join(', ') + if rule_part == :interval + message = "interval(#{value}) " \ + "must be a multiple of " \ + "intervals in #{last_validation.key}(#{validation_values})" + else + message = "intervals in #{last_validation.key}(#{validation_values}, #{value}) " \ + "must be multiples of " \ + "interval(#{interval_validation.interval})" + end + yield ArgumentError.new(message) + end + end end diff --git a/lib/ice_cube/validations/daily_interval.rb b/lib/ice_cube/validations/daily_interval.rb index c23b6bd2..528c56cf 100644 --- a/lib/ice_cube/validations/daily_interval.rb +++ b/lib/ice_cube/validations/daily_interval.rb @@ -4,7 +4,11 @@ module Validations::DailyInterval # Add a new interval validation def interval(interval) - @interval = normalized_interval(interval) + interval = normalized_interval(interval) + verify_alignment(interval, :wday, :interval) { |error| raise error } + verify_alignment(interval, :day, :interval) { |error| raise error } + + @interval = interval replace_validations_for(:interval, [Validation.new(@interval)]) clobber_base_validations(:wday, :day) self diff --git a/lib/ice_cube/validations/day.rb b/lib/ice_cube/validations/day.rb index 71887050..6079ed6f 100644 --- a/lib/ice_cube/validations/day.rb +++ b/lib/ice_cube/validations/day.rb @@ -10,6 +10,8 @@ def day(*days) raise ArgumentError, "expecting Integer or Symbol value for day, got #{day.inspect}" end day = TimeUtil.sym_to_wday(day) + verify_alignment(day, :wday, :day) { |error| raise error } + validations_for(:day) << Validation.new(day) end clobber_base_validations(:wday, :day) @@ -25,6 +27,10 @@ def initialize(day) @day = day end + def key + :day + end + def type :wday end diff --git a/lib/ice_cube/validations/day_of_month.rb b/lib/ice_cube/validations/day_of_month.rb index 85e6f300..88496ed4 100644 --- a/lib/ice_cube/validations/day_of_month.rb +++ b/lib/ice_cube/validations/day_of_month.rb @@ -7,6 +7,7 @@ def day_of_month(*days) unless day.is_a?(Integer) raise ArgumentError, "expecting Integer value for day, got #{day.inspect}" end + verify_alignment(day, :day, :day_of_month) { |error| raise error } validations_for(:day_of_month) << Validation.new(day) end clobber_base_validations(:day, :wday) @@ -22,6 +23,10 @@ def initialize(day) @day = day end + def key + :day_of_month + end + def type :day end diff --git a/lib/ice_cube/validations/hour_of_day.rb b/lib/ice_cube/validations/hour_of_day.rb index 68073b51..cb60c8d1 100644 --- a/lib/ice_cube/validations/hour_of_day.rb +++ b/lib/ice_cube/validations/hour_of_day.rb @@ -8,6 +8,9 @@ def hour_of_day(*hours) unless hour.is_a?(Integer) raise ArgumentError, "expecting Integer value for hour, got #{hour.inspect}" end + + verify_alignment(hour, :hour, :hour_of_day) { |error| raise error } + validations_for(:hour_of_day) << Validation.new(hour) end clobber_base_validations(:hour) @@ -23,6 +26,10 @@ def initialize(hour) @hour = hour end + def key + :hour_of_day + end + def type :hour end diff --git a/lib/ice_cube/validations/hourly_interval.rb b/lib/ice_cube/validations/hourly_interval.rb index c5f1f989..edd73ed0 100644 --- a/lib/ice_cube/validations/hourly_interval.rb +++ b/lib/ice_cube/validations/hourly_interval.rb @@ -3,6 +3,8 @@ module IceCube module Validations::HourlyInterval def interval(interval) + verify_alignment(interval, :hour, :interval) { |error| raise error } + @interval = normalized_interval(interval) replace_validations_for(:interval, [Validation.new(@interval)]) clobber_base_validations(:hour) diff --git a/lib/ice_cube/validations/minute_of_hour.rb b/lib/ice_cube/validations/minute_of_hour.rb index 1c7bd9ef..915d27b3 100644 --- a/lib/ice_cube/validations/minute_of_hour.rb +++ b/lib/ice_cube/validations/minute_of_hour.rb @@ -7,6 +7,9 @@ def minute_of_hour(*minutes) unless minute.is_a?(Integer) raise ArgumentError, "expecting Integer value for minute, got #{minute.inspect}" end + + verify_alignment(minute, :min, :minute_of_hour) { |error| raise error } + validations_for(:minute_of_hour) << Validation.new(minute) end clobber_base_validations(:min) @@ -22,6 +25,10 @@ def initialize(minute) @minute = minute end + def key + :minute_of_hour + end + def type :min end diff --git a/lib/ice_cube/validations/minutely_interval.rb b/lib/ice_cube/validations/minutely_interval.rb index e4e3f757..0599a6ca 100644 --- a/lib/ice_cube/validations/minutely_interval.rb +++ b/lib/ice_cube/validations/minutely_interval.rb @@ -3,6 +3,8 @@ module IceCube module Validations::MinutelyInterval def interval(interval) + verify_alignment(interval, :min, :interval) { |error| raise error } + @interval = normalized_interval(interval) replace_validations_for(:interval, [Validation.new(@interval)]) clobber_base_validations(:min) diff --git a/lib/ice_cube/validations/month_of_year.rb b/lib/ice_cube/validations/month_of_year.rb index 5916a1ac..e85bc2f9 100644 --- a/lib/ice_cube/validations/month_of_year.rb +++ b/lib/ice_cube/validations/month_of_year.rb @@ -8,6 +8,7 @@ def month_of_year(*months) raise ArgumentError, "expecting Integer or Symbol value for month, got #{month.inspect}" end month = TimeUtil.sym_to_month(month) + verify_alignment(month, :month, :month_of_year) { |error| raise error } validations_for(:month_of_year) << Validation.new(month) end clobber_base_validations :month @@ -23,6 +24,10 @@ def initialize(month) @month = month end + def key + :month_of_year + end + def type :month end diff --git a/lib/ice_cube/validations/monthly_interval.rb b/lib/ice_cube/validations/monthly_interval.rb index e0a97cf4..49610a62 100644 --- a/lib/ice_cube/validations/monthly_interval.rb +++ b/lib/ice_cube/validations/monthly_interval.rb @@ -3,7 +3,10 @@ module IceCube module Validations::MonthlyInterval def interval(interval) - @interval = normalized_interval(interval) + interval = normalized_interval(interval) + verify_alignment(interval, :month, :interval) { |error| raise error } + + @interval = interval replace_validations_for(:interval, [Validation.new(@interval)]) clobber_base_validations(:month) self diff --git a/lib/ice_cube/validations/schedule_lock.rb b/lib/ice_cube/validations/schedule_lock.rb index 6640c00d..4dbbacda 100644 --- a/lib/ice_cube/validations/schedule_lock.rb +++ b/lib/ice_cube/validations/schedule_lock.rb @@ -20,6 +20,10 @@ def initialize(type) @type = type end + def key + :base + end + def dst_adjust? case @type when :sec, :min then false diff --git a/lib/ice_cube/validations/second_of_minute.rb b/lib/ice_cube/validations/second_of_minute.rb index 1ea3ec0f..a9cb78ac 100644 --- a/lib/ice_cube/validations/second_of_minute.rb +++ b/lib/ice_cube/validations/second_of_minute.rb @@ -4,9 +4,12 @@ module Validations::SecondOfMinute def second_of_minute(*seconds) seconds.flatten.each do |second| - unless second.is_a?(Integer) - raise ArgumentError, "Expecting Integer value for second, got #{second.inspect}" - end + unless second.is_a?(Integer) + raise ArgumentError, "Expecting Integer value for second, got #{second.inspect}" + end + + verify_alignment(second, :sec, :second_of_minute) { |error| raise error } + validations_for(:second_of_minute) << Validation.new(second) end clobber_base_validations :sec @@ -22,6 +25,10 @@ def initialize(second) @second = second end + def key + :second_of_minute + end + def type :sec end diff --git a/lib/ice_cube/validations/secondly_interval.rb b/lib/ice_cube/validations/secondly_interval.rb index 7f3d5cf2..92047edd 100644 --- a/lib/ice_cube/validations/secondly_interval.rb +++ b/lib/ice_cube/validations/secondly_interval.rb @@ -3,6 +3,8 @@ module IceCube module Validations::SecondlyInterval def interval(interval) + verify_alignment(interval, :sec, :interval) { |error| raise error } + @interval = normalized_interval(interval) replace_validations_for(:interval, [Validation.new(@interval)]) clobber_base_validations(:sec) diff --git a/spec/examples/daily_rule_spec.rb b/spec/examples/daily_rule_spec.rb index 4af68c13..488e7bbe 100644 --- a/spec/examples/daily_rule_spec.rb +++ b/spec/examples/daily_rule_spec.rb @@ -106,5 +106,38 @@ module IceCube ]) end + describe "day validation" do + it "allows multiples of 7" do + expect { IceCube::Rule.daily(21).day(2, 4) }.to_not raise_error + end + + it "raises errors for misaligned interval and day (wday) values" do + expect { + IceCube::Rule.daily(2).day(2, 4) + }.to raise_error(ArgumentError, "day can only be used with multiples of interval(7)") + end + + it "raises errors for misaligned hour_of_day values when changing interval" do + expect { + IceCube::Rule.daily.day(3, 6).interval(5) + }.to raise_error(ArgumentError, "day can only be used with multiples of interval(7)") + end + end + + describe "day_of_month validation" do + it "raises errors for misaligned interval and day_of_month values" do + expect { + IceCube::Rule.daily(2).day_of_month(2, 4) + }.to raise_error(ArgumentError, "day_of_month can only be used with interval(1)") + end + + it "raises errors for misaligned day_of_month values when changing interval" do + expect { + IceCube::Rule.daily.day_of_month(3, 6).interval(5) + }.to raise_error(ArgumentError, "day_of_month can only be used with interval(1)") + end + end + + end end diff --git a/spec/examples/hourly_rule_spec.rb b/spec/examples/hourly_rule_spec.rb index 7ceba67c..0e029758 100644 --- a/spec/examples/hourly_rule_spec.rb +++ b/spec/examples/hourly_rule_spec.rb @@ -69,5 +69,17 @@ module IceCube expect(dates).to eq([DAY, DAY + 3 * ONE_HOUR, DAY + 6 * ONE_HOUR]) end + it "raises errors for misaligned interval and hour_of_day values" do + expect { + IceCube::Rule.hourly(10).hour_of_day(3, 6) + }.to raise_error(ArgumentError, "intervals in hour_of_day(3, 6) must be multiples of interval(10)") + end + + it "raises errors for misaligned hour_of_day values when changing interval" do + expect { + IceCube::Rule.hourly(3).hour_of_day(3, 6).interval(5) + }.to raise_error(ArgumentError, "interval(5) must be a multiple of intervals in hour_of_day(3, 6)") + end + end end diff --git a/spec/examples/minutely_rule_spec.rb b/spec/examples/minutely_rule_spec.rb index 82b90615..326ea822 100644 --- a/spec/examples/minutely_rule_spec.rb +++ b/spec/examples/minutely_rule_spec.rb @@ -3,7 +3,6 @@ describe IceCube::MinutelyRule do describe 'interval validation' do - it 'converts a string integer to an actual int when using the interval method' do rule = Rule.minutely.interval("2") expect(rule.validations_for(:interval).first.interval).to eq(2) @@ -73,4 +72,16 @@ expect(schedule.next_occurrence(Time.new(2013, 11, 1, 1, 4, 0))).to eq(Time.new(2013, 11, 1, 1, 8, 0)) end + it "raises errors for misaligned interval and minute_of_hour values" do + expect { + IceCube::Rule.minutely(10).minute_of_hour(3, 6) + }.to raise_error(ArgumentError, "intervals in minute_of_hour(3, 6) must be multiples of interval(10)") + end + + it "raises errors for misaligned minute_of_hour values when changing interval" do + expect { + IceCube::Rule.minutely(3).minute_of_hour(3, 6).interval(5) + }.to raise_error(ArgumentError, "interval(5) must be a multiple of intervals in minute_of_hour(3, 6)") + end + end diff --git a/spec/examples/monthly_rule_spec.rb b/spec/examples/monthly_rule_spec.rb index 0dbaf5c0..5f8c52c0 100644 --- a/spec/examples/monthly_rule_spec.rb +++ b/spec/examples/monthly_rule_spec.rb @@ -155,5 +155,23 @@ def month_interval(current_date, last_date) current_month - last_month end + describe "month_of_year validation" do + it "allows multiples of 12" do + expect { IceCube::Rule.monthly(24).month_of_year(3, 6) }.to_not raise_error + end + + it "raises errors for misaligned interval and month_of_year values" do + expect { + IceCube::Rule.monthly(10).month_of_year(3, 6) + }.to raise_error(ArgumentError, "month_of_year can only be used with interval(1) or multiples of interval(12)") + end + + it "raises errors for misaligned month_of_year values when changing interval" do + expect { + IceCube::Rule.monthly.month_of_year(3, 6).interval(5) + }.to raise_error(ArgumentError, "month_of_year can only be used with interval(1) or multiples of interval(12)") + end + end + end end diff --git a/spec/examples/secondly_rule_spec.rb b/spec/examples/secondly_rule_spec.rb index be33aab2..b65901a9 100644 --- a/spec/examples/secondly_rule_spec.rb +++ b/spec/examples/secondly_rule_spec.rb @@ -23,5 +23,18 @@ module IceCube Rule.secondly.interval("invalid") }.to raise_error(ArgumentError, "'invalid' is not a valid input for interval. Please pass a postive integer.") end + + it "raises errors for misaligned interval and minute_of_hour values" do + expect { + IceCube::Rule.secondly(10).second_of_minute(3, 6) + }.to raise_error(ArgumentError, "intervals in second_of_minute(3, 6) must be multiples of interval(10)") + end + + it "raises errors for misaligned second_of_minute values when changing interval" do + expect { + IceCube::Rule.secondly(3).second_of_minute(3, 6).interval(5) + }.to raise_error(ArgumentError, "interval(5) must be a multiple of intervals in second_of_minute(3, 6)") + end + end end From 4ba32973423ac6631ece723db870320b25f5f007 Mon Sep 17 00:00:00 2001 From: Andrew Vit Date: Mon, 18 Sep 2017 00:41:02 -0700 Subject: [PATCH 4/6] Realign recurrence rules to their time parts When time parts (hour_of_day, minute_of_hour, second_of_minute) are specified on recurrence rules, the rules would jump over valid occurrences unless they were realigned. --- lib/ice_cube/rules/weekly_rule.rb | 2 +- lib/ice_cube/time_util.rb | 12 +++++++++++- lib/ice_cube/validations/hour_of_day.rb | 16 ++++++++++++++++ lib/ice_cube/validations/minute_of_hour.rb | 9 +++++++++ lib/ice_cube/validations/second_of_minute.rb | 9 +++++++++ spec/examples/hourly_rule_spec.rb | 16 ++++++++++++++++ spec/examples/minutely_rule_spec.rb | 8 ++++++++ spec/examples/regression_spec.rb | 6 ------ spec/examples/secondly_rule_spec.rb | 8 ++++++++ spec/examples/weekly_rule_spec.rb | 8 ++++++++ 10 files changed, 86 insertions(+), 8 deletions(-) diff --git a/lib/ice_cube/rules/weekly_rule.rb b/lib/ice_cube/rules/weekly_rule.rb index 1336ba99..3d7abff2 100644 --- a/lib/ice_cube/rules/weekly_rule.rb +++ b/lib/ice_cube/rules/weekly_rule.rb @@ -28,7 +28,7 @@ def realign(step_time, start_time) time = TimeUtil::TimeWrapper.new(start_time) offset = wday_offset(step_time, start_time) time.add(:day, offset) - time.to_time + super step_time, time.to_time end # Calculate how many days to the first wday validation in the correct diff --git a/lib/ice_cube/time_util.rb b/lib/ice_cube/time_util.rb index 84a90e0f..ff43399e 100644 --- a/lib/ice_cube/time_util.rb +++ b/lib/ice_cube/time_util.rb @@ -311,7 +311,17 @@ def clear_below(type) end end - private + def hour=(value) + @time += (value * ONE_HOUR) - (@time.hour * ONE_HOUR) + end + + def min=(value) + @time += (value * ONE_MINUTE) - (@time.min * ONE_MINUTE) + end + + def sec=(value) + @time += (value) - (@time.sec) + end def clear_sec @time.sec > 0 ? @time -= @time.sec : @time diff --git a/lib/ice_cube/validations/hour_of_day.rb b/lib/ice_cube/validations/hour_of_day.rb index cb60c8d1..291708e6 100644 --- a/lib/ice_cube/validations/hour_of_day.rb +++ b/lib/ice_cube/validations/hour_of_day.rb @@ -17,6 +17,22 @@ def hour_of_day(*hours) self end + def realign(opening_time, start_time) + return super unless validations[:hour_of_day] + freq = base_interval_validation.interval + + first_hour = Array(validations[:hour_of_day]).min_by(&:value) + time = TimeUtil::TimeWrapper.new(start_time, false) + if freq > 1 + offset = first_hour.validate(opening_time, start_time) + time.add(:hour, offset - freq) + else + time.hour = first_hour.value + end + + super opening_time, time.to_time + end + class Validation < Validations::FixedValue attr_reader :hour diff --git a/lib/ice_cube/validations/minute_of_hour.rb b/lib/ice_cube/validations/minute_of_hour.rb index 915d27b3..03c2dd15 100644 --- a/lib/ice_cube/validations/minute_of_hour.rb +++ b/lib/ice_cube/validations/minute_of_hour.rb @@ -16,6 +16,15 @@ def minute_of_hour(*minutes) self end + def realign(opening_time, start_time) + return super unless validations[:minute_of_hour] + + first_minute = validations[:minute_of_hour].min_by(&:value) + time = TimeUtil::TimeWrapper.new(start_time, false) + time.min = first_minute.value + super opening_time, time.to_time + end + class Validation < Validations::FixedValue attr_reader :minute diff --git a/lib/ice_cube/validations/second_of_minute.rb b/lib/ice_cube/validations/second_of_minute.rb index a9cb78ac..f9208107 100644 --- a/lib/ice_cube/validations/second_of_minute.rb +++ b/lib/ice_cube/validations/second_of_minute.rb @@ -16,6 +16,15 @@ def second_of_minute(*seconds) self end + def realign(opening_time, start_time) + return super unless validations[:second_of_minute] + + first_second = Array(validations[:second_of_minute]).min_by(&:value) + time = TimeUtil::TimeWrapper.new(start_time, false) + time.sec = first_second.value + super opening_time, time.to_time + end + class Validation < Validations::FixedValue attr_reader :second diff --git a/spec/examples/hourly_rule_spec.rb b/spec/examples/hourly_rule_spec.rb index 0e029758..c5f1d1cb 100644 --- a/spec/examples/hourly_rule_spec.rb +++ b/spec/examples/hourly_rule_spec.rb @@ -69,6 +69,22 @@ module IceCube expect(dates).to eq([DAY, DAY + 3 * ONE_HOUR, DAY + 6 * ONE_HOUR]) end + it "should realign to the first hour_of_day with interval" do + t0 = Time.utc(2017, 1, 1, 20, 30, 40) + schedule = IceCube::Schedule.new(t0) + schedule.rrule IceCube::Rule.hourly(5).hour_of_day(5, 10) + + expect(schedule.first(2)).to eq [t0 + 9*ONE_HOUR, t0 + 14*ONE_HOUR] + end + + it "should realign to the first hour_of_day without interval" do + t0 = Time.utc(2017, 1, 1, 20, 30, 40) + schedule = IceCube::Schedule.new(t0) + schedule.rrule IceCube::Rule.hourly.hour_of_day(5, 10) + + expect(schedule.first(2)).to eq [t0 + 9*ONE_HOUR, t0 + 14*ONE_HOUR] + end + it "raises errors for misaligned interval and hour_of_day values" do expect { IceCube::Rule.hourly(10).hour_of_day(3, 6) diff --git a/spec/examples/minutely_rule_spec.rb b/spec/examples/minutely_rule_spec.rb index 326ea822..49ef7992 100644 --- a/spec/examples/minutely_rule_spec.rb +++ b/spec/examples/minutely_rule_spec.rb @@ -72,6 +72,14 @@ expect(schedule.next_occurrence(Time.new(2013, 11, 1, 1, 4, 0))).to eq(Time.new(2013, 11, 1, 1, 8, 0)) end + it "should realign to the first minute_of_hour" do + t0 = Time.utc(2017, 1, 1, 20, 30, 40) + schedule = IceCube::Schedule.new(t0) + schedule.rrule IceCube::Rule.minutely(10).minute_of_hour(5, 15) + + expect(schedule.first(2)).to eq [t0 + 35*ONE_MINUTE, t0 + 45*ONE_MINUTE] + end + it "raises errors for misaligned interval and minute_of_hour values" do expect { IceCube::Rule.minutely(10).minute_of_hour(3, 6) diff --git a/spec/examples/regression_spec.rb b/spec/examples/regression_spec.rb index b1530822..a9e8454d 100644 --- a/spec/examples/regression_spec.rb +++ b/spec/examples/regression_spec.rb @@ -36,12 +36,6 @@ module IceCube expect(schedule.occurrences(Date.today >> 12)).to be_an Array end - it 'should not regress [#40]' do - schedule = Schedule.new(Time.local(2011, 11, 16, 11, 31, 58), :duration => 3600) - schedule.add_recurrence_rule Rule.minutely(60).day(4).hour_of_day(14, 15, 16).minute_of_hour(0) - expect(schedule.occurring_at?(Time.local(2011, 11, 17, 15, 30))).to be_falsey - end - it 'should not choke on parsing [#26]' do schedule = Schedule.new(Time.local(2011, 8, 9, 14, 52, 14)) schedule.rrule Rule.weekly(1).day(1, 2, 3, 4, 5) diff --git a/spec/examples/secondly_rule_spec.rb b/spec/examples/secondly_rule_spec.rb index b65901a9..8e76692e 100644 --- a/spec/examples/secondly_rule_spec.rb +++ b/spec/examples/secondly_rule_spec.rb @@ -24,6 +24,14 @@ module IceCube }.to raise_error(ArgumentError, "'invalid' is not a valid input for interval. Please pass a postive integer.") end + it "should realign to the first second_of_minute" do + t0 = Time.utc(2017, 1, 1, 20, 30, 40) + schedule = IceCube::Schedule.new(t0) + schedule.rrule IceCube::Rule.secondly(10).second_of_minute(5, 15) + + expect(schedule.first(2)).to eq [t0 + 25*ONE_SECOND, t0 + 35*ONE_SECOND] + end + it "raises errors for misaligned interval and minute_of_hour values" do expect { IceCube::Rule.secondly(10).second_of_minute(3, 6) diff --git a/spec/examples/weekly_rule_spec.rb b/spec/examples/weekly_rule_spec.rb index f35eb32d..682a85e4 100644 --- a/spec/examples/weekly_rule_spec.rb +++ b/spec/examples/weekly_rule_spec.rb @@ -309,6 +309,14 @@ module IceCube end end + it "should align next_occurrence with the earliest hour validation" do + t0 = Time.utc(2017, 7, 28, 20, 30, 40) + schedule = IceCube::Schedule.new(t0) + schedule.add_recurrence_rule IceCube::Rule.weekly.day(:saturday).hour_of_day(19).minute_of_hour(29).second_of_minute(39) + + expect(schedule.next_occurrence(t0)).to eq Time.utc(2017, 7, 29, 19, 29, 39) + end + describe "using occurs_between with a biweekly schedule" do [[0, 1, 2], [0, 6, 1], [5, 1, 6], [6, 5, 7]].each do |wday, offset, lead| start_time = Time.utc(2014, 1, 5, 9, 0, 0) From d698e842eb97f44dc5dee5b501d6933d3e8dbf2f Mon Sep 17 00:00:00 2001 From: Andrew Vit Date: Mon, 18 Sep 2017 00:49:52 -0700 Subject: [PATCH 5/6] Remove access to illogical rule parts Some combinations of rule parts do not apply. Rather than exposing all possible validations on all recurrence rule types, limit to ones that produce results. --- lib/ice_cube/rules/daily_rule.rb | 9 +++++++++ lib/ice_cube/rules/hourly_rule.rb | 9 +++++++++ lib/ice_cube/rules/minutely_rule.rb | 9 +++++++++ lib/ice_cube/rules/monthly_rule.rb | 9 +++++++++ lib/ice_cube/rules/secondly_rule.rb | 9 +++++++++ lib/ice_cube/rules/weekly_rule.rb | 9 +++++++++ lib/ice_cube/rules/yearly_rule.rb | 9 +++++++++ lib/ice_cube/validated_rule.rb | 9 --------- spec/examples/dst_spec.rb | 4 ++-- spec/examples/from_ical_spec.rb | 4 ++-- spec/examples/to_ical_spec.rb | 4 ++-- 11 files changed, 69 insertions(+), 15 deletions(-) diff --git a/lib/ice_cube/rules/daily_rule.rb b/lib/ice_cube/rules/daily_rule.rb index 68c79fc4..0ecc0a80 100644 --- a/lib/ice_cube/rules/daily_rule.rb +++ b/lib/ice_cube/rules/daily_rule.rb @@ -2,6 +2,15 @@ module IceCube class DailyRule < ValidatedRule + include Validations::HourOfDay + include Validations::MinuteOfHour + include Validations::SecondOfMinute + include Validations::DayOfMonth + include Validations::DayOfWeek + include Validations::Day + include Validations::MonthOfYear + # include Validations::DayOfYear # n/a + include Validations::DailyInterval def initialize(interval = 1) diff --git a/lib/ice_cube/rules/hourly_rule.rb b/lib/ice_cube/rules/hourly_rule.rb index 60bb76ce..a4e0ba1b 100644 --- a/lib/ice_cube/rules/hourly_rule.rb +++ b/lib/ice_cube/rules/hourly_rule.rb @@ -2,6 +2,15 @@ module IceCube class HourlyRule < ValidatedRule + include Validations::HourOfDay + include Validations::MinuteOfHour + include Validations::SecondOfMinute + include Validations::DayOfMonth + include Validations::DayOfWeek + include Validations::Day + include Validations::MonthOfYear + include Validations::DayOfYear + include Validations::HourlyInterval def initialize(interval = 1) diff --git a/lib/ice_cube/rules/minutely_rule.rb b/lib/ice_cube/rules/minutely_rule.rb index c9ea99d9..640e1ab7 100644 --- a/lib/ice_cube/rules/minutely_rule.rb +++ b/lib/ice_cube/rules/minutely_rule.rb @@ -2,6 +2,15 @@ module IceCube class MinutelyRule < ValidatedRule + include Validations::HourOfDay + include Validations::MinuteOfHour + include Validations::SecondOfMinute + include Validations::DayOfMonth + include Validations::DayOfWeek + include Validations::Day + include Validations::MonthOfYear + include Validations::DayOfYear + include Validations::MinutelyInterval def initialize(interval = 1) diff --git a/lib/ice_cube/rules/monthly_rule.rb b/lib/ice_cube/rules/monthly_rule.rb index 20803267..dd8e92a1 100644 --- a/lib/ice_cube/rules/monthly_rule.rb +++ b/lib/ice_cube/rules/monthly_rule.rb @@ -2,6 +2,15 @@ module IceCube class MonthlyRule < ValidatedRule + include Validations::HourOfDay + include Validations::MinuteOfHour + include Validations::SecondOfMinute + include Validations::DayOfMonth + include Validations::DayOfWeek + include Validations::Day + include Validations::MonthOfYear + # include Validations::DayOfYear # n/a + include Validations::MonthlyInterval def initialize(interval = 1) diff --git a/lib/ice_cube/rules/secondly_rule.rb b/lib/ice_cube/rules/secondly_rule.rb index b82ad38b..ecd62532 100644 --- a/lib/ice_cube/rules/secondly_rule.rb +++ b/lib/ice_cube/rules/secondly_rule.rb @@ -2,6 +2,15 @@ module IceCube class SecondlyRule < ValidatedRule + include Validations::HourOfDay + include Validations::MinuteOfHour + include Validations::SecondOfMinute + include Validations::DayOfMonth + include Validations::DayOfWeek + include Validations::Day + include Validations::MonthOfYear + include Validations::DayOfYear + include Validations::SecondlyInterval def initialize(interval = 1) diff --git a/lib/ice_cube/rules/weekly_rule.rb b/lib/ice_cube/rules/weekly_rule.rb index 3d7abff2..dd39f2b3 100644 --- a/lib/ice_cube/rules/weekly_rule.rb +++ b/lib/ice_cube/rules/weekly_rule.rb @@ -2,6 +2,15 @@ module IceCube class WeeklyRule < ValidatedRule + include Validations::HourOfDay + include Validations::MinuteOfHour + include Validations::SecondOfMinute + # include Validations::DayOfMonth # n/a + include Validations::DayOfWeek + include Validations::Day + include Validations::MonthOfYear + # include Validations::DayOfYear # n/a + include Validations::WeeklyInterval attr_reader :week_start diff --git a/lib/ice_cube/rules/yearly_rule.rb b/lib/ice_cube/rules/yearly_rule.rb index 1f3e915a..3a18b0a6 100644 --- a/lib/ice_cube/rules/yearly_rule.rb +++ b/lib/ice_cube/rules/yearly_rule.rb @@ -2,6 +2,15 @@ module IceCube class YearlyRule < ValidatedRule + include Validations::HourOfDay + include Validations::MinuteOfHour + include Validations::SecondOfMinute + include Validations::DayOfMonth + include Validations::DayOfWeek + include Validations::Day + include Validations::MonthOfYear + include Validations::DayOfYear + include Validations::YearlyInterval def initialize(interval = 1) diff --git a/lib/ice_cube/validated_rule.rb b/lib/ice_cube/validated_rule.rb index e0e340dc..7903be51 100644 --- a/lib/ice_cube/validated_rule.rb +++ b/lib/ice_cube/validated_rule.rb @@ -4,15 +4,6 @@ class ValidatedRule < Rule include Validations::ScheduleLock - include Validations::HourOfDay - include Validations::MinuteOfHour - include Validations::SecondOfMinute - include Validations::DayOfMonth - include Validations::DayOfWeek - include Validations::Day - include Validations::MonthOfYear - include Validations::DayOfYear - include Validations::Count include Validations::Until diff --git a/spec/examples/dst_spec.rb b/spec/examples/dst_spec.rb index 4757053b..d437720a 100644 --- a/spec/examples/dst_spec.rb +++ b/spec/examples/dst_spec.rb @@ -248,10 +248,10 @@ module IceCube expect(schedule.first(3)).to eq([Time.local(2010, 3, 7, 12, 0, 0), Time.local(2010, 3, 14, 12, 0, 0), Time.local(2010, 3, 21, 12, 0, 0)]) end - it "local - should make dates on monthly (day of year) inverval over dst - github issue 5" do + it "local - should make dates on yearly (day of year) inverval over dst - github issue 5" do start_time = Time.local(2010, 3, 7, 12, 0, 0) schedule = Schedule.new(start_time) - schedule.add_recurrence_rule Rule.monthly.day_of_year(1) + schedule.add_recurrence_rule Rule.yearly.day_of_year(1) expect(schedule.first(3)).to eq([Time.local(2011, 1, 1, 12, 0, 0), Time.local(2012, 1, 1, 12, 0, 0), Time.local(2013, 1, 1, 12, 0, 0)]) end diff --git a/spec/examples/from_ical_spec.rb b/spec/examples/from_ical_spec.rb index 113080ea..12cf8bf8 100644 --- a/spec/examples/from_ical_spec.rb +++ b/spec/examples/from_ical_spec.rb @@ -46,8 +46,8 @@ module IceCube end it 'should be able to parse a .day_of_year rule' do - rule = IceCube::Rule.from_ical("FREQ=DAILY;BYYEARDAY=100,200") - expect(rule).to eq(IceCube::Rule.daily.day_of_year(100,200)) + rule = IceCube::Rule.from_ical("FREQ=YEARLY;BYYEARDAY=100,200") + expect(rule).to eq(IceCube::Rule.yearly.day_of_year(100,200)) end it 'should be able to serialize a .month_of_year rule' do diff --git a/spec/examples/to_ical_spec.rb b/spec/examples/to_ical_spec.rb index 92d09cb7..10521739 100644 --- a/spec/examples/to_ical_spec.rb +++ b/spec/examples/to_ical_spec.rb @@ -54,8 +54,8 @@ end it 'should be able to serialize a .day_of_year rule to_ical' do - rule = IceCube::Rule.daily.day_of_year(100,200) - expect(rule.to_ical).to eq("FREQ=DAILY;BYYEARDAY=100,200") + rule = IceCube::Rule.yearly.day_of_year(100,200) + expect(rule.to_ical).to eq("FREQ=YEARLY;BYYEARDAY=100,200") end it 'should be able to serialize a .month_of_year rule to_ical' do From 7f7d5a76ad3cd3f10d370373f97394ed6b2c567f Mon Sep 17 00:00:00 2001 From: Andrew Vit Date: Sun, 24 Sep 2017 20:53:01 -0700 Subject: [PATCH 6/6] Refactor: extract class for verifying input Move methods related to input verification into their own object. --- lib/ice_cube/input_alignment.rb | 89 ++++++++++++++++++++++++++++++ lib/ice_cube/rules/daily_rule.rb | 20 ------- lib/ice_cube/rules/monthly_rule.rb | 13 ----- lib/ice_cube/validated_rule.rb | 33 ++--------- 4 files changed, 94 insertions(+), 61 deletions(-) create mode 100644 lib/ice_cube/input_alignment.rb diff --git a/lib/ice_cube/input_alignment.rb b/lib/ice_cube/input_alignment.rb new file mode 100644 index 00000000..2fecb5fb --- /dev/null +++ b/lib/ice_cube/input_alignment.rb @@ -0,0 +1,89 @@ +module IceCube + class InputAlignment + + def initialize(rule, value, rule_part) + @rule = rule + @value = value + @rule_part = rule_part + end + + attr_reader :rule, :value, :rule_part + + def verify(freq, options={}, &block) + @rule.validations[:interval] or return + + case @rule + when DailyRule + verify_wday_alignment(freq, &block) + when MonthlyRule + verify_month_alignment(freq, &block) + else + verify_freq_alignment(freq, &block) + end + end + + private + + def interval_validation + @interval_validation ||= @rule.validations[:interval].first + end + + def interval_value + @interval_value ||= (rule_part == :interval) ? value : interval_validation.interval + end + + def fixed_validations + @fixed_validations ||= @rule.validations.values.flatten.select { |v| + interval_type = (v.type == :wday ? :day : v.type) + v.class < Validations::FixedValue && + interval_type == rule.base_interval_validation.type + } + end + + def verify_freq_alignment(freq) + interval_validation.type == freq or return + (last_validation = fixed_validations.min_by(&:value)) or return + + alignment = (value - last_validation.value) % interval_validation.interval + return if alignment.zero? + + validation_values = fixed_validations.map(&:value).join(', ') + if rule_part == :interval + message = "interval(#{value}) " \ + "must be a multiple of " \ + "intervals in #{last_validation.key}(#{validation_values})" + else + message = "intervals in #{last_validation.key}(#{validation_values}, #{value}) " \ + "must be multiples of " \ + "interval(#{interval_validation.interval})" + end + + yield ArgumentError.new(message) + end + + def verify_month_alignment(_freq) + return if interval_value == 1 || (interval_value % 12).zero? + return if fixed_validations.empty? + + message = "month_of_year can only be used with interval(1) or multiples of interval(12)" + + yield ArgumentError.new(message) + end + + def verify_wday_alignment(freq) + return if interval_value == 1 + + if freq == :wday + return if (interval_value % 7).zero? + return if Array(@rule.validations[:day]).empty? + message = "day can only be used with multiples of interval(7)" + else + (fixed_validation = fixed_validations.first) or return + message = "#{fixed_validation.key} can only be used with interval(1)" + end + + yield ArgumentError.new(message) + end + + end +end diff --git a/lib/ice_cube/rules/daily_rule.rb b/lib/ice_cube/rules/daily_rule.rb index 0ecc0a80..e5b77dac 100644 --- a/lib/ice_cube/rules/daily_rule.rb +++ b/lib/ice_cube/rules/daily_rule.rb @@ -20,26 +20,6 @@ def initialize(interval = 1) reset end - def verify_alignment(value, freq, rule_part) - return unless freq == :wday || freq == :day - return unless @validations[:interval] - - interval_validation = @validations[:interval].first - interval_value = (rule_part == :interval) ? value : interval_validation.interval - return if interval_value == 1 - - if freq == :wday - return if (interval_value % 7).zero? - return if Array(@validations[:day]).empty? - message = "day can only be used with multiples of interval(7)" - else - (fixed_validation = other_fixed_value_validations.first) or return - message = "#{fixed_validation.key} can only be used with interval(1)" - end - - yield ArgumentError.new(message) - end - end end diff --git a/lib/ice_cube/rules/monthly_rule.rb b/lib/ice_cube/rules/monthly_rule.rb index dd8e92a1..3e1307fb 100644 --- a/lib/ice_cube/rules/monthly_rule.rb +++ b/lib/ice_cube/rules/monthly_rule.rb @@ -20,19 +20,6 @@ def initialize(interval = 1) reset end - def verify_alignment(value, freq, rule_part) - return unless freq == :month - return unless @validations[:interval] - - interval_validation = @validations[:interval].first - interval_value = (rule_part == :interval) ? value : interval_validation.interval - return if interval_value == 1 || (interval_value % 12).zero? - return if other_fixed_value_validations.empty? - - message = "month_of_year can only be used with interval(1) or multiples of interval(12)" - yield ArgumentError.new(message) - end - end end diff --git a/lib/ice_cube/validated_rule.rb b/lib/ice_cube/validated_rule.rb index 7903be51..69830fab 100644 --- a/lib/ice_cube/validated_rule.rb +++ b/lib/ice_cube/validated_rule.rb @@ -1,3 +1,5 @@ +require 'ice_cube/input_alignment' + module IceCube class ValidatedRule < Rule @@ -42,14 +44,6 @@ def other_interval_validations Array(@validations[base_interval_validation.type]) end - def other_fixed_value_validations - @validations.values.flatten.select { |v| - interval_type = (v.type == :wday ? :day : v.type) - v.class < Validations::FixedValue && - interval_type == base_interval_validation.type - } - end - # Compute the next time after (or including) the specified time in respect # to the given start time def next_time(time, start_time, closing_time) @@ -184,27 +178,10 @@ def validation_names VALIDATION_ORDER & @validations.keys end - def verify_alignment(value, freq, rule_part, options={}) - @validations[:interval] or return - interval_validation = @validations[:interval].first - interval_validation.type == freq or return - fixed_validations = other_fixed_value_validations - (last_validation = fixed_validations.min_by(&:value)) or return - - alignment = (value - last_validation.value) % interval_validation.interval - return if alignment.zero? - - validation_values = fixed_validations.map(&:value).join(', ') - if rule_part == :interval - message = "interval(#{value}) " \ - "must be a multiple of " \ - "intervals in #{last_validation.key}(#{validation_values})" - else - message = "intervals in #{last_validation.key}(#{validation_values}, #{value}) " \ - "must be multiples of " \ - "interval(#{interval_validation.interval})" + def verify_alignment(value, freq, rule_part) + InputAlignment.new(self, value, rule_part).verify(freq) do |error| + yield error end - yield ArgumentError.new(message) end end