From ea22de3c8ed07f375f1bad1fff7e35c9c03a4df0 Mon Sep 17 00:00:00 2001 From: Steven Maguire Date: Fri, 10 Jan 2025 11:04:46 -0600 Subject: [PATCH 01/11] Remove trailing whitespace on empty lines --- src/ActionManager.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ActionManager.php b/src/ActionManager.php index 94647b6..10f5ed2 100755 --- a/src/ActionManager.php +++ b/src/ActionManager.php @@ -73,7 +73,7 @@ public function getDesignPatterns(): array public function registerDesignPattern(DesignPattern $designPattern): ActionManager { $this->designPatterns[] = $designPattern; - + return $this; } @@ -136,7 +136,7 @@ public function identifyFromBacktrace($usedTraits, ?BacktraceFrame &$frame = nul $designPatterns = $this->getDesignPatternsMatching($usedTraits); $backtraceOptions = DEBUG_BACKTRACE_PROVIDE_OBJECT | DEBUG_BACKTRACE_IGNORE_ARGS; - + $ownNumberOfFrames = 2; $frames = array_slice( debug_backtrace($backtraceOptions, $ownNumberOfFrames + $this->backtraceLimit), From 15b77ded9232c84f91939ecc9bb4b838ea91fa83 Mon Sep 17 00:00:00 2001 From: Steven Maguire Date: Fri, 10 Jan 2025 11:22:27 -0600 Subject: [PATCH 02/11] Add support for AsPipeline concern --- src/ActionServiceProvider.php | 2 + src/Concerns/AsAction.php | 1 + src/Concerns/AsPipeline.php | 42 +++++++++++ src/Decorators/PipelineDecorator.php | 24 ++++++ src/DesignPatterns/PipelineDesignPattern.php | 27 +++++++ tests/AsPipelineTest.php | 79 ++++++++++++++++++++ 6 files changed, 175 insertions(+) create mode 100644 src/Concerns/AsPipeline.php create mode 100644 src/Decorators/PipelineDecorator.php create mode 100644 src/DesignPatterns/PipelineDesignPattern.php create mode 100644 tests/AsPipelineTest.php diff --git a/src/ActionServiceProvider.php b/src/ActionServiceProvider.php index 4fe9b87..82c280c 100644 --- a/src/ActionServiceProvider.php +++ b/src/ActionServiceProvider.php @@ -8,6 +8,7 @@ use Lorisleiva\Actions\DesignPatterns\CommandDesignPattern; use Lorisleiva\Actions\DesignPatterns\ControllerDesignPattern; use Lorisleiva\Actions\DesignPatterns\ListenerDesignPattern; +use Lorisleiva\Actions\DesignPatterns\PipelineDesignPattern; class ActionServiceProvider extends ServiceProvider { @@ -21,6 +22,7 @@ public function register(): void new ControllerDesignPattern(), new ListenerDesignPattern(), new CommandDesignPattern(), + new PipelineDesignPattern(), ]); }); diff --git a/src/Concerns/AsAction.php b/src/Concerns/AsAction.php index c34ff59..6c813e3 100644 --- a/src/Concerns/AsAction.php +++ b/src/Concerns/AsAction.php @@ -10,4 +10,5 @@ trait AsAction use AsJob; use AsCommand; use AsFake; + use AsPipeline; } diff --git a/src/Concerns/AsPipeline.php b/src/Concerns/AsPipeline.php new file mode 100644 index 0000000..7d9c345 --- /dev/null +++ b/src/Concerns/AsPipeline.php @@ -0,0 +1,42 @@ +handle($passable); + + if (! is_null($returned)) { + return $closure($returned); + } else { + return $closure($passable); + } + } +} diff --git a/src/Decorators/PipelineDecorator.php b/src/Decorators/PipelineDecorator.php new file mode 100644 index 0000000..f118414 --- /dev/null +++ b/src/Decorators/PipelineDecorator.php @@ -0,0 +1,24 @@ +setAction($action); + } + + public function __invoke(mixed ...$arguments): mixed + { + if ($this->hasMethod('asPipeline')) { + return $this->resolveAndCallMethod('asPipeline', $arguments); + } + } +} diff --git a/src/DesignPatterns/PipelineDesignPattern.php b/src/DesignPatterns/PipelineDesignPattern.php new file mode 100644 index 0000000..b11e0d3 --- /dev/null +++ b/src/DesignPatterns/PipelineDesignPattern.php @@ -0,0 +1,27 @@ +matches(Pipeline::class, 'Illuminate\Pipeline\{closure}'); + } + + public function decorate($instance, BacktraceFrame $frame) + { + return app(PipelineDecorator::class, ['action' => $instance]); + } +} diff --git a/tests/AsPipelineTest.php b/tests/AsPipelineTest.php new file mode 100644 index 0000000..4438690 --- /dev/null +++ b/tests/AsPipelineTest.php @@ -0,0 +1,79 @@ +increment(); + } +} + +class AsPipelineImplicitTest +{ + use AsAction; + + public function handle($passable): void + { + $passable->increment(); + } +} + +function getAnonymous() { + return function ($p, $next) { + $p->increment(); + + return $next($p); + }; +} + +function getPassable() { + return new class { + public function __construct(public int $count = 0) + { + // + } + + public function increment() + { + $this->count++; + } + }; +} + +it('can run as a pipe in a pipeline, with explicit trait', function () { + $anonymous = getAnonymous(); + $passable = Pipeline::send(getPassable()) + ->through([ + AsPipelineExplicitTest::class, + $anonymous, + AsPipelineExplicitTest::class, + $anonymous, + ]) + ->thenReturn(); + + expect(is_object($passable))->toBe(true); + expect($passable->count)->toBe(4); +}); + +it('can run as a pipe in a pipeline, with implicit trait', function () { + $anonymous = getAnonymous(); + $passable = Pipeline::send(getPassable()) + ->through([ + AsPipelineImplicitTest::class, + $anonymous, + AsPipelineImplicitTest::class, + $anonymous, + ]) + ->thenReturn(); + + expect(is_object($passable))->toBe(true); + expect($passable->count)->toBe(4); +}); From 5da4c161da220a5854e12eb60f387a72801f3628 Mon Sep 17 00:00:00 2001 From: Steven Maguire Date: Fri, 10 Jan 2025 12:11:02 -0600 Subject: [PATCH 03/11] Remove superflous else condition --- src/Concerns/AsPipeline.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Concerns/AsPipeline.php b/src/Concerns/AsPipeline.php index 7d9c345..c642a73 100644 --- a/src/Concerns/AsPipeline.php +++ b/src/Concerns/AsPipeline.php @@ -35,8 +35,8 @@ public function asPipeline(mixed ...$arguments): mixed if (! is_null($returned)) { return $closure($returned); - } else { - return $closure($passable); } + + return $closure($passable); } } From 044569e84b775f45a74792e6d121a9c331e791eb Mon Sep 17 00:00:00 2001 From: Steven Maguire Date: Fri, 10 Jan 2025 12:12:17 -0600 Subject: [PATCH 04/11] Add backup handling logic in PipelineDecorator if asPipeline method is not available --- src/Decorators/PipelineDecorator.php | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/Decorators/PipelineDecorator.php b/src/Decorators/PipelineDecorator.php index f118414..102c442 100644 --- a/src/Decorators/PipelineDecorator.php +++ b/src/Decorators/PipelineDecorator.php @@ -16,9 +16,23 @@ public function __construct($action) } public function __invoke(mixed ...$arguments): mixed + { + return $this->handleFromAnyMethod(...$arguments); + } + + public function handle(mixed ...$arguments): mixed + { + return $this->handleFromAnyMethod(...$arguments); + } + + protected function handleFromAnyMethod(mixed ...$arguments): mixed { if ($this->hasMethod('asPipeline')) { return $this->resolveAndCallMethod('asPipeline', $arguments); } + + if ($this->hasMethod('handle')) { + return $this->resolveFromArgumentsAndCall('handle', $arguments); + } } } From 8db415007854fda81f9caacfdd6f4b5db7c2c89c Mon Sep 17 00:00:00 2001 From: Steven Maguire Date: Fri, 10 Jan 2025 12:30:32 -0600 Subject: [PATCH 05/11] Update code style in the name of elegance --- src/Concerns/AsPipeline.php | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/Concerns/AsPipeline.php b/src/Concerns/AsPipeline.php index c642a73..eaedad1 100644 --- a/src/Concerns/AsPipeline.php +++ b/src/Concerns/AsPipeline.php @@ -31,12 +31,6 @@ public function asPipeline(mixed ...$arguments): mixed $passable = array_shift($arguments); $closure = array_pop($arguments); - $returned = $this->handle($passable); - - if (! is_null($returned)) { - return $closure($returned); - } - - return $closure($passable); + return $closure($this->handle($passable) ?? $passable); } } From 6aa97e6ab87247c5658951a654faa238c1b276ae Mon Sep 17 00:00:00 2001 From: Steven Maguire Date: Fri, 10 Jan 2025 12:36:26 -0600 Subject: [PATCH 06/11] Remove possible dead code path --- src/Decorators/PipelineDecorator.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/Decorators/PipelineDecorator.php b/src/Decorators/PipelineDecorator.php index 102c442..b7daa3a 100644 --- a/src/Decorators/PipelineDecorator.php +++ b/src/Decorators/PipelineDecorator.php @@ -30,9 +30,5 @@ protected function handleFromAnyMethod(mixed ...$arguments): mixed if ($this->hasMethod('asPipeline')) { return $this->resolveAndCallMethod('asPipeline', $arguments); } - - if ($this->hasMethod('handle')) { - return $this->resolveFromArgumentsAndCall('handle', $arguments); - } } } From af9f9c58ee51d3ee5dec26d7c9b619bc02a42048 Mon Sep 17 00:00:00 2001 From: Steven Maguire Date: Sat, 11 Jan 2025 20:42:00 -0600 Subject: [PATCH 07/11] Move pipeline closure resolution logic to decorator and expand test cases --- src/Concerns/AsPipeline.php | 30 +---- src/Decorators/PipelineDecorator.php | 28 ++++- tests/AsPipelineTest.php | 165 ++++++++++++++++++++++++--- 3 files changed, 179 insertions(+), 44 deletions(-) diff --git a/src/Concerns/AsPipeline.php b/src/Concerns/AsPipeline.php index eaedad1..2fbb9ce 100644 --- a/src/Concerns/AsPipeline.php +++ b/src/Concerns/AsPipeline.php @@ -4,33 +4,5 @@ trait AsPipeline { - /** - * Typical pipeline behavior expects two things: - * - * 1) The pipe class to expect a single incoming parameter (along with - * a closure) and single return value. - * 2) The pipe class to be aware of the next closure and determine what - * should be passed into the next pipe. - * - * Because of these expectations, this behavior is asserting two opinions: - * - * 1) Regardless of the number of parameters provided to the asPipeline - * method implemented here, only the first will be supplied to the - * invoked Action. - * 2) If the invoked Action does not return anything, then the next - * closure will be supplied the same parameter. However, if the - * invoked action does return a non-null value, that value will - * be supplied to the next closure. - * - * Also, this logic is implemented in the trait rather than the decorator - * to afford some flexibility to consuming projects, should the wish to - * implement their own logic in their Action classes directly. - */ - public function asPipeline(mixed ...$arguments): mixed - { - $passable = array_shift($arguments); - $closure = array_pop($arguments); - - return $closure($this->handle($passable) ?? $passable); - } + // } diff --git a/src/Decorators/PipelineDecorator.php b/src/Decorators/PipelineDecorator.php index b7daa3a..283f100 100644 --- a/src/Decorators/PipelineDecorator.php +++ b/src/Decorators/PipelineDecorator.php @@ -25,10 +25,36 @@ public function handle(mixed ...$arguments): mixed return $this->handleFromAnyMethod(...$arguments); } + /** + * Typical pipeline behavior expects two things: + * + * 1) The pipe class to expect a single incoming parameter (along with + * a closure) and single return value. + * 2) The pipe class to be aware of the next closure and determine what + * should be passed into the next pipe. + * + * Because of these expectations, this behavior is asserting two opinions: + * + * 1) Regardless of the number of parameters provided to the asPipeline + * method implemented here, only the first will be supplied to the + * invoked Action. + * 2) If the invoked Action does not return anything, then the next + * closure will be supplied the same parameter. However, if the + * invoked action does return a non-null value, that value will + * be supplied to the next closure. + */ protected function handleFromAnyMethod(mixed ...$arguments): mixed { + $passable = array_shift($arguments); + $closure = array_pop($arguments); + $returned = null; + if ($this->hasMethod('asPipeline')) { - return $this->resolveAndCallMethod('asPipeline', $arguments); + $returned = $this->callMethod('asPipeline', [$passable]); + } elseif ($this->hasMethod('handle')) { + $returned = $this->callMethod('handle', [$passable]); } + + return $closure($returned ?? $passable); } } diff --git a/tests/AsPipelineTest.php b/tests/AsPipelineTest.php index 4438690..7da64db 100644 --- a/tests/AsPipelineTest.php +++ b/tests/AsPipelineTest.php @@ -2,32 +2,103 @@ namespace Lorisleiva\Actions\Tests; +use ArgumentCountError; +use Closure; use Illuminate\Support\Facades\Pipeline; use Lorisleiva\Actions\Concerns\AsAction; use Lorisleiva\Actions\Concerns\AsPipeline; +class AsPipelinePassable +{ + public function __construct(public int $count = 0) + { + // + } + + public function increment(int $multiplier = 1) + { + $this->count = $this->count + (1 * $multiplier); + } +} + class AsPipelineExplicitTest { use AsPipeline; - public function handle($passable): void + public function handle(AsPipelinePassable $passable): void { $passable->increment(); } + + public function asPipeline(AsPipelinePassable $passable): AsPipelinePassable + { + $this->handle($passable); + + return $passable; + } } class AsPipelineImplicitTest { use AsAction; - public function handle($passable): void + public function handle(AsPipelinePassable $passable): void + { + $passable->increment(); + } + + public function asPipeline(AsPipelinePassable $passable): AsPipelinePassable + { + $this->handle($passable); + + return $passable; + } +} + +class AsPipelineMultipleParamTest +{ + use AsAction; + + public function handle(AsPipelinePassable $passable): void + { + $passable->increment($multiplier); + } + + public function asPipeline(AsPipelinePassable $passable, int $multiplier): AsPipelinePassable + { + $this->handle($passable); + + return $passable; + } +} + +class AsPipelineSingleParamHandleOnlyTest +{ + use AsAction; + + public function handle(AsPipelinePassable $passable): void { $passable->increment(); } } +class AsPipelineMultipleParamHandleOnlyTest +{ + use AsAction; + + public function handle(AsPipelinePassable $passable, int $multiplier): void + { + $passable->increment($multiplier); + } +} + +class AsPipelineWithoutHandleOrAsPipeline +{ + use AsAction; +} + function getAnonymous() { - return function ($p, $next) { + return function (AsPipelinePassable $p, $next) { $p->increment(); return $next($p); @@ -35,17 +106,7 @@ function getAnonymous() { } function getPassable() { - return new class { - public function __construct(public int $count = 0) - { - // - } - - public function increment() - { - $this->count++; - } - }; + return new AsPipelinePassable; } it('can run as a pipe in a pipeline, with explicit trait', function () { @@ -77,3 +138,79 @@ public function increment() expect(is_object($passable))->toBe(true); expect($passable->count)->toBe(4); }); + +it('can run as a pipe in a pipeline, without an explicit asPipeline method', function () { + $anonymous = getAnonymous(); + $passable = Pipeline::send(getPassable()) + ->through([ + AsPipelineSingleParamHandleOnlyTest::class, + $anonymous, + AsPipelineSingleParamHandleOnlyTest::class, + $anonymous, + ]) + ->thenReturn(); + + expect(is_object($passable))->toBe(true); + expect($passable->count)->toBe(4); +}); + +it('it can run as a noop/passthrough pipe in a pipeline, without a handle or asPipeline method', function () { + $anonymous = getAnonymous(); + $passable = Pipeline::send(getPassable()) + ->through([ + AsPipelineWithoutHandleOrAsPipeline::class, + $anonymous, + AsPipelineWithoutHandleOrAsPipeline::class, + $anonymous, + ]) + ->thenReturn(); + + expect(is_object($passable))->toBe(true); + expect($passable->count)->toBe(2); +}); + +it('it can run with an arbitrary via method configured on Pipeline', function () { + $anonymous = getAnonymous(); + $passable = Pipeline::send(getPassable()) + ->via('foobar') + ->through([ + AsPipelineImplicitTest::class, + $anonymous, + AsPipelineImplicitTest::class, + $anonymous, + ]) + ->thenReturn(); + + expect(is_object($passable))->toBe(true); + expect($passable->count)->toBe(4); +}); + +it('cannot run as a pipe in a pipeline, with an explicit asPipeline method expecting multiple non-optional params', function () { + $anonymous = getAnonymous(); + $passable = Pipeline::send(getPassable()) + ->through([ + AsPipelineMultipleParamTest::class, + $anonymous, + AsPipelineMultipleParamTest::class, + $anonymous, + ]) + ->thenReturn(); + + expect(is_object($passable))->toBe(true); + expect($passable->count)->toBe(10); +})->throws(ArgumentCountError::class, 'Too few arguments to function Lorisleiva\Actions\Tests\AsPipelineMultipleParamTest::asPipeline(), 1 passed and exactly 2 expected'); + +it('cannot run as a pipe in a pipeline, without an explicit asPipeline method and multiple non-optional handle params', function () { + $anonymous = getAnonymous(); + $passable = Pipeline::send(getPassable()) + ->through([ + AsPipelineMultipleParamHandleOnlyTest::class, + $anonymous, + AsPipelineMultipleParamHandleOnlyTest::class, + $anonymous, + ]) + ->thenReturn(); + + expect(is_object($passable))->toBe(true); + expect($passable->count)->toBe(10); +})->throws(ArgumentCountError::class, 'Too few arguments to function Lorisleiva\Actions\Tests\AsPipelineMultipleParamHandleOnlyTest::handle(), 1 passed and exactly 2 expected'); From 96a9a196ebeef55202f47166668421aec83472bb Mon Sep 17 00:00:00 2001 From: Steven Maguire Date: Sat, 11 Jan 2025 21:03:12 -0600 Subject: [PATCH 08/11] Remove superfluous code from tests --- tests/AsPipelineTest.php | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/tests/AsPipelineTest.php b/tests/AsPipelineTest.php index 7da64db..908d4bd 100644 --- a/tests/AsPipelineTest.php +++ b/tests/AsPipelineTest.php @@ -15,9 +15,9 @@ public function __construct(public int $count = 0) // } - public function increment(int $multiplier = 1) + public function increment() { - $this->count = $this->count + (1 * $multiplier); + $this->count++; } } @@ -61,10 +61,10 @@ class AsPipelineMultipleParamTest public function handle(AsPipelinePassable $passable): void { - $passable->increment($multiplier); + $passable->increment(); } - public function asPipeline(AsPipelinePassable $passable, int $multiplier): AsPipelinePassable + public function asPipeline(AsPipelinePassable $passable, int $foo): AsPipelinePassable { $this->handle($passable); @@ -86,9 +86,9 @@ class AsPipelineMultipleParamHandleOnlyTest { use AsAction; - public function handle(AsPipelinePassable $passable, int $multiplier): void + public function handle(AsPipelinePassable $passable, int $foo): void { - $passable->increment($multiplier); + $passable->increment(); } } @@ -195,9 +195,6 @@ function getPassable() { $anonymous, ]) ->thenReturn(); - - expect(is_object($passable))->toBe(true); - expect($passable->count)->toBe(10); })->throws(ArgumentCountError::class, 'Too few arguments to function Lorisleiva\Actions\Tests\AsPipelineMultipleParamTest::asPipeline(), 1 passed and exactly 2 expected'); it('cannot run as a pipe in a pipeline, without an explicit asPipeline method and multiple non-optional handle params', function () { @@ -210,7 +207,4 @@ function getPassable() { $anonymous, ]) ->thenReturn(); - - expect(is_object($passable))->toBe(true); - expect($passable->count)->toBe(10); })->throws(ArgumentCountError::class, 'Too few arguments to function Lorisleiva\Actions\Tests\AsPipelineMultipleParamHandleOnlyTest::handle(), 1 passed and exactly 2 expected'); From 4d936c0133646f69fc78ed4395b446f17a9311a8 Mon Sep 17 00:00:00 2001 From: Steven Maguire Date: Sun, 12 Jan 2025 13:18:05 -0600 Subject: [PATCH 09/11] Remover superflous label from test conditions --- tests/AsPipelineTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/AsPipelineTest.php b/tests/AsPipelineTest.php index 908d4bd..e00cecc 100644 --- a/tests/AsPipelineTest.php +++ b/tests/AsPipelineTest.php @@ -154,7 +154,7 @@ function getPassable() { expect($passable->count)->toBe(4); }); -it('it can run as a noop/passthrough pipe in a pipeline, without a handle or asPipeline method', function () { +it('can run as a noop/passthrough pipe in a pipeline, without a handle or asPipeline method', function () { $anonymous = getAnonymous(); $passable = Pipeline::send(getPassable()) ->through([ @@ -169,7 +169,7 @@ function getPassable() { expect($passable->count)->toBe(2); }); -it('it can run with an arbitrary via method configured on Pipeline', function () { +it('can run with an arbitrary via method configured on Pipeline', function () { $anonymous = getAnonymous(); $passable = Pipeline::send(getPassable()) ->via('foobar') From b5f72e9cb4a769ca07d7c780ba24f5bc0d502021 Mon Sep 17 00:00:00 2001 From: Steven Maguire Date: Tue, 25 Feb 2025 12:21:05 -0600 Subject: [PATCH 10/11] Add tests and reorganize code based on further refined requirements and feedback --- src/ActionManager.php | 4 +- src/Decorators/PipelineDecorator.php | 37 +--- tests/AsPipelinePassable.php | 19 ++ tests/AsPipelineTest.php | 188 ++---------------- tests/AsPipelineWithExplicitTraitTest.php | 30 +++ tests/AsPipelineWithImplicitTraitTest.php | 30 +++ ...eWithMultipleNonOptionalParametersTest.php | 32 +++ 7 files changed, 130 insertions(+), 210 deletions(-) create mode 100644 tests/AsPipelinePassable.php create mode 100644 tests/AsPipelineWithExplicitTraitTest.php create mode 100644 tests/AsPipelineWithImplicitTraitTest.php create mode 100644 tests/AsPipelineWithMultipleNonOptionalParametersTest.php diff --git a/src/ActionManager.php b/src/ActionManager.php index 10f5ed2..94647b6 100755 --- a/src/ActionManager.php +++ b/src/ActionManager.php @@ -73,7 +73,7 @@ public function getDesignPatterns(): array public function registerDesignPattern(DesignPattern $designPattern): ActionManager { $this->designPatterns[] = $designPattern; - + return $this; } @@ -136,7 +136,7 @@ public function identifyFromBacktrace($usedTraits, ?BacktraceFrame &$frame = nul $designPatterns = $this->getDesignPatternsMatching($usedTraits); $backtraceOptions = DEBUG_BACKTRACE_PROVIDE_OBJECT | DEBUG_BACKTRACE_IGNORE_ARGS; - + $ownNumberOfFrames = 2; $frames = array_slice( debug_backtrace($backtraceOptions, $ownNumberOfFrames + $this->backtraceLimit), diff --git a/src/Decorators/PipelineDecorator.php b/src/Decorators/PipelineDecorator.php index 283f100..e901938 100644 --- a/src/Decorators/PipelineDecorator.php +++ b/src/Decorators/PipelineDecorator.php @@ -16,45 +16,12 @@ public function __construct($action) } public function __invoke(mixed ...$arguments): mixed - { - return $this->handleFromAnyMethod(...$arguments); - } - - public function handle(mixed ...$arguments): mixed - { - return $this->handleFromAnyMethod(...$arguments); - } - - /** - * Typical pipeline behavior expects two things: - * - * 1) The pipe class to expect a single incoming parameter (along with - * a closure) and single return value. - * 2) The pipe class to be aware of the next closure and determine what - * should be passed into the next pipe. - * - * Because of these expectations, this behavior is asserting two opinions: - * - * 1) Regardless of the number of parameters provided to the asPipeline - * method implemented here, only the first will be supplied to the - * invoked Action. - * 2) If the invoked Action does not return anything, then the next - * closure will be supplied the same parameter. However, if the - * invoked action does return a non-null value, that value will - * be supplied to the next closure. - */ - protected function handleFromAnyMethod(mixed ...$arguments): mixed { $passable = array_shift($arguments); $closure = array_pop($arguments); - $returned = null; - if ($this->hasMethod('asPipeline')) { - $returned = $this->callMethod('asPipeline', [$passable]); - } elseif ($this->hasMethod('handle')) { - $returned = $this->callMethod('handle', [$passable]); - } + $method = $this->hasMethod('asPipeline') ? 'asPipeline' : 'handle'; - return $closure($returned ?? $passable); + return $closure($this->callMethod($method, [$passable]) ?? $passable); } } diff --git a/tests/AsPipelinePassable.php b/tests/AsPipelinePassable.php new file mode 100644 index 0000000..31aafe8 --- /dev/null +++ b/tests/AsPipelinePassable.php @@ -0,0 +1,19 @@ +count++; + } +} diff --git a/tests/AsPipelineTest.php b/tests/AsPipelineTest.php index e00cecc..30b773b 100644 --- a/tests/AsPipelineTest.php +++ b/tests/AsPipelineTest.php @@ -2,43 +2,10 @@ namespace Lorisleiva\Actions\Tests; -use ArgumentCountError; -use Closure; use Illuminate\Support\Facades\Pipeline; use Lorisleiva\Actions\Concerns\AsAction; -use Lorisleiva\Actions\Concerns\AsPipeline; -class AsPipelinePassable -{ - public function __construct(public int $count = 0) - { - // - } - - public function increment() - { - $this->count++; - } -} - -class AsPipelineExplicitTest -{ - use AsPipeline; - - public function handle(AsPipelinePassable $passable): void - { - $passable->increment(); - } - - public function asPipeline(AsPipelinePassable $passable): AsPipelinePassable - { - $this->handle($passable); - - return $passable; - } -} - -class AsPipelineImplicitTest +class AsPipelineTest { use AsAction; @@ -55,156 +22,31 @@ public function asPipeline(AsPipelinePassable $passable): AsPipelinePassable } } -class AsPipelineMultipleParamTest -{ - use AsAction; - - public function handle(AsPipelinePassable $passable): void - { - $passable->increment(); - } - - public function asPipeline(AsPipelinePassable $passable, int $foo): AsPipelinePassable - { - $this->handle($passable); - - return $passable; - } -} - -class AsPipelineSingleParamHandleOnlyTest -{ - use AsAction; - - public function handle(AsPipelinePassable $passable): void - { - $passable->increment(); - } -} - -class AsPipelineMultipleParamHandleOnlyTest -{ - use AsAction; - - public function handle(AsPipelinePassable $passable, int $foo): void - { - $passable->increment(); - } -} - -class AsPipelineWithoutHandleOrAsPipeline -{ - use AsAction; -} - -function getAnonymous() { - return function (AsPipelinePassable $p, $next) { - $p->increment(); - - return $next($p); - }; -} - -function getPassable() { - return new AsPipelinePassable; -} - -it('can run as a pipe in a pipeline, with explicit trait', function () { - $anonymous = getAnonymous(); - $passable = Pipeline::send(getPassable()) - ->through([ - AsPipelineExplicitTest::class, - $anonymous, - AsPipelineExplicitTest::class, - $anonymous, - ]) - ->thenReturn(); - - expect(is_object($passable))->toBe(true); - expect($passable->count)->toBe(4); -}); - -it('can run as a pipe in a pipeline, with implicit trait', function () { - $anonymous = getAnonymous(); - $passable = Pipeline::send(getPassable()) - ->through([ - AsPipelineImplicitTest::class, - $anonymous, - AsPipelineImplicitTest::class, - $anonymous, - ]) - ->thenReturn(); - - expect(is_object($passable))->toBe(true); - expect($passable->count)->toBe(4); -}); - -it('can run as a pipe in a pipeline, without an explicit asPipeline method', function () { - $anonymous = getAnonymous(); - $passable = Pipeline::send(getPassable()) +it('can run as a pipe in a pipeline, with an explicit asPipeline method', function () { + $passable = Pipeline::send(new AsPipelinePassable) ->through([ - AsPipelineSingleParamHandleOnlyTest::class, - $anonymous, - AsPipelineSingleParamHandleOnlyTest::class, - $anonymous, + AsPipelineTest::class, + AsPipelineTest::class, + AsPipelineTest::class, + AsPipelineTest::class, ]) ->thenReturn(); - expect(is_object($passable))->toBe(true); + expect(is_a($passable, AsPipelinePassable::class))->toBe(true); expect($passable->count)->toBe(4); }); -it('can run as a noop/passthrough pipe in a pipeline, without a handle or asPipeline method', function () { - $anonymous = getAnonymous(); - $passable = Pipeline::send(getPassable()) - ->through([ - AsPipelineWithoutHandleOrAsPipeline::class, - $anonymous, - AsPipelineWithoutHandleOrAsPipeline::class, - $anonymous, - ]) - ->thenReturn(); - - expect(is_object($passable))->toBe(true); - expect($passable->count)->toBe(2); -}); - it('can run with an arbitrary via method configured on Pipeline', function () { - $anonymous = getAnonymous(); - $passable = Pipeline::send(getPassable()) - ->via('foobar') + $passable = Pipeline::send(new AsPipelinePassable) + ->via('arbitraryMethodThatDoesNotExistOnTheAction') ->through([ - AsPipelineImplicitTest::class, - $anonymous, - AsPipelineImplicitTest::class, - $anonymous, + AsPipelineTest::class, + AsPipelineTest::class, + AsPipelineTest::class, + AsPipelineTest::class, ]) ->thenReturn(); - expect(is_object($passable))->toBe(true); + expect(is_a($passable, AsPipelinePassable::class))->toBe(true); expect($passable->count)->toBe(4); }); - -it('cannot run as a pipe in a pipeline, with an explicit asPipeline method expecting multiple non-optional params', function () { - $anonymous = getAnonymous(); - $passable = Pipeline::send(getPassable()) - ->through([ - AsPipelineMultipleParamTest::class, - $anonymous, - AsPipelineMultipleParamTest::class, - $anonymous, - ]) - ->thenReturn(); -})->throws(ArgumentCountError::class, 'Too few arguments to function Lorisleiva\Actions\Tests\AsPipelineMultipleParamTest::asPipeline(), 1 passed and exactly 2 expected'); - -it('cannot run as a pipe in a pipeline, without an explicit asPipeline method and multiple non-optional handle params', function () { - $anonymous = getAnonymous(); - $passable = Pipeline::send(getPassable()) - ->through([ - AsPipelineMultipleParamHandleOnlyTest::class, - $anonymous, - AsPipelineMultipleParamHandleOnlyTest::class, - $anonymous, - ]) - ->thenReturn(); -})->throws(ArgumentCountError::class, 'Too few arguments to function Lorisleiva\Actions\Tests\AsPipelineMultipleParamHandleOnlyTest::handle(), 1 passed and exactly 2 expected'); diff --git a/tests/AsPipelineWithExplicitTraitTest.php b/tests/AsPipelineWithExplicitTraitTest.php new file mode 100644 index 0000000..4fbfc75 --- /dev/null +++ b/tests/AsPipelineWithExplicitTraitTest.php @@ -0,0 +1,30 @@ +increment(); + } +} + +it('can run as a pipe in a pipeline, with explicit trait, without asPipeline method', function () { + $passable = Pipeline::send(new AsPipelinePassable) + ->through([ + AsPipelineWithExplicitTraitTest::class, + AsPipelineWithExplicitTraitTest::class, + AsPipelineWithExplicitTraitTest::class, + AsPipelineWithExplicitTraitTest::class, + ]) + ->thenReturn(); + + expect(is_a($passable, AsPipelinePassable::class))->toBe(true); + expect($passable->count)->toBe(4); +}); diff --git a/tests/AsPipelineWithImplicitTraitTest.php b/tests/AsPipelineWithImplicitTraitTest.php new file mode 100644 index 0000000..a6ae369 --- /dev/null +++ b/tests/AsPipelineWithImplicitTraitTest.php @@ -0,0 +1,30 @@ +increment(); + } +} + +it('can run as a pipe in a pipeline, with implicit trait, without asPipeline method', function () { + $passable = Pipeline::send(new AsPipelinePassable) + ->through([ + AsPipelineWithImplicitTraitTest::class, + AsPipelineWithImplicitTraitTest::class, + AsPipelineWithImplicitTraitTest::class, + AsPipelineWithImplicitTraitTest::class, + ]) + ->thenReturn(); + + expect(is_a($passable, AsPipelinePassable::class))->toBe(true); + expect($passable->count)->toBe(4); +}); diff --git a/tests/AsPipelineWithMultipleNonOptionalParametersTest.php b/tests/AsPipelineWithMultipleNonOptionalParametersTest.php new file mode 100644 index 0000000..14fed54 --- /dev/null +++ b/tests/AsPipelineWithMultipleNonOptionalParametersTest.php @@ -0,0 +1,32 @@ +increment(); + } + + public function asPipeline(AsPipelinePassable $passable): AsPipelinePassable + { + $this->handle($passable); + + return $passable; + } +} + +it('cannot run as a pipe in a pipeline expecting multiple non-optional parameters', function () { + $passable = Pipeline::send(new AsPipelinePassable) + ->through([ + AsPipelineWithMultipleNonOptionalParametersTest::class, + ]) + ->thenReturn(); +})->throws(ArgumentCountError::class); From f5016107a56101ae355e2fe38ebd8334df295d4d Mon Sep 17 00:00:00 2001 From: Steven Maguire Date: Wed, 26 Feb 2025 12:00:26 -0600 Subject: [PATCH 11/11] Update solution to ensure compatiblity with PHP 8.4 --- src/Decorators/PipelineDecorator.php | 2 +- src/DesignPatterns/PipelineDesignPattern.php | 3 +- tests/AsPipelineTest.php | 59 +++++++++++++++---- tests/AsPipelineWithExplicitTraitTest.php | 13 ++-- tests/AsPipelineWithImplicitTraitTest.php | 13 ++-- ...eWithMultipleNonOptionalParametersTest.php | 18 +++++- .../PipelinePassable.php} | 4 +- 7 files changed, 79 insertions(+), 33 deletions(-) rename tests/{AsPipelinePassable.php => Stubs/PipelinePassable.php} (77%) diff --git a/src/Decorators/PipelineDecorator.php b/src/Decorators/PipelineDecorator.php index e901938..7a10b04 100644 --- a/src/Decorators/PipelineDecorator.php +++ b/src/Decorators/PipelineDecorator.php @@ -22,6 +22,6 @@ public function __invoke(mixed ...$arguments): mixed $method = $this->hasMethod('asPipeline') ? 'asPipeline' : 'handle'; - return $closure($this->callMethod($method, [$passable]) ?? $passable); + return $closure($this->callMethod($method, [$passable]) ?? $passable) ?? $passable; } } diff --git a/src/DesignPatterns/PipelineDesignPattern.php b/src/DesignPatterns/PipelineDesignPattern.php index b11e0d3..e828a53 100644 --- a/src/DesignPatterns/PipelineDesignPattern.php +++ b/src/DesignPatterns/PipelineDesignPattern.php @@ -17,7 +17,8 @@ public function getTrait(): string public function recognizeFrame(BacktraceFrame $frame): bool { - return $frame->matches(Pipeline::class, 'Illuminate\Pipeline\{closure}'); + return $frame->matches(Pipeline::class, 'Illuminate\Pipeline\{closure}') + || $frame->matches(Pipeline::class, '{closure:{closure:Illuminate\Pipeline\Pipeline::carry():184}:185}'); } public function decorate($instance, BacktraceFrame $frame) diff --git a/tests/AsPipelineTest.php b/tests/AsPipelineTest.php index 30b773b..abfe115 100644 --- a/tests/AsPipelineTest.php +++ b/tests/AsPipelineTest.php @@ -4,26 +4,25 @@ use Illuminate\Support\Facades\Pipeline; use Lorisleiva\Actions\Concerns\AsAction; +use Lorisleiva\Actions\Tests\Stubs\PipelinePassable; class AsPipelineTest { use AsAction; - public function handle(AsPipelinePassable $passable): void + public function handle(PipelinePassable $passable): void { $passable->increment(); } - public function asPipeline(AsPipelinePassable $passable): AsPipelinePassable + public function asPipeline(PipelinePassable $passable): void { $this->handle($passable); - - return $passable; } } it('can run as a pipe in a pipeline, with an explicit asPipeline method', function () { - $passable = Pipeline::send(new AsPipelinePassable) + $passable = Pipeline::send(new PipelinePassable) ->through([ AsPipelineTest::class, AsPipelineTest::class, @@ -32,21 +31,57 @@ public function asPipeline(AsPipelinePassable $passable): AsPipelinePassable ]) ->thenReturn(); - expect(is_a($passable, AsPipelinePassable::class))->toBe(true); + expect(is_a($passable, PipelinePassable::class))->toBe(true); expect($passable->count)->toBe(4); }); it('can run with an arbitrary via method configured on Pipeline', function () { - $passable = Pipeline::send(new AsPipelinePassable) + $passable = Pipeline::send(new PipelinePassable) ->via('arbitraryMethodThatDoesNotExistOnTheAction') ->through([ AsPipelineTest::class, - AsPipelineTest::class, - AsPipelineTest::class, - AsPipelineTest::class, + app()->make(AsPipelineTest::class), ]) ->thenReturn(); - expect(is_a($passable, AsPipelinePassable::class))->toBe(true); - expect($passable->count)->toBe(4); + expect(is_a($passable, PipelinePassable::class))->toBe(true); + expect($passable->count)->toBe(2); +}); + +it('can run as a pipe in a pipeline with only one explicit container resolved instance at the bottom of the stack', function () { + $passable = Pipeline::send(new PipelinePassable) + ->through([ + AsPipelineTest::class, // implicit container resolved instance + app()->make(AsPipelineTest::class), // explicit container resolved instance + ]) + ->thenReturn(); + + expect(is_a($passable, PipelinePassable::class))->toBe(true); + expect($passable->count)->toBe(2); +}); + +it('cannot run as a pipe in a pipeline with an explicit container resolved instance in the middle of the stack', function () { + $passable = Pipeline::send(new PipelinePassable) + ->through([ + AsPipelineTest::class, // implicit container resolved instance + app()->make(AsPipelineTest::class), // explicit container resolved instance + AsPipelineTest::class, // implicit container resolved instance + AsPipelineTest::class, // implicit container resolved instance + ]) + ->thenReturn(); + + expect(is_a($passable, PipelinePassable::class))->toBe(true); + expect($passable->count)->toBe(2); +}); + +it('cannot run as a pipe in a pipeline as an standalone instance', function () { + $passable = Pipeline::send(new PipelinePassable) + ->through([ + new AsPipelineTest, // standalone instance + AsPipelineTest::class, // implicit container resolved instance + app()->make(AsPipelineTest::class), // explicit container resolved instance + ]) + ->thenReturn(); + + expect(is_null($passable))->toBe(true); }); diff --git a/tests/AsPipelineWithExplicitTraitTest.php b/tests/AsPipelineWithExplicitTraitTest.php index 4fbfc75..d491ccb 100644 --- a/tests/AsPipelineWithExplicitTraitTest.php +++ b/tests/AsPipelineWithExplicitTraitTest.php @@ -4,27 +4,26 @@ use Illuminate\Support\Facades\Pipeline; use Lorisleiva\Actions\Concerns\AsPipeline; +use Lorisleiva\Actions\Tests\Stubs\PipelinePassable; class AsPipelineWithExplicitTraitTest { use AsPipeline; - public function handle(AsPipelinePassable $passable): void + public function handle(PipelinePassable $passable): void { $passable->increment(); } } it('can run as a pipe in a pipeline, with explicit trait, without asPipeline method', function () { - $passable = Pipeline::send(new AsPipelinePassable) + $passable = Pipeline::send(new PipelinePassable) ->through([ AsPipelineWithExplicitTraitTest::class, - AsPipelineWithExplicitTraitTest::class, - AsPipelineWithExplicitTraitTest::class, - AsPipelineWithExplicitTraitTest::class, + app()->make(AsPipelineWithExplicitTraitTest::class), ]) ->thenReturn(); - expect(is_a($passable, AsPipelinePassable::class))->toBe(true); - expect($passable->count)->toBe(4); + expect(is_a($passable, PipelinePassable::class))->toBe(true); + expect($passable->count)->toBe(2); }); diff --git a/tests/AsPipelineWithImplicitTraitTest.php b/tests/AsPipelineWithImplicitTraitTest.php index a6ae369..1e15405 100644 --- a/tests/AsPipelineWithImplicitTraitTest.php +++ b/tests/AsPipelineWithImplicitTraitTest.php @@ -4,27 +4,26 @@ use Illuminate\Support\Facades\Pipeline; use Lorisleiva\Actions\Concerns\AsAction; +use Lorisleiva\Actions\Tests\Stubs\PipelinePassable; class AsPipelineWithImplicitTraitTest { use AsAction; - public function handle(AsPipelinePassable $passable): void + public function handle(PipelinePassable $passable): void { $passable->increment(); } } it('can run as a pipe in a pipeline, with implicit trait, without asPipeline method', function () { - $passable = Pipeline::send(new AsPipelinePassable) + $passable = Pipeline::send(new PipelinePassable) ->through([ AsPipelineWithImplicitTraitTest::class, - AsPipelineWithImplicitTraitTest::class, - AsPipelineWithImplicitTraitTest::class, - AsPipelineWithImplicitTraitTest::class, + app()->make(AsPipelineWithImplicitTraitTest::class), ]) ->thenReturn(); - expect(is_a($passable, AsPipelinePassable::class))->toBe(true); - expect($passable->count)->toBe(4); + expect(is_a($passable, PipelinePassable::class))->toBe(true); + expect($passable->count)->toBe(2); }); diff --git a/tests/AsPipelineWithMultipleNonOptionalParametersTest.php b/tests/AsPipelineWithMultipleNonOptionalParametersTest.php index 14fed54..b023d6d 100644 --- a/tests/AsPipelineWithMultipleNonOptionalParametersTest.php +++ b/tests/AsPipelineWithMultipleNonOptionalParametersTest.php @@ -5,17 +5,19 @@ use ArgumentCountError; use Illuminate\Support\Facades\Pipeline; use Lorisleiva\Actions\Concerns\AsAction; +use Lorisleiva\Actions\Tests\Stubs\PipelinePassable; +use TypeError; class AsPipelineWithMultipleNonOptionalParametersTest { use AsAction; - public function handle(AsPipelinePassable $passable, int $nonOptionalAdditionalParameter): void + public function handle(PipelinePassable $passable, int $nonOptionalAdditionalParameter): void { $passable->increment(); } - public function asPipeline(AsPipelinePassable $passable): AsPipelinePassable + public function asPipeline(PipelinePassable $passable): PipelinePassable { $this->handle($passable); @@ -24,9 +26,19 @@ public function asPipeline(AsPipelinePassable $passable): AsPipelinePassable } it('cannot run as a pipe in a pipeline expecting multiple non-optional parameters', function () { - $passable = Pipeline::send(new AsPipelinePassable) + $passable = Pipeline::send(new PipelinePassable) ->through([ AsPipelineWithMultipleNonOptionalParametersTest::class, + app()->make(AsPipelineWithMultipleNonOptionalParametersTest::class), ]) ->thenReturn(); })->throws(ArgumentCountError::class); + +it('cannot run as a pipe in a pipeline as an explicit container resolved instance preceding an implicit container resolved instance', function () { + $passable = Pipeline::send(new PipelinePassable) + ->through([ + app()->make(AsPipelineWithMultipleNonOptionalParametersTest::class), + AsPipelineWithMultipleNonOptionalParametersTest::class, + ]) + ->thenReturn(); +})->throws(TypeError::class); diff --git a/tests/AsPipelinePassable.php b/tests/Stubs/PipelinePassable.php similarity index 77% rename from tests/AsPipelinePassable.php rename to tests/Stubs/PipelinePassable.php index 31aafe8..ebb2c7d 100644 --- a/tests/AsPipelinePassable.php +++ b/tests/Stubs/PipelinePassable.php @@ -1,11 +1,11 @@