Permalink
Browse files

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.
  • Loading branch information...
1 parent 6d37cdd commit b10a06aa6e753652bab897c4c068b8835440b003 @andreas andreas committed Mar 14, 2012
Showing with 60 additions and 64 deletions.
  1. +15 −6 README.markdown
  2. +1 −0 lib/resque/scheduler.rb
  3. +3 −15 lib/resque_scheduler.rb
  4. +25 −0 lib/resque_scheduler/plugin.rb
  5. +13 −42 test/scheduler_hooks_test.rb
  6. +3 −1 test/scheduler_test.rb
View
@@ -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)
View
@@ -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
View
@@ -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|
@@ -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$/
@ouranos

ouranos Mar 30, 2012

Contributor

Isn't that always equal to nil or 0 ?

@andreas

andreas Mar 30, 2012

Contributor

Yeah, that's intended behaviour: 0 is interpreted as truthy and nil as falsy. Or do you see something I don't?

@ouranos

ouranos Apr 2, 2012

Contributor

My mistake, it was crashing and I assumed 0 was interpreted as falsy. All good.

+ job = args.shift
+ run_hooks job, $1, *args
+ else
+ super
+ end
+ end
+ end
+end
@@ -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
View
@@ -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

0 comments on commit b10a06a

Please sign in to comment.