From 9d76b84c5b9156183e37259d2cb6b95375145a7b Mon Sep 17 00:00:00 2001
From: Jonathan Mast <jon@jonmast.com>
Date: Mon, 6 Nov 2017 11:27:30 -0500
Subject: [PATCH] Use ActiveSupport::Duration to apply time offsets

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.
---
 delayed_cron.gemspec         |  1 +
 lib/delayed_cron.rb          |  1 +
 lib/delayed_cron/cron_job.rb |  6 +++---
 spec/cron_job_spec.rb        | 32 ++++++++++++++++++++++++++++++++
 4 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/delayed_cron.gemspec b/delayed_cron.gemspec
index fdfac31..bf25bae 100644
--- a/delayed_cron.gemspec
+++ b/delayed_cron.gemspec
@@ -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")
diff --git a/lib/delayed_cron.rb b/lib/delayed_cron.rb
index c804c59..df8daa7 100644
--- a/lib/delayed_cron.rb
+++ b/lib/delayed_cron.rb
@@ -1,3 +1,4 @@
+require 'active_support/all'
 require 'delayed_cron/jobs'
 require 'delayed_cron/scheduling'
 require 'delayed_cron/railtie'
diff --git a/lib/delayed_cron/cron_job.rb b/lib/delayed_cron/cron_job.rb
index 0f1290d..8cd3f05 100644
--- a/lib/delayed_cron/cron_job.rb
+++ b/lib/delayed_cron/cron_job.rb
@@ -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
 
diff --git a/spec/cron_job_spec.rb b/spec/cron_job_spec.rb
index a180d8f..eaad74e 100644
--- a/spec/cron_job_spec.rb
+++ b/spec/cron_job_spec.rb
@@ -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" }
@@ -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