Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: [Auto Routing Improved] fallback to default method #7162

Merged
merged 5 commits into from
Jan 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -76,18 +76,37 @@ public function read(string $class, string $defaultController = 'Home', string $
);

if ($routeWithoutController !== []) {
// Route for the default controller.
$output = [...$output, ...$routeWithoutController];

continue;
}

$params = [];
$routeParams = '';
$refParams = $method->getParameters();

foreach ($refParams as $param) {
$required = true;
if ($param->isOptional()) {
$required = false;

$routeParams .= '[/..]';
} else {
$routeParams .= '/..';
}

// [variable_name => required?]
$params[$param->getName()] = $required;
}

// Route for the default method.
$output[] = [
'method' => $httpVerb,
'route' => $classInUri,
'route_params' => '',
'route_params' => $routeParams,
'handler' => '\\' . $classname . '::' . $methodName,
'params' => [],
'params' => $params,
];

continue;
Expand Down
41 changes: 32 additions & 9 deletions system/Router/AutoRouterImproved.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
namespace CodeIgniter\Router;

use CodeIgniter\Exceptions\PageNotFoundException;
use CodeIgniter\Router\Exceptions\MethodNotFoundException;
use ReflectionClass;
use ReflectionException;

Expand Down Expand Up @@ -168,17 +169,30 @@ public function getRoute(string $uri): array
'\\'
);

// Ensure routes registered via $routes->cli() are not accessible via web.
// Ensure the controller is not defined in routes.
$this->protectDefinedRoutes();

// Check _remap()
// Ensure the controller does not have _remap() method.
$this->checkRemap();

// Check parameters
// Check parameter count
try {
$this->checkParameters($uri);
} catch (ReflectionException $e) {
throw PageNotFoundException::forControllerNotFound($this->controller, $this->method);
} catch (MethodNotFoundException $e) {
// Fallback to the default method
if (! isset($methodSegment)) {
throw PageNotFoundException::forControllerNotFound($this->controller, $this->method);
}

array_unshift($this->params, $methodSegment);
$method = $this->method;
$this->method = $this->defaultMethod;

try {
$this->checkParameters($uri);
} catch (MethodNotFoundException $e) {
throw PageNotFoundException::forControllerNotFound($this->controller, $method);
}
}

return [$this->directory, $this->controller, $this->method, $this->params];
Expand All @@ -201,12 +215,21 @@ private function protectDefinedRoutes(): void

private function checkParameters(string $uri): void
{
$refClass = new ReflectionClass($this->controller);
$refMethod = $refClass->getMethod($this->method);
$refParams = $refMethod->getParameters();
try {
$refClass = new ReflectionClass($this->controller);
} catch (ReflectionException $e) {
throw PageNotFoundException::forControllerNotFound($this->controller, $this->method);
}

try {
$refMethod = $refClass->getMethod($this->method);
$refParams = $refMethod->getParameters();
} catch (ReflectionException $e) {
throw new MethodNotFoundException();
}

if (! $refMethod->isPublic()) {
throw PageNotFoundException::forMethodNotFound($this->method);
throw new MethodNotFoundException();
}

if (count($refParams) < count($this->params)) {
Expand Down
21 changes: 21 additions & 0 deletions system/Router/Exceptions/MethodNotFoundException.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<?php

/**
* This file is part of CodeIgniter 4 framework.
*
* (c) CodeIgniter Foundation <admin@codeigniter.com>
*
* For the full copyright and license information, please view
* the LICENSE file that was distributed with this source code.
*/

namespace CodeIgniter\Router\Exceptions;

use RuntimeException;

/**
* @internal
*/
final class MethodNotFoundException extends RuntimeException
{
}
2 changes: 1 addition & 1 deletion tests/_support/Controllers/Newautorouting.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

class Newautorouting extends Controller
{
public function getIndex()
public function getIndex(string $m = '')
{
return 'Hello';
}
Expand Down
2 changes: 1 addition & 1 deletion tests/system/Commands/RoutesTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ public function testRoutesCommandAutoRouteImproved()
| TRACE | testing | testing-index | \App\Controllers\TestController::index | | toolbar |
| CONNECT | testing | testing-index | \App\Controllers\TestController::index | | toolbar |
| CLI | testing | testing-index | \App\Controllers\TestController::index | | |
| GET(auto) | newautorouting | | \Tests\Support\Controllers\Newautorouting::getIndex | | toolbar |
| GET(auto) | newautorouting[/..] | | \Tests\Support\Controllers\Newautorouting::getIndex | | toolbar |
| POST(auto) | newautorouting/save/../..[/..] | | \Tests\Support\Controllers\Newautorouting::postSave | | toolbar |
+------------+--------------------------------+---------------+-----------------------------------------------------+----------------+---------------+
EOL;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public function testGetFilterMatches()
$expected = [
0 => [
'GET(auto)',
'newautorouting',
'newautorouting[/..]',
'',
'\\Tests\\Support\\Controllers\\Newautorouting::getIndex',
'',
Expand Down Expand Up @@ -90,7 +90,7 @@ public function testGetFilterDoesNotMatch()
$expected = [
0 => [
'GET(auto)',
'newautorouting',
'newautorouting[/..]',
'',
'\\Tests\\Support\\Controllers\\Newautorouting::getIndex',
'',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,11 @@ public function testRead()
0 => [
'method' => 'get',
'route' => 'newautorouting',
'route_params' => '',
'route_params' => '[/..]',
'handler' => '\Tests\Support\Controllers\Newautorouting::getIndex',
'params' => [],
'params' => [
'm' => false,
],
],
[
'method' => 'post',
Expand Down
13 changes: 13 additions & 0 deletions tests/system/Router/AutoRouterImprovedTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,19 @@ public function testAutoRouteFindsDefaultDashFolder()
$this->assertSame([], $params);
}

public function testAutoRouteFallbackToDefaultMethod()
{
$router = $this->createNewAutoRouter();

[$directory, $controller, $method, $params]
= $router->getRoute('index/15');

$this->assertNull($directory);
$this->assertSame('\\' . Index::class, $controller);
$this->assertSame('getIndex', $method);
$this->assertSame(['15'], $params);
}

public function testAutoRouteRejectsSingleDot()
{
$this->expectException(PageNotFoundException::class);
Expand Down
2 changes: 1 addition & 1 deletion tests/system/Router/Controllers/Index.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

class Index extends Controller
{
public function getIndex()
public function getIndex($p1 = '')
{
}

Expand Down
5 changes: 4 additions & 1 deletion user_guide_src/source/changelogs/v4.4.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,11 @@ Helpers and Functions

Others
======
- **View:** Added optional 2nd parameter ``$saveData`` on ``renderSection()`` to prevent from auto cleans the data after displaying. See :ref:`View Layouts <creating-a-layout>` for details.

- **View:** Added optional 2nd parameter ``$saveData`` on ``renderSection()`` to prevent from auto cleans the data after displaying. See :ref:`View Layouts <creating-a-layout>` for details.
- **Auto Routing (Improved)**: Now you can use URI without a method name like
``product/15`` where ``15`` is an arbitrary number.
See :ref:`controller-default-method-fallback` for details.

Message Changes
***************
Expand Down
23 changes: 23 additions & 0 deletions user_guide_src/source/incoming/controllers.rst
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,29 @@ Your method will be passed URI segments 3 and 4 (``'sandals'`` and ``'123'``):

.. literalinclude:: controllers/022.php

.. _controller-default-method-fallback:

Default Method Fallback
=======================

.. versionadded:: 4.4.0

If the controller method corresponding to the URI segment of the method name
does not exist, and if the default method is defined, the URI segments are
passed to the default method for execution.

.. literalinclude:: controllers/024.php

Load the following URL::

example.com/index.php/product/15/edit

The method will be passed URI segments 2 and 3 (``'15'`` and ``'edit'``):

.. note:: If there are more parameters in the URI than the method parameters,
Auto Routing (Improved) does not execute the method, and it results in 404
Not Found.

Defining a Default Controller
=============================

Expand Down
11 changes: 11 additions & 0 deletions user_guide_src/source/incoming/controllers/024.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?php

namespace App\Controllers;

class Product extends BaseController
{
public function getIndex($id = null, $action = '')
{
// ...
}
}