Fix handling of default values for placeholders.

In order to make default placeholders work, we had to modify the fit and path outline
routines to ignore them.  That also necessitated switching back to the original
outline/ancestors logic from Drupal 7, which with a very slight tweak to the masks
and '/'-prefix on paths still works just as it should.

core/lib/Drupal/Core/Routing/PathMatcher.php

@@ -79,20 +79,26 @@ public function matchRequestPartial(Request $request) {
    *   An array of outlines that could match the specified path parts.
    */
   public function getCandidateOutlines(array $parts) {
-
     $number_parts = count($parts);
+    $ancestors = array();
     $length =  $number_parts - 1;
     $end = (1 << $number_parts) - 1;
-    $candidates = array();
-
-    $start = pow($number_parts-1, 2);
 
     // The highest possible mask is a 1 bit for every part of the path. We will
     // check every value down from there to generate a possible outline.
-    $masks = range($end, $start);
+    $masks = range($end, pow($number_parts - 1, 2));
 
+    // Only examine patterns that actually exist as router items (the masks).
     foreach ($masks as $i) {
-      $current = '/';
+      if ($i > $end) {
+        // Only look at masks that are not longer than the path of interest.
+        continue;
+      }
+      elseif ($i < (1 << $length)) {
+        // We have exhausted the masks of a given length, so decrease the length.
+        --$length;
+      }
+      $current = '';
       for ($j = $length; $j >= 0; $j--) {
         // Check the bit on the $j offset.
         if ($i & (1 << $j)) {
@@ -108,10 +114,9 @@ public function getCandidateOutlines(array $parts) {
           $current .= '/';
         }
       }
-      $candidates[] = $current;
+      $ancestors[] = '/' . $current;
     }
-
-    return $candidates;
+    return $ancestors;
   }
 }
 

core/lib/Drupal/Core/Routing/RouteCompiler.php

@@ -28,9 +28,11 @@ class RouteCompiler implements RouteCompilerInterface {
     */
   public function compile(Route $route) {
 
-    $fit = $this->getFit($route->getPattern());
+    $stripped_path = $this->getPathWithoutDefaults($route);
 
-    $pattern_outline = $this->getPatternOutline($route->getPattern());
+    $fit = $this->getFit($stripped_path);
+
+    $pattern_outline = $this->getPatternOutline($stripped_path);
 
     $num_parts = count(explode('/', trim($pattern_outline, '/')));
 
@@ -159,10 +161,10 @@ private function computeRegexp(array $tokens, $index, $firstOptional) {
    * Returns the pattern outline.
    *
    * The pattern outline is the path pattern but normalized so that all
-   * placeholders are equal strings.
+   * placeholders are equal strings and default values are removed.
    *
    * @param string $path
-   *   The path pattern to normalize to an outline.
+   *   The path for which we want the normalized outline.
    *
    * @return string
    *   The path pattern outline.
@@ -181,7 +183,6 @@ public function getPatternOutline($path) {
    *   The fitness of the path, as an integer.
    */
   public function getFit($path) {
-
     $parts = explode('/', trim($path, '/'), static::MAX_PARTS);
     $number_parts = count($parts);
     // We store the highest index of parts here to save some work in the fit
@@ -197,5 +198,34 @@ public function getFit($path) {
 
     return $fit;
   }
+
+  /**
+   * Returns the path of the route, without placeholders with a default value.
+   *
+   * When computing the path outline and fit, we want to skip default-value
+   * placeholders.  If we didn't, the path would never match.  Note that this
+   * only works for placeholders at the end of the path. Infix placeholders
+   * with default values don't make sense anyway, so that should not be a
+   * problem.
+   *
+   * @param Route $route
+   *
+   * @return string
+   *   The path string, stripped of placeholders that have default values.
+   */
+  protected function getPathWithoutDefaults(Route $route) {
+    $path = $route->getPattern();
+    $defaults = $route->getDefaults();
+
+    // Remove placeholders with default values from the outline, so that they
+    // will still match.
+    $remove = array_map(function($a) {
+      return '/{' . $a . '}';
+    }, array_keys($defaults));
+    $path = str_replace($remove, '', $path);
+
+    return $path;
+  }
+
 }
 

core/modules/system/lib/Drupal/system/Tests/Routing/PathMatcherTest.php

@@ -155,8 +155,7 @@ function testOutlinePathMatchDefaults() {
       // All of the matching paths have the correct pattern.
       foreach ($routes as $route) {
         $compiled = $route->compile();
-        debug($compiled->getPatternOutline());
-        $this->assertEqual($route->compile()->getPatternOutline(), '/path/path/%', 'Found path has correct pattern');
+        $this->assertEqual($route->compile()->getPatternOutline(), '/some/path', 'Found path has correct pattern');
       }
 
       $this->assertEqual(count($routes->all()), 1, 'The correct number of routes was found.');
@@ -167,6 +166,85 @@ function testOutlinePathMatchDefaults() {
     }
   }
 
+  /**
+   * Confirms that we can find routes whose pattern would match the request.
+   */
+  function testOutlinePathMatchDefaultsCollision() {
+    $connection = Database::getConnection();
+    $matcher = new PathMatcher($connection, 'test_routes');
+
+    $this->fixtures->createTables($connection);
+
+    $collection = new RouteCollection();
+    $collection->add('poink', new Route('/some/path/{value}', array(
+      'value' => 'poink',
+    )));
+    $collection->add('narf', new Route('/some/path/here'));
+
+    $dumper = new MatcherDumper($connection, 'test_routes');
+    $dumper->addRoutes($collection);
+    $dumper->dump();
+
+    $path = '/some/path';
+
+    $request = Request::create($path, 'GET');
+
+    try {
+      $routes = $matcher->matchRequestPartial($request);
+
+      // All of the matching paths have the correct pattern.
+      foreach ($routes as $route) {
+        $compiled = $route->compile();
+        $this->assertEqual($route->compile()->getPatternOutline(), '/some/path', 'Found path has correct pattern');
+      }
+
+      $this->assertEqual(count($routes->all()), 1, 'The correct number of routes was found.');
+      $this->assertNotNull($routes->get('poink'), 'The first matching route was found.');
+    }
+    catch (ResourceNotFoundException $e) {
+      $this->fail('No matching route found with default argument value.');
+    }
+  }
+
+  /**
+   * Confirms that we can find routes whose pattern would match the request.
+   */
+  function testOutlinePathMatchDefaultsCollision2() {
+    $connection = Database::getConnection();
+    $matcher = new PathMatcher($connection, 'test_routes');
+
+    $this->fixtures->createTables($connection);
+
+    $collection = new RouteCollection();
+    $collection->add('poink', new Route('/some/path/{value}', array(
+      'value' => 'poink',
+    )));
+    $collection->add('narf', new Route('/some/path/here'));
+
+    $dumper = new MatcherDumper($connection, 'test_routes');
+    $dumper->addRoutes($collection);
+    $dumper->dump();
+
+    $path = '/some/path/here';
+
+    $request = Request::create($path, 'GET');
+
+    try {
+      $routes = $matcher->matchRequestPartial($request);
+
+      // All of the matching paths have the correct pattern.
+      foreach ($routes as $route) {
+        $this->assertEqual($route->compile()->getPatternOutline(), '/some/path/here', 'Found path has correct pattern');
+      }
+
+      $this->assertEqual(count($routes->all()), 1, 'The correct number of routes was found.');
+      $this->assertNotNull($routes->get('narf'), 'The first matching route was found.');
+    }
+    catch (ResourceNotFoundException $e) {
+      $this->fail('No matching route found with default argument value.');
+    }
+  }
+
   /**
    * Confirm that an exception is thrown when no matching path is found.
    */

core/modules/system/lib/Drupal/system/Tests/Routing/RouteTest.php

@@ -28,6 +28,9 @@ function setUp() {
     parent::setUp();
   }
 
+  /**
+   * Confirms that a route compiles properly with the necessary data.
+   */
   public function testCompilation() {
     $route = new Route('/test/{something}/more');
     $route->setOption('compiler_class', 'Drupal\Core\Routing\RouteCompiler');
@@ -38,4 +41,21 @@ public function testCompilation() {
     $this->assertEqual($compiled->getPatternOutline(), '/test/%/more', 'The pattern outline was correct.');
   }
 
+  /**
+   * Confirms that a compiled route with default values has the correct outline.
+   */
+  public function testCompilationDefaultValue() {
+    // Because "here" has a default value, it should not factor into the
+    // outline or the fitness.
+    $route = new Route('/test/{something}/more/{here}', array(
+      'here' => 'there',
+    ));
+    $route->setOption('compiler_class', 'Drupal\Core\Routing\RouteCompiler');
+    $compiled = $route->compile();
+
+    $this->assertEqual($route, $compiled->getRoute(), 'Compiled route has the correct route object.');
+    $this->assertEqual($compiled->getFit(), 5 /* That's 101 binary*/, 'The fit was correct.');
+    $this->assertEqual($compiled->getPatternOutline(), '/test/%/more', 'The pattern outline was correct.');
+  }
+
 }