Merge pull request #7 from Minstel/router-cleanup

'Not found' middleware

4 files changed, 408 insertions, 3 deletions

diff --git a/src/Router/Middleware/NotFound.php b/src/Router/Middleware/NotFound.php
new file mode 100644
index 0000000..97b4a51
--- /dev/null
+++ b/src/Router/Middleware/NotFound.php
@@ -0,0 +1,105 @@
+<?php
+
+namespace Jasny\Router\Middleware;
+
+use Jasny\Router\Routes;
+use Psr\Http\Message\ServerRequestInterface;
+use Psr\Http\Message\ResponseInterface;
+
+/**
+ * Set response to 'not found' or 'method not allowed' if route is non exist 
+ */
+class NotFound
+{    
+    /**
+     * Routes
+     * @var Routes
+     */
+    protected $routes = null;
+
+    /**
+     * Action for 'not found' case
+     * @var callback|int
+     **/
+    protected $notFound = null;
+
+    /**
+     * Action for 'method not allowed' case
+     * @var callback|int
+     **/
+    protected $methodNotAllowed = null;
+
+    /**
+     * Class constructor
+     * 
+     * @param Routes $routes
+     * @param callback|int $notFound 
+     * @param callback|int $methodNotAllowed 
+     */
+    public function __construct(Routes $routes, $notFound = 404, $methodNotAllowed = null)
+    {
+        if (!(is_numeric($notFound) && $notFound >= 100 && $notFound <= 599) && !is_callable($notFound)) {
+            throw new \InvalidArgumentException("'Not found' parameter should be a code in range 100-599 or a callback");
+        }
+
+        if ($methodNotAllowed && !(is_numeric($methodNotAllowed) && $methodNotAllowed >= 100 && $methodNotAllowed <= 599) && !is_callable($methodNotAllowed)) {
+            throw new \InvalidArgumentException("'Method not allowed' parameter should be a code in range 100-599 or a callback");   
+        }
+
+        $this->routes = $routes;
+        $this->notFound = $notFound;
+        $this->methodNotAllowed = $methodNotAllowed;
+    }
+
+    /**
+     * Get routes
+     *
+     * @return Routes
+     */
+    public function getRoutes()
+    {
+        return $this->routes;
+    }
+
+    /**
+     * Run middleware action
+     *
+     * @param ServerRequestInterface $request
+     * @param ResponseInterface      $response
+     * @param callback               $next
+     * @return ResponseInterface
+     */
+    public function __invoke(ServerRequestInterface $request, ResponseInterface $response, $next = null)
+    {
+        if ($next && !is_callable($next)) {
+            throw new \InvalidArgumentException("'next' should be a callback");            
+        }
+
+        if ($this->getRoutes()->hasRoute($request)) {
+            return $next ? $next($request, $response) : $response;    
+        }
+
+        $status = $this->methodNotAllowed && $this->getRoutes()->hasRoute($request, false) ? 
+            $this->methodNotAllowed : $this->notFound;
+
+        return is_numeric($status) ? $this->simpleResponse($response, $status) : call_user_func($status, $request, $response);
+    }
+
+    /**
+     * Simple response
+     *
+     * @param ResponseInterface $response
+     * @param int $code 
+     * @return ResponseInterface
+     */
+    protected function simpleResponse(ResponseInterface $response, $code)
+    {
+        $message = 'Not Found';
+
+        $body = $response->getBody();        
+        $body->rewind();
+        $body->write($message);
+
+        return $response->withStatus($code, $message)->withBody($body);
+    }
+}
diff --git a/src/Router/Routes/Glob.php b/src/Router/Routes/Glob.php
index 97483d8..a9c2148 100644
--- a/src/Router/Routes/Glob.php
+++ b/src/Router/Routes/Glob.php
@@ -158,7 +158,7 @@ class Glob extends ArrayObject implements Routes
             if ($path !== '/') $path = rtrim($path, '/');
             
             if ($this->fnmatch($path, $url)) {
-                if ((empty($inc) || in_array($method, $inc)) && !in_array($method, $excl)) {
+                if (!$method || ((empty($inc) || in_array($method, $inc)) && !in_array($method, $excl))) {
                     $ret = $route;
                     break;
                 }
@@ -369,9 +369,9 @@ class Glob extends ArrayObject implements Routes
      * @param ServerRequestInterface $request
      * @return boolean
      */
-    public function hasRoute(ServerRequestInterface $request)
+    public function hasRoute(ServerRequestInterface $request, $withMethod = true)
     {
-        $route = $this->findRoute($request->getUri(), $request->getMethod());
+        $route = $this->findRoute($request->getUri(), $withMethod ? $request->getMethod() : null);
         return isset($route);
     }
     
diff --git a/tests/Router/Middleware/NotFoundTest.php b/tests/Router/Middleware/NotFoundTest.php
new file mode 100644
index 0000000..0c40a68
--- /dev/null
+++ b/tests/Router/Middleware/NotFoundTest.php
@@ -0,0 +1,265 @@
+<?php
+
+use Jasny\Router\Routes;
+use Jasny\Router\Middleware\NotFound;
+use Psr\Http\Message\ServerRequestInterface;
+use Psr\Http\Message\ResponseInterface;
+use Psr\Http\Message\StreamInterface;
+
+class NotFoundTest extends PHPUnit_Framework_TestCase
+{
+    /**
+     * Test creating object with false parameters
+     *
+     * @dataProvider constructProvider
+     * @param string $notFound 
+     * @param string $notAllowed
+     * @param boolean $positive 
+     */
+    public function testConstruct($notFound, $notAllowed, $positive)
+    {   
+        if (!$positive) $this->expectException(\InvalidArgumentException::class);
+
+        $middleware = new NotFound($this->getRoutes(), $notFound, $notAllowed);
+
+        if ($positive) $this->skipTest();
+    }
+
+    /**
+     * Provide data for testing '__contruct'
+     */
+    public function constructProvider()
+    {
+        return [
+            [null, 405, false],
+            [true, true, false],
+            [99, null, false],
+            [600, null, false],
+            [404, 99, false],
+            [404, 600, false],
+            [200, 405, true],
+            [404, 200, true]
+        ];
+    }
+
+    /**
+     * Test invoke with invalid 'next' param
+     */
+    public function testInvokeInvalidNext()
+    {   
+        $middleware = new NotFound($this->getRoutes(), 404, 405);
+        list($request, $response) = $this->getRequests();
+
+        $this->expectException(\InvalidArgumentException::class);
+
+        $result = $middleware($request, $response, 'not_callable');
+    }
+
+    /**
+     * Test that 'next' callback is invoked when route is found
+     *
+     * @dataProvider invokeProvider
+     * @param callback|int $notFound 
+     * @param callback|int $notAllowed 
+     * @param callback $next
+     */
+    public function testInvokeFound($notFound, $notAllowed, $next)
+    {
+        if (!$next) return $this->skipTest();
+
+        list($request, $response) = $this->getRequests();
+        $routes = $this->getRoutes();
+        $middleware = new NotFound($routes, $notFound, $notAllowed);
+
+        $this->expectRoute($routes, $request, 'found');
+        $this->notExpectSimpleError($response);
+
+        $result = $middleware($request, $response, $next);
+
+        $this->assertEquals(get_class($response), get_class($result), "Result must be an instance of 'ResponseInterface'");
+        $this->assertTrue($result->nextCalled, "'next' was not called");
+        $this->assertFalse(isset($result->notAllowedCalled), "'Not allowed' callback was called");
+        $this->assertFalse(isset($result->notFoundCalled), "'Not found' callback was called");
+    }
+
+    /**
+     * Test __invoke method in case of route is found with another method
+     *
+     * @dataProvider invokeProvider
+     * @param callback|int $notFound 
+     * @param callback|int $notAllowed 
+     * @param callback $next
+     */
+    public function testInvokeNotAllowed($notFound, $notAllowed, $next)
+    {
+        if (!$notAllowed) return $this->skipTest();
+
+        list($request, $response) = $this->getRequests();
+        $routes = $this->getRoutes();
+        $middleware = new NotFound($routes, $notFound, $notAllowed);
+
+        $this->expectRoute($routes, $request, 'notAllowed');
+        if (is_numeric($notAllowed)) {
+            $this->expectSimpleError($response, $notAllowed);
+        }
+
+        $result = $middleware($request, $response, $next);
+
+        $this->assertEquals(get_class($response), get_class($result), "Result must be an instance of 'ResponseInterface'");
+        $this->assertFalse(isset($result->nextCalled), "'next' was called");
+        
+        if (is_callable($notAllowed)) {
+            $this->assertTrue($result->notAllowedCalled, "'Not allowed' callback was not called");
+        }
+    }
+
+    /**
+     * Test __invoke method in case of route not found at all
+     *
+     * @dataProvider invokeProvider
+     * @param callback|int $notFound 
+     * @param callback|int $notAllowed 
+     * @param callback $next
+     */
+    public function testInvokeNotFound($notFound, $notAllowed, $next)
+    {
+        list($request, $response) = $this->getRequests();
+        $routes = $this->getRoutes();
+        $middleware = new NotFound($routes, $notFound, $notAllowed);
+
+        $case = $notAllowed ? 'notFoundTwice' : 'notFoundOnce';
+        $this->expectRoute($routes, $request, $case);
+
+        if (is_numeric($notFound)) {
+            $this->expectSimpleError($response, $notFound);
+        }
+
+        $result = $middleware($request, $response, $next);
+
+        $this->assertEquals(get_class($response), get_class($result), "Result must be an instance of 'ResponseInterface'");
+        $this->assertFalse(isset($result->nextCalled), "'next' was called");
+        
+        if (is_callable($notAllowed)) {
+            $this->assertFalse(isset($result->notAllowedCalled), "'Not allowed' callback was called");
+        }
+        if (is_callable($notFound)) {
+            $this->assertTrue($result->notFoundCalled, "'Not found' callback was not called");
+        }
+    }
+
+    /**
+     * Set expectations on finding route
+     *
+     * @param Routes $routes
+     * @param ServerRequestInterface $request
+     * @param string $case 
+     */
+    public function expectRoute($routes, $request, $case)
+    {
+        if ($case === 'found' || $case === 'notFoundOnce') {
+            $found = $case === 'found';
+
+            $routes->expects($this->once())->method('hasRoute')
+                ->with($this->equalTo($request))->will($this->returnValue($found));            
+        } elseif ($case === 'notAllowed' || $case === 'notFoundTwice') {
+            $routes->expects($this->exactly(2))->method('hasRoute')
+                ->withConsecutive(
+                    [$this->equalTo($request)],
+                    [$this->equalTo($request), $this->equalTo(false)]
+                )->will($this->returnCallback(function($request, $searchMethod = true) use ($case) {
+                    return $case === 'notFoundTwice' ? false : !$searchMethod;
+                }));
+        }
+    }
+
+    /**
+     * Provide data for testing invoke method
+     */
+    public function invokeProvider()
+    {
+        $callbacks = [];
+        foreach (['notFound', 'notAllowed', 'next'] as $type) {
+            $var = $type . 'Called';
+
+            $callbacks[$type] = function($request, $response) use ($var) {
+                $response->$var = true;
+                return $response;
+            };
+        }
+
+        return [
+            [404, 405, $callbacks['next']],
+            [404, 405, null],
+            [404, null, $callbacks['next']],
+            [404, null, null],
+            [$callbacks['notFound'], $callbacks['notAllowed'], $callbacks['next']],
+            [$callbacks['notFound'], $callbacks['notAllowed'], null],
+            [$callbacks['notFound'], null, $callbacks['next']],
+            [$callbacks['notFound'], null, null]
+        ];
+    }
+
+    /**
+     * Expect that response is set to simple deny answer
+     *
+     * @param ResponseInterface $response
+     * @param int $statusCode 
+     */
+    public function expectSimpleError(ResponseInterface $response, $statusCode)
+    {
+        $stream = $this->createMock(StreamInterface::class);
+        $stream->expects($this->once())->method('rewind');
+        $stream->expects($this->once())->method('write')->with($this->equalTo('Not Found'));
+
+        $response->method('getBody')->will($this->returnValue($stream));
+        $response->expects($this->once())->method('withBody')->with($this->equalTo($stream))->will($this->returnSelf());
+        $response->expects($this->once())->method('withStatus')->with($this->equalTo($statusCode), $this->equalTo('Not Found'))->will($this->returnSelf());
+    }
+
+    /**
+     * Expect that there would be no simple error response
+     *
+     * @param ResponseInterface $response
+     */
+    public function notExpectSimpleError(ResponseInterface $response)
+    {
+        $stream = $this->createMock(StreamInterface::class);
+        $stream->expects($this->never())->method('rewind');
+        $stream->expects($this->never())->method('write');
+
+        $response->expects($this->never())->method('getBody');
+        $response->expects($this->never())->method('withBody');
+        $response->expects($this->never())->method('withStatus');   
+    }
+
+    /**
+     * Get requests for testing
+     *
+     * @return array
+     */
+    public function getRequests()
+    {
+        $request = $this->createMock(ServerRequestInterface::class);
+        $response = $this->createMock(ResponseInterface::class);
+
+        return [$request, $response];
+    }
+
+    /**
+     * Get routes array
+     *
+     * @return Routes
+     */
+    public function getRoutes()
+    {
+        return $this->getMockBuilder(Routes::class)->disableOriginalConstructor()->getMock();        
+    }
+
+    /**
+     * Skip the test pass
+     */
+    public function skipTest()
+    {
+        return $this->assertTrue(true);
+    }
+}
diff --git a/tests/Router/Routes/GlobTest.php b/tests/Router/Routes/GlobTest.php
index 565c51b..30ceddc 100644
--- a/tests/Router/Routes/GlobTest.php
+++ b/tests/Router/Routes/GlobTest.php
@@ -247,6 +247,41 @@ class GlobTest extends PHPUnit_Framework_TestCase
     }
 
     /**
+     * Test checking if route exists regardless of request method
+     *
+     * @dataProvider getHasRouteNoMethodProvider
+     */
+    public function testHasRouteNoMethod($uri, $method)
+    {
+        $values = [
+            '/' => ['controller' => 'value0'],
+            '/foo/bar' => ['controller' => 'value1'],
+            '/foo +GET' => ['controller' => 'value2'],
+            '/bar/foo/zet -POST' => ['controller' => 'value3']
+        ];
+
+        $glob = new Glob($values);
+        $request = $this->getServerRequest($uri, $method);
+
+        $this->assertTrue($glob->hasRoute($request, false), "Route not exists");
+    }
+
+    /**
+     * Provide data for creating ServerRequestInterface objects
+     */
+    public function getHasRouteNoMethodProvider()
+    {
+        return [
+            ['/', 'GET'],
+            ['/foo/bar', 'GET'],
+            ['/foo', 'GET'],
+            ['/foo', 'POST'],
+            ['/bar/foo/zet', 'GET'],
+            ['/bar/foo/zet', 'POST']
+        ];
+    }
+
+    /**
      * Test binding simple string when getting route
      */
     public function testBindVarString()