Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Manually deleted schedules #21

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 14 additions & 17 deletions lib/sidekiq/debounce.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
1 change: 1 addition & 0 deletions sidekiq-debounce.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -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
22 changes: 14 additions & 8 deletions spec/sidekiq/debounce_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
3 changes: 2 additions & 1 deletion spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,6 @@

require 'minitest/spec'
require 'minitest/autorun'
require 'mocha/mini_test'
require 'mocha/minitest'
require 'sidekiq_helper'
require "active_support/inflector"