From 08ed5084d5f84756cf36fba05f3dfb985698a93f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samuel=20=C5=A0tancl?= Date: Fri, 8 May 2020 16:57:14 +0200 Subject: [PATCH] JobPipeline now works fully --- assets/TenancyServiceProvider.stub.php | 2 +- src/Events/Listeners/JobPipeline.php | 61 +++++++++++++------------- tests/v3/JobPipelineTest.php | 13 +++--- 3 files changed, 37 insertions(+), 39 deletions(-) diff --git a/assets/TenancyServiceProvider.stub.php b/assets/TenancyServiceProvider.stub.php index 693ffc8a..a53e5525 100644 --- a/assets/TenancyServiceProvider.stub.php +++ b/assets/TenancyServiceProvider.stub.php @@ -63,7 +63,7 @@ class TenancyServiceProvider extends ServiceProvider foreach ($this->events() as $event => $listeners) { foreach (array_unique($listeners) as $listener) { if ($listener instanceof JobPipeline) { - $listener = $listener->toClosure(); + $listener = $listener->toListener(); } Event::listen($event, $listener); diff --git a/src/Events/Listeners/JobPipeline.php b/src/Events/Listeners/JobPipeline.php index 2d993a41..07b519e8 100644 --- a/src/Events/Listeners/JobPipeline.php +++ b/src/Events/Listeners/JobPipeline.php @@ -4,7 +4,6 @@ namespace Stancl\Tenancy\Events\Listeners; use Closure; use Illuminate\Contracts\Queue\ShouldQueue; -use Illuminate\Pipeline\Pipeline; class JobPipeline implements ShouldQueue { @@ -12,13 +11,18 @@ class JobPipeline implements ShouldQueue public static $shouldQueueByDefault = true; /** @var callable[]|string[] */ - public $jobs = []; - - /** @var callable */ + public $jobs; + + /** @var callable|null */ public $send; - + + /** + * A value passed to the jobs. This is the return value of $send. + */ + public $passable; + /** @var bool */ - public $shouldQueue = true; + public $shouldQueue; public function __construct($jobs, callable $send = null, bool $shouldQueue = null) { @@ -36,13 +40,6 @@ class JobPipeline implements ShouldQueue return new static($jobs); } - public function queue(bool $shouldQueue): self - { - $this->shouldQueue = $shouldQueue; - - return $this; - } - public function send(callable $send): self { $this->send = $send; @@ -50,7 +47,6 @@ class JobPipeline implements ShouldQueue return $this; } - /** @return bool|$this */ public function shouldQueue(bool $shouldQueue = null) { if ($shouldQueue !== null) { @@ -62,30 +58,33 @@ class JobPipeline implements ShouldQueue return $this->shouldQueue; } - public function handle($event): void + public function handle(): void { - /** @var Pipeline $pipeline */ - $pipeline = app(Pipeline::class); - - $pipeline - ->send(($this->send)($event)) - ->through($this->jobs) - ->thenReturn(); + foreach ($this->jobs as $job) { + app($job)->handle($this->passable); + } } /** - * Generate a closure that runs this listener. - * - * Technically, the string|Closure typehint is not enforced by - * Laravel, but for correct typing we wrap this callable in - * simple Closures, to match Laravel's docblock typehint. - * - * @return Closure + * Generate a closure that can be used as a listener. */ - public function toClosure(): Closure + public function toListener(): Closure { return function (...$args) { - $this->handle(...$args); + dispatch($this->queueFriendly($args)); }; } + + /** + * Return a serializable version of the current object. + */ + public function queueFriendly($listenerArgs): self + { + $clone = clone $this; + + $clone->passable = ($clone->send)(...$listenerArgs); + unset($clone->send); + + return $clone; + } } diff --git a/tests/v3/JobPipelineTest.php b/tests/v3/JobPipelineTest.php index e12ed9ee..4eea7f37 100644 --- a/tests/v3/JobPipelineTest.php +++ b/tests/v3/JobPipelineTest.php @@ -16,7 +16,7 @@ class JobPipelineTest extends TestCase { Event::listen(TenantCreated::class, JobPipeline::make([ FooJob::class, - ])->toClosure()); + ])->toListener()); $this->assertFalse(app()->bound('foo')); @@ -28,20 +28,20 @@ class JobPipelineTest extends TestCase /** @test */ public function job_pipeline_can_be_queued() { - // todo: This does not work because of toClosure - Queue::fake(); Event::listen(TenantCreated::class, JobPipeline::make([ FooJob::class, - ])->queue(true)->toClosure()); + ])->shouldQueue(true)->toListener()); Queue::assertNothingPushed(); Tenant::create(); $this->assertFalse(app()->bound('foo')); - Queue::assertPushed(JobPipeline::class); + Queue::pushed(JobPipeline::class, function (JobPipeline $pipeline) { + $this->assertSame([FooJob::class], $pipeline->jobs); + }); } /** @test */ @@ -52,11 +52,10 @@ class JobPipelineTest extends TestCase SecondJob::class, ])->send(function (TenantCreated $event) { return $event->tenant; - })->toClosure()); + })->toListener()); $this->assertFalse(app()->bound('foo')); - // todo: for some reason, SecondJob is not reached in the pipeline Tenant::create(); $this->assertSame('first job changed property', app('foo'));