Skip to content

Commit

Permalink
Use ActiveSupport::Duration to apply time offsets
Browse files Browse the repository at this point in the history
Using seconds to apply the time offsets when the parsed time is in the
past is not time zone aware. This causes issues when DST begins/ends
between the two times. For example, daily jobs are scheduled an hour
earlier than the are supposed to when DST ends since it doesn't take the
extra hour into account. This causes further issues when calculating the
next offset time, the offset to prevent scheduling past times doesn't
apply as expected and the job is scheduled over and over. As a related
issue, we sometimes need a two hour jump to bring the parsed time to the
future, add a while loop to handle this case.
  • Loading branch information
jonmast committed Nov 6, 2017
1 parent 89fd255 commit 9d76b84
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 3 deletions.
1 change: 1 addition & 0 deletions delayed_cron.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ Gem::Specification.new do |s|
s.add_development_dependency "simplecov"
s.add_development_dependency "codeclimate-test-reporter", "~> 1.0.0"
s.add_development_dependency "hashie"
s.add_development_dependency "pry-byebug"

s.files = `git ls-files`.split("\n")
s.test_files = `git ls-files -- {test,spec,features}/*`.split("\n")
Expand Down
1 change: 1 addition & 0 deletions lib/delayed_cron.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
require 'active_support/all'
require 'delayed_cron/jobs'
require 'delayed_cron/scheduling'
require 'delayed_cron/railtie'
Expand Down
6 changes: 3 additions & 3 deletions lib/delayed_cron/cron_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,15 @@ def convert_time_string_to_seconds_interval(scheduled_time_string, zone_name)
zone = Time.find_zone!(zone_name)

if hourly?
period_in_seconds = 60 * 60
period_duration = 1.hour
scheduled_time_string = "%H:#{scheduled_time_string}"
else
period_in_seconds = 60 * 60 * 24
period_duration = 1.day
end

scheduled_time = zone.now.strftime("%Y-%m-%d #{scheduled_time_string}")
scheduled_time = zone.parse(scheduled_time)
scheduled_time += period_in_seconds if zone.now >= scheduled_time
scheduled_time += period_duration while zone.now >= scheduled_time
scheduled_time.to_i - zone.now.to_i
end

Expand Down
32 changes: 32 additions & 0 deletions spec/cron_job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,11 @@ module DelayedCron
let(:next_occurrence) do
job.send(:convert_time_string_to_seconds_interval, scheduled_time, "Eastern Time (US & Canada)")
end
let(:zone) { Time.find_zone!("Eastern Time (US & Canada)") }

# Set Time.now to January 1, 2014 12:00:00 PM
before { Timecop.freeze(Time.utc(2014, 1, 1, 12, 0, 0)) }

context "next occurrence is today" do
let(:known_interval) { 21600 }
let(:scheduled_time) { "13:00:00 -0500" }
Expand Down Expand Up @@ -132,6 +135,35 @@ module DelayedCron

expect(next_occurrence).to be(known_interval)
end

it 'handles DST end' do
time = zone.local(2017, 11, 5, 1, 0) + 1.hour # ST start
Timecop.freeze(time)

expect(next_occurrence).to eq(1.hour.to_i)
end
end

context 'DST ends before next time' do
let(:scheduled_time) { "00:05:00" }

it 'adds an extra hour to the offset' do
time = zone.local(2017, 11, 5, 0, 5)
Timecop.freeze(time)

expect(next_occurrence).to eq(25.hours.to_i)
end
end

context 'calculation is on day of DST end' do
let(:scheduled_time) { "00:05:00" }

it 'still calculates correct offset' do
time = zone.local(2017, 11, 5, 23, 5)
Timecop.freeze(time)

expect(next_occurrence).to eq(1.hour.to_i)
end
end

end
Expand Down

0 comments on commit 9d76b84

Please sign in to comment.