fix: filters are executed when controller does not exist with Auto Routing (Legacy).

[ping-yee]

Description
See #7205

Checklist:

• Securely signed commits
• Component(s) with PHPDoc blocks, only if necessary or adds value
• Unit testing, with >80% coverage
• User guide updated
• Conforms to style guide

[kenjis]

@ping-yee The commit f2f9638 does not add any feature.
The commit message prefix should be test: .

[ping-yee]

@kenjis Got it and I have modified that.

[kenjis]

@ping-yee I think AutoRouter::getRoute() should throw PageNotFoundException when the controller is not found.
It already throws PageNotFoundException to invalid name:

CodeIgniter4/system/Router/AutoRouter.php

Lines 99 to 101 in f35f956

	if (! $this->isValidSegment($controllerName)) {
	throw new PageNotFoundException($this->controller . ' is not a valid controller name');
	}

[ping-yee]

@kenjis Seems that it will have some problem occur after add the pageNotFoundException into AutoRouter::getRoute().

[kenjis]

Do you mean the following PHPUnit errors?
It is a breaking change, so some existing tests fail.
This change is a bug fix but not urgent. It may be better to go to 4.5 because of the impact.

There were 16 errors:

1) CodeIgniter\Router\RouteCollectionTest::testAutoRoutesControllerNameReturnsFQCN with data set "with \ prefix" ('\App\Controllers')
CodeIgniter\Exceptions\PageNotFoundException: Controller or its method is not found: \App\Controllers\Product::index

/home/runner/work/CodeIgniter4/CodeIgniter4/system/Router/AutoRouter.php:179
/home/runner/work/CodeIgniter4/CodeIgniter4/system/Router/Router.php:514
/home/runner/work/CodeIgniter4/CodeIgniter4/system/Router/Router.php:210
/home/runner/work/CodeIgniter4/CodeIgniter4/tests/system/Router/RouteCollectionTest.php:1684
phpvfscomposer:///home/runner/work/CodeIgniter4/CodeIgniter4/vendor/phpunit/phpunit/phpunit:106

2) CodeIgniter\Router\RouteCollectionTest::testAutoRoutesControllerNameReturnsFQCN with data set "without \ prefix" ('App\Controllers')
CodeIgniter\Exceptions\PageNotFoundException: Controller or its method is not found: \App\Controllers\Product::index

/home/runner/work/CodeIgniter4/CodeIgniter4/system/Router/AutoRouter.php:179
/home/runner/work/CodeIgniter4/CodeIgniter4/system/Router/Router.php:514
/home/runner/work/CodeIgniter4/CodeIgniter4/system/Router/Router.php:210
/home/runner/work/CodeIgniter4/CodeIgniter4/tests/system/Router/RouteCollectionTest.php:1684
phpvfscomposer:///home/runner/work/CodeIgniter4/CodeIgniter4/vendor/phpunit/phpunit/phpunit:106

3) CodeIgniter\Router\RouterTest::testAutoRouteFindsDefaultControllerAndMethod
CodeIgniter\Exceptions\PageNotFoundException: Controller or its method is not found: Test::test

/home/runner/work/CodeIgniter4/CodeIgniter4/system/Router/AutoRouter.php:179
/home/runner/work/CodeIgniter4/CodeIgniter4/system/Router/Router.php:514
/home/runner/work/CodeIgniter4/CodeIgniter4/tests/system/Router/RouterTest.php:204
phpvfscomposer:///home/runner/work/CodeIgniter4/CodeIgniter4/vendor/phpunit/phpunit/phpunit:106

4) CodeIgniter\Router\RouterTest::testAutoRouteFindsControllerWithFileAndMethod
CodeIgniter\Exceptions\PageNotFoundException: Controller or its method is not found: MyController::someMethod

/home/runner/work/CodeIgniter4/CodeIgniter4/system/Router/AutoRouter.php:179
/home/runner/work/CodeIgniter4/CodeIgniter4/system/Router/Router.php:514
/home/runner/work/CodeIgniter4/CodeIgniter4/tests/system/Router/RouterTest.php:215
phpvfscomposer:///home/runner/work/CodeIgniter4/CodeIgniter4/vendor/phpunit/phpunit/phpunit:106

5) CodeIgniter\Router\RouterTest::testAutoRouteFindsControllerWithFile
ErrorException: file_put_contents(/home/runner/work/CodeIgniter4/CodeIgniter4/app/Controllers/Φ): failed to open stream: Is a directory

/home/runner/work/CodeIgniter4/CodeIgniter4/tests/system/Router/RouterTest.php:342
phpvfscomposer:///home/runner/work/CodeIgniter4/CodeIgniter4/vendor/phpunit/phpunit/phpunit:106

13) CodeIgniter\Router\RouterTest::testTranslateURIDashesForAutoRoute
CodeIgniter\Exceptions\PageNotFoundException: Controller or its method is not found: Admin-user::show-list

/home/runner/work/CodeIgniter4/CodeIgniter4/system/Router/AutoRouter.php:179
/home/runner/work/CodeIgniter4/CodeIgniter4/system/Router/Router.php:514
/home/runner/work/CodeIgniter4/CodeIgniter4/tests/system/Router/RouterTest.php:740
phpvfscomposer:///home/runner/work/CodeIgniter4/CodeIgniter4/vendor/phpunit/phpunit/phpunit:106

14) CodeIgniter\Router\RouterTest::testAutoRouteMatchesZeroParams
CodeIgniter\Exceptions\PageNotFoundException: Controller or its method is not found: MyController::someMethod

/home/runner/work/CodeIgniter4/CodeIgniter4/system/Router/AutoRouter.php:179
/home/runner/work/CodeIgniter4/CodeIgniter4/system/Router/Router.php:514
/home/runner/work/CodeIgniter4/CodeIgniter4/tests/system/Router/RouterTest.php:754
phpvfscomposer:///home/runner/work/CodeIgniter4/CodeIgniter4/vendor/phpunit/phpunit/phpunit:106

15) CodeIgniter\Router\RouterTest::testAutoRouteMethodEmpty
CodeIgniter\Exceptions\PageNotFoundException: Controller or its method is not found: Home::index

/home/runner/work/CodeIgniter4/CodeIgniter4/system/Router/AutoRouter.php:179
/home/runner/work/CodeIgniter4/CodeIgniter4/system/Router/Router.php:514
/home/runner/work/CodeIgniter4/CodeIgniter4/system/Router/Router.php:210
/home/runner/work/CodeIgniter4/CodeIgniter4/tests/system/Router/RouterTest.php:775
phpvfscomposer:///home/runner/work/CodeIgniter4/CodeIgniter4/vendor/phpunit/phpunit/phpunit:106

16) CodeIgniter\Router\RouterTest::testRouterPriorDirectory
CodeIgniter\Exceptions\PageNotFoundException: Controller or its method is not found: Some_controller::some_method

/home/runner/work/CodeIgniter4/CodeIgniter4/system/Router/AutoRouter.php:179
/home/runner/work/CodeIgniter4/CodeIgniter4/system/Router/Router.php:514
/home/runner/work/CodeIgniter4/CodeIgniter4/system/Router/Router.php:210
/home/runner/work/CodeIgniter4/CodeIgniter4/tests/system/Router/RouterTest.php:826
phpvfscomposer:///home/runner/work/CodeIgniter4/CodeIgniter4/vendor/phpunit/phpunit/phpunit:106

--

https://github.com/codeigniter4/CodeIgniter4/actions/runs/6159447007/job/16714289271?pr=7925

[kenjis]

For example, see the following test.
In this test, $router->handle() is expected to set controller/method names from the URI path,
even if the controller does not exist.
But I don't see a reason that $router finds controllers that do not exist.

RouteCollectionTest:

    /**
     * @dataProvider provideRouteDefaultNamespace
     *
     * @param mixed $namespace
     */
    public function testAutoRoutesControllerNameReturnsFQCN($namespace): void
    {
        $routes = $this->getCollector();
        $routes->setAutoRoute(true);
        $routes->setDefaultNamespace($namespace);

        $router = new Router($routes, Services::request());
        $router->handle('/product');

        $this->assertSame('\App\\Controllers\\Product', $router->controllerName());
    }

[github-actions]

👋 Hi, @ping-yee!

We detected conflicts in your PR against the base branch 🙊
You may want to sync 🔄 your branch with upstream!

Ref: Syncing Your Branch

[ping-yee]

In this test, $router->handle() is expected to set controller/method names from the URI path,
even if the controller does not exist.
But I don't see a reason that $router finds controllers that do not exist.

I am also feel confused about this and I wonder how these test cases work as we expect, like this case as below.

public function testAutoRouteFindsControllerWithSubfolder(): void
    {
        $this->collection->setAutoRoute(true);

        $router = new Router($this->collection, $this->request);

        mkdir(APPPATH . 'Controllers/Subfolder');

        $router->autoRoute('subfolder/myController/someMethod');

        rmdir(APPPATH . 'Controllers/Subfolder');

        $this->assertSame('MyController', $router->controllerName());
        $this->assertSame('someMethod', $router->methodName());
    }

It make a Subfolder folder but doesn't make a myController file and someMethod.
So what we expect is it only will throw the PageNotFound exception for now?

[kenjis]

That is a test to find the controller in a subfolder, so the controller should be present.
The test method is AutoRouteFindsControllerWithSubfolder.

In the test code, the controller file that should be prepared is not created because the test passes without a controller file.

[ping-yee]

In the test code, the controller file that should be prepared is not created because the test passes without a controller file.

Yes. And I also find that the testing used controllers has been prepared in the same directory.

https://github.com/codeigniter4/CodeIgniter4/tree/develop/tests/system/Router

But I think the controllers aren't being used because it's different method name between the test case and mock controller.

Or I have some misunderstandings.

public function testAutoRouteFindsControllerWithSubfolder(): void
    {
        $this->collection->setAutoRoute(true);

        $router = new Router($this->collection, $this->request);

        mkdir(APPPATH . 'Controllers/Subfolder');

        $router->autoRoute('subfolder/myController/someMethod');

        rmdir(APPPATH . 'Controllers/Subfolder');

        $this->assertSame('MyController', $router->controllerName());
        $this->assertSame('someMethod', $router->methodName());
    }

namespace CodeIgniter\Router\Controllers\Subfolder;

use CodeIgniter\Controller;

class Mycontroller extends Controller
{
    public function getSomemethod(): void
    {
    }
}

[ping-yee]

@kenjis I fix the problem that the controller isn't exist into few test cases at the latest commit, please review.
If the change is feasible, I will apply the change into the other test cases.

[kenjis]

Yes. And I also find that the testing used controllers has been prepared in the same directory.
https://github.com/codeigniter4/CodeIgniter4/tree/develop/tests/system/Router
But I think the controllers aren't being used because it's different method name between the test case and mock controller.

Yes, these controllers are for testing Auto Routing Improved.
But they can be used for Auto Routing Legacy as you wrote test code.

[kenjis]

@ping-yee Thank you. Your way seems to be okay.

It is better not to copy files during testing, but the Auto Routing Legacy has hard coded APPPATH . 'Controllers/',
so we must copy the controllers to app/Controllers now.

[kenjis]

Thank you!