[Routing] UrlGenerator: add a method 'hasRoute' to check whether a route exists #19273

voronkovich · 2016-07-02T18:07:49Z

Q	A
Branch?	3.4
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	-
License	MIT
Doc PR	-

This method is usefull when you want just to check whether a route exists:

if ($router->hasRoute('homepage')) {
    // ...
}

See #19270 and symfony/symfony-docs#6710

wouterj · 2016-07-02T18:14:45Z

src/Symfony/Component/Routing/Generator/UrlGeneratorInterface.php

+     *
+     * @return bool
+     */
+    public function hasRoute($name);


because of backwards compatibility breaks, it cannot be added to the interface before Symfony 4.0.

In any case, I would not add it even in 4.0 as this does not belongs to a generator interface IMHO.

Hmm, indeed, it would make more sense in the Router interface.

@wouterj, @fabpot I've removed the hasRoute method from the UrlGeneratorInterface

unkind · 2016-07-02T22:20:16Z

Can you describe use-case when you need optional routes?

From my point of view, optional routes make code more fragile and unpredictable.

wouterj · 2016-07-02T22:22:32Z

@unkind imagine loading routes from a database (e.g. using Symfony CMF's DynamicRouter). This is one of the core concepts of a CMS (and thus a core concept of all CMSes built on Symfony).

voronkovich · 2016-07-02T22:48:29Z

@unkind, For example, I want to automatically create CRUD buttons for resources like 'products', 'options', and etc:

$resource = 'product';
...
$actions = [];
foreach ([ 'view', 'edit', 'delete' ] as $action) {
    if ($router->hasRoute($resource . '_' . $action)) {
        $actions[$action] = $router->generate($resource . '_' . $action, [ 'id' => $resourceId ]);
    }
}

unkind · 2016-07-02T22:55:14Z

src/Symfony/Component/Routing/Router.php

+     */
+    public function hasRoute($name)
+    {
+        return $this->getGenerator()->hasRoute($name);


Since the interface doesn't have this method anymore, you shouldn't directly use it.
getGenerator() may return different implementation.

@unkind, you're rigth! I think we could add a method availability checking:

public function hasRoute($name) { if (!method_exists($this->getGenerator(), 'hasRoute')) { throw new \RuntimeException("Current implementation of an url generator doesn't support this method."); } return $this->getGenerator()->hasRoute($name); }

It's terrible IMO.
It's clear that new interface is missing.

unkind · 2016-07-02T23:06:52Z

@wouterj does it make sense to make separate contract DynamicRouter with hasRoute method then?

This behaviour is specific for dynamic routers, but in general it's not great.

voronkovich · 2016-07-02T23:27:55Z

@unkind I think that the word "dynamic" means nothing in this case. IMO the word "dynamic" means that we can to add new routes at the runtime. But we just want to check a route existence.

unkind · 2016-07-02T23:34:18Z

Maybe "dynamic" is not good naming for this case, but I failed to find better one. RoutePresenceChecker?

voronkovich · 2016-07-03T00:04:32Z

@unkind RoutePresenceCheckableInterface?

voronkovich · 2016-07-03T17:55:47Z

@unkind, I've added the RoutePresenceCheckableInterface

unkind · 2016-07-03T19:24:07Z

I'm sorry for confusion, but my idea is to make truly separate service which has nothing to do with Router class.
If Router can't guarantee implementation of the hasRoute, then this method shouldn't be there.
Anyway, wait for Symfony team comments, I'm just casual observer.

Tobion · 2016-07-04T10:50:11Z

Seems strange that only the generator implements hasRoute but the matcher doesn't.

voronkovich · 2016-07-04T16:28:48Z

@Tobion, the url matche is dumped to class with match method like this:

public function match($pathinfo)
{
    $allow = array();
    $pathinfo = rawurldecode($pathinfo);
    $context = $this->context;
    $request = $this->request;

    if (0 === strpos($pathinfo, '/_')) {

        // _wdt
       if (0 === strpos($pathinfo, '/_wdt') && preg_match('#^/_wdt/(?P<token>[^/]++)$#s', $pathinfo, $matches)) {
            return $this->mergeDefaults(array_replace($matches, array('_route' => '_wdt')), array ('_controller' => 'web_profiler.controller.profiler:toolbarAction'));
        }

        if (0 === strpos($pathinfo, '/_profiler')) {

            // _profiler_home
            if (rtrim($pathinfo, '/') === '/_profiler') {
                 if (substr($pathinfo, -1) !== '/') {
                     return $this->redirect($pathinfo.'/', '_profiler_home');
                 }

                return array (  '_controller' => 'web_profiler.controller.profiler:homeAction',  '_route' => '_profiler_home',);
            }
        }

    // ...
}

So, we haven't access to the route names after matcher was dumped.

fabpot · 2017-07-06T09:23:24Z

src/Symfony/Component/Routing/Generator/UrlGenerator.php

+use Symfony\Component\Routing\Exception\RouteNotFoundException;
+use Symfony\Component\Routing\RequestContext;
+use Symfony\Component\Routing\RouteCollection;
+use Symfony\Component\Routing\RoutePresenceCheckableInterface;


The order change of use statements must be reverted.

@fabpot, Fixed.

javiereguiluz · 2017-08-01T07:10:52Z

src/Symfony/Component/Routing/RoutePresenceCheckableInterface.php

+ *
+ * @author Oleg Voronkovich <oleg-voronkovich@yandex.ru>
+ */
+interface RoutePresenceCheckableInterface


Can we add this method instead in Symfony 4.0 and not create this RoutePresenceCheckableInterface? It looks overkill to me.

@javiereguiluz, So, to which interface the hasRoute method should be moved?

any hint @javiereguiluz ?

I don't know about the internal details, but I'd like to call this method on the router object:

// old Symfony public function indexAction() { if ($this->get('router')->hasRoute('special_route')) { // ... } } // new Symfony public function indexAction(RouterInterface $router) { if ($router->hasRoute('special_route')) { // ... } }

@javiereguiluz, the router implements the RoutePresenceCheckableInterface interface, so your code should work fine.

Yes. I was suggesting to add it in UrlGeneratorInterface (only in Sf 4.0) to avoid creating micro-interfaces.

nicolas-grekas · 2018-02-19T10:16:35Z

I'm really no fan of the extra interface, so 👎 to add it in the Routing component.

Instead, I'd propose to add a new twig has_route function (because the same use case exists in twig templates), and add a ControllerTrait::hasRoute() method. For other use cases, the implementation is simple (try/catch) so I ppl could easily create their own helper if needed.

nicolas-grekas · 2018-03-23T10:14:36Z

What about making this interface a bit more capable, to cover #19302 also?
Isn't the compiled data structure already containing all the required info to recreate the original route collection? We could add a getRouteCollection method here, and make its implementation "clever" (ie using just-in-time Route instantiantion and generators?

voronkovich · 2018-03-23T13:49:48Z

@nicolas-grekas, The dumper doesn't dump some matching-related data like route conditions and maybe something else.

nicolas-grekas · 2018-03-23T14:15:21Z

This doesn't have to be true tomorrow :)

fabpot · 2018-09-04T15:42:40Z

@nicolas-grekas How do we move forward here?

nicolas-grekas · 2018-09-24T07:45:27Z

Instead, I'd propose to add a new twig has_route function (because the same use case exists in twig templates), and add a ControllerTrait::hasRoute() method. For other use cases, the implementation is simple (try/catch) so I ppl could easily create their own helper if needed.

@voronkovich would that work for you? up to implement it?

(let's forget about #19273 (comment))

voronkovich added 3 commits July 2, 2016 20:30

[Routing] UrlGenerator: add 'hasRoute' method

38e28a8

Add tests for url generator dumper

f3cd05f

Fix typo

24beded

carsonbot added Status: Needs Review Routing Feature labels Jul 2, 2016

wouterj reviewed Jul 2, 2016
View reviewed changes

Remove 'hasRoute' method from UrlGeneratorInterface

d119285

unkind reviewed Jul 2, 2016
View reviewed changes

Add RoutePresenceCheckableInterface

4185363

voronkovich mentioned this pull request Jul 6, 2016

[Routing][FrameworkBundle] Add route collection caching #19302

Closed

nicolas-grekas added this to the 3.x milestone Dec 6, 2016

fabpot requested changes Jul 6, 2017

View reviewed changes

Revert use statements order

48bb977

voronkovich changed the base branch from master to 3.4 July 6, 2017 17:02

javiereguiluz reviewed Aug 1, 2017

View reviewed changes

nicolas-grekas modified the milestones: 3.4, 4.1 Oct 8, 2017

nicolas-grekas modified the milestones: 4.1, next Apr 20, 2018

nicolas-grekas mentioned this pull request Sep 24, 2018

[FrameworkBundle][TwigBridge] add a method to check if a route exists from controllers and twig templates #28578

Closed

voronkovich closed this Sep 24, 2018

nicolas-grekas modified the milestones: next, 4.2 Nov 1, 2018

Uh oh!

[Routing] UrlGenerator: add a method 'hasRoute' to check whether a route exists #19273

[Routing] UrlGenerator: add a method 'hasRoute' to check whether a route exists #19273

Uh oh!

Conversation

voronkovich commented Jul 2, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

unkind commented Jul 2, 2016

Uh oh!

wouterj commented Jul 2, 2016

Uh oh!

voronkovich commented Jul 2, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

unkind commented Jul 2, 2016

Uh oh!

voronkovich commented Jul 2, 2016

Uh oh!

unkind commented Jul 2, 2016

Uh oh!

voronkovich commented Jul 3, 2016

Uh oh!

voronkovich commented Jul 3, 2016

Uh oh!

unkind commented Jul 3, 2016

Uh oh!

Tobion commented Jul 4, 2016

Uh oh!

voronkovich commented Jul 4, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nicolas-grekas commented Feb 19, 2018

Uh oh!

nicolas-grekas commented Mar 23, 2018

Uh oh!

voronkovich commented Mar 23, 2018

Uh oh!

nicolas-grekas commented Mar 23, 2018 via email

Uh oh!

fabpot commented Sep 4, 2018

Uh oh!

nicolas-grekas commented Sep 24, 2018

Uh oh!

Uh oh!

voronkovich commented Jul 2, 2016 •

edited

Loading