Fixes #4179 - CacheClearJob.perform_now might not run if timeplans time window...

lib/background_services/service/process_scheduled_jobs.rb

@@ -3,8 +3,8 @@
 class BackgroundServices
   class Service
     class ProcessScheduledJobs < Service
-      SLEEP_AFTER_JOB_START = 10.seconds
-      SLEEP_AFTER_LOOP = 60.seconds
+      SLEEP_AFTER_JOB_START = 1.second
+      SLEEP_AFTER_LOOP = 10.seconds
 
       attr_reader :jobs_started
 
@@ -27,8 +27,8 @@ class BackgroundServices
 
       def run_jobs
         scope.each do |job|
-          Manager.new(job, jobs_started).run
-          sleep SLEEP_AFTER_JOB_START
+          result = Manager.new(job, jobs_started).run
+          sleep SLEEP_AFTER_JOB_START if result
         end
       end
 

lib/monitoring_helper/health_checker/scheduler.rb

@@ -31,16 +31,18 @@ module MonitoringHelper
           .order(last_run: :asc, period: :asc)
       end
 
+      def last_execution_deadline(scheduler)
+        return scheduler.last_run if scheduler.timeplan.blank?
+
+        calculator = TimeplanCalculation.new(scheduler.timeplan, Setting.get('timezone_default').presence || 'UTC')
+        intermediary = calculator.next_at(scheduler.last_run + 10.minutes)
+        calculator.next_at(intermediary + 10.minutes)
+      end
+
       def last_execution_on_time?(scheduler)
-        expected_next_run = if scheduler.timeplan.present?
-                              calculator = TimeplanCalculation.new(scheduler.timeplan, Setting.get('timezone_default').presence || 'UTC')
-                              intermediary = calculator.next_at(scheduler.last_run)
-                              calculator.next_at(intermediary + 10.minutes)
-                            else
-                              scheduler.last_run + scheduler.period.seconds
-                            end
-
-        expected_next_run >= LAST_EXECUTION_TOLERANCE.ago
+        return false if scheduler.last_run.blank?
+
+        last_execution_deadline(scheduler) + scheduler.period.seconds >= LAST_EXECUTION_TOLERANCE.ago
       end
 
       def none_running

lib/tasks/zammad/ci/settings.rake

@@ -8,6 +8,7 @@ namespace :zammad do
     task :settings, [:elasticsearch] => :environment do |_task, args|
       Setting.set('developer_mode', true)
       Setting.set('chat_agent_idle_timeout', '45')
+      Scheduler.find_by(method: 'SessionTimeoutJob.perform_now').update!(active: false)
 
       next if args[:elasticsearch] != 'with_elasticsearch'
 

lib/timeplan_calculation.rb

@@ -18,6 +18,9 @@ class TimeplanCalculation
     @timezone = timezone
   end
 
+  # Checks if given time matches timeplan
+  # @param [Time]
+  # @return [Boolean]
   def contains?(time)
     return false if !valid?
 
@@ -26,6 +29,9 @@ class TimeplanCalculation
     day?(time_in_zone) && hour?(time_in_zone) && minute?(time_in_zone)
   end
 
+  # Calculates next time in timeplan after the given time
+  # @param [Time]
+  # @return [Time, nil]
   def next_at(time)
     return nil if !valid?
 
@@ -34,6 +40,17 @@ class TimeplanCalculation
     next_run_at_same_day(time_in_zone) || next_run_at_coming_week(time_in_zone)
   end
 
+  # Calculates previous time in timeplan before the given time
+  # @param [Time]
+  # @return [Time, nil]
+  def previous_at(time)
+    return nil if !valid?
+
+    time_in_zone = ensure_matching_time(time)
+
+    previous_run_at_same_day(time_in_zone) || previous_run_at_past_week(time_in_zone)
+  end
+
   private
 
   def ensure_matching_time(time)
@@ -60,31 +77,52 @@ class TimeplanCalculation
     timeplan.dig 'minutes', match_minutes(time.min).to_s
   end
 
-  def loop_minutes(base_time)
+  def next_loop_minutes(base_time)
+    loop_minutes base_time, range: 0.step(50, 10)
+  end
+
+  def previous_loop_minutes(base_time)
+    loop_minutes base_time, range: 50.step(0, -10)
+  end
+
+  def loop_minutes(base_time, range:)
     return if !hour?(base_time)
 
-    0
-      .step(50, 10)
+    range
       .lazy
       .map  { |minute| base_time.change min: minute }
       .find { |time| minute?(time) }
   end
 
-  def loop_hours(base_time)
+  def next_loop_hours(base_time)
+    loop_hours base_time, range: (base_time.hour..23), minutes_symbol: :next_loop_minutes
+  end
+
+  def previous_loop_hours(base_time)
+    loop_hours base_time, range: (0..base_time.hour).entries.reverse, minutes_symbol: :previous_loop_minutes
+  end
+
+  def loop_hours(base_time, range:, minutes_symbol:)
     return if !day?(base_time)
 
-    (base_time.hour..23)
+    range
       .lazy
-      .map { |hour| loop_minutes base_time.change hour: hour }
+      .map { |hour| send(minutes_symbol, base_time.change(hour: hour)) }
       .find(&:present?)
   end
 
-  def loop_partial_hour(base_time)
+  def next_loop_partial_hour(base_time)
+    loop_partial_hour base_time, range: base_time.min.step(50, 10)
+  end
+
+  def previous_loop_partial_hour(base_time)
+    loop_partial_hour base_time, range: base_time.min.step(0, -10)
+  end
+
+  def loop_partial_hour(base_time, range:)
     return if !day?(base_time)
 
-    base_time
-      .min
-      .step(50, 10)
+    range
       .lazy
       .map  { |minute| base_time.change(min: minute) }
       .find { |time| hour?(time) && minute?(time) }
@@ -94,7 +132,7 @@ class TimeplanCalculation
     day_to_check = time.change min: match_minutes(time.min)
 
     if day_to_check.min.nonzero?
-      date = loop_partial_hour(day_to_check)
+      date = next_loop_partial_hour(day_to_check)
 
       return date if date
 
@@ -102,13 +140,35 @@ class TimeplanCalculation
       day_to_check += 1.hour
     end
 
-    loop_hours(day_to_check)
+    next_loop_hours(day_to_check)
   end
 
   def next_run_at_coming_week(time)
     (1..7)
       .lazy
-      .map { |day| loop_hours (time + day.day).midnight }
+      .map { |day| next_loop_hours (time + day.day).beginning_of_day }
+      .find(&:present?)
+  end
+
+  def previous_run_at_same_day(time)
+    day_to_check = time.change min: match_minutes(time.min)
+
+    if day_to_check.min.nonzero?
+      date = previous_loop_partial_hour(day_to_check)
+
+      return date if date
+
+      day_to_check = day_to_check.change(min: 0)
+      day_to_check -= 1.second
+    end
+
+    previous_loop_hours(day_to_check)
+  end
+
+  def previous_run_at_past_week(time)
+    (1..7)
+      .lazy
+      .map { |day| previous_loop_hours (time - day.day).end_of_day }
       .find(&:present?)
   end
 end

spec/lib/monitoring_helper/health_checker/scheduler_spec.rb

@@ -85,17 +85,31 @@ RSpec.describe MonitoringHelper::HealthChecker::Scheduler do
       expect(instance.send(:last_execution_on_time?, scheduler)).to be_falsey
     end
 
-    # https://github.com/zammad/zammad/issues/4079
-    it 'returns true if timeplan scheduler was skipped once only' do
-      travel_to Time.current.noon
-      scheduler = create(:scheduler, :timeplan, last_run: 1.day.ago)
-      expect(instance.send(:last_execution_on_time?, scheduler)).to be_truthy
-    end
+    context 'with timeplan' do
+      it 'returns true if timeplan scheduler was not skipped' do
+        travel_to Time.current.beginning_of_day
+        scheduler = create(:scheduler, :timeplan, last_run: 55.minutes.ago)
+        expect(instance.send(:last_execution_on_time?, scheduler)).to be_truthy
+      end
 
-    it 'returns false if timeplan scheduler was skipped multiple times' do
-      travel_to Time.current.noon
-      scheduler = create(:scheduler, :timeplan, last_run: 2.days.ago)
-      expect(instance.send(:last_execution_on_time?, scheduler)).to be_falsey
+      # https://github.com/zammad/zammad/issues/4079
+      it 'returns true if timeplan scheduler was slightly late only' do
+        travel_to Time.current.beginning_of_day - 10.minutes
+        scheduler = create(:scheduler, :timeplan, last_run: 6.hours.ago)
+        expect(instance.send(:last_execution_on_time?, scheduler)).to be_truthy
+      end
+
+      it 'returns true if timeplan scheduler was skipped once' do
+        travel_to Time.current.noon
+        scheduler = create(:scheduler, :timeplan, last_run: 1.day.ago)
+        expect(instance.send(:last_execution_on_time?, scheduler)).to be_truthy
+      end
+
+      it 'returns false if timeplan scheduler was skipped twice' do
+        travel_to Time.current.noon
+        scheduler = create(:scheduler, :timeplan, last_run: 2.days.ago)
+        expect(instance.send(:last_execution_on_time?, scheduler)).to be_falsey
+      end
     end
   end
 

spec/lib/timeplan_calculation_spec.rb

@@ -156,6 +156,101 @@ RSpec.describe TimeplanCalculation do
     end
   end
 
+  describe '#previous_at' do
+    context 'without a valid timeplan' do
+      let(:timeplan) { {} }
+
+      it { expect(instance.previous_at(Time.zone.now)).to be_nil }
+    end
+
+    context 'with monday 09:20' do
+      let(:timeplan) { { 'days' => { 'Mon' => true }, 'hours' => { '9' => true }, 'minutes' => { '20' => true } } }
+
+      it { expect(instance.previous_at(Time.zone.parse('2020-12-28 09:31'))).to eq(Time.zone.parse('2020-12-28 09:20')) }
+      it { expect(instance.previous_at(Time.zone.parse('2020-12-28 09:30'))).to eq(Time.zone.parse('2020-12-28 09:20')) }
+      it { expect(instance.previous_at(Time.zone.parse('2020-12-28 09:20'))).to eq(Time.zone.parse('2020-12-28 09:20')) }
+      it { expect(instance.previous_at(Time.zone.parse('2020-12-21 09:21'))).to eq(Time.zone.parse('2020-12-21 09:20')) }
+      it { expect(instance.previous_at(Time.zone.parse('2020-12-21 09:35'))).to eq(Time.zone.parse('2020-12-21 09:20')) }
+      it { expect(instance.previous_at(Time.zone.parse('2020-12-21 10:20'))).to eq(Time.zone.parse('2020-12-21 09:20')) }
+      it { expect(instance.previous_at(Time.zone.parse('2020-12-21 10:21'))).to eq(Time.zone.parse('2020-12-21 09:20')) }
+      it { expect(instance.previous_at(Time.zone.parse('2020-12-21 9:10'))).to eq(Time.zone.parse('2020-12-14 09:20')) }
+      it { expect(instance.previous_at(Time.zone.parse('2020-12-22 9:20'))).to eq(Time.zone.parse('2020-12-21 09:20')) }
+      it { expect(instance.previous_at(Time.zone.parse('2020-12-20 9:20'))).to eq(Time.zone.parse('2020-12-14 09:20')) }
+    end
+
+    context 'with monday 23:00' do
+      let(:timeplan) { { 'days' => { 'Mon' => true }, 'hours' => { '23' => true }, 'minutes' => { '0' => true } } }
+
+      it { expect(instance.previous_at(Time.zone.parse('2020-12-29 00:00'))).to eq(Time.zone.parse('2020-12-28 23:00')) }
+    end
+
+    context 'with monday and tuesday 09:20' do
+      let(:timeplan) { { 'days' => { 'Mon' => true, 'Tue' => true }, 'hours' => { '9' => true }, 'minutes' => { '20' => true } } }
+
+      it { expect(instance.previous_at(Time.zone.parse('2020-12-28 09:30'))).to eq(Time.zone.parse('2020-12-28 09:20')) }
+      it { expect(instance.previous_at(Time.zone.parse('2020-12-29 09:30'))).to eq(Time.zone.parse('2020-12-29 09:20')) }
+      it { expect(instance.previous_at(Time.zone.parse('2020-12-21 09:25'))).to eq(Time.zone.parse('2020-12-21 09:20')) }
+      it { expect(instance.previous_at(Time.zone.parse('2020-12-21 09:35'))).to eq(Time.zone.parse('2020-12-21 09:20')) }
+      it { expect(instance.previous_at(Time.zone.parse('2020-12-22 8:20'))).to eq(Time.zone.parse('2020-12-21 09:20')) }
+      it { expect(instance.previous_at(Time.zone.parse('2020-12-22 8:21'))).to eq(Time.zone.parse('2020-12-21 09:20')) }
+      it { expect(instance.previous_at(Time.zone.parse('2020-12-21 9:30'))).to eq(Time.zone.parse('2020-12-21 09:20')) }
+      it { expect(instance.previous_at(Time.zone.parse('2020-12-22 9:10'))).to eq(Time.zone.parse('2020-12-21 09:20')) }
+      it { expect(instance.previous_at(Time.zone.parse('2020-12-23 9:10'))).to eq(Time.zone.parse('2020-12-22 09:20')) }
+    end
+
+    context 'with monday 09:20 and 10:20' do
+      let(:timeplan) { { 'days' => { 'Mon' => true }, 'hours' => { '9' => true, '10' => true }, 'minutes' => { '20' => true } } }
+
+      it { expect(instance.previous_at(Time.zone.parse('2020-12-29 09:10'))).to eq(Time.zone.parse('2020-12-28 10:20')) }
+      it { expect(instance.previous_at(Time.zone.parse('2020-12-21 09:30'))).to eq(Time.zone.parse('2020-12-21 09:20')) }
+      it { expect(instance.previous_at(Time.zone.parse('2020-12-21 09:35'))).to eq(Time.zone.parse('2020-12-21 09:20')) }
+      it { expect(instance.previous_at(Time.zone.parse('2020-12-22 08:07'))).to eq(Time.zone.parse('2020-12-21 10:20')) }
+      it { expect(instance.previous_at(Time.zone.parse('2020-12-21 10:30'))).to eq(Time.zone.parse('2020-12-21 10:20')) }
+      it { expect(instance.previous_at(Time.zone.parse('2020-12-21 10:31'))).to eq(Time.zone.parse('2020-12-21 10:20')) }
+      it { expect(instance.previous_at(Time.zone.parse('2020-12-21 9:10'))).to eq(Time.zone.parse('2020-12-14 10:20')) }
+      it { expect(instance.previous_at(Time.zone.parse('2020-12-21 9:47'))).to eq(Time.zone.parse('2020-12-21 09:20')) }
+      it { expect(instance.previous_at(Time.zone.parse('2020-12-22 9:20'))).to eq(Time.zone.parse('2020-12-21 10:20')) }
+      it { expect(instance.previous_at(Time.zone.parse('2020-12-23 9:30'))).to eq(Time.zone.parse('2020-12-21 10:20')) }
+    end
+
+    context 'with monday 09:30 and 9:10' do
+      let(:timeplan) { { 'days' => { 'Mon' => true }, 'hours' => { '9' => true }, 'minutes' => { '30' => true, '10' => true } } }
+
+      it { expect(instance.previous_at(Time.zone.parse('2020-12-28 09:40'))).to eq(Time.zone.parse('2020-12-28 09:30')) }
+      it { expect(instance.previous_at(Time.zone.parse('2020-12-28 07:40'))).to eq(Time.zone.parse('2020-12-21 09:30')) }
+      it { expect(instance.previous_at(Time.zone.parse('2020-12-21 09:30'))).to eq(Time.zone.parse('2020-12-21 09:30')) }
+      it { expect(instance.previous_at(Time.zone.parse('2020-12-21 09:25'))).to eq(Time.zone.parse('2020-12-21 09:10')) }
+      it { expect(instance.previous_at(Time.zone.parse('2020-12-21 09:35'))).to eq(Time.zone.parse('2020-12-21 09:30')) }
+      it { expect(instance.previous_at(Time.zone.parse('2020-12-21 05:45'))).to eq(Time.zone.parse('2020-12-14 09:30')) }
+      it { expect(instance.previous_at(Time.zone.parse('2020-12-21 14:07'))).to eq(Time.zone.parse('2020-12-21 09:30')) }
+      it { expect(instance.previous_at(Time.zone.parse('2020-12-28 10:20'))).to eq(Time.zone.parse('2020-12-28 09:30')) }
+      it { expect(instance.previous_at(Time.zone.parse('2020-12-28 10:21'))).to eq(Time.zone.parse('2020-12-28 09:30')) }
+      it { expect(instance.previous_at(Time.zone.parse('2020-12-21 9:10'))).to eq(Time.zone.parse('2020-12-21 09:10')) }
+      it { expect(instance.previous_at(Time.zone.parse('2020-12-21 9:20'))).to eq(Time.zone.parse('2020-12-21 09:10')) }
+      it { expect(instance.previous_at(Time.zone.parse('2020-12-22 9:20'))).to eq(Time.zone.parse('2020-12-21 09:30')) }
+      it { expect(instance.previous_at(Time.zone.parse('2020-12-25 9:20'))).to eq(Time.zone.parse('2020-12-21 09:30')) }
+    end
+
+    context 'with monday 01:00 and time zone' do
+      let(:timezone) { 'Europe/Vilnius' }
+      let(:timeplan) { { 'days' => { 'Mon' => true }, 'hours' => { '1' => true }, 'minutes' => { '0' => true } } }
+
+      it 'calculates time respecting time zone' do
+        from = Time.use_zone('Europe/Vilnius') { Time.zone.parse('2020-12-21 00:15') }
+        to   = Time.use_zone('Europe/Vilnius') { Time.zone.parse('2020-12-14 01:00') }
+
+        expect(instance.previous_at(from)).to eq(to)
+      end
+
+      it 'calculates time converting to time zone' do
+        from = Time.zone.parse('2020-12-20 23:10')
+        to   = Time.use_zone('Europe/Vilnius') { Time.zone.parse('2020-12-21 01:00') }
+
+        expect(instance.previous_at(from)).to eq(to)
+      end
+    end
+  end
+
   describe 'legacy tests moved from Job model' do
     let(:job)      { create(:job, :never_on) }
     let(:timeplan) { job.timeplan }