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

Fix schedule hooks when enqueuing configured job #792

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
21 changes: 10 additions & 11 deletions lib/resque/scheduler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -331,17 +331,16 @@ def enqueue_from_config(job_config)
# for non-existent classes (for example: running scheduler in
# one app that schedules for another.
if Class === klass
Resque::Scheduler::Plugin.run_before_delayed_enqueue_hooks(
klass, *params
)

# If the class is a custom job class, call self#scheduled on it.
# This allows you to do things like Resque.enqueue_at(timestamp,
# CustomJobClass). Otherwise, pass off to Resque.
if klass.respond_to?(:scheduled)
klass.scheduled(queue, klass_name, *params)
else
Resque.enqueue_to(queue, klass, *params)
Resque::Scheduler::Plugin.process_schedule_hooks(klass, *params) do
Resque::Scheduler::Plugin.run_before_delayed_enqueue_hooks(klass, *params)
# If the class is a custom job class, call self#scheduled on it.
# This allows you to do things like Resque.enqueue_at(timestamp,
# CustomJobClass). Otherwise, pass off to Resque.
if klass.respond_to?(:scheduled)
klass.scheduled(queue, klass_name, *params)
else
Resque.enqueue_to(queue, klass, *params)
end
end
else
# This will not run the before_hooks in rescue, but will at least
Expand Down
28 changes: 13 additions & 15 deletions lib/resque/scheduler/delaying_extensions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,24 +22,22 @@ def enqueue_at(timestamp, klass, *args)
# timestamp has passed. It respects Resque.inline option, by
# creating the job right away instead of adding to the queue.
def enqueue_at_with_queue(queue, timestamp, klass, *args)
return false unless plugin.run_before_schedule_hooks(klass, *args)

if Resque.inline? || timestamp.to_i <= Time.now.to_i
# Just create the job and let resque perform it right away with
# inline. If the class is a custom job class, call self#scheduled
# on it. This allows you to do things like
# Resque.enqueue_at(timestamp, CustomJobClass, :opt1 => val1).
# Otherwise, pass off to Resque.
if klass.respond_to?(:scheduled)
klass.scheduled(queue, klass.to_s, *args)
plugin.process_schedule_hooks(klass, *args) do
if Resque.inline? || timestamp.to_i <= Time.now.to_i
# Just create the job and let resque perform it right away with
# inline. If the class is a custom job class, call self#scheduled
# on it. This allows you to do things like
# Resque.enqueue_at(timestamp, CustomJobClass, :opt1 => val1).
# Otherwise, pass off to Resque.
if klass.respond_to?(:scheduled)
klass.scheduled(queue, klass.to_s, *args)
else
Resque.enqueue_to(queue, klass, *args)
end
else
Resque.enqueue_to(queue, klass, *args)
delayed_push(timestamp, job_to_hash_with_queue(queue, klass, args))
end
else
delayed_push(timestamp, job_to_hash_with_queue(queue, klass, args))
end

plugin.run_after_schedule_hooks(klass, *args)
end

# Identical to enqueue_at but takes number_of_seconds_from_now
Expand Down
9 changes: 9 additions & 0 deletions lib/resque/scheduler/plugin.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,15 @@
module Resque
module Scheduler
module Plugin
def self.process_schedule_hooks(klass, *args)
# the documentation states that if any before_schedule hook returns
# false, the process should not be enqueued
return unless run_before_schedule_hooks(klass, *args)

yield
run_after_schedule_hooks(klass, *args)
end

def self.hooks(job, pattern)
job.methods.grep(/^#{pattern}/).sort
end
Expand Down
42 changes: 30 additions & 12 deletions test/scheduler_hooks_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,36 @@
context 'scheduling jobs with hooks' do
setup { Resque.data_store.redis.flushall }

def config
{
'cron' => '* * * * *',
'class' => 'SomeRealClass',
'args' => '/tmp'
}
end

# helper to inspected the queue
def enqueued
Resque.redis.lrange("queue:#{SomeRealClass.queue}", 0, -1).map(&JSON.method(:parse))
end

test 'before_schedule and after_scheduler hooks are called when enqueued from config' do
SomeRealClass.expects(:before_schedule_example).with('/tmp')
SomeRealClass.expects(:after_schedule_example).with('/tmp')
Resque::Scheduler.enqueue(config)

assert_equal [{ 'class' => 'SomeRealClass', 'args' => ['/tmp'] }], enqueued
end

test 'any before_schedule returning false will halt the job from being enqueued' do
SomeRealClass.expects(:before_schedule_a).with('/tmp').returns(false)
SomeRealClass.expects(:before_schedule_b).with('/tmp')
SomeRealClass.expects(:after_schedule_example).never
Resque::Scheduler.enqueue(config)

assert_equal [], enqueued
end

test 'before_schedule hook that does not return false should be enqueued' do
enqueue_time = Time.now + 1
SomeRealClass.expects(:before_schedule_example).with(:foo)
Expand All @@ -23,12 +53,6 @@
end

test 'default failure hooks are called when enqueueing a job fails' do
config = {
'cron' => '* * * * *',
'class' => 'SomeRealClass',
'args' => '/tmp'
}

e = RuntimeError.new('custom error')
Resque::Scheduler.expects(:enqueue_from_config).raises(e)

Expand All @@ -38,12 +62,6 @@

test 'failure hooks are called when enqueueing a job fails' do
with_failure_handler(ExceptionHandlerClass) do
config = {
'cron' => '* * * * *',
'class' => 'SomeRealClass',
'args' => '/tmp'
}

e = RuntimeError.new('custom error')
Resque::Scheduler.expects(:enqueue_from_config).raises(e)

Expand Down
2 changes: 2 additions & 0 deletions test/scheduler_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,11 @@
Resque::Job.expects(:create).with(
SomeJobWithResqueHooks.queue, SomeJobWithResqueHooks, '/tmp'
)
SomeJobWithResqueHooks.expects(:before_schedule).with('/tmp')
SomeJobWithResqueHooks.expects(:before_delayed_enqueue_example).with('/tmp')
SomeJobWithResqueHooks.expects(:before_enqueue_example).with('/tmp')
SomeJobWithResqueHooks.expects(:after_enqueue_example).with('/tmp')
SomeJobWithResqueHooks.expects(:after_schedule).with('/tmp')

Resque::Scheduler.enqueue_from_config(config)
end
Expand Down
Loading