Permalink
Browse files

Added hooks around perform, complete. release, and failure

  • Loading branch information...
1 parent 7240117 commit f70f6bf1a149252c95cdbbaa438a4e2864cf14bd @bkeepers committed Oct 9, 2011
Showing with 325 additions and 13 deletions.
  1. +1 −0 lib/qu.rb
  2. +75 −0 lib/qu/hooks.rb
  3. +3 −0 lib/qu/job.rb
  4. +16 −8 lib/qu/payload.rb
  5. +182 −0 spec/qu/hooks_spec.rb
  6. +8 −0 spec/qu/job_spec.rb
  7. +39 −4 spec/qu/payload_spec.rb
  8. +1 −1 spec/spec_helper.rb
View
@@ -1,5 +1,6 @@
require 'qu/version'
require 'qu/logger'
+require 'qu/hooks'
require 'qu/failure'
require 'qu/payload'
require 'qu/job'
View
@@ -0,0 +1,75 @@
+module Qu
+ module Hooks
+ def self.included(base)
+ base.extend ClassMethods
+ base.send :include, InstanceMethods
+ end
+
+ module ClassMethods
+ def define_hooks(*hooks)
+ hooks.each do |hook|
+ %w(before after around).each do |kind|
+ class_eval <<-end_eval, __FILE__, __LINE__
+ def self.#{kind}_#{hook}(*methods)
+ hooks(:#{hook}).add(:#{kind}, *methods)
+ end
+ end_eval
+ end
+ end
+ end
+
+ def hooks(name)
+ @hooks ||= {}
+ @hooks[name] ||= Chain.new
+ end
+ end
+
+ module InstanceMethods
+ def run_hook(name, *args, &block)
+ hooks = if self.class.superclass < Qu::Hooks
+ self.class.superclass.hooks(name).dup.concat self.class.hooks(name)
+ else
+ self.class.hooks(name)
+ end
+
+ hooks.run(self, args, &block)
+ end
+
+ def halt
+ throw :halt
+ end
+ end
+
+ class Chain < Array
+ def run(object, args, &block)
+ catch :halt do
+ reverse.inject(block) do |chain, hook|
+ lambda { hook.call(object, args, &chain) }
+ end.call
+ end
+ end
+
+ def add(kind, *methods)
+ methods.each {|method| self << Hook.new(kind, method) }
+ end
+ end
+
+ class Hook
+ attr_reader :type, :method
+
+ def initialize(type, method)
+ @type, @method = type, method
+ end
+
+ def call(obj, args, &chain)
+ if type == :around
+ obj.send method, *args, &chain
+ else
+ obj.send method, *args if type == :before
+ chain.call
+ obj.send method, *args if type == :after
+ end
+ end
+ end
+ end
+end
View
@@ -1,5 +1,8 @@
module Qu
class Job
+ include Qu::Hooks
+ define_hooks :perform, :complete, :release, :failure
+
attr_accessor :payload
def self.queue(name = nil)
View
@@ -15,22 +15,30 @@ def klass
constantize(super)
end
+ def job
+ @job ||= klass.load(self)
+ end
+
def queue
(klass.instance_variable_get(:@queue) || 'default').to_s
end
def perform
- klass.load(self).perform
- Qu.backend.completed(self)
+ job.run_hook(:perform) { job.perform }
+ job.run_hook(:complete) { Qu.backend.completed(self) }
rescue Qu::Worker::Abort
- logger.debug "Releasing job #{self}"
- Qu.backend.release(self)
+ job.run_hook(:release) do
+ logger.debug "Releasing job #{self}"
+ Qu.backend.release(self)
+ end
raise
rescue Exception => e
- logger.fatal "Job #{self} failed"
- log_exception(e)
- Qu.failure.create(self, e) if Qu.failure
- Qu.backend.failed(self, e)
+ job.run_hook(:failure, e) do
+ logger.fatal "Job #{self} failed"
+ log_exception(e)
+ Qu.failure.create(self, e) if Qu.failure
+ Qu.backend.failed(self, e)
+ end
end
def to_s
View
@@ -0,0 +1,182 @@
+require 'spec_helper'
+
+describe Qu::Hooks do
+ before do
+ class Pirate
+ include Qu::Hooks
+ define_hooks :pillage, :plunder
+
+ attr_reader :events
+
+ def initialize
+ @events = []
+ end
+
+ def pillage
+ run_hook(:pillage) do
+ @events << :pillage
+ end
+ end
+
+ private
+
+ def drink
+ @events << :drink
+ end
+
+ def be_merry
+ @events << :be_merry
+ end
+
+ def rest
+ @events << :rest_before
+ yield
+ @events << :rest_after
+ end
+ end
+
+ class Captain < Pirate
+ def fight_peter_pan
+ @events << :fight
+ halt
+ end
+ end
+ end
+
+ after do
+ Object.send :remove_const, :Captain
+ Object.send :remove_const, :Pirate
+ end
+
+ let(:captain) { Captain.new }
+
+ describe 'define_hooks' do
+ it 'should create an empty chain' do
+ Captain.hooks(:pillage).should be_instance_of(Qu::Hooks::Chain)
+ Captain.hooks(:pillage).length.should == 0
+ end
+
+ it 'should define before, after and around methods' do
+ Captain.respond_to?(:before_pillage).should be_true
+ Captain.respond_to?(:after_pillage).should be_true
+ Captain.respond_to?(:around_pillage).should be_true
+ end
+ end
+
+ describe 'before_hook' do
+ it 'should add hook with given method' do
+ Captain.before_pillage :drink
+ captain.pillage
+ captain.events.should == [:drink, :pillage]
+ end
+
+ it 'should add hook with multiple methods' do
+ Captain.before_pillage :drink, :be_merry
+ captain.pillage
+ captain.events.should == [:drink, :be_merry, :pillage]
+ end
+
+ it 'should inherit hooks from parent class' do
+ Captain.before_pillage :be_merry
+ Pirate.before_pillage :drink
+
+ captain.pillage
+ captain.events.should == [:drink, :be_merry, :pillage]
+ end
+ end
+
+ describe 'after_hook' do
+ it 'should add hook with given method' do
+ Captain.after_pillage :drink
+ captain.pillage
+ captain.events.should == [:pillage, :drink]
+ end
+
+ it 'should add hook with multiple methods' do
+ Captain.after_pillage :drink, :be_merry
+ captain.pillage
+ captain.events.should == [:pillage, :be_merry, :drink]
+ end
+
+ it 'should run declared hooks in reverse order' do
+ Captain.after_pillage :drink
+ Captain.after_pillage :be_merry
+ captain.pillage
+ captain.events.should == [:pillage, :be_merry, :drink]
+ end
+ end
+
+ describe 'around_hook' do
+ it 'should add hook with given method' do
+ Captain.around_pillage :rest
+ captain.pillage
+ captain.events.should == [:rest_before, :pillage, :rest_after]
+ end
+
+ it 'should maintain order with before and after hooks' do
+ Captain.around_pillage :rest
+ Captain.before_pillage :drink
+ Captain.after_pillage :be_merry
+ captain.pillage
+ captain.events.should == [:rest_before, :drink, :pillage, :be_merry, :rest_after]
+ end
+
+ it 'should halt chain if it does not yield' do
+ Captain.around_pillage :drink
+ Captain.before_pillage :be_merry
+ captain.pillage
+ captain.events.should == [:drink]
+ end
+ end
+
+ describe 'run_hook' do
+ it 'should call block when no hooks are declared' do
+ captain.pillage
+ captain.events.should == [:pillage]
+ end
+
+ it 'should pass args to method' do
+ Captain.before_pillage :drink
+ captain.should_receive(:drink).with(:rum)
+ captain.run_hook(:pillage, :rum) { }
+ end
+
+ describe 'with a halt before' do
+ let(:captain) { Captain.new }
+
+ before do
+ Captain.before_pillage :fight_peter_pan, :drink
+ end
+
+ it 'should not call other hooks' do
+ captain.should_not_receive :drink
+ captain.run_hook(:pillage) {}
+ end
+
+ it 'should not invoke block' do
+ called = false
+ captain.run_hook(:pillage) { called = true }
+ called.should be_false
+ end
+ end
+
+ describe 'with a halt after' do
+ let(:captain) { Captain.new }
+
+ before do
+ Captain.after_pillage :drink, :fight_peter_pan
+ end
+
+ it 'should not call other hooks' do
+ captain.should_not_receive :drink
+ captain.run_hook(:pillage) {}
+ end
+
+ it 'should invoke block' do
+ called = false
+ captain.run_hook(:pillage) { called = true }
+ called.should be_true
+ end
+ end
+ end
+end
View
@@ -1,6 +1,14 @@
require 'spec_helper'
describe Qu::Job do
+ %w(perform complete failure release).each do |hook|
+ it "should define hooks for #{hooks}" do
+ Qu::Job.should respond_to("before_#{hook}")
+ Qu::Job.should respond_to("around_#{hook}")
+ Qu::Job.should respond_to("after_#{hook}")
+ end
+ end
+
describe '.queue' do
it 'should allow setting the queue name' do
CustomQueue.queue.should == 'custom'
Oops, something went wrong. Retry.

4 comments on commit f70f6bf

@stellard

How close is this to being ready for master? I was looking at doing this myself as I wanted to add a 'plugin'.

Have you thought about how (or if) you are going to allow people to extend qu? Are you thinking they would be separate projects or should they be included as a gem inside this one?

@bkeepers
Owner

@stellard It's getting close. I made a little more progress on it last night. There are a few other hooks that I need to add before releasing 0.2.

I want to have a clearly defined plugin API for allowing people to extend Qu. I'd like to include a fair number of common plugins in Qu itself (like a retry plugin), but I'll probably encourage people to make most of them as separate gems.

@stellard
@bkeepers
Owner

@stellard Sounds great. I don't personally have a need for it, but I know other people often ask for that. Why don't you start by implementing it as a stand-alone gem, and if it makes sense to pull it into core, then talk about it then.

If you're working off the hooks branch, the easiest thing to do would be to implement it using ActiveSupport::Concern. I wrote more about that here: http://opensoul.org/blog/archives/2011/02/07/concerning-activesupportconcern/

It might look something like this:

module Uniqify
  extend ActiveSupport::Concern

  included do
    before_enqueue :ensure_unique
  end

  def ensure_unique
    halt unless job_is_uniq?
  end

  def job_is_unique?
    true # use payload.klass and payload.args to check if job exists in queue
  end
end

class MyJob < Qu::Job
  include Uniqify

  # …
end

The challenge with this plugin is that the implementation will vary depending on the backend. I haven't really thought about how to handle this well with plugins. I'd love your feedback.

Please sign in to comment.