Add tests and reorganize code based on further refined requirements and feedback

Author: stevenmaguire

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),

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);
     }
 }

tests/AsPipelinePassable.php

@@ -0,0 +1,19 @@
+<?php
+
+namespace Lorisleiva\Actions\Tests;
+
+/**
+ * Test fixture used in all of the AsPipeline{*} tests.
+ */
+class AsPipelinePassable
+{
+    public function __construct(public int $count = 0)
+    {
+        //
+    }
+
+    public function increment()
+    {
+        $this->count++;
+    }
+}

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');

tests/AsPipelineWithExplicitTraitTest.php

@@ -0,0 +1,30 @@
+<?php
+
+namespace Lorisleiva\Actions\Tests;
+
+use Illuminate\Support\Facades\Pipeline;
+use Lorisleiva\Actions\Concerns\AsPipeline;
+
+class AsPipelineWithExplicitTraitTest
+{
+    use AsPipeline;
+
+    public function handle(AsPipelinePassable $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)
+        ->through([
+            AsPipelineWithExplicitTraitTest::class,
+            AsPipelineWithExplicitTraitTest::class,
+            AsPipelineWithExplicitTraitTest::class,
+            AsPipelineWithExplicitTraitTest::class,
+        ])
+        ->thenReturn();
+
+    expect(is_a($passable, AsPipelinePassable::class))->toBe(true);
+    expect($passable->count)->toBe(4);
+});

tests/AsPipelineWithImplicitTraitTest.php

@@ -0,0 +1,30 @@
+<?php
+
+namespace Lorisleiva\Actions\Tests;
+
+use Illuminate\Support\Facades\Pipeline;
+use Lorisleiva\Actions\Concerns\AsAction;
+
+class AsPipelineWithImplicitTraitTest
+{
+    use AsAction;
+
+    public function handle(AsPipelinePassable $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)
+        ->through([
+            AsPipelineWithImplicitTraitTest::class,
+            AsPipelineWithImplicitTraitTest::class,
+            AsPipelineWithImplicitTraitTest::class,
+            AsPipelineWithImplicitTraitTest::class,
+        ])
+        ->thenReturn();
+
+    expect(is_a($passable, AsPipelinePassable::class))->toBe(true);
+    expect($passable->count)->toBe(4);
+});

tests/AsPipelineWithMultipleNonOptionalParametersTest.php

@@ -0,0 +1,32 @@
+<?php
+
+namespace Lorisleiva\Actions\Tests;
+
+use ArgumentCountError;
+use Illuminate\Support\Facades\Pipeline;
+use Lorisleiva\Actions\Concerns\AsAction;
+
+class AsPipelineWithMultipleNonOptionalParametersTest
+{
+    use AsAction;
+
+    public function handle(AsPipelinePassable $passable, int $nonOptionalAdditionalParameter): void
+    {
+        $passable->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);