Skip to content

Commit

Permalink
added ability to throw a method not found instead of 404
Browse files Browse the repository at this point in the history
  • Loading branch information
@n0nag0n
n0nag0n committed May 17, 2024
1 parent f6da4c4 commit 27718cf
Show file tree
Hide file tree
Showing 6 changed files with 186 additions and 128 deletions.
49 changes: 27 additions & 22 deletions flight/Engine.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -388,15 +388,14 @@ protected function processMiddleware(Route $route, string $eventName): bool
{
$atLeastOneMiddlewareFailed = false;

// Process things normally for before, and then in reverse order for after.
$middlewares = $eventName === Dispatcher::FILTER_BEFORE
? $route->middleware
: array_reverse($route->middleware);
// Process things normally for before, and then in reverse order for after.
$middlewares = $eventName === Dispatcher::FILTER_BEFORE
? $route->middleware
: array_reverse($route->middleware);
$params = $route->params;

foreach ($middlewares as $middleware) {

// Assume that nothing is going to be executed for the middleware.
// Assume that nothing is going to be executed for the middleware.
$middlewareObject = false;

// Closure functions can only run on the before event
Expand Down Expand Up @@ -426,12 +425,12 @@ protected function processMiddleware(Route $route, string $eventName): bool
}
}

// If nothing was resolved, go to the next thing
// If nothing was resolved, go to the next thing
if ($middlewareObject === false) {
continue;
}

// This is the way that v3 handles output buffering (which captures output correctly)
// This is the way that v3 handles output buffering (which captures output correctly)
$useV3OutputBuffering =
$this->response()->v2_output_buffering === false &&
$route->is_streamed === false;
Expand All @@ -441,16 +440,16 @@ protected function processMiddleware(Route $route, string $eventName): bool
}

// Here is the array callable $middlewareObject that we created earlier.
// It looks bizarre but it's really calling [ $class, $method ]($params)
// Which loosely translates to $class->$method($params)
// It looks bizarre but it's really calling [ $class, $method ]($params)
// Which loosely translates to $class->$method($params)
$middlewareResult = $middlewareObject($params);

if ($useV3OutputBuffering === true) {
$this->response()->write(ob_get_clean());
}

// If you return false in your middleware, it will halt the request
// and throw a 403 forbidden error by default.
// If you return false in your middleware, it will halt the request
// and throw a 403 forbidden error by default.
if ($middlewareResult === false) {
$atLeastOneMiddlewareFailed = true;
break;
Expand Down Expand Up @@ -579,7 +578,13 @@ public function _start(): void
if ($failedMiddlewareCheck === true) {
$this->halt(403, 'Forbidden', empty(getenv('PHPUNIT_TEST')));
} elseif ($dispatched === false) {
$this->notFound();
// Get the previous route and check if the method failed, but the URL was good.
$lastRouteExecuted = $router->executedRoute;
if ($lastRouteExecuted !== null && $lastRouteExecuted->matchUrl($request->url) === true && $lastRouteExecuted->matchMethod($request->method) === false) {
$this->halt(405, 'Method Not Allowed', empty(getenv('PHPUNIT_TEST')));
} else {
$this->notFound();
}
}
}

Expand Down Expand Up @@ -670,8 +675,8 @@ public function _group(string $pattern, callable $callback, array $group_middlew
* @param string $pattern URL pattern to match
* @param callable|string $callback Callback function or string class->method
* @param bool $pass_route Pass the matching route object to the callback
*
* @return Route
*
* @return Route
*/
public function _post(string $pattern, $callback, bool $pass_route = false, string $route_alias = ''): Route
{
Expand All @@ -684,8 +689,8 @@ public function _post(string $pattern, $callback, bool $pass_route = false, stri
* @param string $pattern URL pattern to match
* @param callable|string $callback Callback function or string class->method
* @param bool $pass_route Pass the matching route object to the callback
*
* @return Route
*
* @return Route
*/
public function _put(string $pattern, $callback, bool $pass_route = false, string $route_alias = ''): Route
{
Expand All @@ -698,12 +703,12 @@ public function _put(string $pattern, $callback, bool $pass_route = false, strin
* @param string $pattern URL pattern to match
* @param callable|string $callback Callback function or string class->method
* @param bool $pass_route Pass the matching route object to the callback
*
* @return Route
*
* @return Route
*/
public function _patch(string $pattern, $callback, bool $pass_route = false, string $route_alias = ''): Route
{
return $this->router()->map('PATCH ' . $pattern, $callback, $pass_route, $route_alias);
return $this->router()->map('PATCH ' . $pattern, $callback, $pass_route, $route_alias);
}

/**
Expand All @@ -712,8 +717,8 @@ public function _patch(string $pattern, $callback, bool $pass_route = false, str
* @param string $pattern URL pattern to match
* @param callable|string $callback Callback function or string class->method
* @param bool $pass_route Pass the matching route object to the callback
*
* @return Route
*
* @return Route
*/
public function _delete(string $pattern, $callback, bool $pass_route = false, string $route_alias = ''): Route
{
Expand Down
18 changes: 9 additions & 9 deletions flight/commands/RouteCommand.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,10 @@ public function execute()
{
$io = $this->app()->io();

if(isset($this->config['index_root']) === false) {
$io->error('index_root not set in .runway-config.json', true);
return;
}
if (isset($this->config['index_root']) === false) {
$io->error('index_root not set in .runway-config.json', true);
return;
}

$io->bold('Routes', true);

Expand All @@ -69,12 +69,12 @@ public function execute()
return preg_match("/^class@anonymous/", end($middleware_class_name)) ? 'Anonymous' : end($middleware_class_name);
}, $route->middleware);
} catch (\TypeError $e) {
$middlewares[] = 'Bad Middleware';
$middlewares[] = 'Bad Middleware';
} finally {
if(is_string($route->middleware) === true) {
$middlewares[] = $route->middleware;
}
}
if (is_string($route->middleware) === true) {
$middlewares[] = $route->middleware;
}
}
}

$arrayOfRoutes[] = [
Expand Down
53 changes: 33 additions & 20 deletions flight/net/Router.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class Router
/**
* The current route that is has been found and executed.
*/
protected ?Route $executedRoute = null;
public ?Route $executedRoute = null;

/**
* Pointer to current route.
Expand All @@ -42,21 +42,21 @@ class Router
/**
* When groups are used, this is mapped against all the routes
*/
protected string $group_prefix = '';
protected string $groupPrefix = '';

/**
* Group Middleware
*
* @var array<int,mixed>
*/
protected array $group_middlewares = [];
protected array $groupMiddlewares = [];

/**
* Allowed HTTP methods
*
* @var array<int, string>
*/
protected array $allowed_methods = ['GET', 'POST', 'PUT', 'PATCH', 'DELETE', 'HEAD', 'OPTIONS'];
protected array $allowedMethods = ['GET', 'POST', 'PUT', 'PATCH', 'DELETE', 'HEAD', 'OPTIONS'];

/**
* Gets mapped routes.
Expand Down Expand Up @@ -93,7 +93,7 @@ public function map(string $pattern, $callback, bool $pass_route = false, string
// Flight::route('', function() {});
// }
// Keep the space so that it can execute the below code normally
if ($this->group_prefix !== '') {
if ($this->groupPrefix !== '') {
$url = ltrim($pattern);
} else {
$url = trim($pattern);
Expand All @@ -113,14 +113,14 @@ public function map(string $pattern, $callback, bool $pass_route = false, string
}

// And this finishes it off.
if ($this->group_prefix !== '') {
$url = rtrim($this->group_prefix . $url);
if ($this->groupPrefix !== '') {
$url = rtrim($this->groupPrefix . $url);
}

$route = new Route($url, $callback, $methods, $pass_route, $route_alias);

// to handle group middleware
foreach ($this->group_middlewares as $gm) {
foreach ($this->groupMiddlewares as $gm) {
$route->addMiddleware($gm);
}

Expand Down Expand Up @@ -197,20 +197,20 @@ public function delete(string $pattern, $callback, bool $pass_route = false, str
/**
* Group together a set of routes
*
* @param string $group_prefix group URL prefix (such as /api/v1)
* @param string $groupPrefix group URL prefix (such as /api/v1)
* @param callable $callback The necessary calling that holds the Router class
* @param array<int, callable|object> $group_middlewares
* @param array<int, callable|object> $groupMiddlewares
* The middlewares to be applied to the group. Example: `[$middleware1, $middleware2]`
*/
public function group(string $group_prefix, callable $callback, array $group_middlewares = []): void
public function group(string $groupPrefix, callable $callback, array $groupMiddlewares = []): void
{
$old_group_prefix = $this->group_prefix;
$old_group_middlewares = $this->group_middlewares;
$this->group_prefix .= $group_prefix;
$this->group_middlewares = array_merge($this->group_middlewares, $group_middlewares);
$oldGroupPrefix = $this->groupPrefix;
$oldGroupMiddlewares = $this->groupMiddlewares;
$this->groupPrefix .= $groupPrefix;
$this->groupMiddlewares = array_merge($this->groupMiddlewares, $groupMiddlewares);
$callback($this);
$this->group_prefix = $old_group_prefix;
$this->group_middlewares = $old_group_middlewares;
$this->groupPrefix = $oldGroupPrefix;
$this->groupMiddlewares = $oldGroupMiddlewares;
}

/**
Expand All @@ -221,9 +221,14 @@ public function group(string $group_prefix, callable $callback, array $group_mid
public function route(Request $request)
{
while ($route = $this->current()) {
if ($route->matchMethod($request->method) && $route->matchUrl($request->url, $this->case_sensitive)) {
$urlMatches = $route->matchUrl($request->url, $this->case_sensitive);
$methodMatches = $route->matchMethod($request->method);
if ($urlMatches === true && $methodMatches === true) {
$this->executedRoute = $route;
return $route;
// capture the route but don't execute it. We'll use this in Engine->start() to throw a 405
} elseif ($urlMatches === true && $methodMatches === false) {
$this->executedRoute = $route;
}
$this->next();
}
Expand Down Expand Up @@ -299,19 +304,27 @@ public function current()
return $this->routes[$this->index] ?? false;
}

/**
* Gets the previous route.
*/
public function previous(): void
{
--$this->index;
}

/**
* Gets the next route.
*/
public function next(): void
{
$this->index++;
++$this->index;
}

/**
* Reset to the first route.
*/
public function reset(): void
{
$this->index = 0;
$this->rewind();
}
}
43 changes: 43 additions & 0 deletions tests/EngineTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -899,4 +899,47 @@ public function testContainerPsr11MethodNotFound() {

$engine->start();
}

public function testRouteFoundButBadMethod() {
$engine = new class extends Engine {
public function getLoader()
{
return $this->loader;
}
};
// doing this so we can overwrite some parts of the response
$engine->getLoader()->register('response', function () {
return new class extends Response {
public function setRealHeader(
string $header_string,
bool $replace = true,
int $response_code = 0
): self {
return $this;
}
};
});

$engine->route('POST /path1/@id', function ($id) {
echo 'OK' . $id;
});

$engine->route('GET /path2/@id', function ($id) {
echo 'OK' . $id;
});

$engine->route('PATCH /path3/@id', function ($id) {
echo 'OK' . $id;
});

$engine->request()->url = '/path1/123';
$engine->request()->method = 'GET';

$engine->start();

$this->expectOutputString('Method Not Allowed');
$this->assertEquals(405, $engine->response()->status());
$this->assertEquals('Method Not Allowed', $engine->response()->getBody());
}

}
Loading

0 comments on commit 27718cf

Please sign in to comment.