[WIP] Remove global routing

[enov]

This is a tentative PR to remove global routing, i.e. the use of Route::set, while relying only on injecting the routes when constructing Requests or overriding injected ones via set_routes.

This will solve #524 as well as go forward with #499.

I am also in favor of config routing. I think we had routing based on config files in Kohana 2? But never used Kohana 2 so I am not sure.

I am awaiting your comments regarding this.

Cheers!

[enov]

Ah, by the way, removing global routes makes HMVC Request::factory calls a bit painful, as routing needs to be re-injected. Hopefully config based routing would ease that.

[enov]

So I was wondering if you want me to continue working on this PR.

An implementation of loading the routes from config would be:

public static function load()
{
    $config = Kohana::$config->load('routes');

    $routes = array();
    foreach ($config as $name => $def)
    {
        $r = new Route(Arr::get($def, 'url'), Arr::get($def, 'regex'));
        $r->defaults(Arr::get($def, 'defaults'));

        if ($filter = Arr::get($def, 'filter')) {
            $r->filter($filter);
        }

        $routes[$name] = $r;
    }

    // return
    return $routes;
}

This could go in a separate class Route_Config::load or even in the Route class itself Route::load_from_config. It could have caching mechanism built-in, triggered by an additional flag parameter.

As a separate note, upgrading from Route::set to config should not be hard, as a small script with var_export at the end could create an array of existing routes.

[enov]

If you are too worried about the HMVC calls complexity, we can remake Request to load the routes, but this time from config, during its during construction, when a NULL is passed instead of injecting routes.

[rjd22]

@enov this looks good but what I'm missing is the guide on how routing will work from here on. Could you please update the guide and also add a upgrade 3.3 to 3.4 note?

[enov]

Thanks @rjd22.

I will add the guide and an upgrade note once it is complete, and there is consensus where it is heading.

Could you also confirm if you are in favor of config based routing? i.e to have routes.php in your config folder with potentially the following schema?

return [
    'default' => [
        'uri' => '(<controller>(/<action>(/<id>)))',
        'defaults' => [
                'controller' => 'welcome',
                'action'     => 'index',
        ],
    ],
];

[rjd22]

@enov I don't have a problem with config based routing. As long as you can also do it by injecting route objects into the router.

[acoulton]

@enov does this also remove the ability to do reverse routing (eg with Route::get)? I think that's quite widely used, certainly by us and in places that may be tricky to refactor quickly.

I'm less in favour of config-based routing, I think routes provided by modules shouldn't be automatically included but should be explicitly required by the application. Otherwise it can get annoying figuring out what urls are actually in use/valid/conflicting. So a module could define a routes.php with defaults but the application should include that file rather than it happen automatically. I think.

[enov]

does this also remove the ability to do reverse routing (eg with Route::get)?

It does not remove the ability of reverse routing, as Route::uri and Route::url are still there. At this moment it does remove Route::get as it reads from the global variable. In all cases we can bring it back:

public function get($name)
{
    $routes = Route::load();
    return $routes[$name];
}

If we do not bring back Route::get, you need to inject that to your views from your controllers $this->request->get_routes() or maybe reload the routes from config in your view-models. We should probably do some caching for multiple config reloads, in case Config does not do that already.

[enov]

I think routes provided by modules shouldn't be automatically included but should be explicitly required by the application.

There should be an additional $enabled property of Route. Modules should leave that to its FALSE, disabled default. You could then enable the route in your application folder's config file:

return [
    'module.route' => [
        'enabled' => TRUE,
    ],
]

Then you can have all your application routes visible in your config. The rules of config files play well in this case, as values are merged.

I also suggest to have an $index, $sort, or $order property. So that you can define the sorting order of your routes. You can also promote, demote your routes later in your application folder's config file. I can't seem to be able to choose a single name for it tough.

[enov]

I'm less in favour of config-based routing

Not pushing this hard on anybody, though I think it is to the right direction. Some frameworks are config based, while others define routes via HTTP-verbs-like-API methods.

In all cases, this needs some time to be fully implemented.

#524 complains about:

It forces modules to be loaded in the wrong (reverse) order.

ATM, modules are loaded top-to-bottom. This became a more explicit issue in v3.4, when @lenton rendered the core to become itself a standard module. We can leave this as a known issue, and go ahead with the general feeling to release Kohana 3.4.

[enov]

Closing this, as it might not go to 3.4, and can be scheduled for later.