From 349fd737047e1cf12cd698de71374322b93cd753 Mon Sep 17 00:00:00 2001 From: Heinz Wiesinger Date: Thu, 29 Dec 2022 16:40:27 +0100 Subject: [PATCH 1/3] Add base Job class --- lib/Job/Job.php | 60 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 lib/Job/Job.php diff --git a/lib/Job/Job.php b/lib/Job/Job.php new file mode 100644 index 0000000..34b7aee --- /dev/null +++ b/lib/Job/Job.php @@ -0,0 +1,60 @@ + + * @license http://www.opensource.org/licenses/mit-license.php + */ +abstract class Job +{ + /** + * Job arguments + * @var array + */ + public $args; + + /** + * Associated JobHandler instance + * @var JobHandler + */ + public $job; + + /** + * Name of the queue the job was in + * @var string + */ + public $queue; + + /** + * (Optional) Job setup + * + * @return void + */ + public function setUp(): void + { + // no-op + } + + /** + * (Optional) Job teardown + * + * @return void + */ + public function tearDown(): void + { + // no-op + } + + /** + * Main method of the Job + * + * @return mixed|void + */ + abstract public function perform(); +} From 4a7d48f8e61faf9b9cd32d9bbadcea1447a7190d Mon Sep 17 00:00:00 2001 From: Heinz Wiesinger Date: Sun, 28 Apr 2024 13:10:01 +0200 Subject: [PATCH 2/3] Require every job to extend the base Job class --- README.md | 8 +++--- lib/Job/Factory.php | 10 ++----- lib/Job/FactoryInterface.php | 4 +-- lib/Job/JobInterface.php | 11 -------- lib/JobHandler.php | 23 +++++++-------- test/Resque/Tests/JobHandlerTest.php | 14 +++++----- test/Resque/Tests/JobStatusTest.php | 42 ++++++++++++++++++++++++++++ test/bootstrap.php | 29 ++++++++++--------- 8 files changed, 83 insertions(+), 58 deletions(-) delete mode 100644 lib/Job/JobInterface.php diff --git a/README.md b/README.md index 7c71e01..8934a12 100644 --- a/README.md +++ b/README.md @@ -110,7 +110,7 @@ Resque\Resque::enqueue('default', 'My_Job', $args); Each job should be in its own class, and include a `perform` method. ```php -class My_Job +class My_Job extends \Resque\Job\Job { public function perform() { @@ -132,9 +132,9 @@ defined, it will be called before the `perform` method is run. The `tearDown` method, if defined, will be called after the job finishes. ```php -class My_Job +class My_Job extends \Resque\Job\Job { - public function setUp() + public function setUp(): void { // ... Set up environment for this job } @@ -144,7 +144,7 @@ class My_Job // .. Run job } - public function tearDown() + public function tearDown(): void { // ... Remove environment for this job } diff --git a/lib/Job/Factory.php b/lib/Job/Factory.php index 13c5277..08ff52f 100644 --- a/lib/Job/Factory.php +++ b/lib/Job/Factory.php @@ -10,10 +10,10 @@ class Factory implements FactoryInterface * @param $className * @param array $args * @param $queue - * @return \Resque\Job\JobInterface + * @return \Resque\Job\Job * @throws \Resque\Exceptions\ResqueException */ - public function create($className, $args, $queue) + public function create($className, $args, $queue): Job { if (!class_exists($className)) { throw new ResqueException( @@ -21,12 +21,6 @@ public function create($className, $args, $queue) ); } - if (!method_exists($className, 'perform')) { - throw new ResqueException( - 'Job class ' . $className . ' does not contain a perform method.' - ); - } - $instance = new $className(); $instance->args = $args; $instance->queue = $queue; diff --git a/lib/Job/FactoryInterface.php b/lib/Job/FactoryInterface.php index fc92941..926e165 100644 --- a/lib/Job/FactoryInterface.php +++ b/lib/Job/FactoryInterface.php @@ -8,7 +8,7 @@ interface FactoryInterface * @param $className * @param array $args * @param $queue - * @return \Resque\Job\JobInterface + * @return \Resque\Job\Job */ - public function create($className, $args, $queue); + public function create($className, $args, $queue): Job; } diff --git a/lib/Job/JobInterface.php b/lib/Job/JobInterface.php deleted file mode 100644 index b92a99f..0000000 --- a/lib/Job/JobInterface.php +++ /dev/null @@ -1,11 +0,0 @@ -instance)) { return $this->instance; @@ -212,9 +213,8 @@ public function getInstance() * Actually execute a job by calling the perform method on the class * associated with the job with the supplied arguments. * - * @return bool - * @throws Resque\Exceptions\ResqueException When the job's class could not be found - * or it does not contain a perform method. + * @return mixed + * @throws Resque\Exceptions\ResqueException When the job's class could not be found. */ public function perform() { @@ -225,15 +225,12 @@ public function perform() $this->startTime = microtime(true); $instance = $this->getInstance(); - if (is_callable([$instance, 'setUp'])) { - $instance->setUp(); - } + + $instance->setUp(); $result = $instance->perform(); - if (is_callable([$instance, 'tearDown'])) { - $instance->tearDown(); - } + $instance->tearDown(); $this->endTime = microtime(true); @@ -343,7 +340,7 @@ public function setJobFactory(FactoryInterface $jobFactory) /** * @return Resque\Job\FactoryInterface */ - public function getJobFactory() + public function getJobFactory(): FactoryInterface { if ($this->jobFactory === null) { $this->jobFactory = new Factory(); diff --git a/test/Resque/Tests/JobHandlerTest.php b/test/Resque/Tests/JobHandlerTest.php index 7cd9470..5683676 100644 --- a/test/Resque/Tests/JobHandlerTest.php +++ b/test/Resque/Tests/JobHandlerTest.php @@ -7,7 +7,7 @@ use \Resque\Redis; use \Resque\JobHandler; use \Resque\Stat; -use \Resque\Job\JobInterface; +use \Resque\Job\Job; use \Resque\Job\FactoryInterface; use \Test_Job_With_SetUp; use \Test_Job_With_TearDown; @@ -410,7 +410,7 @@ public function testUseFactoryToGetJobInstance() $factory = new Some_Stub_Factory(); $job->setJobFactory($factory); $instance = $job->getInstance(); - $this->assertInstanceOf('Resque\Job\JobInterface', $instance); + $this->assertInstanceOf('Resque\Job\Job', $instance); } public function testDoNotUseFactoryToGetInstance() @@ -422,11 +422,11 @@ public function testDoNotUseFactoryToGetInstance() $job = new JobHandler('jobs', $payload); $factory = $this->getMockBuilder('Resque\Job\FactoryInterface') ->getMock(); - $testJob = $this->getMockBuilder('Resque\Job\JobInterface') + $testJob = $this->getMockBuilder('Resque\Job\Job') ->getMock(); $factory->expects(self::never())->method('create')->will(self::returnValue($testJob)); $instance = $job->getInstance(); - $this->assertInstanceOf('Resque\Job\JobInterface', $instance); + $this->assertInstanceOf('Resque\Job\Job', $instance); } public function testJobStatusIsNullIfIdMissingFromPayload() @@ -505,7 +505,7 @@ public function testJobHandlerSetsStartAndEndTimeForFailedJob() } } -class Some_Job_Class implements JobInterface +class Some_Job_Class extends Job { /** @@ -524,9 +524,9 @@ class Some_Stub_Factory implements FactoryInterface * @param $className * @param array $args * @param $queue - * @return Resque\Job\JobInterface + * @return Resque\Job\Job */ - public function create($className, $args, $queue) + public function create($className, $args, $queue): Job { return new Some_Job_Class(); } diff --git a/test/Resque/Tests/JobStatusTest.php b/test/Resque/Tests/JobStatusTest.php index 827447f..ebc503c 100644 --- a/test/Resque/Tests/JobStatusTest.php +++ b/test/Resque/Tests/JobStatusTest.php @@ -6,6 +6,7 @@ use \Resque\Job\Status; use \Resque\JobHandler; use \Resque\Resque; +use \stdClass; /** * Status tests. @@ -30,6 +31,25 @@ public function setUp(): void $this->worker->setLogger($this->logger); } + /** + * Unit test data provider for potential return values from perform(). + * + * @return array + */ + public static function performResultProvider(): array + { + $data = []; + + $data['boolean'] = [ true ]; + $data['float'] = [ 1.0 ]; + $data['integer'] = [ 100 ]; + $data['string'] = [ 'string' ]; + $data['null'] = [ null ]; + $data['array'] = [[ 'key' => 'value' ]]; + + return $data; + } + public function testJobStatusCanBeTracked() { $token = Resque::enqueue('jobs', 'Test_Job', null, true); @@ -76,6 +96,28 @@ public function testCompletedJobReturnsCompletedStatus() $this->assertEquals(Status::STATUS_COMPLETE, $status->get()); } + /** + * @param mixed $value Potential return value from perform() + * + * @dataProvider performResultProvider + */ + public function testCompletedJobReturnsResult($value) + { + $token = Resque::enqueue('jobs', 'Returning_Job', [ 'return' => $value ], true); + $this->worker->work(0); + $status = new Status($token); + $this->assertEquals($value, $status->result()); + } + + public function testCompletedJobReturnsObjectResultAsArray() + { + $value = new stdClass(); + $token = Resque::enqueue('jobs', 'Returning_Job', [ 'return' => $value ], true); + $this->worker->work(0); + $status = new Status($token); + $this->assertEquals([], $status->result()); + } + public function testStatusIsNotTrackedWhenToldNotTo() { $token = Resque::enqueue('jobs', 'Test_Job', null, false); diff --git a/test/bootstrap.php b/test/bootstrap.php index 31f9402..dad6d00 100644 --- a/test/bootstrap.php +++ b/test/bootstrap.php @@ -85,7 +85,7 @@ function sigint() pcntl_signal(SIGTERM, 'sigint'); } -class Test_Job +class Test_Job extends \Resque\Job\Job { public static $called = false; @@ -95,12 +95,20 @@ public function perform() } } +class Returning_Job extends \Resque\Job\Job +{ + public function perform() + { + return $this->args['return'] ?? NULL; + } +} + class Failing_Job_Exception extends \Exception { } -class Failing_Job +class Failing_Job extends \Resque\Job\Job { public function perform() { @@ -114,7 +122,7 @@ public function perform() * * CAUTION Use this test job only with Worker::work, i.e. only when you actually trigger the fork in tests. */ -class InProgress_Job +class InProgress_Job extends \Resque\Job\Job { public function perform() { @@ -126,17 +134,12 @@ public function perform() } } -class Test_Job_Without_Perform_Method -{ - -} - -class Test_Job_With_SetUp +class Test_Job_With_SetUp extends \Resque\Job\Job { public static $called = false; public $args = false; - public function setUp() + public function setUp(): void { self::$called = true; } @@ -148,7 +151,7 @@ public function perform() } -class Test_Job_With_TearDown +class Test_Job_With_TearDown extends \Resque\Job\Job { public static $called = false; public $args = false; @@ -158,13 +161,13 @@ public function perform() } - public function tearDown() + public function tearDown(): void { self::$called = true; } } -class Test_Infinite_Recursion_Job +class Test_Infinite_Recursion_Job extends \Resque\Job\Job { public function perform() { From 18bf93532993e87ebedbbee80c84f2a5581f8c21 Mon Sep 17 00:00:00 2001 From: Heinz Wiesinger Date: Wed, 3 Jul 2024 10:12:19 +0200 Subject: [PATCH 3/3] Add more strict types for Job class properties --- lib/Job/Factory.php | 2 +- lib/Job/FactoryInterface.php | 2 +- lib/JobHandler.php | 11 ++--------- lib/Resque.php | 2 +- test/Resque/Tests/JobHandlerTest.php | 17 ++++------------- test/Resque/Tests/JobPIDTest.php | 6 +++--- test/Resque/Tests/JobStatusTest.php | 16 ++++++++-------- test/bootstrap.php | 4 ++-- 8 files changed, 22 insertions(+), 38 deletions(-) diff --git a/lib/Job/Factory.php b/lib/Job/Factory.php index 08ff52f..05b0bf6 100644 --- a/lib/Job/Factory.php +++ b/lib/Job/Factory.php @@ -13,7 +13,7 @@ class Factory implements FactoryInterface * @return \Resque\Job\Job * @throws \Resque\Exceptions\ResqueException */ - public function create($className, $args, $queue): Job + public function create(string $className, array $args, string $queue): Job { if (!class_exists($className)) { throw new ResqueException( diff --git a/lib/Job/FactoryInterface.php b/lib/Job/FactoryInterface.php index 926e165..c4f2d3a 100644 --- a/lib/Job/FactoryInterface.php +++ b/lib/Job/FactoryInterface.php @@ -10,5 +10,5 @@ interface FactoryInterface * @param $queue * @return \Resque\Job\Job */ - public function create($className, $args, $queue): Job; + public function create(string $className, array $args, string $queue): Job; } diff --git a/lib/JobHandler.php b/lib/JobHandler.php index 3c346c9..3279f0a 100755 --- a/lib/JobHandler.php +++ b/lib/JobHandler.php @@ -2,7 +2,6 @@ namespace Resque; -use InvalidArgumentException; use Resque\Job\PID; use Resque\Job\Status; use Resque\Exceptions\DoNotPerformException; @@ -84,19 +83,13 @@ public function __construct($queue, $payload) * @param string $prefix The prefix needs to be set for the status key * * @return string - * @throws \InvalidArgumentException */ - public static function create($queue, $class, $args = null, $monitor = false, $id = null, $prefix = "") + public static function create($queue, $class, array $args = [], $monitor = false, $id = null, $prefix = "") { if (is_null($id)) { $id = Resque::generateJobId(); } - if ($args !== null && !is_array($args)) { - throw new InvalidArgumentException( - 'Supplied $args must be an array.' - ); - } Resque::push($queue, array( 'class' => $class, 'args' => array($args), @@ -184,7 +177,7 @@ public function getStatus() * * @return array Array of arguments. */ - public function getArguments() + public function getArguments(): array { if (!isset($this->payload['args'])) { return array(); diff --git a/lib/Resque.php b/lib/Resque.php index 4e8ce60..384dfec 100644 --- a/lib/Resque.php +++ b/lib/Resque.php @@ -234,7 +234,7 @@ public static function size($queue) * * @return string|boolean Job ID when the job was created, false if creation was cancelled due to beforeEnqueue */ - public static function enqueue($queue, $class, $args = null, $trackStatus = false, $prefix = "") + public static function enqueue($queue, $class, array $args = [], $trackStatus = false, $prefix = "") { $id = Resque::generateJobId(); $hookParams = array( diff --git a/test/Resque/Tests/JobHandlerTest.php b/test/Resque/Tests/JobHandlerTest.php index 5683676..4b376ec 100644 --- a/test/Resque/Tests/JobHandlerTest.php +++ b/test/Resque/Tests/JobHandlerTest.php @@ -68,15 +68,6 @@ public function testQeueuedJobCanBeReserved() $this->assertEquals('Test_Job', $job->payload['class']); } - public function testObjectArgumentsCannotBePassedToJob() - { - $this->expectException('InvalidArgumentException'); - - $args = new stdClass(); - $args->test = 'somevalue'; - Resque::enqueue('jobs', 'Test_Job', $args); - } - public function testQueuedJobReturnsExactSamePassedInArguments() { $args = array( @@ -168,10 +159,10 @@ public function testJobWithSetUpCallbackFiresSetUp() { $payload = array( 'class' => 'Test_Job_With_SetUp', - 'args' => array( + 'args' => array(array( 'somevar', 'somevar2', - ), + )), ); $job = new JobHandler('jobs', $payload); $job->perform(); @@ -183,10 +174,10 @@ public function testJobWithTearDownCallbackFiresTearDown() { $payload = array( 'class' => 'Test_Job_With_TearDown', - 'args' => array( + 'args' => array(array( 'somevar', 'somevar2', - ), + )), ); $job = new JobHandler('jobs', $payload); $job->perform(); diff --git a/test/Resque/Tests/JobPIDTest.php b/test/Resque/Tests/JobPIDTest.php index f9c66c9..365d778 100644 --- a/test/Resque/Tests/JobPIDTest.php +++ b/test/Resque/Tests/JobPIDTest.php @@ -34,7 +34,7 @@ public function testQueuedJobDoesNotReturnPID() $this->logger->expects($this->never()) ->method('log'); - $token = Resque::enqueue('jobs', 'Test_Job', null, true); + $token = Resque::enqueue('jobs', 'Test_Job', [], true); $this->assertEquals(0, PID::get($token)); } @@ -43,14 +43,14 @@ public function testRunningJobReturnsPID() // Cannot use InProgress_Job on non-forking OS. if(!function_exists('pcntl_fork')) return; - $token = Resque::enqueue('jobs', 'InProgress_Job', null, true); + $token = Resque::enqueue('jobs', 'InProgress_Job', [], true); $this->worker->work(0); $this->assertNotEquals(0, PID::get($token)); } public function testFinishedJobDoesNotReturnPID() { - $token = Resque::enqueue('jobs', 'Test_Job', null, true); + $token = Resque::enqueue('jobs', 'Test_Job', [], true); $this->worker->work(0); $this->assertEquals(0, PID::get($token)); } diff --git a/test/Resque/Tests/JobStatusTest.php b/test/Resque/Tests/JobStatusTest.php index ebc503c..32d1735 100644 --- a/test/Resque/Tests/JobStatusTest.php +++ b/test/Resque/Tests/JobStatusTest.php @@ -52,28 +52,28 @@ public static function performResultProvider(): array public function testJobStatusCanBeTracked() { - $token = Resque::enqueue('jobs', 'Test_Job', null, true); + $token = Resque::enqueue('jobs', 'Test_Job', [], true); $status = new Status($token); $this->assertTrue($status->isTracking()); } public function testJobStatusIsReturnedViaJobInstance() { - $token = Resque::enqueue('jobs', 'Test_Job', null, true); + $token = Resque::enqueue('jobs', 'Test_Job', [], true); $job = JobHandler::reserve('jobs'); $this->assertEquals(Status::STATUS_WAITING, $job->getStatus()); } public function testQueuedJobReturnsQueuedStatus() { - $token = Resque::enqueue('jobs', 'Test_Job', null, true); + $token = Resque::enqueue('jobs', 'Test_Job', [], true); $status = new Status($token); $this->assertEquals(Status::STATUS_WAITING, $status->get()); } public function testRunningJobReturnsRunningStatus() { - $token = Resque::enqueue('jobs', 'Failing_Job', null, true); + $token = Resque::enqueue('jobs', 'Failing_Job', [], true); $job = $this->worker->reserve(); $this->worker->workingOn($job); $status = new Status($token); @@ -82,7 +82,7 @@ public function testRunningJobReturnsRunningStatus() public function testFailedJobReturnsFailedStatus() { - $token = Resque::enqueue('jobs', 'Failing_Job', null, true); + $token = Resque::enqueue('jobs', 'Failing_Job', [], true); $this->worker->work(0); $status = new Status($token); $this->assertEquals(Status::STATUS_FAILED, $status->get()); @@ -90,7 +90,7 @@ public function testFailedJobReturnsFailedStatus() public function testCompletedJobReturnsCompletedStatus() { - $token = Resque::enqueue('jobs', 'Test_Job', null, true); + $token = Resque::enqueue('jobs', 'Test_Job', [], true); $this->worker->work(0); $status = new Status($token); $this->assertEquals(Status::STATUS_COMPLETE, $status->get()); @@ -120,7 +120,7 @@ public function testCompletedJobReturnsObjectResultAsArray() public function testStatusIsNotTrackedWhenToldNotTo() { - $token = Resque::enqueue('jobs', 'Test_Job', null, false); + $token = Resque::enqueue('jobs', 'Test_Job', [], false); $status = new Status($token); $this->assertFalse($status->isTracking()); } @@ -136,7 +136,7 @@ public function testStatusTrackingCanBeStopped() public function testRecreatedJobWithTrackingStillTracksStatus() { - $originalToken = Resque::enqueue('jobs', 'Test_Job', null, true); + $originalToken = Resque::enqueue('jobs', 'Test_Job', [], true); $job = $this->worker->reserve(); // Mark this job as being worked on to ensure that the new status is still diff --git a/test/bootstrap.php b/test/bootstrap.php index dad6d00..76f7b7a 100644 --- a/test/bootstrap.php +++ b/test/bootstrap.php @@ -137,7 +137,7 @@ public function perform() class Test_Job_With_SetUp extends \Resque\Job\Job { public static $called = false; - public $args = false; + public $args = []; public function setUp(): void { @@ -154,7 +154,7 @@ public function perform() class Test_Job_With_TearDown extends \Resque\Job\Job { public static $called = false; - public $args = false; + public $args = []; public function perform() {