From 737c6067104e06a9b96c11956062e10eccebf9e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Wed, 13 Jan 2016 09:08:26 +0100 Subject: [PATCH] FIX: 'cancel_scheduled_job' wasn't working due to sidekiq upgrade --- app/jobs/base.rb | 27 +++++++++--------------- app/models/topic.rb | 2 +- config/initializers/100-sidekiq.rb | 2 -- spec/jobs/jobs_spec.rb | 33 +++++++++++++++++------------- 4 files changed, 30 insertions(+), 34 deletions(-) diff --git a/app/jobs/base.rb b/app/jobs/base.rb index 60bbd75f0b5..427a720079d 100644 --- a/app/jobs/base.rb +++ b/app/jobs/base.rb @@ -238,30 +238,23 @@ module Jobs end def self.cancel_scheduled_job(job_name, params={}) - jobs = scheduled_for(job_name, params) - return false if jobs.empty? - jobs.each { |job| job.delete } - true + scheduled_for(job_name, params).each(&:delete) end def self.scheduled_for(job_name, params={}) + params = params.with_indifferent_access job_class = "Jobs::#{job_name.to_s.camelcase}" Sidekiq::ScheduledSet.new.select do |scheduled_job| - if scheduled_job.klass == 'Sidekiq::Extensions::DelayedClass' - job_args = YAML.load(scheduled_job.args[0]) - job_args_class, _, (job_args_params, *) = job_args - if job_args_class.to_s == job_class && job_args_params - matched = true - params.each do |key, value| - unless job_args_params[key] == value - matched = false - break - end + if scheduled_job.klass.to_s == job_class + matched = true + job_params = scheduled_job.item["args"][0].with_indifferent_access + params.each do |key, value| + if job_params[key] != value + matched = false + break end - matched - else - false end + matched else false end diff --git a/app/models/topic.rb b/app/models/topic.rb index 82472fa410a..39b233977c2 100644 --- a/app/models/topic.rb +++ b/app/models/topic.rb @@ -208,7 +208,7 @@ class Topic < ActiveRecord::Base def cancel_auto_close_job if (auto_close_at_changed? && !auto_close_at_was.nil?) || (auto_close_user_id_changed? && auto_close_at) self.auto_close_started_at ||= Time.zone.now if auto_close_at - Jobs.cancel_scheduled_job(:close_topic, { topic_id: id }) + Jobs.cancel_scheduled_job(:close_topic, topic_id: id) end end diff --git a/config/initializers/100-sidekiq.rb b/config/initializers/100-sidekiq.rb index bfac8028f19..3940cdd4c8e 100644 --- a/config/initializers/100-sidekiq.rb +++ b/config/initializers/100-sidekiq.rb @@ -74,5 +74,3 @@ end Sidekiq.error_handlers.clear Sidekiq.error_handlers << SidekiqLogsterReporter.new - - diff --git a/spec/jobs/jobs_spec.rb b/spec/jobs/jobs_spec.rb index fa31b7ea18d..57d2717b255 100644 --- a/spec/jobs/jobs_spec.rb +++ b/spec/jobs/jobs_spec.rb @@ -1,3 +1,4 @@ +require "sidekiq/testing" require 'rails_helper' require_dependency 'jobs/base' @@ -76,23 +77,27 @@ describe Jobs do end describe 'cancel_scheduled_job' do + it 'deletes the matching job' do - job_to_delete = stub_everything(klass: 'Sidekiq::Extensions::DelayedClass', args: [YAML.dump(['Jobs::DrinkBeer', :delayed_perform, [{beer_id: 42}]])]) - job_to_delete.expects(:delete) - job_to_keep1 = stub_everything(klass: 'Sidekiq::Extensions::DelayedClass', args: [YAML.dump(['Jobs::DrinkBeer', :delayed_perform, [{beer_id: 43}]])]) - job_to_keep1.expects(:delete).never - job_to_keep2 = stub_everything(klass: 'Sidekiq::Extensions::DelayedClass', args: [YAML.dump(['Jobs::DrinkBeer', :delayed_perform, [{beer_id: 44}]])]) - job_to_keep2.expects(:delete).never - Sidekiq::ScheduledSet.stubs(:new).returns( [job_to_keep1, job_to_delete, job_to_keep2] ) - expect(Jobs.cancel_scheduled_job(:drink_beer, {beer_id: 42})).to eq(true) + SiteSetting.queue_jobs = true + + Sidekiq::Testing.disable! do + scheduled_jobs = Sidekiq::ScheduledSet.new + scheduled_jobs.clear + + expect(scheduled_jobs.size).to eq(0) + + Jobs.enqueue_in(1.year, :run_heartbeat, topic_id: 1234) + Jobs.enqueue_in(2.years, :run_heartbeat, topic_id: 5678) + + expect(scheduled_jobs.size).to eq(2) + + Jobs.cancel_scheduled_job(:run_heartbeat, topic_id: 1234) + + expect(scheduled_jobs.size).to eq(1) + end end - it 'returns false when no matching job is scheduled' do - job_to_keep = stub_everything(klass: 'Sidekiq::Extensions::DelayedClass', args: [YAML.dump(['Jobs::DrinkBeer', :delayed_perform, [{beer_id: 43}]])]) - job_to_keep.expects(:delete).never - Sidekiq::ScheduledSet.stubs(:new).returns( [job_to_keep] ) - expect(Jobs.cancel_scheduled_job(:drink_beer, {beer_id: 42})).to eq(false) - end end describe 'enqueue_at' do