Skip to content
This repository has been archived by the owner on Jan 21, 2020. It is now read-only.

Commit

Permalink
Merge branch 'hotfix/route-should-not-allow-empty-methods' into relea…
Browse files Browse the repository at this point in the history
…se-3.0.0

Close #59
  • Loading branch information
weierophinney committed Mar 7, 2018
2 parents 97ea994 + c0bf7dd commit c4380e0
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 10 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,11 @@ All notable changes to this project will be documented in this file, in reverse
or `OPTIONS` requests if they are not explicitly in the list of allowed
methods.

- [#59](https://github.com/zendframework/zend-expressive-router/pull/59) changes
the behavior of the `Route` constructor: it now raises an exception if the
list of HTTP methods provided to it is empty. Routes MUST have one or more
HTTP methods associated.

### Deprecated

- Nothing.
Expand Down
6 changes: 6 additions & 0 deletions src/Route.php
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,12 @@ public function getOptions() : array
*/
private function validateHttpMethods(array $methods) : array
{
if (empty($methods)) {
throw new Exception\InvalidArgumentException(
'HTTP methods argument was empty; must contain at least one method'
);
}

if (false === array_reduce($methods, function ($valid, $method) {
if (false === $valid) {
return false;
Expand Down
13 changes: 7 additions & 6 deletions test/Middleware/PathBasedRoutingMiddlewareTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

namespace ZendTest\Expressive\Router\Middleware;

use Fig\Http\Message\RequestMethodInterface as RequestMethod;
use PHPUnit\Framework\TestCase;
use Prophecy\Argument;
use Prophecy\Prophecy\ObjectProphecy;
Expand Down Expand Up @@ -72,11 +73,11 @@ public function process(
public function commonHttpMethods()
{
return [
'GET' => ['GET'],
'POST' => ['POST'],
'PUT' => ['PUT'],
'PATCH' => ['PATCH'],
'DELETE' => ['DELETE'],
RequestMethod::METHOD_GET => [RequestMethod::METHOD_GET],
RequestMethod::METHOD_POST => [RequestMethod::METHOD_POST],
RequestMethod::METHOD_PUT => [RequestMethod::METHOD_PUT],
RequestMethod::METHOD_PATCH => [RequestMethod::METHOD_PATCH],
RequestMethod::METHOD_DELETE => [RequestMethod::METHOD_DELETE],
];
}

Expand Down Expand Up @@ -178,7 +179,7 @@ public function testCommonHttpMethodsAreExposedAsClassMethodsAndReturnRoutes($me
public function testCreatingHttpRouteMethodWithExistingPathButDifferentMethodCreatesNewRouteInstance()
{
$this->router->addRoute(Argument::type(Route::class))->shouldBeCalledTimes(2);
$route = $this->middleware->route('/foo', $this->noopMiddleware, []);
$route = $this->middleware->route('/foo', $this->noopMiddleware, [RequestMethod::METHOD_POST]);

$middleware = $this->createNoopMiddleware();
$test = $this->middleware->get('/foo', $middleware);
Expand Down
17 changes: 13 additions & 4 deletions test/RouteTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -71,13 +71,13 @@ public function testRouteCanMatchMethod()

public function testRouteHeadMethodIsNotAllowedByDefault()
{
$route = new Route('/foo', $this->noopMiddleware, []);
$route = new Route('/foo', $this->noopMiddleware, [RequestMethod::METHOD_GET]);
$this->assertFalse($route->allowsMethod(RequestMethod::METHOD_HEAD));
}

public function testRouteOptionsMethodIsNotAllowedByDefault()
{
$route = new Route('/foo', $this->noopMiddleware, []);
$route = new Route('/foo', $this->noopMiddleware, [RequestMethod::METHOD_GET]);
$this->assertFalse($route->allowsMethod(RequestMethod::METHOD_OPTIONS));
}

Expand All @@ -103,7 +103,7 @@ public function testRouteNameForRouteAcceptingAnyMethodMatchesPathByDefault()

public function testRouteNameWithConstructor()
{
$route = new Route('/test', $this->noopMiddleware, [], 'test');
$route = new Route('/test', $this->noopMiddleware, [RequestMethod::METHOD_GET], 'test');
$this->assertSame('test', $route->getName());
}

Expand All @@ -116,7 +116,7 @@ public function testRouteNameWithGET()
public function testRouteNameWithGetAndPost()
{
$route = new Route('/test', $this->noopMiddleware, [RequestMethod::METHOD_GET, RequestMethod::METHOD_POST]);
$this->assertSame('/test^GET' . Route::HTTP_METHOD_SEPARATOR . 'POST', $route->getName());
$this->assertSame('/test^GET' . Route::HTTP_METHOD_SEPARATOR . RequestMethod::METHOD_POST, $route->getName());
}

public function testThrowsExceptionDuringConstructionIfPathIsNotString()
Expand Down Expand Up @@ -230,4 +230,13 @@ public function testRouteIsMiddlewareAndProxiesToComposedMiddleware()
$route = new Route('/foo', $middleware->reveal());
$this->assertSame($response, $route->process($request, $handler));
}

public function testConstructorShouldRaiseExceptionIfMethodsArgumentIsAnEmptyArray()
{
$middleware = $this->prophesize(MiddlewareInterface::class)->reveal();

$this->expectException(InvalidArgumentException::class);
$this->expectExceptionMessage('empty');
new Route('/foo', $middleware, []);
}
}

0 comments on commit c4380e0

Please sign in to comment.