Merge pull request #3 from sasezaki/return-type-clear

Introduce DispatchResult - Dispatcher must not have Response itself
sasezaki · Oct 29, 2015 · a23edff · a23edff
2 parents 388f52e + 0b94c0e
commit a23edff
Show file tree

Hide file tree

Showing 7 changed files with 95 additions and 50 deletions.
diff --git a/src/Backbeard/DispatchResult.php b/src/Backbeard/DispatchResult.php
@@ -0,0 +1,37 @@
+<?php
+
+namespace Backbeard;
+
+use Psr\Http\Message\ResponseInterface as Response;
+use LogicException;
+
+class DispatchResult implements DispatchResultInterface
+{
+    private $dispatched;
+    private $response;
+
+    public function __construct($dispatched, $response = null)
+    {
+        $this->dispatched = $dispatched;
+        $this->response = $response;
+    }
+
+    /**
+     * @return bool
+     */
+    public function isDispatched()
+    {
+        return $this->dispatched;
+    }
+
+    /**
+     * @return Response;
+     */
+    public function getResponse()
+    {
+        if ($this->response instanceof Response) {
+            return $this->response;
+        }
+        throw new LogicException("Don't call when dispatch return false");    
+    }
+}
diff --git a/src/Backbeard/DispatchResultInterface.php b/src/Backbeard/DispatchResultInterface.php
@@ -0,0 +1,16 @@
+<?php
+
+namespace Backbeard;
+
+interface DispatchResultInterface
+{
+    /**
+     * @return bool
+     */
+    public function isDispatched();
+
+    /**
+     * @return \Psr\Http\Message\ResponseInterface
+     */
+    public function getResponse();
+}
diff --git a/src/Backbeard/Dispatcher.php b/src/Backbeard/Dispatcher.php
@@ -29,11 +29,6 @@ class Dispatcher
      */
     private $templatePathResolver;
 
-    /**
-     * @var \Psr\Http\Message\ResponseInterface
-     */
-    private $actionResponse;
-
     public function __construct(Generator $routing, ViewInterface $view, RouterInterface $router)
     {
         $this->routing = $routing;
@@ -45,7 +40,7 @@ public function __construct(Generator $routing, ViewInterface $view, RouterInter
      * @param Request  $request
      * @param Response $response
      *
-     * @return bool
+     * @return DispatchResultInterface
      */
     public function dispatch(Request $request, Response $response)
     {
@@ -89,32 +84,18 @@ public function dispatch(Request $request, Response $response)
                     continue;
                 }
 
-                $this->actionResponse = $this->handleActionResult($routeResult, $actionResult, $response);
+                $response = $this->handleActionResult($routeResult, $actionResult, $response);
 
-                return true;
+                return new DispatchResult(true, $response);
             }
             $this->routing->next();
         }
 
-        return false;
-    }
-
-    /**
-     * @throws \LogicException
-     *
-     * @return \Psr\Http\Message\ResponseInterface
-     */
-    public function getActionResponse()
-    {
-        if ($this->actionResponse instanceof Response) {
-            return $this->actionResponse;
-        }
-
-        throw new \LogicException("Don't call when dispatch return false");
+        return new DispatchResult(false);
     }
 
     /**
-     * @return \SfpBackbeard\TemplatePathResolverInterface
+     * @return \Backbeard\TemplatePathResolverInterface
      */
     public function getTemplatePathResolver()
     {
@@ -125,6 +106,10 @@ public function getTemplatePathResolver()
         return $this->templatePathResolver;
     }
 
+    /**
+     * @return bool
+     * @throws InvalidArgumentException
+     */
     protected function dispatchByType($route, Request $request)
     {
         switch (gettype($route)) {
@@ -166,6 +151,9 @@ protected function dispatchByType($route, Request $request)
         }
     }
 
+    /**
+     * @return Response
+     */
     protected function handleActionResult(RouteMatch $routeMatch, $actionResult, Response $response)
     {
         if (is_string($actionResult)) {

diff --git a/src/Backbeard/RouteMatch.php b/src/Backbeard/RouteMatch.php
@@ -3,7 +3,7 @@
 namespace Backbeard;
 
 /**
- * RouteInterface match result.
+ * Route match result.
  */
 class RouteMatch
 {
@@ -35,8 +35,6 @@ public function __construct(array $params)
      * Set name of matched route.
      *
      * @param string $name
-     *
-     * @return RouteMatch
      */
     public function setMatchedRouteName($name)
     {

diff --git a/src/Backbeard/Router.php b/src/Backbeard/Router.php
@@ -31,6 +31,9 @@ public function match($route, $uri)
         return false;
     }
 
+    /**
+     * @return array
+     */
     private function buildRegexForRoute($routeData)
     {
         $regex = '';

diff --git a/src/Backbeard/TemplatePathResolver.php b/src/Backbeard/TemplatePathResolver.php
@@ -6,6 +6,9 @@ class TemplatePathResolver implements TemplatePathResolverInterface
 {
     private $suffix = '.phtml';
 
+    /**
+     * @return string
+     */
     public function resolve(RouteMatch $routeMatch)
     {
         $name = $routeMatch->getMatchedRouteName();

diff --git a/tests/DispatcherTest.php b/tests/DispatcherTest.php
@@ -36,8 +36,8 @@ public function testReturnFalseWhenNotMatched()
             yield function () {return false;} => function () {};
         }), $this->view, $this->router);
         $result = $dispatcher->dispatch($request, $response);
-        $this->assertFalse($result);
-        $dispatcher->getActionResponse();
+        $this->assertFalse($result->isDispatched());
+        $result->getResponse();
     }
 
     public function testActionScope()
@@ -52,7 +52,7 @@ public function testActionScope()
             };
         }), $this->view, $this->router);
         $result = $dispatcher->dispatch($request, $response);
-        $this->assertTrue($result);
+        $this->assertTrue($result->isDispatched());
         $this->assertTrue($actionScopeResult['request'] instanceof ServerRequestInterface);
         $this->assertTrue($actionScopeResult['response'] instanceof ResponseInterface);
     }
@@ -75,16 +75,16 @@ public function testRoutingStringHandleAsRoute()
 
         $result = $dispatcher->dispatch($request, $response);
 
-        $this->assertTrue($result);
-        $response = $dispatcher->getActionResponse();
+        $this->assertTrue($result->isDispatched());
+        $response = $result->getResponse();
         $this->assertInstanceof(Response::class, $response);
         $this->assertSame('5', (string) $response->getBody());
 
         // not matched
         $request = ServerRequestFactory::fromGlobals()->withUri(new Uri('http://example.com/bar/6'));
         $response = new Response();
         $result = $dispatcher->dispatch($request, $response);
-        $response = $dispatcher->getActionResponse();
+        $response = $result->getResponse();
         $this->assertInstanceof(Response::class, $response);
         $this->assertSame('6', (string) $response->getBody());
     }
@@ -104,16 +104,16 @@ public function testRoutingKeyHandleStringAsPath()
         $dispatcher3 = new Dispatcher($gen(), $this->view, $this->router);
 
         $result = $dispatcher1->dispatch($request, $response);
-        $this->assertFalse($result);
+        $this->assertFalse($result->isDispatched());
 
         $request = $request->withUri((new Uri())->withPath('/foo'))->withMethod('GET');
         $result = $dispatcher2->dispatch($request, $response);
-        $this->assertTrue($result);
+        $this->assertTrue($result->isDispatched());
         $this->assertSame('hello', (string) $response->getBody());
 
         $request = $request->withUri((new Uri())->withPath('/foo'))->withMethod('POST');
         $result = $dispatcher3->dispatch($request, $response);
-        $this->assertFalse($result);
+        $this->assertFalse($result->isDispatched());
 
         $gen = function () {
             yield ['GET' => '/foo',
@@ -132,12 +132,12 @@ public function testRoutingKeyHandleStringAsPath()
 
         $request = $request->withUri((new Uri())->withPath('/foo'))->withMethod('GET');
         $result = $dispatcher4->dispatch($request, $response);
-        $this->assertFalse($result);
+        $this->assertFalse($result->isDispatched());
 
         $request = $request->withUri((new Uri())->withPath('/foo'))->withMethod('GET');
         $request = $request->withHeader('User-Agent', 'Mozilla/5.0');
         $result = $dispatcher5->dispatch($request, $response);
-        $this->assertTrue($result);
+        $this->assertTrue($result->isDispatched());
     }
 
     public function testRoutingKeyHandleClosureAsMatcher()
@@ -149,7 +149,7 @@ public function testRoutingKeyHandleClosureAsMatcher()
             yield function () {return true;} => function () {return 'bar';};
         }), $this->view, $this->router);
         $result = $dispatcher->dispatch($request, $response);
-        $response = $dispatcher->getActionResponse();
+        $response = $result->getResponse();
 
         $this->assertSame('bar', (string) $response->getBody());
     }
@@ -163,7 +163,7 @@ public function testRouterResultArrayShouldPassAction()
             yield function () {return ['var1', 'var2'];} => function ($var1, $var2) {return $var1.$var2;};
         }), $this->view, $this->router);
         $result = $dispatcher->dispatch($request, $response);
-        $response = $dispatcher->getActionResponse();
+        $response = $result->getResponse();
         $this->assertSame('var1var2', (string) $response->getBody());
     }
 
@@ -176,7 +176,7 @@ public function testRouterResultRouteMatchShouldPassAction()
             yield function () {return new RouteMatch(['var1', 'var2']);} => function ($var1, $var2) {return $var1.$var2;};
         }), $this->view, $this->router);
         $result = $dispatcher->dispatch($request, $response);
-        $response = $dispatcher->getActionResponse();
+        $response = $result->getResponse();
         $this->assertSame('var1var2', (string) $response->getBody());
     }
 
@@ -206,7 +206,7 @@ public function testActionReturnResponseShouldBeUsed()
             };
         }), $this->view, $this->router);
         $result = $dispatcher->dispatch($request, $response);
-        $response2 = $dispatcher->getActionResponse();
+        $response2 = $result->getResponse();
         $this->assertNotSame(spl_object_hash($response), spl_object_hash($response2));
     }
 
@@ -220,7 +220,7 @@ public function testIntActionAsStatusCode()
         }), $this->view, $this->router);
 
         $result = $dispatcher->dispatch($request, $response);
-        $response = $dispatcher->getActionResponse();
+        $response = $result->getResponse();
         $this->assertSame(503, $response->getStatusCode());
     }
 
@@ -234,7 +234,7 @@ public function testActionResultIsIntUsedAsStatusCode()
         }), $this->view, $this->router);
 
         $result = $dispatcher->dispatch($request, $response);
-        $response = $dispatcher->getActionResponse();
+        $response = $result->getResponse();
         $this->assertSame(503, $response->getStatusCode());
     }
 
@@ -251,7 +251,7 @@ public function testActionReturnUnkown()
         }), $this->view, $this->router);
 
         $result = $dispatcher->dispatch($request, $response);
-        $response = $dispatcher->getActionResponse();
+        $response = $result->getResponse();
         $this->assertSame('a', (string) $response->getBody());
     }
 
@@ -266,7 +266,7 @@ public function testRoutenameWouldbeResolveAsTemplateName()
         }), $this->view, $this->router);
 
         $result = $dispatcher->dispatch($request, $response);
-        $response = $dispatcher->getActionResponse();
+        $response = $result->getResponse();
         $this->assertInstanceof(ResponseInterface::class, $response);
         $this->assertSame('var', (string) $response->getBody());
     }
@@ -283,7 +283,7 @@ public function testContinueWhenActionReturnIsFalse()
         }), $this->view, $this->router);
 
         $result = $dispatcher->dispatch($request, $response);
-        $response = $dispatcher->getActionResponse();
+        $response = $result->getResponse();
         $this->assertSame('oh matched', (string) $response->getBody());
     }
 
@@ -299,7 +299,7 @@ public function testContinueWhenActionReturnHasActionContinueInterface()
         }), $this->view, $this->router);
 
         $result = $dispatcher->dispatch($request, $response);
-        $response = $dispatcher->getActionResponse();
+        $response = $result->getResponse();
 
         $this->assertSame('bar', (string) $response->getBody());
     }
@@ -325,7 +325,7 @@ public function testValidationErrorShouldContinueRoutingAndHasError()
         }), $this->view, $this->router);
 
         $result = $dispatcher->dispatch($request, $response);
-        $response = $dispatcher->getActionResponse();
+        $response = $result->getResponse();
 
         $this->assertSame('foo', (string) $response->getBody());
     }
@@ -350,7 +350,7 @@ public function testValidationThroughWhenNotMatchedRouting()
         }), $this->view, $this->router);
 
         $result = $dispatcher->dispatch($request, $response);
-        $response = $dispatcher->getActionResponse();
+        $response = $result->getResponse();
         $this->assertSame('error is NULL', (string) $response->getBody());
     }
 }