Regex parameters requirements are not considered to differentiate routes with the same path

Here is a short test which tries to explain existing behavior.

Both routes do match /news/eu so one of them will be chosen. Maybe you have to implement some additional class implement RouteFilterInterface to have the exact controller you need.

The Symfony router will always choose the first matching route it finds.

Well, it finds the first matching route, we don't ensure order when we push and pull routes from the database.

The only problem is that we just sort routes by fit.

    $routes = $this->connection->query("SELECT name, route FROM {" . $this->connection->escapeTable($this->tableName) . "} WHERE pattern_outline IN (:patterns) ORDER BY fit DESC", array(
      ':patterns' => $ancestors,
    ))
      ->fetchAllKeyed()

I am open to adding another order by column to help with these cases if we can figure out a good one to use. "Order defined in the file" doesn't quite work in Drupal's case as well as it does in fullstack or Silex. :-/

Hm, actually, should we instead integrate the presence of regex requirements into calculating the "fitness" of a route? Right now it's just path placeholder based, but we could certainly adjust that if needed. Adding a regex makes a route "more specific", and we order most specific to least specific, so...

This bug is at least be major, I think.

IIRC, e.g. in #2068471: Normalize Controller/View-listener behavior with a Page object we were not able to rename a route, because the current matching algo ignores other route aspects, so another definition (of the same routing.yml file) suddenly came first.

I first wanted to agree with simply lumping this as a factor into 'fit', but on a second thought, I don't think that's going to be a reliable and maintainable approach → the final fitness would become a "magic" value and we'd also have to lump 'methods' and possibly other factors into that sum.

I first wanted to agree with simply lumping this as a factor into 'fit', but on a second thought, I don't think that's going to be a reliable and maintainable approach → the final fitness would become a "magic" value and we'd also have to lump 'methods' and possibly other factors into that sum.

An alternative could be to introduce a step / use RouteFilters to just sort the returned routes, so more specific ones comes later.

That's more work along the critical path. If we can put this logic into the provider, I'd rather do that.

Of course, the schema of the router table is an internal detail, not public (since the provider is swappable and we don't promise that it's SQL at all), so we can certainly do things like add a column for "has a regex", "has a method", "has a mime type", etc. Or even just a "number of restrictions" column. That's quite easy for us to do, I'd think, and would let us order by fitness, then the number of specifiers/restrictions.

We could also, on selected backends, bake the regex into the query itself. That's probably not worth doing for the default implementation, though; it just needs to work on all SQL backends, not be fully optimized.

@crell
So do you have any recommendations how/if we should move this issue forward? It seems indeed like an issue,
but can't we figure out on route compile time whether we need to do regex checking, which should basically no overhead at all? Maybe one function call by default + the calls to the regexes in case somebody decided to use them.

Here's an attempt to pick this up. It includes the test, rerolled and amended, from #1. Not sure if this is the appropriate way to do this sorting in the route provider or not.

The patch in #11 breaks certain local action links (eg, 'Add a custom block', etc) for some reason.

The fails were because views-provided routes had more specificity than the routes they replace (eg, node.add_page vs views.content.page_1--the latter had _method requirement).

This patch filters out the deprecated _method and _scheme requirements before sorting on requirements.

re-rolled with auto merge

Still a bug, but has not proven major over time.

Rerolled the patch.

1. +++ b/core/lib/Drupal/Core/Routing/RouteProvider.php
   @@ -371,6 +374,16 @@ protected function getRoutesByPath($path) {
   +      // @todo This won't be needed once Symfony 3 is in use.
   

   Do you mind filling a follow up so we don't forget about it?

2. +++ b/core/tests/Drupal/KernelTests/Core/Routing/RouteProviderTest.php
   @@ -722,6 +722,40 @@ public function testGetRoutesPaged() {
   +  public function testOutlineMatchPathWithRegex() {
   ...
   +    $collection = new RouteCollection();
   +    // Create routes with increasing order of specificity.
   +    $collection->add('test_a', new Route('/example/{foo}/{otherwise}'));
   +    $collection->add('test_c', new Route('/example/{foo}/{something}', [], ['foo' => 'foo']));
   +    $collection->add('test_b', new Route('/example/{foo}/{bar}', [], [
   +      'foo' => 'foo|bar',
   

   For a better test coverage it feels like it would be better to include access checkers as well, given that they have to be ignored in that counting process.

#35 fixes the remarks @dawehner had in #32. I don't really understand the routing system well enough to feel comfortable setting this issue to RTBC, but I couldn't find anything that I'd like to see changed.

The Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.