Merge pull request #29273 from DarkGhostHunter/master

[6.0] Moved Response code from the Pipeline to the Routing Pipeline

src/Illuminate/Pipeline/Pipeline.php

@@ -6,9 +6,7 @@
 use Exception;
 use Throwable;
 use RuntimeException;
-use Illuminate\Http\Request;
 use Illuminate\Contracts\Container\Container;
-use Illuminate\Contracts\Support\Responsable;
 use Symfony\Component\Debug\Exception\FatalThrowableError;
 use Illuminate\Contracts\Pipeline\Pipeline as PipelineContract;
 
@@ -169,13 +167,11 @@ protected function carry()
                         $parameters = [$passable, $stack];
                     }
 
-                    $response = method_exists($pipe, $this->method)
+                    $carry = method_exists($pipe, $this->method)
                                     ? $pipe->{$this->method}(...$parameters)
                                     : $pipe(...$parameters);
 
-                    return $response instanceof Responsable
-                                ? $response->toResponse($this->getContainer()->make(Request::class))
-                                : $response;
+                    return $this->handleCarry($carry);
                 } catch (Exception $e) {
                     return $this->handleException($passable, $e);
                 } catch (Throwable $e) {
@@ -218,6 +214,17 @@ protected function getContainer()
         return $this->container;
     }
 
+    /**
+     * Handles the value returned from each pipe before passing it to the next.
+     *
+     * @param  mixed $carry
+     * @return mixed
+     */
+    protected function handleCarry($carry)
+    {
+        return $carry;
+    }
+
     /**
      * Handle the given exception.
      *
@@ -229,6 +236,6 @@ protected function getContainer()
      */
     protected function handleException($passable, Exception $e)
     {
-        throw $e; // actually handled in the Routing Pipeline
+        throw $e;
     }
 }

src/Illuminate/Routing/Pipeline.php

@@ -4,6 +4,7 @@
 
 use Exception;
 use Illuminate\Http\Request;
+use Illuminate\Contracts\Support\Responsable;
 use Illuminate\Contracts\Debug\ExceptionHandler;
 use Illuminate\Pipeline\Pipeline as BasePipeline;
 
@@ -14,6 +15,19 @@
  */
 class Pipeline extends BasePipeline
 {
+    /**
+     * Handles the value returned from each pipe before passing it to the next.
+     *
+     * @param  mixed $carry
+     * @return mixed
+     */
+    protected function handleCarry($carry)
+    {
+        return $carry instanceof Responsable
+            ? $carry->toResponse($this->getContainer()->make(Request::class))
+            : $carry;
+    }
+
     /**
      * Handle the given exception.
      *

tests/Pipeline/PipelineTest.php

@@ -6,7 +6,6 @@
 use PHPUnit\Framework\TestCase;
 use Illuminate\Pipeline\Pipeline;
 use Illuminate\Container\Container;
-use Illuminate\Contracts\Support\Responsable;
 
 class PipelineTest extends TestCase
 {
@@ -65,23 +64,6 @@ function ($piped) {
         unset($_SERVER['__test.pipe.one']);
     }
 
-    public function testPipelineUsageWithResponsableObjects()
-    {
-        $result = (new Pipeline(new Container))
-            ->send('foo')
-            ->through([new PipelineTestPipeResponsable])
-            ->then(
-                function ($piped) {
-                    return $piped;
-                }
-            );
-
-        $this->assertEquals('bar', $result);
-        $this->assertEquals('foo', $_SERVER['__test.pipe.responsable']);
-
-        unset($_SERVER['__test.pipe.responsable']);
-    }
-
     public function testPipelineUsageWithCallable()
     {
         $function = function ($piped, $next) {
@@ -192,14 +174,6 @@ public function differentMethod($piped, $next)
     }
 }
 
-class PipeResponsable implements Responsable
-{
-    public function toResponse($request)
-    {
-        return 'bar';
-    }
-}
-
 class PipelineTestPipeTwo
 {
     public function __invoke($piped, $next)
@@ -210,16 +184,6 @@ public function __invoke($piped, $next)
     }
 }
 
-class PipelineTestPipeResponsable
-{
-    public function handle($piped, $next)
-    {
-        $_SERVER['__test.pipe.responsable'] = $piped;
-
-        return new PipeResponsable;
-    }
-}
-
 class PipelineTestParameterPipe
 {
     public function handle($piped, $next, $parameter1 = null, $parameter2 = null)

tests/Routing/RoutingRouteTest.php

@@ -23,6 +23,7 @@
 use Illuminate\Routing\ResourceRegistrar;
 use Illuminate\Contracts\Routing\Registrar;
 use Illuminate\Auth\Middleware\Authenticate;
+use Illuminate\Contracts\Support\Responsable;
 use Illuminate\Http\Exceptions\HttpResponseException;
 use Illuminate\Routing\Middleware\SubstituteBindings;
 use Illuminate\Database\Eloquent\ModelNotFoundException;
@@ -220,6 +221,28 @@ public function testMiddlewareWorksIfControllerThrowsHttpResponseException()
         $this->assertEquals('hello caught', $response);
     }
 
+    public function testReturnsResponseWhenMiddlewareReturnsResponsable()
+    {
+        $router = $this->getRouter();
+        $router->get('foo/bar', [
+            'uses' => RouteTestClosureMiddlewareController::class.'@index',
+            'middleware' => ['foo', 'bar', 'baz'],
+        ]);
+        $router->aliasMiddleware('foo', function ($request, $next) {
+            return $next($request);
+        });
+        $router->aliasMiddleware('bar', function ($request, $next) {
+            return new ResponsableResponse;
+        });
+        $router->aliasMiddleware('baz', function ($request, $next) {
+            return $next($request);
+        });
+        $this->assertEquals(
+            'bar',
+            $router->dispatch(Request::create('foo/bar', 'GET'))->getContent()
+        );
+    }
+
     public function testDefinedClosureMiddleware()
     {
         $router = $this->getRouter();
@@ -1849,6 +1872,14 @@ public function handle($request, $next)
     }
 }
 
+class ResponsableResponse implements Responsable
+{
+    public function toResponse($request)
+    {
+        return new Response('bar');
+    }
+}
+
 class RouteBindingStub
 {
     public function bind($value, $route)