From dc77bd92f43026b5a89180289e2a22c57c91f872 Mon Sep 17 00:00:00 2001 From: ac Date: Sat, 26 Jan 2019 15:18:16 +0900 Subject: [PATCH 1/2] pretest setups --- sidekiq-debounce.gemspec | 1 + spec/spec_helper.rb | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/sidekiq-debounce.gemspec b/sidekiq-debounce.gemspec index 29b9029..93fc59c 100644 --- a/sidekiq-debounce.gemspec +++ b/sidekiq-debounce.gemspec @@ -33,4 +33,5 @@ DESC spec.add_development_dependency 'simplecov' spec.add_development_dependency 'codeclimate-test-reporter', '~> 1.0.0' spec.add_development_dependency 'minitest' + spec.add_development_dependency 'activesupport' end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index d068894..b54c855 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -3,5 +3,6 @@ require 'minitest/spec' require 'minitest/autorun' -require 'mocha/mini_test' +require 'mocha/minitest' require 'sidekiq_helper' +require "active_support/inflector" From 1009766b5556b03e5efd72cfe777f8df57fadc58 Mon Sep 17 00:00:00 2001 From: ac Date: Sat, 26 Jan 2019 15:23:18 +0900 Subject: [PATCH 2/2] add a case when a scheduled job is manually deleted --- lib/sidekiq/debounce.rb | 31 ++++++++++++++----------------- spec/sidekiq/debounce_spec.rb | 22 ++++++++++++++-------- 2 files changed, 28 insertions(+), 25 deletions(-) diff --git a/lib/sidekiq/debounce.rb b/lib/sidekiq/debounce.rb index e7f0d77..6a3320c 100644 --- a/lib/sidekiq/debounce.rb +++ b/lib/sidekiq/debounce.rb @@ -11,15 +11,19 @@ def call(worker, msg, _queue, redis_pool = nil) block = Proc.new do |conn| # Get JID of the already-scheduled job, if there is one - scheduled_jid = conn.get(debounce_key) - - # Reschedule the old job to when this new job is scheduled for - # Or yield if there isn't one scheduled yet - jid = scheduled_jid ? reschedule(scheduled_jid, @msg['at']) : yield - - store_expiry(conn, jid, @msg['at']) - return false if scheduled_jid - jid + current_scheduled_job = scheduled_set.find_job(conn.get(debounce_key)) + if current_scheduled_job + # Reschedule the old job to when this new job is scheduled for + current_scheduled_job.reschedule(@msg['at']) + store_expiry(conn, current_scheduled_job.jid, @msg['at']) + false # gracefully ignore newly created scheduled job + else + # Or yield if there isn't one scheduled yet + conn.del(debounce_key) # just in case the scheduled job was deleted before the expiry + yield.tap do |job_hash| + store_expiry(conn, job_hash["jid"], @msg['at']) + end + end end if redis_pool @@ -31,8 +35,7 @@ def call(worker, msg, _queue, redis_pool = nil) private - def store_expiry(conn, job, time) - jid = job.respond_to?(:has_key?) && job.key?('jid') ? job['jid'] : job + def store_expiry(conn, jid, time) conn.set(debounce_key, jid) conn.expireat(debounce_key, time.to_i) end @@ -46,12 +49,6 @@ def scheduled_set @scheduled_set ||= Sidekiq::ScheduledSet.new end - def reschedule(jid, at) - job = scheduled_set.find_job(jid) - job.reschedule(at) unless job.nil? - jid - end - def debounce? (@msg['at'] && @worker.get_sidekiq_options['debounce']) || false end diff --git a/spec/sidekiq/debounce_spec.rb b/spec/sidekiq/debounce_spec.rb index 26faa19..4fe7755 100644 --- a/spec/sidekiq/debounce_spec.rb +++ b/spec/sidekiq/debounce_spec.rb @@ -11,10 +11,6 @@ def perform(_a, _b); end end describe Sidekiq::Debounce do - before do - stub_scheduled_set - end - after do Sidekiq.redis(&:flushdb) end @@ -27,12 +23,22 @@ def perform(_a, _b); end set.size.must_equal 1, 'set.size must be 1' end - it 'ignores repeat jobs within the debounce time and reschedules' do - sorted_entry.expects(:reschedule) + it 'ignores repeat jobs within the debounce time' do + DebouncedWorker.perform_in(60, 'foo', 'bar').wont_be_nil + DebouncedWorker.perform_in(60, 'foo', 'bar').must_be_nil + set.size.must_equal 1, 'set.size must be 1' + end + it "creates another job if the job is manually deleted within the expiry" do + DebouncedWorker.perform_in(60, 'foo', 'bar').wont_be_nil + set.each{|job| job.delete if job.klass == "DebouncedWorker" } + DebouncedWorker.perform_in(60, 'foo', 'bar').wont_be_nil + end + + it "reschedules" do + stub_scheduled_set + sorted_entry.expects(:reschedule) DebouncedWorker.perform_in(60, 'foo', 'bar') - DebouncedWorker.perform_in(60, 'foo', 'bar') - set.size.must_equal 1, 'set.size must be 1' end it 'debounces jobs based on their arguments' do