Merge pull request resque#943 from jaredonline/refactoring-perform

Refactoring `Resque::JobPerformer#perform`

lib/resque/job_performer.rb

@@ -1,52 +1,81 @@
 module Resque
   class JobPerformer
+    attr_reader :job, :job_args, :hooks
+
+    # This is the actual performer for a single unit of work.  It's called
+    # by Resque::Job#perform
+    # Args:
+    #   palyoad_class: The class to call ::perform on
+    #   args: An array of args to pass to the payload_class::perform
+    #   hook: A hash with keys :before, :after and :around, all arrays of
+    #         methods to call on the payload class with args
     def perform(payload_class, args, hooks)
-      job = payload_class
-      job_args = args || []
-      job_was_performed = false
+      @job      = payload_class
+      @job_args = args || []
+      @hooks    = hooks
+
+      # before_hooks can raise a Resque::DontPerform exception
+      # in which case we exit this method, returning false (because
+      # the job was never performed)
+      return false unless call_before_hooks
+      execute_job
+      call_hooks(:after)
 
-      # Execute before_perform hook. Abort the job gracefully if
-      # Resque::DontPerform is raised.
+      performed?
+    end
+
+    private
+    def call_before_hooks
       begin
-        hooks[:before].each do |hook|
-          job.send(hook, *job_args)
-        end
+        call_hooks(:before)
+        true
       rescue Resque::DontPerform
-        return false
+        false
       end
+    end
 
+    def execute_job
       # Execute the job. Do it in an around_perform hook if available.
       if hooks[:around].empty?
-        job.perform(*job_args)
-        job_was_performed = true
+        perform_job
       else
-        # We want to nest all around_perform plugins, with the last one
-        # finally calling perform
-        stack = hooks[:around].reverse.inject(nil) do |last_hook, hook|
-          if last_hook
-            lambda do
-              job.send(hook, *job_args) { last_hook.call }
-            end
-          else
-            lambda do
-              job.send(hook, *job_args) do
-                result = job.perform(*job_args)
-                job_was_performed = true
-                result
-              end
-            end
-          end
-        end
-        stack.call
+        call_around_hooks
       end
+    end
+
+    def call_around_hooks
+      nested_around_hooks.call
+    end
 
-      # Execute after_perform hook
-      hooks[:after].each do |hook|
-        job.send(hook, *job_args)
+    # We want to nest all around_perform plugins, with the last one
+    # finally calling perform
+    def nested_around_hooks
+      final_hook = lambda { perform_job }
+      hooks[:around].reverse.inject(final_hook) do |last_hook, hook|
+        lambda { perform_hook(hook) { last_hook.call } }
       end
+    end
+
+    def call_hooks(hook_type)
+      hooks[hook_type].each { |hook| perform_hook(hook) }
+    end
+
+    def perform_hook(hook, &block)
+      job.__send__(hook, *job_args, &block)
+    end
+
+    def perform_job
+      result = job.perform(*job_args)
+      job_performed
+      result
+    end
+
+    def performed?
+      @performed ||= false
+    end
 
-      # Return true if the job was performed
-      job_was_performed
+    def job_performed
+      @performed = true
     end
   end
 end

test/resque/job_performer_test.rb

@@ -0,0 +1,77 @@
+require 'test_helper'
+
+describe Resque::JobPerformer do
+  before do
+    @mock          = MiniTest::Mock.new
+    @job_performer = Resque::JobPerformer.new
+    @job_args = [:foo]
+  end
+
+  describe '#perform' do
+    before do
+      @options  = {
+        :before => [
+          :before_one,
+          :before_two
+        ],
+        :around => [],
+        :after  => [
+          :after_one,
+          :after_two
+        ]
+      }
+    end
+
+    it 'runs the before hooks' do
+      @mock.expect :before_one, true, @job_args
+      @mock.expect :before_two, true, @job_args
+      @mock.expect :perform, true, @job_args
+      @mock.expect :after_one, true, @job_args
+      @mock.expect :after_two, true, @job_args
+      @job_performer.perform(@mock, @job_args, @options).must_equal true
+      @mock.verify
+    end
+
+    it 'returns false when a before mock raises DontPerform' do
+      @options = {
+        :before => [:before_one],
+        :after  => [],
+        :around => []
+      }
+      def @mock.before_one(*args)
+        raise Resque::DontPerform
+      end
+      @mock.expect :perform, nil, @job_args
+      @job_performer.perform(@mock, @job_args, @options).must_equal false
+    end
+
+    describe 'when around_perform is present' do
+      before do
+        @options = {
+          :before => [],
+          :around => [
+            :around_one,
+            :around_two
+          ],
+          :after => []
+        }
+      end
+
+      it 'runs the around hooks' do
+        @mock.expect :perform, true, @job_args
+        @mock.expect :around_two, true, @job_args
+        @mock.expect :around_one, true, @job_args
+        def @mock.around_two(*args)
+          method_missing(:around_two, *args)
+          yield
+        end
+        def @mock.around_one(*args)
+          method_missing(:around_one, *args)
+          yield
+        end
+        @job_performer.perform(@mock, @job_args, @options)
+        @mock.verify
+      end
+    end
+  end
+end