From f53bbd669bfe8cb8cccd0b29de44e26a10648852 Mon Sep 17 00:00:00 2001 From: John Weir Date: Mon, 8 Jul 2019 13:51:50 -0700 Subject: [PATCH 1/2] move config to a shared method --- test/scheduler_hooks_test.rb | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/test/scheduler_hooks_test.rb b/test/scheduler_hooks_test.rb index 1128384e..9f1a318f 100644 --- a/test/scheduler_hooks_test.rb +++ b/test/scheduler_hooks_test.rb @@ -4,6 +4,14 @@ context 'scheduling jobs with hooks' do setup { Resque.data_store.redis.flushall } + def config + { + 'cron' => '* * * * *', + 'class' => 'SomeRealClass', + 'args' => '/tmp' + } + 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) @@ -23,12 +31,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) @@ -38,12 +40,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) From a32d444d829a56acc2f63a0f4855301e93eba80e Mon Sep 17 00:00:00 2001 From: John Weir Date: Mon, 8 Jul 2019 15:08:54 -0700 Subject: [PATCH 2/2] enqueue(config) calls {before, after}_schedule methods https://github.com/resque/resque-scheduler#hooks enqueue_at works as documented, but schedules enqueued from a config have not been calling the before/after schedule hooks. fixes #679 --- lib/resque/scheduler.rb | 21 ++++++++-------- lib/resque/scheduler/delaying_extensions.rb | 28 ++++++++++----------- lib/resque/scheduler/plugin.rb | 9 +++++++ test/scheduler_hooks_test.rb | 22 ++++++++++++++++ test/scheduler_test.rb | 2 ++ 5 files changed, 56 insertions(+), 26 deletions(-) diff --git a/lib/resque/scheduler.rb b/lib/resque/scheduler.rb index 9f54c4d3..9c9be094 100644 --- a/lib/resque/scheduler.rb +++ b/lib/resque/scheduler.rb @@ -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 diff --git a/lib/resque/scheduler/delaying_extensions.rb b/lib/resque/scheduler/delaying_extensions.rb index 5a13cb41..7857ad1a 100644 --- a/lib/resque/scheduler/delaying_extensions.rb +++ b/lib/resque/scheduler/delaying_extensions.rb @@ -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 diff --git a/lib/resque/scheduler/plugin.rb b/lib/resque/scheduler/plugin.rb index bab898b4..0cd872f4 100644 --- a/lib/resque/scheduler/plugin.rb +++ b/lib/resque/scheduler/plugin.rb @@ -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 diff --git a/test/scheduler_hooks_test.rb b/test/scheduler_hooks_test.rb index 9f1a318f..532f1948 100644 --- a/test/scheduler_hooks_test.rb +++ b/test/scheduler_hooks_test.rb @@ -12,6 +12,28 @@ def config } 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) diff --git a/test/scheduler_test.rb b/test/scheduler_test.rb index ab47461d..124ae6b2 100644 --- a/test/scheduler_test.rb +++ b/test/scheduler_test.rb @@ -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