From f70f6bf1a149252c95cdbbaa438a4e2864cf14bd Mon Sep 17 00:00:00 2001 From: Brandon Keepers Date: Sun, 9 Oct 2011 09:16:39 -0400 Subject: [PATCH] Added hooks around perform, complete. release, and failure --- lib/qu.rb | 1 + lib/qu/hooks.rb | 75 +++++++++++++++++ lib/qu/job.rb | 3 + lib/qu/payload.rb | 24 ++++-- spec/qu/hooks_spec.rb | 182 ++++++++++++++++++++++++++++++++++++++++ spec/qu/job_spec.rb | 8 ++ spec/qu/payload_spec.rb | 43 +++++++++- spec/spec_helper.rb | 2 +- 8 files changed, 325 insertions(+), 13 deletions(-) create mode 100644 lib/qu/hooks.rb create mode 100644 spec/qu/hooks_spec.rb diff --git a/lib/qu.rb b/lib/qu.rb index bb3626c..b0b09c4 100644 --- a/lib/qu.rb +++ b/lib/qu.rb @@ -1,5 +1,6 @@ require 'qu/version' require 'qu/logger' +require 'qu/hooks' require 'qu/failure' require 'qu/payload' require 'qu/job' diff --git a/lib/qu/hooks.rb b/lib/qu/hooks.rb new file mode 100644 index 0000000..fc46a57 --- /dev/null +++ b/lib/qu/hooks.rb @@ -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 \ No newline at end of file diff --git a/lib/qu/job.rb b/lib/qu/job.rb index c831e36..ef18c36 100644 --- a/lib/qu/job.rb +++ b/lib/qu/job.rb @@ -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) diff --git a/lib/qu/payload.rb b/lib/qu/payload.rb index abd6096..3bf306b 100644 --- a/lib/qu/payload.rb +++ b/lib/qu/payload.rb @@ -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 diff --git a/spec/qu/hooks_spec.rb b/spec/qu/hooks_spec.rb new file mode 100644 index 0000000..5b6dd15 --- /dev/null +++ b/spec/qu/hooks_spec.rb @@ -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 diff --git a/spec/qu/job_spec.rb b/spec/qu/job_spec.rb index 5d2792a..5dca19c 100644 --- a/spec/qu/job_spec.rb +++ b/spec/qu/job_spec.rb @@ -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' diff --git a/spec/qu/payload_spec.rb b/spec/qu/payload_spec.rb index 44692b8..cd366c0 100644 --- a/spec/qu/payload_spec.rb +++ b/spec/qu/payload_spec.rb @@ -29,14 +29,30 @@ end end + describe 'job' do + subject { Qu::Payload.new(:klass => SimpleJob) } + + it 'should load the job' do + SimpleJob.should_receive(:load).with(subject) + subject.job + end + + it 'should return the job' do + subject.job.should be_instance_of(SimpleJob) + end + end + describe 'perform' do subject { Qu::Payload.new(:klass => SimpleJob) } - it 'should load job and call perform' do - job = mock('job instance') - job.should_receive(:perform) - SimpleJob.should_receive(:load).with(subject).and_return(job) + it 'should call perform on job' do + subject.job.should_receive(:perform) + subject.perform + end + it 'should run perform hooks' do + subject.job.stub(:run_hook).and_yield + subject.job.should_receive(:run_hook).with(:perform) subject.perform end @@ -45,6 +61,12 @@ subject.perform end + it 'should run complete hooks' do + subject.job.stub(:run_hook).and_yield + subject.job.should_receive(:run_hook).with(:complete) + subject.perform + end + context 'when being aborted' do before do SimpleJob.any_instance.stub(:perform).and_raise(Qu::Worker::Abort) @@ -54,6 +76,12 @@ Qu.backend.should_receive(:release).with(subject) lambda { subject.perform }.should raise_error(Qu::Worker::Abort) end + + it 'should run release hook' do + subject.job.stub(:run_hook).and_yield + subject.job.should_receive(:run_hook).with(:release) + lambda { subject.perform }.should raise_error(Qu::Worker::Abort) + end end context 'when the job raises an error' do @@ -78,6 +106,13 @@ Qu.failure.should_receive(:create).with(subject, error) subject.perform end + + it 'should run failure hook with exception' do + subject.job.stub(:run_hook).and_yield + subject.job.should_receive(:run_hook).with(:failure, error) + subject.perform + end + end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 24bf4e8..5be90d1 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -5,7 +5,7 @@ RSpec.configure do |config| config.before do - Qu.backend = mock('a backend', :reserve => nil, :failed => nil, :completed => nil, + Qu.backend = mock('a backend', :reserve => nil, :failed => nil, :completed => nil, :release => nil, :register_worker => nil, :unregister_worker => nil) Qu.failure = nil end