From ea22de3c8ed07f375f1bad1fff7e35c9c03a4df0 Mon Sep 17 00:00:00 2001 From: Steven Maguire Date: Fri, 10 Jan 2025 11:04:46 -0600 Subject: [PATCH 1/9] 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 2/9] 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 3/9] 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 4/9] 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 5/9] 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 6/9] 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 7/9] 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 8/9] 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 9/9] 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')