From b10a06aa6e753652bab897c4c068b8835440b003 Mon Sep 17 00:00:00 2001 From: andreas Date: Wed, 14 Mar 2012 11:04:36 +0100 Subject: [PATCH] Refactor hooks. - New module, ResqueScheduler::Plugin, for running hooks. - Refactored before_schedule- and after_schedule-hooks. - Added before_delayed_enqueue-hook. - Simplified tests for hooks. - Updated README. --- README.markdown | 21 +++++++++---- lib/resque/scheduler.rb | 1 + lib/resque_scheduler.rb | 18 ++--------- lib/resque_scheduler/plugin.rb | 25 ++++++++++++++++ test/scheduler_hooks_test.rb | 55 ++++++++-------------------------- test/scheduler_test.rb | 4 ++- 6 files changed, 60 insertions(+), 64 deletions(-) create mode 100644 lib/resque_scheduler/plugin.rb diff --git a/README.markdown b/README.markdown index 0d1e467a..5acfb76d 100644 --- a/README.markdown +++ b/README.markdown @@ -128,12 +128,6 @@ down when a particular job is supposed to be queue, they will simply "catch up" once they are started again. Jobs are guaranteed to run (provided they make it into the delayed queue) after their given queue_at time has passed. -Similar to `before_enqueue` and `after_enqueue` hooks provided in Resque -(>= 1.19.1), your jobs can specify one or more `before_schedule` and -`after_schedule` hooks, to be run before or after scheduling. If *any* of your -`before_schedule` hooks returns `false`, the job will *not* be scheduled and -your `after_schedule` hooks will *not* be run. - One other thing to note is that insertion into the delayed queue is O(log(n)) since the jobs are stored in a redis sorted set (zset). I can't imagine this being an issue for someone since redis is stupidly fast even at log(n), but full @@ -220,6 +214,21 @@ from the `config.time_zone` value, make sure it's the right format, e.g. with: A future version of resque-scheduler may do this for you. +#### Hooks + +Similar to the `before_enqueue`- and `after_enqueue`-hooks provided in Resque +(>= 1.19.1), your jobs can specify one or more of the following hooks: + +* `before_schedule`: Called with the job args before a job is placed on + the delayed queue. If the hook returns `false`, the job will not be placed on + the queue. +* `after_schedule`: Called with the job args after a job is placed on the + delayed queue. Any exception raised propagates up to the code with queued the + job. +* `before_delayed_enqueue`: Called with the job args after the job has been + removed from the delayed queue, but not yet put on a normal queue. It is + called before `before_enqueue`-hooks, and on the same job instance as the + `before_enqueue`-hooks will be invoked on. Return values are ignored. #### Support for resque-status (and other custom jobs) diff --git a/lib/resque/scheduler.rb b/lib/resque/scheduler.rb index 3c6b1323..ba1aa0ed 100644 --- a/lib/resque/scheduler.rb +++ b/lib/resque/scheduler.rb @@ -218,6 +218,7 @@ def enqueue_from_config(job_config) # for non-existent classes (for example: running scheduler in # one app that schedules for another if Class === klass + ResqueScheduler::Plugin.run_before_delayed_enqueue_hooks(klass, *params) Resque.enqueue(klass, *params) else # This will not run the before_hooks in rescue, but will at least diff --git a/lib/resque_scheduler.rb b/lib/resque_scheduler.rb index bd89abad..df48f824 100644 --- a/lib/resque_scheduler.rb +++ b/lib/resque_scheduler.rb @@ -4,6 +4,7 @@ require 'resque_scheduler/version' require 'resque/scheduler' require 'resque_scheduler/server' +require 'resque_scheduler/plugin' module ResqueScheduler @@ -119,16 +120,11 @@ def enqueue_at(timestamp, klass, *args) # a queue in which the job will be placed after the # timestamp has passed. def enqueue_at_with_queue(queue, timestamp, klass, *args) - before_hooks = before_schedule_hooks(klass).collect do |hook| - klass.send(hook,*args) - end - return false if before_hooks.any? { |result| result == false } + return false unless Plugin.run_before_schedule_hooks(klass, *args) delayed_push(timestamp, job_to_hash_with_queue(queue, klass, args)) - after_schedule_hooks(klass).collect do |hook| - klass.send(hook,*args) - end + Plugin.run_after_schedule_hooks(klass, *args) end # Identical to enqueue_at but takes number_of_seconds_from_now @@ -274,14 +270,6 @@ def validate_job!(klass) end end - def before_schedule_hooks(klass) - klass.methods.grep(/^before_schedule/).sort - end - - def after_schedule_hooks(klass) - klass.methods.grep(/^after_schedule/).sort - end - def prepare_schedule(schedule_hash) prepared_hash = {} schedule_hash.each do |name, job_spec| diff --git a/lib/resque_scheduler/plugin.rb b/lib/resque_scheduler/plugin.rb new file mode 100644 index 00000000..91b5b14f --- /dev/null +++ b/lib/resque_scheduler/plugin.rb @@ -0,0 +1,25 @@ +module ResqueScheduler + module Plugin + extend self + def hooks(job, pattern) + job.methods.grep(/^#{pattern}/).sort + end + + def run_hooks(job, pattern, *args) + results = hooks(job, pattern).collect do |hook| + job.send(hook, *args) + end + + results.all? { |result| result != false } + end + + def method_missing(method_name, *args, &block) + if method_name =~ /^run_(.*)_hooks$/ + job = args.shift + run_hooks job, $1, *args + else + super + end + end + end +end diff --git a/test/scheduler_hooks_test.rb b/test/scheduler_hooks_test.rb index 83384cba..d12016a0 100644 --- a/test/scheduler_hooks_test.rb +++ b/test/scheduler_hooks_test.rb @@ -1,52 +1,23 @@ require File.dirname(__FILE__) + '/test_helper' context "scheduling jobs with hooks" do - class JobThatCannotBeScheduledWithoutArguments < Resque::Job - @queue = :job_that_cannot_be_scheduled_without_arguments - def self.perform(*x);end - def self.before_schedule_return_nil_if_arguments_not_supplied(*args) - counters[:before_schedule] += 1 - return false if args.empty? - end - - def self.after_schedule_do_something(*args) - counters[:after_schedule] += 1 - end - - class << self - def counters - @counters ||= Hash.new{|h,k| h[k]=0} - end - def clean - counters.clear - self - end - end - end - setup do - Resque::Scheduler.dynamic = false - Resque.redis.del(:schedules) - Resque.redis.del(:schedules_changed) - Resque::Scheduler.mute = true - Resque::Scheduler.clear_schedule! - Resque::Scheduler.send(:class_variable_set, :@@scheduled_jobs, {}) + Resque.redis.flushall end - test "before_schedule hook that does not return false should not block" do - enqueue_time = Time.now + 12 - Resque.enqueue_at(enqueue_time, JobThatCannotBeScheduledWithoutArguments.clean, :foo) - assert_equal(1, Resque.delayed_timestamp_size(enqueue_time.to_i), "delayed queue should have one entry now") - assert_equal(1, JobThatCannotBeScheduledWithoutArguments.counters[:before_schedule], 'before_schedule was not run') - assert_equal(1, JobThatCannotBeScheduledWithoutArguments.counters[:after_schedule], 'after_schedule was not run') + test "before_schedule hook that does not return false should be enqueued" do + enqueue_time = Time.now + SomeRealClass.expects(:before_schedule_example).with(:foo) + SomeRealClass.expects(:after_schedule_example).with(:foo) + Resque.enqueue_at(enqueue_time.to_i, SomeRealClass, :foo) + assert_equal(1, Resque.delayed_timestamp_size(enqueue_time.to_i), "job should be enqueued") end - test "before_schedule hook that returns false should block" do - enqueue_time = Time.now + 60 - assert_equal(0, JobThatCannotBeScheduledWithoutArguments.clean.counters[:before_schedule], 'before_schedule should be zero') - Resque.enqueue_at(enqueue_time, JobThatCannotBeScheduledWithoutArguments.clean) - assert_equal(0, Resque.delayed_timestamp_size(enqueue_time.to_i), "job should not have been put in queue") - assert_equal(1, JobThatCannotBeScheduledWithoutArguments.counters[:before_schedule], 'before_schedule was not run') - assert_equal(0, JobThatCannotBeScheduledWithoutArguments.counters[:after_schedule], 'after_schedule was run') + test "before_schedule hook that returns false should not be enqueued" do + enqueue_time = Time.now + SomeRealClass.expects(:before_schedule_example).with(:foo).returns(false) + SomeRealClass.expects(:after_schedule_example).never + Resque.enqueue_at(enqueue_time.to_i, SomeRealClass, :foo) + assert_equal(0, Resque.delayed_timestamp_size(enqueue_time.to_i), "job should not be enqueued") end end diff --git a/test/scheduler_test.rb b/test/scheduler_test.rb index 00aefbbd..15222d25 100644 --- a/test/scheduler_test.rb +++ b/test/scheduler_test.rb @@ -25,7 +25,9 @@ config = {'cron' => "* * * * *", 'class' => 'SomeRealClass', 'args' => "/tmp"} Resque::Job.expects(:create).with(SomeRealClass.queue, SomeRealClass, '/tmp') - SomeRealClass.expects(:after_enqueue_example) + SomeRealClass.expects(:before_delayed_enqueue_example).with("/tmp") + SomeRealClass.expects(:before_enqueue_example).with("/tmp") + SomeRealClass.expects(:after_enqueue_example).with("/tmp") Resque::Scheduler.enqueue_from_config(config) end