From ce39a0b095c5a6e69c3f089ba11849a6b8180f1b Mon Sep 17 00:00:00 2001 From: John Weir Date: Mon, 8 Jul 2019 15:08:54 -0700 Subject: [PATCH] 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 523b1840..4777a47b 100644 --- a/lib/resque/scheduler.rb +++ b/lib/resque/scheduler.rb @@ -267,17 +267,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 ee000296..d0b1b9e7 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::Job.create(queue, klass, *args) + end else - Resque::Job.create(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 87019ad1..b4fd12cf 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 SomeRealClass.expects(:before_schedule_example).with(:foo) diff --git a/test/scheduler_test.rb b/test/scheduler_test.rb index 582aac3f..841bf69f 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