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

@@ -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)
 

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

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|

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

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

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