Merge pull request #303 from serroba/BIG-28720

Allowing to use a factory instead of manually instantiate the Jobs

lib/Resque/Job.php

@@ -24,10 +24,15 @@ class Resque_Job
 	public $payload;
 
 	/**
-	 * @var object Instance of the class performing work for this job.
+	 * @var object|Resque_JobInterface Instance of the class performing work for this job.
 	 */
 	private $instance;
 
+	/**
+	 * @var Resque_Job_FactoryInterface
+	 */
+	private $jobFactory;
+
 	/**
 	 * Instantiate a new instance of a job.
 	 *
@@ -50,6 +55,7 @@ class Resque_Job
 	 * @param string $id Unique identifier for tracking the job. Generated if not supplied.
 	 *
 	 * @return string
+	 * @throws \InvalidArgumentException
 	 */
 	public static function create($queue, $class, $args = null, $monitor = false, $id = null)
 	{
@@ -76,41 +82,41 @@ class Resque_Job
 		return $id;
 	}
 
-    /**
+	/**
-     * Find the next available job from the specified queue and return an
+	 * Find the next available job from the specified queue and return an
-     * instance of Resque_Job for it.
+	 * instance of Resque_Job for it.
-     *
+	 *
-     * @param string $queue The name of the queue to check for a job in.
+	 * @param string $queue The name of the queue to check for a job in.
-     * @return null|object Null when there aren't any waiting jobs, instance of Resque_Job when a job was found.
+	 * @return false|object Null when there aren't any waiting jobs, instance of Resque_Job when a job was found.
-     */
+	 */
-    public static function reserve($queue)
+	public static function reserve($queue)
-    {
+	{
-        $payload = Resque::pop($queue);
+		$payload = Resque::pop($queue);
-        if(!is_array($payload)) {
+		if(!is_array($payload)) {
-            return false;
+			return false;
-        }
+		}
 
-        return new Resque_Job($queue, $payload);
+		return new Resque_Job($queue, $payload);
-    }
+	}
 
-    /**
+	/**
-     * Find the next available job from the specified queues using blocking list pop
+	 * Find the next available job from the specified queues using blocking list pop
-     * and return an instance of Resque_Job for it.
+	 * and return an instance of Resque_Job for it.
-     *
+	 *
-     * @param array             $queues
+	 * @param array             $queues
-     * @param int               $timeout
+	 * @param int               $timeout
-     * @return null|object Null when there aren't any waiting jobs, instance of Resque_Job when a job was found.
+	 * @return false|object Null when there aren't any waiting jobs, instance of Resque_Job when a job was found.
-     */
+	 */
-    public static function reserveBlocking(array $queues, $timeout = null)
+	public static function reserveBlocking(array $queues, $timeout = null)
-    {
+	{
-        $item = Resque::blpop($queues, $timeout);
+		$item = Resque::blpop($queues, $timeout);
 
-        if(!is_array($item)) {
+		if(!is_array($item)) {
-            return false;
+			return false;
-        }
+		}
 
-        return new Resque_Job($item['queue'], $item['payload']);
+		return new Resque_Job($item['queue'], $item['payload']);
-    }
+	}
 
 	/**
 	 * Update the status of the current job.
@@ -154,8 +160,8 @@ class Resque_Job
 
 	/**
 	 * Get the instantiated object for this job that will be performing work.
-	 *
+	 * @return Resque_JobInterface Instance of the object that this job belongs to.
-	 * @return object Instance of the object that this job belongs to.
+	 * @throws Resque_Exception
 	 */
 	public function getInstance()
 	{
@@ -163,23 +169,9 @@ class Resque_Job
 			return $this->instance;
 		}
 
-		if(!class_exists($this->payload['class'])) {
+        $this->instance = $this->getJobFactory()->create($this->payload['class'], $this->getArguments(), $this->queue);
-			throw new Resque_Exception(
+        $this->instance->job = $this;
-				'Could not find job class ' . $this->payload['class'] . '.'
+        return $this->instance;
-			);
-		}
-
-		if(!method_exists($this->payload['class'], 'perform')) {
-			throw new Resque_Exception(
-				'Job class ' . $this->payload['class'] . ' does not contain a perform method.'
-			);
-		}
-
-		$this->instance = new $this->payload['class'];
-		$this->instance->job = $this;
-		$this->instance->args = $this->getArguments();
-		$this->instance->queue = $this->queue;
-		return $this->instance;
 	}
 
 	/**
@@ -272,4 +264,26 @@ class Resque_Job
 		}
 		return '(' . implode(' | ', $name) . ')';
 	}
+
+	/**
+	 * @param Resque_Job_FactoryInterface $jobFactory
+	 * @return Resque_Job
+	 */
+	public function setJobFactory(Resque_Job_FactoryInterface $jobFactory)
+	{
+		$this->jobFactory = $jobFactory;
+
+		return $this;
+	}
+
+    /**
+     * @return Resque_Job_FactoryInterface
+     */
+    public function getJobFactory()
+    {
+        if ($this->jobFactory === null) {
+            $this->jobFactory = new Resque_Job_Factory();
+        }
+        return $this->jobFactory;
+    }
 }

lib/Resque/Job/Factory.php

@@ -0,0 +1,32 @@
+<?php
+
+class Resque_Job_Factory implements Resque_Job_FactoryInterface
+{
+
+    /**
+     * @param $className
+     * @param array $args
+     * @param $queue
+     * @return Resque_JobInterface
+     * @throws \Resque_Exception
+     */
+    public function create($className, $args, $queue)
+    {
+        if (!class_exists($className)) {
+            throw new Resque_Exception(
+                'Could not find job class ' . $className . '.'
+            );
+        }
+
+        if (!method_exists($className, 'perform')) {
+            throw new Resque_Exception(
+                'Job class ' . $className . ' does not contain a perform method.'
+            );
+        }
+
+        $instance = new $className;
+        $instance->args = $args;
+        $instance->queue = $queue;
+        return $instance;
+    }
+}

lib/Resque/Job/FactoryInterface.php

@@ -0,0 +1,12 @@
+<?php
+
+interface Resque_Job_FactoryInterface
+{
+	/**
+	 * @param $className
+	 * @param array $args
+	 * @param $queue
+	 * @return Resque_JobInterface
+	 */
+	public function create($className, $args, $queue);
+}

lib/Resque/JobInterface.php

@@ -0,0 +1,9 @@
+<?php
+
+interface Resque_JobInterface
+{
+	/**
+	 * @return bool
+	 */
+	public function perform();
+}

test/Resque/Tests/EventTest.php

@@ -31,7 +31,7 @@ class Resque_Tests_EventTest extends Resque_Tests_TestCase
 		$payload = array(
 			'class' => 'Test_Job',
 			'args' => array(
-				'somevar',
+				array('somevar'),
 			),
 		);
 		$job = new Resque_Job('jobs', $payload);

test/Resque/Tests/JobTest.php

@@ -183,16 +183,16 @@ class Resque_Tests_JobTest extends Resque_Tests_TestCase
 
 	public function testJobWithNamespace()
 	{
-	    Resque_Redis::prefix('php');
+		Resque_Redis::prefix('php');
-	    $queue = 'jobs';
+		$queue = 'jobs';
-	    $payload = array('another_value');
+		$payload = array('another_value');
-        Resque::enqueue($queue, 'Test_Job_With_TearDown', $payload);
+		Resque::enqueue($queue, 'Test_Job_With_TearDown', $payload);
 
-        $this->assertEquals(Resque::queues(), array('jobs'));
+		$this->assertEquals(Resque::queues(), array('jobs'));
-        $this->assertEquals(Resque::size($queue), 1);
+		$this->assertEquals(Resque::size($queue), 1);
 
-        Resque_Redis::prefix('resque');
+		Resque_Redis::prefix('resque');
-        $this->assertEquals(Resque::size($queue), 0);        
+		$this->assertEquals(Resque::size($queue), 0);
 	}
 
 	public function testDequeueAll()
@@ -362,4 +362,68 @@ class Resque_Tests_JobTest extends Resque_Tests_TestCase
 		$this->assertEquals(Resque::size($queue), 2);
 	}
 
+	public function testUseDefaultFactoryToGetJobInstance()
+	{
+		$payload = array(
+			'class' => 'Some_Job_Class',
+			'args' => null
+		);
+		$job = new Resque_Job('jobs', $payload);
+		$instance = $job->getInstance();
+		$this->assertInstanceOf('Some_Job_Class', $instance);
+	}
+
+	public function testUseFactoryToGetJobInstance()
+	{
+		$payload = array(
+			'class' => 'Some_Job_Class',
+			'args' => array(array())
+		);
+		$job = new Resque_Job('jobs', $payload);
+		$factory = new Some_Stub_Factory();
+		$job->setJobFactory($factory);
+		$instance = $job->getInstance();
+		$this->assertInstanceOf('Resque_JobInterface', $instance);
+	}
+
+	public function testDoNotUseFactoryToGetInstance()
+	{
+		$payload = array(
+			'class' => 'Some_Job_Class',
+			'args' => array(array())
+		);
+		$job = new Resque_Job('jobs', $payload);
+		$factory = $this->getMock('Resque_Job_FactoryInterface');
+		$testJob = $this->getMock('Resque_JobInterface');
+		$factory->expects(self::never())->method('create')->will(self::returnValue($testJob));
+		$instance = $job->getInstance();
+		$this->assertInstanceOf('Resque_JobInterface', $instance);
+	}
+}
+
+class Some_Job_Class implements Resque_JobInterface
+{
+
+	/**
+	 * @return bool
+	 */
+	public function perform()
+	{
+		return true;
+	}
+}
+
+class Some_Stub_Factory implements Resque_Job_FactoryInterface
+{
+
+	/**
+	 * @param $className
+	 * @param array $args
+	 * @param $queue
+	 * @return Resque_JobInterface
+	 */
+	public function create($className, $args, $queue)
+	{
+		return new Some_Job_Class();
+	}
 }

test/Resque/Tests/TestCase.php

@@ -11,6 +11,11 @@ class Resque_Tests_TestCase extends PHPUnit_Framework_TestCase
 	protected $resque;
 	protected $redis;
 
+	public static function setUpBeforeClass()
+	{
+		date_default_timezone_set('UTC');
+	}
+
 	public function setUp()
 	{
 		$config = file_get_contents(REDIS_CONF);