From 5c8c96636e81f44ae896737b15f5a44acfb91c50 Mon Sep 17 00:00:00 2001 From: Kevin Ullyott Date: Wed, 30 Sep 2020 18:28:58 -0700 Subject: [PATCH 1/9] Allow for the creation of a chain in a batch Signed-off-by: Kevin Ullyott --- src/Illuminate/Bus/Batch.php | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/src/Illuminate/Bus/Batch.php b/src/Illuminate/Bus/Batch.php index 8eb916094c79..7ec40a1d4b0f 100644 --- a/src/Illuminate/Bus/Batch.php +++ b/src/Illuminate/Bus/Batch.php @@ -161,18 +161,32 @@ public function fresh() */ public function add($jobs) { - $jobs = Collection::wrap($jobs)->map(function ($job) { + $jobTotal = 0; + $jobs = Collection::wrap($jobs)->map(function ($job) use (&$jobTotal) { if ($job instanceof Closure) { $job = CallQueuedClosure::create($job); } - $job->withBatchId($this->id); + if (is_array($job)) { + $jobChain = $job; + foreach ($jobChain as $j) { + $j->withBatchId($this->id); + } + + $jobTotal += count($jobChain); + + $chainHead = array_shift($jobChain); + return $chainHead->chain($jobChain); + } else { + $job->withBatchId($this->id); + $jobTotal++; + } return $job; }); - $this->repository->transaction(function () use ($jobs) { - $this->repository->incrementTotalJobs($this->id, count($jobs)); + $this->repository->transaction(function () use ($jobs, $jobTotal) { + $this->repository->incrementTotalJobs($this->id, $jobTotal); $this->queue->connection($this->options['connection'] ?? null)->bulk( $jobs->all(), From 77676a28826edb3a748ee8981ea9cf8499d4fd75 Mon Sep 17 00:00:00 2001 From: Kevin Ullyott Date: Wed, 30 Sep 2020 19:19:41 -0700 Subject: [PATCH 2/9] Create a test that a chain can be added Signed-off-by: Kevin Ullyott --- tests/Bus/BusBatchTest.php | 46 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/tests/Bus/BusBatchTest.php b/tests/Bus/BusBatchTest.php index 3a7c7e8056de..d24376e16cc9 100644 --- a/tests/Bus/BusBatchTest.php +++ b/tests/Bus/BusBatchTest.php @@ -8,11 +8,16 @@ use Illuminate\Bus\BatchFactory; use Illuminate\Bus\DatabaseBatchRepository; use Illuminate\Bus\PendingBatch; +use Illuminate\Bus\Queueable; use Illuminate\Container\Container; use Illuminate\Contracts\Queue\Factory; +use Illuminate\Contracts\Queue\ShouldQueue; use Illuminate\Database\Capsule\Manager as DB; use Illuminate\Database\Eloquent\Model; +use Illuminate\Foundation\Bus\Dispatchable; use Illuminate\Queue\CallQueuedClosure; +use Illuminate\Queue\InteractsWithQueue; +use Illuminate\Queue\SerializesModels; use Mockery as m; use PHPUnit\Framework\TestCase; use RuntimeException; @@ -300,6 +305,47 @@ public function test_batch_state_can_be_inspected() $this->assertTrue(is_string(json_encode($batch))); } + public function test_chain_can_be_added_to_batch() + { + $queue = m::mock(Factory::class); + + $batch = $this->createTestBatch($queue); + + $chainHeadJob = new class implements ShouldQueue { + use Dispatchable, InteractsWithQueue, Queueable, SerializesModels, Batchable; + }; + + $secondJob = new class implements ShouldQueue { + use Dispatchable, InteractsWithQueue, Queueable, SerializesModels, Batchable; + }; + + $thirdJob = new class implements ShouldQueue { + use Dispatchable, InteractsWithQueue, Queueable, SerializesModels, Batchable; + }; + + $queue->shouldReceive('connection')->once() + ->with('test-connection') + ->andReturn($connection = m::mock(stdClass::class)); + + $connection->shouldReceive('bulk')->once()->with(\Mockery::on(function ($args) use ($chainHeadJob, $secondJob, $thirdJob) { + return + $args[0] == $chainHeadJob + && serialize($secondJob) == $args[0]->chained[0] + && serialize($thirdJob) == $args[0]->chained[1]; + }), '', 'test-queue'); + + $batch = $batch->add([ + [$chainHeadJob, $secondJob, $thirdJob] + ]); + + $this->assertEquals(3, $batch->totalJobs); + $this->assertEquals(3, $batch->pendingJobs); + $this->assertTrue(is_string($chainHeadJob->batchId)); + $this->assertTrue(is_string($secondJob->batchId)); + $this->assertTrue(is_string($thirdJob->batchId)); + $this->assertInstanceOf(CarbonImmutable::class, $batch->createdAt); + } + protected function createTestBatch($queue, $allowFailures = false) { $repository = new DatabaseBatchRepository(new BatchFactory($queue), DB::connection(), 'job_batches'); From 109ead3f95dfd6c0ebf25e5f3502980de37e331b Mon Sep 17 00:00:00 2001 From: Kevin Ullyott Date: Thu, 1 Oct 2020 09:21:39 -0700 Subject: [PATCH 3/9] Allow closures to be included in the chain Signed-off-by: Kevin Ullyott --- src/Illuminate/Bus/Batch.php | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/Illuminate/Bus/Batch.php b/src/Illuminate/Bus/Batch.php index 7ec40a1d4b0f..bda12a3f0800 100644 --- a/src/Illuminate/Bus/Batch.php +++ b/src/Illuminate/Bus/Batch.php @@ -169,9 +169,13 @@ public function add($jobs) if (is_array($job)) { $jobChain = $job; - foreach ($jobChain as $j) { - $j->withBatchId($this->id); - } + $batchId = $this->id; + array_walk($jobChain, function (&$job) use ($batchId) { + if ($job instanceof Closure) { + $job = CallQueuedClosure::create($job); + } + $job->withBatchId($batchId); + }); $jobTotal += count($jobChain); From f82202c358bc53297f0e37beb9ade3147ea72311 Mon Sep 17 00:00:00 2001 From: Kevin Ullyott Date: Thu, 1 Oct 2020 14:19:17 -0700 Subject: [PATCH 4/9] Code Style fixes Signed-off-by: Kevin Ullyott --- src/Illuminate/Bus/Batch.php | 1 + tests/Bus/BusBatchTest.php | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Illuminate/Bus/Batch.php b/src/Illuminate/Bus/Batch.php index bda12a3f0800..06b832dd6a48 100644 --- a/src/Illuminate/Bus/Batch.php +++ b/src/Illuminate/Bus/Batch.php @@ -180,6 +180,7 @@ public function add($jobs) $jobTotal += count($jobChain); $chainHead = array_shift($jobChain); + return $chainHead->chain($jobChain); } else { $job->withBatchId($this->id); diff --git a/tests/Bus/BusBatchTest.php b/tests/Bus/BusBatchTest.php index d24376e16cc9..9a5d2c47f9ba 100644 --- a/tests/Bus/BusBatchTest.php +++ b/tests/Bus/BusBatchTest.php @@ -335,7 +335,7 @@ public function test_chain_can_be_added_to_batch() }), '', 'test-queue'); $batch = $batch->add([ - [$chainHeadJob, $secondJob, $thirdJob] + [$chainHeadJob, $secondJob, $thirdJob], ]); $this->assertEquals(3, $batch->totalJobs); From 71745f4f9d3e14f240a32ebffcddf379f807926e Mon Sep 17 00:00:00 2001 From: Kevin Ullyott Date: Thu, 1 Oct 2020 15:03:09 -0700 Subject: [PATCH 5/9] Use test job classes to fix seralization issue Signed-off-by: Kevin Ullyott --- tests/Bus/BusBatchTest.php | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/tests/Bus/BusBatchTest.php b/tests/Bus/BusBatchTest.php index 9a5d2c47f9ba..c58d87473ca2 100644 --- a/tests/Bus/BusBatchTest.php +++ b/tests/Bus/BusBatchTest.php @@ -311,17 +311,11 @@ public function test_chain_can_be_added_to_batch() $batch = $this->createTestBatch($queue); - $chainHeadJob = new class implements ShouldQueue { - use Dispatchable, InteractsWithQueue, Queueable, SerializesModels, Batchable; - }; + $chainHeadJob = new ChainHeadJob(); - $secondJob = new class implements ShouldQueue { - use Dispatchable, InteractsWithQueue, Queueable, SerializesModels, Batchable; - }; + $secondJob = new SecondTestJob(); - $thirdJob = new class implements ShouldQueue { - use Dispatchable, InteractsWithQueue, Queueable, SerializesModels, Batchable; - }; + $thirdJob = new ThirdTestJob(); $queue->shouldReceive('connection')->once() ->with('test-connection') @@ -391,3 +385,18 @@ protected function schema() return $this->connection()->getSchemaBuilder(); } } + +class ChainHeadJob implements ShouldQueue +{ + use Dispatchable, Queueable, Batchable; +} + +class SecondTestJob implements ShouldQueue +{ + use Dispatchable, Queueable, Batchable; +} + +class ThirdTestJob implements ShouldQueue +{ + use Dispatchable, Queueable, Batchable; +} From eb4f30763780089289f54aad916b79bfb53d7a50 Mon Sep 17 00:00:00 2001 From: Kevin Ullyott Date: Thu, 1 Oct 2020 15:09:14 -0700 Subject: [PATCH 6/9] Remove unused use statements Signed-off-by: Kevin Ullyott --- tests/Bus/BusBatchTest.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/Bus/BusBatchTest.php b/tests/Bus/BusBatchTest.php index c58d87473ca2..381200daa171 100644 --- a/tests/Bus/BusBatchTest.php +++ b/tests/Bus/BusBatchTest.php @@ -16,8 +16,6 @@ use Illuminate\Database\Eloquent\Model; use Illuminate\Foundation\Bus\Dispatchable; use Illuminate\Queue\CallQueuedClosure; -use Illuminate\Queue\InteractsWithQueue; -use Illuminate\Queue\SerializesModels; use Mockery as m; use PHPUnit\Framework\TestCase; use RuntimeException; From 213879406838bd94b1c7b7129efdd08da624fc21 Mon Sep 17 00:00:00 2001 From: Kevin Ullyott Date: Tue, 6 Oct 2020 09:37:19 -0700 Subject: [PATCH 7/9] Allow for a PendingChain to be added to batch Signed-off-by: Kevin Ullyott --- src/Illuminate/Bus/Batch.php | 17 +++++++++------- .../Foundation/Bus/PendingChain.php | 20 +++++++++++++++---- 2 files changed, 26 insertions(+), 11 deletions(-) diff --git a/src/Illuminate/Bus/Batch.php b/src/Illuminate/Bus/Batch.php index 06b832dd6a48..401ef5e74205 100644 --- a/src/Illuminate/Bus/Batch.php +++ b/src/Illuminate/Bus/Batch.php @@ -6,6 +6,7 @@ use Closure; use Illuminate\Contracts\Queue\Factory as QueueFactory; use Illuminate\Contracts\Support\Arrayable; +use Illuminate\Foundation\Bus\PendingChain; use Illuminate\Queue\CallQueuedClosure; use Illuminate\Queue\SerializableClosure; use Illuminate\Support\Arr; @@ -167,21 +168,23 @@ public function add($jobs) $job = CallQueuedClosure::create($job); } - if (is_array($job)) { - $jobChain = $job; + if ($job instanceof PendingChain) { $batchId = $this->id; - array_walk($jobChain, function (&$job) use ($batchId) { + + $job->job->withBatchId($batchId); + + $jobTotal++; + + array_walk($job->chain, function (&$job) use ($batchId) { if ($job instanceof Closure) { $job = CallQueuedClosure::create($job); } $job->withBatchId($batchId); }); - $jobTotal += count($jobChain); - - $chainHead = array_shift($jobChain); + $jobTotal += count($job->chain); - return $chainHead->chain($jobChain); + return $job->prepare(); } else { $job->withBatchId($this->id); $jobTotal++; diff --git a/src/Illuminate/Foundation/Bus/PendingChain.php b/src/Illuminate/Foundation/Bus/PendingChain.php index 8965e9923fa1..1dde29390cc7 100644 --- a/src/Illuminate/Foundation/Bus/PendingChain.php +++ b/src/Illuminate/Foundation/Bus/PendingChain.php @@ -109,11 +109,11 @@ public function catchCallbacks() } /** - * Dispatch the job with the given arguments. + * Prepare a job chain for dispatch with the given arguments * - * @return \Illuminate\Foundation\Bus\PendingDispatch + * @return CallQueuedClosure|mixed */ - public function dispatch() + public function prepare() { if (is_string($this->job)) { $firstJob = new $this->job(...func_get_args()); @@ -128,6 +128,18 @@ public function dispatch() $firstJob->chain($this->chain); $firstJob->chainCatchCallbacks = $this->catchCallbacks(); - return app(Dispatcher::class)->dispatch($firstJob); + return $firstJob; + } + + /** + * Dispatch the job with the given arguments. + * + * @return \Illuminate\Foundation\Bus\PendingDispatch + */ + public function dispatch() + { + $preparedJob = $this->prepare(); + + return app(Dispatcher::class)->dispatch($preparedJob); } } From ffde55e2bc38df031c57cf2a1c7562bca64648bd Mon Sep 17 00:00:00 2001 From: Kevin Ullyott Date: Tue, 6 Oct 2020 10:34:26 -0700 Subject: [PATCH 8/9] Modify the testing Signed-off-by: Kevin Ullyott --- tests/Bus/BusBatchTest.php | 35 +++++++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/tests/Bus/BusBatchTest.php b/tests/Bus/BusBatchTest.php index 381200daa171..4eea834231db 100644 --- a/tests/Bus/BusBatchTest.php +++ b/tests/Bus/BusBatchTest.php @@ -15,7 +15,9 @@ use Illuminate\Database\Capsule\Manager as DB; use Illuminate\Database\Eloquent\Model; use Illuminate\Foundation\Bus\Dispatchable; +use Illuminate\Foundation\Bus\PendingChain; use Illuminate\Queue\CallQueuedClosure; +use Illuminate\Support\Facades\Bus; use Mockery as m; use PHPUnit\Framework\TestCase; use RuntimeException; @@ -309,30 +311,43 @@ public function test_chain_can_be_added_to_batch() $batch = $this->createTestBatch($queue); - $chainHeadJob = new ChainHeadJob(); + $firstJob = new FirstTestJob(); $secondJob = new SecondTestJob(); $thirdJob = new ThirdTestJob(); + $fourthJob = function (){ + }; + $queue->shouldReceive('connection')->once() ->with('test-connection') ->andReturn($connection = m::mock(stdClass::class)); - $connection->shouldReceive('bulk')->once()->with(\Mockery::on(function ($args) use ($chainHeadJob, $secondJob, $thirdJob) { + $connection->shouldReceive('bulk')->once()->with(\Mockery::on(function ($args) use ($firstJob, $secondJob, $thirdJob, $fourthJob) { return - $args[0] == $chainHeadJob - && serialize($secondJob) == $args[0]->chained[0] - && serialize($thirdJob) == $args[0]->chained[1]; + $args[0] == $firstJob + && $args[1] == $secondJob + && unserialize($args[1]->chained[0]) == $thirdJob + && unserialize($args[1]->chained[1]) instanceof CallQueuedClosure + && is_string(unserialize($args[1]->chained[1])->batchId); }), '', 'test-queue'); + // Without "RuntimeException: A facade root has not been set" is thrown + Bus::partialMock()->shouldReceive('chain')->andReturn(new PendingChain($secondJob, [$thirdJob, $fourthJob])); + $batch = $batch->add([ - [$chainHeadJob, $secondJob, $thirdJob], + $firstJob, + Bus::chain([ + $secondJob, + $thirdJob, + $fourthJob, + ]), ]); - $this->assertEquals(3, $batch->totalJobs); - $this->assertEquals(3, $batch->pendingJobs); - $this->assertTrue(is_string($chainHeadJob->batchId)); + $this->assertEquals(4, $batch->totalJobs); + $this->assertEquals(4, $batch->pendingJobs); + $this->assertTrue(is_string($firstJob->batchId)); $this->assertTrue(is_string($secondJob->batchId)); $this->assertTrue(is_string($thirdJob->batchId)); $this->assertInstanceOf(CarbonImmutable::class, $batch->createdAt); @@ -384,7 +399,7 @@ protected function schema() } } -class ChainHeadJob implements ShouldQueue +class FirstTestJob implements ShouldQueue { use Dispatchable, Queueable, Batchable; } From c6a4e7d409a894e3e94e522792827c2137e93027 Mon Sep 17 00:00:00 2001 From: Kevin Ullyott Date: Tue, 6 Oct 2020 11:00:42 -0700 Subject: [PATCH 9/9] Apply styleci changes Signed-off-by: Kevin Ullyott --- src/Illuminate/Foundation/Bus/PendingChain.php | 2 +- tests/Bus/BusBatchTest.php | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Illuminate/Foundation/Bus/PendingChain.php b/src/Illuminate/Foundation/Bus/PendingChain.php index 1dde29390cc7..568eb70f0598 100644 --- a/src/Illuminate/Foundation/Bus/PendingChain.php +++ b/src/Illuminate/Foundation/Bus/PendingChain.php @@ -109,7 +109,7 @@ public function catchCallbacks() } /** - * Prepare a job chain for dispatch with the given arguments + * Prepare a job chain for dispatch with the given arguments. * * @return CallQueuedClosure|mixed */ diff --git a/tests/Bus/BusBatchTest.php b/tests/Bus/BusBatchTest.php index 4eea834231db..f04b83e827d4 100644 --- a/tests/Bus/BusBatchTest.php +++ b/tests/Bus/BusBatchTest.php @@ -317,14 +317,14 @@ public function test_chain_can_be_added_to_batch() $thirdJob = new ThirdTestJob(); - $fourthJob = function (){ + $fourthJob = function () { }; $queue->shouldReceive('connection')->once() ->with('test-connection') ->andReturn($connection = m::mock(stdClass::class)); - $connection->shouldReceive('bulk')->once()->with(\Mockery::on(function ($args) use ($firstJob, $secondJob, $thirdJob, $fourthJob) { + $connection->shouldReceive('bulk')->once()->with(\Mockery::on(function ($args) use ($firstJob, $secondJob, $thirdJob) { return $args[0] == $firstJob && $args[1] == $secondJob